# HG changeset patch # User Kevin Jacobs # Date 1560499074 0 # Fri Jun 14 07:57:54 2019 +0000 # Branch NSS_3_44_BRANCH # Node ID 876bca2723a1f969422edc93e7504420d8331d3c # Parent 20671f860c2619dc27208d64a84a759fdffc5ed1 Bug 1540541 - Don't unnecessarily strip leading 0's from key material during PKCS11 import. r=jcj,mt Differential Revision: https://phabricator.services.mozilla.com/D31671 diff --git a/lib/freebl/ecl/ecp_25519.c b/lib/freebl/ecl/ecp_25519.c --- a/lib/freebl/ecl/ecp_25519.c +++ b/lib/freebl/ecl/ecp_25519.c @@ -114,6 +114,9 @@ ec_Curve25519_pt_mul(SECItem *X, SECItem } px = P->data; } + if (k->len != 32) { + return SECFailure; + } SECStatus rv = ec_Curve25519_mul(X->data, k->data, px); if (NSS_SecureMemcmpZero(X->data, X->len) == 0) { diff --git a/lib/pk11wrap/pk11akey.c b/lib/pk11wrap/pk11akey.c --- a/lib/pk11wrap/pk11akey.c +++ b/lib/pk11wrap/pk11akey.c @@ -190,7 +190,6 @@ PK11_ImportPublicKey(PK11SlotInfo *slot, attrs++; PK11_SETATTRS(attrs, CKA_DERIVE, &cktrue, sizeof(CK_BBOOL)); attrs++; - signedattr = attrs; PK11_SETATTRS(attrs, CKA_EC_PARAMS, pubKey->u.ec.DEREncodedParams.data, pubKey->u.ec.DEREncodedParams.len); @@ -222,12 +221,14 @@ PK11_ImportPublicKey(PK11SlotInfo *slot, PORT_SetError(SEC_ERROR_BAD_KEY); return CK_INVALID_HANDLE; } - templateCount = attrs - theTemplate; - signedcount = attrs - signedattr; PORT_Assert(templateCount <= (sizeof(theTemplate) / sizeof(CK_ATTRIBUTE))); - for (attrs = signedattr; signedcount; attrs++, signedcount--) { - pk11_SignedToUnsigned(attrs); + if (pubKey->keyType != ecKey) { + PORT_Assert(signedattr); + signedcount = attrs - signedattr; + for (attrs = signedattr; signedcount; attrs++, signedcount--) { + pk11_SignedToUnsigned(attrs); + } } rv = PK11_CreateNewObject(slot, CK_INVALID_SESSION, theTemplate, templateCount, isToken, &objectID); @@ -1074,9 +1075,13 @@ pk11_loadPrivKeyWithFlags(PK11SlotInfo * &cktrue, &ckfalse); /* Not everyone can handle zero padded key values, give - * them the raw data as unsigned */ - for (ap = attrs; extra_count; ap++, extra_count--) { - pk11_SignedToUnsigned(ap); + * them the raw data as unsigned. The exception is EC, + * where the values are encoded or zero-preserving + * per-RFC5915 */ + if (privKey->keyType != ecKey) { + for (ap = attrs; extra_count; ap++, extra_count--) { + pk11_SignedToUnsigned(ap); + } } /* now Store the puppies */ diff --git a/lib/pk11wrap/pk11cert.c b/lib/pk11wrap/pk11cert.c --- a/lib/pk11wrap/pk11cert.c +++ b/lib/pk11wrap/pk11cert.c @@ -184,7 +184,9 @@ PK11_IsUserCert(PK11SlotInfo *slot, CERT SECKEY_DestroyPublicKey(pubKey); return PR_FALSE; } - pk11_SignedToUnsigned(&theTemplate); + if (pubKey->keyType != ecKey) { + pk11_SignedToUnsigned(&theTemplate); + } if (pk11_FindObjectByTemplate(slot, &theTemplate, 1) != CK_INVALID_HANDLE) { SECKEY_DestroyPublicKey(pubKey); return PR_TRUE; diff --git a/lib/pk11wrap/pk11pk12.c b/lib/pk11wrap/pk11pk12.c --- a/lib/pk11wrap/pk11pk12.c +++ b/lib/pk11wrap/pk11pk12.c @@ -505,7 +505,7 @@ PK11_ImportAndReturnPrivateKey(PK11SlotI } PK11_SETATTRS(attrs, CKA_ID, ck_id->data, ck_id->len); attrs++; - signedattr = attrs; + /* No signed attrs for EC */ /* curveOID always is a copy of AlgorithmID.parameters. */ PK11_SETATTRS(attrs, CKA_EC_PARAMS, lpk->u.ec.curveOID.data, lpk->u.ec.curveOID.len); @@ -523,11 +523,12 @@ PK11_ImportAndReturnPrivateKey(PK11SlotI } templateCount = attrs - theTemplate; PORT_Assert(templateCount <= sizeof(theTemplate) / sizeof(CK_ATTRIBUTE)); - PORT_Assert(signedattr != NULL); - signedcount = attrs - signedattr; - - for (ap = signedattr; signedcount; ap++, signedcount--) { - pk11_SignedToUnsigned(ap); + if (lpk->keyType != ecKey) { + PORT_Assert(signedattr); + signedcount = attrs - signedattr; + for (ap = signedattr; signedcount; ap++, signedcount--) { + pk11_SignedToUnsigned(ap); + } } rv = PK11_CreateNewObject(slot, CK_INVALID_SESSION, diff --git a/lib/softoken/legacydb/lgattr.c b/lib/softoken/legacydb/lgattr.c --- a/lib/softoken/legacydb/lgattr.c +++ b/lib/softoken/legacydb/lgattr.c @@ -950,9 +950,9 @@ lg_FindECPrivateKeyAttribute(NSSLOWKEYPr case CKA_UNWRAP: return LG_CLONE_ATTR(attribute, type, lg_StaticFalseAttr); case CKA_VALUE: - return lg_CopyPrivAttrSigned(attribute, type, - key->u.ec.privateValue.data, - key->u.ec.privateValue.len, sdbpw); + return lg_CopyPrivAttribute(attribute, type, + key->u.ec.privateValue.data, + key->u.ec.privateValue.len, sdbpw); case CKA_EC_PARAMS: return lg_CopyAttributeSigned(attribute, type, key->u.ec.ecParams.DEREncoding.data, diff --git a/lib/softoken/pkcs11c.c b/lib/softoken/pkcs11c.c --- a/lib/softoken/pkcs11c.c +++ b/lib/softoken/pkcs11c.c @@ -7747,7 +7747,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession rv = ECDH_Derive(&ecPoint, &privKey->u.ec.ecParams, &ecScalar, withCofactor, &tmp); - PORT_Free(ecScalar.data); + PORT_ZFree(ecScalar.data, ecScalar.len); ecScalar.data = NULL; if (privKey != sourceKey->objectInfo) { nsslowkey_DestroyPrivateKey(privKey);