Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1697303 NSS needs to update it's csp clearing to FIPS 180-3 stand…
…ards.

FIPS 180-3 updated the standard for clearing sensitive key material in FIPS modules. I've done a complete review of the portions of NSS affected by the FIPS requirements and identified all the areas where we need to update. The report is available here: https://docs.google.com/document/d/1v9kedUiwVYYIUagyT_vQdtrktjGUrA3SFsVP-LA6vOw/edit?usp=sharing

This patch does the following:
    - Clears the stack in gcm and ecc to deal with large stack leakages.
This only happens in FIPS enabled case. The size of the stack is based on the
size of the leakage, with some extra to make sure we reach down into that area.
Most of the leakage happens in either auto generated code or machine dependent
acceleration code.
    - Clears hash related data that wasn't cleared previously
    - Clears public key exponents that wasn't cleared previously.
    - Clears components that should have been cleared previously but wasn't.

Usually clearing takes one of the following forms:
    PORT_Free(x) -> PORT_Free(x, size). This means we need to know what
the size is supposed to be. In some cases we need to add code to preserve
the size.
    PORT_Free(x.data) -> SECITEM_ZfreeItem(&x, PR_FALSE). In this case x is
a SECITEM, which carries the length. PR_FALSE means clear and free the data in
the item, not the item itself. The code should have had SECITEM_FreeItem before
anyway.
    SECIEM_FreeItem(item, bool) -> SECITEM_ZfreeItem(item, bool). Simply change
the normal SECITEM free call to the one that clears the item.
    PR_ArenaFree(arena, PR_FALSE) -> PR_ArenaFree(arena, PR_TRUE). The bool here
means whether or not to clear as well as free the data in the arena.
    PORT_Memset(value, 0, size). This the obvious clear operation. It happens
if the variable is a stack variable, or if the memory isn't cleared with one
of the three clearing functions above.

In addition this patch fixes the following:
    - moves the determination if whether or not a slot is in FIPS mode by
slotID to a macro. NSS allows user defined slots to be opened. If you open a
user defined slot using the FIPS slot, the resulting slots will also be FIPS
slots. In areas where the semantics change based on the slot, these slots should
have the FIPS semantics. Directly checking if the slot is the FIPS slot now
only happens when we really mean the main FIPS slot and not just any FIPS slot.
    - In handling the clearing of PSS and OAEP, I identified an issue. These
functions where holding a pointer to the pMechanismParams in their C_XXXXInit
calls for later use in the C_XXXXUpdate/C_XXXXFinal/C_XXXX calls. The problem
is applications are allowed to free their pMechanismParams once C_XXXXInit is
complete. We need to make a copy of the params to use them.

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

--HG--
extra : rebase_source : 7ac6f9d40bffbaf1a45e9e0996893afe618d4fc5
  • Loading branch information
rjrelyea committed Mar 11, 2021
1 parent f945d56 commit 888f7ca
Show file tree
Hide file tree
Showing 28 changed files with 466 additions and 356 deletions.
10 changes: 10 additions & 0 deletions lib/freebl/blapii.h
Expand Up @@ -98,4 +98,14 @@ PRBool arm_sha1_support();
PRBool arm_sha2_support();
PRBool ppc_crypto_support();

#ifdef NSS_FIPS_DISABLED
#define BLAPI_CLEAR_STACK(stack_size)
#else
#define BLAPI_CLEAR_STACK(stack_size) \
{ \
volatile char _stkclr[stack_size]; \
PORT_Memset((void *)&_stkclr[0], 0, stack_size); \
}
#endif

#endif /* _BLAPII_H_ */
6 changes: 6 additions & 0 deletions lib/freebl/drbg.c
Expand Up @@ -145,6 +145,7 @@ prng_Hash_df(PRUint8 *requested_bytes, unsigned int no_of_bytes_to_return,
requested_bytes += hash_return_len;
no_of_bytes_to_return -= hash_return_len;
}
SHA256_DestroyContext(&ctx, PR_FALSE);
return SECSuccess;
}

