# 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

                    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);