Bug 1736763 - correctly delimit ipv6 hostnames for keying certificate overrides r=keeler

Differential Revision: https://phabricator.services.mozilla.com/D136499

Author: R. Martinho Fernandes

security/manager/ssl/nsCertOverrideService.cpp

@@ -324,12 +324,10 @@ nsresult nsCertOverrideService::Read(const MutexAutoLock& aProofOfLock) {
     Tokenizer parser(buffer);
     nsDependentCSubstring host;
     if (parser.CheckChar('[')) {  // this is a IPv6 address
-      parser.Record(Tokenizer::INCLUDE_LAST);
       if (!parser.ReadUntil(Tokenizer::Token::Char(']'), host) ||
           host.Length() == 0 || !parser.CheckChar(':')) {
         continue;
       }
-      parser.Claim(host);
     } else if (!parser.ReadUntil(Tokenizer::Token::Char(':'), host) ||
                host.Length() == 0) {
       continue;
@@ -818,7 +816,16 @@ nsCertOverrideService::GetOverrides(
 void nsCertOverrideService::GetHostWithPort(const nsACString& aHostName,
                                             int32_t aPort,
                                             nsACString& aRetval) {
-  nsAutoCString hostPort(aHostName);
+  nsAutoCString hostPort;
+  if (aHostName.Contains(':')) {
+    // if aHostName is an IPv6 address, add brackets to match the internal
+    // representation, which always stores IPv6 addresses with brackets
+    hostPort.Append('[');
+    hostPort.Append(aHostName);
+    hostPort.Append(']');
+  } else {
+    hostPort.Append(aHostName);
+  }
   if (aPort == -1) {
     aPort = 443;
   }

security/manager/ssl/tests/unit/test_cert_override_read.js

@@ -134,7 +134,7 @@ function run_test() {
       attributes: {},
     },
     {
-      host: "[::1]",
+      host: "::1",
       port: 443,
       cert: cert2,
       bits: Ci.nsICertOverrideService.ERROR_MISMATCH,

security/manager/ssl/tests/unit/test_cert_overrides.js

@@ -566,6 +566,14 @@ function add_simple_tests() {
       expectedBits,
       false
     );
+    certOverrideService.rememberValidityOverride(
+      "::1",
+      80,
+      {},
+      cert,
+      expectedBits,
+      false
+    );
     Assert.ok(
       certOverrideService.hasMatchingOverride(
         "example.com",
@@ -596,6 +604,10 @@ function add_simple_tests() {
       ),
       "Should have added override for example.org:443"
     );
+    Assert.ok(
+      certOverrideService.hasMatchingOverride("::1", 80, {}, cert, {}, {}),
+      "Should have added override for [::1]:80"
+    );
     Assert.ok(
       !certOverrideService.hasMatchingOverride(
         "example.org",