Skip to content

Commit

Permalink
Bug 1515342 - Tests for invalid DH public keys, r=jcj
Browse files Browse the repository at this point in the history 
Summary:
This prevents crashes on invalid, particularly NULL, keys for DH and ECDH.

I factored out test code already landed for this.

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

--HG--
extra : rebase_source : 60cd6595e7d6800f19ff9ae76edb9fd7c80da159
extra : amend_source : 5b35922ce52fea1d296cb3958d2fed13cce337fa
extra : absorb_source : 44edeb5c178c18da753030c134563936e320a2a0
extra : histedit_source : 00f70cda905c30f71602d06718feb03d1a87f738
  • Loading branch information
@martinthomson
martinthomson committed Feb 24, 2019
1 parent 2fee826 commit 2323fb4
Show file tree
Hide file tree
Showing 6 changed files with 289 additions and 144 deletions.
4 changes: 3 additions & 1 deletion gtests/pk11_gtest/manifest.mn
View file
Expand Up @@ -12,18 +12,20 @@ CPPSRCS = \
pk11_cbc_unittest.cc \
pk11_chacha20poly1305_unittest.cc \
pk11_curve25519_unittest.cc \
pk11_der_private_key_import_unittest.cc \
pk11_ecdsa_unittest.cc \
pk11_encrypt_derive_unittest.cc \
pk11_export_unittest.cc \
pk11_find_certs_unittest.cc \
pk11_import_unittest.cc \
pk11_keygen.cc \
pk11_key_unittest.cc \
pk11_pbkdf2_unittest.cc \
pk11_prf_unittest.cc \
pk11_prng_unittest.cc \
pk11_rsapkcs1_unittest.cc \
pk11_rsapss_unittest.cc \
pk11_seed_cbc_unittest.cc \
pk11_der_private_key_import_unittest.cc \
$(NULL)

INCLUDES += -I$(CORE_DEPTH)/gtests/google_test/gtest/include \
Expand Down
6 changes: 4 additions & 2 deletions gtests/pk11_gtest/pk11_gtest.gyp
View file
Expand Up @@ -11,24 +11,26 @@
'target_name': 'pk11_gtest',
'type': 'executable',
'sources': [
'pk11_aeskeywrap_unittest.cc',
'pk11_aes_cmac_unittest.cc',
'pk11_aes_gcm_unittest.cc',
'pk11_aeskeywrap_unittest.cc',
'pk11_cbc_unittest.cc',
'pk11_chacha20poly1305_unittest.cc',
'pk11_cipherop_unittest.cc',
'pk11_curve25519_unittest.cc',
'pk11_der_private_key_import_unittest.cc',
'pk11_ecdsa_unittest.cc',
'pk11_encrypt_derive_unittest.cc',
'pk11_find_certs_unittest.cc',
'pk11_import_unittest.cc',
'pk11_keygen.cc',
'pk11_key_unittest.cc',
'pk11_pbkdf2_unittest.cc',
'pk11_prf_unittest.cc',
'pk11_prng_unittest.cc',
'pk11_rsapkcs1_unittest.cc',
'pk11_rsapss_unittest.cc',
'pk11_seed_cbc_unittest.cc',
'pk11_der_private_key_import_unittest.cc',
'<(DEPTH)/gtests/common/gtests.cc'
],
'dependencies': [
Expand Down
166 changes: 25 additions & 141 deletions gtests/pk11_gtest/pk11_import_unittest.cc
View file
Expand Up @@ -15,6 +15,7 @@
#include "nss_scoped_ptrs.h"
#include "gtest/gtest.h"
#include "databuffer.h"
#include "pk11_keygen.h"

namespace nss_test {

Expand All @@ -30,7 +31,7 @@ struct PK11GenericObjectsDeleter {

class Pk11KeyImportTestBase : public ::testing::Test {
public:
Pk11KeyImportTestBase(CK_MECHANISM_TYPE mech) : mech_(mech) {}
Pk11KeyImportTestBase() = default;
virtual ~Pk11KeyImportTestBase() = default;

void SetUp() override {
Expand All @@ -42,20 +43,19 @@ class Pk11KeyImportTestBase : public ::testing::Test {
password_.reset(SECITEM_DupItem(&pwItem));
}

void Test() {
void Test(const Pkcs11KeyPairGenerator& generator) {
// Generate a key and export it.
KeyType key_type;
KeyType key_type = nullKey;
ScopedSECKEYEncryptedPrivateKeyInfo key_info;
ScopedSECItem public_value;
GenerateAndExport(&key_type, &key_info, &public_value);
GenerateAndExport(generator, &key_type, &key_info, &public_value);

ASSERT_NE(nullptr, public_value);
// Note: NSS is currently unable export wrapped DH keys, so this doesn't
// test
// CKM_DH_PKCS_KEY_PAIR_GEN beyond generate and verify
// test those beyond generate and verify.
if (key_type == dhKey) {
return;
}
ASSERT_NE(nullptr, public_value);
ASSERT_NE(nullptr, key_info);

// Now import the encrypted key.
Expand All @@ -73,17 +73,6 @@ class Pk11KeyImportTestBase : public ::testing::Test {
CheckForPublicKey(priv_key, public_value.get());
}

protected:
class ParamHolder {
public:
virtual ~ParamHolder() = default;
virtual void* get() = 0;
};

virtual std::unique_ptr<ParamHolder> MakeParams() = 0;

CK_MECHANISM_TYPE mech_;

private:
SECItem GetPublicComponent(ScopedSECKEYPublicKey& pub_key) {
SECItem null = {siBuffer, NULL, 0};
Expand Down Expand Up @@ -202,20 +191,14 @@ class Pk11KeyImportTestBase : public ::testing::Test {
}
}

void GenerateAndExport(KeyType* key_type,
void GenerateAndExport(const Pkcs11KeyPairGenerator& generator,
KeyType* key_type,
ScopedSECKEYEncryptedPrivateKeyInfo* key_info,
ScopedSECItem* public_value) {
auto params = MakeParams();
ASSERT_NE(nullptr, params);

SECKEYPublicKey* pub_tmp;
ScopedSECKEYPrivateKey priv_key(
PK11_GenerateKeyPair(slot_.get(), mech_, params->get(), &pub_tmp,
PR_FALSE, PR_TRUE, nullptr));
ASSERT_NE(nullptr, priv_key) << "PK11_GenerateKeyPair failed: "
<< PORT_ErrorToName(PORT_GetError());
ScopedSECKEYPublicKey pub_key(pub_tmp);
ASSERT_NE(nullptr, pub_key);
ScopedSECKEYPrivateKey priv_key;
ScopedSECKEYPublicKey pub_key;
generator.GenerateKey(&priv_key, &pub_key);
ASSERT_TRUE(priv_key);

// Save the public value, which we will need on import */
SECItem* pub_val;
Expand All @@ -240,14 +223,13 @@ class Pk11KeyImportTestBase : public ::testing::Test {
CheckForPublicKey(priv_key, pub_val);

*key_type = t;
public_value->reset(SECITEM_DupItem(pub_val));

// Note: NSS is currently unable export wrapped DH keys, so this doesn't
// test
// CKM_DH_PKCS_KEY_PAIR_GEN beyond generate and verify
if (mech_ == CKM_DH_PKCS_KEY_PAIR_GEN) {
// test those beyond generate and verify.
if (t == dhKey) {
return;
}
public_value->reset(SECITEM_DupItem(pub_val));

// Wrap and export the key.
ScopedSECKEYEncryptedPrivateKeyInfo epki(PK11_ExportEncryptedPrivKeyInfo(
slot_.get(), SEC_OID_AES_256_CBC, password_.get(), priv_key.get(), 1,
Expand All @@ -266,82 +248,13 @@ class Pk11KeyImportTest
: public Pk11KeyImportTestBase,
public ::testing::WithParamInterface<CK_MECHANISM_TYPE> {
public:
Pk11KeyImportTest() : Pk11KeyImportTestBase(GetParam()) {}
Pk11KeyImportTest() = default;
virtual ~Pk11KeyImportTest() = default;

protected:
std::unique_ptr<ParamHolder> MakeParams() override {
switch (mech_) {
case CKM_RSA_PKCS_KEY_PAIR_GEN:
return std::unique_ptr<ParamHolder>(new RsaParamHolder());

case CKM_DSA_KEY_PAIR_GEN:
case CKM_DH_PKCS_KEY_PAIR_GEN: {
PQGParams* pqg_params = nullptr;
PQGVerify* pqg_verify = nullptr;
const unsigned int key_size = 1024;
SECStatus rv = PK11_PQG_ParamGenV2(key_size, 0, key_size / 16,
&pqg_params, &pqg_verify);
if (rv != SECSuccess) {
ADD_FAILURE() << "PK11_PQG_ParamGenV2 failed";
return nullptr;
}
EXPECT_NE(nullptr, pqg_verify);
EXPECT_NE(nullptr, pqg_params);
PK11_PQG_DestroyVerify(pqg_verify);
if (mech_ == CKM_DSA_KEY_PAIR_GEN) {
return std::unique_ptr<ParamHolder>(new PqgParamHolder(pqg_params));
}
return std::unique_ptr<ParamHolder>(new DhParamHolder(pqg_params));
}

default:
ADD_FAILURE() << "unknown OID " << mech_;
}
return nullptr;
}

private:
class RsaParamHolder : public ParamHolder {
public:
RsaParamHolder()
: params_({/*.keySizeInBits = */ 1024, /*.pe = */ 0x010001}) {}
~RsaParamHolder() = default;

void* get() override { return &params_; }

private:
PK11RSAGenParams params_;
};

class PqgParamHolder : public ParamHolder {
public:
PqgParamHolder(PQGParams* params) : params_(params) {}
~PqgParamHolder() = default;

void* get() override { return params_.get(); }

private:
ScopedPQGParams params_;
};

class DhParamHolder : public PqgParamHolder {
public:
DhParamHolder(PQGParams* params)
: PqgParamHolder(params),
params_({/*.arena = */ nullptr,
/*.prime = */ params->prime,
/*.base = */ params->base}) {}
~DhParamHolder() = default;

void* get() override { return &params_; }

private:
SECKEYDHParams params_;
};
};

TEST_P(Pk11KeyImportTest, GenerateExportImport) { Test(); }
TEST_P(Pk11KeyImportTest, GenerateExportImport) {
Test(Pkcs11KeyPairGenerator(GetParam()));
}

INSTANTIATE_TEST_CASE_P(Pk11KeyImportTest, Pk11KeyImportTest,
::testing::Values(CKM_RSA_PKCS_KEY_PAIR_GEN,
Expand All @@ -351,42 +264,13 @@ INSTANTIATE_TEST_CASE_P(Pk11KeyImportTest, Pk11KeyImportTest,
class Pk11KeyImportTestEC : public Pk11KeyImportTestBase,
public ::testing::WithParamInterface<SECOidTag> {
public:
Pk11KeyImportTestEC() : Pk11KeyImportTestBase(CKM_EC_KEY_PAIR_GEN) {}
Pk11KeyImportTestEC() = default;
virtual ~Pk11KeyImportTestEC() = default;

protected:
std::unique_ptr<ParamHolder> MakeParams() override {
return std::unique_ptr<ParamHolder>(new EcParamHolder(GetParam()));
}

private:
class EcParamHolder : public ParamHolder {
public:
EcParamHolder(SECOidTag curve_oid) {
SECOidData* curve = SECOID_FindOIDByTag(curve_oid);
EXPECT_NE(nullptr, curve);

size_t plen = curve->oid.len + 2;
extra_.reset(new uint8_t[plen]);
extra_[0] = SEC_ASN1_OBJECT_ID;
extra_[1] = static_cast<uint8_t>(curve->oid.len);
memcpy(&extra_[2], curve->oid.data, curve->oid.len);

ec_params_ = {/*.type = */ siBuffer,
/*.data = */ extra_.get(),
/*.len = */ static_cast<unsigned int>(plen)};
}
~EcParamHolder() = default;

void* get() override { return &ec_params_; }

private:
SECKEYECParams ec_params_;
std::unique_ptr<uint8_t[]> extra_;
};
};

TEST_P(Pk11KeyImportTestEC, GenerateExportImport) { Test(); }
TEST_P(Pk11KeyImportTestEC, GenerateExportImport) {
Test(Pkcs11KeyPairGenerator(CKM_EC_KEY_PAIR_GEN, GetParam()));
}

INSTANTIATE_TEST_CASE_P(Pk11KeyImportTestEC, Pk11KeyImportTestEC,
::testing::Values(SEC_OID_SECG_EC_SECP256R1,
Expand Down
80 changes: 80 additions & 0 deletions gtests/pk11_gtest/pk11_key_unittest.cc
View file
@@ -0,0 +1,80 @@
/* 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 "pk11pqg.h"
#include "prerror.h"
#include "secoid.h"

#include "gtest/gtest.h"
#include "nss_scoped_ptrs.h"
#include "pk11_keygen.h"

namespace nss_test {

class Pkcs11NullKeyTestBase : public ::testing::Test {
protected:
// This constructs a key pair, then erases the public value from the public
// key. NSS should reject this.
void Test(const Pkcs11KeyPairGenerator& generator,
CK_MECHANISM_TYPE dh_mech) {
ScopedSECKEYPrivateKey priv;
ScopedSECKEYPublicKey pub;
generator.GenerateKey(&priv, &pub);
ASSERT_TRUE(priv);

// These don't leak because they are allocated to the arena associated with
// the public key.
SECItem* pub_val = nullptr;
switch (SECKEY_GetPublicKeyType(pub.get())) {
case rsaKey:
pub_val = &pub->u.rsa.modulus;
break;

case dsaKey:
pub_val = &pub->u.dsa.publicValue;
break;

case dhKey:
pub_val = &pub->u.dh.publicValue;
break;

case ecKey:
pub_val = &pub->u.ec.publicValue;
break;

default:
FAIL() << "Unknown key type " << SECKEY_GetPublicKeyType(pub.get());
}
pub_val->data = nullptr;
pub_val->len = 0;

ScopedPK11SymKey symKey(PK11_PubDeriveWithKDF(
priv.get(), pub.get(), false, nullptr, nullptr, dh_mech,
CKM_SHA512_HMAC, CKA_DERIVE, 0, CKD_NULL, nullptr, nullptr));
ASSERT_FALSE(symKey);
}
};

class Pkcs11DhNullKeyTest : public Pkcs11NullKeyTestBase {};
TEST_F(Pkcs11DhNullKeyTest, UseNullPublicValue) {
Test(Pkcs11KeyPairGenerator(CKM_DH_PKCS_KEY_PAIR_GEN), CKM_DH_PKCS_DERIVE);
}

class Pkcs11EcdhNullKeyTest : public Pkcs11NullKeyTestBase,
public ::testing::WithParamInterface<SECOidTag> {
};
TEST_P(Pkcs11EcdhNullKeyTest, UseNullPublicValue) {
Test(Pkcs11KeyPairGenerator(CKM_EC_KEY_PAIR_GEN, GetParam()),
CKM_ECDH1_DERIVE);
}
INSTANTIATE_TEST_CASE_P(Pkcs11EcdhNullKeyTest, Pkcs11EcdhNullKeyTest,
::testing::Values(SEC_OID_SECG_EC_SECP256R1,
SEC_OID_SECG_EC_SECP384R1,
SEC_OID_SECG_EC_SECP521R1,
SEC_OID_CURVE25519));

} // namespace nss_test

0 comments on commit 2323fb4

Please sign in to comment.