Remove SSL_CTX_set_rsa_pss_rsae_certs_enabled. We never ended up using this, and it'll only become less relevant over time. Change-Id: I44c750aee24df8e9eecc28b46540d8b3139004ff Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39608 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index d23d0f2..da77311 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -2524,13 +2524,6 @@ // Ed25519. OPENSSL_EXPORT void SSL_CTX_set_ed25519_enabled(SSL_CTX *ctx, int enabled); -// SSL_CTX_set_rsa_pss_rsae_certs_enabled configures whether |ctx| advertises -// support for rsa_pss_rsae_* signatures within the certificate chain. It is -// enabled by default but should be disabled if using a custom certificate -// verifier which does not support RSA-PSS signatures. -OPENSSL_EXPORT void SSL_CTX_set_rsa_pss_rsae_certs_enabled(SSL_CTX *ctx, - int enabled); - // SSL_CTX_set_verify_algorithm_prefs configures |ctx| to use |prefs| as the // preference list when verifying signature's from the peer's long-term key. It // returns one on zero on error. |prefs| should not include the internal-only
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index c7d7fb6..dfe14bf 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -1092,12 +1092,9 @@ !CBB_add_u8_length_prefixed(&body, &cert_types) || !CBB_add_u8(&cert_types, SSL3_CT_RSA_SIGN) || !CBB_add_u8(&cert_types, TLS_CT_ECDSA_SIGN) || - // TLS 1.2 has no way to specify different signature algorithms for - // certificates and the online signature, so emit the more restrictive - // certificate list. (ssl_protocol_version(ssl) >= TLS1_2_VERSION && (!CBB_add_u16_length_prefixed(&body, &sigalgs_cbb) || - !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb, true /* certs */))) || + !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb))) || !ssl_add_client_CA_list(hs, &body) || !ssl_add_message_cbb(ssl, cbb.get())) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
diff --git a/ssl/internal.h b/ssl/internal.h index 8b55a9a..bf4dd2f 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1998,10 +1998,8 @@ Span<const uint16_t> tls1_get_peer_verify_algorithms(const SSL_HANDSHAKE *hs); // tls12_add_verify_sigalgs adds the signature algorithms acceptable for the -// peer signature to |out|. It returns true on success and false on error. If -// |for_certs| is true, the potentially more restrictive list of algorithms for -// certificates is used. Otherwise, the online signature one is used. -bool tls12_add_verify_sigalgs(const SSL *ssl, CBB *out, bool for_certs); +// peer signature to |out|. It returns true on success and false on error. +bool tls12_add_verify_sigalgs(const SSL *ssl, CBB *out); // tls12_check_peer_sigalg checks if |sigalg| is acceptable for the peer // signature. It returns true on success and false on error, setting @@ -2009,11 +2007,6 @@ bool tls12_check_peer_sigalg(const SSL *ssl, uint8_t *out_alert, uint16_t sigalg); -// tls12_has_different_verify_sigalgs_for_certs returns whether |ssl| has a -// different, more restrictive, list of signature algorithms acceptable for the -// certificate than the online signature. -bool tls12_has_different_verify_sigalgs_for_certs(const SSL *ssl); - // Underdocumented functions. // @@ -3317,10 +3310,6 @@ // ed25519_enabled is whether Ed25519 is advertised in the handshake. bool ed25519_enabled : 1; - // rsa_pss_rsae_certs_enabled is whether rsa_pss_rsae_* are supported by the - // certificate verifier. - bool rsa_pss_rsae_certs_enabled : 1; - // false_start_allowed_without_alpn is whether False Start (if // |SSL_MODE_ENABLE_FALSE_START| is enabled) is allowed without ALPN. bool false_start_allowed_without_alpn : 1;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 0e0a6cb..4764000 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -565,7 +565,6 @@ grease_enabled(false), allow_unknown_alpn_protos(false), ed25519_enabled(false), - rsa_pss_rsae_certs_enabled(true), false_start_allowed_without_alpn(false), ignore_tls13_downgrade(false), handoff(false),
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 0a1cef4..7958b5c 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc
@@ -411,10 +411,6 @@ // kVerifySignatureAlgorithms is the default list of accepted signature // algorithms for verifying. -// -// For now, RSA-PSS signature algorithms are not enabled on Android's system -// BoringSSL. Once the change in Chrome has stuck and the values are finalized, -// restore them. static const uint16_t kVerifySignatureAlgorithms[] = { // List our preferred algorithms first. SSL_SIGN_ED25519, @@ -432,15 +428,10 @@ // For now, SHA-1 is still accepted but least preferable. SSL_SIGN_RSA_PKCS1_SHA1, - }; // kSignSignatureAlgorithms is the default list of supported signature // algorithms for signing. -// -// For now, RSA-PSS signature algorithms are not enabled on Android's system -// BoringSSL. Once the change in Chrome has stuck and the values are finalized, -// restore them. static const uint16_t kSignSignatureAlgorithms[] = { // List our preferred algorithms first. SSL_SIGN_ED25519, @@ -472,39 +463,17 @@ if (skip_ed25519 && sigalg == SSL_SIGN_ED25519) { continue; } - if (skip_rsa_pss_rsae && SSL_is_signature_algorithm_rsa_pss(sigalg)) { - continue; - } *out = sigalg; return true; } return false; } - bool operator==(const SSLSignatureAlgorithmList &other) const { - SSLSignatureAlgorithmList a = *this; - SSLSignatureAlgorithmList b = other; - uint16_t a_val, b_val; - while (a.Next(&a_val)) { - if (!b.Next(&b_val) || - a_val != b_val) { - return false; - } - } - return !b.Next(&b_val); - } - - bool operator!=(const SSLSignatureAlgorithmList &other) const { - return !(*this == other); - } - Span<const uint16_t> list; bool skip_ed25519 = false; - bool skip_rsa_pss_rsae = false; }; -static SSLSignatureAlgorithmList tls12_get_verify_sigalgs(const SSL *ssl, - bool for_certs) { +static SSLSignatureAlgorithmList tls12_get_verify_sigalgs(const SSL *ssl) { SSLSignatureAlgorithmList ret; if (!ssl->config->verify_sigalgs.empty()) { ret.list = ssl->config->verify_sigalgs; @@ -512,14 +481,11 @@ ret.list = kVerifySignatureAlgorithms; ret.skip_ed25519 = !ssl->ctx->ed25519_enabled; } - if (for_certs) { - ret.skip_rsa_pss_rsae = !ssl->ctx->rsa_pss_rsae_certs_enabled; - } return ret; } -bool tls12_add_verify_sigalgs(const SSL *ssl, CBB *out, bool for_certs) { - SSLSignatureAlgorithmList list = tls12_get_verify_sigalgs(ssl, for_certs); +bool tls12_add_verify_sigalgs(const SSL *ssl, CBB *out) { + SSLSignatureAlgorithmList list = tls12_get_verify_sigalgs(ssl); uint16_t sigalg; while (list.Next(&sigalg)) { if (!CBB_add_u16(out, sigalg)) { @@ -531,7 +497,7 @@ bool tls12_check_peer_sigalg(const SSL *ssl, uint8_t *out_alert, uint16_t sigalg) { - SSLSignatureAlgorithmList list = tls12_get_verify_sigalgs(ssl, false); + SSLSignatureAlgorithmList list = tls12_get_verify_sigalgs(ssl); uint16_t verify_sigalg; while (list.Next(&verify_sigalg)) { if (verify_sigalg == sigalg) { @@ -544,11 +510,6 @@ return false; } -bool tls12_has_different_verify_sigalgs_for_certs(const SSL *ssl) { - return tls12_get_verify_sigalgs(ssl, true) != - tls12_get_verify_sigalgs(ssl, false); -} - // tls_extension represents a TLS extension that is handled internally. The // |init| function is called for each handshake, before any other functions of // the extension. Then the add and parse callbacks are called as needed. @@ -980,23 +941,11 @@ return true; } - // Prior to TLS 1.3, there was no way to signal different signature algorithm - // preferences between the online signature and certificates. If we do not - // send the signature_algorithms_cert extension, use the potentially more - // restrictive certificate list. - // - // TODO(davidben): When TLS 1.3 is finalized, we can likely remove the TLS 1.3 - // check both here and in signature_algorithms_cert. |hs->max_version| is not - // the negotiated version. Rather the expectation is that any server consuming - // signature algorithms added in TLS 1.3 will also know to look at - // signature_algorithms_cert. For now, TLS 1.3 is not quite yet final and it - // seems prudent to condition this new extension on it. - bool for_certs = hs->max_version < TLS1_3_VERSION; CBB contents, sigalgs_cbb; if (!CBB_add_u16(out, TLSEXT_TYPE_signature_algorithms) || !CBB_add_u16_length_prefixed(out, &contents) || !CBB_add_u16_length_prefixed(&contents, &sigalgs_cbb) || - !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb, for_certs) || + !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb) || !CBB_flush(out)) { return false; } @@ -1022,35 +971,6 @@ } -// Signature Algorithms for Certificates. -// -// https://tools.ietf.org/html/rfc8446#section-4.2.3 - -static bool ext_sigalgs_cert_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) { - SSL *const ssl = hs->ssl; - // If this extension is omitted, it defaults to the signature_algorithms - // extension, so only emit it if the list is different. - // - // This extension is also new in TLS 1.3, so omit it if TLS 1.3 is disabled. - // There is a corresponding version check in |ext_sigalgs_add_clienthello|. - if (hs->max_version < TLS1_3_VERSION || - !tls12_has_different_verify_sigalgs_for_certs(ssl)) { - return true; - } - - CBB contents, sigalgs_cbb; - if (!CBB_add_u16(out, TLSEXT_TYPE_signature_algorithms_cert) || - !CBB_add_u16_length_prefixed(out, &contents) || - !CBB_add_u16_length_prefixed(&contents, &sigalgs_cbb) || - !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb, true /* certs */) || - !CBB_flush(out)) { - return false; - } - - return true; -} - - // OCSP Stapling. // // https://tools.ietf.org/html/rfc6066#section-8 @@ -2932,14 +2852,6 @@ dont_add_serverhello, }, { - TLSEXT_TYPE_signature_algorithms_cert, - NULL, - ext_sigalgs_cert_add_clienthello, - forbid_parse_serverhello, - ignore_parse_clienthello, - dont_add_serverhello, - }, - { TLSEXT_TYPE_next_proto_neg, NULL, ext_npn_add_clienthello, @@ -3962,7 +3874,3 @@ void SSL_CTX_set_ed25519_enabled(SSL_CTX *ctx, int enabled) { ctx->ed25519_enabled = !!enabled; } - -void SSL_CTX_set_rsa_pss_rsae_certs_enabled(SSL_CTX *ctx, int enabled) { - ctx->rsa_pss_rsae_certs_enabled = !!enabled; -}
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index a62050a..26fc440 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -10,7 +10,6 @@ "crypto/ecdsa" "crypto/rand" "crypto/x509" - "errors" "fmt" "io" "math/big" @@ -532,15 +531,6 @@ NumRSABadValues ) -type RSAPSSSupport int - -const ( - RSAPSSSupportAny RSAPSSSupport = iota - RSAPSSSupportNone - RSAPSSSupportOnlineSignatureOnly - RSAPSSSupportBoth -) - type ProtocolBugs struct { // InvalidSignature specifies that the signature in a ServerKeyExchange // or CertificateVerify message should be invalid. @@ -1580,10 +1570,6 @@ // curves to use compressed coordinates. SendCompressedCoordinates bool - // ExpectRSAPSSSupport specifies the level of RSA-PSS support expected - // from the peer. - ExpectRSAPSSSupport RSAPSSSupport - // SetX25519HighBit, if true, causes X25519 key shares to set their // high-order bit. SetX25519HighBit bool @@ -2074,50 +2060,3 @@ } return false } - -func checkRSAPSSSupport(support RSAPSSSupport, sigAlgs, sigAlgsCert []signatureAlgorithm) error { - if sigAlgsCert == nil { - sigAlgsCert = sigAlgs - } else if eqSignatureAlgorithms(sigAlgs, sigAlgsCert) { - // The peer should have only sent the list once. - return errors.New("tls: signature_algorithms and signature_algorithms_cert extensions were identical") - } - - if support == RSAPSSSupportAny { - return nil - } - - var foundPSS, foundPSSCert bool - for _, sigAlg := range sigAlgs { - if sigAlg == signatureRSAPSSWithSHA256 || sigAlg == signatureRSAPSSWithSHA384 || sigAlg == signatureRSAPSSWithSHA512 { - foundPSS = true - break - } - } - for _, sigAlg := range sigAlgsCert { - if sigAlg == signatureRSAPSSWithSHA256 || sigAlg == signatureRSAPSSWithSHA384 || sigAlg == signatureRSAPSSWithSHA512 { - foundPSSCert = true - break - } - } - - expectPSS := support != RSAPSSSupportNone - if foundPSS != expectPSS { - if expectPSS { - return errors.New("tls: peer did not support PSS") - } else { - return errors.New("tls: peer unexpectedly supported PSS") - } - } - - expectPSSCert := support == RSAPSSSupportBoth - if foundPSSCert != expectPSSCert { - if expectPSSCert { - return errors.New("tls: peer did not support PSS in certificates") - } else { - return errors.New("tls: peer unexpectedly supported PSS in certificates") - } - } - - return nil -}
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 09e08a8..df69505 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -905,10 +905,6 @@ return errors.New("tls: expected no certificate_authorities extension") } - if err := checkRSAPSSSupport(c.config.Bugs.ExpectRSAPSSSupport, certReq.signatureAlgorithms, certReq.signatureAlgorithmsCert); err != nil { - return err - } - if c.config.Bugs.IgnorePeerSignatureAlgorithmPreferences { certReq.signatureAlgorithms = c.config.signSignatureAlgorithms() } @@ -1262,9 +1258,6 @@ certReq, ok := msg.(*certificateRequestMsg) if ok { certRequested = true - if err := checkRSAPSSSupport(c.config.Bugs.ExpectRSAPSSSupport, certReq.signatureAlgorithms, certReq.signatureAlgorithmsCert); err != nil { - return err - } if c.config.Bugs.IgnorePeerSignatureAlgorithmPreferences { certReq.signatureAlgorithms = c.config.signSignatureAlgorithms() }
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 47011d2..48dd239 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -356,10 +356,6 @@ } } - if err := checkRSAPSSSupport(config.Bugs.ExpectRSAPSSSupport, hs.clientHello.signatureAlgorithms, hs.clientHello.signatureAlgorithmsCert); err != nil { - return err - } - applyBugsToClientHello(hs.clientHello, config) return nil
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index b3ea83a..20866d7 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -9882,117 +9882,6 @@ "-verify-prefs", strconv.Itoa(int(signatureEd25519)), }, }) - - rsaPSSSupportTests := []struct { - name string - expectRSAPSSSupport RSAPSSSupport - verifyPrefs []signatureAlgorithm - noCerts bool - }{ - // By default, RSA-PSS is fully advertised. - { - name: "Default", - expectRSAPSSSupport: RSAPSSSupportBoth, - }, - // Disabling RSA-PSS certificates makes it online-signature-only. - { - name: "Default-NoCerts", - expectRSAPSSSupport: RSAPSSSupportOnlineSignatureOnly, - noCerts: true, - }, - // The above also apply if verify preferences were explicitly configured. - { - name: "ConfigPSS", - expectRSAPSSSupport: RSAPSSSupportBoth, - verifyPrefs: []signatureAlgorithm{signatureRSAPSSWithSHA256, signatureRSAPKCS1WithSHA256, signatureECDSAWithP256AndSHA256}, - }, - { - name: "ConfigPSS-NoCerts", - expectRSAPSSSupport: RSAPSSSupportOnlineSignatureOnly, - verifyPrefs: []signatureAlgorithm{signatureRSAPSSWithSHA256, signatureRSAPKCS1WithSHA256, signatureECDSAWithP256AndSHA256}, - noCerts: true, - }, - // If verify preferences were explicitly configured without RSA-PSS support, - // NoCerts is a no-op and the shim correctly only sends one extension. - // (This is checked internally in the runner.) - { - name: "ConfigNoPSS", - expectRSAPSSSupport: RSAPSSSupportNone, - verifyPrefs: []signatureAlgorithm{signatureRSAPKCS1WithSHA256, signatureECDSAWithP256AndSHA256}, - }, - { - name: "ConfigNoPSS-NoCerts", - expectRSAPSSSupport: RSAPSSSupportNone, - verifyPrefs: []signatureAlgorithm{signatureRSAPKCS1WithSHA256, signatureECDSAWithP256AndSHA256}, - noCerts: true, - }, - } - - for _, test := range rsaPSSSupportTests { - var pssFlags []string - for _, pref := range test.verifyPrefs { - pssFlags = append(pssFlags, "-verify-prefs", strconv.Itoa(int(pref))) - } - if test.noCerts { - pssFlags = append(pssFlags, "-no-rsa-pss-rsae-certs") - } - for _, ver := range tlsVersions { - if ver.version < VersionTLS12 { - continue - } - - // TLS 1.2 cannot express different RSAPSSSupportOnlineSignatureOnly, - // so it decays to RSAPSSSupportNone. - expect := test.expectRSAPSSSupport - if ver.version < VersionTLS13 && expect == RSAPSSSupportOnlineSignatureOnly { - expect = RSAPSSSupportNone - } - - // If the configuration results in no RSA-PSS support, the handshake won't complete. - // (The test, however, still covers the RSA-PSS assertion.) - var localError string - var shouldFail bool - if ver.version >= VersionTLS13 && expect == RSAPSSSupportNone { - shouldFail = true - localError = "tls: no common signature algorithms" - } - - flags := []string{"-max-version", ver.shimFlag(tls)} - flags = append(flags, pssFlags...) - testCases = append(testCases, testCase{ - name: fmt.Sprintf("RSAPSSSupport-%s-%s-Client", test.name, ver.name), - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - Certificates: []Certificate{rsaCertificate}, - Bugs: ProtocolBugs{ - ExpectRSAPSSSupport: expect, - }, - }, - flags: flags, - shouldFail: shouldFail, - expectedLocalError: localError, - }) - - serverFlags := []string{"-require-any-client-certificate"} - serverFlags = append(flags, serverFlags...) - testCases = append(testCases, testCase{ - testType: serverTest, - name: fmt.Sprintf("RSAPSSSupport-%s-%s-Server", test.name, ver.name), - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - Certificates: []Certificate{rsaCertificate}, - Bugs: ProtocolBugs{ - ExpectRSAPSSSupport: expect, - }, - }, - flags: serverFlags, - shouldFail: shouldFail, - expectedLocalError: localError, - }) - } - } } // timeouts is the retransmit schedule for BoringSSL. It doubles and
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index ab8df10..c9163fa 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -135,7 +135,6 @@ {"-ignore-tls13-downgrade", &TestConfig::ignore_tls13_downgrade}, {"-expect-tls13-downgrade", &TestConfig::expect_tls13_downgrade}, {"-handoff", &TestConfig::handoff}, - {"-no-rsa-pss-rsae-certs", &TestConfig::no_rsa_pss_rsae_certs}, {"-use-ocsp-callback", &TestConfig::use_ocsp_callback}, {"-set-ocsp-in-callback", &TestConfig::set_ocsp_in_callback}, {"-decline-ocsp-callback", &TestConfig::decline_ocsp_callback}, @@ -1263,9 +1262,6 @@ if (enable_ed25519) { SSL_CTX_set_ed25519_enabled(ssl_ctx.get(), 1); } - if (no_rsa_pss_rsae_certs) { - SSL_CTX_set_rsa_pss_rsae_certs_enabled(ssl_ctx.get(), 0); - } if (!verify_prefs.empty()) { std::vector<uint16_t> u16s(verify_prefs.begin(), verify_prefs.end());
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index d588483..24211bb 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -159,7 +159,6 @@ bool ignore_tls13_downgrade = false; bool expect_tls13_downgrade = false; bool handoff = false; - bool no_rsa_pss_rsae_certs = false; bool use_ocsp_callback = false; bool set_ocsp_in_callback = false; bool decline_ocsp_callback = false;
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index b4f2134..c6c496e 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -627,22 +627,10 @@ !CBB_add_u16_length_prefixed(&cert_request_extensions, &sigalg_contents) || !CBB_add_u16_length_prefixed(&sigalg_contents, &sigalgs_cbb) || - !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb, - false /* online signature */)) { + !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb)) { return ssl_hs_error; } - if (tls12_has_different_verify_sigalgs_for_certs(ssl)) { - if (!CBB_add_u16(&cert_request_extensions, - TLSEXT_TYPE_signature_algorithms_cert) || - !CBB_add_u16_length_prefixed(&cert_request_extensions, - &sigalg_contents) || - !CBB_add_u16_length_prefixed(&sigalg_contents, &sigalgs_cbb) || - !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb, true /* certs */)) { - return ssl_hs_error; - } - } - if (ssl_has_client_CAs(hs->config)) { CBB ca_contents; if (!CBB_add_u16(&cert_request_extensions,