-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: publicKeyCallback cannot handshake using ssh-rsa keys signed using the ssh-rsa-sha2-256 algorithm #39885
Comments
/cc @hanwen @FiloSottile per owners. |
I've opened a patchset at https://go-review.googlesource.com/c/crypto/+/240717 |
Is there anything else the maintainers need from me in order to make this ready for a review? |
Hi, I know it's annoying having someone nagging at an issue, but it's been a while and having some kind of solution to this is a blocker for some people using Packer. Is there any way @hanwen or @FiloSottile could take a look at the patch I've submitted? |
Would love to see this reviewed, it is a blocker for me with packer. |
If I understand correctly, I think in this case the ssh client should be responsible for the signing algorithm negotiation. Because the user may not know if the server configuration will accept the legacy format or not, I don't think think we can place the burden on the user to determine the signing algorithm, plus it's kind of cumbersome since changing the algorithm for most keys isn't valid anyway. The It looks to me like we should be able to easily negotiate the signing algorithm while verifying the allowed public keys with the server, and pass the discovered algorithm along. We probably need to add |
Yeah I think that's a better approach too. This patchset was a very timid "I don't want to muck around too much inside anything or change any current behaviors without explicit interaction" approach, which I'd hoped would help get it adopted faster since it seemed pretty low risk to current users. That said, I bet we could simply have the validateKey check determine whether the PublicKey.Type() is SigAlgoRSA ("ssh-rsa"), and if the validation fails try again with SigAlgoRSASHA2256 ("rsa-sha2-256") instead. It would be a lot less code and prevent users from having to implement this. |
Getting back up to speed, as RFC8332 and RFC8308 are much newer than my previous ssh work. It looks like the correct negotiation here is to implement The key types should however still be decoupled from the algorithm name, as the public key format and algorithm were only the same by coincidence and |
SHA-1 is (and will be) increasingly unsupported which causes
Detecting the correct signature algorithm would be better than trying multiple algorithms since there are often signing limits with agents. Either raw throughput, or potentially even requiring a human interaction per signature (2nd factor). However, perhaps we should consider simply using
golang.org/x/crypto/ssh agent code example for testingpackage main
import (
"log"
"net"
"os"
"golang.org/x/crypto/ssh"
"golang.org/x/crypto/ssh/agent"
)
func main() {
if len(os.Args) < 3 {
log.Fatalf("usage: %s USERNAME HOSTNAME", os.Args[0])
}
username := os.Args[1]
target := os.Args[2] + ":22"
conn, err := net.Dial("unix", os.Getenv("SSH_AUTH_SOCK"))
check(err, "Agent Dial")
ag := agent.NewClient(conn)
client, err := ssh.Dial("tcp", target, &ssh.ClientConfig{
User: username,
Auth: []ssh.AuthMethod{ssh.PublicKeysCallback(ag.Signers)},
HostKeyCallback: ssh.InsecureIgnoreHostKey(),
})
check(err, "SSH Dial")
sess, err := client.NewSession()
check(err, "SSH NewSession")
sess.Stdout = os.Stdout
sess.Stderr = os.Stderr
err = sess.Run("echo good")
check(err, "SSH Run")
}
func check(err error, msg string) {
if err != nil {
log.Fatalf("%s: %v", msg, err)
}
}
Cc @FiloSottile @katiehockman @rolandshoemaker (current maintainers) |
With Fedora 32 gone EOL a couple months ago (the last release Packer worked with in AWS EC2), this issue has become more critical for me. Happy to help test. |
Hey, I'm on the Git Protocols team at GitHub, and we're responsible for the service that terminates and serves Git connections, including connections over SSH. We're in the process of planning some changes to the algorithms we support over SSH. Specifically, we're looking into getting rid of most support for SHA-1, since it's dangerously weak. This includes, tentatively, prohibiting newly added RSA keys from using SHA-1, but allowing existing RSA keys to continue using it. The Go SSH library is one of the largest clients we see still using RSA with SHA-1, and it would be great if it were possible for it to support RSA with SHA-256 and SHA-512. I realize that the Go SSH client offers other algorithms which are secure, but users may wish to use RSA keys in the future. We're still looking at options and we'll announce plans more openly when we're more definitive on them, but we wanted to reach out on this issue and inform you of our current intentions. Feel free to let me know if you have any questions. |
Update on my earlier comment re SHA1 deprecation timelines. OpenSSH 8.7 was released on 2021-08-20. The release notes indicate:
x/crypto/ssh is failing on a growing list of machines. The next OpenSSH release will make this worse again. |
OpenSSH 8.8 was released on 2021-09-26. RSA + SHA-1 has now been disabled by default (release notes):
SHA-256 and SHA-512 have been supported by SSH servers for many years now. The Go SSH package should default to using SHA-256 to avoid ongoing connectivity failures with well maintained servers. |
This still affects golang 1.17 and we've recently noticed this on Gitea go-gitea/gitea#17175 The underlying problem is that: for _, k := range t.hostKeys {
msg.ServerHostKeyAlgos = append(
msg.ServerHostKeyAlgos, k.PublicKey().Type())
} Makes the assumption that a public key type is the same as the key algorithm that that key can produce. There's not really any way of defining what the algorithms are separate from these keys - and in fact once we start needing to completely deprecate the In Gitea the workaround I've come up with is to simply wrap around the |
Fundamentally this is an issue with the The issue is that the type type Signer interface {
// PublicKey returns an associated PublicKey instance.
PublicKey() PublicKey
// Sign returns raw signature for the given data. This method
// will apply the hash specified for the keytype to the data.
Sign(rand io.Reader, data []byte) (*Signature, error)
} Doesn't provide a mechanism to say which type of algorithm is supported by this Signer - and its assumed incorrectly that that is the PublicKey type and although the type AlgorithmSigner interface {
Signer
// SignWithAlgorithm is like Signer.Sign, but allows specification of a
// non-default signing algorithm. See the SigAlgo* constants in this
// package for signature algorithms supported by this package. Callers may
// pass an empty string for the algorithm in which case the AlgorithmSigner
// will use its default algorithm.
SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error)
} And so it is not wired into handshake.go. The Signer interface would actually need to be something like: type TypedSigner interface {
Signer
// Type returns the algorithm type for this signer
Type() String
} This would then allow backwards compatibility with the current system and allow handshake.go to be changed to: for _, k := range t.hostKeys {
if typed, ok := k.(TypeSigner); ok {
msg.ServerHostKeyAlgos = append(
msg.ServerHostKeyAlgos, typed.Type())
continue
}
msg.ServerHostKeyAlgos = append(
msg.ServerHostKeyAlgos, k.PublicKey().Type())
} A couple of utility types and associated functions would need to be provided - the most simple being one that takes an A type MultipleAlgorithmSigner interface {
AlgorithmSigner
// Algorithms returns the available algorithms
Algorithms() []string
// SignWithAlgorithm is like Signer.Sign, but allows specification of a
// non-default signing algorithm. See the SigAlgo* constants in this
// package for signature algorithms supported by this package. Callers may
// pass an empty string for the algorithm in which case the AlgorithmSigner
// will use its default algorithm.
SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error)
} My suspicion is that this would be better used as a way of an At present there is no programmatic way of determining which algorithms are supported. Edit: This is only part of the problem -- we also need RFC8308 supported. (see #49269) |
Due to golang/go#39885 if we only allow rsa-sha2-256,rsa-sha2-512 golang/x/crypto doesn't work as it presents the key as a regular ssh-rsa (which fallsback to using sha1 hashes which are insecure) Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
At CircleCI (where we fall back to a Go based Git+SSH for CI runs where the tools are not in user's images) we recently started carrying a couple of patches to x/crypto to support this. I suspect we were one of the larger contributors to your stats there. However, we feel rather uncomfortable carrying out of tree patches. We'd be happy to dedicate some effort towards this issue, in the form of helping finish PRs, if that would be useful here? |
Mind sharing your patched x/crypto? I have been paying close attention to #49952 but the progress has been slowed to fully address the issue. I'm planning to use a working forked x/crypto for now for my projects. |
Change https://go.dev/cl/392394 mentions this issue: |
CL 220037 had implemented support for host authentication using rsa-sha2-256/512, but not client public key authentication. OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8 (after pre-announcing it in versions 8.2, 8.3, 8.4, 8.5, 8.6, and 8.7) although some distributions re-enable it. GitHub will start rejecting ssh-rsa for keys uploaded before November 2, 2021 on March 15, 2022. https://github.blog/2021-09-01-improving-git-protocol-security-github/ The server side already worked, as long as the client selected one of the SHA-2 algorithms, because the signature flowed freely to Verify. There was however nothing verifying that the signature algorithm matched the advertised one. The comment suggested the check was being performed, but it got lost back in CL 86190043. Not a security issue because the signature had to pass the callback's Verify method regardless, and both values were checked to be acceptable. Tested with OpenSSH 8.8 configured with "PubkeyAcceptedKeyTypes -ssh-rsa" and no application-side changes. The Signers returned by ssh/agent (when backed by an agent client) didn't actually implement AlgorithmSigner but ParameterizedSigner, an interface defined in an earlier version of CL 123955. Changed the type in TestClientAuthErrorList just so we wouldn't get 3 errors for one key. Fixes golang/go#39885 For golang/go#49952 Change-Id: I13b41db8041f1112a70f106c55f077b904b12cb8
CL 220037 had implemented support for host authentication using rsa-sha2-256/512, but not client public key authentication. OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8 (after pre-announcing it in versions 8.2, 8.3, 8.4, 8.5, 8.6, and 8.7) although some distributions re-enable it. GitHub will start rejecting ssh-rsa for keys uploaded before November 2, 2021 on March 15, 2022. https://github.blog/2021-09-01-improving-git-protocol-security-github/ The server side already worked, as long as the client selected one of the SHA-2 algorithms, because the signature flowed freely to Verify. There was however nothing verifying that the signature algorithm matched the advertised one. The comment suggested the check was being performed, but it got lost back in CL 86190043. Not a security issue because the signature had to pass the callback's Verify method regardless, and both values were checked to be acceptable. Tested with OpenSSH 8.8 configured with "PubkeyAcceptedKeyTypes -ssh-rsa" and no application-side changes. The Signers returned by ssh/agent (when backed by an agent client) didn't actually implement AlgorithmSigner but ParameterizedSigner, an interface defined in an earlier version of CL 123955. Updates golang/go#49269 Fixes golang/go#39885 For golang/go#49952 Change-Id: I13b41db8041f1112a70f106c55f077b904b12cb8
Thanks! The test code in #39885 (comment) now works after upgrading to: |
Change https://go.dev/cl/392974 mentions this issue: |
The server-sig-algs logic was not working for certificate algorithms. Follow-up on CL 392394. Tested with OpenSSH 8.8 configured with PubkeyAcceptedKeyTypes -ssh-rsa-cert-v01@openssh.com Updates golang/go#39885 For golang/go#49952 Change-Id: Ic230dd6f98e96b7938acbd0128ab37d33b70abe5 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392974 Trust: Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
The linked PR only appears to implement handshake for the client. Is it intended to do this for the server part? |
The server-sig-algs logic was not working for certificate algorithms. Follow-up on CL 392394. Tested with OpenSSH 8.8 configured with PubkeyAcceptedKeyTypes -ssh-rsa-cert-v01@openssh.com Updates golang/go#39885 For golang/go#49952 Change-Id: Ic230dd6f98e96b7938acbd0128ab37d33b70abe5 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392974 Trust: Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8. See more: golang/go#39885 Signed-off-by: zwtop <wang.zhan@smartx.com>
OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8. See more: golang/go#39885 Signed-off-by: zwtop <wang.zhan@smartx.com>
OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8. See more: golang/go#39885 Signed-off-by: zwtop <wang.zhan@smartx.com>
OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8. See more: golang/go#39885 Signed-off-by: zwtop <wang.zhan@smartx.com>
OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8. See more: golang/go#39885 Signed-off-by: zwtop <wang.zhan@smartx.com>
From what I see this is still not working (127.0.0.1 accepts only unsigned keys, 10.19.197.10 accepts only signed keys):
|
This looks like an issue in the agent package. Do you mind opening this as a new issue with details on the agent and server versions, and tag me in it? |
@FiloSottile done: #54027 |
CL 220037 had implemented support for host authentication using rsa-sha2-256/512, but not client public key authentication. OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8 (after pre-announcing it in versions 8.2, 8.3, 8.4, 8.5, 8.6, and 8.7) although some distributions re-enable it. GitHub will start rejecting ssh-rsa for keys uploaded before November 2, 2021 on March 15, 2022. https://github.blog/2021-09-01-improving-git-protocol-security-github/ The server side already worked, as long as the client selected one of the SHA-2 algorithms, because the signature flowed freely to Verify. There was however nothing verifying that the signature algorithm matched the advertised one. The comment suggested the check was being performed, but it got lost back in CL 86190043. Not a security issue because the signature had to pass the callback's Verify method regardless, and both values were checked to be acceptable. Tested with OpenSSH 8.8 configured with "PubkeyAcceptedKeyTypes -ssh-rsa" and no application-side changes. The Signers returned by ssh/agent (when backed by an agent client) didn't actually implement AlgorithmSigner but ParameterizedSigner, an interface defined in an earlier version of CL 123955. Updates golang/go#49269 Fixes golang/go#39885 For golang/go#49952 Change-Id: I13b41db8041f1112a70f106c55f077b904b12cb8 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392394 Trust: Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
The server-sig-algs logic was not working for certificate algorithms. Follow-up on CL 392394. Tested with OpenSSH 8.8 configured with PubkeyAcceptedKeyTypes -ssh-rsa-cert-v01@openssh.com Updates golang/go#39885 For golang/go#49952 Change-Id: Ic230dd6f98e96b7938acbd0128ab37d33b70abe5 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392974 Trust: Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
CL 220037 had implemented support for host authentication using rsa-sha2-256/512, but not client public key authentication. OpenSSH disabled the SHA-1 based ssh-rsa by default in version 8.8 (after pre-announcing it in versions 8.2, 8.3, 8.4, 8.5, 8.6, and 8.7) although some distributions re-enable it. GitHub will start rejecting ssh-rsa for keys uploaded before November 2, 2021 on March 15, 2022. https://github.blog/2021-09-01-improving-git-protocol-security-github/ The server side already worked, as long as the client selected one of the SHA-2 algorithms, because the signature flowed freely to Verify. There was however nothing verifying that the signature algorithm matched the advertised one. The comment suggested the check was being performed, but it got lost back in CL 86190043. Not a security issue because the signature had to pass the callback's Verify method regardless, and both values were checked to be acceptable. Tested with OpenSSH 8.8 configured with "PubkeyAcceptedKeyTypes -ssh-rsa" and no application-side changes. The Signers returned by ssh/agent (when backed by an agent client) didn't actually implement AlgorithmSigner but ParameterizedSigner, an interface defined in an earlier version of CL 123955. Updates golang/go#49269 Fixes golang/go#39885 For golang/go#49952 Change-Id: I13b41db8041f1112a70f106c55f077b904b12cb8 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/392394 Trust: Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
This relates to:
x/crypto/ssh: cannot sign certificate with different algorithm #36261
x/crypto/ssh: support RSA SHA-2 host key signatures #37278
What version of Go are you using (
go version
)?1.14.2
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Here's a gist containing code to reproduce this issue, provided you have an instance to connect to that's similarly set up to mine. I've got full steps to create such an instance in the gist's README.md.
https://gist.github.com/SwampDragons/8b913208add452f1b50f6f47426ac329
What did you expect to see?
I expected to be able to connect to the instance using a custom AlgorithmSigner.
I am able to connect to an instance before the crypto policy on that instance is updated to deny keys signed using the "ssh-rsa" algorithm. Note that the policy being applied does accept the ssh-rsa keys if they are instead signed with the "rsa-sha2-256" algorithm. Theoretically, I should be able to create my own AlgorithmSigner to apply this algorithm to my key.
What did you see instead?
I get denied access to the instance with an authentication error.
I have traced this to the validateKey and publicKeyCallback methods in ssh/client_auth. These methods assume that the algorithm is always the same as the key type, which is not the case in my situation.
Possible Solutions
Create a new interface, AlgorithmSignerWithAlgo, which has the method Algorithm(). When called, Algorithm() will return the type of algorithm used, so that publicKeyCallback can set this field accurately. We will also need to update the validateKey method to not return an error if the algorithm used to sign the validation request doesn't match the key type.
Here's a diff of a lightweight solution that enables users to implement their own signers that the default publicKeyCallback can use to correctly handshake: https://github.com/SwampDragons/crypto/pull/1/files
The text was updated successfully, but these errors were encountered: