rpms/nss
5
0
Fork 0
Code Issues Pull Requests Releases Activity

Resolves: rhbz#2179385

Browse Source 
Make DH parameter processing in FIPS mode more strict.
Fix memory leak in dh keygen.
This commit is contained in:
Bob Relyea 2023-03-22 09:38:23 -07:00
parent 2ed3d453e9
commit 7391e8d0cd
2 changed files with 199 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

213
nss-3.79-fips-review.patches
View File

@ -1,6 +1,6 @@
diff -up ./lib/freebl/dh.c.fips-review ./lib/freebl/dh.c
--- ./lib/freebl/dh.c.fips-review 2022-05-26 02:54:33.000000000 -0700
+++ ./lib/freebl/dh.c 2023-03-16 11:54:37.839935303 -0700
+++ ./lib/freebl/dh.c 2023-03-17 10:53:23.552658592 -0700
@@ -445,7 +445,7 @@ cleanup:
PRBool
KEA_Verify(SECItem *Y, SECItem *prime, SECItem *subPrime)
@ -50,8 +50,8 @@ diff -up ./lib/freebl/dh.c.fips-review ./lib/freebl/dh.c
MP_TO_SEC_ERROR(err);
return PR_FALSE;
diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
--- ./lib/softoken/pkcs11c.c.fips-review 2023-03-16 11:53:04.703068972 -0700
+++ ./lib/softoken/pkcs11c.c 2023-03-16 11:55:23.498360007 -0700
--- ./lib/softoken/pkcs11c.c.fips-review 2023-03-17 10:53:13.569590567 -0700
+++ ./lib/softoken/pkcs11c.c 2023-03-17 10:53:23.552658592 -0700
@@ -4780,6 +4780,10 @@ NSC_GenerateKey(CK_SESSION_HANDLE hSessi
* handle the base object stuff
*/
@ -189,9 +189,29 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
/* FIPS requires full validation, but in fipx mode NSC_Derive
* only does partial validation with approved primes, now handle
* full validation */
@@ -5166,18 +5174,41 @@ sftk_PairwiseConsistencyCheck(CK_SESSION
}
@@ -5154,44 +5162,78 @@ sftk_PairwiseConsistencyCheck(CK_SESSION
SECItem pubKey;
SECItem prime;
SECItem subPrime;
+ SECItem base;
+ SECItem generator;
const SECItem *subPrimePtr = &subPrime;
pubKey.data = pubAttribute->attrib.pValue;
pubKey.len = pubAttribute->attrib.ulValueLen;
- prime.data = subPrime.data = NULL;
- prime.len = subPrime.len = 0;
+ base.data = prime.data = subPrime.data = NULL;
+ base.len = prime.len = subPrime.len = 0;
crv = sftk_Attribute2SecItem(NULL, &prime, privateKey, CKA_PRIME);
if (crv != CKR_OK) {
goto done;
}
- crv = sftk_Attribute2SecItem(NULL, &prime, privateKey, CKA_PRIME);
+ crv = sftk_Attribute2SecItem(NULL, &base, privateKey, CKA_BASE);
+ if (crv != CKR_OK) {
+ goto done;
+ }
/* we ignore the return code an only look at the length */
- if (subPrime.len == 0) {
- /* subprime not supplied, In this case look it up.
@ -201,12 +221,12 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
- if (subPrimePtr == NULL) {
- crv = CKR_GENERAL_ERROR;
+ /* do we have a known prime ? */
+ subPrimePtr = sftk_VerifyDH_Prime(&prime, isFIPS);
+ subPrimePtr = sftk_VerifyDH_Prime(&prime, &generator, isFIPS);
+ if (subPrimePtr == NULL) {
+ if (subPrime.len == 0) {
+ /* if not a known prime, subprime must be supplied */
+ crv = CKR_ATTRIBUTE_VALUE_INVALID;
goto done;
+ goto done;
+ } else {
+ /* not a known prime, check for primality of prime
+ * and subPrime */
@ -218,16 +238,25 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
+ crv = CKR_ATTRIBUTE_VALUE_INVALID;
+ goto done;
+ }
}
+ /* if we aren't using a defined group, make sure base is in the
+ * subgroup. If it's not, then our key could fail or succeed sometimes.
+ * This makes the failure reliable */
+ if (!KEA_Verify(&base, &prime, (SECItem *)subPrimePtr)) {
+ crv = CKR_ATTRIBUTE_VALUE_INVALID;
+ }
+ }
+ subPrimePtr = &subPrime;
+ } else {
+ /* we're using a known group, make sure we are using the known generator for that group */
+ if (SECITEM_CompareItem(&generator, &base) != 0) {
+ crv = CKR_ATTRIBUTE_VALUE_INVALID;
goto done;
}
+ if (subPrime.len != 0) {
+ /* we have a known prime and a supplied subPrime,
+ * make sure the subPrime matches the subPrime for
+ * the known Prime */
+ if ((subPrimePtr->len != subPrimeLen) ||
+ (PORT_Memcmp(subPrimePtr->data, subPrime.data,
+ subPrime.len) != 0)) {
+ if (SECITEM_CompareItem(subPrimePtr, &subPrime) != 0) {
+ crv = CKR_ATTRIBUTE_VALUE_INVALID;
+ goto done;
+ }
@ -238,8 +267,9 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
+ crv = CKR_ATTRIBUTE_VALUE_INVALID;
}
done:
+ SECITEM_ZfreeItem(&base, PR_FALSE);
SECITEM_ZfreeItem(&subPrime, PR_FALSE);
@@ -5185,13 +5216,9 @@ sftk_PairwiseConsistencyCheck(CK_SESSION
SECITEM_ZfreeItem(&prime, PR_FALSE);
}
/* clean up before we return */
sftk_FreeAttribute(pubAttribute);
@ -253,7 +283,7 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
}
return CKR_OK;
@@ -5709,8 +5736,8 @@ NSC_GenerateKeyPair(CK_SESSION_HANDLE hS
@@ -5709,8 +5751,8 @@ NSC_GenerateKeyPair(CK_SESSION_HANDLE hS
* created and linked.
*/
crv = sftk_handleObject(publicKey, session);
@ -263,7 +293,7 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
sftk_FreeObject(publicKey);
NSC_DestroyObject(hSession, privateKey->handle);
sftk_FreeObject(privateKey);
@@ -5752,6 +5779,7 @@ NSC_GenerateKeyPair(CK_SESSION_HANDLE hS
@@ -5752,6 +5794,7 @@ NSC_GenerateKeyPair(CK_SESSION_HANDLE hS
}
if (crv != CKR_OK) {
@ -271,7 +301,7 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
NSC_DestroyObject(hSession, publicKey->handle);
sftk_FreeObject(publicKey);
NSC_DestroyObject(hSession, privateKey->handle);
@@ -5761,6 +5789,8 @@ NSC_GenerateKeyPair(CK_SESSION_HANDLE hS
@@ -5761,6 +5804,8 @@ NSC_GenerateKeyPair(CK_SESSION_HANDLE hS
/* we need to do this check at the end to make sure the generated key meets the key length requirements */
privateKey->isFIPS = sftk_operationIsFIPS(slot, pMechanism, CKA_NSS_GENERATE_KEY_PAIR, privateKey);
publicKey->isFIPS = privateKey->isFIPS;
@ -280,7 +310,16 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
*phPrivateKey = privateKey->handle;
*phPublicKey = publicKey->handle;
@@ -8563,6 +8593,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
@@ -8381,7 +8426,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
/* if the prime is an approved prime, we can skip all the other
* checks. */
- subPrime = sftk_VerifyDH_Prime(&dhPrime, isFIPS);
+ subPrime = sftk_VerifyDH_Prime(&dhPrime, NULL, isFIPS);
if (subPrime == NULL) {
SECItem dhSubPrime;
/* If the caller set the subprime value, it means that
@@ -8563,6 +8608,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
secretlen = tmp.len;
} else {
secretlen = keySize;
@ -290,7 +329,7 @@ diff -up ./lib/softoken/pkcs11c.c.fips-review ./lib/softoken/pkcs11c.c
mechParams->ulSharedDataLen, mechParams->kdf);
diff -up ./lib/softoken/pkcs11.c.fips-review ./lib/softoken/pkcs11.c
--- ./lib/softoken/pkcs11.c.fips-review 2022-05-26 02:54:33.000000000 -0700
+++ ./lib/softoken/pkcs11.c 2023-03-16 11:54:37.840935312 -0700
+++ ./lib/softoken/pkcs11.c 2023-03-17 10:53:23.553658599 -0700
@@ -4599,7 +4599,10 @@ NSC_CreateObject(CK_SESSION_HANDLE hSess
if (object == NULL) {
return CKR_HOST_MEMORY;
@ -303,9 +342,147 @@ diff -up ./lib/softoken/pkcs11.c.fips-review ./lib/softoken/pkcs11.c
* it's not a FIPS object */
/*
diff -up ./lib/softoken/pkcs11i.h.fips-review ./lib/softoken/pkcs11i.h
--- ./lib/softoken/pkcs11i.h.fips-review 2023-03-17 10:53:13.569590567 -0700
+++ ./lib/softoken/pkcs11i.h 2023-03-17 10:53:23.553658599 -0700
@@ -971,7 +971,7 @@ char **NSC_ModuleDBFunc(unsigned long fu
/* dh verify functions */
/* verify that dhPrime matches one of our known primes, and if so return
* it's subprime value */
-const SECItem *sftk_VerifyDH_Prime(SECItem *dhPrime, PRBool isFIPS);
+const SECItem *sftk_VerifyDH_Prime(SECItem *dhPrime, SECItem *generator, PRBool isFIPS);
/* check if dhSubPrime claims dhPrime is a safe prime. */
SECStatus sftk_IsSafePrime(SECItem *dhPrime, SECItem *dhSubPrime, PRBool *isSafe);
/* map an operation Attribute to a Mechanism flag */
diff -up ./lib/softoken/pkcs11u.c.fips-review ./lib/softoken/pkcs11u.c
--- ./lib/softoken/pkcs11u.c.fips-review 2023-03-17 10:53:13.570590574 -0700
+++ ./lib/softoken/pkcs11u.c 2023-03-17 10:54:53.009268164 -0700
@@ -2404,15 +2404,27 @@ sftk_handleSpecial(SFTKSlot *slot, CK_ME
switch (mechInfo->special) {
case SFTKFIPSDH: {
SECItem dhPrime;
+ SECItem dhBase;
+ SECItem dhGenerator;
+ PRBool val = PR_FALSE;
const SECItem *dhSubPrime;
CK_RV crv = sftk_Attribute2SecItem(NULL, &dhPrime,
source, CKA_PRIME);
if (crv != CKR_OK) {
return PR_FALSE;
}
- dhSubPrime = sftk_VerifyDH_Prime(&dhPrime, PR_TRUE);
+ crv = sftk_Attribute2SecItem(NULL, &dhBase, source, CKA_BASE);
+ if (crv != CKR_OK) {
+ return PR_FALSE;
+ }
+ dhSubPrime = sftk_VerifyDH_Prime(&dhPrime, &dhGenerator, PR_TRUE);
+ val = (dhSubPrime) ? PR_TRUE : PR_FALSE;
+ if (val && (SECITEM_CompareItem(&dhBase, &dhGenerator) != 0)) {
+ val = PR_FALSE;
+ }
SECITEM_ZfreeItem(&dhPrime, PR_FALSE);
- return (dhSubPrime) ? PR_TRUE : PR_FALSE;
+ SECITEM_ZfreeItem(&dhBase, PR_FALSE);
+ return val;
}
case SFTKFIPSNone:
return PR_FALSE;
diff -up ./lib/softoken/sftkdhverify.c.fips-review ./lib/softoken/sftkdhverify.c
--- ./lib/softoken/sftkdhverify.c.fips-review 2022-05-26 02:54:33.000000000 -0700
+++ ./lib/softoken/sftkdhverify.c 2023-03-17 10:53:23.553658599 -0700
@@ -1167,11 +1167,20 @@ static const SECItem subprime_tls_8192 =
(unsigned char *)subprime_tls_8192_data,
sizeof(subprime_tls_8192_data) };
+/* generator for all the groups is 2 */
+static const unsigned char generator_2_data[] = { 2 };
+
+
+static const SECItem generator_2 =
+ { siBuffer,
+ (unsigned char *)generator_2_data,
+ sizeof(generator_2_data) };
+
/*
* verify that dhPrime matches one of our known primes
*/
const SECItem *
-sftk_VerifyDH_Prime(SECItem *dhPrime, PRBool isFIPS)
+sftk_VerifyDH_Prime(SECItem *dhPrime, SECItem *g, PRBool isFIPS)
{
/* use the length to decide which primes to check */
switch (dhPrime->len) {
@@ -1182,56 +1191,67 @@ sftk_VerifyDH_Prime(SECItem *dhPrime, PR
}
if (PORT_Memcmp(dhPrime->data, prime_ike_1536,
sizeof(prime_ike_1536)) == 0) {
+ if (g) *g = generator_2;
return &subprime_ike_1536;
}
break;
case 2048 / PR_BITS_PER_BYTE:
if (PORT_Memcmp(dhPrime->data, prime_tls_2048,
sizeof(prime_tls_2048)) == 0) {
+ if (g) *g = generator_2;
return &subprime_tls_2048;
}
if (PORT_Memcmp(dhPrime->data, prime_ike_2048,
sizeof(prime_ike_2048)) == 0) {
+ if (g) *g = generator_2;
return &subprime_ike_2048;
}
break;
case 3072 / PR_BITS_PER_BYTE:
if (PORT_Memcmp(dhPrime->data, prime_tls_3072,
sizeof(prime_tls_3072)) == 0) {
+ if (g) *g = generator_2;
return &subprime_tls_3072;
}
if (PORT_Memcmp(dhPrime->data, prime_ike_3072,
sizeof(prime_ike_3072)) == 0) {
+ if (g) *g = generator_2;
return &subprime_ike_3072;
}
break;
case 4096 / PR_BITS_PER_BYTE:
if (PORT_Memcmp(dhPrime->data, prime_tls_4096,
sizeof(prime_tls_4096)) == 0) {
+ if (g) *g = generator_2;
return &subprime_tls_4096;
}
if (PORT_Memcmp(dhPrime->data, prime_ike_4096,
sizeof(prime_ike_4096)) == 0) {
+ if (g) *g = generator_2;
return &subprime_ike_4096;
}
break;
case 6144 / PR_BITS_PER_BYTE:
if (PORT_Memcmp(dhPrime->data, prime_tls_6144,
sizeof(prime_tls_6144)) == 0) {
+ if (g) *g = generator_2;
return &subprime_tls_6144;
}
if (PORT_Memcmp(dhPrime->data, prime_ike_6144,
sizeof(prime_ike_6144)) == 0) {
+ if (g) *g = generator_2;
return &subprime_ike_6144;
}
break;
case 8192 / PR_BITS_PER_BYTE:
if (PORT_Memcmp(dhPrime->data, prime_tls_8192,
sizeof(prime_tls_8192)) == 0) {
+ if (g) *g = generator_2;
return &subprime_tls_8192;
}
if (PORT_Memcmp(dhPrime->data, prime_ike_8192,
sizeof(prime_ike_8192)) == 0) {
+ if (g) *g = generator_2;
return &subprime_ike_8192;
}
break;
diff -up ./lib/softoken/sftkike.c.fips-review ./lib/softoken/sftkike.c
--- ./lib/softoken/sftkike.c.fips-review 2022-05-26 02:54:33.000000000 -0700
+++ ./lib/softoken/sftkike.c 2023-03-16 11:54:37.840935312 -0700
+++ ./lib/softoken/sftkike.c 2023-03-17 10:53:23.553658599 -0700
@@ -516,6 +516,11 @@ sftk_ike_prf(CK_SESSION_HANDLE hSession,
goto fail;
}

5
nss.spec
View File

@ -1,6 +1,6 @@
%global nss_version 3.79.0
%global nspr_version 4.34.0
%global baserelease 17
%global baserelease 18
%global nss_release %baserelease
# NOTE: To avoid NVR clashes of nspr* packages:
# use "%%global nspr_release %%[%%baserelease+n]" to handle offsets when
@ -1171,6 +1171,9 @@ update-crypto-policies &> /dev/null || :
%changelog
* Fri Mar 17 2023 Bob Relyea <rrelyea@redhat.com> - 3.79.0-18
- fix memory leak, add generator test in FIPS mode.
* Thu Mar 16 2023 Bob Relyea <rrelyea@redhat.com> - 3.79.0-17
- fix consistency return errors. We shouldn't lock the FIPS
token if the application asked for invalid DH parameters on