Expand Down Expand Up @@ -197,6 +198,7 @@ prng_initEntropy(void)
SHA256_End(&ctx, globalrng->previousEntropyHash, NULL,
sizeof(globalrng->previousEntropyHash));
PORT_Memset(block, 0, sizeof(block));
SHA256_DestroyContext(&ctx, PR_FALSE);
return PR_SUCCESS;
}

Expand Down Expand Up @@ -244,6 +246,7 @@ prng_getEntropy(PRUint8 *buffer, size_t requestLength)
}

out:
PORT_Memset(hash, 0, sizeof hash);
PORT_Memset(block, 0, sizeof block);
return rv;
}
Expand Down Expand Up @@ -388,6 +391,7 @@ prng_Hashgen(RNGContext *rng, PRUint8 *returned_bytes,
* This increments data if no_of_returned_bytes is not zero */
carry = no_of_returned_bytes;
PRNG_ADD_CARRY_ONLY(data, (sizeof data) - 1, carry);
SHA256_DestroyContext(&ctx, PR_FALSE);
}
PORT_Memset(data, 0, sizeof data);
PORT_Memset(thisHash, 0, sizeof thisHash);
Expand Down Expand Up @@ -430,6 +434,7 @@ prng_generateNewBytes(RNGContext *rng,
SHA256_End(&ctx, w, NULL, sizeof w);
PRNG_ADD_BITS_AND_CARRY(V(rng), VSize(rng), w, sizeof w, carry)
PORT_Memset(w, 0, sizeof w);
SHA256_DestroyContext(&ctx, PR_FALSE);
#undef w
}

Expand All @@ -450,6 +455,7 @@ prng_generateNewBytes(RNGContext *rng,
PRNG_ADD_CARRY_ONLY(rng->reseed_counter, (sizeof rng->reseed_counter) - 1, carry);

/* if the prng failed, don't return any output, signal softoken */
PORT_Memset(H, 0, sizeof H);
if (!rng->isValid) {
PORT_Memset(returned_bytes, 0, no_of_returned_bytes);
PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
Expand Down
12 changes: 8 additions & 4 deletions lib/freebl/dsa.c
Expand Up @@ -260,7 +260,7 @@ DSA_NewRandom(PLArenaPool *arena, const SECItem *q, SECItem *seed)
PORT_SetError(SEC_ERROR_NEED_RANDOM);
loser:
if (arena != NULL) {
SECITEM_FreeItem(seed, PR_FALSE);
SECITEM_ZfreeItem(seed, PR_FALSE);
}
return SECFailure;
}
Expand Down Expand Up @@ -295,7 +295,7 @@ DSA_NewKey(const PQGParams *params, DSAPrivateKey **privKey)
rv = dsa_NewKeyExtended(params, &seed, privKey);
}
}
SECITEM_FreeItem(&seed, PR_FALSE);
SECITEM_ZfreeItem(&seed, PR_FALSE);
return rv;
}

Expand Down Expand Up @@ -403,6 +403,8 @@ dsa_SignDigest(DSAPrivateKey *key, SECItem *signature, const SECItem *digest,
CHECK_MPI_OK(mp_exptmod(&g, &t, &p, &r)); /* r = g**t mod p */
/* r is now g**(k+q*fuzz) == g**k mod p */
CHECK_MPI_OK(mp_mod(&r, &q, &r)); /* r = r mod q */
/* make sure fuzz is cleared off the stack and not optimized away */
*(volatile mp_digit *)&fuzz = 0;

/*
** FIPS 186-1, Section 5, Step 2
Expand All @@ -415,14 +417,14 @@ dsa_SignDigest(DSAPrivateKey *key, SECItem *signature, const SECItem *digest,
goto cleanup;
}
SECITEM_TO_MPINT(t2, &t); /* t <-$ Zq */
SECITEM_FreeItem(&t2, PR_FALSE);
SECITEM_ZfreeItem(&t2, PR_FALSE);
if (DSA_NewRandom(NULL, &key->params.subPrime, &t2) != SECSuccess) {
PORT_SetError(SEC_ERROR_NEED_RANDOM);
rv = SECFailure;
goto cleanup;
}
SECITEM_TO_MPINT(t2, &ar); /* ar <-$ Zq */
SECITEM_FreeItem(&t2, PR_FALSE);
SECITEM_ZfreeItem(&t2, PR_FALSE);

/* Using mp_invmod on k directly would leak bits from k. */
CHECK_MPI_OK(mp_mul(&k, &ar, &k)); /* k = k * ar */
Expand Down Expand Up @@ -530,6 +532,7 @@ DSA_SignDigest(DSAPrivateKey *key, SECItem *signature, const SECItem *digest)
rv = dsa_SignDigest(key, signature, digest, kSeed);
} while (rv != SECSuccess && PORT_GetError() == SEC_ERROR_NEED_RANDOM &&
--retries > 0);
PORT_Memset(kSeed, 0, sizeof kSeed);
return rv;
}

