From a89929ad297555fcb9e3633436cb5d3e7055a2b7 Mon Sep 17 00:00:00 2001 From: Cykesiopka Date: Thu, 5 Mar 2015 16:41:00 +0100 Subject: [PATCH] Bug 1139177 - RSA public key size checking cleanups. r=keeler --- config/external/nss/nss.def | 1 - security/apps/AppTrustDomain.cpp | 8 ++--- security/apps/AppTrustDomain.h | 2 +- security/certverifier/CertVerifier.cpp | 34 +++++++------------ .../certverifier/NSSCertDBTrustDomain.cpp | 6 ++-- security/certverifier/NSSCertDBTrustDomain.h | 4 +-- .../ssl/tests/unit/test_keysize/generate.py | 3 -- .../manager/ssl/tests/unit/test_keysize_ev.js | 2 +- 8 files changed, 24 insertions(+), 36 deletions(-) mode change 100644 => 100755 security/manager/ssl/tests/unit/test_keysize/generate.py diff --git a/config/external/nss/nss.def b/config/external/nss/nss.def index 732f2b07a77..c333bcd5d5a 100644 --- a/config/external/nss/nss.def +++ b/config/external/nss/nss.def @@ -525,7 +525,6 @@ SECKEY_ExtractPublicKey SECKEY_GetPublicKeyType SECKEY_ImportDERPublicKey SECKEY_PublicKeyStrength -SECKEY_PublicKeyStrengthInBits SECKEY_RSAPSSParamsTemplate DATA SECKEY_SignatureLen SECMIME_DecryptionAllowed diff --git a/security/apps/AppTrustDomain.cpp b/security/apps/AppTrustDomain.cpp index 8ca6e8b6fce..795f58bc219 100644 --- a/security/apps/AppTrustDomain.cpp +++ b/security/apps/AppTrustDomain.cpp @@ -30,14 +30,14 @@ using namespace mozilla::pkix; extern PRLogModuleInfo* gPIPNSSLog; #endif -static const unsigned int DEFAULT_MINIMUM_NON_ECC_BITS = 2048; +static const unsigned int DEFAULT_MIN_RSA_BITS = 2048; namespace mozilla { namespace psm { AppTrustDomain::AppTrustDomain(ScopedCERTCertList& certChain, void* pinArg) : mCertChain(certChain) , mPinArg(pinArg) - , mMinimumNonECCBits(DEFAULT_MINIMUM_NON_ECC_BITS) + , mMinRSABits(DEFAULT_MIN_RSA_BITS) { } @@ -75,7 +75,7 @@ AppTrustDomain::SetTrustedRoot(AppTrustedRoot trustedRoot) trustedDER.data = const_cast(marketplaceStageRoot); trustedDER.len = mozilla::ArrayLength(marketplaceStageRoot); // The staging root was generated with a 1024-bit key. - mMinimumNonECCBits = 1024u; + mMinRSABits = 1024u; break; case nsIX509CertDB::AppXPCShellRoot: @@ -254,7 +254,7 @@ Result AppTrustDomain::CheckRSAPublicKeyModulusSizeInBits( EndEntityOrCA /*endEntityOrCA*/, unsigned int modulusSizeInBits) { - if (modulusSizeInBits < mMinimumNonECCBits) { + if (modulusSizeInBits < mMinRSABits) { return Result::ERROR_INADEQUATE_KEY_SIZE; } return Success; diff --git a/security/apps/AppTrustDomain.h b/security/apps/AppTrustDomain.h index 977ceea9f40..102cb62b144 100644 --- a/security/apps/AppTrustDomain.h +++ b/security/apps/AppTrustDomain.h @@ -61,7 +61,7 @@ private: /*out*/ ScopedCERTCertList& mCertChain; void* mPinArg; // non-owning! ScopedCERTCertificate mTrustedRoot; - unsigned int mMinimumNonECCBits; + unsigned int mMinRSABits; }; } } // namespace mozilla::psm diff --git a/security/certverifier/CertVerifier.cpp b/security/certverifier/CertVerifier.cpp index fca5f89e48a..29d8544022e 100644 --- a/security/certverifier/CertVerifier.cpp +++ b/security/certverifier/CertVerifier.cpp @@ -156,8 +156,8 @@ BuildCertChainForOneKeyUsage(NSSCertDBTrustDomain& trustDomain, Input certDER, return rv; } -static const unsigned int MINIMUM_NON_ECC_BITS = 2048; -static const unsigned int MINIMUM_NON_ECC_BITS_WEAK = 1024; +static const unsigned int MIN_RSA_BITS = 2048; +static const unsigned int MIN_RSA_BITS_WEAK = 1024; SECStatus CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, @@ -240,8 +240,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, // just use trustEmail as it is the closest alternative. NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, @@ -267,7 +266,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ? NSSCertDBTrustDomain::LocalOnlyOCSPForEV : NSSCertDBTrustDomain::FetchOCSPForEV, mOCSPCache, pinArg, ocspGETConfig, mPinningMode, - MINIMUM_NON_ECC_BITS, hostname, builtChain); + MIN_RSA_BITS, hostname, builtChain); rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time, KeyUsage::digitalSignature,// (EC)DHE KeyUsage::keyEncipherment, // RSA @@ -292,8 +291,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, // Now try non-EV. NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, mPinningMode, - MINIMUM_NON_ECC_BITS, hostname, - builtChain); + MIN_RSA_BITS, hostname, builtChain); rv = BuildCertChainForOneKeyUsage(trustDomain, certDER, time, KeyUsage::digitalSignature, // (EC)DHE KeyUsage::keyEncipherment, // RSA @@ -312,7 +310,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, // If that failed, try again with a smaller minimum key size. NSSCertDBTrustDomain trustDomainWeak(trustSSL, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, mPinningMode, - MINIMUM_NON_ECC_BITS_WEAK, hostname, + MIN_RSA_BITS_WEAK, hostname, builtChain); rv = BuildCertChainForOneKeyUsage(trustDomainWeak, certDER, time, KeyUsage::digitalSignature, // (EC)DHE @@ -336,8 +334,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, case certificateUsageSSLCA: { NSSCertDBTrustDomain trustDomain(trustSSL, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeCA, KeyUsage::keyCertSign, KeyPurposeId::id_kp_serverAuth, @@ -348,8 +345,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, case certificateUsageEmailSigner: { NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, @@ -371,8 +367,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, // based on the result of the verification(s). NSSCertDBTrustDomain trustDomain(trustEmail, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::keyEncipherment, // RSA @@ -392,8 +387,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, NSSCertDBTrustDomain trustDomain(trustObjectSigning, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(trustDomain, certDER, time, EndEntityOrCA::MustBeEndEntity, KeyUsage::digitalSignature, @@ -422,16 +416,14 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, NSSCertDBTrustDomain sslTrust(trustSSL, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(sslTrust, certDER, time, endEntityOrCA, keyUsage, eku, CertPolicyId::anyPolicy, stapledOCSPResponse); if (rv == Result::ERROR_UNKNOWN_ISSUER) { NSSCertDBTrustDomain emailTrust(trustEmail, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, nullptr, - builtChain); + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(emailTrust, certDER, time, endEntityOrCA, keyUsage, eku, CertPolicyId::anyPolicy, stapledOCSPResponse); @@ -440,7 +432,7 @@ CertVerifier::VerifyCert(CERTCertificate* cert, SECCertificateUsage usage, ocspFetching, mOCSPCache, pinArg, ocspGETConfig, pinningDisabled, - MINIMUM_NON_ECC_BITS_WEAK, + MIN_RSA_BITS_WEAK, nullptr, builtChain); rv = BuildCertChain(objectSigningTrust, certDER, time, endEntityOrCA, keyUsage, eku, diff --git a/security/certverifier/NSSCertDBTrustDomain.cpp b/security/certverifier/NSSCertDBTrustDomain.cpp index 001b39f7dcc..7909a93c805 100644 --- a/security/certverifier/NSSCertDBTrustDomain.cpp +++ b/security/certverifier/NSSCertDBTrustDomain.cpp @@ -53,7 +53,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType, /*optional but shouldn't be*/ void* pinArg, CertVerifier::OcspGetConfig ocspGETConfig, CertVerifier::PinningMode pinningMode, - unsigned int minimumNonECCKeyBits, + unsigned int minRSABits, /*optional*/ const char* hostname, /*optional*/ ScopedCERTCertList* builtChain) : mCertDBTrustType(certDBTrustType) @@ -62,7 +62,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(SECTrustType certDBTrustType, , mPinArg(pinArg) , mOCSPGetConfig(ocspGETConfig) , mPinningMode(pinningMode) - , mMinimumNonECCBits(minimumNonECCKeyBits) + , mMinRSABits(minRSABits) , mHostname(hostname) , mBuiltChain(builtChain) , mCertBlocklist(do_GetService(NS_CERTBLOCKLIST_CONTRACTID)) @@ -720,7 +720,7 @@ Result NSSCertDBTrustDomain::CheckRSAPublicKeyModulusSizeInBits( EndEntityOrCA /*endEntityOrCA*/, unsigned int modulusSizeInBits) { - if (modulusSizeInBits < mMinimumNonECCBits) { + if (modulusSizeInBits < mMinRSABits) { return Result::ERROR_INADEQUATE_KEY_SIZE; } return Success; diff --git a/security/certverifier/NSSCertDBTrustDomain.h b/security/certverifier/NSSCertDBTrustDomain.h index 8c003272f94..f3d6f10be50 100644 --- a/security/certverifier/NSSCertDBTrustDomain.h +++ b/security/certverifier/NSSCertDBTrustDomain.h @@ -56,7 +56,7 @@ public: OCSPCache& ocspCache, void* pinArg, CertVerifier::OcspGetConfig ocspGETConfig, CertVerifier::PinningMode pinningMode, - unsigned int minimumNonECCKeyBits, + unsigned int minRSABits, /*optional*/ const char* hostname = nullptr, /*optional out*/ ScopedCERTCertList* builtChain = nullptr); @@ -130,7 +130,7 @@ private: void* mPinArg; // non-owning! const CertVerifier::OcspGetConfig mOCSPGetConfig; CertVerifier::PinningMode mPinningMode; - const unsigned int mMinimumNonECCBits; + const unsigned int mMinRSABits; const char* mHostname; // non-owning - only used for pinning checks ScopedCERTCertList* mBuiltChain; // non-owning nsCOMPtr mCertBlocklist; diff --git a/security/manager/ssl/tests/unit/test_keysize/generate.py b/security/manager/ssl/tests/unit/test_keysize/generate.py old mode 100644 new mode 100755 index 1f039fb6174..8239dcabce1 --- a/security/manager/ssl/tests/unit/test_keysize/generate.py +++ b/security/manager/ssl/tests/unit/test_keysize/generate.py @@ -245,9 +245,6 @@ def generate_combination_chains(): # Create a NSS DB for use by the OCSP responder. CertUtils.init_nss_db(srcdir) -# TODO(bug 636807): SECKEY_PublicKeyStrengthInBits() rounds up the number of -# bits to the next multiple of 8 - therefore the highest key size less than 1024 -# that can be tested is 1016, less than 2048 is 2040 and so on. generate_rsa_chains('1016', '1024', False) generate_rsa_chains('2040', '2048', True) generate_ecc_chains() diff --git a/security/manager/ssl/tests/unit/test_keysize_ev.js b/security/manager/ssl/tests/unit/test_keysize_ev.js index c7039a12640..f9c3a25671f 100644 --- a/security/manager/ssl/tests/unit/test_keysize_ev.js +++ b/security/manager/ssl/tests/unit/test_keysize_ev.js @@ -85,7 +85,7 @@ function addKeySizeTestForEV(expectedNamesForOCSP, * none of the chains validate as EV. * * Note: This function assumes that the key size requirements for EV are greater - * than or equal to the requirements for DV. + * than the requirements for DV. * * @param {Number} inadequateKeySize * The inadequate key size of the generated certs.