Bug 1347499 - Don't try to derive again when we have a curve25519 (ECPoint_XOnly) key and fix ASAN issue, r=ttaubert
authorFranziskus Kiefer <franziskuskiefer@gmail.com>
Wed, 15 Mar 2017 13:07:26 +0100
changeset 13208 81a1d9f72ea5b08a95b837e9395e883ac8b9553c
parent 13207 560e80c021a543a21d174eea1465a52fec290563
child 13209 48e2bf231184503f7c03d703f3bcd5e057d48ca3
push id2079
push userfranziskuskiefer@gmail.com
push dateWed, 15 Mar 2017 13:37:11 +0000
reviewersttaubert
bugs1347499
Bug 1347499 - Don't try to derive again when we have a curve25519 (ECPoint_XOnly) key and fix ASAN issue, r=ttaubert Differential Revision: https://nss-review.dev.mozaws.net/D251
lib/freebl/ecl/curve25519_64.c
lib/pk11wrap/pk11skey.c
--- a/lib/freebl/ecl/curve25519_64.c
+++ b/lib/freebl/ecl/curve25519_64.c
@@ -201,17 +201,17 @@ fsquare(felem *output, const felem *in)
 /* Take a 32-byte number and expand it into polynomial form */
 static void NO_SANITIZE_ALIGNMENT
 fexpand(felem *output, const u8 *in)
 {
     output[0] = *((const uint64_t *)(in)) & MASK51;
     output[1] = (*((const uint64_t *)(in + 6)) >> 3) & MASK51;
     output[2] = (*((const uint64_t *)(in + 12)) >> 6) & MASK51;
     output[3] = (*((const uint64_t *)(in + 19)) >> 1) & MASK51;
-    output[4] = (*((const uint64_t *)(in + 25)) >> 4) & MASK51;
+    output[4] = (*((const uint64_t *)(in + 24)) >> 12) & MASK51;
 }
 
 /* Take a fully reduced polynomial form number and contract it into a
  * 32-byte array
  */
 static void
 fcontract(u8 *output, const felem *input)
 {
--- a/lib/pk11wrap/pk11skey.c
+++ b/lib/pk11wrap/pk11skey.c
@@ -13,16 +13,18 @@
 #include "secmodti.h"
 #include "pkcs11.h"
 #include "pk11func.h"
 #include "secitem.h"
 #include "secoid.h"
 #include "secerr.h"
 #include "hasht.h"
 
+static ECPointEncoding pk11_ECGetPubkeyEncoding(const SECKEYPublicKey *pubKey);
+
 static void
 pk11_EnterKeyMonitor(PK11SymKey *symKey)
 {
     if (!symKey->sessionOwner || !(symKey->slot->isThreadSafe))
         PK11_EnterSlotMonitor(symKey->slot);
 }
 
 static void
@@ -2000,17 +2002,17 @@ PK11_PubDerive(SECKEYPrivateKey *privKey
             pk11_EnterKeyMonitor(symKey);
             crv = PK11_GETTAB(slot)->C_DeriveKey(symKey->session,
                                                  &mechanism, privKey->pkcs11ID, keyTemplate,
                                                  templateCount, &symKey->objectID);
             pk11_ExitKeyMonitor(symKey);
 
             /* old PKCS #11 spec was ambiguous on what needed to be passed,
              * try this again with and encoded public key */
-            if (crv != CKR_OK) {
+            if (crv != CKR_OK && pk11_ECGetPubkeyEncoding(pubKey) != ECPoint_XOnly) {
                 SECItem *pubValue = SEC_ASN1EncodeItem(NULL, NULL,
                                                        &pubKey->u.ec.publicValue,
                                                        SEC_ASN1_GET(SEC_OctetStringTemplate));
                 if (pubValue == NULL) {
                     PORT_ZFree(mechParams, sizeof(CK_ECDH1_DERIVE_PARAMS));
                     break;
                 }
                 mechParams->ulPublicDataLen = pubValue->len;
@@ -2206,16 +2208,21 @@ pk11_PubDeriveECKeyWithKDF(
     crv = PK11_GETTAB(slot)->C_DeriveKey(symKey->session, &mechanism,
                                          privKey->pkcs11ID, keyTemplate,
                                          templateCount, &symKey->objectID);
     pk11_ExitKeyMonitor(symKey);
 
     /* old PKCS #11 spec was ambiguous on what needed to be passed,
      * try this again with an encoded public key */
     if (crv != CKR_OK) {
+        /* For curves that only use X as public value and no encoding we don't
+         * have to try again. (Currently onlye Curve25519) */
+        if (pk11_ECGetPubkeyEncoding(pubKey) == ECPoint_XOnly) {
+            goto loser;
+        }
         SECItem *pubValue = SEC_ASN1EncodeItem(NULL, NULL,
                                                &pubKey->u.ec.publicValue,
                                                SEC_ASN1_GET(SEC_OctetStringTemplate));
         if (pubValue == NULL) {
             goto loser;
         }
         mechParams->ulPublicDataLen = pubValue->len;
         mechParams->pPublicData = pubValue->data;