golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.71k stars 17.5k forks source link

x/crypto/ssh: rsa-sha2-256/rsa-sha2-512 tracking issue #49952

Open FiloSottile opened 2 years ago

FiloSottile commented 2 years ago

OpenSSH migrated the ssh-rsa key type, which historically used the ssh-rsa signature algorithm based on SHA-1, to the new rsa-sha2-256 and rsa-sha2-512 signature algorithms.

x/crypto/ssh was not ready for the key type / signature algorithm mismatch, so it needs a few changes. Moreover, OpenSSH implemented a few mechanisms at the same time to enable the rollout, which we need to assess and expose.

This is a tracking issue for the effort in general. Here's a list of not-duplicate related issues:

We'll also need some tests against OpenSSH proper, like the crypto/tls recorded tests against OpenSSL, since https://golang.org/cl/220037 turned out to be a partial fix.

owenthereal commented 2 years ago

👋 I ran into this issue with my project: https://github.com/owenthereal/upterm/issues/93#issuecomment-1045387517. I'm wondering whether there is anything I could help.

gopherbot commented 2 years ago

Change https://go.dev/cl/392355 mentions this issue: ssh: don't advertise rsa-sha2 algorithms if we can't use them

gopherbot commented 2 years ago

Change https://go.dev/cl/392354 mentions this issue: ssh: deprecate and replace SigAlgo constants

gopherbot commented 2 years ago

Change https://go.dev/cl/392394 mentions this issue: ssh: support rsa-sha2-256/512 for client authentication (client-side)

pete-woods commented 2 years ago

Should this issue (and others), be marked resolved now?

FiloSottile commented 2 years ago

Almost but not quite, we should also implement server-sig-algs on the server side before we can claim we're done.

Then we can open follow-up issues for adding OpenSSH recorded tests, extending the AlgorithmSigner interface (with another interface upgrade) to allow advertising specific supported algorithms and their preference order, and finally to disable SHA-1 by default throughout (making sure it's possible to turn it back on at least for a bit, which will be complex).

gopherbot commented 2 years ago

Change https://go.dev/cl/392974 mentions this issue: ssh: support rsa-sha2-256/512 for client certificates

cipherboy commented 2 years ago

@FiloSottile I believe a fifth part is also missing, which is allowing x/crypto/ssh based CAs to sign keys with these algorithms? Per documentation it appears the new types aren't enabled for SSH Certificate usage. This prohibits us from signing SSH certificates with these newer types, based on my read of the code.

Is this tracked elsewhere or should I file a new issue for it so it can be tracked here?

FiloSottile commented 2 years ago

@cipherboy What you describe is tracked by #36261, and would be resolved by #52132.

By the way, "CertAlgoRSASHA256v01 and CertAlgoRSASHA512v01 can't appear as a Certificate.Type (or PublicKey.Type)" doesn't mean they can't be used for signing, but means that the certificate type will still be CertAlgoRSAv01, the same way as a KeyAlgoRSA public key generates KeyAlgoRSASHA256/512 signatures.

It's all a bit complicated, but we'll add an example as part of #52132.

cipherboy commented 2 years ago

Thanks @FiloSottile for the pointers! Reading that after reading the cert spec format again makes sense to me how that'd work.

vielmetti commented 1 year ago

It's been a few months, and we've bumped up against this issue again, so I'd love to hear any status from anyone interested in this, who've worked on this, or perhaps you're carrying a patch in some fork because you couldn't wait.

Noting to @cipherboy attention (as a courtesy only).

asymmetricia commented 1 year ago

To add on to @vielmetti wrote, the SSH present in macos Ventura suffers from this issue, so it seems likely we'll start seeing problems related to it crop up more and more.

FiloSottile commented 1 year ago

Ah, I was hoping to do a round of x/crypto/ssh work once the stdlib freeze started, but losing compatibility with a widespread client would make this urgent. I'll look into it next week.

Can you provide some reproduction steps over at #49269 (if that's indeed the relevant sub-issue)? I assume it involves connecting to an x/crypto/ssh server with an RSA public key using the OS stock OpenSSH_9.0p1?

It would help to know what other clients to test, aside from macOS Ventura, if folks encountered breakage elsewhere.

drakkan commented 1 year ago

Ah, I was hoping to do a round of x/crypto/ssh work once the stdlib freeze started, but losing compatibility with a widespread client would make this urgent. I'll look into it next week.

Can you provide some reproduction steps over at #49269 (if that's indeed the relevant sub-issue)? I assume it involves connecting to an x/crypto/ssh server with an RSA public key using the OS stock OpenSSH_9.0p1?

yes, this is enough. I have been using this patch for about 6 months without any issues. Maybe you can use it as a starting point. You can find the full patch set I am using in SFTPGo here. This patch should be replaced with this one. Thanks!

It would help to know what other clients to test, aside from macOS Ventura, if folks encountered breakage elsewhere.

asymmetricia commented 1 year ago

Can you provide some reproduction steps over at #49269 (if that's indeed the relevant sub-issue)? I assume it involves connecting to an x/crypto/ssh server with an RSA public key using the OS stock OpenSSH_9.0p1?

It would help to know what other clients to test, aside from macOS Ventura, if folks encountered breakage elsewhere.

Here's a POC/reproducer: https://github.com/asymmetricia/sshpoc

Aside from Ventura, this issue effects Ubuntu Jammy (22.04) and Debian Sid.

gopherbot commented 1 year ago

Change https://go.dev/cl/447757 mentions this issue: ssh: support rsa-sha2-256/512 on the server side

FiloSottile commented 1 year ago

Please test https://go.dev/cl/447757, which is a simplified version of golang/crypto#211.

I also filed #56561 to track the changes necessary to make it possible to turn off ssh-rsa.

asymmetricia commented 1 year ago

Thanks! This is working for me based on the POC I put together:

with the referenced commit:

$ go run .
2022/11/03 16:56:38 OpenSSH_8.9p1 Ubuntu-3, OpenSSL 3.0.2 15 Mar 2022
2022/11/03 16:56:38 ✅ connection succeeded with key id_rsa_client
2022/11/03 16:56:38 ✅ connection succeeded with key id_ed25519_client

with current v0.1.0:

$ go run .
2022/11/03 16:56:45 OpenSSH_8.9p1 Ubuntu-3, OpenSSL 3.0.2 15 Mar 2022
2022/11/03 16:56:45 â›” connection failed with key id_rsa_client
2022/11/03 16:56:45 â›” debug1: send_pubkey_test: no mutual signature algorithm
2022/11/03 16:56:45 ✅ connection succeeded with key id_ed25519_client
euank commented 1 year ago

Please test https://go.dev/cl/447757

Thanks @FiloSottile!

I help maintain a (unfortunately closed source) ssh server in go. We saw the issue described here (newer openssh clients not finding any mutual signature algorithm). That CL fixes the issue for me, and I haven't seen any other issues from deploying/running with that change.

The CL's diff LGTM too :)

FiloSottile commented 1 year ago

Server-side support for rsa-sha2-256/512 is now in master, and available as version v0.2.1-0.20221112162523-6fad3dfc1891.

@golang/release, can we get a v0.2.1 tag, since this fixes compatibility with a wide range of clients?

heschi commented 1 year ago

If v0.3.0 is okay, I can use it as a chance to test http://go.dev/cl/445955. If you want exactly v0.2.1 some code changes will be necessary.

FiloSottile commented 1 year ago

@heschi sure, v0.3.0, test away :)

