Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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

--HG--
extra : rebase_source : d58be09155b7b197f2500b8831f3b0f1ae8a30f0
extra : amend_source : 0a4cfe26f33dcf3cee44e1001f844d8012350fd0
  • Loading branch information
ueno committed Jul 10, 2020
1 parent 5440fbb commit 69f4843
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 13 deletions.
41 changes: 41 additions & 0 deletions gtests/ssl_gtest/ssl_auth_unittest.cc
Expand Up @@ -1591,6 +1591,47 @@ TEST_P(TlsConnectTls13, Tls13DsaIsNotAdvertisedServer) {
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>
Expand Down
54 changes: 45 additions & 9 deletions lib/ssl/ssl3con.c
Expand Up @@ -784,15 +784,19 @@ ssl_HasCert(const sslSocket *ss, PRUint16 maxVersion, SSLAuthType authType)
* 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;
}
Expand Down Expand Up @@ -851,7 +855,8 @@ ssl_CheckSignatureSchemes(sslSocket *ss)
/* 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;
}
}
Expand Down Expand Up @@ -880,7 +885,7 @@ ssl_HasSignatureScheme(const sslSocket *ss, SSLAuthType authType)
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;
}
}
Expand Down Expand Up @@ -9803,12 +9808,13 @@ ssl3_SendServerKeyExchange(sslSocket *ss)
}

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) {
Expand Down Expand Up @@ -9843,8 +9849,21 @@ ssl3_EncodeFilteredSigAlgs(const sslSocket *ss, const SSLSignatureScheme *scheme
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)
{
Expand All @@ -9856,15 +9875,32 @@ ssl3_FilterSigAlgs(const sslSocket *ss, PRUint16 minVersion, PRBool disableRsae,
}

*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;
}

Expand Down Expand Up @@ -9901,7 +9937,7 @@ ssl3_SendCertificateRequest(sslSocket *ss)

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;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ssl/ssl3exthandle.c
Expand Up @@ -1652,7 +1652,7 @@ ssl3_SendSigAlgsXtn(const sslSocket *ss, TLSExtensionData *xtnData,
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;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/ssl/sslimpl.h
Expand Up @@ -1688,12 +1688,12 @@ SECStatus ssl3_HandleServerSpki(sslSocket *ss);
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,
Expand Down
3 changes: 2 additions & 1 deletion lib/ssl/tls13exthandle.c
Expand Up @@ -1519,7 +1519,8 @@ tls13_ClientSendDelegatedCredentialsXtn(const sslSocket *ss,
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);
Expand Down

0 comments on commit 69f4843

Please sign in to comment.