ssh: relax RSA signature check in SSH_MSG_USERAUTH_REQUEST

[stanhu]

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 golang/go#53391

[gopherbot]

This PR (HEAD: 443b8cb) 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]

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!

[drakkan]

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
                                }

[gopherbot]

This PR (HEAD: 08edff8) 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]

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]

This PR (HEAD: 630f719) 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]

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!

[stanhu]

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

[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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

This PR (HEAD: 9c9ba6a) 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]

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]

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]

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]

Message from Ash McKenzie:

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]

Message from David Chase:

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]

Message from tricium-prod@appspot.gserviceaccount.com:

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]

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]

Message from Stan Hu:

Patch Set 5:

(2 comments)

---

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

[stanhu]

Closing in favor of #261.

[stanhu]

Will reopen to see whether #261 is preferable.

[gopherbot]

This PR is being closed because golang.org/cl/412854 has been abandoned.

GitHub PR #221 has been closed.