golang/go

x/crypto/ssh: publicKeyCallback cannot handshake using ssh-rsa keys signed using the ssh-rsa-sha2-256 algorithm

SwampDragons opened this issue ยท 24 comments

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 Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/mmarsh/Library/Caches/go-build"
GOENV="/Users/mmarsh/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/mmarsh/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/mmarsh/go/src/golang.org/x/crypto/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8t/0yb5q0_x6mb2jldqq_vjn3lr0000gn/T/go-build245084723=/tmp/go-build -gno-record-gcc-switches -fno-common"

What 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

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.
Thanks!

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 AlgorithmSignerWithAlgo also has the drawback of not working with agent signers, which have been the most common in my experience.

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 SignWithAlgorithm to the agent signer, and have some special handling of Certificates to deal with the nist names they have wrapped around the public key types, but I think that might be a better approach to solve the issue transparently for users.

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 server-sig-algs to get the supported algorithms before polling for an accepted key algorithm, to avoid auth penalties with certain server configurations. Agent usage often encounters these limits already (which is a primary use for the IdentityFile openssh option with an agent), and tripling the requests is likely to exacerbate the problem significantly.

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 AlgorithmSigner alone can't break that association.

mpx commented

SHA-1 is (and will be) increasingly unsupported which causes golang.org/x/crypto/ssh to fail for more platforms. Eg:

  • Fedora 33 (2020-10-27) disabled SHA-1 [release notes]
  • OpenSSH maintainers have been warning they "will disable this signature scheme by default in the near
    future." since OpenSSH 8.2 (2020-02-14) [release notes]

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 rsa-sha2-256 instead?

  • SHA-1 is increasingly unsupported (above)
  • SHA-2 has been supported by SSH servers for many years now (eg, OpenSSH 7.2 released 2016-02-29).
  • Avoids a potential extra round trip to determine signature algorithms? (faster)
  • Better fit with Go's Cryptography Principles?:
    • Less code / complexity
    • Uses the safer/more secure/modern algorithm without extra work from callers
golang.org/x/crypto/ssh agent code example for testing
package 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)
    }
}
$ ./test-ssh mpx fedora32
good
$ ./test-ssh mpx fedora34
2021/05/08 17:52:46 SSH Dial: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain

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.

mpx commented

Update on my earlier comment re SHA1 deprecation timelines.

OpenSSH 8.7 was released on 2021-08-20. The release notes indicate:

OpenSSH will disable the ssh-rsa signature scheme by default in the next release.

x/crypto/ssh is failing on a growing list of machines. The next OpenSSH release will make this worse again.

mpx commented

OpenSSH 8.8 was released on 2021-09-26. RSA + SHA-1 has now been disabled by default (release notes):

This release disables RSA signatures using the SHA-1 hash algorithm
by default. This change has been made as the SHA-1 hash algorithm is
cryptographically broken, and it is possible to create chosen-prefix
hash collisions for <USD$50K

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:

https://github.com/golang/crypto/blob/089bfa5675191fd96a44247682f76ebca03d7916/ssh/handshake.go#L459-L462

		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 ssh-rsa method there doesn't appear to be a way of defining which Algorithms we support separate from the keys.


In Gitea the workaround I've come up with is to simply wrap around the Signer and its PublicKey to shadow the Type() and switch the algorithm to the alternative as per the Type().

See go-gitea/gitea#17281

Fundamentally this is an issue with the ssh.Signer type and/or its use in handshake.go.

The issue is that the type ssh.Signer:

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 AlgorithmSigner does provide a different signing method it's still not saying which type of Signer it is:

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 AlgorithmSigner and creates a TypeSigner for a given algorithm.


A MultipleAlgorithmSigner could be created as below but it would be more complex to wire in:

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 AlgorithmSigner advertising which TypeSigner they could become rather than being used directly in handshake.go. The idea being that it would be good if there was some programmatic way of listing all of the things could be supported. This could at first be used with some utility function to provide Signers for everything but later cut down to only support specific Signers.

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)

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.

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?

@pete-woods:

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: ssh: support rsa-sha2-256/512 for client authentication (client-side)

mpx commented

Thanks! The test code in #39885 (comment) now works after upgrading to:
golang.org/x/crypto v0.0.0-20220314234724-5d542ad81a58.

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

The linked PR only appears to implement handshake for the client.

Is it intended to do this for the server part?

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):

$ ssh-add -l
4096 SHA256:m+Hthc93TjF0wcAoq8OyrKZjDl8LE5ddhQwzwnBA02c /home/XXX/.ssh/id_rsa (RSA)
4096 SHA256:m+Hthc93TjF0wcAoq8OyrKZjDl8LE5ddhQwzwnBA02c /home/XXX/.ssh/id_rsa (RSA-CERT)
$ ssh 127.0.0.1 uptime
Warning: Permanently added '127.0.0.1' (ECDSA) to the list of known hosts.
 10:47:52 up 423 days, 20:25,  8 users,  load average: 1.15, 0.97, 1.00
$ ssh 10.19.197.10 uptime
 10:48:03 up 157 days,  1:37,  0 users,  load average: 1.40, 1.02, 0.73
$ cat 001-ssh-test.go 
package main

import (
        "log"
        "net"
        "os"
        "os/user"

        "golang.org/x/crypto/ssh"
        "golang.org/x/crypto/ssh/agent"
)

func logFatal(err error) {
        if err != nil {
                log.Fatal(err)
        }
}

func main() {

        sock, err := net.Dial("unix", os.Getenv("SSH_AUTH_SOCK"))
        logFatal(err)

        signers, err := agent.NewClient(sock).Signers()
        logFatal(err)

        u, err := user.Current()
        logFatal(err)

        cfg := &ssh.ClientConfig{
                User: u.Username,
                Auth: []ssh.AuthMethod{ssh.PublicKeys(signers...)},
                HostKeyCallback: ssh.InsecureIgnoreHostKey(),
        }

        log.Printf("Connecting to %s\n", os.Args[1])
        client, err := ssh.Dial("tcp", os.Args[1], cfg)
        logFatal(err)

        _, err = client.NewSession()
        logFatal(err)

        log.Println("We've got a live session!")
}
$ cat go.mod 
module test

require golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa

require golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect

go 1.18
$ go run 001-ssh-test.go 127.0.0.1:22
2022/07/23 10:48:52 Connecting to 127.0.0.1:22
2022/07/23 10:48:52 We've got a live session!
$ go run 001-ssh-test.go 10.19.197.10:22
2022/07/23 10:49:02 Connecting to 10.19.197.10:22
2022/07/23 10:49:05 ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain
exit status 1
$

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?