Bug 1581609 rename extension contentSecurityPolicy to support multiple V3 CSP values r=rpl,webidl
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 01 Nov 2019 06:02:22 +0000
changeset 500095 1a07c94b54c54c3a4570794c01e4434b2a20ff5b
parent 500094 6e0719109ea2bcc7a2560e20cff1d496b59faf5a
child 500096 a56f917583a6d6942cc4b984dfb2d87146665b70
push id99395
push userscaraveo@mozilla.com
push dateFri, 01 Nov 2019 06:59:57 +0000
treeherderautoland@23c113d65b48 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrpl, webidl
bugs1581609
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1581609 rename extension contentSecurityPolicy to support multiple V3 CSP values r=rpl,webidl Differential Revision: https://phabricator.services.mozilla.com/D46818
caps/nsIAddonPolicyService.idl
dom/base/Document.cpp
dom/chrome-webidl/WebExtensionPolicy.webidl
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionPolicyService.cpp
toolkit/components/extensions/ExtensionProcessScript.jsm
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/WebExtensionPolicy.cpp
toolkit/components/extensions/WebExtensionPolicy.h
toolkit/components/extensions/schemas/manifest.json
toolkit/components/extensions/test/xpcshell/test_csp_custom_policies.js
toolkit/components/extensions/test/xpcshell/test_ext_manifest_content_security_policy.js
--- a/caps/nsIAddonPolicyService.idl
+++ b/caps/nsIAddonPolicyService.idl
@@ -26,17 +26,17 @@ interface nsIAddonPolicyService : nsISup
    */
   readonly attribute AString defaultCSP;
 
   /**
    * Returns the content security policy which applies to documents belonging
    * to the extension with the given ID. This may be either a custom policy,
    * if one was supplied, or the default policy if one was not.
    */
