Bug 1415171, Fix handling of default RSA-PSS parameters, r=mt

Reviewers: mt, rrelyea

Reviewed By: mt

Bug #: 1415171

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

4 files changed, 149 insertions, 24 deletions

diff --git a/cmd/lib/secutil.c b/cmd/lib/secutil.c
index cedecee2d..665c9494c 100644
--- a/cmd/lib/secutil.c
+++ b/cmd/lib/secutil.c
@@ -1192,7 +1192,7 @@ secu_PrintRSAPSSParams(FILE *out, SECItem *value, char *m, int level)
             SECU_Indent(out, level + 1);
             fprintf(out, "Salt length: default, %i (0x%2X)\n", 20, 20);
         } else {
-            SECU_PrintInteger(out, &param.saltLength, "Salt Length", level + 1);
+            SECU_PrintInteger(out, &param.saltLength, "Salt length", level + 1);
         }
     } else {
         SECU_Indent(out, level + 1);
diff --git a/lib/cryptohi/seckey.c b/lib/cryptohi/seckey.c
index f30052213..92e338817 100644
--- a/lib/cryptohi/seckey.c
+++ b/lib/cryptohi/seckey.c
@@ -2056,9 +2056,13 @@ sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
         mech->mgf = CKG_MGF1_SHA1; /* default, MGF1 with SHA-1 */
     }
 
