Bug 1434936 - Rework ChainHasValidPins to use nsNSSCertList r?keeler r?fkiefer
This commit reworks PublicKeyPinningService::ChainHasValidPins and
PublicKeyPinningService::EvalChain to use nsNSSCertList directly. It also
updates nsSiteSecurityService::ProcessPKPHeader. This will be made more
efficient in
Bug 1406854, where the call to VerifySSLServerCert gets replaced
with one to GetSucceededCertChain. (Such a change is premeature now because
before
Bug 731478 lands this would lead to a session resumption regression
causing pins to not be set properly, which is triggered repeatedly in the
xpcshell tests.)
MozReview-Commit-ID: 1l186n1lXLH
--- a/security/certverifier/NSSCertDBTrustDomain.cpp
+++ b/security/certverifier/NSSCertDBTrustDomain.cpp
@@ -815,17 +815,17 @@ NSSCertDBTrustDomain::IsChainValid(const
// If mHostname isn't set, we're not verifying in the context of a TLS
// handshake, so don't verify HPKP in those cases.
if (mHostname && (mPinningMode != CertVerifier::pinningDisabled) &&
!skipPinningChecksBecauseOfMITMMode) {
bool enforceTestMode =
(mPinningMode == CertVerifier::pinningEnforceTestMode);
bool chainHasValidPins;
nsrv = PublicKeyPinningService::ChainHasValidPins(
- certList, mHostname, time, enforceTestMode, mOriginAttributes,
+ nssCertList, mHostname, time, enforceTestMode, mOriginAttributes,
chainHasValidPins, mPinningTelemetryInfo);
if (NS_FAILED(nsrv)) {
return Result::FATAL_ERROR_LIBRARY_FAILURE;
}
if (!chainHasValidPins) {
return Result::ERROR_KEY_PINNING_FAILURE;
}
}
--- a/security/manager/ssl/PublicKeyPinningService.cpp
+++ b/security/manager/ssl/PublicKeyPinningService.cpp
@@ -95,47 +95,51 @@ EvalCert(const CERTCertificate* cert, co
}
/*
* Sets certListIntersectsPinset to true if a given chain matches any
* fingerprints from the given static fingerprints or the
* dynamicFingerprints array, or to false otherwise.
*/
static nsresult
-EvalChain(const UniqueCERTCertList& certList,
+EvalChain(const RefPtr<nsNSSCertList>& certList,
const StaticFingerprints* fingerprints,
const nsTArray<nsCString>* dynamicFingerprints,
/*out*/ bool& certListIntersectsPinset)
{
certListIntersectsPinset = false;
- CERTCertificate* currentCert;
-
if (!fingerprints && !dynamicFingerprints) {
MOZ_ASSERT(false, "Must pass in at least one type of pinset");
return NS_ERROR_FAILURE;
}
- CERTCertListNode* node;
- for (node = CERT_LIST_HEAD(certList); !CERT_LIST_END(node, certList);
- node = CERT_LIST_NEXT(node)) {
- currentCert = node->cert;
- MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
- ("pkpin: certArray subject: '%s'\n", currentCert->subjectName));
- MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
- ("pkpin: certArray issuer: '%s'\n", currentCert->issuerName));
- nsresult rv = EvalCert(currentCert, fingerprints, dynamicFingerprints,
- certListIntersectsPinset);
- if (NS_FAILED(rv)) {
- return rv;
- }
- if (certListIntersectsPinset) {
+ certList->ForEachCertificateInChain(
+ [&certListIntersectsPinset, &fingerprints, &dynamicFingerprints]
+ (nsCOMPtr<nsIX509Cert> aCert, bool aHasMore, /* out */ bool& aContinue) {
+ // We need an owning handle when calling nsIX509Cert::GetCert().
+ UniqueCERTCertificate nssCert(aCert->GetCert());
+ MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
+ ("pkpin: certArray subject: '%s'\n", nssCert->subjectName));
+ MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
+ ("pkpin: certArray issuer: '%s'\n", nssCert->issuerName));
+ nsresult rv = EvalCert(nssCert.get(), fingerprints, dynamicFingerprints,
+ certListIntersectsPinset);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
+ if (certListIntersectsPinset) {
+ aContinue = false;
+ }
return NS_OK;
- }
+ });
+
+ if (!certListIntersectsPinset) {
+ MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug, ("pkpin: no matches found\n"));
}
- MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug, ("pkpin: no matches found\n"));
return NS_OK;
}
class TransportSecurityPreloadBinarySearchComparator
{
public:
explicit TransportSecurityPreloadBinarySearchComparator(
const char* aTargetHost)
@@ -146,17 +150,17 @@ public:
return strcmp(mTargetHost, val.mHost);
}
private:
const char* mTargetHost; // non-owning
};
nsresult
-PublicKeyPinningService::ChainMatchesPinset(const UniqueCERTCertList& certList,
+PublicKeyPinningService::ChainMatchesPinset(const RefPtr<nsNSSCertList>& certList,
const nsTArray<nsCString>& aSHA256keys,
/*out*/ bool& chainMatchesPinset)
{
return EvalChain(certList, nullptr, &aSHA256keys, chainMatchesPinset);
}
// Returns via one of the output parameters the most relevant pinning
// information that is valid for the given host at the given time.
@@ -235,17 +239,17 @@ FindPinningInformation(const char* hostn
}
// Returns true via the output parameter if the given certificate list meets
// pinning requirements for the given host at the given time. It must be the
// case that either there is an intersection between the set of hashes of
// subject public key info data in the list and the most relevant non-expired
// pinset for the host or there is no pinning information for the host.
static nsresult
-CheckPinsForHostname(const UniqueCERTCertList& certList, const char* hostname,
+CheckPinsForHostname(const RefPtr<nsNSSCertList>& certList, const char* hostname,
bool enforceTestMode, mozilla::pkix::Time time,
const OriginAttributes& originAttributes,
/*out*/ bool& chainHasValidPins,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo)
{
chainHasValidPins = false;
if (!certList) {
return NS_ERROR_INVALID_ARG;
@@ -300,21 +304,28 @@ CheckPinsForHostname(const UniqueCERTCer
pinningTelemetryInfo->certPinningResultBucket =
enforceTestModeResult ? 1 : 0;
}
pinningTelemetryInfo->accumulateResult = true;
pinningTelemetryInfo->certPinningResultHistogram = histogram;
}
// We only collect per-CA pinning statistics upon failures.
- CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);
+ nsCOMPtr<nsIX509Cert> rootCert;
+ rv = certList->GetRootCertificate(rootCert);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
// Only log telemetry if the certificate list is non-empty.
- if (!CERT_LIST_END(rootNode, certList)) {
- if (!enforceTestModeResult && pinningTelemetryInfo) {
- int32_t binNumber = RootCABinNumber(&rootNode->cert->derCert);
+ if (rootCert && !enforceTestModeResult && pinningTelemetryInfo) {
+ UniqueCERTCertificate rootCertObj =
+ UniqueCERTCertificate(rootCert.get()->GetCert());
+ if (rootCertObj) {
+ int32_t binNumber = RootCABinNumber(&rootCertObj->derCert);
if (binNumber != ROOT_CERTIFICATE_UNKNOWN ) {
pinningTelemetryInfo->accumulateForRoot = true;
pinningTelemetryInfo->rootBucket = binNumber;
}
}
}
MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
@@ -324,17 +335,17 @@ CheckPinsForHostname(const UniqueCERTCer
hostname, staticFingerprints->mTestMode ? "test" : "production"));
}
return NS_OK;
}
nsresult
PublicKeyPinningService::ChainHasValidPins(
- const UniqueCERTCertList& certList,
+ const RefPtr<nsNSSCertList>& certList,
const char* hostname,
mozilla::pkix::Time time,
bool enforceTestMode,
const OriginAttributes& originAttributes,
/*out*/ bool& chainHasValidPins,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo)
{
chainHasValidPins = false;
--- a/security/manager/ssl/PublicKeyPinningService.h
+++ b/security/manager/ssl/PublicKeyPinningService.h
@@ -3,16 +3,17 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef PublicKeyPinningService_h
#define PublicKeyPinningService_h
#include "CertVerifier.h"
#include "ScopedNSSTypes.h"
#include "cert.h"
+#include "nsNSSCertificate.h"
#include "nsString.h"
#include "nsTArray.h"
#include "pkix/Time.h"
namespace mozilla {
class OriginAttributes;
}
@@ -28,29 +29,29 @@ public:
* Sets chainHasValidPins to true if the given (host, certList) passes pinning
* checks, or to false otherwise. If the host is pinned, returns true via
* chainHasValidPins if one of the keys in the given certificate chain matches
* the pin set specified by the hostname. The certList's head is the EE cert
* and the tail is the trust anchor.
* Note: if an alt name is a wildcard, it won't necessarily find a pinset
* that would otherwise be valid for it
*/
- static nsresult ChainHasValidPins(const UniqueCERTCertList& certList,
+ static nsresult ChainHasValidPins(const RefPtr<nsNSSCertList>& certList,
const char* hostname,
mozilla::pkix::Time time,
bool enforceTestMode,
const OriginAttributes& originAttributes,
/*out*/ bool& chainHasValidPins,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo);
/**
* Sets chainMatchesPinset to true if there is any intersection between the
* certificate list and the pins specified in the aSHA256keys array.
* Values passed in are assumed to be in base64 encoded form.
*/
- static nsresult ChainMatchesPinset(const UniqueCERTCertList& certList,
+ static nsresult ChainMatchesPinset(const RefPtr<nsNSSCertList>& certList,
const nsTArray<nsCString>& aSHA256keys,
/*out*/ bool& chainMatchesPinset);
/**
* Returns true via the output parameter hostHasPins if there is pinning
* information for the given host that is valid at the given time, and false
* otherwise.
*/
--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -1104,16 +1104,17 @@ nsSiteSecurityService::ProcessPKPHeader(
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<nsIX509Cert> cert;
rv = aSSLStatus->GetServerCert(getter_AddRefs(cert));
NS_ENSURE_SUCCESS(rv, rv);
NS_ENSURE_TRUE(cert, NS_ERROR_FAILURE);
UniqueCERTCertificate nssCert(cert->GetCert());
NS_ENSURE_TRUE(nssCert, NS_ERROR_FAILURE);
+ // This use of VerifySSLServerCert should be able to be removed in Bug #1406854
mozilla::pkix::Time now(mozilla::pkix::Now());
UniqueCERTCertList certList;
RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
NS_ENSURE_TRUE(certVerifier, NS_ERROR_UNEXPECTED);
// We don't want this verification to cause any network traffic that would
// block execution. Also, since we don't have access to the original stapled
// OCSP response, we can't enforce this aspect of the TLS Feature extension.
// This is ok, because it will have been enforced when we originally connected
@@ -1129,23 +1130,32 @@ nsSiteSecurityService::ProcessPKPHeader(
certList,
false, // don't store intermediates
flags,
aOriginAttributes)
!= mozilla::pkix::Success) {
return NS_ERROR_FAILURE;
}
- CERTCertListNode* rootNode = CERT_LIST_TAIL(certList);
- if (CERT_LIST_END(rootNode, certList)) {
- return NS_ERROR_FAILURE;
+ // This copy to produce an nsNSSCertList should also be removed in Bug #1406854
+ nsCOMPtr<nsIX509CertList> x509CertList = new nsNSSCertList(Move(certList));
+ if (!x509CertList) {
+ return rv;
}
+
+ RefPtr<nsNSSCertList> nssCertList = x509CertList->GetCertList();
+ nsCOMPtr<nsIX509Cert> rootCert;
+ rv = nssCertList->GetRootCertificate(rootCert);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
bool isBuiltIn = false;
- mozilla::pkix::Result result = IsCertBuiltInRoot(rootNode->cert, isBuiltIn);
- if (result != mozilla::pkix::Success) {
+ rv = rootCert->GetIsBuiltInRoot(&isBuiltIn);
+ if (NS_FAILED(rv)) {
return NS_ERROR_FAILURE;
}
if (!isBuiltIn && !mProcessPKPHeadersFromNonBuiltInRoots) {
if (aFailureResult) {
*aFailureResult = nsISiteSecurityService::ERROR_ROOT_NOT_BUILT_IN;
}
return NS_ERROR_FAILURE;
@@ -1157,17 +1167,17 @@ nsSiteSecurityService::ProcessPKPHeader(
}
// clamp maxAge to the maximum set by pref
if (maxAge > mMaxMaxAge) {
maxAge = mMaxMaxAge;
}
bool chainMatchesPinset;
- rv = PublicKeyPinningService::ChainMatchesPinset(certList, sha256keys,
+ rv = PublicKeyPinningService::ChainMatchesPinset(nssCertList, sha256keys,
chainMatchesPinset);
if (NS_FAILED(rv)) {
return rv;
}
if (!chainMatchesPinset) {
// is invalid
SSSLOG(("SSS: Pins provided by %s are invalid no match with certList\n", host.get()));
if (aFailureResult) {
@@ -1178,17 +1188,17 @@ nsSiteSecurityService::ProcessPKPHeader(
// finally we need to ensure that there is a "backup pin" ie. There must be
// at least one fingerprint hash that does NOT validate against the verified
// chain (Section 2.5 of the spec)
bool hasBackupPin = false;
for (uint32_t i = 0; i < sha256keys.Length(); i++) {
nsTArray<nsCString> singlePin;
singlePin.AppendElement(sha256keys[i]);
- rv = PublicKeyPinningService::ChainMatchesPinset(certList, singlePin,
+ rv = PublicKeyPinningService::ChainMatchesPinset(nssCertList, singlePin,
chainMatchesPinset);
if (NS_FAILED(rv)) {
return rv;
}
if (!chainMatchesPinset) {
hasBackupPin = true;
}
}