Skip to content

Write the certificate where OpenSSH will find it#224

Merged
EthanHeilman merged 5 commits into
openpubkey:mainfrom
syskill:cert-filename
Jun 26, 2025
Merged

Write the certificate where OpenSSH will find it#224
EthanHeilman merged 5 commits into
openpubkey:mainfrom
syskill:cert-filename

Conversation

@syskill
Copy link
Copy Markdown
Contributor

@syskill syskill commented Jun 12, 2025

If given the filename of a private key, and no certificates have been specified, OpenSSH will append "-cert.pub" and try to load certificate data from there. (Cf. ssh(1) man page)

If given the filename of a private key, and no certificates have been
specified, OpenSSH will append "-cert.pub" and try to load certificate
data from there. (Cf. ssh(1) man page)

Signed-off-by: Ben Slusky <bslusky@smartling.com>
@EthanHeilman
Copy link
Copy Markdown
Member

Thanks for this!

Is this motivated by a bug where SSH isn't finding the public key? What opkssh and ssh commands are you running where this happens?

@syskill
Copy link
Copy Markdown
Contributor Author

syskill commented Jun 18, 2025

The motivation is to reduce the amount of explicit configuration of OpenSSH. As shown in the change in README.md, there is no need to specify the filename for certificate data, in addition to the private key, if the certificate data file is already at this path.

EthanHeilman and others added 3 commits June 18, 2025 13:04
Fix one more instance from another branch that was merged.

Signed-off-by: Ben Slusky <bslusky@smartling.com>
@syskill
Copy link
Copy Markdown
Contributor Author

syskill commented Jun 25, 2025

Forgot Signed-off-by for my one line fix, going to force-push to add it 😑

@syskill
Copy link
Copy Markdown
Contributor Author

syskill commented Jun 25, 2025

	if !strings.Contains(string(configContent), privKeyPath) {
		configContent = slices.Concat(
			[]byte("IdentityFile "+privKeyPath+"\n"),
			configContent,
		)
	}

This also shows how appending "-cert.pub" instead of just ".pub" to the public key file reduces the need for explicit configuration of OpenSSH. Without the filename change, this SSH config fragment (from #122) is not sufficient for OpenSSH to find the opkssh certificate data.

@EthanHeilman
Copy link
Copy Markdown
Member

Ok, I understand this change now. i'll review it sometime this week.

Are there are conflict with this PR #122 as it lets you configure a special opkssh ssh key directory?

@syskill
Copy link
Copy Markdown
Contributor Author

syskill commented Jun 25, 2025

There was one conflict, but I fixed it in my latest commit.

@EthanHeilman
Copy link
Copy Markdown
Member

The code changes look good. Let me take this for a test drive tomorrow and if I don't run into any issues I'll approve and merge it.

Thanks for this change, it will be nice to not have to specify the private key and the public key to log in.

@EthanHeilman EthanHeilman self-assigned this Jun 26, 2025
@EthanHeilman EthanHeilman added the enhancement New feature or request label Jun 26, 2025
Copy link
Copy Markdown
Member

@EthanHeilman EthanHeilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@EthanHeilman EthanHeilman merged commit f43221b into openpubkey:main Jun 26, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants