Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/crypto/ssh: support RSA SHA-2 host key signatures #37278

Closed
Tracked by #49952
hansnielsen opened this issue Feb 18, 2020 · 37 comments
Closed
Tracked by #49952

x/crypto/ssh: support RSA SHA-2 host key signatures #37278

hansnielsen opened this issue Feb 18, 2020 · 37 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@hansnielsen
Copy link

What version of Go are you using (go version)?

$ go version
1.13.8

Version of x/crypto: 1d94cc7ab1c630336ab82ccb9c9cda72a875c382

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
n/a

What did you do?

I tried to connect to an OpenSSH 8.2 server with the ssh-rsa host key algorithm disabled on the server. I also tried to run an x/crypto/ssh server and connect from an OpenSSH client with ssh-rsa disabled. Lastly, I tried to sign a host certificate with ssh.Certificate.SignCert with a SHA-2 based signature.

What did you expect to see?

I expected the RSA host key and certificate to validate successfully with the new SHA-2 based signatures introduced in RFC 8332. I also expected to be able to sign host certificates and have them automatically received a SHA-2 based signature.

OpenSSH has already deprecated ssh-rsa (i.e. SHA-1 based) signatures in host certificates in version 8.2 because of safety reasons. They can still be used by the host key algorithm must be manually specified.

What did you see instead?

I was unable to connect either as a server or a client if ssh-rsa wasn't enabled while using RSA host keys or host certificates. I was able to sign a certificate with the AlgorithmSigner wrapper approach (i.e. by forcefully overriding Sign) proposed by @stoggi in #36261, but it's not a great experience for users.

@gopherbot gopherbot added this to the Unreleased milestone Feb 18, 2020
@hansnielsen
Copy link
Author

hansnielsen commented Feb 18, 2020

I spent some time hacking away at a solution for this and believe I have something largely ready to make a CL from: hansnielsen/golang-x-crypto@master...hans-rsa-sha2-support

The main change is that when given an ssh-rsa-type key or an RSA crypto.Signer (and not a custom ssh.Signer), the code will automatically register the ssh-rsa, rsa-sha2-256, and rsa-sha2-512 host key algorithms for use. This means that for users who haven't specified explicit algorithm preferences, they'll get the new SHA-2 based signatures just by updating. There shouldn't be any external-facing changes beyond algorithm support and the certificate signing choice mentioned below.

The approach OpenSSH chose for these new signature types is somewhat interesting: they have the same key type (ssh-rsa) but different signature algorithms. This makes it slightly tricky to integrate while keeping the existing tests working. There's a little more explicit special-casing of the RSA SHA-2 signature family than I'd like.

The one main choice I made in here (beyond just adding support for the new signature types) is that certificates now default to rsa-sha2-512 instead of ssh-rsa. I think given the already-deprecated nature of ssh-rsa plus the reasonable threat model for host certificates, this is the correct choice. The "wrapped AlgorithmSigner" approach can still be used to force an RSA SHA-1 signature and there's no need to encourage this.

Feedback welcome! If the approach in the code above looks good, I'll work on submitting a CL.

@ALTree ALTree added Proposal-Crypto Proposal related to crypto packages or other security issues NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Feb 18, 2020
@josharian
Copy link
Contributor

cc @hanwen @FiloSottile

@FiloSottile
Copy link
Contributor

Sounds good on both adding the algorithms and changing the certificate default.

OpenSSH 8.2 already stopped supporting ssh-rsa signed certificates, right?

@alex
Copy link
Contributor

alex commented Feb 18, 2020

Only for host certificates I think, client certs still can use it.

@hansnielsen
Copy link
Author

I considered removing the SHA-1 RSA signature for host certs but wanted to keep this issue to algorithm addition. There are some choices in the client host key algorithm list that are probably worth revisiting as well and it’d probably be best to handle that as one issue.

@FiloSottile
Copy link
Contributor

How long has OpenSSH supported rsa-sha2-512 certificates?

@hansnielsen
Copy link
Author

OpenSSH 7.3 was the first version with rsa-sha2-512 certificate support, released on 2016-08-01. Ubuntu xenial stands out as being the largest distro that's still on 7.2 with backported security fixes.

@alex
Copy link
Contributor

alex commented Feb 18, 2020

I always regret asking things like this, but surely support non-SHA1 signature algorithms is as deserving of a security backport as an actual CVE?

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/220037 mentions this issue: ssh: support RSA SHA-2 (RFC8332) signatures

@rsc
Copy link
Contributor

rsc commented Feb 26, 2020

Based on the discussion above, this seems like a likely accept.

@rsc
Copy link
Contributor

rsc commented Mar 4, 2020

No change in consensus, so accepted.

@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 4, 2020
@awly
Copy link
Contributor

awly commented May 12, 2020

Looks like https://go-review.googlesource.com/c/crypto/+/220037 has had no progress since Feb 20th.
Since everyone seems to agree on the change, @hansnielsen @FiloSottile do you have the time to finish that CL review?

This is causing teleport some interop problems with newer openssh versions.

peterverraedt pushed a commit to kuleuven/crypto that referenced this issue Jun 15, 2020
This change adds support for RSA SHA-2 based signatures for host keys and certificates. It also switches the default certificate signature algorithm for RSA to use SHA-512. This is implemented by treating ssh.Signer specially when the key type is `ssh-rsa` by also allowing SHA-256 and SHA-512 signatures.

Fixes golang/go#37278

Change-Id: I2ee1ac4ae4c9c1de441a2d6cf1e806357ef18910
@SwampDragons
Copy link

This issue is hitting us over in Packer at hashicorp/packer#8609. We use the ssh client, generating the Auth method first by calling ParsePrivateKey and then feeding that signer into the PublicKeys method to retrieve the publickeycallback. code: https://github.com/hashicorp/packer/blob/master/helper/communicator/config.go#L320-L324

