From 1e560deff8c37164eb7496e7d87aa7b30de80398 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Wed, 17 Jul 2019 01:31:34 +0200 Subject: Don't unnecessarily strip leading 0's from key material during PKCS11 import. --- security/nss/lib/freebl/ecl/ecp_25519.c | 3 +++ security/nss/lib/pk11wrap/pk11akey.c | 20 +++++++++++++------- security/nss/lib/pk11wrap/pk11cert.c | 4 +++- security/nss/lib/pk11wrap/pk11pk12.c | 13 +++++++------ security/nss/lib/softoken/legacydb/lgattr.c | 6 +++--- security/nss/lib/softoken/pkcs11c.c | 2 +- 6 files changed, 30 insertions(+), 18 deletions(-) diff --git a/security/nss/lib/freebl/ecl/ecp_25519.c b/security/nss/lib/freebl/ecl/ecp_25519.c index 38bd34c501..7b331b28f5 100644 --- a/security/nss/lib/freebl/ecl/ecp_25519.c +++ b/security/nss/lib/freebl/ecl/ecp_25519.c @@ -114,6 +114,9 @@ ec_Curve25519_pt_mul(SECItem *X, SECItem *k, SECItem *P) } 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/security/nss/lib/pk11wrap/pk11akey.c b/security/nss/lib/pk11wrap/pk11akey.c index c6070e264d..9a914272d5 100644 --- a/security/nss/lib/pk11wrap/pk11akey.c +++ b/security/nss/lib/pk11wrap/pk11akey.c @@ -190,7 +190,6 @@ PK11_ImportPublicKey(PK11SlotInfo *slot, SECKEYPublicKey *pubKey, 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); @@ -224,10 +223,13 @@ PK11_ImportPublicKey(PK11SlotInfo *slot, SECKEYPublicKey *pubKey, } 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 +1076,13 @@ pk11_loadPrivKeyWithFlags(PK11SlotInfo *slot, SECKEYPrivateKey *privKey, &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/security/nss/lib/pk11wrap/pk11cert.c b/security/nss/lib/pk11wrap/pk11cert.c index 8197696431..a880b1ab8a 100644 --- a/security/nss/lib/pk11wrap/pk11cert.c +++ b/security/nss/lib/pk11wrap/pk11cert.c @@ -184,7 +184,9 @@ PK11_IsUserCert(PK11SlotInfo *slot, CERTCertificate *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/security/nss/lib/pk11wrap/pk11pk12.c b/security/nss/lib/pk11wrap/pk11pk12.c index 47b6702c6d..9888ace696 100644 --- a/security/nss/lib/pk11wrap/pk11pk12.c +++ b/security/nss/lib/pk11wrap/pk11pk12.c @@ -505,7 +505,7 @@ PK11_ImportAndReturnPrivateKey(PK11SlotInfo *slot, SECKEYRawPrivateKey *lpk, } 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(PK11SlotInfo *slot, SECKEYRawPrivateKey *lpk, } 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/security/nss/lib/softoken/legacydb/lgattr.c b/security/nss/lib/softoken/legacydb/lgattr.c index 542b0c968c..3d77bd0562 100644 --- a/security/nss/lib/softoken/legacydb/lgattr.c +++ b/security/nss/lib/softoken/legacydb/lgattr.c @@ -950,9 +950,9 @@ lg_FindECPrivateKeyAttribute(NSSLOWKEYPrivateKey *key, CK_ATTRIBUTE_TYPE type, 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/security/nss/lib/softoken/pkcs11c.c b/security/nss/lib/softoken/pkcs11c.c index 7eec3d7ee8..884702cc16 100644 --- a/security/nss/lib/softoken/pkcs11c.c +++ b/security/nss/lib/softoken/pkcs11c.c @@ -7568,7 +7568,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); -- cgit v1.2.3