# HG changeset patch # User Daiki Ueno # Date 1594360877 -7200 # Fri Jul 10 08:01:17 2020 +0200 # Node ID df1d2695e115ed9e6f7e8df6ad4d7be2c9bc77d8 # Parent de661583d46713c9b4873a904dda3a8ba4a61976 Bug 1646324, advertise rsa_pkcs1_* schemes in CH and CR for certs, r=mt Summary: In TLS 1.3, unless "signature_algorithms_cert" is advertised, the "signature_algorithms" extension is used as an indication of supported algorithms for signatures on certificates. While rsa_pkcs1_* signatures schemes cannot be used for signing handshake messages, they should be advertised if the peer wants to to support certificates signed with RSA PKCS#1. This adds a flag to ssl3_EncodeSigAlgs() and ssl3_FilterSigAlgs() to preserve rsa_pkcs1_* schemes in the output. Reviewers: mt Reviewed By: mt Bug #: 1646324 Differential Revision: https://phabricator.services.mozilla.com/D80881 diff -r de661583d467 -r df1d2695e115 gtests/ssl_gtest/ssl_auth_unittest.cc --- a/gtests/ssl_gtest/ssl_auth_unittest.cc Thu Jul 09 22:45:27 2020 +0000 +++ b/gtests/ssl_gtest/ssl_auth_unittest.cc Fri Jul 10 08:01:17 2020 +0200 @@ -1591,6 +1591,47 @@ capture->extension()); } +TEST_P(TlsConnectTls13, Tls13RsaPkcs1IsAdvertisedClient) { + EnsureTlsSetup(); + static const SSLSignatureScheme kSchemes[] = {ssl_sig_rsa_pkcs1_sha256, + ssl_sig_rsa_pss_rsae_sha256}; + client_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes)); + auto capture = + MakeTlsFilter(client_, ssl_signature_algorithms_xtn); + Connect(); + // We should only have the one signature algorithm advertised. + static const uint8_t kExpectedExt[] = {0, + 4, + ssl_sig_rsa_pss_rsae_sha256 >> 8, + ssl_sig_rsa_pss_rsae_sha256 & 0xff, + ssl_sig_rsa_pkcs1_sha256 >> 8, + ssl_sig_rsa_pkcs1_sha256 & 0xff}; + ASSERT_EQ(DataBuffer(kExpectedExt, sizeof(kExpectedExt)), + capture->extension()); +} + +TEST_P(TlsConnectTls13, Tls13RsaPkcs1IsAdvertisedServer) { + EnsureTlsSetup(); + static const SSLSignatureScheme kSchemes[] = {ssl_sig_rsa_pkcs1_sha256, + ssl_sig_rsa_pss_rsae_sha256}; + server_->SetSignatureSchemes(kSchemes, PR_ARRAY_SIZE(kSchemes)); + auto capture = MakeTlsFilter( + server_, ssl_signature_algorithms_xtn, true); + capture->SetHandshakeTypes({kTlsHandshakeCertificateRequest}); + capture->EnableDecryption(); + server_->RequestClientAuth(false); // So we get a CertificateRequest. + Connect(); + // We should only have the one signature algorithm advertised. + static const uint8_t kExpectedExt[] = {0, + 4, + ssl_sig_rsa_pss_rsae_sha256 >> 8, + ssl_sig_rsa_pss_rsae_sha256 & 0xff, + ssl_sig_rsa_pkcs1_sha256 >> 8, + ssl_sig_rsa_pkcs1_sha256 & 0xff}; + ASSERT_EQ(DataBuffer(kExpectedExt, sizeof(kExpectedExt)), + capture->extension()); +} + // variant, version, certificate, auth type, signature scheme typedef std::tuple diff -r de661583d467 -r df1d2695e115 lib/ssl/ssl3con.c --- a/lib/ssl/ssl3con.c Thu Jul 09 22:45:27 2020 +0000 +++ b/lib/ssl/ssl3con.c Fri Jul 10 08:01:17 2020 +0200 @@ -784,15 +784,19 @@ * Both by policy and by having a token that supports it. */ static PRBool ssl_SignatureSchemeAccepted(PRUint16 minVersion, - SSLSignatureScheme scheme) + SSLSignatureScheme scheme, + PRBool forCert) { /* Disable RSA-PSS schemes if there are no tokens to verify them. */ if (ssl_IsRsaPssSignatureScheme(scheme)) { if (!PK11_TokenExists(auth_alg_defs[ssl_auth_rsa_pss])) { return PR_FALSE; } - } else if (ssl_IsRsaPkcs1SignatureScheme(scheme)) { - /* Disable PKCS#1 signatures if we are limited to TLS 1.3. */ + } else if (!forCert && ssl_IsRsaPkcs1SignatureScheme(scheme)) { + /* Disable PKCS#1 signatures if we are limited to TLS 1.3. + * We still need to advertise PKCS#1 signatures in CH and CR + * for certificate signatures. + */ if (minVersion >= SSL_LIBRARY_VERSION_TLS_1_3) { return PR_FALSE; } @@ -851,7 +855,8 @@ /* Ensure that there is a signature scheme that can be accepted.*/ for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) { if (ssl_SignatureSchemeAccepted(ss->vrange.min, - ss->ssl3.signatureSchemes[i])) { + ss->ssl3.signatureSchemes[i], + PR_FALSE /* forCert */)) { return SECSuccess; } } @@ -880,7 +885,7 @@ PRBool acceptable = authType == schemeAuthType || (schemeAuthType == ssl_auth_rsa_pss && authType == ssl_auth_rsa_sign); - if (acceptable && ssl_SignatureSchemeAccepted(ss->version, scheme)) { + if (acceptable && ssl_SignatureSchemeAccepted(ss->version, scheme, PR_FALSE /* forCert */)) { return PR_TRUE; } } @@ -9803,12 +9808,13 @@ } SECStatus -ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint16 minVersion, sslBuffer *buf) +ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint16 minVersion, PRBool forCert, + sslBuffer *buf) { SSLSignatureScheme filtered[MAX_SIGNATURE_SCHEMES] = { 0 }; unsigned int filteredCount = 0; - SECStatus rv = ssl3_FilterSigAlgs(ss, minVersion, PR_FALSE, + SECStatus rv = ssl3_FilterSigAlgs(ss, minVersion, PR_FALSE, forCert, PR_ARRAY_SIZE(filtered), filtered, &filteredCount); if (rv != SECSuccess) { @@ -9843,8 +9849,21 @@ return sslBuffer_InsertLength(buf, lengthOffset, 2); } +/* + * In TLS 1.3 we are permitted to advertise support for PKCS#1 + * schemes. This doesn't affect the signatures in TLS itself, just + * those on certificates. Not advertising PKCS#1 signatures creates a + * serious compatibility risk as it excludes many certificate chains + * that include PKCS#1. Hence, forCert is used to enable advertising + * PKCS#1 support. Note that we include these in signature_algorithms + * because we don't yet support signature_algorithms_cert. TLS 1.3 + * requires that PKCS#1 schemes are placed last in the list if they + * are present. This sorting can be removed once we support + * signature_algorithms_cert. + */ SECStatus ssl3_FilterSigAlgs(const sslSocket *ss, PRUint16 minVersion, PRBool disableRsae, + PRBool forCert, unsigned int maxSchemes, SSLSignatureScheme *filteredSchemes, unsigned int *numFilteredSchemes) { @@ -9856,15 +9875,32 @@ } *numFilteredSchemes = 0; + PRBool allowUnsortedPkcs1 = forCert && minVersion < SSL_LIBRARY_VERSION_TLS_1_3; for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) { if (disableRsae && ssl_IsRsaeSignatureScheme(ss->ssl3.signatureSchemes[i])) { continue; } if (ssl_SignatureSchemeAccepted(minVersion, - ss->ssl3.signatureSchemes[i])) { + ss->ssl3.signatureSchemes[i], + allowUnsortedPkcs1)) { filteredSchemes[(*numFilteredSchemes)++] = ss->ssl3.signatureSchemes[i]; } } + if (forCert && !allowUnsortedPkcs1) { + for (unsigned int i = 0; i < ss->ssl3.signatureSchemeCount; ++i) { + if (disableRsae && ssl_IsRsaeSignatureScheme(ss->ssl3.signatureSchemes[i])) { + continue; + } + if (!ssl_SignatureSchemeAccepted(minVersion, + ss->ssl3.signatureSchemes[i], + PR_FALSE) && + ssl_SignatureSchemeAccepted(minVersion, + ss->ssl3.signatureSchemes[i], + PR_TRUE)) { + filteredSchemes[(*numFilteredSchemes)++] = ss->ssl3.signatureSchemes[i]; + } + } + } return SECSuccess; } @@ -9901,7 +9937,7 @@ length = 1 + certTypesLength + 2 + calen; if (isTLS12) { - rv = ssl3_EncodeSigAlgs(ss, ss->version, &sigAlgsBuf); + rv = ssl3_EncodeSigAlgs(ss, ss->version, PR_TRUE /* forCert */, &sigAlgsBuf); if (rv != SECSuccess) { return rv; } diff -r de661583d467 -r df1d2695e115 lib/ssl/ssl3exthandle.c --- a/lib/ssl/ssl3exthandle.c Thu Jul 09 22:45:27 2020 +0000 +++ b/lib/ssl/ssl3exthandle.c Fri Jul 10 08:01:17 2020 +0200 @@ -1652,7 +1652,7 @@ minVersion = ss->vrange.min; /* ClientHello */ } - SECStatus rv = ssl3_EncodeSigAlgs(ss, minVersion, buf); + SECStatus rv = ssl3_EncodeSigAlgs(ss, minVersion, PR_TRUE /* forCert */, buf); if (rv != SECSuccess) { return SECFailure; } diff -r de661583d467 -r df1d2695e115 lib/ssl/sslimpl.h --- a/lib/ssl/sslimpl.h Thu Jul 09 22:45:27 2020 +0000 +++ b/lib/ssl/sslimpl.h Fri Jul 10 08:01:17 2020 +0200 @@ -1688,12 +1688,12 @@ SECStatus ssl3_AuthCertificate(sslSocket *ss); SECStatus ssl_ReadCertificateStatus(sslSocket *ss, PRUint8 *b, PRUint32 length); -SECStatus ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint16 minVersion, +SECStatus ssl3_EncodeSigAlgs(const sslSocket *ss, PRUint16 minVersion, PRBool forCert, sslBuffer *buf); SECStatus ssl3_EncodeFilteredSigAlgs(const sslSocket *ss, const SSLSignatureScheme *schemes, PRUint32 numSchemes, sslBuffer *buf); -SECStatus ssl3_FilterSigAlgs(const sslSocket *ss, PRUint16 minVersion, PRBool disableRsae, +SECStatus ssl3_FilterSigAlgs(const sslSocket *ss, PRUint16 minVersion, PRBool disableRsae, PRBool forCert, unsigned int maxSchemes, SSLSignatureScheme *filteredSchemes, unsigned int *numFilteredSchemes); SECStatus ssl_GetCertificateRequestCAs(const sslSocket *ss, diff -r de661583d467 -r df1d2695e115 lib/ssl/tls13exthandle.c --- a/lib/ssl/tls13exthandle.c Thu Jul 09 22:45:27 2020 +0000 +++ b/lib/ssl/tls13exthandle.c Fri Jul 10 08:01:17 2020 +0200 @@ -1519,7 +1519,8 @@ SSLSignatureScheme filtered[MAX_SIGNATURE_SCHEMES] = { 0 }; unsigned int filteredCount = 0; SECStatus rv = ssl3_FilterSigAlgs(ss, ss->vrange.max, - PR_TRUE, + PR_TRUE /* disableRsae */, + PR_FALSE /* forCert */, MAX_SIGNATURE_SCHEMES, filtered, &filteredCount);