Skip to content
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

crypto/tls: back RSA-PSS out of TLS 1.2 in Go 1.13 #32425

Closed
FiloSottile opened this issue Jun 4, 2019 · 3 comments
Closed

crypto/tls: back RSA-PSS out of TLS 1.2 in Go 1.13 #32425

FiloSottile opened this issue Jun 4, 2019 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@FiloSottile
Copy link
Contributor

8834353 enabled again RSA-PSS in TLS 1.2. Unfortunately, there are broken crypto.Signer implementations out there that do the wrong thing when asked to generate RSA-PSS signatures.

TLS 1.3 requires RSA-PSS, so there is no way around ripping this band-aid off, but partially because of this TLS 1.3 was opt-in in Go 1.12 and is opt-out in Go 1.13 (#30055). RSA-PSS in TLS 1.2 would just be enabled with no way to turn it off, and both adding another GODEBUG option, or making GODEBUG=tls13=0 impact TLS 1.2 feel wrong.

What finally tipped the scale is that #28660 provides a nice way for code to opt-out of RSA-PSS in TLS 1.2 if needed. Let's wait to force RSA-PSS in TLS 1.2 until Go 1.14, when TLS 1.3 is also forced on, and there is a code path to disabling it.

/cc @agl @rsc

@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Jun 4, 2019
@FiloSottile FiloSottile added this to the Go1.13 milestone Jun 4, 2019
@gopherbot
Copy link

Change https://golang.org/cl/182339 mentions this issue: crypto/tls: disable RSA-PSS in TLS 1.2 again

ceseo pushed a commit to ceseo/go that referenced this issue Jun 19, 2019
Signing with RSA-PSS can uncover faulty crypto.Signer implementations,
and it can fail for (broken) small keys. We'll have to take that
breakage eventually, but it would be nice for it to be opt-out at first.

TLS 1.3 requires RSA-PSS and is opt-out in Go 1.13. Instead of making a
TLS 1.3 opt-out influence a TLS 1.2 behavior, let's wait to add RSA-PSS
to TLS 1.2 until TLS 1.3 is on without opt-out.

Note that since the Client Hello is sent before a protocol version is
selected, we have to advertise RSA-PSS there to support TLS 1.3.
That means that we still support RSA-PSS on the client in TLS 1.2 for
verifying server certificates, which is fine, as all issues arise on the
signing side. We have to be careful not to pick (or consider available)
RSA-PSS on the client for client certificates, though.

We'd expect tests to change only in TLS 1.2:

    * the server won't pick PSS to sign the key exchange
      (Server-TLSv12-* w/ RSA, TestHandshakeServerRSAPSS);
    * the server won't advertise PSS in CertificateRequest
      (Server-TLSv12-ClientAuthRequested*, TestClientAuth);
    * and the client won't pick PSS for its CertificateVerify
      (Client-TLSv12-ClientCert-RSA-*, TestHandshakeClientCertRSAPSS,
      Client-TLSv12-Renegotiate* because "R" requests a client cert).

Client-TLSv13-ClientCert-RSA-RSAPSS was updated because of a fix in the test.

This effectively reverts 8834353.

Testing was made more complex by the undocumented semantics of OpenSSL's
-[client_]sigalgs (see openssl/openssl#9172).

Updates golang#32425

Change-Id: Iaddeb2df1f5c75cd090cc8321df2ac8e8e7db349
Reviewed-on: https://go-review.googlesource.com/c/go/+/182339
Reviewed-by: Adam Langley <agl@golang.org>
@FiloSottile
Copy link
Contributor Author

Go 1.13 part done, retargeting to Go 1.14 to revert once #28660 is fixed.

@FiloSottile FiloSottile modified the milestones: Go1.13, Go1.14 Jul 10, 2019
@gopherbot
Copy link

Change https://golang.org/cl/205063 mentions this issue: crypto/tls: re-enable RSA-PSS in TLS 1.2 again

@golang golang locked and limited conversation to collaborators Nov 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants