test: adds tests for rsa-pss signatures

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 {

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)
+		}
+	}
+}

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
 )

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")
+	}
+}