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
--- a/lib/freebl/ecl/ecp_25519.c
+++ b/lib/freebl/ecl/ecp_25519.c
@@ -109,15 +109,18 @@ ec_Curve25519_pt_mul(SECItem *X, SECItem
px = basePoint;
} else {
PORT_Assert(P->len == 32);
if (P->len != 32) {
return SECFailure;
}
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) {
return SECFailure;
}
return rv;
}
--- a/lib/pk11wrap/pk11akey.c
+++ b/lib/pk11wrap/pk11akey.c
@@ -185,17 +185,16 @@ PK11_ImportPublicKey(PK11SlotInfo *slot,
attrs++;
break;
case ecKey:
keyType = CKK_EC;
PK11_SETATTRS(attrs, CKA_VERIFY, &cktrue, sizeof(CK_BBOOL));
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);
attrs++;
if (PR_GetEnvSecure("NSS_USE_DECODED_CKA_EC_POINT")) {
PK11_SETATTRS(attrs, CKA_EC_POINT,
pubKey->u.ec.publicValue.data,
pubKey->u.ec.publicValue.len);
@@ -217,22 +216,24 @@ PK11_ImportPublicKey(PK11SlotInfo *slot,
break;
default:
if (ckaId) {
SECITEM_FreeItem(ckaId, PR_TRUE);
}
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);
if (ckaId) {
SECITEM_FreeItem(ckaId, PR_TRUE);
}
if (pubValue) {
SECITEM_FreeItem(pubValue, PR_TRUE);
@@ -1069,19 +1070,23 @@ pk11_loadPrivKeyWithFlags(PK11SlotInfo *
return NULL;
}
/* Set token, private, modifiable, sensitive, and extractable */
count += pk11_AttrFlagsToAttributes(attrFlags, &privTemplate[count],
&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 */
rv = PK11_CreateNewObject(slot, CK_INVALID_SESSION, privTemplate,
count, token, &objectID);
PORT_FreeArena(arena, PR_TRUE);
if (rv != SECSuccess) {
return NULL;
--- a/lib/pk11wrap/pk11cert.c
+++ b/lib/pk11wrap/pk11cert.c
@@ -179,17 +179,19 @@ PK11_IsUserCert(PK11SlotInfo *slot, CERT
/* fall through and return false */
break;
}
if (theTemplate.ulValueLen == 0) {
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;
}
SECKEY_DestroyPublicKey(pubKey);
}
return PR_FALSE;
}
--- a/lib/pk11wrap/pk11pk12.c
+++ b/lib/pk11wrap/pk11pk12.c
@@ -500,17 +500,17 @@ PK11_ImportAndReturnPrivateKey(PK11SlotI
sizeof(CK_BBOOL));
attrs++;
ck_id = PK11_MakeIDFromPubKey(&lpk->u.ec.publicValue);
if (ck_id == NULL) {
goto loser;
}
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);
attrs++;
PK11_SETATTRS(attrs, CKA_VALUE, lpk->u.ec.privateValue.data,
lpk->u.ec.privateValue.len);
attrs++;
PK11_SETATTRS(attrs, CKA_EC_POINT, lpk->u.ec.publicValue.data,
@@ -518,21 +518,22 @@ PK11_ImportAndReturnPrivateKey(PK11SlotI
attrs++;
break;
default:
PORT_SetError(SEC_ERROR_BAD_KEY);
goto loser;
}
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,
theTemplate, templateCount, isPerm, &objectID);
/* create and return a SECKEYPrivateKey */
if (rv == SECSuccess && privk != NULL) {
*privk = PK11_MakePrivKey(slot, lpk->keyType, !isPerm, objectID, wincx);
--- a/lib/softoken/legacydb/lgattr.c
+++ b/lib/softoken/legacydb/lgattr.c
@@ -945,19 +945,19 @@ lg_FindECPrivateKeyAttribute(NSSLOWKEYPr
case CKA_DERIVE:
case CKA_SIGN:
return LG_CLONE_ATTR(attribute, type, lg_StaticTrueAttr);
case CKA_DECRYPT:
case CKA_SIGN_RECOVER:
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,
key->u.ec.ecParams.DEREncoding.len);
case CKA_NETSCAPE_DB:
return lg_CopyAttributeSigned(attribute, type,
key->u.ec.publicValue.data,
key->u.ec.publicValue.len);
--- a/lib/softoken/pkcs11c.c
+++ b/lib/softoken/pkcs11c.c
@@ -7756,17 +7756,17 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession
}
if (mechanism == CKM_ECDH1_COFACTOR_DERIVE) {
withCofactor = PR_TRUE;
}
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);
privKey = NULL;
}
if (arena) {
PORT_FreeArena(arena, PR_FALSE);
arena = NULL;