Bug 1558234 - Additional EC key tests, r=jcj

Adds additional EC key corner case testing.

Differential Revision: https://phabricator.services.mozilla.com/D34388

5 files changed, 167 insertions, 23 deletions

diff --git a/gtests/common/testvectors/curve25519-vectors.h b/gtests/common/testvectors/curve25519-vectors.h
index bb5a79c14..bf350cfa6 100644
--- a/gtests/common/testvectors/curve25519-vectors.h
+++ b/gtests/common/testvectors/curve25519-vectors.h
@@ -70,6 +70,66 @@ const curve25519_testvector kCurve25519Vectors[] = {
       0x5b, 0x61, 0xc2, 0xec, 0xe4, 0x35, 0x37, 0x3f, 0x83, 0x43, 0xc8, 0x5b,
       0x78, 0x67, 0x4d, 0xad, 0xfc, 0x7e, 0x14, 0x6f, 0x88, 0x2b, 0x4f, 0x34},
      {},
+     false},
+
+    // A private key with leading zeros (they should not be stripped)
+    {{0x30, 0x67, 0x02, 0x01, 0x00, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48,
+      0xce, 0x3d, 0x02, 0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda,
+      0x47, 0x0f, 0x01, 0x04, 0x4c, 0x30, 0x4a, 0x02, 0x01, 0x01, 0x04, 0x20,
+      0x00, 0x99, 0xD1, 0x90, 0x60, 0xCF, 0x79, 0xF0, 0x6F, 0x4F, 0x2E, 0x47,
+      0x97, 0x5B, 0x2A, 0x90, 0x01, 0x6C, 0x94, 0xF4, 0x3D, 0x94, 0x02, 0x57,
+      0x13, 0xDB, 0xB2, 0xA3, 0xD9, 0x54, 0x0B, 0xE5, 0xa1, 0x23, 0x03, 0x21,
+      0x05, 0x66, 0xA7, 0x26, 0xE0, 0xFC, 0x83, 0xEF, 0xA2, 0x56, 0xF4, 0xCC,
+      0xEA, 0x71, 0x07, 0x4D, 0xBB, 0x5C, 0x76, 0x0A, 0x9F, 0xF4, 0x7E, 0x5C,
+      0x5D, 0x4C, 0xB8, 0xDA, 0x9E, 0x44, 0x60, 0x52, 0x00},
+     {0x30, 0x39, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
+      0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01,
+      0x03, 0x21, 0x00, 0xde, 0x9e, 0xdb, 0x7d, 0x7b, 0x7d, 0xc1, 0xb4, 0xd3,
+      0x5b, 0x61, 0xc2, 0xec, 0xe4, 0x35, 0x37, 0x3f, 0x83, 0x43, 0xc8, 0x5b,
+      0x78, 0x67, 0x4d, 0xad, 0xfc, 0x7e, 0x14, 0x6f, 0x88, 0x2b, 0x4f},
+     {0xB9, 0x4B, 0x92, 0xEA, 0xDA, 0x64, 0x40, 0xD3, 0x08, 0x63, 0x06,
+      0x45, 0xF4, 0x4C, 0xCD, 0x19, 0x7B, 0xE6, 0x0A, 0xBC, 0x6C, 0x9D,
+      0x96, 0x8F, 0x5D, 0x70, 0x44, 0x55, 0xD0, 0x1B, 0xEE, 0x4A},
+     true},
+
+    // A private key that's too short
+    {{0x30, 0x66, 0x02, 0x01, 0x00, 0x30, 0x14, 0x06, 0x07, 0x2A, 0x86, 0x48,
+      0xCE, 0x3D, 0x02, 0x01, 0x06, 0x09, 0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA,
+      0x47, 0x0F, 0x01, 0x04, 0x4B, 0x30, 0x49, 0x02, 0x01, 0x01, 0x04, 0x1F,
+      0x07, 0x6D, 0x0A, 0x73, 0x18, 0xA5, 0x7D, 0x3C, 0x16, 0xC1, 0x72, 0x51,
+      0xB2, 0x66, 0x45, 0xDF, 0x4C, 0x2F, 0x87, 0xEB, 0xC0, 0x99, 0x2A, 0xB1,
+      0x77, 0xFB, 0xA5, 0x1D, 0xB9, 0x2C, 0x2A, 0xA1, 0x23, 0x03, 0x21, 0x00,
+      0x85, 0x20, 0xF0, 0x09, 0x89, 0x30, 0xA7, 0x54, 0x74, 0x8B, 0x7D, 0xDC,
+      0xB4, 0x3E, 0xF7, 0x5A, 0x0D, 0xBF, 0x3A, 0x0D, 0x26, 0x38, 0x1A, 0xF4,
+      0xEB, 0xA4, 0xA9, 0x8E, 0xAA, 0x9B, 0x4E, 0x6A},
+     {0x30, 0x39, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
+      0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01,
+      0x03, 0x21, 0x00, 0xde, 0x9e, 0xdb, 0x7d, 0x7b, 0x7d, 0xc1, 0xb4, 0xd3,
+      0x5b, 0x61, 0xc2, 0xec, 0xe4, 0x35, 0x37, 0x3f, 0x83, 0x43, 0xc8, 0x5b,
+      0x78, 0x67, 0x4d, 0xad, 0xfc, 0x7e, 0x14, 0x6f, 0x88, 0x2b, 0x4f},
+     {0x4a, 0x5d, 0x9d, 0x5b, 0xa4, 0xce, 0x2d, 0xe1, 0x72, 0x8e, 0x3b,
+      0xf4, 0x80, 0x35, 0x0f, 0x25, 0xe0, 0x7e, 0x21, 0xc9, 0x47, 0xd1,
+      0x9e, 0x33, 0x76, 0xf0, 0x9b, 0x3c, 0x1e, 0x16, 0x17, 0x42},
+     false},
+
+    // A private key that's too long
+    {{0x30, 0x68, 0x02, 0x01, 0x00, 0x30, 0x14, 0x06, 0x07, 0x2A, 0x86, 0x48,
+      0xCE, 0x3D, 0x02, 0x01, 0x06, 0x09, 0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA,
+      0x47, 0x0F, 0x01, 0x04, 0x4D, 0x30, 0x4B, 0x02, 0x01, 0x01, 0x04, 0x21,
+      0x43, 0x77, 0x07, 0x6D, 0x0A, 0x73, 0x18, 0xA5, 0x7D, 0x3C, 0x16, 0xC1,
+      0x72, 0x51, 0xB2, 0x66, 0x45, 0xDF, 0x4C, 0x2F, 0x87, 0xEB, 0xC0, 0x99,
+      0x2A, 0xB1, 0x77, 0xFB, 0xA5, 0x1D, 0xB9, 0x2C, 0x2A, 0xA1, 0x23, 0x03,
+      0x21, 0x00, 0x85, 0x20, 0xF0, 0x09, 0x89, 0x30, 0xA7, 0x54, 0x74, 0x8B,
+      0x7D, 0xDC, 0xB4, 0x3E, 0xF7, 0x5A, 0x0D, 0xBF, 0x3A, 0x0D, 0x26, 0x38,
+      0x1A, 0xF4, 0xEB, 0xA4, 0xA9, 0x8E, 0xAA, 0x9B, 0x4E, 0x6A},
+     {0x30, 0x39, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
+      0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01,
+      0x03, 0x21, 0x00, 0xde, 0x9e, 0xdb, 0x7d, 0x7b, 0x7d, 0xc1, 0xb4, 0xd3,
+      0x5b, 0x61, 0xc2, 0xec, 0xe4, 0x35, 0x37, 0x3f, 0x83, 0x43, 0xc8, 0x5b,
+      0x78, 0x67, 0x4d, 0xad, 0xfc, 0x7e, 0x14, 0x6f, 0x88, 0x2b, 0x4f},
+     {0x4a, 0x5d, 0x9d, 0x5b, 0xa4, 0xce, 0x2d, 0xe1, 0x72, 0x8e, 0x3b,
+      0xf4, 0x80, 0x35, 0x0f, 0x25, 0xe0, 0x7e, 0x21, 0xc9, 0x47, 0xd1,
+      0x9e, 0x33, 0x76, 0xf0, 0x9b, 0x3c, 0x1e, 0x16, 0x17, 0x42},
      false}};
 
 // Testvectors from project wycheproof
diff --git a/gtests/pk11_gtest/pk11_curve25519_unittest.cc b/gtests/pk11_gtest/pk11_curve25519_unittest.cc
index fb50ce2de..647e3a706 100644
--- a/gtests/pk11_gtest/pk11_curve25519_unittest.cc
+++ b/gtests/pk11_gtest/pk11_curve25519_unittest.cc
@@ -5,7 +5,7 @@
 #include <memory>
 #include "nss.h"
 #include "pk11pub.h"
-
+#include "prerror.h"
 #include "cpputil.h"
 #include "nss_scoped_ptrs.h"
 
@@ -20,46 +20,90 @@ class Pkcs11Curve25519Test
   void Derive(const uint8_t* pkcs8, size_t pkcs8_len, const uint8_t* spki,
               size_t spki_len, const uint8_t* secret, size_t secret_len,
               bool expect_success) {
-    ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
+    ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
     ASSERT_TRUE(slot);
 
-    SECItem pkcs8Item = {siBuffer, toUcharPtr(pkcs8),
-                         static_cast<unsigned int>(pkcs8_len)};
+    SECItem pkcs8_item = {siBuffer, toUcharPtr(pkcs8),
+                          static_cast<unsigned int>(pkcs8_len)};
 
     SECKEYPrivateKey* key = nullptr;
     SECStatus rv = PK11_ImportDERPrivateKeyInfoAndReturnKey(
-        slot.get(), &pkcs8Item, nullptr, nullptr, false, false, KU_ALL, &key,
+        slot.get(), &pkcs8_item, nullptr, nullptr, false, false, KU_ALL, &key,
         nullptr);
     EXPECT_EQ(SECSuccess, rv);
 
-    ScopedSECKEYPrivateKey privKey(key);
-    ASSERT_TRUE(privKey);
+    ScopedSECKEYPrivateKey priv_key_sess(key);
+    ASSERT_TRUE(priv_key_sess);
 
-    SECItem spkiItem = {siBuffer, toUcharPtr(spki),
-                        static_cast<unsigned int>(spki_len)};
+    SECItem spki_item = {siBuffer, toUcharPtr(spki),
+                         static_cast<unsigned int>(spki_len)};
 
-    ScopedCERTSubjectPublicKeyInfo certSpki(
-        SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
-    if (!expect_success && !certSpki) {
+    ScopedCERTSubjectPublicKeyInfo cert_spki(
+        SECKEY_DecodeDERSubjectPublicKeyInfo(&spki_item));
+    if (!expect_success && !cert_spki) {
       return;
     }
-    ASSERT_TRUE(certSpki);
+    ASSERT_TRUE(cert_spki);
 
-    ScopedSECKEYPublicKey pubKey(SECKEY_ExtractPublicKey(certSpki.get()));
-    ASSERT_TRUE(pubKey);
+    ScopedSECKEYPublicKey pub_key_remote(
+        SECKEY_ExtractPublicKey(cert_spki.get()));
+    ASSERT_TRUE(pub_key_remote);
 
-    ScopedPK11SymKey symKey(PK11_PubDeriveWithKDF(
-        privKey.get(), pubKey.get(), false, nullptr, nullptr, CKM_ECDH1_DERIVE,
-        CKM_SHA512_HMAC, CKA_DERIVE, 0, CKD_NULL, nullptr, nullptr));
-    ASSERT_EQ(expect_success, !!symKey);
+    // sym_key_sess = ECDH(session_import(private_test), public_test)
+    ScopedPK11SymKey sym_key_sess(PK11_PubDeriveWithKDF(
+        priv_key_sess.get(), pub_key_remote.get(), false, nullptr, nullptr,
+        CKM_ECDH1_DERIVE, CKM_SHA512_HMAC, CKA_DERIVE, 0, CKD_NULL, nullptr,
+        nullptr));
+    ASSERT_EQ(expect_success, !!sym_key_sess);
 
     if (expect_success) {
-      rv = PK11_ExtractKeyValue(symKey.get());
+      rv = PK11_ExtractKeyValue(sym_key_sess.get());
       EXPECT_EQ(SECSuccess, rv);
 
-      SECItem* keyData = PK11_GetKeyData(symKey.get());
-      EXPECT_EQ(secret_len, keyData->len);
-      EXPECT_EQ(memcmp(keyData->data, secret, secret_len), 0);
+      SECItem* key_data = PK11_GetKeyData(sym_key_sess.get());
+      EXPECT_EQ(secret_len, key_data->len);
+      EXPECT_EQ(memcmp(key_data->data, secret, secret_len), 0);
+
+      // Perform wrapped export on the imported private, import it as
+      // permanent, and verify we derive the same shared secret
+      static const uint8_t pw[] = "pw";
+      SECItem pwItem = {siBuffer, toUcharPtr(pw), sizeof(pw)};
+      ScopedSECKEYEncryptedPrivateKeyInfo epki(PK11_ExportEncryptedPrivKeyInfo(
+          slot.get(), SEC_OID_AES_256_CBC, &pwItem, priv_key_sess.get(), 1,
+          nullptr));
+      ASSERT_NE(nullptr, epki) << "PK11_ExportEncryptedPrivKeyInfo failed: "
+                               << PORT_ErrorToName(PORT_GetError());
+
+      ScopedSECKEYPublicKey pub_key_local(
+          SECKEY_ConvertToPublicKey(priv_key_sess.get()));
+
+      SECKEYPrivateKey* priv_key_tok = nullptr;
+      rv = PK11_ImportEncryptedPrivateKeyInfoAndReturnKey(
+          slot.get(), epki.get(), &pwItem, nullptr,
+          &pub_key_local->u.ec.publicValue, PR_TRUE, PR_TRUE, ecKey, 0,
+          &priv_key_tok, nullptr);
+      ASSERT_EQ(SECSuccess, rv) << "PK11_ImportEncryptedPrivateKeyInfo failed "
+                                << PORT_ErrorToName(PORT_GetError());
+      ASSERT_TRUE(priv_key_tok);
+
+      // sym_key_tok = ECDH(token_import(export(private_test)),
+      // public_test)
+      ScopedPK11SymKey sym_key_tok(PK11_PubDeriveWithKDF(
+          priv_key_tok, pub_key_remote.get(), false, nullptr, nullptr,
+          CKM_ECDH1_DERIVE, CKM_SHA512_HMAC, CKA_DERIVE, 0, CKD_NULL, nullptr,
+          nullptr));
+      EXPECT_TRUE(sym_key_tok);
+
+      if (sym_key_tok) {
+        rv = PK11_ExtractKeyValue(sym_key_tok.get());
+        EXPECT_EQ(SECSuccess, rv);
+
+        key_data = PK11_GetKeyData(sym_key_tok.get());
+        EXPECT_EQ(secret_len, key_data->len);
+        EXPECT_EQ(memcmp(key_data->data, secret, secret_len), 0);
+      }
+      rv = PK11_DeleteTokenPrivateKey(priv_key_tok, true);
+      EXPECT_EQ(SECSuccess, rv);
     }
   };
 
diff --git a/gtests/pk11_gtest/pk11_ecdsa_unittest.cc b/gtests/pk11_gtest/pk11_ecdsa_unittest.cc
index e905f7835..1816e3c9c 100644
--- a/gtests/pk11_gtest/pk11_ecdsa_unittest.cc
+++ b/gtests/pk11_gtest/pk11_ecdsa_unittest.cc
@@ -45,6 +45,11 @@ static const Pkcs11EcdsaTestParams kEcdsaVectors[] = {
       DataBuffer(kP256Spki, sizeof(kP256Spki)),
       DataBuffer(kP256Data, sizeof(kP256Data)),
       DataBuffer(kP256Signature, sizeof(kP256Signature))}},
+    {SEC_OID_SHA256,
+     {DataBuffer(kP256Pkcs8ZeroPad, sizeof(kP256Pkcs8ZeroPad)),
+      DataBuffer(kP256SpkiZeroPad, sizeof(kP256SpkiZeroPad)),
+      DataBuffer(kP256DataZeroPad, sizeof(kP256DataZeroPad)),
+      DataBuffer(kP256SignatureZeroPad, sizeof(kP256SignatureZeroPad))}},
     {SEC_OID_SHA384,
      {DataBuffer(kP384Pkcs8, sizeof(kP384Pkcs8)),
       DataBuffer(kP384Spki, sizeof(kP384Spki)),
diff --git a/gtests/pk11_gtest/pk11_ecdsa_vectors.h b/gtests/pk11_gtest/pk11_ecdsa_vectors.h
index 1dd2c8728..9f625dd08 100644
--- a/gtests/pk11_gtest/pk11_ecdsa_vectors.h
+++ b/gtests/pk11_gtest/pk11_ecdsa_vectors.h
@@ -130,6 +130,38 @@ const uint8_t kP521Signature[] = {
     0xd8, 0xb8, 0xc3, 0x7f, 0xf0, 0x77, 0x7b, 0x1a, 0x20, 0xf8, 0xcc, 0xb1,
     0xdc, 0xcc, 0x43, 0x99, 0x7f, 0x1e, 0xe0, 0xe4, 0x4d, 0xa4, 0xa6, 0x7a};
 
+// ECDSA P256 test case with a leading zero in the private key
+const uint8_t kP256Pkcs8ZeroPad[] = {
+    0x30, 0x81, 0x87, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86,
+    0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d,
+    0x03, 0x01, 0x07, 0x04, 0x6d, 0x30, 0x6b, 0x02, 0x01, 0x01, 0x04, 0x20,
+    0x00, 0x16, 0x40, 0x71, 0x99, 0xe3, 0x07, 0xaa, 0xdc, 0x98, 0x0b, 0x21,
+    0x62, 0xce, 0x66, 0x1f, 0xe4, 0x1a, 0x86, 0x9a, 0x23, 0x33, 0xf6, 0x72,
+    0xb4, 0xa3, 0xdc, 0x3b, 0x50, 0xba, 0x20, 0xce, 0xa1, 0x44, 0x03, 0x42,
+    0x00, 0x04, 0x53, 0x11, 0x9a, 0x86, 0xa0, 0xc2, 0x99, 0x4f, 0xa6, 0xf8,
+    0x08, 0xf8, 0x61, 0x01, 0x0e, 0x6b, 0x04, 0x9c, 0xd8, 0x15, 0x63, 0x2e,
+    0xd1, 0x38, 0x00, 0x10, 0xee, 0xe4, 0xc9, 0x11, 0xff, 0x05, 0xba, 0xd6,
+    0xcd, 0x94, 0xea, 0x00, 0xec, 0x85, 0x26, 0x2c, 0xbd, 0x4d, 0x85, 0xbd,
+    0x20, 0xce, 0xa5, 0xb1, 0x3f, 0x4d, 0x82, 0x9b, 0x9f, 0x28, 0x2e, 0xd3,
+    0x8a, 0x87, 0x1f, 0x89, 0xf8, 0x02};
+const uint8_t kP256SpkiZeroPad[] = {
+    0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
+    0x01, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03,
+    0x42, 0x00, 0x04, 0x53, 0x11, 0x9a, 0x86, 0xa0, 0xc2, 0x99, 0x4f, 0xa6,
+    0xf8, 0x08, 0xf8, 0x61, 0x01, 0x0e, 0x6b, 0x04, 0x9c, 0xd8, 0x15, 0x63,
+    0x2e, 0xd1, 0x38, 0x00, 0x10, 0xee, 0xe4, 0xc9, 0x11, 0xff, 0x05, 0xba,
+    0xd6, 0xcd, 0x94, 0xea, 0x00, 0xec, 0x85, 0x26, 0x2c, 0xbd, 0x4d, 0x85,
+    0xbd, 0x20, 0xce, 0xa5, 0xb1, 0x3f, 0x4d, 0x82, 0x9b, 0x9f, 0x28, 0x2e,
+    0xd3, 0x8a, 0x87, 0x1f, 0x89, 0xf8, 0x02};
+const uint8_t kP256DataZeroPad[] = {'s', 'a', 'm', 'p', 'l', 'e'};
+const uint8_t kP256SignatureZeroPad[] = {
+    0xa6, 0xf4, 0xe4, 0xa8, 0x3f, 0x03, 0x59, 0x89, 0x60, 0x53, 0xe7,
+    0xdc, 0xb5, 0xbe, 0x78, 0xaf, 0xc1, 0xca, 0xc0, 0x65, 0xba, 0xa4,
+    0x3c, 0xf1, 0xe4, 0xae, 0xe3, 0xba, 0x22, 0x3d, 0xac, 0x9d, 0x6d,
+    0x1b, 0x26, 0x00, 0xcf, 0x47, 0xa1, 0xe1, 0x04, 0x21, 0x8d, 0x0b,
+    0xbb, 0x16, 0xfa, 0x3e, 0x59, 0x32, 0x01, 0xb0, 0x45, 0x3e, 0x27,
+    0xa4, 0xc4, 0xfd, 0x31, 0xc9, 0x1a, 0x8e, 0x74, 0xd8};
+
 // ECDSA test vectors, SPKI and PKCS#8 edge cases.
 const uint8_t kP256Pkcs8NoCurveOIDOrAlgorithmParams[] = {
     0x30, 0x7d, 0x02, 0x01, 0x00, 0x30, 0x09, 0x06, 0x07, 0x2a, 0x86, 0x48,
diff --git a/gtests/pk11_gtest/pk11_signature_test.h b/gtests/pk11_gtest/pk11_signature_test.h
index 0526fea55..cd46f17d7 100644
--- a/gtests/pk11_gtest/pk11_signature_test.h
+++ b/gtests/pk11_gtest/pk11_signature_test.h
@@ -59,6 +59,9 @@ class Pk11SignatureTest : public ::testing::Test {
 
     ScopedCERTSubjectPublicKeyInfo certSpki(
         SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
+    if (!certSpki) {
+      return nullptr;
+    }
 
     return ScopedSECKEYPublicKey(SECKEY_ExtractPublicKey(certSpki.get()));
   }