Bug 1334108, ECParams shouldn't have been modified, revert the change…

…, r=franziskus
sailfishos-mirror · Feb 15, 2017 · 2f1d3df · 2f1d3df
1 parent ef28d1c
commit 2f1d3df
Show file tree

Hide file tree

Showing 13 changed files with 51 additions and 23 deletions.
diff --git a/cmd/bltest/blapitest.c b/cmd/bltest/blapitest.c
@@ -1870,7 +1870,6 @@ bltest_ecdsa_init(bltestCipherInfo *cipherInfo, PRBool encrypt)
         pubkey->ecParams.DEREncoding.len = key->ecParams.DEREncoding.len;
         pubkey->ecParams.DEREncoding.data = key->ecParams.DEREncoding.data;
         pubkey->ecParams.name = key->ecParams.name;
-        pubkey->ecParams.pointSize = key->ecParams.pointSize;
         pubkey->publicValue.len = key->publicValue.len;
         pubkey->publicValue.data = key->publicValue.data;
         asymk->pubKey = pubkey;

diff --git a/cmd/ecperf/ecperf.c b/cmd/ecperf/ecperf.c
@@ -564,7 +564,6 @@ ectest_curve_freebl(ECCurveName curve, int iterations, int numThreads,
     ecParams.curve.seed.len = 0;
     ecParams.DEREncoding.data = NULL;
     ecParams.DEREncoding.len = 0;
-    ecParams.pointSize = ecCurve_map[curve]->pointSize;
 
     ecParams.fieldID.size = ecCurve_map[curve]->size;
     ecParams.fieldID.type = fieldType;

diff --git a/cmd/fbectest/fbectest.c b/cmd/fbectest/fbectest.c
@@ -71,7 +71,6 @@ init_params(ECParams *ecParams, ECCurveName curve, PLArenaPool **arena,
     ecParams->fieldID.size = ecCurve_map[curve]->size;
     ecParams->fieldID.type = type;
     ecParams->cofactor = ecCurve_map[curve]->cofactor;
-    ecParams->pointSize = ecCurve_map[curve]->pointSize;
 
     return SECSuccess;
 }

diff --git a/cmd/fipstest/fipstest.c b/cmd/fipstest/fipstest.c
@@ -2523,7 +2523,7 @@ ecdsa_pkv_test(char *reqfn)
                 PORT_Free(pubkey.data);
                 pubkey.data = NULL;
             }
-            SECITEM_AllocItem(NULL, &pubkey, ecparams->pointSize);
+            SECITEM_AllocItem(NULL, &pubkey, EC_GetPointSize(ecparams));
             if (pubkey.data == NULL) {
                 goto loser;
             }

diff --git a/lib/freebl/blapi.h b/lib/freebl/blapi.h
@@ -1599,7 +1599,6 @@ extern const SECHashObject *HASH_GetRawHashObject(HASH_HashType hashType);
 
 extern void BL_SetForkState(PRBool forked);
 
-#ifndef NSS_DISABLE_ECC
 /*
 ** pepare an ECParam structure from DEREncoded params
  */
@@ -1609,7 +1608,11 @@ extern SECStatus EC_DecodeParams(const SECItem *encodedParams,
                                  ECParams **ecparams);
 extern SECStatus EC_CopyParams(PLArenaPool *arena, ECParams *dstParams,
                                const ECParams *srcParams);
-#endif
+
+/*
+ * use the internal table to get the size in bytes of a single EC point
+ */
+extern int EC_GetPointSize(const ECParams *params);
 
 SEC_END_PROTOS
 

diff --git a/lib/freebl/blapit.h b/lib/freebl/blapit.h
@@ -377,7 +377,6 @@ struct ECParamsStr {
     SECItem DEREncoding;
     ECCurveName name;
     SECItem curveOID;
-    int pointSize;
 };
 typedef struct ECParamsStr ECParams;
 

