Bug 1806496 In FIPS mode, nss should reject RSASSA-PSS salt lengths larger than the output size of the hash function used, or provide an indicator

This patch adds a new mechanism specific check for PSS in fip_algorithms.h.
The new check uses the hash mechanism provided in the pss mechanism list to look up the hash length. A static utility function in pkcs11c.c is moved to pkcs11u.c and made global so it can be reused in this code.

We know that mechanism supplied in the parameters matches the hash because that check is enforces in pkcs11c.c for the combined hash and signed functions.

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

4 files changed, 93 insertions, 65 deletions

diff --git a/lib/softoken/fips_algorithms.h b/lib/softoken/fips_algorithms.h
index 9084c8341..5ead006d5 100644
--- a/lib/softoken/fips_algorithms.h
+++ b/lib/softoken/fips_algorithms.h
@@ -11,9 +11,10 @@
  * in place for that class */
 typedef enum {
     SFTKFIPSNone = 0,
-    SFTKFIPSDH,  /* allow only specific primes */
-    SFTKFIPSECC, /* not just keys but specific curves */
-    SFTKFIPSAEAD /* single shot AEAD functions not allowed in FIPS mode */
+    SFTKFIPSDH,   /* allow only specific primes */
+    SFTKFIPSECC,  /* not just keys but specific curves */
+    SFTKFIPSAEAD, /* single shot AEAD functions not allowed in FIPS mode */
+    SFTKFIPSRSAPSS
 } SFTKFIPSSpecialClass;
 
 typedef struct SFTKFIPSAlgorithmListStr SFTKFIPSAlgorithmList;