heschi commented 1 year ago

Something has gone wonky and some builders haven't run on the release branches for the crypto repository. I'll look harder tomorrow if they don't clean themselves up overnight.

heschi commented 1 year ago

Tagging is blocked because the latest commit encountered a data race: https://build.golang.org/log/08e983d39cbf137a9609419873f7a32e8f354d8f. It'll probably tag the next commit, whatever that happens to be. (Perhaps a fix for that race...?)

idcmp commented 1 year ago

Just for tracking purposes, this doesn't fix the client-half regression mentioned in https://github.com/golang/go/issues/56342

heschi commented 1 year ago

Tagged.

drakkan commented 9 months ago

I think this issue can be closed, now we should support everything related to rsa-sha-2 and since version 0.16.0 we have also fixed all known regressions.

kzhui125 commented 6 months ago
config := ssh.ClientConfig{
  User: user,
  Auth: []ssh.AuthMethod{
      ssh.PublicKeys(signer),
  },
  HostKeyCallback:   ssh.FixedHostKey(hostKey),
  HostKeyAlgorithms: []string{"ssh-rsa"},
}

"ssh-rsa" is parsed using known_hosts file

"ssh: handshake failed: ssh: no common algorithm for host key; client offered: [ssh-rsa], server offered: [rsa-sha2-512 rsa-sha2-256]"

maybe this should be well documented for HostKeyAlgorithms, or support [rsa-sha2-512 rsa-sha2-256] with [ssh-rsa]

drakkan commented 6 months ago

@kzhui125 the ssh-rsa key type supports the old and insecure ssh-rsa signature algorithm (SHA-1 based) and also rsa-sha2-256, rsa-sha2-512. HostKeyAlgorithms sets the accepted signature algorithms. With the above configuration you are asking the SSH client to only use the ssh-rsa signature algorithm. The known_hosts file contains the key type not the signature algorithm.

More details