Bug 1558569. Stop using [array] in nsISiteSecurityService. r=keeler

Differential Revision: https://phabricator.services.mozilla.com/D34568
2019-06-11 21:23:21 +00:00
parent f313cee8d6
commit ee739bd612
7 changed files with 14 additions and 28 deletions
--- a/security/manager/ssl/RemoteSecuritySettings.jsm
+++ b/security/manager/ssl/RemoteSecuritySettings.jsm
@@ -254,7 +254,6 @@ async function updatePinningList({ data: { current: records } }) {
          siteSecurityService.setKeyPins(item.hostName,
            item.includeSubdomains,
            item.expires,
-            pins.length,
            pins, true);
        }
        if (pinType == "STSPin") {
--- a/security/manager/ssl/nsISiteSecurityService.idl
+++ b/security/manager/ssl/nsISiteSecurityService.idl
@@ -264,7 +264,6 @@ interface nsISiteSecurityService : nsISupports
     * @param aHost the hostname (punycode) that pins will apply to
     * @param aIncludeSubdomains whether these pins also apply to subdomains
     * @param aExpires the time this pin should expire (millis since epoch)
-     * @param aPinCount number of keys being pinnned
     * @param aSha256Pins array of hashed key fingerprints (SHA-256, base64)
     * @param aIsPreload are these key pins for a preload entry? (false by
     *        default)
@@ -276,8 +275,7 @@ interface nsISiteSecurityService : nsISupports
     */
    [implicit_jscontext, optional_argc, must_use]
    boolean setKeyPins(in ACString aHost, in boolean aIncludeSubdomains,
-                       in int64_t aExpires, in unsigned long aPinCount,
-                       [array, size_is(aPinCount)] in string aSha256Pins,
+                       in int64_t aExpires, in Array<ACString> aSha256Pins,
                       [optional] in boolean aIsPreload,
                       [optional] in jsval aOriginAttributes);

--- a/security/manager/ssl/nsSiteSecurityService.cpp
+++ b/security/manager/ssl/nsSiteSecurityService.cpp
@@ -372,7 +372,7 @@ SiteHPKPState::SiteHPKPState(const nsCString& aHost,
                             const OriginAttributes& aOriginAttributes,
                             PRTime aExpireTime, SecurityPropertyState aState,
                             bool aIncludeSubdomains,
-                             nsTArray<nsCString>& aSHA256keys)
+                             const nsTArray<nsCString>& aSHA256keys)
    : mHostname(aHost),
      mOriginAttributes(aOriginAttributes),
      mExpireTime(aExpireTime),