diff --git a/lib/freebl/ec.c b/lib/freebl/ec.c
@@ -233,7 +233,6 @@ ec_NewKey(ECParams *ecParams, ECPrivateKey **privKey,
     key->ecParams.type = ecParams->type;
     key->ecParams.fieldID.size = ecParams->fieldID.size;
     key->ecParams.fieldID.type = ecParams->fieldID.type;
-    key->ecParams.pointSize = ecParams->pointSize;
     if (ecParams->fieldID.type == ec_field_GFp ||
         ecParams->fieldID.type == ec_field_plain) {
         CHECK_SEC_OK(SECITEM_CopyItem(arena, &key->ecParams.fieldID.u.prime,
@@ -262,7 +261,7 @@ ec_NewKey(ECParams *ecParams, ECPrivateKey **privKey,
     CHECK_SEC_OK(SECITEM_CopyItem(arena, &key->ecParams.curveOID,
                                   &ecParams->curveOID));
 
-    SECITEM_AllocItem(arena, &key->publicValue, ecParams->pointSize);
+    SECITEM_AllocItem(arena, &key->publicValue, EC_GetPointSize(ecParams));
     len = ecParams->order.len;
     SECITEM_AllocItem(arena, &key->privateValue, len);
 
@@ -579,7 +578,7 @@ ECDH_Derive(SECItem *publicValue,
     if (ecParams->fieldID.type == ec_field_plain) {
         const ECMethod *method;
         memset(derivedSecret, 0, sizeof(*derivedSecret));
-        derivedSecret = SECITEM_AllocItem(NULL, derivedSecret, ecParams->pointSize);
+        derivedSecret = SECITEM_AllocItem(NULL, derivedSecret, EC_GetPointSize(ecParams));
         if (derivedSecret == NULL) {
             PORT_SetError(SEC_ERROR_NO_MEMORY);
             return SECFailure;
@@ -605,8 +604,8 @@ ECDH_Derive(SECItem *publicValue,
     MP_DIGITS(&k) = 0;
     memset(derivedSecret, 0, sizeof *derivedSecret);
     len = (ecParams->fieldID.size + 7) >> 3;
-    pointQ.len = ecParams->pointSize;
-    if ((pointQ.data = PORT_Alloc(ecParams->pointSize)) == NULL)
+    pointQ.len = EC_GetPointSize(ecParams);
+    if ((pointQ.data = PORT_Alloc(pointQ.len)) == NULL)
         goto cleanup;
 
     CHECK_MPI_OK(mp_init(&k));
@@ -653,7 +652,7 @@ ECDH_Derive(SECItem *publicValue,
     }
 
     if (pointQ.data) {
-        PORT_ZFree(pointQ.data, ecParams->pointSize);
+        PORT_ZFree(pointQ.data, pointQ.len);
     }
 #else
     PORT_SetError(SEC_ERROR_UNSUPPORTED_KEYALG);
@@ -768,8 +767,8 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
     **
     ** Compute kG
     */
-    kGpoint.len = ecParams->pointSize;
-    kGpoint.data = PORT_Alloc(ecParams->pointSize);
+    kGpoint.len = EC_GetPointSize(ecParams);
+    kGpoint.data = PORT_Alloc(kGpoint.len);
     if ((kGpoint.data == NULL) ||
         (ec_points_mul(ecParams, &k, NULL, NULL, &kGpoint) != SECSuccess))
         goto cleanup;
@@ -888,7 +887,7 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
     }
 
     if (kGpoint.data) {
-        PORT_ZFree(kGpoint.data, ecParams->pointSize);
+        PORT_ZFree(kGpoint.data, kGpoint.len);
     }
 
     if (err) {
@@ -1011,7 +1010,7 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature,
      * The incoming point has been verified in sftk_handlePublicKeyObject.
      */
 
-    SECITEM_AllocItem(NULL, &pointC, ecParams->pointSize);
+    SECITEM_AllocItem(NULL, &pointC, EC_GetPointSize(ecParams));
     if (pointC.data == NULL) {
         goto cleanup;
     }

diff --git a/lib/freebl/ecdecode.c b/lib/freebl/ecdecode.c
@@ -85,7 +85,6 @@ EC_CopyParams(PLArenaPool *arena, ECParams *dstParams,
     dstParams->type = srcParams->type;
     dstParams->fieldID.size = srcParams->fieldID.size;
     dstParams->fieldID.type = srcParams->fieldID.type;
-    dstParams->pointSize = srcParams->pointSize;
     if (srcParams->fieldID.type == ec_field_GFp ||
         srcParams->fieldID.type == ec_field_plain) {
         CHECK_SEC_OK(SECITEM_CopyItem(arena, &dstParams->fieldID.u.prime,
@@ -135,7 +134,6 @@ gf_populate_params(ECCurveName name, ECFieldType field_type, ECParams *params)
     CHECK_OK(curveParams);
     params->fieldID.size = curveParams->size;
     params->fieldID.type = field_type;
-    params->pointSize = curveParams->pointSize;
     if (field_type == ec_field_GFp ||
         field_type == ec_field_plain) {
         CHECK_OK(hexString2SECItem(params->arena, &params->fieldID.u.prime,
@@ -294,4 +292,20 @@ EC_DecodeParams(const SECItem *encodedParams, ECParams **ecparams)
     }
 }
 
+int
+EC_GetPointSize(const ECParams *params)
+{
+    ECCurveName name = params->name;
+    const ECCurveParams *curveParams;
+
+    if ((name < ECCurve_noName) || (name > ECCurve_pastLastCurve) ||
+        ((curveParams = ecCurve_map[name]) == NULL)) {
+        /* unknown curve, calculate point size from params. assume standard curves with 2 points 
+         * and a point compression indicator byte */
+        int sizeInBytes = (params->fieldID.size + 7) / 8;
+        return sizeInBytes * 2 + 1;
+    }
+    return curveParams->pointSize;
+}
+
 #endif /* NSS_DISABLE_ECC */
diff --git a/lib/freebl/ldvector.c b/lib/freebl/ldvector.c
@@ -294,9 +294,13 @@ static const struct FREEBLVectorStr vector =
       ChaCha20Poly1305_CreateContext,
       ChaCha20Poly1305_DestroyContext,
       ChaCha20Poly1305_Seal,
-      ChaCha20Poly1305_Open
+      ChaCha20Poly1305_Open,
 
       /* End of Version 3.018 */
+
+      EC_GetPointSize
+
+      /* End of Version 3.019 */
     };
 
 const FREEBLVector*

diff --git a/lib/freebl/loader.c b/lib/freebl/loader.c
@@ -2116,3 +2116,11 @@ ChaCha20Poly1305_Open(const ChaCha20Poly1305Context *ctx,
         ctx, output, outputLen, maxOutputLen, input, inputLen,
         nonce, nonceLen, ad, adLen);
 }
+
+int
+EC_GetPointSize(const ECParams *params)
+{
+    if (!vector && PR_SUCCESS != freebl_RunLoaderOnce())
+        return SECFailure;
+    return (vector->p_EC_GetPointSize)(params);
+}
diff --git a/lib/freebl/loader.h b/lib/freebl/loader.h
@@ -10,7 +10,7 @@
 
 #include "blapi.h"
 
-#define FREEBL_VERSION 0x0312
+#define FREEBL_VERSION 0x0313
 
 struct FREEBLVectorStr {
 
@@ -732,6 +732,10 @@ struct FREEBLVectorStr {
 
     /* Version 3.018 came to here */
 
+    int (*p_EC_GetPointSize)(const ECParams *);
+
+    /* Version 3.019 came to here */
+
     /* Add new function pointers at the end of this struct and bump
      * FREEBL_VERSION at the beginning of this file. */
 };

diff --git a/lib/softoken/pkcs11.c b/lib/softoken/pkcs11.c
@@ -1799,7 +1799,7 @@ sftk_GetPubKey(SFTKObject *object, CK_KEY_TYPE key_type,
             crv = sftk_Attribute2SSecItem(arena, &pubKey->u.ec.publicValue,
                                           object, CKA_EC_POINT);
             if (crv == CKR_OK) {
-                unsigned int keyLen = pubKey->u.ec.ecParams.pointSize;
+                unsigned int keyLen = EC_GetPointSize(&pubKey->u.ec.ecParams);
 
                 /* special note: We can't just use the first byte to distinguish
                  * between EC_POINT_FORM_UNCOMPRESSED and SEC_ASN1_OCTET_STRING.

diff --git a/lib/softoken/pkcs11c.c b/lib/softoken/pkcs11c.c
@@ -7240,7 +7240,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,
             ecPoint.data = mechParams->pPublicData;
             ecPoint.len = mechParams->ulPublicDataLen;
 
-            pubKeyLen = privKey->u.ec.ecParams.pointSize;
+            pubKeyLen = EC_GetPointSize(&privKey->u.ec.ecParams);
 
             /* if the len is too small, can't be a valid point */
             if (ecPoint.len < pubKeyLen) {