Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 380351 - always validate public ec keys, r=ttaubert,mt
Differential Revision: https://nss-review.dev.mozaws.net/D88

--HG--
extra : rebase_source : 7910c15e5c259078141f1bf900c5cfb36810eca3
extra : amend_source : fe7541ef97266071d9aa233582f4791043874f4e
  • Loading branch information
franziskuskiefer committed Jan 18, 2017
1 parent 393719c commit c1457ef
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
20 changes: 15 additions & 5 deletions lib/freebl/ec.c
Expand Up @@ -566,6 +566,15 @@ ECDH_Derive(SECItem *publicValue,
return SECFailure;
}

/*
* Make sure the point is on the requested curve to avoid
* certain small subgroup attacks.
*/
if (EC_ValidatePublicKey(ecParams, publicValue) != SECSuccess) {
PORT_SetError(SEC_ERROR_BAD_KEY);
return SECFailure;
}

/* Perform curve specific multiplication using ECMethod */
if (ecParams->fieldID.type == ec_field_plain) {
const ECMethod *method;
Expand All @@ -581,10 +590,6 @@ ECDH_Derive(SECItem *publicValue,
PORT_SetError(SEC_ERROR_UNSUPPORTED_ELLIPTIC_CURVE);
return SECFailure;
}
if (method->validate(publicValue) != SECSuccess) {
PORT_SetError(SEC_ERROR_BAD_KEY);
return SECFailure;
}
return method->mul(derivedSecret, privateValue, publicValue);
}

Expand Down Expand Up @@ -1002,9 +1007,14 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature,
}
slen = signature->len / 2;

/*
* The incoming point has been verified in sftk_handlePublicKeyObject.
*/

SECITEM_AllocItem(NULL, &pointC, ecParams->pointSize);
if (pointC.data == NULL)
if (pointC.data == NULL) {
goto cleanup;
}

CHECK_MPI_OK(mp_init(&r_));
CHECK_MPI_OK(mp_init(&s_));
Expand Down
8 changes: 0 additions & 8 deletions lib/softoken/pkcs11c.c
Expand Up @@ -7267,14 +7267,6 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,

if (mechanism == CKM_ECDH1_COFACTOR_DERIVE) {
withCofactor = PR_TRUE;
} else {
/* When not using cofactor derivation, one should
* validate the public key to avoid small subgroup
* attacks.
*/
if (EC_ValidatePublicKey(&privKey->u.ec.ecParams, &ecPoint) != SECSuccess) {
goto ec_loser;
}
}

rv = ECDH_Derive(&ecPoint, &privKey->u.ec.ecParams, &ecScalar,
Expand Down

0 comments on commit c1457ef

Please sign in to comment.