Expand Down Expand Up @@ -670,6 +673,7 @@ DSA_VerifyDigest(DSAPublicKey *key, const SECItem *signature,
verified = SECSuccess; /* Signature verified. */
}
cleanup:
PORT_Memset(localDigestData, 0, sizeof localDigestData);
mp_clear(&p);
mp_clear(&q);
mp_clear(&g);
Expand Down
10 changes: 8 additions & 2 deletions lib/freebl/ec.c
Expand Up @@ -7,6 +7,7 @@
#endif

#include "blapi.h"
#include "blapii.h"
#include "prerr.h"
#include "secerr.h"
#include "secmpi.h"
Expand Down Expand Up @@ -146,6 +147,10 @@ ec_points_mul(const ECParams *params, const mp_int *k1, const mp_int *k2,
CHECK_MPI_OK(ECPoints_mul(group, k1, NULL, NULL, NULL, &Qx, &Qy));
}

/* our ECC codes uses large stack variables to store intermediate results,
* clear our stack before returning to prevent CSP leakage */
BLAPI_CLEAR_STACK(2048)

/* Construct the SECItem representation of point Q */
pointQ->data[0] = EC_POINT_FORM_UNCOMPRESSED;
CHECK_MPI_OK(mp_to_fixlen_octets(&Qx, pointQ->data + 1,
Expand Down Expand Up @@ -531,7 +536,6 @@ ECDH_Derive(SECItem *publicValue,
unsigned int len = 0;
SECItem pointQ = { siBuffer, NULL, 0 };
mp_int k; /* to hold the private value */
mp_int cofactor;
mp_err err = MP_OKAY;
#if EC_DEBUG
int i;
Expand Down Expand Up @@ -596,11 +600,13 @@ ECDH_Derive(SECItem *publicValue,
(mp_size)privateValue->len));

if (withCofactor && (ecParams->cofactor != 1)) {
mp_int cofactor;
/* multiply k with the cofactor */
MP_DIGITS(&cofactor) = 0;
CHECK_MPI_OK(mp_init(&cofactor));
mp_set(&cofactor, ecParams->cofactor);
CHECK_MPI_OK(mp_mul(&k, &cofactor, &k));
mp_clear(&cofactor);
}

/* Multiply our private key and peer's public point */
Expand Down Expand Up @@ -858,7 +864,7 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
mp_clear(&ar);

if (t2) {
PORT_Free(t2);
PORT_ZFree(t2, 2 * ecParams->order.len);
}

if (kGpoint.data) {
Expand Down
5 changes: 4 additions & 1 deletion lib/freebl/ecl/ecp_jm.c
Expand Up @@ -192,7 +192,7 @@ ec_GFp_pt_mul_jm_wNAF(const mp_int *n, const mp_int *px, const mp_int *py,
mp_int raz4;
mp_int scratch[MAX_SCRATCH];
signed char *naf = NULL;
int i, orderBitSize;
int i, orderBitSize = 0;

MP_DIGITS(&rz) = 0;
MP_DIGITS(&raz4) = 0;
Expand Down Expand Up @@ -289,6 +289,9 @@ ec_GFp_pt_mul_jm_wNAF(const mp_int *n, const mp_int *px, const mp_int *py,
mp_clear(&tpy);
mp_clear(&rz);
mp_clear(&raz4);
if (naf) {
memset(naf, 0, orderBitSize + 1);
}
free(naf);
return res;
}
16 changes: 12 additions & 4 deletions lib/freebl/gcm.c
Expand Up @@ -593,15 +593,19 @@ GCM_CreateContext(void *context, freeblCipherFunc cipher,
if (rv != SECSuccess) {
goto loser;
}
PORT_Memset(H, 0, AES_BLOCK_SIZE);
gcm->ctr_context_init = PR_TRUE;
return gcm;

loser:
PORT_Memset(H, 0, AES_BLOCK_SIZE);
if (ghash && ghash->mem) {
PORT_Free(ghash->mem);
void *mem = ghash->mem;
PORT_Memset(ghash, 0, sizeof(gcmHashContext));
PORT_Free(mem);
}
if (gcm) {
PORT_Free(gcm);
PORT_ZFree(gcm, sizeof(GCMContext));
}
return NULL;
}
Expand Down Expand Up @@ -675,9 +679,11 @@ gcm_InitCounter(GCMContext *gcm, const unsigned char *iv, unsigned int ivLen,
goto loser;
}

PORT_Memset(&ctrParams, 0, sizeof ctrParams);
return SECSuccess;

loser:
PORT_Memset(&ctrParams, 0, sizeof ctrParams);
if (freeCtr) {
CTR_DestroyContext(&gcm->ctr_context, PR_FALSE);
}
Expand All @@ -687,13 +693,15 @@ gcm_InitCounter(GCMContext *gcm, const unsigned char *iv, unsigned int ivLen,
void
GCM_DestroyContext(GCMContext *gcm, PRBool freeit)
{
/* these two are statically allocated and will be freed when we free
void *mem = gcm->ghash_context->mem;
/* ctr_context is statically allocated and will be freed when we free
* gcm. call their destroy functions to free up any locally
* allocated data (like mp_int's) */
if (gcm->ctr_context_init) {
CTR_DestroyContext(&gcm->ctr_context, PR_FALSE);
}
PORT_Free(gcm->ghash_context->mem);
PORT_Memset(gcm->ghash_context, 0, sizeof(gcmHashContext));
PORT_Free(mem);
PORT_Memset(&gcm->tagBits, 0, sizeof(gcm->tagBits));
PORT_Memset(gcm->tagKey, 0, sizeof(gcm->tagKey));
if (freeit) {
Expand Down
5 changes: 5 additions & 0 deletions lib/freebl/hmacct.c
Expand Up @@ -274,6 +274,11 @@ MAC(unsigned char *mdOut,
hashObj->end(mdState, mdOut, mdOutLen, mdOutMax);
hashObj->destroy(mdState, PR_TRUE);

PORT_Memset(lengthBytes, 0, sizeof lengthBytes);
PORT_Memset(hmacPad, 0, sizeof hmacPad);
PORT_Memset(firstBlock, 0, sizeof firstBlock);
PORT_Memset(macOut, 0, sizeof macOut);

return SECSuccess;
}

Expand Down
6 changes: 5 additions & 1 deletion lib/freebl/mpi/mpmontg.c
Expand Up @@ -1006,7 +1006,11 @@ mp_exptmod_safe_i(const mp_int *montBase,
mp_clear(&accum[2]);
mp_clear(&accum[3]);
mp_clear(&tmp);
/* PORT_Memset(powers,0,num_powers*nLen*sizeof(mp_digit)); */
/* zero required by FIPS here, can't use PORT_ZFree
* because mpi doesn't link with util */
if (powers) {
PORT_Memset(powers, 0, num_powers * sizeof(mp_digit));
}
free(powersArray);
return res;
}
Expand Down

0 comments on commit 888f7ca

Please sign in to comment.