-  AString getAddonCSP(in AString aAddonId);
+  AString getExtensionPageCSP(in AString aAddonId);
 
   /**
    * Returns the generated background page as a data-URI, if any. If the addon
    * does not have an auto-generated background page, an empty string is
    * returned.
    */
   ACString getGeneratedBackgroundPageUrl(in ACString aAddonId);
 
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -3171,21 +3171,22 @@ nsresult Document::InitCSP(nsIChannel* a
     return NS_OK;
   }
 
   MOZ_LOG(gCspPRLog, LogLevel::Debug,
           ("Document is an add-on or CSP header specified %p", this));
 
   // ----- if the doc is an addon, apply its CSP.
   if (addonPolicy) {
-    nsAutoString addonCSP;
-    Unused << ExtensionPolicyService::GetSingleton().GetBaseCSP(addonCSP);
-    mCSP->AppendPolicy(addonCSP, false, false);
-
-    mCSP->AppendPolicy(addonPolicy->ContentSecurityPolicy(), false, false);
+    nsAutoString extensionPageCSP;
+    Unused << ExtensionPolicyService::GetSingleton().GetBaseCSP(
+        extensionPageCSP);
+    mCSP->AppendPolicy(extensionPageCSP, false, false);
+
+    mCSP->AppendPolicy(addonPolicy->ExtensionPageCSP(), false, false);
     // Bug 1548468: Move CSP off ExpandedPrincipal
     // Currently the LoadInfo holds the source of truth for every resource load
     // because LoadInfo::GetCSP() queries the CSP from an ExpandedPrincipal
     // (and not from the Client) if the load was triggered by an extension.
     auto* basePrin = BasePrincipal::Cast(principal);
     if (basePrin->Is<ExpandedPrincipal>()) {
       basePrin->As<ExpandedPrincipal>()->SetCsp(mCSP);
     }
--- a/dom/chrome-webidl/WebExtensionPolicy.webidl
+++ b/dom/chrome-webidl/WebExtensionPolicy.webidl
@@ -49,18 +49,17 @@ interface WebExtensionPolicy {
   [Constant]
   readonly attribute boolean isPrivileged;
 
   /**
    * The content security policy string to apply to all pages loaded from the
    * extension's moz-extension: protocol.
    */
   [Constant]
-  readonly attribute DOMString contentSecurityPolicy;
-
+  readonly attribute DOMString extensionPageCSP;
 
   /**
    * The list of currently-active permissions for the extension, as specified
    * in its manifest.json file. May be updated to reflect changes in the
    * extension's optional permissions.
    */
   [Cached, Frozen, Pure]
   attribute sequence<DOMString> permissions;
@@ -227,14 +226,14 @@ dictionary WebExtensionInit {
   required MatchPatternSetOrStringSequence allowedOrigins;
 
   sequence<DOMString> permissions = [];
 
   sequence<MatchGlobOrString> webAccessibleResources = [];
 
   sequence<WebExtensionContentScriptInit> contentScripts = [];
 
-  DOMString? contentSecurityPolicy = null;
+  DOMString? extensionPageCSP = null;
 
   sequence<DOMString>? backgroundScripts = null;
 
   Promise<WebExtensionPolicy> readyPromise;
 };
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -1858,18 +1858,25 @@ class Extension extends ExtensionData {
 
     if (this.errors.length) {
       return Promise.reject({ errors: this.errors });
     }
 
     return manifest;
   }
 
-  get contentSecurityPolicy() {
-    return this.manifest.content_security_policy;
+  get extensionPageCSP() {
+    const { content_security_policy } = this.manifest;
+    if (
+      content_security_policy &&
+      typeof content_security_policy === "object"
+    ) {
+      return content_security_policy.extension_pages;
+    }
+    return content_security_policy;
   }
 
   get backgroundScripts() {
     return this.manifest.background && this.manifest.background.scripts;
   }
 
   get optionalPermissions() {
     return this.manifest.optional_permissions;
@@ -1886,17 +1893,17 @@ class Extension extends ExtensionData {
   // Representation of the extension to send to content
   // processes. This should include anything the content process might
   // need.
   serialize() {
     return {
       id: this.id,
       uuid: this.uuid,
       name: this.name,
-      contentSecurityPolicy: this.contentSecurityPolicy,
+      extensionPageCSP: this.extensionPageCSP,
       instanceId: this.instanceId,
       resourceURL: this.resourceURL,
       contentScripts: this.contentScripts,
       webAccessibleResources: this.webAccessibleResources.map(res => res.glob),
       whiteListedHosts: this.whiteListedHosts.patterns.map(pat => pat.pattern),
       permissions: this.permissions,
       optionalPermissions: this.optionalPermissions,
       isPrivileged: this.isPrivileged,
--- a/toolkit/components/extensions/ExtensionPolicyService.cpp
+++ b/toolkit/components/extensions/ExtensionPolicyService.cpp
@@ -531,20 +531,20 @@ nsresult ExtensionPolicyService::GetBase
   return NS_OK;
 }
 
 nsresult ExtensionPolicyService::GetDefaultCSP(nsAString& aDefaultCSP) {
   DefaultCSP(aDefaultCSP);
   return NS_OK;
 }
 
-nsresult ExtensionPolicyService::GetAddonCSP(const nsAString& aAddonId,
-                                             nsAString& aResult) {
+nsresult ExtensionPolicyService::GetExtensionPageCSP(const nsAString& aAddonId,
+                                                     nsAString& aResult) {
   if (WebExtensionPolicy* policy = GetByID(aAddonId)) {
-    policy->GetContentSecurityPolicy(aResult);
+    policy->GetExtensionPageCSP(aResult);
     return NS_OK;
   }
   return NS_ERROR_INVALID_ARG;
 }
 
 nsresult ExtensionPolicyService::GetGeneratedBackgroundPageUrl(
     const nsACString& aHostname, nsACString& aResult) {
   if (WebExtensionPolicy* policy = GetByHost(aHostname)) {
--- a/toolkit/components/extensions/ExtensionProcessScript.jsm
+++ b/toolkit/components/extensions/ExtensionProcessScript.jsm
@@ -203,17 +203,17 @@ ExtensionManager = {
         name: extension.name,
         baseURL: extension.resourceURL,
 
         isPrivileged: extension.isPrivileged,
         permissions: extension.permissions,
         allowedOrigins: extension.whiteListedHosts,
         webAccessibleResources: extension.webAccessibleResources,
 
-        contentSecurityPolicy: extension.contentSecurityPolicy,
+        extensionPageCSP: extension.extensionPageCSP,
 
         localizeCallback,
 
         backgroundScripts,
 
         contentScripts: extension.contentScripts,
       });
 
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -1058,16 +1058,20 @@ const FORMATS = {
       }
     }
     return string;
   },
 
   contentSecurityPolicy(string, context) {
     let error = contentPolicyService.validateAddonCSP(string);
     if (error != null) {
+      // The SyntaxError raised below is not reported as part of the "choices" error message,
+      // we log the CSP validation error explicitly here to make it easier for the addon developers
+      // to see and fix the extension CSP.
+      context.logError(`Error processing ${context.currentTarget}: ${error}`);
       throw new SyntaxError(error);
     }
     return string;
   },
 
   date(string, context) {
     // A valid ISO 8601 timestamp.
     const PATTERN = /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}:\d{2}(\.\d{3})?(Z|([-+]\d{2}:?\d{2})))?$/;
--- a/toolkit/components/extensions/WebExtensionPolicy.cpp
+++ b/toolkit/components/extensions/WebExtensionPolicy.cpp
@@ -126,17 +126,17 @@ already_AddRefed<MatchPatternSet> ParseM
  *****************************************************************************/
 
 WebExtensionPolicy::WebExtensionPolicy(GlobalObject& aGlobal,
                                        const WebExtensionInit& aInit,
                                        ErrorResult& aRv)
     : mId(NS_AtomizeMainThread(aInit.mId)),
       mHostname(aInit.mMozExtensionHostname),
       mName(aInit.mName),
-      mContentSecurityPolicy(aInit.mContentSecurityPolicy),
+      mExtensionPageCSP(aInit.mExtensionPageCSP),
       mLocalizeCallback(aInit.mLocalizeCallback),
       mIsPrivileged(aInit.mIsPrivileged),
       mPermissions(new AtomSet(aInit.mPermissions)) {
   if (!ParseGlobs(aGlobal, aInit.mWebAccessibleResources, mWebAccessiblePaths,
                   aRv)) {
     return;
   }
 
@@ -153,18 +153,18 @@ WebExtensionPolicy::WebExtensionPolicy(G
     return;
   }
 
   if (!aInit.mBackgroundScripts.IsNull()) {
     mBackgroundScripts.SetValue().AppendElements(
         aInit.mBackgroundScripts.Value());
   }
 
-  if (mContentSecurityPolicy.IsVoid()) {
-    EPS().DefaultCSP(mContentSecurityPolicy);
+  if (mExtensionPageCSP.IsVoid()) {
+    EPS().DefaultCSP(mExtensionPageCSP);
   }
 
   mContentScripts.SetCapacity(aInit.mContentScripts.Length());
   for (const auto& scriptInit : aInit.mContentScripts) {
     // The activeTab permission is only for dynamically injected scripts,
     // it cannot be used for declarative content scripts.
     if (scriptInit.mHasActiveTabPermission) {
       aRv.Throw(NS_ERROR_INVALID_ARG);
--- a/toolkit/components/extensions/WebExtensionPolicy.h
+++ b/toolkit/components/extensions/WebExtensionPolicy.h
@@ -97,22 +97,18 @@ class WebExtensionPolicy final : public 
   nsCString BackgroundPageHTML() const;
 
   MOZ_CAN_RUN_SCRIPT
   void Localize(const nsAString& aInput, nsString& aResult) const;
 
   const nsString& Name() const { return mName; }
   void GetName(nsAString& aName) const { aName = mName; }
 
-  const nsString& ContentSecurityPolicy() const {
-    return mContentSecurityPolicy;
-  }
-  void GetContentSecurityPolicy(nsAString& aCSP) const {
-    aCSP = mContentSecurityPolicy;
-  }
+  const nsString& ExtensionPageCSP() const { return mExtensionPageCSP; }
+  void GetExtensionPageCSP(nsAString& aCSP) const { aCSP = mExtensionPageCSP; }
 
   already_AddRefed<MatchPatternSet> AllowedOrigins() {
     return do_AddRef(mHostPermissions);
   }
   void SetAllowedOrigins(MatchPatternSet& aAllowedOrigins) {
     mHostPermissions = &aAllowedOrigins;
   }
 
@@ -178,17 +174,17 @@ class WebExtensionPolicy final : public 
 
   nsCOMPtr<nsISupports> mParent;
 
   RefPtr<nsAtom> mId;
   nsCString mHostname;
   nsCOMPtr<nsIURI> mBaseURI;
 
   nsString mName;
-  nsString mContentSecurityPolicy;
+  nsString mExtensionPageCSP;
 
   bool mActive = false;
   bool mAllowPrivateBrowsingByDefault = true;
 
   RefPtr<WebExtensionLocalizeCallback> mLocalizeCallback;
 
   bool mIsPrivileged;
   RefPtr<AtomSet> mPermissions;
--- a/toolkit/components/extensions/schemas/manifest.json
+++ b/toolkit/components/extensions/schemas/manifest.json
@@ -173,20 +173,33 @@
 
           "content_scripts": {
             "type": "array",
             "optional": true,
             "items": { "$ref": "ContentScript" }
           },
 
           "content_security_policy": {
-            "type": "string",
             "optional": true,
-            "format": "contentSecurityPolicy",
-            "onError": "warn"
+            "onError": "warn",
+            "choices": [
+              {
+                "type": "string",
+                "format": "contentSecurityPolicy"
+              },
+              {
+                "type": "object",
+                "properties": {
+                  "extension_pages": {
+                    "type": "string",
+                    "format": "contentSecurityPolicy"
+                  }
+                }
+              }
+            ]
           },
 
           "permissions": {
             "type": "array",
             "default": [],
             "items": {
               "$ref": "PermissionOrOrigin",
               "onError": "warn"
--- a/toolkit/components/extensions/test/xpcshell/test_csp_custom_policies.js
+++ b/toolkit/components/extensions/test/xpcshell/test_csp_custom_policies.js
@@ -9,30 +9,30 @@ const { Preferences } = ChromeUtils.impo
 const ADDON_ID = "test@web.extension";
 
 const aps = Cc["@mozilla.org/addons/policy-service;1"].getService(
   Ci.nsIAddonPolicyService
 );
 
 let policy = null;
 
-function setAddonCSP(csp) {
+function setExtensionPageCSP(csp) {
   if (policy) {
     policy.active = false;
   }
 
   policy = new WebExtensionPolicy({
     id: ADDON_ID,
     mozExtensionHostname: ADDON_ID,
     baseURL: "file:///",
 
     allowedOrigins: new MatchPatternSet([]),
     localizeCallback() {},
 
-    contentSecurityPolicy: csp,
+    extensionPageCSP: csp,
   });
 
   policy.active = true;
 }
 
 registerCleanupFunction(() => {
   policy.active = false;
 });
@@ -48,24 +48,24 @@ add_task(async function test_addon_csp()
     aps.defaultCSP,
     Preferences.get("extensions.webextensions.default-content-security-policy"),
     "Expected default CSP value"
   );
 
   const CUSTOM_POLICY =
     "script-src: 'self' https://xpcshell.test.custom.csp; object-src: 'none'";
 
-  setAddonCSP(CUSTOM_POLICY);
+  setExtensionPageCSP(CUSTOM_POLICY);
 
   equal(
-    aps.getAddonCSP(ADDON_ID),
+    aps.getExtensionPageCSP(ADDON_ID),
     CUSTOM_POLICY,
     "CSP should point to add-on's custom policy"
   );
 
-  setAddonCSP(null);
+  setExtensionPageCSP(null);
 
   equal(
-    aps.getAddonCSP(ADDON_ID),
+    aps.getExtensionPageCSP(ADDON_ID),
     aps.defaultCSP,
     "CSP should revert to default when set to null"
   );
 });
--- a/toolkit/components/extensions/test/xpcshell/test_ext_manifest_content_security_policy.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_manifest_content_security_policy.js
@@ -7,33 +7,69 @@ add_task(async function test_manifest_cs
     content_security_policy: "script-src 'self'; object-src 'none'",
   });
 
   equal(normalized.error, undefined, "Should not have an error");
   equal(normalized.errors.length, 0, "Should not have warnings");
   equal(
     normalized.value.content_security_policy,
     "script-src 'self'; object-src 'none'",
-    "Should have the expected poilcy string"
+    "Should have the expected policy string"
   );
 
   ExtensionTestUtils.failOnSchemaWarnings(false);
   normalized = await ExtensionTestUtils.normalizeManifest({
     content_security_policy: "object-src 'none'",
   });
   ExtensionTestUtils.failOnSchemaWarnings(true);
 
   equal(normalized.error, undefined, "Should not have an error");
 
   Assert.deepEqual(
     normalized.errors,
     [
-      "Error processing content_security_policy: SyntaxError: Policy is missing a required \u2018script-src\u2019 directive",
+      "Error processing content_security_policy: Policy is missing a required ‘script-src’ directive",
+      'Error processing content_security_policy: Value "object-src \'none\'" must either: match the format "contentSecurityPolicy", or be an object value',
     ],
     "Should have the expected warning"
   );
 
   equal(
     normalized.value.content_security_policy,
     null,
     "Invalid policy string should be omitted"
   );
 });
+
+add_task(async function test_manifest_csp_v3() {
+  let normalized = await ExtensionTestUtils.normalizeManifest({
+    content_security_policy: {
+      extension_pages: "script-src 'self'; object-src 'none'",
+    },
+  });
+
+  equal(normalized.error, undefined, "Should not have an error");
+  equal(normalized.errors.length, 0, "Should not have warnings");
+  equal(
+    normalized.value.content_security_policy.extension_pages,
+    "script-src 'self'; object-src 'none'",
+    "Should have the expected poilcy string"
+  );
+
+  ExtensionTestUtils.failOnSchemaWarnings(false);
+  normalized = await ExtensionTestUtils.normalizeManifest({
+    content_security_policy: {
+      extension_pages: "object-src 'none'",
+    },
+  });
+  ExtensionTestUtils.failOnSchemaWarnings(true);
+
+  equal(normalized.error, undefined, "Should not have an error");
+  equal(normalized.errors.length, 2, "Should have warnings");
+  Assert.deepEqual(
+    normalized.errors,
+    [
+      "Error processing content_security_policy.extension_pages: Policy is missing a required ‘script-src’ directive",
+      'Error processing content_security_policy: Value must either: be a string value, or .extension_pages must match the format "contentSecurityPolicy"',
+    ],
+    "Should have the expected warning"
+  );
+});