-    rv = SEC_ASN1DecodeInteger((SECItem *)&params->saltLength, &saltLength);
-    if (rv != SECSuccess) {
-        return rv;
+    if (params->saltLength.data) {
+        rv = SEC_ASN1DecodeInteger((SECItem *)&params->saltLength, &saltLength);
+        if (rv != SECSuccess) {
+            return rv;
+        }
+    } else {
+        saltLength = 20; /* default, 20 */
     }
     mech->sLen = saltLength;
 
diff --git a/lib/cryptohi/secsign.c b/lib/cryptohi/secsign.c
index 693e79c65..dc10f2fa6 100644
--- a/lib/cryptohi/secsign.c
+++ b/lib/cryptohi/secsign.c
@@ -610,6 +610,7 @@ sec_CreateRSAPSSParameters(PLArenaPool *arena,
     SECKEYRSAPSSParams pssParams;
     int modBytes, hashLength;
     unsigned long saltLength;
+    PRBool defaultSHA1 = PR_FALSE;
     SECStatus rv;
 
     if (key->keyType != rsaKey && key->keyType != rsaPssKey) {
@@ -631,6 +632,7 @@ sec_CreateRSAPSSParameters(PLArenaPool *arena,
         if (rv != SECSuccess) {
             return NULL;
         }
+        defaultSHA1 = PR_TRUE;
     }
 
     if (pssParams.trailerField.data) {
@@ -652,15 +654,23 @@ sec_CreateRSAPSSParameters(PLArenaPool *arena,
     /* Determine the hash algorithm to use, based on hashAlgTag and
      * pssParams.hashAlg; there are four cases */
     if (hashAlgTag != SEC_OID_UNKNOWN) {
+        SECOidTag tag = SEC_OID_UNKNOWN;
+
         if (pssParams.hashAlg) {
-            if (SECOID_GetAlgorithmTag(pssParams.hashAlg) != hashAlgTag) {
-                PORT_SetError(SEC_ERROR_INVALID_ARGS);
-                return NULL;
-            }
+            tag = SECOID_GetAlgorithmTag(pssParams.hashAlg);
+        } else if (defaultSHA1) {
+            tag = SEC_OID_SHA1;
+        }
+
+        if (tag != SEC_OID_UNKNOWN && tag != hashAlgTag) {
+            PORT_SetError(SEC_ERROR_INVALID_ARGS);
+            return NULL;
         }
     } else if (hashAlgTag == SEC_OID_UNKNOWN) {
         if (pssParams.hashAlg) {
             hashAlgTag = SECOID_GetAlgorithmTag(pssParams.hashAlg);
+        } else if (defaultSHA1) {
+            hashAlgTag = SEC_OID_SHA1;
         } else {
             /* Find a suitable hash algorithm based on the NIST recommendation */
             if (modBytes <= 384) { /* 128, in NIST 800-57, Part 1 */
@@ -709,6 +719,11 @@ sec_CreateRSAPSSParameters(PLArenaPool *arena,
             PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
             return NULL;
         }
+    } else if (defaultSHA1) {
+        if (hashAlgTag != SEC_OID_SHA1) {
+            PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
+            return NULL;
+        }
     }
 
     hashLength = HASH_ResultLenByOidTag(hashAlgTag);
@@ -725,6 +740,8 @@ sec_CreateRSAPSSParameters(PLArenaPool *arena,
             PORT_SetError(SEC_ERROR_INVALID_ARGS);
             return NULL;
         }
+    } else if (defaultSHA1) {
+        saltLength = 20;
     }
 
     /* Fill in the parameters */
diff --git a/tests/cert/cert.sh b/tests/cert/cert.sh
index 9fb29c645..2daabbbff 100755
--- a/tests/cert/cert.sh
+++ b/tests/cert/cert.sh
@@ -516,6 +516,9 @@ cert_all_CA()
 	cert_rsa_pss_CA $CADIR TestCA-rsa-pss -x "CTu,CTu,CTu" ${D_CA} "1" SHA256
 	rm $CLIENT_CADIR/rsapssroot.cert $SERVER_CADIR/rsapssroot.cert
 
+	ALL_CU_SUBJECT="CN=NSS Test CA (RSA-PSS-SHA1), O=BOGUS NSS, L=Mountain View, ST=California, C=US"
+	cert_rsa_pss_CA $CADIR TestCA-rsa-pss-sha1 -x "CTu,CTu,CTu" ${D_CA} "1" SHA1
+	rm $CLIENT_CADIR/rsapssroot.cert $SERVER_CADIR/rsapssroot.cert
 
 #
 #       Create EC version of TestCA
@@ -2054,7 +2057,7 @@ check_sign_algo()
 {
   certu -L -n "$CERTNAME" -d "${PROFILEDIR}" -f "${R_PWFILE}" | \
       sed -n '/^ *Data:/,/^$/{
-/^        Signature Algorithm/,/^ *Salt Length/s/^        //p
+/^        Signature Algorithm/,/^ *Salt length/s/^        //p
 }' > ${TMP}/signalgo.txt
 
   diff ${TMP}/signalgo.exp ${TMP}/signalgo.txt
@@ -2088,6 +2091,12 @@ cert_test_rsapss()
   CU_ACTION="Verify RSA-PSS CA Cert"
   certu -V -u L -e -n "TestCA-rsa-pss" -d "${PROFILEDIR}" -f "${R_PWFILE}"
 
+  CU_ACTION="Import RSA-PSS CA Cert (SHA1)"
+  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
+
+  CERTSERIAL=200
+
   # Subject certificate: RSA
   # Issuer certificate: RSA
   # Signature: RSA-PSS (explicit, with --pss-sign)
@@ -2098,7 +2107,7 @@ cert_test_rsapss()
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA" --pss-sign -m 200 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA" --pss-sign -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2113,10 +2122,12 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-256
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-256
-        Salt Length: 32 (0x20)
+        Salt length: 32 (0x20)
 EOF
   check_sign_algo
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA
   # Issuer certificate: RSA
   # Signature: RSA-PSS (explict, with --pss-sign -Z SHA512)
@@ -2127,7 +2138,7 @@ EOF
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA" --pss-sign -Z SHA512 -m 201 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA" --pss-sign -Z SHA512 -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2142,10 +2153,12 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-512
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-512
-        Salt Length: 64 (0x40)
+        Salt length: 64 (0x40)
 EOF
   check_sign_algo
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA
   # Issuer certificate: RSA-PSS
   # Signature: RSA-PSS
@@ -2156,7 +2169,7 @@ EOF
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA-rsa-pss" -m 202 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA-rsa-pss" -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2171,10 +2184,12 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-256
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-256
-        Salt Length: 32 (0x20)
+        Salt length: 32 (0x20)
 EOF
   check_sign_algo
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA-PSS
   # Issuer certificate: RSA
   # Signature: RSA-PSS (explicit, with --pss-sign)
@@ -2185,7 +2200,7 @@ EOF
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" --pss -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA" --pss-sign -m 203 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA" --pss-sign -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2200,10 +2215,12 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-256
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-256
-        Salt Length: 32 (0x20)
+        Salt length: 32 (0x20)
 EOF
   check_sign_algo
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA-PSS
   # Issuer certificate: RSA-PSS
   # Signature: RSA-PSS (explicit, with --pss-sign)
@@ -2214,7 +2231,7 @@ EOF
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" --pss -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA-rsa-pss" --pss-sign -m 204 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA-rsa-pss" --pss-sign -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2229,10 +2246,12 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-256
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-256
-        Salt Length: 32 (0x20)
+        Salt length: 32 (0x20)
 EOF
   check_sign_algo
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA-PSS
   # Issuer certificate: RSA-PSS
   # Signature: RSA-PSS (implicit, without --pss-sign)
@@ -2243,7 +2262,8 @@ EOF
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" --pss -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA-rsa-pss" -m 205 -v 60 -d "${P_R_CADIR}" \
+  # Sign without --pss-sign nor -Z option
+  certu -C -c "TestCA-rsa-pss" -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2258,10 +2278,12 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-256
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-256
-        Salt Length: 32 (0x20)
+        Salt length: 32 (0x20)
 EOF
   check_sign_algo
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA-PSS
   # Issuer certificate: RSA-PSS
   # Signature: RSA-PSS (with conflicting hash algorithm)
@@ -2273,10 +2295,12 @@ EOF
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
   RETEXPECTED=255
-  certu -C -c "TestCA-rsa-pss" --pss-sign -Z SHA512 -m 206 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA-rsa-pss" --pss-sign -Z SHA512 -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
   RETEXPECTED=0
 
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
   # Subject certificate: RSA-PSS
   # Issuer certificate: RSA-PSS
   # Signature: RSA-PSS (with compatible hash algorithm)
@@ -2287,7 +2311,7 @@ EOF
   certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" --pss -o req  2>&1
 
   CU_ACTION="Sign ${CERTNAME}'s Request"
-  certu -C -c "TestCA-rsa-pss" --pss-sign -Z SHA256 -m 207 -v 60 -d "${P_R_CADIR}" \
+  certu -C -c "TestCA-rsa-pss" --pss-sign -Z SHA256 -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
         -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
 
   CU_ACTION="Import $CERTNAME's Cert"
@@ -2302,9 +2326,89 @@ Signature Algorithm: PKCS #1 RSA-PSS Signature
         Hash algorithm: SHA-256
         Mask algorithm: PKCS #1 MGF1 Mask Generation Function
         Mask hash algorithm: SHA-256
-        Salt Length: 32 (0x20)
+        Salt length: 32 (0x20)
 EOF
   check_sign_algo
+
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
+  # Subject certificate: RSA
+  # Issuer certificate: RSA
+  # Signature: RSA-PSS (explict, with --pss-sign -Z SHA1)
+  CERTNAME="TestUser-rsa-pss9"
+
+  CU_ACTION="Generate Cert Request for $CERTNAME"
+  CU_SUBJECT="CN=$CERTNAME, E=${CERTNAME}@bogus.com, O=BOGUS NSS, L=Mountain View, ST=California, C=US"
+  certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" -o req  2>&1
+
+  CU_ACTION="Sign ${CERTNAME}'s Request"
+  certu -C -c "TestCA" --pss-sign -Z SHA1 -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
+        -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
+
+  CU_ACTION="Import $CERTNAME's Cert"
+  certu -A -n "$CERTNAME" -t ",," -d "${PROFILEDIR}" -f "${R_PWFILE}" \
+        -i "${CERTNAME}.cert" 2>&1
+
+  CU_ACTION="Verify $CERTNAME's Cert"
+  certu -V -u V -e -n "$CERTNAME" -d "${PROFILEDIR}" -f "${R_PWFILE}"
+  cat > ${TMP}/signalgo.exp <<EOF
+Signature Algorithm: PKCS #1 RSA-PSS Signature
+    Parameters:
+        Hash algorithm: default, SHA-1
+        Mask algorithm: default, MGF1
+        Mask hash algorithm: default, SHA-1
+        Salt length: default, 20 (0x14)
+EOF
+  check_sign_algo
+
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
+  # Subject certificate: RSA-PSS
+  # Issuer certificate: RSA-PSS
+  # Signature: RSA-PSS (implicit, without --pss-sign, default parameters)
+  CERTNAME="TestUser-rsa-pss10"
+
+  CU_ACTION="Generate Cert Request for $CERTNAME"
+  CU_SUBJECT="CN=$CERTNAME, E=${CERTNAME}@bogus.com, O=BOGUS NSS, L=Mountain View, ST=California, C=US"
+  certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" -o req  2>&1
+
+  CU_ACTION="Sign ${CERTNAME}'s Request"
+  # Sign without --pss-sign nor -Z option
+  certu -C -c "TestCA-rsa-pss-sha1" -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
+        -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
+
+  CU_ACTION="Import $CERTNAME's Cert"
+  certu -A -n "$CERTNAME" -t ",," -d "${PROFILEDIR}" -f "${R_PWFILE}" \
+        -i "${CERTNAME}.cert" 2>&1
+
+  CU_ACTION="Verify $CERTNAME's Cert"
+  certu -V -u V -e -n "$CERTNAME" -d "${PROFILEDIR}" -f "${R_PWFILE}"
+  cat > ${TMP}/signalgo.exp <<EOF
+Signature Algorithm: PKCS #1 RSA-PSS Signature
+    Parameters:
+        Hash algorithm: default, SHA-1
+        Mask algorithm: default, MGF1
+        Mask hash algorithm: default, SHA-1
+        Salt length: default, 20 (0x14)
+EOF
+  check_sign_algo
+
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
+  # Subject certificate: RSA-PSS
+  # Issuer certificate: RSA-PSS
+  # Signature: RSA-PSS (with conflicting hash algorithm, default parameters)
+  CERTNAME="TestUser-rsa-pss11"
+
+  CU_ACTION="Generate Cert Request for $CERTNAME"
+  CU_SUBJECT="CN=$CERTNAME, E=${CERTNAME}@bogus.com, O=BOGUS NSS, L=Mountain View, ST=California, C=US"
+  certu -R -d "${PROFILEDIR}" -f "${R_PWFILE}" -z "${R_NOISE_FILE}" --pss -o req  2>&1
+
+  CU_ACTION="Sign ${CERTNAME}'s Request"
+  RETEXPECTED=255
+  certu -C -c "TestCA-rsa-pss-sha1" --pss-sign -Z SHA256 -m "${CERTSERIAL}" -v 60 -d "${P_R_CADIR}" \
+        -i req -o "${CERTNAME}.cert" -f "${R_PWFILE}" "$1" 2>&1
+  RETEXPECTED=0
 }
 
 ############################## cert_cleanup ############################