[tor-commits] [tor-browser/tor-browser-45.2.0esr-6.5-1] Bug 1233328 - Part 1: Ignore SHA-1 pins in PublicKeyPinningService.cpp. r=keeler
gk at torproject.org
gk at torproject.org
Fri Jun 3 20:52:32 UTC 2016
commit cc0fcf624033578259dab28ccaaa90bbd85d3a12
Author: Cykesiopka <cykesiopka.bmo at gmail.com>
Date: Wed Jan 20 20:40:01 2016 -0800
Bug 1233328 - Part 1: Ignore SHA-1 pins in PublicKeyPinningService.cpp. r=keeler
---
security/manager/ssl/PublicKeyPinningService.cpp | 88 ++++++++----------------
security/manager/ssl/PublicKeyPinningService.h | 18 +++--
2 files changed, 35 insertions(+), 71 deletions(-)
diff --git a/security/manager/ssl/PublicKeyPinningService.cpp b/security/manager/ssl/PublicKeyPinningService.cpp
index 073b3db..7fa7bf7 100644
--- a/security/manager/ssl/PublicKeyPinningService.cpp
+++ b/security/manager/ssl/PublicKeyPinningService.cpp
@@ -31,12 +31,11 @@ PRLogModuleInfo* gPublicKeyPinningLog =
of the DER Encoded subject Public Key Info for the given cert
*/
static nsresult
-GetBase64HashSPKI(const CERTCertificate* cert, SECOidTag hashType,
- nsACString& hashSPKIDigest)
+GetBase64HashSPKI(const CERTCertificate* cert, nsACString& hashSPKIDigest)
{
hashSPKIDigest.Truncate();
Digest digest;
- nsresult rv = digest.DigestBuf(hashType, cert->derPublicKey.data,
+ nsresult rv = digest.DigestBuf(SEC_OID_SHA256, cert->derPublicKey.data,
cert->derPublicKey.len);
if (NS_FAILED(rv)) {
return rv;
@@ -48,24 +47,23 @@ GetBase64HashSPKI(const CERTCertificate* cert, SECOidTag hashType,
}
/*
- * Returns true if a given cert matches any hashType fingerprints from the
- * given pinset or the dynamicFingeprints array, false otherwise.
+ * Sets certMatchesPinset to true if a given cert matches any fingerprints from
+ * the given pinset or the dynamicFingerprints array, or to false otherwise.
*/
static nsresult
-EvalCertWithHashType(const CERTCertificate* cert, SECOidTag hashType,
- const StaticFingerprints* fingerprints,
- const nsTArray<nsCString>* dynamicFingerprints,
- /*out*/ bool& certMatchesPinset)
+EvalCert(const CERTCertificate* cert, const StaticFingerprints* fingerprints,
+ const nsTArray<nsCString>* dynamicFingerprints,
+ /*out*/ bool& certMatchesPinset)
{
certMatchesPinset = false;
if (!fingerprints && !dynamicFingerprints) {
MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
- ("pkpin: No hashes found for hash type: %d\n", hashType));
+ ("pkpin: No hashes found\n"));
return NS_ERROR_INVALID_ARG;
}
nsAutoCString base64Out;
- nsresult rv = GetBase64HashSPKI(cert, hashType, base64Out);
+ nsresult rv = GetBase64HashSPKI(cert, base64Out);
if (NS_FAILED(rv)) {
MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
("pkpin: GetBase64HashSPKI failed!\n"));
@@ -96,30 +94,25 @@ EvalCertWithHashType(const CERTCertificate* cert, SECOidTag hashType,
}
/*
- * Returns true if a given chain matches any hashType fingerprints from the
- * given pinset or the dynamicFingerprints array, false otherwise.
+ * Sets certListIntersectsPinset to true if a given chain matches any
+ * fingerprints from the given pinset or the dynamicFingerprints array, or to
+ * false otherwise.
*/
static nsresult
-EvalChainWithHashType(const CERTCertList* certList, SECOidTag hashType,
- const StaticPinset* pinset,
- const nsTArray<nsCString>* dynamicFingerprints,
- /*out*/ bool& certListIntersectsPinset)
+EvalChain(const CERTCertList* certList, const StaticPinset* pinset,
+ const nsTArray<nsCString>* dynamicFingerprints,
+ /*out*/ bool& certListIntersectsPinset)
{
certListIntersectsPinset = false;
CERTCertificate* currentCert;
const StaticFingerprints* fingerprints = nullptr;
if (pinset) {
- if (hashType == SEC_OID_SHA256) {
- fingerprints = pinset->sha256;
- } else if (hashType == SEC_OID_SHA1) {
- fingerprints = pinset->sha1;
- }
+ fingerprints = pinset->sha256;
}
- // This can happen if dynamicFingerprints is null and the static pinset
- // doesn't have any pins of this hash type.
if (!fingerprints && !dynamicFingerprints) {
- return NS_OK;
+ MOZ_ASSERT(false, "Must pass in at least one type of pinset");
+ return NS_ERROR_FAILURE;
}
CERTCertListNode* node;
@@ -130,9 +123,8 @@ EvalChainWithHashType(const CERTCertList* certList, SECOidTag hashType,
("pkpin: certArray subject: '%s'\n", currentCert->subjectName));
MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
("pkpin: certArray issuer: '%s'\n", currentCert->issuerName));
- nsresult rv = EvalCertWithHashType(currentCert, hashType, fingerprints,
- dynamicFingerprints,
- certListIntersectsPinset);
+ nsresult rv = EvalCert(currentCert, fingerprints, dynamicFingerprints,
+ certListIntersectsPinset);
if (NS_FAILED(rv)) {
return rv;
}
@@ -145,29 +137,6 @@ EvalChainWithHashType(const CERTCertList* certList, SECOidTag hashType,
}
/**
- * Given a pinset and certlist, return true if one of the certificates on
- * the list matches a fingerprint in the pinset, false otherwise.
- */
-static nsresult
-EvalChainWithPinset(const CERTCertList* certList,
- const StaticPinset* pinset,
- /*out*/ bool& certListIntersectsPinset)
-{
- certListIntersectsPinset = false;
- // SHA256 is more trustworthy, try that first.
- nsresult rv = EvalChainWithHashType(certList, SEC_OID_SHA256, pinset,
- nullptr, certListIntersectsPinset);
- if (NS_FAILED(rv)) {
- return rv;
- }
- if (certListIntersectsPinset) {
- return NS_OK;
- }
- return EvalChainWithHashType(certList, SEC_OID_SHA1, pinset, nullptr,
- certListIntersectsPinset);
-}
-
-/**
Comparator for the is public key pinned host.
*/
static int
@@ -184,8 +153,7 @@ PublicKeyPinningService::ChainMatchesPinset(const CERTCertList* certList,
const nsTArray<nsCString>& aSHA256keys,
/*out*/ bool& chainMatchesPinset)
{
- return EvalChainWithHashType(certList, SEC_OID_SHA256, nullptr, &aSHA256keys,
- chainMatchesPinset);
+ return EvalChain(certList, nullptr, &aSHA256keys, chainMatchesPinset);
}
// Returns via one of the output parameters the most relevant pinning
@@ -206,10 +174,9 @@ FindPinningInformation(const char* hostname, mozilla::pkix::Time time,
if (!sssService) {
return NS_ERROR_FAILURE;
}
- SiteHPKPState dynamicEntry;
- TransportSecurityPreload *foundEntry = nullptr;
- char *evalHost = const_cast<char*>(hostname);
- char *evalPart;
+ TransportSecurityPreload* foundEntry = nullptr;
+ char* evalHost = const_cast<char*>(hostname);
+ char* evalPart;
// Notice how the (xx = strchr) prevents pins for unqualified domain names.
while (!foundEntry && (evalPart = strchr(evalHost, '.'))) {
MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
@@ -293,13 +260,12 @@ CheckPinsForHostname(const CERTCertList* certList, const char* hostname,
return NS_OK;
}
if (dynamicFingerprints.Length() > 0) {
- return EvalChainWithHashType(certList, SEC_OID_SHA256, nullptr,
- &dynamicFingerprints, chainHasValidPins);
+ return EvalChain(certList, nullptr, &dynamicFingerprints, chainHasValidPins);
}
if (staticFingerprints) {
bool enforceTestModeResult;
- rv = EvalChainWithPinset(certList, staticFingerprints->pinset,
- enforceTestModeResult);
+ rv = EvalChain(certList, staticFingerprints->pinset, nullptr,
+ enforceTestModeResult);
if (NS_FAILED(rv)) {
return rv;
}
diff --git a/security/manager/ssl/PublicKeyPinningService.h b/security/manager/ssl/PublicKeyPinningService.h
index 9a02ab9..601f624 100644
--- a/security/manager/ssl/PublicKeyPinningService.h
+++ b/security/manager/ssl/PublicKeyPinningService.h
@@ -18,13 +18,11 @@ class PublicKeyPinningService
{
public:
/**
- * Returns true if the given (host, certList) passes pinning checks,
- * false otherwise. If the host is pinned, return true if one of the keys in
- * the given certificate chain matches the pin set specified by the
- * hostname. If the hostname is null or empty evaluate against all the
- * possible names for the EE cert (Common Name (CN) plus all DNS Name:
- * subject Alt Name entries). The certList's head is the EE cert and the
- * tail is the trust anchor.
+ * 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
*/
@@ -35,9 +33,9 @@ public:
/*out*/ bool& chainHasValidPins,
/*optional out*/ PinningTelemetryInfo* pinningTelemetryInfo);
/**
- * Returns true if there is any intersection between the certificate list
- * and the pins specified in the aSHA256key array. Values passed in are
- * assumed to be in base64 encoded form
+ * 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 CERTCertList* certList,
const nsTArray<nsCString>& aSHA256keys,
More information about the tor-commits
mailing list