Bug 1423557, cryptohi: make RSA-PSS parameter check stricter, r=mt

Summary: This adds a check on unsupported hash/mask algorithms and invalid trailer field, when converting SECKEYRSAPSSParams to CK_RSA_PKCS_PSS_PARAMS for both signing and verification.  It also add missing support for SHA224 as underlying hash algorithm.

Reviewers: mt

Reviewed By: mt

Bug #: 1423557

Differential Revision: https://phabricator.services.mozilla.com/D322

4 files changed, 94 insertions, 8 deletions

diff --git a/lib/cryptohi/seckey.c b/lib/cryptohi/seckey.c
index 0f141b5c5..0f9353f3b 100644
--- a/lib/cryptohi/seckey.c
+++ b/lib/cryptohi/seckey.c
@@ -1984,13 +1984,14 @@ sec_GetHashMechanismByOidTag(SECOidTag tag)
             return CKM_SHA384;
         case SEC_OID_SHA256:
             return CKM_SHA256;
+        case SEC_OID_SHA224:
+            return CKM_SHA224;
+        case SEC_OID_SHA1:
+            return CKM_SHA_1;
         default:
             PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
-        /* fallthrough */
-        case SEC_OID_SHA1:
-            break;
+            return CKM_INVALID_MECHANISM;
     }
-    return CKM_SHA_1;
 }
 
 static CK_RSA_PKCS_MGF_TYPE
@@ -2003,13 +2004,14 @@ sec_GetMgfTypeByOidTag(SECOidTag tag)
             return CKG_MGF1_SHA384;
         case SEC_OID_SHA256:
             return CKG_MGF1_SHA256;
+        case SEC_OID_SHA224:
+            return CKG_MGF1_SHA224;
+        case SEC_OID_SHA1:
+            return CKG_MGF1_SHA1;
         default:
             PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
-        /* fallthrough */
-        case SEC_OID_SHA1:
-            break;
+            return 0;
     }
-    return CKG_MGF1_SHA1;
 }
 
 SECStatus
@@ -2019,6 +2021,7 @@ sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
     SECStatus rv = SECSuccess;
     SECOidTag hashAlgTag;
     unsigned long saltLength;
+    unsigned long trailerField;
 
     PORT_Memset(mech, 0, sizeof(CK_RSA_PKCS_PSS_PARAMS));
 
@@ -2028,6 +2031,9 @@ sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
         hashAlgTag = SEC_OID_SHA1; /* default, SHA-1 */
     }
     mech->hashAlg = sec_GetHashMechanismByOidTag(hashAlgTag);
+    if (mech->hashAlg == CKM_INVALID_MECHANISM) {
+        return SECFailure;
+    }
 
     if (params->maskAlg) {
         SECAlgorithmID maskHashAlg;
@@ -2050,6 +2056,9 @@ sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
         }
         maskHashAlgTag = SECOID_GetAlgorithmTag(&maskHashAlg);
         mech->mgf = sec_GetMgfTypeByOidTag(maskHashAlgTag);
+        if (mech->mgf == 0) {
+            return SECFailure;
+        }
     } else {
         mech->mgf = CKG_MGF1_SHA1; /* default, MGF1 with SHA-1 */
     }
@@ -2064,5 +2073,18 @@ sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
     }
     mech->sLen = saltLength;
 
+    if (params->trailerField.data) {
+        rv = SEC_ASN1DecodeInteger((SECItem *)&params->trailerField, &trailerField);
+        if (rv != SECSuccess) {
+            return rv;
+        }
+        if (trailerField != 1) {
+            /* the value must be 1, which represents the trailer field
+             * with hexadecimal value 0xBC */
+            PORT_SetError(SEC_ERROR_INVALID_ARGS);
+            return SECFailure;
+        }
+    }
+
     return rv;
 }
diff --git a/tests/cert/TestCA-bogus-rsa-pss1.crt b/tests/cert/TestCA-bogus-rsa-pss1.crt
new file mode 100644
index 000000000..e3c8fcdcf
--- /dev/null
+++ b/tests/cert/TestCA-bogus-rsa-pss1.crt
@@ -0,0 +1,26 @@
+-----BEGIN CERTIFICATE-----
+MIIEbDCCAxqgAwIBAgIBATBHBgkqhkiG9w0BAQowOqAPMA0GCWCGSAFlAwQCAQUA

+oRwwGgYJKoZIhvcNAQEIMA0GCWCGSAFlAwQCAQUAogMCASCjBAICEmcwgYMxCzAJ

+BgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFp

+biBWaWV3MRIwEAYDVQQKEwlCT0dVUyBOU1MxMzAxBgNVBAMTKk5TUyBUZXN0IENB

+IChSU0EtUFNTIGludmFsaWQgdHJhaWxlckZpZWxkKTAgFw0xNzEyMDcxMjU3NDBa

+GA8yMDY3MTIwNzEyNTc0MFowgYMxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxp

+Zm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRIwEAYDVQQKEwlCT0dVUyBO

+U1MxMzAxBgNVBAMTKk5TUyBUZXN0IENBIChSU0EtUFNTIGludmFsaWQgdHJhaWxl

+ckZpZWxkKTCCAVwwRwYJKoZIhvcNAQEKMDqgDzANBglghkgBZQMEAgEFAKEcMBoG

+CSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgowQCAhJnA4IBDwAwggEKAoIB

+AQDgkKJk+PoFpESak7kMQ0w147/xilUZCG7hDGG2uuGTbX8jqy9N9pxzB9sJjgJX

+yYND0XEmrUQ2Memmy8jufhXML5DekW1tr3Gi2L3VivbIReJZfXk1xDMvNbB/Gjjo

+SoPyu8C4hnevjgMlmqG3KdMkB+eN6PnBG64YFyki3vnLO5iTNHEBTgFYo0gTX4uK

+xl0hLtiDL+4K5l7BwVgxZwQF6uHoHjrjjlhkzR0FwjjqR8U0pH20Pb6IlRsFMv07

+/1GHf+jm34pKb/1ZNzAbiKxYv7YAQUWEZ7e/GSXgA6gbTpV9ueiLkVucUeXN/mXK

+Tqb4zivi5FaSGVl8SJnqsJXJAgMBAAGjOTA3MBQGCWCGSAGG+EIBAQEB/wQEAwIC

+BDAPBgNVHRMECDAGAQH/AgEAMA4GA1UdDwEB/wQEAwICBDBHBgkqhkiG9w0BAQow

+OqAPMA0GCWCGSAFlAwQCAQUAoRwwGgYJKoZIhvcNAQEIMA0GCWCGSAFlAwQCAQUA

+ogMCASCjBAICEmcDggEBAJht9t9p/dlhJtx7ShDvUXyq8N4tCoGKdREM83K/jlW8

+HxdHOz5PuvZx+UMlaUtqZVIriSCnRtEWkoSo0hWmcv1rp80it2G1zLfLPYdyrPba

+nQmE1iFb69Wr9dwrX7o/CII+WHQgoIGeFGntZ8YRZTe5+JeiGAlAyZCqUKbl9lhh

+pCpf1YYxb3VI8mAGVi0jwabWBEbInGBZYH9HP0nK7/Tflk6UY3f4h4Fbkk5D4WZA

+hFfkebx6Wh90QGiKQhp4/N+dYira8bKvWqqn0VqwzBoJBU/RmMaJVpwqFFvcaUJh

+uEKUPeQbqkYvj1WJYmy4ettVwi4OZU50+kCaRQhMsFA=
+-----END CERTIFICATE-----
diff --git a/tests/cert/TestCA-bogus-rsa-pss2.crt b/tests/cert/TestCA-bogus-rsa-pss2.crt
new file mode 100644
index 000000000..d46442dc4
--- /dev/null
+++ b/tests/cert/TestCA-bogus-rsa-pss2.crt
@@ -0,0 +1,24 @@
+-----BEGIN CERTIFICATE-----
+MIIEFzCCAs2gAwIBAgIBATA/BgkqhkiG9w0BAQowMqAOMAwGCCqGSIb3DQIFBQCh

+GzAZBgkqhkiG9w0BAQgwDAYIKoZIhvcNAgUFAKIDAgEgMH4xCzAJBgNVBAYTAlVT

+MRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRIw

+EAYDVQQKEwlCT0dVUyBOU1MxLjAsBgNVBAMTJU5TUyBUZXN0IENBIChSU0EtUFNT

+IGludmFsaWQgaGFzaEFsZykwIBcNMTcxMjA3MTQwNjQ0WhgPMjA2ODAxMDcxNDA2

+NDRaMH4xCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQH

+Ew1Nb3VudGFpbiBWaWV3MRIwEAYDVQQKEwlCT0dVUyBOU1MxLjAsBgNVBAMTJU5T

+UyBUZXN0IENBIChSU0EtUFNTIGludmFsaWQgaGFzaEFsZykwggEgMAsGCSqGSIb3

+DQEBCgOCAQ8AMIIBCgKCAQEAtDXA73yTOgs8zVYNMCtuQ9a07UgbfeQbjHp3pkF6

+7rsC/Q28mrLh+zLkht5e7qU/Qf/8a2ZkcYhPOBAjCzjgIXOdE2lsWvdVujOJLR0x

+Fesd3hDLRmL6f6momc+j1/Tw3bKyZinaeJ9BFRv9c94SayB3QUe+6+TNJKASwlhj

+sx6mUsND+h3DkuL77gi7hIUpUXfFSwa+zM69VLhIu+/WRZfG8gfKkCAIGUC3WYJa

+eU1HgQKfVSXW0ok4ototXWEe9ohU+Z1tO9LJStcY8mMpig7EU9zbpObhG46Sykfu

+aKsubB9J+gFgwP5Tb85tRYT6SbHeHR6U/N8GBrKdRcomWwIDAQABozwwOjAUBglg

+hkgBhvhCAQEBAf8EBAMCAgQwEgYDVR0TAQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8E

+BAMCAgQwPwYJKoZIhvcNAQEKMDKgDjAMBggqhkiG9w0CBQUAoRswGQYJKoZIhvcN

+AQEIMAwGCCqGSIb3DQIFBQCiAwIBIAOCAQEAjeemeTxh2xrMUJ6Z5Yn2nH2FbcPY

+fTHJcdfXjfNBkrMl5pe2/lk0JyNuACTuTYFCxdWNRL1coN//h9DSUbF3dpF1ex6D

+difo+6PwxkO2aPVGPYw4DSivt4SFbn5dKGgVqBQfnmNK7p/iT91AcErg/grRrNL+

+4jeT0UiRjQYeX9xKJArv+ocIidNpQL3QYxXuBLZxVC92Af69ol7WG8QBRLnFi1p2

+g6q8hOHqOfB29qnsSo3PkI1yuShOl50tRLbNgyotEfZdk1N3oXvapoBsm/jlcdCT

+0aKelCSQYYAfyl5PKCpa1lgBm7zfcHSDStMhEEFu/fbnJhqO9g9znj3STQ==
+-----END CERTIFICATE-----
diff --git a/tests/cert/cert.sh b/tests/cert/cert.sh
index 12594405c..d1a9148a9 100755
--- a/tests/cert/cert.sh
+++ b/tests/cert/cert.sh
@@ -2095,6 +2095,20 @@ cert_test_rsapss()
   certu -A -n "TestCA-rsa-pss-sha1" -t "C,," -d "${PROFILEDIR}" -f "${R_PWFILE}" \
         -i "${R_CADIR}/TestCA-rsa-pss-sha1.ca.cert" 2>&1
 
+  CU_ACTION="Import Bogus RSA-PSS CA Cert (invalid trailerField)"
+  certu -A -n "TestCA-bogus-rsa-pss1" -t "C,," -d "${PROFILEDIR}" -f "${R_PWFILE}" \
+        -i "${QADIR}/cert/TestCA-bogus-rsa-pss1.crt" 2>&1
+  RETEXPECTED=255
+  certu -V -b 1712101010Z -n TestCA-bogus-rsa-pss1 -u L -e -d "${PROFILEDIR}" -f "${R_PWFILE}" 2>&1
+  RETEXPECTED=0
+
+  CU_ACTION="Import Bogus RSA-PSS CA Cert (invalid hashAlg)"
+  certu -A -n "TestCA-bogus-rsa-pss2" -t "C,," -d "${PROFILEDIR}" -f "${R_PWFILE}" \
+        -i "${QADIR}/cert/TestCA-bogus-rsa-pss2.crt" 2>&1
+  RETEXPECTED=255
+  certu -V -b 1712101010Z -n TestCA-bogus-rsa-pss2 -u L -e -d "${PROFILEDIR}" -f "${R_PWFILE}" 2>&1
+  RETEXPECTED=0
+
   CERTSERIAL=200
 
   # Subject certificate: RSA