Skip to content

[v16] support non-RSA SSH signatures#45890

Merged
nklaassen merged 1 commit intobranch/v16from
nklaassen/v16/ssh-algs
Aug 27, 2024
Merged

[v16] support non-RSA SSH signatures#45890
nklaassen merged 1 commit intobranch/v16from
nklaassen/v16/ssh-algs

Conversation

@nklaassen
Copy link
Copy Markdown
Contributor

This PR adds support for signing SSH certs with non-RSA keys. We don't generate any non-RSA keys in v16, but it's possible for a user to import them into a CA manually.

changelog: add support for non-RSA SSH signatures with imported CA keys

// toRSASHA512Signer forces an ssh.MultiAlgorithmSigner into using
// "rsa-sha2-sha512" (instead of its SHA256 default).
func toRSASHA512Signer(signer ssh.Signer) ssh.Signer {
if signer.PublicKey().Type() != ssh.KeyAlgoRSA {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aren't we doing the same check in the caller of toRSASHA512Signer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we do in master, but not in branch/v16, which is the base branch of this PR. Current v16 is broken for all non-RSA keys (but we only ever generate RSA keys so its mostly fine). This is a much simplified version of #45887 which updates toRSASHA512Signer to a new sshSignerFromCryptoSigner which specifically selects signature algorithms for all key types and sizes

@nklaassen nklaassen changed the title support non-RSA SSH signatures [v16] support non-RSA SSH signatures Aug 27, 2024
@nklaassen nklaassen added this pull request to the merge queue Aug 27, 2024
Merged via the queue into branch/v16 with commit 48a772c Aug 27, 2024
@nklaassen nklaassen deleted the nklaassen/v16/ssh-algs branch August 27, 2024 18:25
@fheinecke fheinecke mentioned this pull request Apr 9, 2025
@fheinecke fheinecke mentioned this pull request Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants