bug 1065909 - canonicalize hostnames in nsSiteSecurityService and PublicKeyPinningService r=mmc
--- a/security/manager/boot/src/PublicKeyPinningService.cpp
+++ b/security/manager/boot/src/PublicKeyPinningService.cpp
@@ -360,10 +360,23 @@ PublicKeyPinningService::ChainHasValidPi
bool enforceTestMode)
{
if (!certList) {
return false;
}
if (!hostname || hostname[0] == 0) {
return CheckChainAgainstAllNames(certList, enforceTestMode, time);
}
- return CheckPinsForHostname(certList, hostname, enforceTestMode, time);
+ nsAutoCString canonicalizedHostname(CanonicalizeHostname(hostname));
+ return CheckPinsForHostname(certList, canonicalizedHostname.get(),
+ enforceTestMode, time);
}
+
+nsAutoCString
+PublicKeyPinningService::CanonicalizeHostname(const char* hostname)
+{
+ nsAutoCString canonicalizedHostname(hostname);
+ ToLowerCase(canonicalizedHostname);
+ while (canonicalizedHostname.Last() == '.') {
+ canonicalizedHostname.Truncate(canonicalizedHostname.Length() - 1);
+ }
+ return canonicalizedHostname;
+}
--- a/security/manager/boot/src/PublicKeyPinningService.h
+++ b/security/manager/boot/src/PublicKeyPinningService.h
@@ -33,13 +33,20 @@ public:
bool enforceTestMode);
/**
* 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
*/
static bool ChainMatchesPinset(const CERTCertList* certList,
const nsTArray<nsCString>& aSHA256keys);
+
+ /**
+ * Given a hostname of potentially mixed case with potentially multiple
+ * trailing '.' (see bug 1118522), canonicalizes it to lowercase with no
+ * trailing '.'.
+ */
+ static nsAutoCString CanonicalizeHostname(const char* hostname);
};
}} // namespace mozilla::psm
#endif // PublicKeyPinningServiceService_h
--- a/security/manager/boot/src/nsSiteSecurityService.cpp
+++ b/security/manager/boot/src/nsSiteSecurityService.cpp
@@ -265,25 +265,31 @@ nsSiteSecurityService::Init()
if (!storageWillPersist) {
NS_WARNING("site security information will not be persisted");
}
return NS_OK;
}
nsresult
-nsSiteSecurityService::GetHost(nsIURI *aURI, nsACString &aResult)
+nsSiteSecurityService::GetHost(nsIURI* aURI, nsACString& aResult)
{
nsCOMPtr<nsIURI> innerURI = NS_GetInnermostURI(aURI);
- if (!innerURI) return NS_ERROR_FAILURE;
+ if (!innerURI) {
+ return NS_ERROR_FAILURE;
+ }
- nsresult rv = innerURI->GetAsciiHost(aResult);
+ nsAutoCString host;
+ nsresult rv = innerURI->GetAsciiHost(host);
- if (NS_FAILED(rv) || aResult.IsEmpty())
+ if (NS_FAILED(rv) || host.IsEmpty()) {
return NS_ERROR_UNEXPECTED;
+ }
+
+ aResult.Assign(PublicKeyPinningService::CanonicalizeHostname(host.get()));
return NS_OK;
}
static void
SetStorageKey(nsAutoCString& storageKey, nsCString& hostname, uint32_t aType)
{
storageKey = hostname;
@@ -858,18 +864,17 @@ nsSiteSecurityService::IsSecureHost(uint
*aResult = !PublicKeyPinningService::ChainHasValidPins(certList,
aHost,
mozilla::pkix::Now(),
false);
return NS_OK;
}
// Holepunch chart.apis.google.com and subdomains.
- nsAutoCString host(aHost);
- ToLowerCase(host);
+ nsAutoCString host(PublicKeyPinningService::CanonicalizeHostname(aHost));
if (host.EqualsLiteral("chart.apis.google.com") ||
StringEndsWith(host, NS_LITERAL_CSTRING(".chart.apis.google.com"))) {
return NS_OK;
}
const nsSTSPreload *preload = nullptr;
// First check the exact host. This involves first checking for an entry in
@@ -979,17 +984,17 @@ nsSiteSecurityService::GetKeyPinsForHost
NS_ENSURE_ARG(afound);
NS_ENSURE_ARG(aHostname);
SSSLOG(("Top of GetKeyPinsForHostname"));
*afound = false;
*aIncludeSubdomains = false;
pinArray.Clear();
- nsAutoCString host(aHostname);
+ nsAutoCString host(PublicKeyPinningService::CanonicalizeHostname(aHostname));
nsAutoCString storageKey;
SetStorageKey(storageKey, host, nsISiteSecurityService::HEADER_HPKP);
SSSLOG(("storagekey '%s'\n", storageKey.get()));
mozilla::DataStorageType storageType = mozilla::DataStorage_Persistent;
nsCString value = mSiteStateStorage->Get(storageKey, storageType);
// decode now
SiteHPKPState foundEntry(value);
@@ -1032,17 +1037,18 @@ nsSiteSecurityService::SetKeyPins(const
if (!stringIsBase64EncodingOf256bitValue(pin)) {
return NS_ERROR_INVALID_ARG;
}
sha256keys.AppendElement(pin);
}
SiteHPKPState dynamicEntry(expireTime, SecurityPropertySet,
aIncludeSubdomains, sha256keys);
// we always store data in permanent storage (ie no flags)
- return SetHPKPState(aHost, dynamicEntry, 0);
+ nsAutoCString host(PublicKeyPinningService::CanonicalizeHostname(aHost));
+ return SetHPKPState(host.get(), dynamicEntry, 0);
}
nsresult
nsSiteSecurityService::SetHPKPState(const char* aHost, SiteHPKPState& entry,
uint32_t aFlags)
{
SSSLOG(("Top of SetPKPState"));
nsAutoCString host(aHost);
--- a/security/manager/ssl/tests/unit/test_pinning.js
+++ b/security/manager/ssl/tests/unit/test_pinning.js
@@ -42,16 +42,23 @@ function test_strict() {
// precedence. This prevents overrides for such hosts.
add_connection_test("unknownissuer.include-subdomains.pinning.example.com",
getXPCOMStatusFromNSS(MOZILLA_PKIX_ERROR_KEY_PINNING_FAILURE));
// Issued by otherCA, which is not in the pinset for pinning.example.com.
add_connection_test("bad.include-subdomains.pinning.example.com",
getXPCOMStatusFromNSS(MOZILLA_PKIX_ERROR_KEY_PINNING_FAILURE));
+ // Check that using a FQDN doesn't bypass pinning.
+ add_connection_test("bad.include-subdomains.pinning.example.com.",
+ getXPCOMStatusFromNSS(MOZILLA_PKIX_ERROR_KEY_PINNING_FAILURE));
+ // For some reason this is also navigable (see bug 1118522).
+ add_connection_test("bad.include-subdomains.pinning.example.com..",
+ getXPCOMStatusFromNSS(MOZILLA_PKIX_ERROR_KEY_PINNING_FAILURE));
+
// These domains serve certs that match the pinset.
add_connection_test("include-subdomains.pinning.example.com", Cr.NS_OK);
add_connection_test("good.include-subdomains.pinning.example.com", Cr.NS_OK);
add_connection_test("exclude-subdomains.pinning.example.com", Cr.NS_OK);
// This domain serves a cert that doesn't match the pinset, but subdomains
// are excluded.
add_connection_test("sub.exclude-subdomains.pinning.example.com", Cr.NS_OK);
@@ -138,17 +145,17 @@ function test_enforce_test_mode() {
function check_pinning_telemetry() {
let service = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry);
let prod_histogram = service.getHistogramById("CERT_PINNING_RESULTS")
.snapshot();
let test_histogram = service.getHistogramById("CERT_PINNING_TEST_RESULTS")
.snapshot();
// Because all of our test domains are pinned to user-specified trust
// anchors, effectively only strict mode and enforce test-mode get evaluated
- do_check_eq(prod_histogram.counts[0], 4); // Failure count
+ do_check_eq(prod_histogram.counts[0], 6); // Failure count
do_check_eq(prod_histogram.counts[1], 4); // Success count
do_check_eq(test_histogram.counts[0], 2); // Failure count
do_check_eq(test_histogram.counts[1], 0); // Success count
let moz_prod_histogram = service.getHistogramById("CERT_PINNING_MOZ_RESULTS")
.snapshot();
let moz_test_histogram =
service.getHistogramById("CERT_PINNING_MOZ_TEST_RESULTS").snapshot();
new file mode 100644
--- /dev/null
+++ b/security/manager/ssl/tests/unit/test_sts_fqdn.js
@@ -0,0 +1,45 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+"use strict";
+
+function run_test() {
+ let SSService = Cc["@mozilla.org/ssservice;1"]
+ .getService(Ci.nsISiteSecurityService);
+ do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com", 0));
+ do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com.", 0));
+ // These cases are only relevant as long as bug 1118522 hasn't been fixed.
+ do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com..", 0));
+
+ let uri = Services.io.newURI("https://example.com", null, null);
+ let sslStatus = new FakeSSLStatus();
+ SSService.processHeader(Ci.nsISiteSecurityService.HEADER_HSTS, uri,
+ "max-age=1000;includeSubdomains", sslStatus, 0);
+ do_check_true(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com", 0));
+ do_check_true(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com.", 0));
+ do_check_true(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com..", 0));
+
+ do_check_true(SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS,
+ uri, 0));
+ uri = Services.io.newURI("https://example.com.", null, null);
+ do_check_true(SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS,
+ uri, 0));
+ uri = Services.io.newURI("https://example.com..", null, null);
+ do_check_true(SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS,
+ uri, 0));
+
+ SSService.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
+ do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com", 0));
+ do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com.", 0));
+ do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
+ "example.com..", 0));
+}
--- a/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
+++ b/security/manager/ssl/tests/unit/tlsserver/cmd/BadCertServer.cpp
@@ -47,16 +47,18 @@ const BadCertHost sBadCertHosts[] =
{ "ca-used-as-end-entity-name-mismatch.example.com", "ca-used-as-end-entity" },
// All of include-subdomains.pinning.example.com is pinned to End Entity
// Test Cert with nick localhostAndExampleCom. Any other nick will only
// pass pinning when security.cert_pinning.enforcement.level != strict and
// otherCA is added as a user-specified trust anchor. See StaticHPKPins.h.
{ "include-subdomains.pinning.example.com", "localhostAndExampleCom" },
{ "good.include-subdomains.pinning.example.com", "localhostAndExampleCom" },
{ "bad.include-subdomains.pinning.example.com", "otherIssuerEE" },
+ { "bad.include-subdomains.pinning.example.com.", "otherIssuerEE" },
+ { "bad.include-subdomains.pinning.example.com..", "otherIssuerEE" },
{ "exclude-subdomains.pinning.example.com", "localhostAndExampleCom" },
{ "sub.exclude-subdomains.pinning.example.com", "otherIssuerEE" },
{ "test-mode.pinning.example.com", "otherIssuerEE" },
{ "unknownissuer.include-subdomains.pinning.example.com", "unknownissuer" },
{ "nsCertTypeNotCritical.example.com", "nsCertTypeNotCritical" },
{ "nsCertTypeCriticalWithExtKeyUsage.example.com", "nsCertTypeCriticalWithExtKeyUsage" },
{ "nsCertTypeCritical.example.com", "nsCertTypeCritical" },
{ "end-entity-issued-by-v1-cert.example.com", "eeIssuedByV1Cert" },
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -23,16 +23,17 @@ support-files =
[test_datasignatureverifier.js]
[test_hash_algorithms.js]
[test_hmac.js]
[test_sts_preloadlist_perwindowpb.js]
[test_sts_preloadlist_selfdestruct.js]
[test_sts_holepunch.js]
[test_sts_ipv4_ipv6.js]
+[test_sts_fqdn.js]
[test_sss_eviction.js]
[test_sss_readstate.js]
[test_sss_readstate_empty.js]
[test_sss_readstate_garbage.js]
[test_sss_readstate_huge.js]
[test_sss_savestate.js]