# HG changeset patch
# User Daiki Ueno <dueno@redhat.com>
# 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<TlsExtensionCapture>(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<TlsExtensionCapture>(
+ 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<SSLProtocolVariant, uint16_t, std::string, SSLAuthType,
SSLSignatureScheme>
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);