Bug 1233328 - Part 1: Ignore SHA-1 pins in PublicKeyPinningService.cpp. r=keeler (47644f2a) · Commits · blackZwork / Tor Browser

security/manager/ssl/PublicKeyPinningService.cpp

+27 −61

Original line number	Diff line number	Diff line
		@@ -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,
		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,12 +94,12 @@ 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,
		EvalChain(const CERTCertList* certList, const StaticPinset* pinset,
		const nsTArray<nsCString>* dynamicFingerprints,
		/out/ bool& certListIntersectsPinset)
		{
		@@ -110,16 +108,11 @@ EvalChainWithHashType(const CERTCertList* certList, SECOidTag hashType,

		const StaticFingerprints* fingerprints = nullptr;
		if (pinset) {
		if (hashType == SEC_OID_SHA256) {
		fingerprints = pinset->sha256;
		} else if (hashType == SEC_OID_SHA1) {
		fingerprints = pinset->sha1;
		}
		}
		// 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,8 +123,7 @@ 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,
		nsresult rv = EvalCert(currentCert, fingerprints, dynamicFingerprints,
		certListIntersectsPinset);
		if (NS_FAILED(rv)) {
		return rv;
		@@ -144,29 +136,6 @@ EvalChainWithHashType(const CERTCertList* certList, SECOidTag hashType,
		return NS_OK;
		}

		/**
		* 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.
		*/
		@@ -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,7 +174,6 @@ 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;
		@@ -293,12 +260,11 @@ 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,
		rv = EvalChain(certList, staticFingerprints->pinset, nullptr,
		enforceTestModeResult);
		if (NS_FAILED(rv)) {
		return rv;

security/manager/ssl/PublicKeyPinningService.h

+8 −10

Original line number	Diff line number	Diff line
		@@ -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,