Bug 1712972 - only call IsCertBuiltInRoot on the socket thread during certificate verification r=jschanck

Based on a patch authored by R. Martinho Fernandes <bugs@rmf.io>.

Differential Revision: https://phabricator.services.mozilla.com/D116505
This commit is contained in:
Dana Keeler 2021-12-10 21:14:23 +00:00
parent c4778085df
commit d74d5e91d6
5 changed files with 110 additions and 87 deletions

View File

@ -22,6 +22,7 @@
#include "mozilla/IntegerPrintfMacros.h"
#include "mozilla/Logging.h"
#include "nsNSSComponent.h"
#include "mozilla/SyncRunnable.h"
#include "nsPromiseFlatString.h"
#include "nsServiceManagerUtils.h"
#include "pk11pub.h"
@ -30,6 +31,7 @@
#include "mozpkix/pkixnss.h"
#include "mozpkix/pkixutil.h"
#include "secmod.h"
#include "nsNetCID.h"
using namespace mozilla::ct;
using namespace mozilla::pkix;
@ -145,20 +147,6 @@ CertVerifier::CertVerifier(OcspDownloadConfig odc, OcspStrictConfig osc,
CertVerifier::~CertVerifier() = default;
Result IsCertChainRootBuiltInRoot(const nsTArray<nsTArray<uint8_t>>& chain,
bool& result) {
if (chain.IsEmpty()) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
const nsTArray<uint8_t>& rootBytes = chain.LastElement();
Input rootInput;
Result rv = rootInput.Init(rootBytes.Elements(), rootBytes.Length());
if (rv != Result::Success) {
return rv;
}
return IsCertBuiltInRoot(rootInput, result);
}
Result IsDelegatedCredentialAcceptable(const DelegatedCredentialInfo& dcInfo) {
bool isEcdsa = dcInfo.scheme == ssl_sig_ecdsa_secp256r1_sha256 ||
dcInfo.scheme == ssl_sig_ecdsa_secp384r1_sha384 ||
@ -179,6 +167,12 @@ Result IsDelegatedCredentialAcceptable(const DelegatedCredentialInfo& dcInfo) {
// has been added to the NSS trust store, because it has been approved
// for inclusion according to the Mozilla CA policy, and might be accepted
// by Mozilla applications as an issuer for certificates seen on the public web.
// Ideally this would only be called from the socket thread. In the context of
// TLS server certificate verification, IsCertBuiltInRootWithSyncDispatch
// ensures this function is only called from the socket thread. However, there
// are other code paths that reach this function that do not run on the socket
// thread. None of these code paths should be reachable during TLS server
// certificate verification.
Result IsCertBuiltInRoot(Input certInput, bool& result) {
if (NS_FAILED(BlockUntilLoadableCertsLoaded())) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
@ -496,7 +490,8 @@ Result CertVerifier::VerifyCert(
/*optional out*/ KeySizeStatus* keySizeStatus,
/*optional out*/ SHA1ModeResult* sha1ModeResult,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo,
/*optional out*/ CertificateTransparencyInfo* ctInfo) {
/*optional out*/ CertificateTransparencyInfo* ctInfo,
/*optional out*/ bool* isBuiltChainRootBuiltInRoot) {
MOZ_LOG(gCertVerifierLog, LogLevel::Debug, ("Top of VerifyCert\n"));
MOZ_ASSERT(usage == certificateUsageSSLServer || !(flags & FLAG_MUST_BE_EV));
@ -538,6 +533,10 @@ Result CertVerifier::VerifyCert(
return Result::FATAL_ERROR_INVALID_ARGS;
}
if (isBuiltChainRootBuiltInRoot) {
*isBuiltChainRootBuiltInRoot = false;
}
Input certDER;
Result rv = certDER.Init(certBytes.Elements(), certBytes.Length());
if (rv != Success) {
@ -665,15 +664,9 @@ Result CertVerifier::VerifyCert(
KeyPurposeId::id_kp_serverAuth, evPolicy, stapledOCSPResponse,
ocspStaplingStatus);
if (rv == Success &&
sha1ModeConfigurations[i] == SHA1Mode::ImportedRoot) {
bool isBuiltInRoot = false;
rv = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
if (rv != Success) {
break;
}
if (isBuiltInRoot) {
rv = Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
}
sha1ModeConfigurations[i] == SHA1Mode::ImportedRoot &&
trustDomain.GetIsBuiltChainRootBuiltInRoot()) {
rv = Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
}
if (rv == Success) {
MOZ_LOG(gCertVerifierLog, LogLevel::Debug,
@ -690,6 +683,10 @@ Result CertVerifier::VerifyCert(
if (rv != Success) {
break;
}
if (isBuiltChainRootBuiltInRoot) {
*isBuiltChainRootBuiltInRoot =
trustDomain.GetIsBuiltChainRootBuiltInRoot();
}
}
}
if (rv == Success) {
@ -756,15 +753,9 @@ Result CertVerifier::VerifyCert(
rv = Result::ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED;
}
if (rv == Success &&
sha1ModeConfigurations[j] == SHA1Mode::ImportedRoot) {
bool isBuiltInRoot = false;
rv = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
if (rv != Success) {
break;
}
if (isBuiltInRoot) {
rv = Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
}
sha1ModeConfigurations[j] == SHA1Mode::ImportedRoot &&
trustDomain.GetIsBuiltChainRootBuiltInRoot()) {
rv = Result::ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED;
}
if (rv == Success) {
if (keySizeStatus) {
@ -778,6 +769,10 @@ Result CertVerifier::VerifyCert(
if (rv != Success) {
break;
}
if (isBuiltChainRootBuiltInRoot) {
*isBuiltChainRootBuiltInRoot =
trustDomain.GetIsBuiltChainRootBuiltInRoot();
}
}
}
}
@ -904,12 +899,12 @@ Result CertVerifier::VerifySSLServerCert(
/*optional out*/ SHA1ModeResult* sha1ModeResult,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo,
/*optional out*/ CertificateTransparencyInfo* ctInfo,
/*optional out*/ bool* isBuiltCertChainRootBuiltInRoot) {
/*optional out*/ bool* isBuiltChainRootBuiltInRoot) {
// XXX: MOZ_ASSERT(pinarg);
MOZ_ASSERT(!hostname.IsEmpty());
if (isBuiltCertChainRootBuiltInRoot) {
*isBuiltCertChainRootBuiltInRoot = false;
if (isBuiltChainRootBuiltInRoot) {
*isBuiltChainRootBuiltInRoot = false;
}
if (evStatus) {
@ -928,11 +923,13 @@ Result CertVerifier::VerifySSLServerCert(
if (rv != Success) {
return rv;
}
bool isBuiltChainRootBuiltInRootLocal;
rv = VerifyCert(peerCertBytes, certificateUsageSSLServer, time, pinarg,
PromiseFlatCString(hostname).get(), builtChain, flags,
extraCertificates, stapledOCSPResponse, sctsFromTLS,
originAttributes, evStatus, ocspStaplingStatus, keySizeStatus,
sha1ModeResult, pinningTelemetryInfo, ctInfo);
sha1ModeResult, pinningTelemetryInfo, ctInfo,
&isBuiltChainRootBuiltInRootLocal);
if (rv != Success) {
// we don't use the certificate for path building, so this parameter doesn't
// matter
@ -1007,19 +1004,11 @@ Result CertVerifier::VerifySSLServerCert(
if (rv != Success) {
return Result::FATAL_ERROR_INVALID_ARGS;
}
bool isBuiltInRoot;
rv = IsCertChainRootBuiltInRoot(builtChain, isBuiltInRoot);
if (rv != Success) {
return rv;
}
if (isBuiltCertChainRootBuiltInRoot) {
*isBuiltCertChainRootBuiltInRoot = isBuiltInRoot;
}
BRNameMatchingPolicy nameMatchingPolicy(
isBuiltInRoot ? mNameMatchingMode
: BRNameMatchingPolicy::Mode::DoNotEnforce);
isBuiltChainRootBuiltInRootLocal
? mNameMatchingMode
: BRNameMatchingPolicy::Mode::DoNotEnforce);
rv = CheckCertHostname(peerCertInput, hostnameInput, nameMatchingPolicy);
if (rv != Success) {
// Treat malformed name information as a domain mismatch.
@ -1030,6 +1019,10 @@ Result CertVerifier::VerifySSLServerCert(
return rv;
}
if (isBuiltChainRootBuiltInRoot) {
*isBuiltChainRootBuiltInRoot = isBuiltChainRootBuiltInRootLocal;
}
return Success;
}

View File

@ -172,7 +172,8 @@ class CertVerifier {
/*optional out*/ KeySizeStatus* keySizeStatus = nullptr,
/*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr,
/*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr);
/*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr,
/*optional out*/ bool* isBuiltChainRootBuiltInRoot = nullptr);
mozilla::pkix::Result VerifySSLServerCert(
const nsTArray<uint8_t>& peerCert, mozilla::pkix::Time time, void* pinarg,
@ -193,7 +194,7 @@ class CertVerifier {
/*optional out*/ SHA1ModeResult* sha1ModeResult = nullptr,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo = nullptr,
/*optional out*/ CertificateTransparencyInfo* ctInfo = nullptr,
/*optional out*/ bool* isBuiltCertChainRootBuiltInRoot = nullptr);
/*optional out*/ bool* isBuiltChainRootBuiltInRoot = nullptr);
enum class SHA1Mode {
Allowed = 0,

View File

@ -97,6 +97,7 @@ NSSCertDBTrustDomain::NSSCertDBTrustDomain(
mThirdPartyIntermediateInputs(thirdPartyIntermediateInputs),
mExtraCertificates(extraCertificates),
mBuiltChain(builtChain),
mIsBuiltChainRootBuiltInRoot(false),
mPinningTelemetryInfo(pinningTelemetryInfo),
mHostname(hostname),
mCertStorage(do_GetService(NS_CERT_STORAGE_CID)),
@ -1260,6 +1261,36 @@ nsresult isDistrustedCertificateChain(
return NS_OK;
}
// This is used by NSSCertDBTrustDomain to ensure IsCertBuiltInRoot is only
// called from the socket thread during TLS server certificate verification.
Result IsCertBuiltInRootWithSyncDispatch(Input certInput, bool& result) {
nsCOMPtr<nsIEventTarget> socketThread(
do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID));
if (!socketThread) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
bool onSocketThread = true;
nsresult rv = socketThread->IsOnCurrentThread(&onSocketThread);
if (NS_FAILED(rv) || onSocketThread) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
result = false;
Result runnableRV = Result::FATAL_ERROR_LIBRARY_FAILURE;
RefPtr<Runnable> isBuiltInRootTask = NS_NewRunnableFunction(
"IsCertBuiltInRoot",
[&]() { runnableRV = IsCertBuiltInRoot(certInput, result); });
rv = SyncRunnable::DispatchToThread(socketThread, isBuiltInRootTask);
if (NS_FAILED(rv)) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
if (runnableRV != Success) {
return runnableRV;
}
return Success;
}
Result NSSCertDBTrustDomain::IsChainValid(const DERArray& reversedDERArray,
Time time,
const CertPolicyId& requiredPolicy) {
@ -1276,15 +1307,14 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& reversedDERArray,
certArray.EmplaceBack(derInput->UnsafeGetData(), derInput->GetLength());
}
bool isBuiltInRoot = false;
const nsTArray<uint8_t>& rootBytes = certArray.LastElement();
Input rootInput;
Result rv = rootInput.Init(rootBytes.Elements(), rootBytes.Length());
if (rv != Success) {
return rv;
}
rv = IsCertBuiltInRoot(rootInput, isBuiltInRoot);
rv = IsCertBuiltInRootWithSyncDispatch(rootInput,
mIsBuiltChainRootBuiltInRoot);
if (rv != Result::Success) {
return rv;
}
@ -1299,8 +1329,8 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& reversedDERArray,
bool chainHasValidPins;
nsrv = PublicKeyPinningService::ChainHasValidPins(
derCertSpanList, mHostname, time, isBuiltInRoot, chainHasValidPins,
mPinningTelemetryInfo);
derCertSpanList, mHostname, time, mIsBuiltChainRootBuiltInRoot,
chainHasValidPins, mPinningTelemetryInfo);
if (NS_FAILED(nsrv)) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
@ -1311,7 +1341,7 @@ Result NSSCertDBTrustDomain::IsChainValid(const DERArray& reversedDERArray,
// Check that the childs' certificate NotBefore date is anterior to
// the NotAfter value of the parent when the root is a builtin.
if (isBuiltInRoot) {
if (mIsBuiltChainRootBuiltInRoot) {
bool isDistrusted;
nsrv =
isDistrustedCertificateChain(certArray, mCertDBTrustType, isDistrusted);
@ -1501,6 +1531,7 @@ void NSSCertDBTrustDomain::ResetAccumulatedState() {
mSCTListFromOCSPStapling = nullptr;
mSCTListFromCertificate = nullptr;
mSawDistrustedCAByPolicyError = false;
mIsBuiltChainRootBuiltInRoot = false;
}
static Input SECItemToInput(const UniqueSECItem& item) {
@ -1524,6 +1555,10 @@ Input NSSCertDBTrustDomain::GetSCTListFromOCSPStapling() const {
return SECItemToInput(mSCTListFromOCSPStapling);
}
bool NSSCertDBTrustDomain::GetIsBuiltChainRootBuiltInRoot() const {
return mIsBuiltChainRootBuiltInRoot;
}
bool NSSCertDBTrustDomain::GetIsErrorDueToDistrustedCAPolicy() const {
return mSawDistrustedCAByPolicyError;
}

View File

@ -212,6 +212,8 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain {
mozilla::pkix::Input GetSCTListFromCertificate() const;
mozilla::pkix::Input GetSCTListFromOCSPStapling() const;
bool GetIsBuiltChainRootBuiltInRoot() const;
bool GetIsErrorDueToDistrustedCAPolicy() const;
private:
@ -263,6 +265,7 @@ class NSSCertDBTrustDomain : public mozilla::pkix::TrustDomain {
mThirdPartyIntermediateInputs; // non-owning
const Maybe<nsTArray<nsTArray<uint8_t>>>& mExtraCertificates; // non-owning
nsTArray<nsTArray<uint8_t>>& mBuiltChain; // non-owning
bool mIsBuiltChainRootBuiltInRoot;
PinningTelemetryInfo* mPinningTelemetryInfo;
const char* mHostname; // non-owning - only used for pinning checks
nsCOMPtr<nsICertStorage> mCertStorage;

View File

@ -522,12 +522,13 @@ static SECStatus BlockServerCertChangeForSpdy(
// Gather telemetry on whether the end-entity cert for a server has the
// required TLS Server Authentication EKU, or any others
void GatherEKUTelemetry(const UniqueCERTCertList& certList) {
MOZ_ASSERT(!CERT_LIST_EMPTY(certList));
if (CERT_LIST_EMPTY(certList)) {
return;
}
CERTCertListNode* endEntityNode = CERT_LIST_HEAD(certList);
CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);
MOZ_ASSERT(!(CERT_LIST_END(endEntityNode, certList) ||
CERT_LIST_END(rootNode, certList)));
if (CERT_LIST_END(endEntityNode, certList) ||
CERT_LIST_END(rootNode, certList)) {
MOZ_ASSERT(endEntityNode);
if (!endEntityNode) {
return;
}
CERTCertificate* endEntityCert = endEntityNode->cert;
@ -536,23 +537,6 @@ void GatherEKUTelemetry(const UniqueCERTCertList& certList) {
return;
}
// Only log telemetry if the root CA is built-in
CERTCertificate* rootCert = rootNode->cert;
MOZ_ASSERT(rootCert);
if (!rootCert) {
return;
}
bool isBuiltIn = false;
Input rootInput;
Result rv = rootInput.Init(rootCert->derCert.data, rootCert->derCert.len);
if (rv != Result::Success) {
return;
}
rv = IsCertBuiltInRoot(rootInput, isBuiltIn);
if (rv != Success || !isBuiltIn) {
return;
}
// Find the EKU extension, if present
bool foundEKU = false;
SECOidTag oidTag;
@ -628,8 +612,12 @@ void GatherRootCATelemetry(const UniqueCERTCertList& certList) {
// There are various things that we want to measure about certificate
// chains that we accept. This is a single entry point for all of them.
void GatherSuccessfulValidationTelemetry(const UniqueCERTCertList& certList) {
GatherEKUTelemetry(certList);
void GatherSuccessfulValidationTelemetry(const UniqueCERTCertList& certList,
bool isCertListRootBuiltInRoot) {
if (isCertListRootBuiltInRoot) {
// Only gather this telemetry if the root CA is built-in
GatherEKUTelemetry(certList);
}
GatherRootCATelemetry(certList);
}
@ -765,6 +753,7 @@ static void CollectCertTelemetry(
KeySizeStatus aKeySizeStatus, SHA1ModeResult aSha1ModeResult,
const PinningTelemetryInfo& aPinningTelemetryInfo,
const nsTArray<nsTArray<uint8_t>>& aBuiltCertChain,
bool aIsBuiltCertChainRootBuiltInRoot,
const CertificateTransparencyInfo& aCertificateTransparencyInfo) {
UniqueCERTCertList builtCertChainList(CERT_NewCertList());
if (!builtCertChainList) {
@ -818,7 +807,8 @@ static void CollectCertTelemetry(
}
if (aCertVerificationResult == Success) {
GatherSuccessfulValidationTelemetry(builtCertChainList);
GatherSuccessfulValidationTelemetry(builtCertChainList,
aIsBuiltCertChainRootBuiltInRoot);
GatherCertificateTransparencyTelemetry(builtCertChainList,
aEVStatus == EVStatus::EV,
aCertificateTransparencyInfo);
@ -830,7 +820,7 @@ static void AuthCertificateSetResults(
nsTArray<nsTArray<uint8_t>>&& aBuiltCertChain,
nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
uint16_t aCertificateTransparencyStatus, EVStatus aEvStatus,
bool aSucceeded, bool aIsCertChainRootBuiltInRoot) {
bool aSucceeded, bool aIsBuiltCertChainRootBuiltInRoot) {
MOZ_ASSERT(aInfoObject);
if (aSucceeded) {
// Certificate verification succeeded. Delete any potential record of
@ -844,7 +834,7 @@ static void AuthCertificateSetResults(
("AuthCertificate setting NEW cert %p", aCert));
aInfoObject->SetIsBuiltCertChainRootBuiltInRoot(
aIsCertChainRootBuiltInRoot);
aIsBuiltCertChainRootBuiltInRoot);
aInfoObject->SetCertificateTransparencyStatus(
aCertificateTransparencyStatus);
} else {
@ -867,7 +857,7 @@ Result AuthCertificate(
/*out*/ nsTArray<nsTArray<uint8_t>>& builtCertChain,
/*out*/ EVStatus& evStatus,
/*out*/ CertificateTransparencyInfo& certificateTransparencyInfo,
/*out*/ bool& aIsCertChainRootBuiltInRoot) {
/*out*/ bool& aIsBuiltCertChainRootBuiltInRoot) {
CertVerifier::OCSPStaplingStatus ocspStaplingStatus =
CertVerifier::OCSP_STAPLING_NEVER_CHECKED;
KeySizeStatus keySizeStatus = KeySizeStatus::NeverChecked;
@ -889,10 +879,11 @@ Result AuthCertificate(
sctsFromTLSExtension, dcInfo, aOriginAttributes, &evStatus,
&ocspStaplingStatus, &keySizeStatus, &sha1ModeResult,
&pinningTelemetryInfo, &certificateTransparencyInfo,
&aIsCertChainRootBuiltInRoot);
&aIsBuiltCertChainRootBuiltInRoot);
CollectCertTelemetry(rv, evStatus, ocspStaplingStatus, keySizeStatus,
sha1ModeResult, pinningTelemetryInfo, builtCertChain,
aIsBuiltCertChainRootBuiltInRoot,
certificateTransparencyInfo);
return rv;
@ -1368,7 +1359,7 @@ void SSLServerCertVerificationResult::Dispatch(
nsTArray<nsTArray<uint8_t>>&& aPeerCertChain,
uint16_t aCertificateTransparencyStatus, EVStatus aEVStatus,
bool aSucceeded, PRErrorCode aFinalError, uint32_t aCollectedErrors,
bool aIsCertChainRootBuiltInRoot, uint32_t aProviderFlags) {
bool aIsBuiltCertChainRootBuiltInRoot, uint32_t aProviderFlags) {
mCert = aCert;
mBuiltChain = std::move(aBuiltChain);
mPeerCertChain = std::move(aPeerCertChain);
@ -1377,7 +1368,7 @@ void SSLServerCertVerificationResult::Dispatch(
mSucceeded = aSucceeded;
mFinalError = aFinalError;
mCollectedErrors = aCollectedErrors;
mIsBuiltCertChainRootBuiltInRoot = aIsCertChainRootBuiltInRoot;
mIsBuiltCertChainRootBuiltInRoot = aIsBuiltCertChainRootBuiltInRoot;
mProviderFlags = aProviderFlags;
nsresult rv;