Skip to content

Commit

Permalink
crypto/tls: refactor certificate and signature algorithm logic
Browse files Browse the repository at this point in the history 
This refactors a lot of the certificate support logic to make it cleaner
and reusable where possible. These changes will make the following CLs
much simpler.

In particular, the heavily overloaded pickSignatureAlgorithm is gone.
That function used to cover both signing and verifying side, would work
both for pre-signature_algorithms TLS 1.0/1.1 and TLS 1.2, and returned
sigalg, type and hash.

Now, TLS 1.0/1.1 and 1.2 are differentiated at the caller, as they have
effectively completely different logic. TLS 1.0/1.1 simply use
legacyTypeAndHashFromPublicKey as they employ a fixed hash function and
signature algorithm for each public key type. TLS 1.2 is instead routed
through selectSignatureScheme (on the signing side) or
isSupportedSignatureAlgorithm (on the verifying side) and
typeAndHashFromSignatureScheme, like TLS 1.3.

On the signing side, signatureSchemesForCertificate was already version
aware (for PKCS#1 v1.5 vs PSS support), so selectSignatureScheme just
had to learn the Section 7.4.1.4.1 defaults for a missing
signature_algorithms to replace pickSignatureAlgorithm.

On the verifying side, pickSignatureAlgorithm was also checking the
public key type, while isSupportedSignatureAlgorithm +
typeAndHashFromSignatureScheme are not, but that check was redundant
with the one in verifyHandshakeSignature.

There should be no major change in behavior so far. A few minor changes
came from the refactor: we now correctly require signature_algorithms in
TLS 1.3 when using a certificate; we won't use Ed25519 in TLS 1.2 if the
client didn't send signature_algorithms; and we don't send
ec_points_format in the ServerHello (a compatibility measure) if we are
not doing ECDHE anyway because there are no mutually supported curves.

The tests also got simpler because they test simpler functions. The
caller logic switching between TLS 1.0/1.1 and 1.2 is tested by the
transcript tests.

Updates #32426

Change-Id: Ice9dcaea78d204718f661f8d60efdb408ba41577
Reviewed-on: https://go-review.googlesource.com/c/go/+/205061
Reviewed-by: Katie Hockman <katie@golang.org>
  • Loading branch information
@FiloSottile
FiloSottile committed Nov 12, 2019
1 parent 4faada9 commit ec73263
Show file tree
Hide file tree
Showing 11 changed files with 398 additions and 348 deletions.
147 changes: 81 additions & 66 deletions src/crypto/tls/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,69 +18,6 @@ import (
"io"
)

// pickSignatureAlgorithm selects a signature algorithm that is compatible with
// the given public key and the list of algorithms from the peer and this side.
// The lists of signature algorithms (peerSigAlgs and ourSigAlgs) are ignored
// for tlsVersion < VersionTLS12.
//
// The returned SignatureScheme codepoint is only meaningful for TLS 1.2,
// previous TLS versions have a fixed hash function.
func pickSignatureAlgorithm(pubkey crypto.PublicKey, peerSigAlgs, ourSigAlgs []SignatureScheme, tlsVersion uint16) (sigAlg SignatureScheme, sigType uint8, hashFunc crypto.Hash, err error) {
if tlsVersion < VersionTLS12 || len(peerSigAlgs) == 0 {
// For TLS 1.1 and before, the signature algorithm could not be
// negotiated and the hash is fixed based on the signature type. For TLS
// 1.2, if the client didn't send signature_algorithms extension then we
// can assume that it supports SHA1. See RFC 5246, Section 7.4.1.4.1.
switch pubkey.(type) {
case *rsa.PublicKey:
if tlsVersion < VersionTLS12 {
return 0, signaturePKCS1v15, crypto.MD5SHA1, nil
} else {
return PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1, nil
}
case *ecdsa.PublicKey:
return ECDSAWithSHA1, signatureECDSA, crypto.SHA1, nil
case ed25519.PublicKey:
if tlsVersion < VersionTLS12 {
// RFC 8422 specifies support for Ed25519 in TLS 1.0 and 1.1,
// but it requires holding on to a handshake transcript to do a
// full signature, and not even OpenSSL bothers with the
// complexity, so we can't even test it properly.
return 0, 0, 0, fmt.Errorf("tls: Ed25519 public keys are not supported before TLS 1.2")
}
return Ed25519, signatureEd25519, directSigning, nil
default:
return 0, 0, 0, fmt.Errorf("tls: unsupported public key: %T", pubkey)
}
}
for _, sigAlg := range peerSigAlgs {
if !isSupportedSignatureAlgorithm(sigAlg, ourSigAlgs) {
continue
}
sigType, hashAlg, err := typeAndHashFromSignatureScheme(sigAlg)
if err != nil {
return 0, 0, 0, fmt.Errorf("tls: internal error: %v", err)
}
switch pubkey.(type) {
case *rsa.PublicKey:
if sigType == signaturePKCS1v15 || sigType == signatureRSAPSS {
return sigAlg, sigType, hashAlg, nil
}
case *ecdsa.PublicKey:
if sigType == signatureECDSA {
return sigAlg, sigType, hashAlg, nil
}
case ed25519.PublicKey:
if sigType == signatureEd25519 {
return sigAlg, sigType, hashAlg, nil
}
default:
return 0, 0, 0, fmt.Errorf("tls: unsupported public key: %T", pubkey)
}
}
return 0, 0, 0, errors.New("tls: peer doesn't support any common signature algorithms")
}

// verifyHandshakeSignature verifies a signature against pre-hashed
// (if required) handshake contents.
func verifyHandshakeSignature(sigType uint8, pubkey crypto.PublicKey, hashFunc crypto.Hash, signed, sig []byte) error {
Expand Down Expand Up @@ -164,12 +101,62 @@ func signedMessage(sigHash crypto.Hash, context string, transcript hash.Hash) []
return h.Sum(nil)
}

// typeAndHashFromSignatureScheme returns the corresponding signature type and
// crypto.Hash for a given TLS SignatureScheme.
func typeAndHashFromSignatureScheme(signatureAlgorithm SignatureScheme) (sigType uint8, hash crypto.Hash, err error) {
switch signatureAlgorithm {
case PKCS1WithSHA1, PKCS1WithSHA256, PKCS1WithSHA384, PKCS1WithSHA512:
sigType = signaturePKCS1v15
case PSSWithSHA256, PSSWithSHA384, PSSWithSHA512:
sigType = signatureRSAPSS
case ECDSAWithSHA1, ECDSAWithP256AndSHA256, ECDSAWithP384AndSHA384, ECDSAWithP521AndSHA512:
sigType = signatureECDSA
case Ed25519:
sigType = signatureEd25519
default:
return 0, 0, fmt.Errorf("unsupported signature algorithm: %#04x", signatureAlgorithm)
}
switch signatureAlgorithm {
case PKCS1WithSHA1, ECDSAWithSHA1:
hash = crypto.SHA1
case PKCS1WithSHA256, PSSWithSHA256, ECDSAWithP256AndSHA256:
hash = crypto.SHA256
case PKCS1WithSHA384, PSSWithSHA384, ECDSAWithP384AndSHA384:
hash = crypto.SHA384
case PKCS1WithSHA512, PSSWithSHA512, ECDSAWithP521AndSHA512:
hash = crypto.SHA512
case Ed25519:
hash = directSigning
default:
return 0, 0, fmt.Errorf("unsupported signature algorithm: %#04x", signatureAlgorithm)
}
return sigType, hash, nil
}

// legacyTypeAndHashFromPublicKey returns the fixed signature type and crypto.Hash for
// a given public key used with TLS 1.0 and 1.1, before the introduction of
// signature algorithm negotiation.
func legacyTypeAndHashFromPublicKey(pub crypto.PublicKey) (sigType uint8, hash crypto.Hash, err error) {
switch pub.(type) {
case *rsa.PublicKey:
return signaturePKCS1v15, crypto.MD5SHA1, nil
case *ecdsa.PublicKey:
return signatureECDSA, crypto.SHA1, nil
case ed25519.PublicKey:
// RFC 8422 specifies support for Ed25519 in TLS 1.0 and 1.1,
// but it requires holding on to a handshake transcript to do a
// full signature, and not even OpenSSL bothers with the
// complexity, so we can't even test it properly.
return 0, 0, fmt.Errorf("tls: Ed25519 public keys are not supported before TLS 1.2")
default:
return 0, 0, fmt.Errorf("tls: unsupported public key: %T", pub)
}
}

// signatureSchemesForCertificate returns the list of supported SignatureSchemes
// for a given certificate, based on the public key and the protocol version.
//
// It does not support the crypto.Decrypter interface, so shouldn't be used for
// server certificates in TLS 1.2 and earlier, and it must be kept in sync with
// supportedSignatureAlgorithms.
// This function must be kept in sync with supportedSignatureAlgorithms.
func signatureSchemesForCertificate(version uint16, cert *Certificate) []SignatureScheme {
priv, ok := cert.PrivateKey.(crypto.Signer)
if !ok {
Expand Down Expand Up @@ -201,12 +188,17 @@ func signatureSchemesForCertificate(version uint16, cert *Certificate) []Signatu
case *rsa.PublicKey:
if version != VersionTLS13 {
return []SignatureScheme{
// Temporarily disable RSA-PSS in TLS 1.2, see Issue 32425.
// PSSWithSHA256,
// PSSWithSHA384,
// PSSWithSHA512,
PKCS1WithSHA256,
PKCS1WithSHA384,
PKCS1WithSHA512,
PKCS1WithSHA1,
}
}
// TLS 1.3 dropped support for PKCS#1 v1.5 in favor of RSA-PSS.
return []SignatureScheme{
PSSWithSHA256,
PSSWithSHA384,
Expand All @@ -219,6 +211,29 @@ func signatureSchemesForCertificate(version uint16, cert *Certificate) []Signatu
}
}

// selectSignatureScheme picks a SignatureScheme from the peer's preference list
// that works with the selected certificate. It's only called for protocol
// versions that support signature algorithms, so TLS 1.2 and 1.3.
func selectSignatureScheme(vers uint16, c *Certificate, peerAlgs []SignatureScheme) (SignatureScheme, error) {
supportedAlgs := signatureSchemesForCertificate(vers, c)
if supportedAlgs == nil {
return 0, unsupportedCertificateError(c)
}
if len(peerAlgs) == 0 && vers == VersionTLS12 {
// For TLS 1.2, if the client didn't send signature_algorithms then we
// can assume that it supports SHA1. See RFC 5246, Section 7.4.1.4.1.
peerAlgs = []SignatureScheme{PKCS1WithSHA1, ECDSAWithSHA1}
}
// Pick signature scheme in the peer's preference order, as our
// preference order is not configurable.
for _, preferredAlg := range peerAlgs {
if isSupportedSignatureAlgorithm(preferredAlg, supportedAlgs) {
return preferredAlg, nil
}
}
return 0, errors.New("tls: peer doesn't support any of the certificate's signature algorithms")
}

// unsupportedCertificateError returns a helpful error for certificates with
// an unsupported private key.
func unsupportedCertificateError(cert *Certificate) error {
Expand Down
158 changes: 102 additions & 56 deletions src/crypto/tls/auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,71 +6,62 @@ package tls

import (
"crypto"
"crypto/ed25519"
"testing"
)

func TestSignatureSelection(t *testing.T) {
rsaCert := &testRSAPrivateKey.PublicKey
ecdsaCert := &testECDSAPrivateKey.PublicKey
ed25519Cert := testEd25519PrivateKey.Public().(ed25519.PublicKey)
sigsPKCS1WithSHA := []SignatureScheme{PKCS1WithSHA256, PKCS1WithSHA1}
sigsPSSWithSHA := []SignatureScheme{PSSWithSHA256, PSSWithSHA384}
sigsECDSAWithSHA := []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1}
rsaCert := &Certificate{
Certificate: [][]byte{testRSACertificate},
PrivateKey: testRSAPrivateKey,
}
ecdsaCert := &Certificate{
Certificate: [][]byte{testP256Certificate},
PrivateKey: testP256PrivateKey,
}
ed25519Cert := &Certificate{
Certificate: [][]byte{testEd25519Certificate},
PrivateKey: testEd25519PrivateKey,
}

tests := []struct {
pubkey crypto.PublicKey
cert *Certificate
peerSigAlgs []SignatureScheme
ourSigAlgs []SignatureScheme
tlsVersion uint16

expectedSigAlg SignatureScheme // if tlsVersion == VersionTLS12
expectedSigAlg SignatureScheme
expectedSigType uint8
expectedHash crypto.Hash
}{
// Hash is fixed for RSA in TLS 1.1 and before.
// https://tools.ietf.org/html/rfc4346#page-44
{rsaCert, nil, nil, VersionTLS11, 0, signaturePKCS1v15, crypto.MD5SHA1},
{rsaCert, nil, nil, VersionTLS10, 0, signaturePKCS1v15, crypto.MD5SHA1},

// Before TLS 1.2, there is no signature_algorithms extension
// nor field in CertificateRequest and digitally-signed and thus
// it should be ignored.
{rsaCert, sigsPKCS1WithSHA, nil, VersionTLS11, 0, signaturePKCS1v15, crypto.MD5SHA1},
{rsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS11, 0, signaturePKCS1v15, crypto.MD5SHA1},
// Use SHA-1 for TLS 1.0 and 1.1 with ECDSA, see https://tools.ietf.org/html/rfc4492#page-20
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS11, 0, signatureECDSA, crypto.SHA1},
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS10, 0, signatureECDSA, crypto.SHA1},
{rsaCert, []SignatureScheme{PKCS1WithSHA1, PKCS1WithSHA256}, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{rsaCert, []SignatureScheme{PKCS1WithSHA512, PKCS1WithSHA1}, VersionTLS12, PKCS1WithSHA512, signaturePKCS1v15, crypto.SHA512},
{rsaCert, []SignatureScheme{PSSWithSHA256, PKCS1WithSHA256}, VersionTLS12, PKCS1WithSHA256, signaturePKCS1v15, crypto.SHA256},
{rsaCert, []SignatureScheme{PSSWithSHA384, PKCS1WithSHA1}, VersionTLS13, PSSWithSHA384, signatureRSAPSS, crypto.SHA384},
{ecdsaCert, []SignatureScheme{ECDSAWithSHA1}, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},
{ecdsaCert, []SignatureScheme{ECDSAWithP256AndSHA256}, VersionTLS12, ECDSAWithP256AndSHA256, signatureECDSA, crypto.SHA256},
{ecdsaCert, []SignatureScheme{ECDSAWithP256AndSHA256}, VersionTLS13, ECDSAWithP256AndSHA256, signatureECDSA, crypto.SHA256},
{ed25519Cert, []SignatureScheme{Ed25519}, VersionTLS12, Ed25519, signatureEd25519, directSigning},
{ed25519Cert, []SignatureScheme{Ed25519}, VersionTLS13, Ed25519, signatureEd25519, directSigning},

// TLS 1.2 without signature_algorithms extension
// https://tools.ietf.org/html/rfc5246#page-47
{rsaCert, nil, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{ecdsaCert, nil, sigsPKCS1WithSHA, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},
{rsaCert, nil, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{ecdsaCert, nil, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},

{rsaCert, []SignatureScheme{PKCS1WithSHA1}, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA1, signaturePKCS1v15, crypto.SHA1},
{rsaCert, []SignatureScheme{PKCS1WithSHA256}, sigsPKCS1WithSHA, VersionTLS12, PKCS1WithSHA256, signaturePKCS1v15, crypto.SHA256},
// "sha_hash" may denote hashes other than SHA-1
// https://tools.ietf.org/html/draft-ietf-tls-rfc4492bis-17#page-17
{ecdsaCert, []SignatureScheme{ECDSAWithSHA1}, sigsECDSAWithSHA, VersionTLS12, ECDSAWithSHA1, signatureECDSA, crypto.SHA1},
{ecdsaCert, []SignatureScheme{ECDSAWithP256AndSHA256}, sigsECDSAWithSHA, VersionTLS12, ECDSAWithP256AndSHA256, signatureECDSA, crypto.SHA256},

// RSASSA-PSS is defined in TLS 1.3 for TLS 1.2
// https://tools.ietf.org/html/draft-ietf-tls-tls13-21#page-45
{rsaCert, []SignatureScheme{PSSWithSHA256}, sigsPSSWithSHA, VersionTLS12, PSSWithSHA256, signatureRSAPSS, crypto.SHA256},

// All results are fixed for Ed25519. RFC 8422, Section 5.10.
{ed25519Cert, []SignatureScheme{Ed25519}, []SignatureScheme{ECDSAWithSHA1, Ed25519}, VersionTLS12, Ed25519, signatureEd25519, directSigning},
{ed25519Cert, nil, nil, VersionTLS12, Ed25519, signatureEd25519, directSigning},
// TLS 1.2 does not restrict the ECDSA curve (our ecdsaCert is P-256)
{ecdsaCert, []SignatureScheme{ECDSAWithP384AndSHA384}, VersionTLS12, ECDSAWithP384AndSHA384, signatureECDSA, crypto.SHA384},
}

for testNo, test := range tests {
sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion)
sigAlg, err := selectSignatureScheme(test.tlsVersion, test.cert, test.peerSigAlgs)
if err != nil {
t.Errorf("test[%d]: unexpected error: %v", testNo, err)
t.Errorf("test[%d]: unexpected selectSignatureScheme error: %v", testNo, err)
}
if test.tlsVersion == VersionTLS12 && test.expectedSigAlg != sigAlg {
if test.expectedSigAlg != sigAlg {
t.Errorf("test[%d]: expected signature scheme %#x, got %#x", testNo, test.expectedSigAlg, sigAlg)
}
sigType, hashFunc, err := typeAndHashFromSignatureScheme(sigAlg)
if err != nil {
t.Errorf("test[%d]: unexpected typeAndHashFromSignatureScheme error: %v", testNo, err)
}
if test.expectedSigType != sigType {
t.Errorf("test[%d]: expected signature algorithm %#x, got %#x", testNo, test.expectedSigType, sigType)
}
Expand All @@ -80,26 +71,81 @@ func TestSignatureSelection(t *testing.T) {
}

badTests := []struct {
pubkey crypto.PublicKey
cert *Certificate
peerSigAlgs []SignatureScheme
ourSigAlgs []SignatureScheme
tlsVersion uint16
}{
{rsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12},
{ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS12},
{ecdsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12},
{rsaCert, []SignatureScheme{0}, sigsPKCS1WithSHA, VersionTLS12},
{ed25519Cert, sigsECDSAWithSHA, sigsECDSAWithSHA, VersionTLS12},
{ed25519Cert, []SignatureScheme{Ed25519}, sigsECDSAWithSHA, VersionTLS12},
{ecdsaCert, []SignatureScheme{Ed25519}, []SignatureScheme{Ed25519}, VersionTLS12},
{ed25519Cert, nil, nil, VersionTLS11},
{ed25519Cert, nil, nil, VersionTLS10},
{rsaCert, []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1}, VersionTLS12},
{ecdsaCert, []SignatureScheme{PKCS1WithSHA256, PKCS1WithSHA1}, VersionTLS12},
{rsaCert, []SignatureScheme{0}, VersionTLS12},
{ed25519Cert, []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1}, VersionTLS12},
{ecdsaCert, []SignatureScheme{Ed25519}, VersionTLS12},
// RFC 5246, Section 7.4.1.4.1, says to only consider {sha1,ecdsa} as
// default when the extension is missing, and RFC 8422 does not update
// it. Anyway, if a stack supports Ed25519 it better support sigalgs.
{ed25519Cert, nil, VersionTLS12},
// TLS 1.3 has no default signature_algorithms.
{rsaCert, nil, VersionTLS13},
{ecdsaCert, nil, VersionTLS13},
{ed25519Cert, nil, VersionTLS13},
// Wrong curve, which TLS 1.3 checks
{ecdsaCert, []SignatureScheme{ECDSAWithP384AndSHA384}, VersionTLS13},
// TLS 1.3 does not support PKCS1v1.5 or SHA-1.
{rsaCert, []SignatureScheme{PKCS1WithSHA256}, VersionTLS13},
{ecdsaCert, []SignatureScheme{ECDSAWithSHA1}, VersionTLS13},
}

for testNo, test := range badTests {
sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion)
sigAlg, err := selectSignatureScheme(test.tlsVersion, test.cert, test.peerSigAlgs)
if err == nil {
t.Errorf("test[%d]: unexpected success, got %#x %#x %#x", testNo, sigAlg, sigType, hashFunc)
t.Errorf("test[%d]: unexpected success, got %#x", testNo, sigAlg)
}
}
}

func TestLegacyTypeAndHash(t *testing.T) {
sigType, hashFunc, err := legacyTypeAndHashFromPublicKey(testRSAPrivateKey.Public())
if err != nil {
t.Errorf("RSA: unexpected error: %v", err)
}
if expectedSigType := signaturePKCS1v15; expectedSigType != sigType {
t.Errorf("RSA: expected signature type %#x, got %#x", expectedSigType, sigType)
}
if expectedHashFunc := crypto.MD5SHA1; expectedHashFunc != hashFunc {
t.Errorf("RSA: expected hash %#x, got %#x", expectedHashFunc, sigType)
}

sigType, hashFunc, err = legacyTypeAndHashFromPublicKey(testECDSAPrivateKey.Public())
if err != nil {
t.Errorf("ECDSA: unexpected error: %v", err)
}
if expectedSigType := signatureECDSA; expectedSigType != sigType {
t.Errorf("ECDSA: expected signature type %#x, got %#x", expectedSigType, sigType)
}
if expectedHashFunc := crypto.SHA1; expectedHashFunc != hashFunc {
t.Errorf("ECDSA: expected hash %#x, got %#x", expectedHashFunc, sigType)
}

// Ed25519 is not supported by TLS 1.0 and 1.1.
_, _, err = legacyTypeAndHashFromPublicKey(testEd25519PrivateKey.Public())
if err == nil {
t.Errorf("Ed25519: unexpected success")
}
}

// TestSupportedSignatureAlgorithms checks that all supportedSignatureAlgorithms
// have valid type and hash information.
func TestSupportedSignatureAlgorithms(t *testing.T) {
for _, sigAlg := range supportedSignatureAlgorithms {
sigType, hash, err := typeAndHashFromSignatureScheme(sigAlg)
if err != nil {
t.Errorf("%#04x: unexpected error: %v", sigAlg, err)
}
if sigType == 0 {
t.Errorf("%#04x: missing signature type", sigAlg)
}
if hash == 0 && sigAlg != Ed25519 {
t.Errorf("%#04x: missing hash", sigAlg)
}
}
}
Loading

0 comments on commit ec73263

Please sign in to comment.