crypto icon indicating copy to clipboard operation
crypto copied to clipboard

ssh: relax RSA signature check in SSH_MSG_USERAUTH_REQUEST

Open stanhu opened this issue 3 years ago • 3 comments

Buggy SSH clients, such as gpg-agent v2.2.4 and OpenSSH v7.6 shipped in Ubuntu 18.04, may send ssh-rsa-512 as the public key algorithm but actually include an rsa-sha signature.

If RFC 3808 (extension negotiation) is implemented, these clients will fail to authenticate with the error:

ssh: signature "ssh-rsa" came in for selected algorithm "rsa-sha2-512", public key is type ssh-rsa

According to RFC 8332 section 3.2:

If the client includes the signature field, the client MUST encode the same algorithm name in the signature as in SSH_MSG_USERAUTH_REQUEST -- either "rsa-sha2-256" or "rsa-sha2-512". If a server receives a mismatching request, it MAY apply arbitrary authentication penalties, including but not limited to authentication failure or disconnect.

...A server MAY, but is not required to, accept this variant or another variant that corresponds to a good-faith implementation and is considered safe to accept.

While the client is expected to do the right thing, in practice older clients may not fully support ssh-rsa-256 and ssh-rsa-512. For example, gpg-agent v2.2.6 added support for these newer signature types.

To accomodate these clients, relax the matching constraint: if the SSH_MSG_USERAUTH_REQUEST message specifies an RSA public key algorithm and includes an RSA public key, then allow any of the following signature types:

  • rsa-sha-512
  • rsa-sha-256
  • rsa-sha

This emulates what OpenSSH does. OpenSSH only considers that the RSA family is specified and then verifies if the signature and public key match.

Closes https://github.com/golang/go/issues/53391

stanhu avatar Jun 16 '22 17:06 stanhu

This PR (HEAD: 443b8cb475cc16514b864a61d02fef52fbb2ae2d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/412854 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off) See the Wiki page for more info

gopherbot avatar Jun 16 '22 17:06 gopherbot

Message from Maxime Tremblay:

Patch Set 1: Code-Review+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 18 '22 06:06 gopherbot

Hi,

thanks for working on this. I planned to take a look this weekend. Maybe something like the following (untested) is a bit more generic and will potentially work for other algos in the future

diff --git a/ssh/server.go b/ssh/server.go
index 48f15c3..3b9d26c 100644
--- a/ssh/server.go
+++ b/ssh/server.go
@@ -437,6 +437,26 @@ var ErrNoAuth = errors.New("ssh: no auth passed yet")
 // specific authentication step succeed
 var ErrPartialSuccess = errors.New("ssh: authenticated with partial success")
 
+func isAlgoCompatible(algo, pubKeyFormat string, sig *Signature) bool {
+       algo = underlyingAlgo(algo)
+       if algo == sig.Format {
+               return true
+       }
+
+       // Buggy SSH clients may send ssh-rsa2-512 as the public key algorithm but
+       // actually include a rsa-sha signature.
+       // According to RFC 8832 Section 3.2:
+       // A server MAY, but is not required to, accept this variant or another variant that
+       // corresponds to a good-faith implementation and is considered safe to
+       // accept.
+       compatibleAlgos := algorithmsForKeyFormat(pubKeyFormat)
+       if contains(compatibleAlgos, algo) && contains(compatibleAlgos, sig.Format) {
+               return true
+       }
+
+       return false
+}
+
 func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, error) {
        sessionID := s.transport.getSessionID()
        var cache pubKeyCache
@@ -626,7 +646,7 @@ userAuthLoop:
                                        authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
                                        break
                                }
-                               if underlyingAlgo(algo) != sig.Format {
+                               if !isAlgoCompatible(algo, pubKey.Type(), sig) {
                                        authErr = fmt.Errorf("ssh: signature %q not compatible with selected algorithm %q", sig.Format, algo)
                                        break
                                }

drakkan avatar Jun 19 '22 13:06 drakkan

This PR (HEAD: 08edff8304716fba1b6a681d937d2ef5f51283c3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/412854 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off) See the Wiki page for more info

gopherbot avatar Feb 15 '23 19:02 gopherbot

Message from Jakub Nyckowski:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Feb 15 '23 19:02 gopherbot

This PR (HEAD: 630f71961527acf21f42f6ef29ae4a2444d605c2) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/412854 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off) See the Wiki page for more info

gopherbot avatar Feb 15 '23 19:02 gopherbot

Message from Maxime Tremblay:

Patch Set 3: Code-Review+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Feb 16 '23 14:02 gopherbot

@FiloSottile Is there anything I can do to get this change merged?

stanhu avatar Apr 24 '23 18:04 stanhu

Message from Stan Hu:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar May 03 '23 21:05 gopherbot

Message from Stan Hu:

Patch Set 3: Code-Review+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar May 03 '23 21:05 gopherbot

Message from Stan Hu:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 19:06 gopherbot

Message from Roland Shoemaker:

Patch Set 3: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Gopher Robot:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Gopher Robot:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Roland Shoemaker:

Patch Set 4: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Gopher Robot:

Patch Set 4:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Gopher Robot:

Patch Set 4: TryBot-Result+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Jakub Nyckowski:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Stan Hu:

Patch Set 4: Code-Review+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 05 '23 20:06 gopherbot

Message from Jakub Nyckowski:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 06 '23 22:06 gopherbot

Message from Stan Hu:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 01:06 gopherbot

Message from Jakub Nyckowski:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 02:06 gopherbot

Message from Stan Hu:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 05:06 gopherbot

Message from Nicola Murino:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 09:06 gopherbot

Message from Jakub Nyckowski:

Patch Set 4:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 15:06 gopherbot

Message from Stan Hu:

Patch Set 4:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 16:06 gopherbot

This PR (HEAD: 9c9ba6a5d725b5247ce43e5c1eafac8a10136195) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/crypto/+/412854 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off) See the Wiki page for more info

gopherbot avatar Jun 07 '23 16:06 gopherbot

Message from Nicola Murino:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 07 '23 19:06 gopherbot

Message from Nicola Murino:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 08 '23 04:06 gopherbot

Message from Stan Hu:

Patch Set 5: Code-Review+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/412854. After addressing review feedback, remember to publish your drafts!

gopherbot avatar Jun 08 '23 14:06 gopherbot