Bug 1444444, apply crypto-policy on RSA-PSS hash algorithms, r=mt

Reviewers: mt

Reviewed By: mt

Bug #: 1444444

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

6 files changed, 169 insertions, 83 deletions

diff --git a/lib/certhigh/certvfy.c b/lib/certhigh/certvfy.c
index 37dc07e19..3a94a4150 100644
--- a/lib/certhigh/certvfy.c
+++ b/lib/certhigh/certvfy.c
@@ -25,7 +25,7 @@
 #include "pkim.h"
 #include "pki3hack.h"
 #include "base.h"
-#include "keyhi.h"
+#include "keyi.h"
 
 /*
  * Check the validity times of a certificate
@@ -73,12 +73,38 @@ checkKeyParams(const SECAlgorithmID *sigAlgorithm, const SECKEYPublicKey *key)
                 return SECFailure;
             }
             return SECSuccess;
+
+        case SEC_OID_PKCS1_RSA_PSS_SIGNATURE: {
+            PORTCheapArenaPool tmpArena;
+            SECOidTag hashAlg;
+            SECOidTag maskHashAlg;
+
+            PORT_InitCheapArena(&tmpArena, DER_DEFAULT_CHUNKSIZE);
+            rv = sec_DecodeRSAPSSParams(&tmpArena.arena,
+                                        &sigAlgorithm->parameters,
+                                        &hashAlg, &maskHashAlg, NULL);
+            PORT_DestroyCheapArena(&tmpArena);
+            if (rv != SECSuccess) {
+                return SECFailure;
+            }
+
+            if (NSS_GetAlgorithmPolicy(hashAlg, &policyFlags) == SECSuccess &&
+                !(policyFlags & NSS_USE_ALG_IN_CERT_SIGNATURE)) {
+                PORT_SetError(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED);
+                return SECFailure;
+            }
+            if (NSS_GetAlgorithmPolicy(maskHashAlg, &policyFlags) == SECSuccess &&
+                !(policyFlags & NSS_USE_ALG_IN_CERT_SIGNATURE)) {
+                PORT_SetError(SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED);
+                return SECFailure;
+            }
+        }
+        /* fall through to RSA key checking */
         case SEC_OID_PKCS1_MD5_WITH_RSA_ENCRYPTION:
         case SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION:
         case SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION:
         case SEC_OID_PKCS1_SHA384_WITH_RSA_ENCRYPTION:
         case SEC_OID_PKCS1_SHA512_WITH_RSA_ENCRYPTION:
-        case SEC_OID_PKCS1_RSA_PSS_SIGNATURE:
         case SEC_OID_ISO_SHA_WITH_RSA_SIGNATURE:
         case SEC_OID_ISO_SHA1_WITH_RSA_SIGNATURE:
             if (key->keyType != rsaKey && key->keyType != rsaPssKey) {
diff --git a/lib/cryptohi/keyi.h b/lib/cryptohi/keyi.h
index ee11fc905..b746d3c8d 100644
--- a/lib/cryptohi/keyi.h
+++ b/lib/cryptohi/keyi.h
@@ -17,8 +17,21 @@ KeyType seckey_GetKeyType(SECOidTag pubKeyOid);
 SECStatus sec_DecodeSigAlg(const SECKEYPublicKey *key, SECOidTag sigAlg,
                            const SECItem *param, SECOidTag *encalg, SECOidTag *hashalg);
 
-SECStatus sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
-                                      const SECKEYRSAPSSParams *params);
+/* extract the RSA-PSS hash algorithms and salt length from
+ * parameters, taking into account of the default implications.
+ *
+ * (parameters is the parameters field of a algorithm ID structure
+ * (SECAlgorithmID)*/
+SECStatus sec_DecodeRSAPSSParams(PLArenaPool *arena,
+                                 const SECItem *params,
+                                 SECOidTag *hashAlg,
+                                 SECOidTag *maskHashAlg,
+                                 unsigned long *saltLength);
+
+/* convert the encoded RSA-PSS parameters into PKCS #11 mechanism parameters */
+SECStatus sec_DecodeRSAPSSParamsToMechanism(PLArenaPool *arena,
+                                            const SECItem *params,
+                                            CK_RSA_PKCS_PSS_PARAMS *mech);
 
 SEC_END_PROTOS
 
