ceb4bbe240
Fix incorrect ssl alerts on signature algorithms.
123 lines
4.9 KiB
Diff
123 lines
4.9 KiB
Diff
diff -up ./lib/ssl/ssl3con.c.alert-fix ./lib/ssl/ssl3con.c
|
|
--- ./lib/ssl/ssl3con.c.alert-fix 2021-06-10 05:33:12.000000000 -0700
|
|
+++ ./lib/ssl/ssl3con.c 2021-07-06 17:08:25.894018521 -0700
|
|
@@ -4319,7 +4319,11 @@ ssl_SignatureSchemeValid(SSLSignatureSch
|
|
if (!ssl_IsSupportedSignatureScheme(scheme)) {
|
|
return PR_FALSE;
|
|
}
|
|
- if (!ssl_SignatureSchemeMatchesSpkiOid(scheme, spkiOid)) {
|
|
+ /* if we are purposefully passed SEC_OID_UNKOWN, it means
|
|
+ * we not checking the scheme against a potential key, so skip
|
|
+ * the call */
|
|
+ if ((spkiOid != SEC_OID_UNKNOWN) &&
|
|
+ !ssl_SignatureSchemeMatchesSpkiOid(scheme, spkiOid)) {
|
|
return PR_FALSE;
|
|
}
|
|
if (isTls13) {
|
|
@@ -4517,7 +4521,8 @@ ssl_CheckSignatureSchemeConsistency(sslS
|
|
}
|
|
|
|
/* Verify that the signature scheme matches the signing key. */
|
|
- if (!ssl_SignatureSchemeValid(scheme, spkiOid, isTLS13)) {
|
|
+ if ((spkiOid == SEC_OID_UNKNOWN) ||
|
|
+ !ssl_SignatureSchemeValid(scheme, spkiOid, isTLS13)) {
|
|
PORT_SetError(SSL_ERROR_INCORRECT_SIGNATURE_ALGORITHM);
|
|
return SECFailure;
|
|
}
|
|
@@ -4533,6 +4538,7 @@ ssl_CheckSignatureSchemeConsistency(sslS
|
|
PRBool
|
|
ssl_IsSupportedSignatureScheme(SSLSignatureScheme scheme)
|
|
{
|
|
+ PRBool isSupported = PR_FALSE;
|
|
switch (scheme) {
|
|
case ssl_sig_rsa_pkcs1_sha1:
|
|
case ssl_sig_rsa_pkcs1_sha256:
|
|
@@ -4552,7 +4558,8 @@ ssl_IsSupportedSignatureScheme(SSLSignat
|
|
case ssl_sig_dsa_sha384:
|
|
case ssl_sig_dsa_sha512:
|
|
case ssl_sig_ecdsa_sha1:
|
|
- return PR_TRUE;
|
|
+ isSupported = PR_TRUE;
|
|
+ break;
|
|
|
|
case ssl_sig_rsa_pkcs1_sha1md5:
|
|
case ssl_sig_none:
|
|
@@ -4560,7 +4567,19 @@ ssl_IsSupportedSignatureScheme(SSLSignat
|
|
case ssl_sig_ed448:
|
|
return PR_FALSE;
|
|
}
|
|
- return PR_FALSE;
|
|
+ if (isSupported) {
|
|
+ SECOidTag hashOID = ssl3_HashTypeToOID(ssl_SignatureSchemeToHashType(scheme));
|
|
+ PRUint32 policy;
|
|
+ const PRUint32 sigSchemePolicy=
|
|
+ NSS_USE_ALG_IN_SSL_KX|NSS_USE_ALG_IN_SIGNATURE;
|
|
+ /* check hash policy */
|
|
+ if ((NSS_GetAlgorithmPolicy(hashOID, &policy) == SECSuccess) &&
|
|
+ ((policy & sigSchemePolicy) != sigSchemePolicy)) {
|
|
+ return PR_FALSE;
|
|
+ }
|
|
+ /* check algorithm policy */
|
|
+ }
|
|
+ return isSupported;
|
|
}
|
|
|
|
PRBool
|
|
@@ -6533,6 +6552,9 @@ ssl_PickSignatureScheme(sslSocket *ss,
|
|
}
|
|
|
|
spkiOid = SECOID_GetAlgorithmTag(&cert->subjectPublicKeyInfo.algorithm);
|
|
+ if (spkiOid == SEC_OID_UNKNOWN) {
|
|
+ goto loser;
|
|
+ }
|
|
|
|
/* Now we have to search based on the key type. Go through our preferred
|
|
* schemes in order and find the first that can be used. */
|
|
@@ -6547,6 +6569,7 @@ ssl_PickSignatureScheme(sslSocket *ss,
|
|
}
|
|
}
|
|
|
|
+loser:
|
|
PORT_SetError(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
|
|
return SECFailure;
|
|
}
|
|
@@ -7700,7 +7723,8 @@ ssl_ParseSignatureSchemes(const sslSocke
|
|
PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
|
|
return SECFailure;
|
|
}
|
|
- if (ssl_IsSupportedSignatureScheme((SSLSignatureScheme)tmp)) {
|
|
+ if (ssl_SignatureSchemeValid((SSLSignatureScheme)tmp, SEC_OID_UNKNOWN,
|
|
+ (PRBool)ss->version >= SSL_LIBRARY_VERSION_TLS_1_3)) {;
|
|
schemes[numSupported++] = (SSLSignatureScheme)tmp;
|
|
}
|
|
}
|
|
@@ -10286,7 +10310,12 @@ ssl3_HandleCertificateVerify(sslSocket *
|
|
PORT_Assert(ss->ssl3.hs.hashType == handshake_hash_record);
|
|
rv = ssl_ConsumeSignatureScheme(ss, &b, &length, &sigScheme);
|
|
if (rv != SECSuccess) {
|
|
- goto loser; /* malformed or unsupported. */
|
|
+ errCode = PORT_GetError();
|
|
+ /* unsupported == illegal_parameter, others == handshake_failure. */
|
|
+ if (errCode == SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM) {
|
|
+ desc = illegal_parameter;
|
|
+ }
|
|
+ goto alert_loser;
|
|
}
|
|
rv = ssl_CheckSignatureSchemeConsistency(
|
|
ss, sigScheme, &ss->sec.peerCert->subjectPublicKeyInfo);
|
|
diff -up ./gtests/ssl_gtest/ssl_extension_unittest.cc.alert-fix ./gtests/ssl_gtest/ssl_extension_unittest.cc
|
|
--- ./gtests/ssl_gtest/ssl_extension_unittest.cc.alert-fix 2021-07-07 11:32:11.634376932 -0700
|
|
+++ ./gtests/ssl_gtest/ssl_extension_unittest.cc 2021-07-07 11:33:30.595841110 -0700
|
|
@@ -428,7 +428,10 @@ TEST_P(TlsExtensionTest12Plus, Signature
|
|
}
|
|
|
|
TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsTrailingData) {
|
|
- const uint8_t val[] = {0x00, 0x02, 0x04, 0x01, 0x00}; // sha-256, rsa
|
|
+ // make sure the test uses an algorithm that is legal for
|
|
+ // tls 1.3 (or tls 1.3 will through and illegalParameter
|
|
+ // instead of a decode error)
|
|
+ const uint8_t val[] = {0x00, 0x02, 0x08, 0x09, 0x00}; // sha-256, rsa-pss-pss
|
|
DataBuffer extension(val, sizeof(val));
|
|
ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
|
|
client_, ssl_signature_algorithms_xtn, extension));
|