@@ -1618,7 +1618,7 @@ nsSiteSecurityService::GetKeyPinsForHostname(
 NS_IMETHODIMP
 nsSiteSecurityService::SetKeyPins(const nsACString& aHost,
                                  bool aIncludeSubdomains, int64_t aExpires,
-                                  uint32_t aPinCount, const char** aSha256Pins,
+                                  const nsTArray<nsCString>& aSha256Pins,
                                  bool aIsPreload,
                                  JS::HandleValue aOriginAttributes,
                                  JSContext* aCx, uint8_t aArgc,
@@ -1631,7 +1631,6 @@ nsSiteSecurityService::SetKeyPins(const nsACString& aHost,
  }

  NS_ENSURE_ARG_POINTER(aResult);
-  NS_ENSURE_ARG_POINTER(aSha256Pins);
  OriginAttributes originAttributes;
  if (aArgc > 1) {
    // OriginAttributes were passed in.
@@ -1646,14 +1645,11 @@ nsSiteSecurityService::SetKeyPins(const nsACString& aHost,

  SSSLOG(("Top of SetKeyPins"));

-  nsTArray<nsCString> sha256keys;
-  for (unsigned int i = 0; i < aPinCount; i++) {
-    nsAutoCString pin(aSha256Pins[i]);
+  for (auto& pin : aSha256Pins) {
    SSSLOG(("SetPins pin=%s\n", pin.get()));
    if (!stringIsBase64EncodingOf256bitValue(pin)) {
      return NS_ERROR_INVALID_ARG;
    }
-    sha256keys.AppendElement(pin);
  }
  // we always store data in permanent storage (ie no flags)
  const nsCString& flatHost = PromiseFlatCString(aHost);
@@ -1661,7 +1657,7 @@ nsSiteSecurityService::SetKeyPins(const nsACString& aHost,
      PublicKeyPinningService::CanonicalizeHostname(flatHost.get()));
  RefPtr<SiteHPKPState> dynamicEntry =
      new SiteHPKPState(host, originAttributes, aExpires, SecurityPropertySet,
-                        aIncludeSubdomains, sha256keys);
+                        aIncludeSubdomains, aSha256Pins);
  return SetHPKPState(host.get(), *dynamicEntry, 0, aIsPreload,
                      originAttributes);
 }
--- a/security/manager/ssl/nsSiteSecurityService.h
+++ b/security/manager/ssl/nsSiteSecurityService.h
@@ -76,7 +76,7 @@ class SiteHPKPState : public nsISiteHPKPState {
  SiteHPKPState(const nsCString& aHost,
                const OriginAttributes& aOriginAttributes, PRTime aExpireTime,
                SecurityPropertyState aState, bool aIncludeSubdomains,
-                nsTArray<nsCString>& SHA256keys);
+                const nsTArray<nsCString>& SHA256keys);

  nsCString mHostname;
  OriginAttributes mOriginAttributes;
--- a/security/manager/ssl/tests/unit/test_pinning_dynamic.js
+++ b/security/manager/ssl/tests/unit/test_pinning_dynamic.js
@@ -155,7 +155,7 @@ async function async_check_pins() {

  // add includeSubdomains to a.pinning2.example.com
  gSSService.setKeyPins("a.pinning2.example.com", true,
-                        new Date().getTime() + 1000000, 2,
+                        new Date().getTime() + 1000000,
                        [NON_ISSUED_KEY_HASH, PINNING_ROOT_KEY_HASH]);
  await checkFail(certFromFile("a.pinning2.example.com-badca"),
                  "a.pinning2.example.com");
@@ -187,7 +187,7 @@ async function async_check_pins() {

  // Now setpins without subdomains
  gSSService.setKeyPins("a.pinning2.example.com", false,
-                        new Date().getTime() + 1000000, 2,
+                        new Date().getTime() + 1000000,
                        [NON_ISSUED_KEY_HASH, PINNING_ROOT_KEY_HASH]);
  await checkFail(certFromFile("a.pinning2.example.com-badca"),
                  "a.pinning2.example.com");
@@ -212,7 +212,7 @@ async function async_check_pins() {
  // failure to insert new pin entry leaves previous pin behavior
  throws(() => {
    gSSService.setKeyPins("a.pinning2.example.com", true,
-                          new Date().getTime() + 1000000, 1, ["not a hash"]);
+                          new Date().getTime() + 1000000, ["not a hash"]);
  }, /NS_ERROR_ILLEGAL_VALUE/, "Attempting to set an invalid pin should fail");
  await checkFail(certFromFile("a.pinning2.example.com-badca"),
                  "a.pinning2.example.com");
@@ -234,13 +234,6 @@ async function async_check_pins() {

  checkDefaultSiteHPKPStatus();

-  // Incorrect size results in failure
-  throws(() => {
-    gSSService.setKeyPins("a.pinning2.example.com", true,
-                          new Date().getTime() + 1000000, 2, ["not a hash"]);
-  }, /NS_ERROR_XPC_NOT_ENOUGH_ELEMENTS_IN_ARRAY/,
-     "Attempting to set a pin with an incorrect size should fail");
-
  // Ensure built-in pins work as expected
  ok(!gSSService.isSecureURI(
       Ci.nsISiteSecurityService.HEADER_HPKP,
@@ -252,7 +245,7 @@ async function async_check_pins() {
     "Built-in include-subdomains.pinning.example.com should have HPKP status");

  gSSService.setKeyPins("a.pinning2.example.com", false, new Date().getTime(),
-                        1, [NON_ISSUED_KEY_HASH]);
+                        [NON_ISSUED_KEY_HASH]);

  // Check that a preload pin loaded from file works as expected
  await checkFail(certFromFile("a.preload.example.com-badca"), "a.preload.example.com");
@@ -264,7 +257,7 @@ async function async_check_pins() {
  // then we add a pin, and we should get a failure (ensuring the expiry is
  // after the test timeout)
  gSSService.setKeyPins("b.preload.example.com", false,
-                        new Date().getTime() + 1000000, 2,
+                        new Date().getTime() + 1000000,
                        [NON_ISSUED_KEY_HASH, PINNING_ROOT_KEY_HASH], true);
  await checkFail(certFromFile("b.preload.example.com-badca"), "b.preload.example.com");
 }
--- a/security/manager/ssl/tests/unit/test_sss_originAttributes.js
+++ b/security/manager/ssl/tests/unit/test_sss_originAttributes.js
@@ -75,7 +75,7 @@ function doTest(originAttributes1, originAttributes2, shouldShare) {
       "URI should be not be secure after removeState");
  }
  // Set HPKP for originAttributes1.
-  sss.setKeyPins(host, false, Date.now() + 1234567890, 2,
+  sss.setKeyPins(host, false, Date.now() + 1234567890,
                 [NON_ISSUED_KEY_HASH, PINNING_ROOT_KEY_HASH], false,
                 originAttributes1);
  ok(sss.isSecureURI(Ci.nsISiteSecurityService.HEADER_HPKP, uri, 0,
@@ -114,7 +114,7 @@ function testInvalidOriginAttributes(originAttributes) {
    }
  }

-  throws(() => sss.setKeyPins(host, false, Date.now() + 1234567890, 2,
+  throws(() => sss.setKeyPins(host, false, Date.now() + 1234567890,
                              [NON_ISSUED_KEY_HASH, PINNING_ROOT_KEY_HASH],
                              false, originAttributes),
         /NS_ERROR_ILLEGAL_VALUE/,
--- a/security/manager/ssl/tests/unit/test_sss_savestate.js
+++ b/security/manager/ssl/tests/unit/test_sss_savestate.js
@@ -100,7 +100,7 @@ function run_test() {
                    .getService(Ci.nsISiteSecurityService);
  // Put an HPKP entry
  SSService.setKeyPins("dynamic-pin.example.com", true,
-                       new Date().getTime() + 1000000, 1,
+                       new Date().getTime() + 1000000,
                       [NON_ISSUED_KEY_HASH]);

  let uris = [ Services.io.newURI("http://includesubdomains.preloaded.test"),