@@ -66,26 +67,26 @@ SFTKFIPSAlgorithmList sftk_fips_mechs[] = {
 #define AES_FB_KEY 128, 256
 #define AES_FB_STEP 64
     { CKM_RSA_PKCS_KEY_PAIR_GEN, { RSA_FB_KEY, CKF_KPG }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
+    { CKM_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSRSAPSS },
     { CKM_RSA_PKCS_OAEP, { RSA_FB_KEY, CKF_ENC }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
+    { CKM_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
     /* -------------- RSA Multipart Signing Operations -------------------- */
     { CKM_SHA224_RSA_PKCS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
     { CKM_SHA256_RSA_PKCS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
     { CKM_SHA384_RSA_PKCS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
     { CKM_SHA512_RSA_PKCS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA224_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA256_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA384_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA512_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA224_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA256_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA384_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA512_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA224_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA256_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA384_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
-    { CKM_SHA512_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSNone },
+    { CKM_SHA224_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA256_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA384_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA512_RSA_PKCS_PSS, { RSA_FB_KEY, CKF_SGN }, RSA_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA224_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA256_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA384_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA512_RSA_PKCS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA224_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA256_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA384_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
+    { CKM_SHA512_RSA_PKCS_PSS, { RSA_LEGACY_FB_KEY, CKF_VERIFY }, RSA_LEGACY_FB_STEP, SFTKFIPSRSAPSS },
     /* ------------------------- DSA Operations --------------------------- */
     { CKM_DSA_KEY_PAIR_GEN, { DSA_FB_KEY, CKF_KPG }, DSA_FB_STEP, SFTKFIPSNone },
     { CKM_DSA, { DSA_FB_KEY, CKF_SGN }, DSA_FB_STEP, SFTKFIPSNone },
diff --git a/lib/softoken/pkcs11c.c b/lib/softoken/pkcs11c.c
index cbdd2108a..f2cc984a8 100644
--- a/lib/softoken/pkcs11c.c
+++ b/lib/softoken/pkcs11c.c
@@ -237,37 +237,6 @@ NSC_DestroyObject(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hObject)
 }
 
 /*
- ************** Crypto Functions:     Utilities ************************
- */
-/*
- * Utility function for converting PSS/OAEP parameter types into
- * HASH_HashTypes. Note: Only SHA family functions are defined in RFC 3447.
- */
-static HASH_HashType
-GetHashTypeFromMechanism(CK_MECHANISM_TYPE mech)
-{
-    switch (mech) {
-        case CKM_SHA_1:
-        case CKG_MGF1_SHA1:
-            return HASH_AlgSHA1;
-        case CKM_SHA224:
-        case CKG_MGF1_SHA224:
-            return HASH_AlgSHA224;
-        case CKM_SHA256:
-        case CKG_MGF1_SHA256:
-            return HASH_AlgSHA256;
-        case CKM_SHA384:
-        case CKG_MGF1_SHA384:
-            return HASH_AlgSHA384;
-        case CKM_SHA512:
-        case CKG_MGF1_SHA512:
-            return HASH_AlgSHA512;
-        default:
-            return HASH_AlgNULL;
-    }
-}
-
-/*
  * Returns true if "params" contains a valid set of PSS parameters
  */
 static PRBool
@@ -276,8 +245,8 @@ sftk_ValidatePssParams(const CK_RSA_PKCS_PSS_PARAMS *params)
     if (!params) {
         return PR_FALSE;
     }
-    if (GetHashTypeFromMechanism(params->hashAlg) == HASH_AlgNULL ||
-        GetHashTypeFromMechanism(params->mgf) == HASH_AlgNULL) {
+    if (sftk_GetHashTypeFromMechanism(params->hashAlg) == HASH_AlgNULL ||
+        sftk_GetHashTypeFromMechanism(params->mgf) == HASH_AlgNULL) {
         return PR_FALSE;
     }
     return PR_TRUE;
@@ -298,8 +267,8 @@ sftk_ValidateOaepParams(const CK_RSA_PKCS_OAEP_PARAMS *params)
      *   ulSourceDataLen must be zero.
      */
     if (params->source != CKZ_DATA_SPECIFIED ||
-        (GetHashTypeFromMechanism(params->hashAlg) == HASH_AlgNULL) ||
-        (GetHashTypeFromMechanism(params->mgf) == HASH_AlgNULL) ||
+        (sftk_GetHashTypeFromMechanism(params->hashAlg) == HASH_AlgNULL) ||
+        (sftk_GetHashTypeFromMechanism(params->mgf) == HASH_AlgNULL) ||
         (params->ulSourceDataLen == 0 && params->pSourceData != NULL) ||
         (params->ulSourceDataLen != 0 && params->pSourceData == NULL)) {
         return PR_FALSE;
@@ -611,8 +580,8 @@ sftk_RSAEncryptOAEP(SFTKOAEPInfo *info, unsigned char *output,
         return SECFailure;
     }
 
-    hashAlg = GetHashTypeFromMechanism(info->params.hashAlg);
-    maskHashAlg = GetHashTypeFromMechanism(info->params.mgf);
+    hashAlg = sftk_GetHashTypeFromMechanism(info->params.hashAlg);
+    maskHashAlg = sftk_GetHashTypeFromMechanism(info->params.mgf);
 
     return RSA_EncryptOAEP(&info->key.pub->u.rsa, hashAlg, maskHashAlg,
                            (const unsigned char *)info->params.pSourceData,
@@ -635,8 +604,8 @@ sftk_RSADecryptOAEP(SFTKOAEPInfo *info, unsigned char *output,
         return SECFailure;
     }
 
-    hashAlg = GetHashTypeFromMechanism(info->params.hashAlg);
-    maskHashAlg = GetHashTypeFromMechanism(info->params.mgf);
+    hashAlg = sftk_GetHashTypeFromMechanism(info->params.hashAlg);
+    maskHashAlg = sftk_GetHashTypeFromMechanism(info->params.mgf);
 
     rv = RSA_DecryptOAEP(&info->key.priv->u.rsa, hashAlg, maskHashAlg,
                          (const unsigned char *)info->params.pSourceData,
@@ -2656,8 +2625,8 @@ sftk_RSASignPSS(SFTKPSSSignInfo *info, unsigned char *sig,
         return SECFailure;
     }
 
-    hashAlg = GetHashTypeFromMechanism(params->hashAlg);
-    maskHashAlg = GetHashTypeFromMechanism(params->mgf);
+    hashAlg = sftk_GetHashTypeFromMechanism(params->hashAlg);
+    maskHashAlg = sftk_GetHashTypeFromMechanism(params->mgf);
 
     rv = RSA_SignPSS(&info->key->u.rsa, hashAlg, maskHashAlg, NULL,
                      params->sLen, sig, sigLen, maxLen, hash, hashLen);
@@ -3031,7 +3000,7 @@ NSC_SignInit(CK_SESSION_HANDLE hSession,
             } else {
                 /* The hash function for the TLS 1.2 PRF */
                 tlsPrfHash =
-                    GetHashTypeFromMechanism(tls12_mac_params->prfHashMechanism);
+                    sftk_GetHashTypeFromMechanism(tls12_mac_params->prfHashMechanism);
                 if (tlsPrfHash == HASH_AlgNULL ||
                     tls12_mac_params->ulMacLength < 12) {
                     crv = CKR_MECHANISM_PARAM_INVALID;
@@ -3549,8 +3518,8 @@ sftk_RSACheckSignPSS(SFTKPSSVerifyInfo *info, const unsigned char *sig,
         return SECFailure;
     }
 
-    hashAlg = GetHashTypeFromMechanism(params->hashAlg);
-    maskHashAlg = GetHashTypeFromMechanism(params->mgf);
+    hashAlg = sftk_GetHashTypeFromMechanism(params->hashAlg);
+    maskHashAlg = sftk_GetHashTypeFromMechanism(params->mgf);
 
     return RSA_CheckSignPSS(&info->key->u.rsa, hashAlg, maskHashAlg,
                             params->sLen, sig, sigLen, digest, digestLen);
@@ -6961,7 +6930,7 @@ sftk_HKDF(CK_HKDF_PARAMS_PTR params, CK_SESSION_HANDLE hSession,
     const unsigned char *prk;               /* psuedo-random key */
     CK_ULONG prkLen;
     const unsigned char *okm; /* output keying material */
-    HASH_HashType hashType = GetHashTypeFromMechanism(params->prfHashMechanism);
+    HASH_HashType hashType = sftk_GetHashTypeFromMechanism(params->prfHashMechanism);
     SFTKObject *saltKey = NULL;
     CK_RV crv = CKR_OK;
 
@@ -7399,7 +7368,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,
                 }
                 CK_TLS12_MASTER_KEY_DERIVE_PARAMS *tls12_master =
                     (CK_TLS12_MASTER_KEY_DERIVE_PARAMS *)pMechanism->pParameter;
-                tlsPrfHash = GetHashTypeFromMechanism(tls12_master->prfHashMechanism);
+                tlsPrfHash = sftk_GetHashTypeFromMechanism(tls12_master->prfHashMechanism);
                 if (tlsPrfHash == HASH_AlgNULL) {
                     crv = CKR_MECHANISM_PARAM_INVALID;
                     break;
@@ -7617,7 +7586,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,
             } else {
                 const SECHashObject *hashObj;
 
-                tlsPrfHash = GetHashTypeFromMechanism(ems_params->prfHashMechanism);
+                tlsPrfHash = sftk_GetHashTypeFromMechanism(ems_params->prfHashMechanism);
                 if (tlsPrfHash == HASH_AlgNULL) {
                     crv = CKR_MECHANISM_PARAM_INVALID;
                     break;
@@ -7675,7 +7644,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,
                 }
                 CK_TLS12_KEY_MAT_PARAMS *tls12_keys =
                     (CK_TLS12_KEY_MAT_PARAMS *)pMechanism->pParameter;
-                tlsPrfHash = GetHashTypeFromMechanism(tls12_keys->prfHashMechanism);
+                tlsPrfHash = sftk_GetHashTypeFromMechanism(tls12_keys->prfHashMechanism);
                 if (tlsPrfHash == HASH_AlgNULL) {
                     crv = CKR_MECHANISM_PARAM_INVALID;
                     break;
diff --git a/lib/softoken/pkcs11i.h b/lib/softoken/pkcs11i.h
index e4719a8ee..b0408dc43 100644
--- a/lib/softoken/pkcs11i.h
+++ b/lib/softoken/pkcs11i.h
@@ -914,6 +914,9 @@ void sftk_MACConstantTime_EndHash(
     void *pctx, void *out, unsigned int *outLength, unsigned int maxLength);
 void sftk_MACConstantTime_DestroyContext(void *pctx, PRBool);
 
+/* Crypto Utilities */
+HASH_HashType sftk_GetHashTypeFromMechanism(CK_MECHANISM_TYPE mech);
+
 /****************************************
  * implement TLS Pseudo Random Function (PRF)
  */
diff --git a/lib/softoken/pkcs11u.c b/lib/softoken/pkcs11u.c
index 1d0f8dacf..f48306098 100644
--- a/lib/softoken/pkcs11u.c
+++ b/lib/softoken/pkcs11u.c
@@ -2248,7 +2248,39 @@ sftk_AttributeToFlags(CK_ATTRIBUTE_TYPE op)
     return flags;
 }
 
+/*
+ * ******************** Hash Utilities **************************
+ */
+/*
+ * Utility function for converting PSS/OAEP parameter types into
+ * HASH_HashTypes. Note: Only SHA family functions are defined in RFC 3447.
+ */
+HASH_HashType
+sftk_GetHashTypeFromMechanism(CK_MECHANISM_TYPE mech)
+{
+    switch (mech) {
+        case CKM_SHA_1:
+        case CKG_MGF1_SHA1:
+            return HASH_AlgSHA1;
+        case CKM_SHA224:
+        case CKG_MGF1_SHA224:
+            return HASH_AlgSHA224;
+        case CKM_SHA256:
+        case CKG_MGF1_SHA256:
+            return HASH_AlgSHA256;
+        case CKM_SHA384:
+        case CKG_MGF1_SHA384:
+            return HASH_AlgSHA384;
+        case CKM_SHA512:
+        case CKG_MGF1_SHA512:
+            return HASH_AlgSHA512;
+        default:
+            return HASH_AlgNULL;
+    }
+}
+
 #ifdef NSS_HAS_FIPS_INDICATORS
+/**************** FIPS Indicator Utilities *************************/
 /* sigh, we probably need a version of this in secutil so that both
  * softoken and NSS can use it */
 static SECOidTag
@@ -2384,6 +2416,29 @@ sftk_handleSpecial(SFTKSlot *slot, CK_MECHANISM *mech,
             }
             return PR_FALSE;
         }
+        case SFTKFIPSRSAPSS: {
+            /* PSS salt must not be longer than the  underlying hash.
+             * We verify that the underlying hash of the
+             * parameters matches Hash of the combined hash mechanisms, so
+             * we don't need to look at the specific PSS mechanism */
+            CK_RSA_PKCS_PSS_PARAMS *pss = (CK_RSA_PKCS_PSS_PARAMS *)
+                                              mech->pParameter;
+            const SECHashObject *hashObj = NULL;
+            if (mech->ulParameterLen != sizeof(*pss)) {
+                return PR_FALSE;
+            }
+            /* we use the existing hash utilities to find the length of
+             * the hash */
+            hashObj = HASH_GetRawHashObject(sftk_GetHashTypeFromMechanism(
+                pss->hashAlg));
+            if (hashObj == NULL) {
+                return PR_FALSE;
+            }
+            if (pss->sLen > hashObj->length) {
+                return PR_FALSE;
+            }
+            return PR_TRUE;
+        }
         default:
             break;
     }