diff -up ./gtests/pk11_gtest/manifest.mn.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/manifest.mn --- ./gtests/pk11_gtest/manifest.mn.handle-malformed-ecdh-gtests 2019-12-05 10:56:41.827832606 -0800 +++ ./gtests/pk11_gtest/manifest.mn 2019-12-05 10:59:53.802966671 -0800 @@ -10,16 +10,18 @@ CPPSRCS = \ pk11_aeskeywrap_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_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_der_private_key_import_unittest.cc \ $(NULL) INCLUDES += -I$(CORE_DEPTH)/gtests/google_test/gtest/include \ diff -up ./gtests/pk11_gtest/pk11_gtest.gyp.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/pk11_gtest.gyp --- ./gtests/pk11_gtest/pk11_gtest.gyp.handle-malformed-ecdh-gtests 2019-12-05 10:56:41.828832617 -0800 +++ ./gtests/pk11_gtest/pk11_gtest.gyp 2019-12-05 11:01:38.874134681 -0800 @@ -11,20 +11,22 @@ 'target_name': 'pk11_gtest', 'type': 'executable', 'sources': [ - 'pk11_aeskeywrap_unittest.cc', 'pk11_aes_gcm_unittest.cc', + 'pk11_aeskeywrap_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_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_der_private_key_import_unittest.cc', '<(DEPTH)/gtests/common/gtests.cc' ], 'dependencies': [ diff -up ./gtests/pk11_gtest/pk11_import_unittest.cc.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/pk11_import_unittest.cc --- ./gtests/pk11_gtest/pk11_import_unittest.cc.handle-malformed-ecdh-gtests 2019-12-05 10:56:41.821832539 -0800 +++ ./gtests/pk11_gtest/pk11_import_unittest.cc 2019-12-05 11:08:42.394842692 -0800 @@ -15,6 +15,7 @@ #include "nss_scoped_ptrs.h" #include "gtest/gtest.h" #include "databuffer.h" +#include "pk11_keygen.h" namespace nss_test { @@ -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 { @@ -42,12 +43,18 @@ class Pk11KeyImportTestBase : public ::t 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); + + // Note: NSS is currently unable export wrapped DH keys, so this doesn't + // test those beyond generate and verify. + if (key_type == dhKey) { + return; + } ASSERT_NE(nullptr, key_info); ASSERT_NE(nullptr, public_value); @@ -66,17 +73,6 @@ class Pk11KeyImportTestBase : public ::t CheckForPublicKey(priv_key, public_value.get()); } - protected: - class ParamHolder { - public: - virtual ~ParamHolder() = default; - virtual void* get() = 0; - }; - - virtual std::unique_ptr MakeParams() = 0; - - CK_MECHANISM_TYPE mech_; - private: SECItem GetPublicComponent(ScopedSECKEYPublicKey& pub_key) { SECItem null = { siBuffer, NULL, 0}; @@ -196,20 +192,14 @@ class Pk11KeyImportTestBase : public ::t } } - 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); // Wrap and export the key. ScopedSECKEYEncryptedPrivateKeyInfo epki(PK11_ExportEncryptedPrivKeyInfo( @@ -239,6 +229,11 @@ class Pk11KeyImportTestBase : public ::t } CheckForPublicKey(priv_key, pub_val); + // Note: NSS is currently unable export wrapped DH keys, so this doesn't + // test those beyond generate and verify. + if (t == dhKey) { + return; + } *key_type = t; key_info->swap(epki); @@ -253,82 +248,13 @@ class Pk11KeyImportTest : public Pk11KeyImportTestBase, public ::testing::WithParamInterface { public: - Pk11KeyImportTest() : Pk11KeyImportTestBase(GetParam()) {} + Pk11KeyImportTest() = default; virtual ~Pk11KeyImportTest() = default; - - protected: - std::unique_ptr MakeParams() override { - switch (mech_) { - case CKM_RSA_PKCS_KEY_PAIR_GEN: - return std::unique_ptr(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(new PqgParamHolder(pqg_params)); - } - return std::unique_ptr(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 ¶ms_; } - - 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 ¶ms_; } - - 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, @@ -339,42 +265,13 @@ INSTANTIATE_TEST_CASE_P(Pk11KeyImportTes class Pk11KeyImportTestEC : public Pk11KeyImportTestBase, public ::testing::WithParamInterface { public: - Pk11KeyImportTestEC() : Pk11KeyImportTestBase(CKM_EC_KEY_PAIR_GEN) {} + Pk11KeyImportTestEC() = default; virtual ~Pk11KeyImportTestEC() = default; - - protected: - std::unique_ptr MakeParams() override { - return std::unique_ptr(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(curve->oid.len); - memcpy(&extra_[2], curve->oid.data, curve->oid.len); - - ec_params_ = {/*.type = */ siBuffer, - /*.data = */ extra_.get(), - /*.len = */ static_cast(plen)}; - } - ~EcParamHolder() = default; - - void* get() override { return &ec_params_; } - - private: - SECKEYECParams ec_params_; - std::unique_ptr 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, diff -up ./gtests/pk11_gtest/pk11_keygen.cc.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/pk11_keygen.cc --- ./gtests/pk11_gtest/pk11_keygen.cc.handle-malformed-ecdh-gtests 2019-12-05 10:56:41.829832628 -0800 +++ ./gtests/pk11_gtest/pk11_keygen.cc 2019-12-05 10:56:41.829832628 -0800 @@ -0,0 +1,143 @@ +/* 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 "pk11_keygen.h" + +#include "pk11pub.h" +#include "pk11pqg.h" +#include "prerror.h" + +#include "gtest/gtest.h" + +namespace nss_test { + +class ParamHolder { + public: + virtual void* get() = 0; + virtual ~ParamHolder() = default; + + protected: + ParamHolder() = default; +}; + +void Pkcs11KeyPairGenerator::GenerateKey(ScopedSECKEYPrivateKey* priv_key, + ScopedSECKEYPublicKey* pub_key) const { + // This function returns if an assertion fails, so don't leak anything. + priv_key->reset(nullptr); + pub_key->reset(nullptr); + + auto params = MakeParams(); + ASSERT_NE(nullptr, params); + + ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot()); + ASSERT_TRUE(slot); + + SECKEYPublicKey* pub_tmp; + ScopedSECKEYPrivateKey priv_tmp(PK11_GenerateKeyPair( + slot.get(), mech_, params->get(), &pub_tmp, PR_FALSE, PR_TRUE, nullptr)); + ASSERT_NE(nullptr, priv_tmp) << "PK11_GenerateKeyPair failed: " + << PORT_ErrorToName(PORT_GetError()); + ASSERT_NE(nullptr, pub_tmp); + + priv_key->swap(priv_tmp); + pub_key->reset(pub_tmp); +} + +class RsaParamHolder : public ParamHolder { + public: + RsaParamHolder() : params_({1024, 0x010001}) {} + ~RsaParamHolder() = default; + + void* get() override { return ¶ms_; } + + 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_({nullptr, params->prime, params->base}) {} + ~DhParamHolder() = default; + + void* get() override { return ¶ms_; } + + private: + SECKEYDHParams params_; +}; + +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(curve->oid.len); + memcpy(&extra_[2], curve->oid.data, curve->oid.len); + + ec_params_ = {siBuffer, extra_.get(), static_cast(plen)}; + } + ~EcParamHolder() = default; + + void* get() override { return &ec_params_; } + + private: + SECKEYECParams ec_params_; + std::unique_ptr extra_; +}; + +std::unique_ptr Pkcs11KeyPairGenerator::MakeParams() const { + switch (mech_) { + case CKM_RSA_PKCS_KEY_PAIR_GEN: + std::cerr << "Generate RSA pair" << std::endl; + return std::unique_ptr(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) { + std::cerr << "Generate DSA pair" << std::endl; + return std::unique_ptr(new PqgParamHolder(pqg_params)); + } + std::cerr << "Generate DH pair" << std::endl; + return std::unique_ptr(new DhParamHolder(pqg_params)); + } + + case CKM_EC_KEY_PAIR_GEN: + std::cerr << "Generate EC pair on " << curve_ << std::endl; + return std::unique_ptr(new EcParamHolder(curve_)); + + default: + ADD_FAILURE() << "unknown OID " << mech_; + } + return nullptr; +} + +} // namespace nss_test diff -up ./gtests/pk11_gtest/pk11_keygen.h.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/pk11_keygen.h --- ./gtests/pk11_gtest/pk11_keygen.h.handle-malformed-ecdh-gtests 2019-12-05 10:56:41.828832617 -0800 +++ ./gtests/pk11_gtest/pk11_keygen.h 2019-12-05 10:56:41.828832617 -0800 @@ -0,0 +1,34 @@ +/* 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 "nss.h" +#include "secoid.h" + +#include "nss_scoped_ptrs.h" + +namespace nss_test { + +class ParamHolder; + +class Pkcs11KeyPairGenerator { + public: + Pkcs11KeyPairGenerator(CK_MECHANISM_TYPE mech, SECOidTag curve_oid) + : mech_(mech), curve_(curve_oid) {} + Pkcs11KeyPairGenerator(CK_MECHANISM_TYPE mech) + : Pkcs11KeyPairGenerator(mech, SEC_OID_UNKNOWN) {} + + CK_MECHANISM_TYPE mechanism() const { return mech_; } + SECOidTag curve() const { return curve_; } + + void GenerateKey(ScopedSECKEYPrivateKey* priv_key, + ScopedSECKEYPublicKey* pub_key) const; + + private: + std::unique_ptr MakeParams() const; + + CK_MECHANISM_TYPE mech_; + SECOidTag curve_; +}; + +} // namespace nss_test diff -up ./gtests/pk11_gtest/pk11_key_unittest.cc.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/pk11_key_unittest.cc --- ./gtests/pk11_gtest/pk11_key_unittest.cc.handle-malformed-ecdh-gtests 2019-12-05 10:56:41.828832617 -0800 +++ ./gtests/pk11_gtest/pk11_key_unittest.cc 2019-12-05 10:56:41.828832617 -0800 @@ -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 +#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 { +}; +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 diff --git ./gtests/pk11_gtest/pk11_curve25519_unittest.cc.handle-malformed-ecdh-gtests ./gtests/pk11_gtest/pk11_curve25519_unittest.cc --- ./gtests/pk11_gtest/pk11_curve25519_unittest.cc.handle-malformed-ecdh-gtests +++ ./gtests/pk11_gtest/pk11_curve25519_unittest.cc @@ -40,6 +40,9 @@ ScopedCERTSubjectPublicKeyInfo certSpki( SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem)); + if (!expect_success && !certSpki) { + return; + } ASSERT_TRUE(certSpki); ScopedSECKEYPublicKey pubKey(SECKEY_ExtractPublicKey(certSpki.get())); diff -up ./gtests/ssl_gtest/tls_connect.cc.addtime ./gtests/ssl_gtest/tls_connect.cc --- ./gtests/ssl_gtest/tls_connect.cc.addtime 2019-12-06 09:02:39.006583359 -0800 +++ ./gtests/ssl_gtest/tls_connect.cc 2019-12-06 09:02:54.120745545 -0800 @@ -292,7 +292,7 @@ void TlsConnectTestBase::Handshake() { ASSERT_TRUE_WAIT((client_->state() != TlsAgent::STATE_CONNECTING) && (server_->state() != TlsAgent::STATE_CONNECTING), - 5000); + 10000); } void TlsConnectTestBase::EnableExtendedMasterSecret() {