Implement rsa_pkcs1_sha256_legacy. See draft-davidben-tls13-pkcs1-00. The code point is disabled by default and must be configured in SSL_set_verify_algorithm_prefs and SSL_set_signing_algorithm_prefs. It is also only defined for TLS 1.3 client certificates and otherwise ignored. This required reworking the tests a bit since this is the first signature algorithm that's disabled by default, and the first algorithm that behaves differently between client and server. Change-Id: Iac4aa96a4963cbc33688c252e958a572c5c3b511 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46187 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index f420fda..73b1d59 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1051,6 +1051,11 @@ #define SSL_SIGN_RSA_PSS_RSAE_SHA512 0x0806 #define SSL_SIGN_ED25519 0x0807 +// SSL_SIGN_RSA_PKCS1_SHA256_LEGACY is a backport of RSASSA-PKCS1-v1_5 with +// SHA-256 to TLS 1.3. It is disabled by default and only defined for client +// certificates. +#define SSL_SIGN_RSA_PKCS1_SHA256_LEGACY 0x0420 + // SSL_SIGN_RSA_PKCS1_MD5_SHA1 is an internal signature algorithm used to // specify raw RSASSA-PKCS1-v1_5 with an MD5/SHA-1 concatenation, as used in TLS // before TLS 1.2.
diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index e800136..f8e206d 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc
@@ -99,29 +99,46 @@ int curve; const EVP_MD *(*digest_func)(void); bool is_rsa_pss; + bool tls12_ok; + bool tls13_ok; } SSL_SIGNATURE_ALGORITHM; static const SSL_SIGNATURE_ALGORITHM kSignatureAlgorithms[] = { + // PKCS#1 v1.5 code points are only allowed in TLS 1.2. {SSL_SIGN_RSA_PKCS1_MD5_SHA1, EVP_PKEY_RSA, NID_undef, &EVP_md5_sha1, - false}, - {SSL_SIGN_RSA_PKCS1_SHA1, EVP_PKEY_RSA, NID_undef, &EVP_sha1, false}, - {SSL_SIGN_RSA_PKCS1_SHA256, EVP_PKEY_RSA, NID_undef, &EVP_sha256, false}, - {SSL_SIGN_RSA_PKCS1_SHA384, EVP_PKEY_RSA, NID_undef, &EVP_sha384, false}, - {SSL_SIGN_RSA_PKCS1_SHA512, EVP_PKEY_RSA, NID_undef, &EVP_sha512, false}, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/false}, + {SSL_SIGN_RSA_PKCS1_SHA1, EVP_PKEY_RSA, NID_undef, &EVP_sha1, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/false}, + {SSL_SIGN_RSA_PKCS1_SHA256, EVP_PKEY_RSA, NID_undef, &EVP_sha256, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/false}, + {SSL_SIGN_RSA_PKCS1_SHA384, EVP_PKEY_RSA, NID_undef, &EVP_sha384, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/false}, + {SSL_SIGN_RSA_PKCS1_SHA512, EVP_PKEY_RSA, NID_undef, &EVP_sha512, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/false}, - {SSL_SIGN_RSA_PSS_RSAE_SHA256, EVP_PKEY_RSA, NID_undef, &EVP_sha256, true}, - {SSL_SIGN_RSA_PSS_RSAE_SHA384, EVP_PKEY_RSA, NID_undef, &EVP_sha384, true}, - {SSL_SIGN_RSA_PSS_RSAE_SHA512, EVP_PKEY_RSA, NID_undef, &EVP_sha512, true}, + // Legacy PKCS#1 v1.5 code points are only allowed in TLS 1.3. See + // draft-davidben-tls13-pkcs1-00. + {SSL_SIGN_RSA_PKCS1_SHA256_LEGACY, EVP_PKEY_RSA, NID_undef, &EVP_sha256, + /*is_rsa_pss=*/false, /*tls12_ok=*/false, /*tls13_ok=*/true}, - {SSL_SIGN_ECDSA_SHA1, EVP_PKEY_EC, NID_undef, &EVP_sha1, false}, + {SSL_SIGN_RSA_PSS_RSAE_SHA256, EVP_PKEY_RSA, NID_undef, &EVP_sha256, + /*is_rsa_pss=*/true, /*tls12_ok=*/true, /*tls13_ok=*/true}, + {SSL_SIGN_RSA_PSS_RSAE_SHA384, EVP_PKEY_RSA, NID_undef, &EVP_sha384, + /*is_rsa_pss=*/true, /*tls12_ok=*/true, /*tls13_ok=*/true}, + {SSL_SIGN_RSA_PSS_RSAE_SHA512, EVP_PKEY_RSA, NID_undef, &EVP_sha512, + /*is_rsa_pss=*/true, /*tls12_ok=*/true, /*tls13_ok=*/true}, + + {SSL_SIGN_ECDSA_SHA1, EVP_PKEY_EC, NID_undef, &EVP_sha1, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/false}, {SSL_SIGN_ECDSA_SECP256R1_SHA256, EVP_PKEY_EC, NID_X9_62_prime256v1, - &EVP_sha256, false}, + &EVP_sha256, /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/true}, {SSL_SIGN_ECDSA_SECP384R1_SHA384, EVP_PKEY_EC, NID_secp384r1, &EVP_sha384, - false}, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/true}, {SSL_SIGN_ECDSA_SECP521R1_SHA512, EVP_PKEY_EC, NID_secp521r1, &EVP_sha512, - false}, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/true}, - {SSL_SIGN_ED25519, EVP_PKEY_ED25519, NID_undef, nullptr, false}, + {SSL_SIGN_ED25519, EVP_PKEY_ED25519, NID_undef, nullptr, + /*is_rsa_pss=*/false, /*tls12_ok=*/true, /*tls13_ok=*/true}, }; static const SSL_SIGNATURE_ALGORITHM *get_signature_algorithm(uint16_t sigalg) { @@ -144,7 +161,7 @@ } static bool pkey_supports_algorithm(const SSL *ssl, EVP_PKEY *pkey, - uint16_t sigalg) { + uint16_t sigalg, bool is_verify) { const SSL_SIGNATURE_ALGORITHM *alg = get_signature_algorithm(sigalg); if (alg == NULL || EVP_PKEY_id(pkey) != alg->pkey_type) { @@ -152,8 +169,14 @@ } if (ssl_protocol_version(ssl) >= TLS1_3_VERSION) { - // RSA keys may only be used with RSA-PSS. - if (alg->pkey_type == EVP_PKEY_RSA && !alg->is_rsa_pss) { + if (!alg->tls13_ok) { + return false; + } + + // Legacy PKCS#1 v1.5 code points for TLS 1.3 are client-only. See + // draft-davidben-tls13-pkcs1-00. + bool is_client_sign = ssl->server == is_verify; + if (alg->pkey_type == EVP_PKEY_RSA && !alg->is_rsa_pss && !is_client_sign) { return false; } @@ -164,6 +187,8 @@ EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(pkey))) != alg->curve)) { return false; } + } else if (!alg->tls12_ok) { + return false; } return true; @@ -171,7 +196,7 @@ static bool setup_ctx(SSL *ssl, EVP_MD_CTX *ctx, EVP_PKEY *pkey, uint16_t sigalg, bool is_verify) { - if (!pkey_supports_algorithm(ssl, pkey, sigalg)) { + if (!pkey_supports_algorithm(ssl, pkey, sigalg, is_verify)) { OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SIGNATURE_TYPE); return false; } @@ -288,7 +313,8 @@ bool ssl_private_key_supports_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t sigalg) { SSL *const ssl = hs->ssl; - if (!pkey_supports_algorithm(ssl, hs->local_pubkey.get(), sigalg)) { + if (!pkey_supports_algorithm(ssl, hs->local_pubkey.get(), sigalg, + /*is_verify=*/false)) { return false; }
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index f209f4a..1393e25 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -211,6 +211,9 @@ // EdDSA algorithms signatureEd25519 signatureAlgorithm = 0x0807 signatureEd448 signatureAlgorithm = 0x0808 + + // draft-davidben-tls13-pkcs1-00 + signatureRSAPKCS1WithSHA256Legacy signatureAlgorithm = 0x0420 ) // supportedSignatureAlgorithms contains the default supported signature
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index efb8a18..524e36d 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -1070,7 +1070,7 @@ c.peerSignatureAlgorithm = certVerifyMsg.signatureAlgorithm input := hs.finishedHash.certificateVerifyInput(serverCertificateVerifyContextTLS13) - err = verifyMessage(c.vers, hs.peerPublicKey, c.config, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature) + err = verifyMessage(c.isClient, c.vers, hs.peerPublicKey, c.config, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature) if err != nil { return err } @@ -1214,14 +1214,14 @@ privKey := chainToSend.PrivateKey var err error - certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.vers, privKey, c.config, certReq.signatureAlgorithms) + certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.isClient, c.vers, privKey, c.config, certReq.signatureAlgorithms) if err != nil { c.sendAlert(alertInternalError) return err } input := hs.finishedHash.certificateVerifyInput(clientCertificateVerifyContextTLS13) - certVerify.signature, err = signMessage(c.vers, privKey, c.config, certVerify.signatureAlgorithm, input) + certVerify.signature, err = signMessage(c.isClient, c.vers, privKey, c.config, certVerify.signatureAlgorithm, input) if err != nil { c.sendAlert(alertInternalError) return err @@ -1437,7 +1437,7 @@ privKey := c.config.Certificates[0].PrivateKey if certVerify.hasSignatureAlgorithm { - certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.vers, privKey, c.config, certReq.signatureAlgorithms) + certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.isClient, c.vers, privKey, c.config, certReq.signatureAlgorithms) if err != nil { c.sendAlert(alertInternalError) return err @@ -1445,7 +1445,7 @@ } if c.vers > VersionSSL30 { - certVerify.signature, err = signMessage(c.vers, privKey, c.config, certVerify.signatureAlgorithm, hs.finishedHash.buffer) + certVerify.signature, err = signMessage(c.isClient, c.vers, privKey, c.config, certVerify.signatureAlgorithm, hs.finishedHash.buffer) if err == nil && c.config.Bugs.SendSignatureAlgorithm != 0 { certVerify.signatureAlgorithm = c.config.Bugs.SendSignatureAlgorithm } @@ -1576,13 +1576,7 @@ return errors.New("tls: failed to parse public key from delegated credential: " + err.Error()) } - verifier, err := getSigner(c.vers, hs.peerPublicKey, c.config, dc.algorithm, true) - if err != nil { - c.sendAlert(alertBadCertificate) - return errors.New("tls: failed to get verifier for delegated credential: " + err.Error()) - } - - if err := verifier.verifyMessage(leafPublicKey, delegatedCredentialSignedMessage(dc.signedBytes, dc.algorithm, certs[0].Raw), dc.signature); err != nil { + if err := verifyMessage(c.isClient, c.vers, leafPublicKey, c.config, dc.algorithm, delegatedCredentialSignedMessage(dc.signedBytes, dc.algorithm, certs[0].Raw), dc.signature); err != nil { c.sendAlert(alertBadCertificate) return errors.New("tls: failed to verify delegated credential: " + err.Error()) }
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 907ea6f..7749f33 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -976,14 +976,14 @@ privKey := hs.cert.PrivateKey var err error - certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.vers, privKey, config, hs.clientHello.signatureAlgorithms) + certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.isClient, c.vers, privKey, config, hs.clientHello.signatureAlgorithms) if err != nil { c.sendAlert(alertInternalError) return err } input := hs.finishedHash.certificateVerifyInput(serverCertificateVerifyContextTLS13) - certVerify.signature, err = signMessage(c.vers, privKey, c.config, certVerify.signatureAlgorithm, input) + certVerify.signature, err = signMessage(c.isClient, c.vers, privKey, c.config, certVerify.signatureAlgorithm, input) if err != nil { c.sendAlert(alertInternalError) return err @@ -1147,7 +1147,7 @@ c.peerSignatureAlgorithm = certVerify.signatureAlgorithm input := hs.finishedHash.certificateVerifyInput(clientCertificateVerifyContextTLS13) - if err := verifyMessage(c.vers, pub, config, certVerify.signatureAlgorithm, input, certVerify.signature); err != nil { + if err := verifyMessage(c.isClient, c.vers, pub, config, certVerify.signatureAlgorithm, input, certVerify.signature); err != nil { c.sendAlert(alertBadCertificate) return err } @@ -1884,7 +1884,7 @@ } if c.vers > VersionSSL30 { - err = verifyMessage(c.vers, pub, c.config, sigAlg, hs.finishedHash.buffer, certVerify.signature) + err = verifyMessage(c.isClient, c.vers, pub, c.config, sigAlg, hs.finishedHash.buffer, certVerify.signature) } else { // SSL 3.0's client certificate construction is // incompatible with signatureAlgorithm.
diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go index b435f18..1461115 100644 --- a/ssl/test/runner/key_agreement.go +++ b/ssl/test/runner/key_agreement.go
@@ -65,13 +65,13 @@ var sigAlg signatureAlgorithm if ka.version >= VersionTLS12 { - sigAlg, err = selectSignatureAlgorithm(ka.version, cert.PrivateKey, config, clientHello.signatureAlgorithms) + sigAlg, err = selectSignatureAlgorithm(false /* server */, ka.version, cert.PrivateKey, config, clientHello.signatureAlgorithms) if err != nil { return nil, err } } - sig, err := signMessage(ka.version, cert.PrivateKey, config, sigAlg, serverRSAParams) + sig, err := signMessage(false /* server */, ka.version, cert.PrivateKey, config, sigAlg, serverRSAParams) if err != nil { return nil, errors.New("failed to sign RSA parameters: " + err.Error()) } @@ -492,13 +492,13 @@ var sigAlg signatureAlgorithm var err error if ka.version >= VersionTLS12 { - sigAlg, err = selectSignatureAlgorithm(ka.version, cert.PrivateKey, config, clientHello.signatureAlgorithms) + sigAlg, err = selectSignatureAlgorithm(false /* server */, ka.version, cert.PrivateKey, config, clientHello.signatureAlgorithms) if err != nil { return nil, err } } - sig, err := signMessage(ka.version, cert.PrivateKey, config, sigAlg, msg) + sig, err := signMessage(false /* server */, ka.version, cert.PrivateKey, config, sigAlg, msg) if err != nil { return nil, err } @@ -574,7 +574,7 @@ } sig = sig[2:] - return verifyMessage(ka.version, publicKey, config, sigAlg, msg, sig) + return verifyMessage(true /* client */, ka.version, publicKey, config, sigAlg, msg, sig) } // ecdheKeyAgreement implements a TLS key agreement where the server
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 6a2ef15..dd3c08c 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -102,6 +102,10 @@ // This is currently used to control tests that enable all curves but may // automatically disable tests in the future. AllCurves []int + + // AllSignatureAlgorithms is the list of all signature algorithm code points + // supported by the shim. + AllSignatureAlgorithms []int } // Setup shimConfig defaults aligning with BoringSSL. @@ -390,12 +394,7 @@ dc = append(dc, pubBytes...) var dummyConfig Config - parentSigner, err := getSigner(tlsVersion, parentPriv, &dummyConfig, config.algo, false /* not for verification */) - if err != nil { - return nil, nil, err - } - - parentSignature, err := parentSigner.signMessage(parentPriv, &dummyConfig, delegatedCredentialSignedMessage(dc, config.algo, parentDER)) + parentSignature, err := signMessage(false /* server */, tlsVersion, parentPriv, &dummyConfig, config.algo, delegatedCredentialSignedMessage(dc, config.algo, parentDER)) if err != nil { return nil, nil, err } @@ -1645,16 +1644,16 @@ case failed && !test.shouldFail: msg = "unexpected failure" case !failed && test.shouldFail: - msg = "unexpected success" + msg = fmt.Sprintf("unexpected success (wanted failure with %q / %q)", expectedError, test.expectedLocalError) case failed && !correctFailure: - msg = "bad error (wanted '" + expectedError + "' / '" + test.expectedLocalError + "')" + msg = fmt.Sprintf("bad error (wanted %q / %q)", expectedError, test.expectedLocalError) case mustFail: msg = "test failure" default: panic("internal error") } - return fmt.Errorf("%s: local error '%s', child error '%s', stdout:\n%s\nstderr:\n%s\n%s", msg, localError, childError, stdout, stderr, extraStderr) + return fmt.Errorf("%s: local error %q, child error %q, stdout:\n%s\nstderr:\n%s\n%s", msg, localError, childError, stdout, stderr, extraStderr) } if len(extraStderr) > 0 || (!failed && len(stderr) > 0) { @@ -9894,6 +9893,7 @@ }{ {"RSA_PKCS1_SHA1", signatureRSAPKCS1WithSHA1, testCertRSA}, {"RSA_PKCS1_SHA256", signatureRSAPKCS1WithSHA256, testCertRSA}, + {"RSA_PKCS1_SHA256_LEGACY", signatureRSAPKCS1WithSHA256Legacy, testCertRSA}, {"RSA_PKCS1_SHA384", signatureRSAPKCS1WithSHA384, testCertRSA}, {"RSA_PKCS1_SHA512", signatureRSAPKCS1WithSHA512, testCertRSA}, {"ECDSA_SHA1", signatureECDSAWithSHA1, testCertECDSAP256}, @@ -9942,51 +9942,94 @@ continue } - var shouldFail, rejectByDefault bool - // ecdsa_sha1 does not exist in TLS 1.3. - if ver.version >= VersionTLS13 && alg.id == signatureECDSAWithSHA1 { - shouldFail = true - } - // RSA-PKCS1 does not exist in TLS 1.3. - if ver.version >= VersionTLS13 && hasComponent(alg.name, "PKCS1") { - shouldFail = true - } - // SHA-224 has been removed from TLS 1.3 and, in 1.3, - // the curve has to match the hash size. - if ver.version >= VersionTLS13 && alg.cert == testCertECDSAP224 { - shouldFail = true - } - - // By default, BoringSSL does not enable ecdsa_sha1, ecdsa_secp521_sha512, and ed25519. - if alg.id == signatureECDSAWithSHA1 || alg.id == signatureECDSAWithP521AndSHA512 || alg.id == signatureEd25519 { - rejectByDefault = true - } - - var signError, signLocalError, verifyError, verifyLocalError, defaultError, defaultLocalError string - if shouldFail { - signError = ":NO_COMMON_SIGNATURE_ALGORITHMS:" - signLocalError = "remote error: handshake failure" - verifyError = ":WRONG_SIGNATURE_TYPE:" - verifyLocalError = "remote error" - rejectByDefault = true - } - if rejectByDefault { - defaultError = ":WRONG_SIGNATURE_TYPE:" - defaultLocalError = "remote error" - } - suffix := "-" + alg.name + "-" + ver.name + for _, signTestType := range []testType{clientTest, serverTest} { + signPrefix := "Client-" + verifyPrefix := "Server-" + verifyTestType := serverTest + if signTestType == serverTest { + verifyTestType = clientTest + signPrefix, verifyPrefix = verifyPrefix, signPrefix + } - for _, testType := range []testType{clientTest, serverTest} { - prefix := "Client-" - if testType == serverTest { - prefix = "Server-" + var shouldFail bool + isTLS12PKCS1 := hasComponent(alg.name, "PKCS1") && !hasComponent(alg.name, "LEGACY") + isTLS13PKCS1 := hasComponent(alg.name, "PKCS1") && hasComponent(alg.name, "LEGACY") + + // TLS 1.3 removes a number of signature algorithms. + if ver.version >= VersionTLS13 && (alg.cert == testCertECDSAP224 || alg.id == signatureECDSAWithSHA1 || isTLS12PKCS1) { + shouldFail = true + } + + // The backported RSA-PKCS1 code points only exist for TLS 1.3 + // client certificates. + if (ver.version < VersionTLS13 || signTestType == serverTest) && isTLS13PKCS1 { + shouldFail = true + } + + // By default, BoringSSL does not sign with these algorithms. + signDefault := true + if isTLS13PKCS1 { + signDefault = false + } + + // By default, BoringSSL does not accept these algorithms. + verifyDefault := true + if alg.id == signatureECDSAWithSHA1 || alg.id == signatureECDSAWithP521AndSHA512 || alg.id == signatureEd25519 || isTLS13PKCS1 { + verifyDefault = false + } + + var signError, signLocalError, verifyError, verifyLocalError string + if shouldFail { + signError = ":NO_COMMON_SIGNATURE_ALGORITHMS:" + signLocalError = "remote error: handshake failure" + verifyError = ":WRONG_SIGNATURE_TYPE:" + verifyLocalError = "remote error" + signDefault = false + verifyDefault = false + } + var signDefaultError, signDefaultLocalError string + if !signDefault { + signDefaultError = ":NO_COMMON_SIGNATURE_ALGORITHMS:" + signDefaultLocalError = "remote error: handshake failure" + } + var verifyDefaultError, verifyDefaultLocalError string + if !verifyDefault { + verifyDefaultError = ":WRONG_SIGNATURE_TYPE:" + verifyDefaultLocalError = "remote error" } // Test the shim using the algorithm for signing. + signTestFlags := []string{ + "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)), + "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)), + } + signTestFlags = append(signTestFlags, flagInts("-curves", shimConfig.AllCurves)...) + signTestFlags = append(signTestFlags, flagInts("-signing-prefs", shimConfig.AllSignatureAlgorithms)...) signTest := testCase{ - testType: testType, - name: prefix + "Sign" + suffix, + testType: signTestType, + name: signPrefix + "Sign" + suffix, + config: Config{ + MaxVersion: ver.version, + VerifySignatureAlgorithms: []signatureAlgorithm{ + fakeSigAlg1, + alg.id, + fakeSigAlg2, + }, + }, + flags: signTestFlags, + shouldFail: shouldFail, + expectedError: signError, + expectedLocalError: signLocalError, + expectations: connectionExpectations{ + peerSignatureAlgorithm: alg.id, + }, + } + + // Test whether the shim enables the algorithm by default. + signDefaultTest := testCase{ + testType: signTestType, + name: signPrefix + "SignDefault" + suffix, config: Config{ MaxVersion: ver.version, VerifySignatureAlgorithms: []signatureAlgorithm{ @@ -10002,9 +10045,9 @@ }, flagInts("-curves", shimConfig.AllCurves)..., ), - shouldFail: shouldFail, - expectedError: signError, - expectedLocalError: signLocalError, + shouldFail: !signDefault, + expectedError: signDefaultError, + expectedLocalError: signDefaultLocalError, expectations: connectionExpectations{ peerSignatureAlgorithm: alg.id, }, @@ -10013,8 +10056,8 @@ // Test that the shim will select the algorithm when configured to only // support it. negotiateTest := testCase{ - testType: testType, - name: prefix + "Sign-Negotiate" + suffix, + testType: signTestType, + name: signPrefix + "Sign-Negotiate" + suffix, config: Config{ MaxVersion: ver.version, VerifySignatureAlgorithms: allAlgorithms, @@ -10032,24 +10075,26 @@ }, } - if testType == serverTest { + if signTestType == serverTest { // TLS 1.2 servers only sign on some cipher suites. signTest.config.CipherSuites = signingCiphers + signDefaultTest.config.CipherSuites = signingCiphers negotiateTest.config.CipherSuites = signingCiphers } else { // TLS 1.2 clients only sign when the server requests certificates. signTest.config.ClientAuth = RequireAnyClientCert + signDefaultTest.config.ClientAuth = RequireAnyClientCert negotiateTest.config.ClientAuth = RequireAnyClientCert } - testCases = append(testCases, signTest) + testCases = append(testCases, signTest, signDefaultTest) if ver.version >= VersionTLS12 && !shouldFail { testCases = append(testCases, negotiateTest) } // Test the shim using the algorithm for verifying. verifyTest := testCase{ - testType: testType, - name: prefix + "Verify" + suffix, + testType: verifyTestType, + name: verifyPrefix + "Verify" + suffix, config: Config{ MaxVersion: ver.version, Certificates: []Certificate{getRunnerCertificate(alg.cert)}, @@ -10081,8 +10126,8 @@ // Test whether the shim expects the algorithm enabled by default. defaultTest := testCase{ - testType: testType, - name: prefix + "VerifyDefault" + suffix, + testType: verifyTestType, + name: verifyPrefix + "VerifyDefault" + suffix, config: Config{ MaxVersion: ver.version, Certificates: []Certificate{getRunnerCertificate(alg.cert)}, @@ -10090,10 +10135,10 @@ alg.id, }, Bugs: ProtocolBugs{ - SkipECDSACurveCheck: rejectByDefault, - IgnoreSignatureVersionChecks: rejectByDefault, + SkipECDSACurveCheck: !verifyDefault, + IgnoreSignatureVersionChecks: !verifyDefault, // Some signature algorithms may not be advertised. - IgnorePeerSignatureAlgorithmPreferences: rejectByDefault, + IgnorePeerSignatureAlgorithmPreferences: !verifyDefault, }, }, flags: append( @@ -10102,16 +10147,16 @@ ), // Resume the session to assert the peer signature // algorithm is reported on both handshakes. - resumeSession: !rejectByDefault, - shouldFail: rejectByDefault, - expectedError: defaultError, - expectedLocalError: defaultLocalError, + resumeSession: verifyDefault, + shouldFail: !verifyDefault, + expectedError: verifyDefaultError, + expectedLocalError: verifyDefaultLocalError, } // Test whether the shim handles invalid signatures for this algorithm. invalidTest := testCase{ - testType: testType, - name: prefix + "InvalidSignature" + suffix, + testType: verifyTestType, + name: verifyPrefix + "InvalidSignature" + suffix, config: Config{ MaxVersion: ver.version, Certificates: []Certificate{getRunnerCertificate(alg.cert)}, @@ -10131,7 +10176,7 @@ expectedError: ":BAD_SIGNATURE:", } - if testType == serverTest { + if verifyTestType == serverTest { // TLS 1.2 servers only verify when they request client certificates. verifyTest.flags = append(verifyTest.flags, "-require-any-client-certificate") defaultTest.flags = append(defaultTest.flags, "-require-any-client-certificate") @@ -16798,6 +16843,14 @@ } } + if shimConfig.AllSignatureAlgorithms == nil { + for _, alg := range testSignatureAlgorithms { + if alg.id != 0 { + shimConfig.AllSignatureAlgorithms = append(shimConfig.AllSignatureAlgorithms, int(alg.id)) + } + } + } + addBasicTests() addCipherSuiteTests() addBadECDSASignatureTests()
diff --git a/ssl/test/runner/sign.go b/ssl/test/runner/sign.go index d57cd60..87e93e2 100644 --- a/ssl/test/runner/sign.go +++ b/ssl/test/runner/sign.go
@@ -26,7 +26,7 @@ verifyMessage(key crypto.PublicKey, msg, sig []byte) error } -func selectSignatureAlgorithm(version uint16, key crypto.PrivateKey, config *Config, peerSigAlgs []signatureAlgorithm) (signatureAlgorithm, error) { +func selectSignatureAlgorithm(isClient bool, version uint16, key crypto.PrivateKey, config *Config, peerSigAlgs []signatureAlgorithm) (signatureAlgorithm, error) { // If the client didn't specify any signature_algorithms extension then // we can assume that it supports SHA1. See // http://tools.ietf.org/html/rfc5246#section-7.4.1.4.1 @@ -39,7 +39,7 @@ continue } - signer, err := getSigner(version, key, config, sigAlg, false) + signer, err := getSigner(isClient, version, key, config, sigAlg, false) if err != nil { continue } @@ -51,7 +51,7 @@ return 0, errors.New("tls: no common signature algorithms") } -func signMessage(version uint16, key crypto.PrivateKey, config *Config, sigAlg signatureAlgorithm, msg []byte) ([]byte, error) { +func signMessage(isClient bool, version uint16, key crypto.PrivateKey, config *Config, sigAlg signatureAlgorithm, msg []byte) ([]byte, error) { if config.Bugs.InvalidSignature { newMsg := make([]byte, len(msg)) copy(newMsg, msg) @@ -59,7 +59,7 @@ msg = newMsg } - signer, err := getSigner(version, key, config, sigAlg, false) + signer, err := getSigner(isClient, version, key, config, sigAlg, false) if err != nil { return nil, err } @@ -67,12 +67,12 @@ return signer.signMessage(key, config, msg) } -func verifyMessage(version uint16, key crypto.PublicKey, config *Config, sigAlg signatureAlgorithm, msg, sig []byte) error { +func verifyMessage(isClient bool, version uint16, key crypto.PublicKey, config *Config, sigAlg signatureAlgorithm, msg, sig []byte) error { if version >= VersionTLS12 && !isSupportedSignatureAlgorithm(sigAlg, config.verifySignatureAlgorithms()) { return errors.New("tls: unsupported signature algorithm") } - signer, err := getSigner(version, key, config, sigAlg, true) + signer, err := getSigner(isClient, version, key, config, sigAlg, true) if err != nil { return err } @@ -272,7 +272,7 @@ return nil } -func getSigner(version uint16, key interface{}, config *Config, sigAlg signatureAlgorithm, isVerify bool) (signer, error) { +func getSigner(isClient bool, version uint16, key interface{}, config *Config, sigAlg signatureAlgorithm, isVerify bool) (signer, error) { // TLS 1.1 and below use legacy signature algorithms. if version < VersionTLS12 { if config.Bugs.UseLegacySigningAlgorithm == 0 || isVerify { @@ -290,6 +290,7 @@ sigAlg = config.Bugs.UseLegacySigningAlgorithm } + isClientSign := isClient != isVerify switch sigAlg { case signatureRSAPKCS1WithMD5: if version < VersionTLS13 || config.Bugs.IgnoreSignatureVersionChecks { @@ -311,6 +312,10 @@ if version < VersionTLS13 || config.Bugs.IgnoreSignatureVersionChecks { return &rsaPKCS1Signer{crypto.SHA512}, nil } + case signatureRSAPKCS1WithSHA256Legacy: + if (isClientSign && version >= VersionTLS13) || config.Bugs.IgnoreSignatureVersionChecks { + return &rsaPKCS1Signer{crypto.SHA256}, nil + } case signatureECDSAWithSHA1: return &ecdsaSigner{version, config, nil, crypto.SHA1}, nil case signatureECDSAWithP256AndSHA256: