Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1340493 - EC_ValidatePublicKey() should reject Curve25519 keys wi…
…th length != 32 bytes r=franziskus

Differential Revision: https://nss-review.dev.mozaws.net/D220

--HG--
extra : amend_source : 9973a953e6f5d852143b515afde214cc8030f048
  • Loading branch information
Tim Taubert committed Feb 17, 2017
1 parent ae0fe96 commit 3db045b
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 8 deletions.
1 change: 1 addition & 0 deletions gtests/pk11_gtest/manifest.mn
Expand Up @@ -9,6 +9,7 @@ MODULE = nss
CPPSRCS = \
pk11_aeskeywrap_unittest.cc \
pk11_chacha20poly1305_unittest.cc \
pk11_curve25519_unittest.cc \
pk11_ecdsa_unittest.cc \
pk11_export_unittest.cc \
pk11_pbkdf2_unittest.cc \
Expand Down
117 changes: 117 additions & 0 deletions gtests/pk11_gtest/pk11_curve25519_unittest.cc
@@ -0,0 +1,117 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <memory>
#include "nss.h"
#include "pk11pub.h"

#include "gtest/gtest.h"
#include "scoped_ptrs.h"

namespace nss_test {

// <https://tools.ietf.org/html/rfc7748#section-6.1>
const uint8_t kPkcs8[] = {
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,
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};
const uint8_t kSpki[] = {
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};
const uint8_t kSecret[] = {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};

// A public key that's too short (31 bytes).
const uint8_t kSpkiShort[] = {
0x30, 0x38, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01,
0x03, 0x20, 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};

// A public key that's too long (33 bytes).
const uint8_t kSpkiLong[] = {
0x30, 0x3a, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02,
0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01,
0x03, 0x22, 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, 0x34};

static unsigned char* toUcharPtr(const uint8_t* v) {
return const_cast<unsigned char*>(static_cast<const unsigned char*>(v));
}

class Pkcs11Curve25519Test : public ::testing::Test {
protected:
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());
ASSERT_TRUE(slot);

SECItem pkcs8Item = {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,
nullptr);
EXPECT_EQ(SECSuccess, rv);

ScopedSECKEYPrivateKey privKey(key);
ASSERT_TRUE(privKey);

SECItem spkiItem = {siBuffer, toUcharPtr(spki),
static_cast<unsigned int>(spki_len)};

ScopedCERTSubjectPublicKeyInfo certSpki(
SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
ASSERT_TRUE(certSpki);

ScopedSECKEYPublicKey pubKey(SECKEY_ExtractPublicKey(certSpki.get()));
ASSERT_TRUE(pubKey);

ScopedPK11SymKey symKey(PK11_PubDeriveWithKDF(
privKey.get(), pubKey.get(), false, nullptr, nullptr, CKM_ECDH1_DERIVE,
CKM_SHA512_HMAC, CKA_DERIVE, 0, CKD_NULL, nullptr, nullptr));
EXPECT_EQ(expect_success, !!symKey);

if (expect_success) {
rv = PK11_ExtractKeyValue(symKey.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);
}
}
};

TEST_F(Pkcs11Curve25519Test, DeriveSharedSecret) {
Derive(kPkcs8, sizeof(kPkcs8), kSpki, sizeof(kSpki), kSecret, sizeof(kSecret),
true);
}

TEST_F(Pkcs11Curve25519Test, DeriveSharedSecretShort) {
Derive(kPkcs8, sizeof(kPkcs8), kSpkiShort, sizeof(kSpkiShort), nullptr, 0,
false);
}

TEST_F(Pkcs11Curve25519Test, DeriveSharedSecretLong) {
Derive(kPkcs8, sizeof(kPkcs8), kSpkiLong, sizeof(kSpkiLong), nullptr, 0,
false);
}

} // namespace nss_test
1 change: 1 addition & 0 deletions gtests/pk11_gtest/pk11_gtest.gyp
Expand Up @@ -13,6 +13,7 @@
'sources': [
'pk11_aeskeywrap_unittest.cc',
'pk11_chacha20poly1305_unittest.cc',
'pk11_curve25519_unittest.cc',
'pk11_ecdsa_unittest.cc',
'pk11_pbkdf2_unittest.cc',
'pk11_prf_unittest.cc',
Expand Down
3 changes: 1 addition & 2 deletions lib/freebl/ecl/ecp_25519.c
Expand Up @@ -79,8 +79,7 @@ ec_Curve25519_pt_validate(const SECItem *px)
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
};

/* The point must not be longer than 32 (it can be smaller). */
if (px->len <= 32) {
if (px->len == 32) {
p = px->data;
} else {
return SECFailure;
Expand Down
7 changes: 1 addition & 6 deletions lib/softoken/pkcs11c.c
Expand Up @@ -7242,12 +7242,7 @@ NSC_DeriveKey(CK_SESSION_HANDLE hSession,

pubKeyLen = EC_GetPointSize(&privKey->u.ec.ecParams);

/* if the len is too small, can't be a valid point */
if (ecPoint.len < pubKeyLen) {
goto ec_loser;
}
/* if the len is too large, must be an encoded point (length is
* equal case just falls through */
/* if the len is too large, might be an encoded point */
if (ecPoint.len > pubKeyLen) {
SECItem newPoint;

Expand Down

0 comments on commit 3db045b

Please sign in to comment.