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

proposal: crypto/tls: allow configurability of supported Signature Hash Algorithms #28660

Closed
e3b0c442 opened this issue Nov 8, 2018 · 6 comments

Comments

@e3b0c442
Copy link

@e3b0c442 e3b0c442 commented Nov 8, 2018

Go up to and including the current version (at this writing, 1.11.2) hardcodes the list of supported algorithms for the TLS 1.2 Signature Algorithms extension. Concurrently, crypto/tls also allows the use of custom signers which may have their own limitations not addressed by the hardcoded list of supported algorithms.

One real-world example of this is a custom signer for a TLS client that uses a hardware backend such as a Trusted Platform Module. TPM 1.2 modules can only support SHA1, and while TPM 2.0 modules can support SHA512, they are only required under the current spec to support SHA1 and SHA256. Depending on the list of algorithms provided by the remote party, the Go implementation may choose a 384-bit or 512-bit algorithm that is not supported by the backing hardware module, causing the handshake to fail.

In order to address this, I propose adding a configuration option to tls.Config to allow a custom list of supported signature algorithms to be provided, which will allow custom signer implementations to express the algorithms they support.

@bradfitz bradfitz changed the title crypto/tls: allow configurability of supported Signature Hash Algorithms proposal: crypto/tls: allow configurability of supported Signature Hash Algorithms Nov 8, 2018
@gopherbot gopherbot added this to the Proposal milestone Nov 8, 2018
@gopherbot gopherbot added the Proposal label Nov 8, 2018
@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Nov 8, 2018

This sounds like it would be more cleanly addressed by an interface upgrade on crypto.Signer allowing it to communicate which ones it supports, than by yet another tls.Config switch. Moreover, this is a property of the selected certificate, not of the system as a whole (forcing the use of GetConfigForClient.

Happy to consider this for Go 1.13. Go 1.12 is now in feature freeze.

@e3b0c442

This comment has been minimized.

Copy link
Author

@e3b0c442 e3b0c442 commented Nov 9, 2018

While I understand the line of thinking, I'm not sure tying this directly to crypto.Signer is the best option.

In order to query the crypto.Signer, there would need to exist some set of values to define the algorithms the crypto.Signer supports. While such a list exists in crypto/tls, I see two problems co-opting that for the crypto.Signer:

  1. crypto.Signer is more generalized and can be used in contexts not related to TLS, so one would not want crypto.Signer to refer to crypto/tls for a list of allowable values.

  2. Given that crypto.Signer can be an arbitrary implementation, I'm not sure it is possible or advisable to try to enumerate the parameters needed for the TLS implementation to determine a list of supported algorithms.

Given these points, I believe it makes the most sense to define this selection in the crypto/tls package somehow. That said, there is another option that satisfies keeping the selection in crypto/tls while not further complicating tls.Config: adding the field to tls.Certificate. This ties the algorithm selection to the crypto.Signer while not encumbering crypto.Signer with TLS knowledge, while at the same not adding additional fields to tls.Config and recognizing that the limitations are with the private key signer and not with the client itself.

@efb2

This comment has been minimized.

Copy link

@efb2 efb2 commented Jun 3, 2019

Curious, has this been looked at since the proposal? It is a practical limitation since many TPM2 implementations do not implement SHA384/512.
It doesn't seem like it, but are there any workarounds aside from modifying crypto/tls.

@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Jun 4, 2019

I like the idea of adding SupportedSignatureAlgorithms to tls.Certificate. The constants are already exposed, so it should be fairly unobtrusive, and it also lets TLS 1.2 clients opt out of RSA-PSS.

Approving for Go 1.14.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 4, 2019

Change https://golang.org/cl/205062 mentions this issue: crypto/tls: implement Certificate.SupportedSignatureAlgorithms

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 4, 2019

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

@gopherbot gopherbot closed this in 5b17b65 Nov 12, 2019
gopherbot pushed a commit that referenced this issue Nov 12, 2019
TLS 1.3, which requires RSA-PSS, is now enabled without a GODEBUG
opt-out, and with the introduction of
Certificate.SupportedSignatureAlgorithms (#28660) there is a
programmatic way to avoid RSA-PSS (disable TLS 1.3 with MaxVersion and
use that field to specify only PKCS#1 v1.5 SignatureSchemes).

This effectively reverts 0b3a57b,
although following CL 205061 all of the signing-side logic is
conveniently centralized in signatureSchemesForCertificate.

Fixes #32425

Change-Id: I7c9a8893bb5d518d86eae7db82612b9b2cd257d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/205063
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.