I can create my own algorithm signer to set the algorithm and call the wrappedSigner's SignWithAlgorithm, but it doesn't work with the proposed change at https://go-review.googlesource.com/c/crypto/+/220037 unless the publickeycallback is also modified, because it's still sending the key type as the algorithm, in this case "ssh-rsa".

By hardcoding lines https://github.com/golang/crypto/blob/75b288015ac94e66e3d6715fb68a9b41bf046ec2/ssh/client_auth.go#L218 and https://github.com/golang/crypto/blob/75b288015ac94e66e3d6715fb68a9b41bf046ec2/ssh/client_auth.go#L232 to SigAlgoRSASHA2256, I can force this to work to connect to an ssh server that doesn't allow the "ssh-rsa" algorithm, but obviously this isn't a viable patch for the module.

This is my first time doing a real dive into this library so I'm wondering if anyone else on this thread has an instinct about how to extend the ssh.PublicKeys() method to make it work for AlgorithmSigners that are actually using custom algorithms.

@awly
Copy link
Contributor

awly commented Jun 26, 2020

I tried working around this by implementing a custom ssh.AlgorithmSigner around an ssh.Signer (which is derived from an SSH certificate).
You can sign new certificates using a CA this way and it seems to work fine with OpenSSH.

However, you can't use such signers for actual SSH handshakes against a Go SSH server, because of https://github.com/golang/crypto/blob/75b288015ac94e66e3d6715fb68a9b41bf046ec2/ssh/server.go#L556-L564. That's a hardcoded check for algorithms supported in handshake signatures.
So, without https://go-review.googlesource.com/c/crypto/+/220037, you can only get cert signing to work.

For reference, here's our workaround for certs: https://github.com/gravitational/teleport/pull/3777/files#diff-193b31b40b2b216b00568f93daa82603

@SwampDragons
Copy link

Turns out my particular client key handling needs aren't actually directly relevant to this patch, though the underlying "this module is having some problems with handling rsa sha-2 algorithms" is the same -- I think it needs its own GH issue. I'll work on setting up my own case and my own patch suggestion 😬

fff7d1bc pushed a commit to fff7d1bc/rclone that referenced this issue Apr 1, 2022
Updates golang.org/x/crypto to v0.0.0-20220331220935-ae2d96664a29.

Fixes the issues with connecting to OpenSSH 8.8+ remotes in case the
client uses RSA key pair due to OpenSSH dropping support for SHA1 based
ssh-rsa signature.

Bug: rclone#6076
Bug: golang/go#37278
Signed-off-by: KARBOWSKI Piotr <piotr.karbowski@gmail.com>
ncw pushed a commit to rclone/rclone that referenced this issue Apr 1, 2022
Updates golang.org/x/crypto to v0.0.0-20220331220935-ae2d96664a29.

Fixes the issues with connecting to OpenSSH 8.8+ remotes in case the
client uses RSA key pair due to OpenSSH dropping support for SHA1 based
ssh-rsa signature.

Bug: #6076
Bug: golang/go#37278
Signed-off-by: KARBOWSKI Piotr <piotr.karbowski@gmail.com>
ncw pushed a commit to rclone/rclone that referenced this issue Apr 1, 2022
Updates golang.org/x/crypto to v0.0.0-20220331220935-ae2d96664a29.

Fixes the issues with connecting to OpenSSH 8.8+ remotes in case the
client uses RSA key pair due to OpenSSH dropping support for SHA1 based
ssh-rsa signature.

Bug: #6076
Bug: golang/go#37278
Signed-off-by: KARBOWSKI Piotr <piotr.karbowski@gmail.com>
@mukundjnathan
Copy link

I have the latest x/crypto. I am using a user cert that is based on rsa generated using ssh-keygen with no options. My ssh client is set to use the agent auth.
If I supply the id_ecdsa public key to generate the user cert everything works. Kex match etc.
If I supply id_rsa.pub I get the error you are all seeing.
If this fix is in (merged..), I expected it to work but no.
(The cert works fine if I use Openssh via command line)
Is there any configuration on the Client side that will help. ?

appleboy added a commit to appleboy/drone-scp that referenced this issue Jun 15, 2022
iamacarpet pushed a commit to affordablemobiles/xcrypto that referenced this issue Aug 2, 2022
This change adds support for RSA SHA-2 based signatures for host keys and certificates. It also switches the default certificate signature algorithm for RSA to use SHA-512. This is implemented by treating ssh.Signer specially when the key type is `ssh-rsa` by also allowing SHA-256 and SHA-512 signatures.

Fixes golang/go#37278

Change-Id: I2ee1ac4ae4c9c1de441a2d6cf1e806357ef18910
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220037
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
This change adds support for RSA SHA-2 based signatures for host keys and certificates. It also switches the default certificate signature algorithm for RSA to use SHA-512. This is implemented by treating ssh.Signer specially when the key type is `ssh-rsa` by also allowing SHA-256 and SHA-512 signatures.

Fixes golang/go#37278

Change-Id: I2ee1ac4ae4c9c1de441a2d6cf1e806357ef18910
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220037
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
@golang golang locked and limited conversation to collaborators Jun 9, 2023
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
This change adds support for RSA SHA-2 based signatures for host keys and certificates. It also switches the default certificate signature algorithm for RSA to use SHA-512. This is implemented by treating ssh.Signer specially when the key type is `ssh-rsa` by also allowing SHA-256 and SHA-512 signatures.

Fixes golang/go#37278

Change-Id: I2ee1ac4ae4c9c1de441a2d6cf1e806357ef18910
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220037
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

No branches or pull requests