diff --git a/auth.go b/auth.go index 94de68a..0c6d240 100644 --- a/auth.go +++ b/auth.go @@ -14,9 +14,11 @@ import ( ) // 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 given public key and the list of algorithms from both sides of connection. +// The lists of signature algorithms (peerSigAlgs and ourSigAlgs) are ignored +// for tlsVersion < VersionTLS12. // -// The returned SignatureScheme codepoint is only meaningful for TLS 1.2, +// The returned SignatureScheme codepoint is only meaningful for TLS 1.2 and newer // previous TLS versions have a fixed hash function. func pickSignatureAlgorithm(pubkey crypto.PublicKey, peerSigAlgs, ourSigAlgs []SignatureScheme, tlsVersion uint16) (SignatureScheme, uint8, crypto.Hash, error) { if tlsVersion < VersionTLS12 || len(peerSigAlgs) == 0 { diff --git a/auth_test.go b/auth_test.go new file mode 100644 index 0000000..3f876b9 --- /dev/null +++ b/auth_test.go @@ -0,0 +1,101 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package tls + +import ( + "crypto" + "testing" +) + +func TestSignatureSelection(t *testing.T) { + rsaCert := &testRSAPrivateKey.PublicKey + ecdsaCert := &testECDSAPrivateKey.PublicKey + sigsPKCS1WithSHA := []SignatureScheme{PKCS1WithSHA256, PKCS1WithSHA1} + sigsPSSWithSHA := []SignatureScheme{PSSWithSHA256, PSSWithSHA384} + sigsECDSAWithSHA := []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithSHA1} + + tests := []struct { + pubkey crypto.PublicKey + peerSigAlgs []SignatureScheme + ourSigAlgs []SignatureScheme + tlsVersion uint16 + + expectedSigAlg SignatureScheme // or 0 if ignored + 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}, + {rsaCert, nil, nil, VersionSSL30, 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}, + + // 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, []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}, + } + + for testNo, test := range tests { + sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion) + if err != nil { + t.Errorf("test[%d]: unexpected error: %v", testNo, err) + } + if test.expectedSigAlg != 0 && test.expectedSigAlg != sigAlg { + t.Errorf("test[%d]: expected signature scheme %#x, got %#x", testNo, test.expectedSigAlg, sigAlg) + } + if test.expectedSigType != sigType { + t.Errorf("test[%d]: expected signature algorithm %#x, got %#x", testNo, test.expectedSigType, sigType) + } + if test.expectedHash != hashFunc { + t.Errorf("test[%d]: expected hash function %#x, got %#x", testNo, test.expectedHash, hashFunc) + } + } + + badTests := []struct { + pubkey crypto.PublicKey + peerSigAlgs []SignatureScheme + ourSigAlgs []SignatureScheme + tlsVersion uint16 + }{ + {rsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12}, + {ecdsaCert, sigsPKCS1WithSHA, sigsPKCS1WithSHA, VersionTLS12}, + {ecdsaCert, sigsECDSAWithSHA, sigsPKCS1WithSHA, VersionTLS12}, + {rsaCert, []SignatureScheme{0}, sigsPKCS1WithSHA, VersionTLS12}, + + // ECDSA is unspecified for SSL 3.0 in RFC 4492. + // TODO a SSL 3.0 client cannot advertise signature_algorithms, + // but if an application feeds an ECDSA certificate anyway, it + // will be accepted rather than trigger a handshake failure. Ok? + //{ecdsaCert, nil, nil, VersionSSL30}, + } + + for testNo, test := range badTests { + sigAlg, sigType, hashFunc, err := pickSignatureAlgorithm(test.pubkey, test.peerSigAlgs, test.ourSigAlgs, test.tlsVersion) + if err == nil { + t.Errorf("test[%d]: unexpected success, got %#x %#x %#x", testNo, sigAlg, sigType, hashFunc) + } + } +} diff --git a/common.go b/common.go index 48c4fdb..7f5021a 100644 --- a/common.go +++ b/common.go @@ -173,9 +173,10 @@ const ( // Rest of these are reserved by the TLS spec ) -// Signature algorithms for TLS 1.2 (See RFC 5246, section A.4.1) +// Signature algorithms (for internal signaling use). Starting at 16 to avoid overlap with +// TLS 1.2 codepoints (RFC 5246, section A.4.1), with which these have nothing to do. const ( - signaturePKCS1v15 uint8 = iota + 1 + signaturePKCS1v15 uint8 = iota + 16 signatureECDSA signatureRSAPSS ) diff --git a/handshake_client_test.go b/handshake_client_test.go index 3c9d122..57ee331 100644 --- a/handshake_client_test.go +++ b/handshake_client_test.go @@ -1581,3 +1581,42 @@ func TestGetClientCertificate(t *testing.T) { } } } + +func TestRSAPSSKeyError(t *testing.T) { + // crypto/tls does not support the rsa_pss_pss_xxx SignatureSchemes. If support for + // public keys with OID RSASSA-PSS is added to crypto/x509, they will be misused with + // the rsa_pss_rsae_xxx SignatureSchemes. Assert that RSASSA-PSS certificates don't + // parse, or that they don't carry *rsa.PublicKey keys. + b, _ := pem.Decode([]byte(` +-----BEGIN CERTIFICATE----- +MIIDZTCCAhygAwIBAgIUCF2x0FyTgZG0CC9QTDjGWkB5vgEwPgYJKoZIhvcNAQEK +MDGgDTALBglghkgBZQMEAgGhGjAYBgkqhkiG9w0BAQgwCwYJYIZIAWUDBAIBogQC +AgDeMBIxEDAOBgNVBAMMB1JTQS1QU1MwHhcNMTgwNjI3MjI0NDM2WhcNMTgwNzI3 +MjI0NDM2WjASMRAwDgYDVQQDDAdSU0EtUFNTMIIBIDALBgkqhkiG9w0BAQoDggEP +ADCCAQoCggEBANxDm0f76JdI06YzsjB3AmmjIYkwUEGxePlafmIASFjDZl/elD0Z +/a7xLX468b0qGxLS5al7XCcEprSdsDR6DF5L520+pCbpfLyPOjuOvGmk9KzVX4x5 +b05YXYuXdsQ0Kjxcx2i3jjCday6scIhMJVgBZxTEyMj1thPQM14SHzKCd/m6HmCL +QmswpH2yMAAcBRWzRpp/vdH5DeOJEB3aelq7094no731mrLUCHRiZ1htq8BDB3ou +czwqgwspbqZ4dnMXl2MvfySQ5wJUxQwILbiuAKO2lVVPUbFXHE9pgtznNoPvKwQT +JNcX8ee8WIZc2SEGzofjk3NpjR+2ADB2u3sCAwEAAaNTMFEwHQYDVR0OBBYEFNEz +AdyJ2f+fU+vSCS6QzohnOnprMB8GA1UdIwQYMBaAFNEzAdyJ2f+fU+vSCS6Qzohn +OnprMA8GA1UdEwEB/wQFMAMBAf8wPgYJKoZIhvcNAQEKMDGgDTALBglghkgBZQME +AgGhGjAYBgkqhkiG9w0BAQgwCwYJYIZIAWUDBAIBogQCAgDeA4IBAQCjEdrR5aab +sZmCwrMeKidXgfkmWvfuLDE+TCbaqDZp7BMWcMQXT9O0UoUT5kqgKj2ARm2pEW0Z +H3Z1vj3bbds72qcDIJXp+l0fekyLGeCrX/CbgnMZXEP7+/+P416p34ChR1Wz4dU1 +KD3gdsUuTKKeMUog3plxlxQDhRQmiL25ygH1LmjLd6dtIt0GVRGr8lj3euVeprqZ +bZ3Uq5eLfsn8oPgfC57gpO6yiN+UURRTlK3bgYvLh4VWB3XXk9UaQZ7Mq1tpXjoD +HYFybkWzibkZp4WRo+Fa28rirH+/wHt0vfeN7UCceURZEx4JaxIIfe4ku7uDRhJi +RwBA9Xk1KBNF +-----END CERTIFICATE-----`)) + if b == nil { + t.Fatal("Failed to decode certificate") + } + cert, err := x509.ParseCertificate(b.Bytes) + if err != nil { + return + } + if _, ok := cert.PublicKey.(*rsa.PublicKey); ok { + t.Error("A RSA-PSS certificate was parsed like a PKCS1 one, and it will be mistakenly used with rsa_pss_rsae_xxx signature algorithms") + } +}