diff --git a/lib/cryptohi/seckey.c b/lib/cryptohi/seckey.c
index 0f9353f3b..080909772 100644
--- a/lib/cryptohi/seckey.c
+++ b/lib/cryptohi/seckey.c
@@ -2015,66 +2015,63 @@ sec_GetMgfTypeByOidTag(SECOidTag tag)
 }
 
 SECStatus
-sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
-                            const SECKEYRSAPSSParams *params)
+sec_DecodeRSAPSSParams(PLArenaPool *arena,
+                       const SECItem *params,
+                       SECOidTag *retHashAlg, SECOidTag *retMaskHashAlg,
+                       unsigned long *retSaltLength)
 {
-    SECStatus rv = SECSuccess;
-    SECOidTag hashAlgTag;
+    SECKEYRSAPSSParams pssParams;
+    SECOidTag hashAlg;
+    SECOidTag maskHashAlg;
     unsigned long saltLength;
     unsigned long trailerField;
+    SECStatus rv;
 
-    PORT_Memset(mech, 0, sizeof(CK_RSA_PKCS_PSS_PARAMS));
+    PORT_Memset(&pssParams, 0, sizeof(pssParams));
+    rv = SEC_QuickDERDecodeItem(arena, &pssParams,
+                                SECKEY_RSAPSSParamsTemplate,
+                                params);
+    if (rv != SECSuccess) {
+        return rv;
+    }
 
-    if (params->hashAlg) {
-        hashAlgTag = SECOID_GetAlgorithmTag(params->hashAlg);
+    if (pssParams.hashAlg) {
+        hashAlg = SECOID_GetAlgorithmTag(pssParams.hashAlg);
     } else {
-        hashAlgTag = SEC_OID_SHA1; /* default, SHA-1 */
-    }
-    mech->hashAlg = sec_GetHashMechanismByOidTag(hashAlgTag);
-    if (mech->hashAlg == CKM_INVALID_MECHANISM) {
-        return SECFailure;
+        hashAlg = SEC_OID_SHA1; /* default, SHA-1 */
     }
 
-    if (params->maskAlg) {
-        SECAlgorithmID maskHashAlg;
-        SECOidTag maskHashAlgTag;
-        PORTCheapArenaPool tmpArena;
+    if (pssParams.maskAlg) {
+        SECAlgorithmID algId;
 
-        if (SECOID_GetAlgorithmTag(params->maskAlg) != SEC_OID_PKCS1_MGF1) {
+        if (SECOID_GetAlgorithmTag(pssParams.maskAlg) != SEC_OID_PKCS1_MGF1) {
             /* only MGF1 is known to PKCS#11 */
             PORT_SetError(SEC_ERROR_INVALID_ALGORITHM);
             return SECFailure;
         }
 
-        PORT_InitCheapArena(&tmpArena, DER_DEFAULT_CHUNKSIZE);
-        rv = SEC_QuickDERDecodeItem(&tmpArena.arena, &maskHashAlg,
+        rv = SEC_QuickDERDecodeItem(arena, &algId,
                                     SEC_ASN1_GET(SECOID_AlgorithmIDTemplate),
-                                    &params->maskAlg->parameters);
-        PORT_DestroyCheapArena(&tmpArena);
+                                    &pssParams.maskAlg->parameters);
         if (rv != SECSuccess) {
             return rv;
         }
-        maskHashAlgTag = SECOID_GetAlgorithmTag(&maskHashAlg);
-        mech->mgf = sec_GetMgfTypeByOidTag(maskHashAlgTag);
-        if (mech->mgf == 0) {
-            return SECFailure;
-        }
+        maskHashAlg = SECOID_GetAlgorithmTag(&algId);
     } else {
-        mech->mgf = CKG_MGF1_SHA1; /* default, MGF1 with SHA-1 */
+        maskHashAlg = SEC_OID_SHA1; /* default, MGF1 with SHA-1 */
     }
 
-    if (params->saltLength.data) {
-        rv = SEC_ASN1DecodeInteger((SECItem *)&params->saltLength, &saltLength);
+    if (pssParams.saltLength.data) {
+        rv = SEC_ASN1DecodeInteger((SECItem *)&pssParams.saltLength, &saltLength);
         if (rv != SECSuccess) {
             return rv;
         }
     } else {
         saltLength = 20; /* default, 20 */
     }
-    mech->sLen = saltLength;
 
-    if (params->trailerField.data) {
-        rv = SEC_ASN1DecodeInteger((SECItem *)&params->trailerField, &trailerField);
+    if (pssParams.trailerField.data) {
+        rv = SEC_ASN1DecodeInteger((SECItem *)&pssParams.trailerField, &trailerField);
         if (rv != SECSuccess) {
             return rv;
         }
@@ -2086,5 +2083,46 @@ sec_RSAPSSParamsToMechanism(CK_RSA_PKCS_PSS_PARAMS *mech,
         }
     }
 
-    return rv;
+    if (retHashAlg) {
+        *retHashAlg = hashAlg;
+    }
+    if (retMaskHashAlg) {
+        *retMaskHashAlg = maskHashAlg;
+    }
+    if (retSaltLength) {
+        *retSaltLength = saltLength;
+    }
+
+    return SECSuccess;
+}
+
+SECStatus
+sec_DecodeRSAPSSParamsToMechanism(PLArenaPool *arena,
+                                  const SECItem *params,
+                                  CK_RSA_PKCS_PSS_PARAMS *mech)
+{
+    SECOidTag hashAlg;
+    SECOidTag maskHashAlg;
+    unsigned long saltLength;
+    SECStatus rv;
+
+    rv = sec_DecodeRSAPSSParams(arena, params,
+                                &hashAlg, &maskHashAlg, &saltLength);
+    if (rv != SECSuccess) {
+        return SECFailure;
+    }
+
+    mech->hashAlg = sec_GetHashMechanismByOidTag(hashAlg);
+    if (mech->hashAlg == CKM_INVALID_MECHANISM) {
+        return SECFailure;
+    }
+
+    mech->mgf = sec_GetMgfTypeByOidTag(maskHashAlg);
+    if (mech->mgf == 0) {
+        return SECFailure;
+    }
+
+    mech->sLen = saltLength;
+
+    return SECSuccess;
 }
diff --git a/lib/cryptohi/secsign.c b/lib/cryptohi/secsign.c
index dc10f2fa6..8a8d0f664 100644
--- a/lib/cryptohi/secsign.c
+++ b/lib/cryptohi/secsign.c
@@ -225,22 +225,13 @@ SGN_End(SGNContext *cx, SECItem *result)
         PORT_Memset(&mech, 0, sizeof(mech));
 
         if (cx->params && cx->params->data) {
-            SECKEYRSAPSSParams params;
-
             arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
             if (!arena) {
                 rv = SECFailure;
                 goto loser;
             }
 
-            PORT_Memset(&params, 0, sizeof(params));
-            rv = SEC_QuickDERDecodeItem(arena, &params,
-                                        SECKEY_RSAPSSParamsTemplate,
-                                        cx->params);
-            if (rv != SECSuccess) {
-                goto loser;
-            }
-            rv = sec_RSAPSSParamsToMechanism(&mech, &params);
+            rv = sec_DecodeRSAPSSParamsToMechanism(arena, cx->params, &mech);
             if (rv != SECSuccess) {
                 goto loser;
             }
diff --git a/lib/cryptohi/secvfy.c b/lib/cryptohi/secvfy.c
index 193738fed..aa3d6778c 100644
--- a/lib/cryptohi/secvfy.c
+++ b/lib/cryptohi/secvfy.c
@@ -257,25 +257,13 @@ sec_DecodeSigAlg(const SECKEYPublicKey *key, SECOidTag sigAlg,
             break;
         case SEC_OID_PKCS1_RSA_PSS_SIGNATURE:
             if (param && param->data) {
-                SECKEYRSAPSSParams pssParam;
-                arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-                if (arena == NULL) {
-                    return SECFailure;
-                }
-                PORT_Memset(&pssParam, 0, sizeof pssParam);
-                rv = SEC_QuickDERDecodeItem(arena, &pssParam,
-                                            SECKEY_RSAPSSParamsTemplate,
-                                            param);
-                if (rv != SECSuccess) {
-                    PORT_FreeArena(arena, PR_FALSE);
-                    return rv;
-                }
-                if (pssParam.hashAlg) {
-                    *hashalg = SECOID_GetAlgorithmTag(pssParam.hashAlg);
-                } else {
-                    *hashalg = SEC_OID_SHA1; /* default, SHA-1 */
-                }
-                PORT_FreeArena(arena, PR_FALSE);
+                PORTCheapArenaPool tmpArena;
+
+                PORT_InitCheapArena(&tmpArena, DER_DEFAULT_CHUNKSIZE);
+                rv = sec_DecodeRSAPSSParams(&tmpArena.arena, param,
+                                            hashalg, NULL, NULL);
+                PORT_DestroyCheapArena(&tmpArena);
+
                 /* only accept hash algorithms */
                 if (HASH_GetHashTypeByOidTag(*hashalg) == HASH_AlgNULL) {
                     /* error set by HASH_GetHashTypeByOidTag */
@@ -658,27 +646,17 @@ VFY_EndWithSignature(VFYContext *cx, SECItem *sig)
             if (cx->encAlg == SEC_OID_PKCS1_RSA_PSS_SIGNATURE) {
                 CK_RSA_PKCS_PSS_PARAMS mech;
                 SECItem mechItem = { siBuffer, (unsigned char *)&mech, sizeof(mech) };
-                SECKEYRSAPSSParams params;
-                PLArenaPool *arena;
+                PORTCheapArenaPool tmpArena;
 
-                arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-                if (arena == NULL) {
-                    return SECFailure;
-                }
-
-                PORT_Memset(&params, 0, sizeof(params));
-                rv = SEC_QuickDERDecodeItem(arena, &params,
-                                            SECKEY_RSAPSSParamsTemplate,
-                                            cx->params);
-                if (rv != SECSuccess) {
-                    PORT_FreeArena(arena, PR_FALSE);
-                    return SECFailure;
-                }
-                rv = sec_RSAPSSParamsToMechanism(&mech, &params);
-                PORT_FreeArena(arena, PR_FALSE);
+                PORT_InitCheapArena(&tmpArena, DER_DEFAULT_CHUNKSIZE);
+                rv = sec_DecodeRSAPSSParamsToMechanism(&tmpArena.arena,
+                                                       cx->params,
+                                                       &mech);
+                PORT_DestroyCheapArena(&tmpArena);
                 if (rv != SECSuccess) {
                     return SECFailure;
                 }
+
                 rsasig.data = cx->u.buffer;
                 rsasig.len = SECKEY_SignatureLen(cx->key);
                 if (rsasig.len == 0) {
diff --git a/tests/cert/cert.sh b/tests/cert/cert.sh
index 6d0547c9b..b74de9be5 100755
--- a/tests/cert/cert.sh
+++ b/tests/cert/cert.sh
@@ -2561,6 +2561,43 @@ cert_test_orphan_key_reuse()
   fi
 }
 
+cert_test_rsapss_policy()
+{
+  CERTSERIAL=`expr $CERTSERIAL + 1`
+
+  CERTNAME="TestUser-rsa-pss-policy"
+
+  # Subject certificate: RSA-PSS
+  # Issuer certificate: RSA
+  # Signature: RSA-PSS (explicit, with --pss-sign and -Z SHA1)
+  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"
+  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 -n "TestUser-rsa-pss-policy" -u V -V -e -d "${PROFILEDIR}" -f "${R_PWFILE}"
+
+  CU_ACTION="Verify $CERTNAME's Cert with Policy"
+  cp ${PROFILEDIR}/pkcs11.txt pkcs11.txt.orig
+  cat >> ${PROFILEDIR}/pkcs11.txt << ++EOF++
+library=
+name=Policy
+config="disallow=SHA1"
+++EOF++
+  RETEXPECTED=255
+  certu -V -n "TestUser-rsa-pss-policy" -u V -V -e -d "${PROFILEDIR}" -f "${R_PWFILE}"
+  RETEXPECTED=0
+  cp pkcs11.txt.orig ${PROFILEDIR}/pkcs11.txt
+}
+
 ############################## cert_cleanup ############################
 # local shell function to finish this script (no exit since it might be
 # sourced)
@@ -2596,6 +2633,9 @@ cert_test_password
 cert_test_distrust
 cert_test_ocspresp
 cert_test_rsapss
+if [ "${TEST_MODE}" = "SHARED_DB" ] ; then
+  cert_test_rsapss_policy
+fi
 cert_test_token_uri
 
 if [ -z "$NSS_TEST_DISABLE_CRL" ] ; then