Extensions: Restrict CSP dictionary to Manifest V3 and above.

Currently it was enabled on Trunk for MV2 extensions.

BUG=896041

Change-Id: Id35c9f3acdbd37739b0079ce04987aaef48ab27d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462686Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815806}

extensions/common/manifest_handlers/csp_info.cc

@@ -11,10 +11,8 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
-#include "components/version_info/channel.h"
 #include "extensions/common/csp_validator.h"
 #include "extensions/common/error_utils.h"
-#include "extensions/common/features/feature_channel.h"
 #include "extensions/common/install_warning.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/sandboxed_page_info.h"
@@ -177,29 +175,16 @@ bool CSPHandler::Parse(Extension* extension, base::string16* error) {
   //     "extension_pages": "",
   //     "sandbox": "",
   //  }
+  // The dictionary is supported (and mandated) for manifest v3 (and above)
+  // extensions.
   const base::Value* csp = GetManifestPath(extension, key);
-
-  // TODO(karandeepb): Remove the channel check since we don't plan to support
-  // the CSP dictionary for Manifest V2.
-  bool csp_dictionary_supported =
-      extension->GetType() == Manifest::TYPE_EXTENSION &&
-      (extension->manifest_version() >= 3 ||
-       GetCurrentChannel() == version_info::Channel::UNKNOWN);
-
-  if (csp_dictionary_supported) {
-    // CSP key as dictionary is mandatory for manifest v3 (and above)
-    // extensions.
-    if (extension->manifest_version() >= 3) {
-      if (csp && !csp->is_dict()) {
-        *error = GetInvalidManifestKeyError(key);
-        return false;
-      }
-      return ParseCSPDictionary(extension, error);
+  bool parse_as_dictionary = extension->manifest_version() >= 3;
+  if (parse_as_dictionary) {
+    if (csp && !csp->is_dict()) {
+      *error = GetInvalidManifestKeyError(key);
+      return false;
     }
-
-    // CSP key as dictionary is optional for manifest v2 extensions.
-    if (csp && csp->is_dict())
-      return ParseCSPDictionary(extension, error);
+    return ParseCSPDictionary(extension, error);
   }
 
   if (!ParseExtensionPagesCSP(extension, error, key, false /* secure_only */,

extensions/common/manifest_handlers/csp_info_unittest.cc

@@ -131,55 +131,33 @@ TEST_F(CSPInfoUnitTest, CSPDictionary_ExtensionPages) {
                 "worker-src 'self'; script-src; default-src 'self'"},
                {"csp_empty_dictionary_valid.json", kDefaultSecureCSP}};
 
-  // Verify that keys::kContentSecurityPolicy key can be used as a dictionary on
-  // trunk.
-  {
-    ScopedCurrentChannel channel(version_info::Channel::UNKNOWN);
-    for (const auto& test_case : cases) {
-      SCOPED_TRACE(
-          base::StringPrintf("%s on channel %s", test_case.file_name, "trunk"));
-      scoped_refptr<Extension> extension =
-          LoadAndExpectSuccess(test_case.file_name);
-      ASSERT_TRUE(extension.get());
-      EXPECT_EQ(test_case.csp, CSPInfo::GetExtensionPagesCSP(extension.get()));
-    }
-  }
-
-  // Verify that keys::kContentSecurityPolicy key can't be used as a dictionary
-  // on Stable.
-  {
-    ScopedCurrentChannel channel(version_info::Channel::STABLE);
-    for (const auto& test_case : cases) {
-      SCOPED_TRACE(base::StringPrintf("%s on channel %s", test_case.file_name,
-                                      "stable"));
-      LoadAndExpectError(
-          test_case.file_name,
-          GetInvalidManifestKeyError(keys::kContentSecurityPolicy));
-    }
+  for (const auto& test_case : cases) {
+    SCOPED_TRACE(base::StringPrintf("Testing %s.", test_case.file_name));
+    scoped_refptr<Extension> extension =
+        LoadAndExpectSuccess(test_case.file_name);
+    ASSERT_TRUE(extension.get());
+    EXPECT_EQ(test_case.csp, CSPInfo::GetExtensionPagesCSP(extension.get()));
   }
 
-  {
-    ScopedCurrentChannel channel(version_info::Channel::UNKNOWN);
-    Testcase testcases[] = {
-        Testcase("csp_invalid_2.json",
-                 GetInvalidManifestKeyError(
-                     keys::kContentSecurityPolicy_ExtensionPagesPath)),
-        Testcase("csp_invalid_3.json",
-                 GetInvalidManifestKeyError(
-                     keys::kContentSecurityPolicy_ExtensionPagesPath)),
-        Testcase(
-            "csp_missing_src.json",
-            ErrorUtils::FormatErrorMessage(
-                errors::kInvalidCSPMissingSecureSrc,
-                keys::kContentSecurityPolicy_ExtensionPagesPath, "script-src")),
-        Testcase("csp_insecure_src.json",
-                 ErrorUtils::FormatErrorMessage(
-                     errors::kInvalidCSPInsecureValueError,
-                     keys::kContentSecurityPolicy_ExtensionPagesPath,
-                     "'unsafe-eval'", "worker-src")),
-    };
-    RunTestcases(testcases, base::size(testcases), EXPECT_TYPE_ERROR);
-  }
+  Testcase testcases[] = {
+      Testcase("csp_invalid_2.json",
+               GetInvalidManifestKeyError(
+                   keys::kContentSecurityPolicy_ExtensionPagesPath)),
+      Testcase("csp_invalid_3.json",
+               GetInvalidManifestKeyError(
+                   keys::kContentSecurityPolicy_ExtensionPagesPath)),
+      Testcase(
+          "csp_missing_src.json",
+          ErrorUtils::FormatErrorMessage(
+              errors::kInvalidCSPMissingSecureSrc,
+              keys::kContentSecurityPolicy_ExtensionPagesPath, "script-src")),
+      Testcase("csp_insecure_src.json",
+               ErrorUtils::FormatErrorMessage(
+                   errors::kInvalidCSPInsecureValueError,
+                   keys::kContentSecurityPolicy_ExtensionPagesPath,
+                   "'unsafe-eval'", "worker-src")),
+  };
+  RunTestcases(testcases, base::size(testcases), EXPECT_TYPE_ERROR);
 }
 
 TEST_F(CSPInfoUnitTest, CSPDictionary_Sandbox) {
@@ -251,4 +229,10 @@ TEST_F(CSPInfoUnitTest, CSPDictionaryMandatoryForV3) {
   }
 }
 
+// Ensure the CSP dictionary is disallowed for mv2 extensions.
+TEST_F(CSPInfoUnitTest, CSPDictionaryDisallowedForV2) {
+  LoadAndExpectError("csp_dictionary_mv2.json",
+                     GetInvalidManifestKeyError(keys::kContentSecurityPolicy));
+}
+
 }  // namespace extensions

extensions/test/data/manifest_tests/csp_dictionary_mv2.json

+{
+  "name": "test",
+  "version": "0.1",
+  "manifest_version": 2,
+  "content_security_policy": {
+    "extension_pages" : "default-src 'none'"
+  }
+}

extensions/test/data/manifest_tests/csp_dictionary_valid_1.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
     "extension_pages" : "default-src 'none'"
   }

extensions/test/data/manifest_tests/csp_dictionary_valid_2.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
     "extension_pages" : "worker-src 'self'; script-src; default-src 'self'"
   }

extensions/test/data/manifest_tests/csp_empty_dictionary_valid.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
   }
 }

extensions/test/data/manifest_tests/csp_insecure_src.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
     "extension_pages" : "default-src 'none'; worker-src 'unsafe-eval';"
   }

extensions/test/data/manifest_tests/csp_invalid_2.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
     "extension_pages" : {}
   }

extensions/test/data/manifest_tests/csp_invalid_3.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
     "extension_pages" : "\r\n"
   }

extensions/test/data/manifest_tests/csp_missing_src.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "content_security_policy": {
     "extension_pages" : ""
   }

extensions/test/data/manifest_tests/sandbox_both_keys.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "sandbox": {
     "pages": ["test"],
     "content_security_policy": "sandbox;"

extensions/test/data/manifest_tests/sandbox_csp_with_dictionary.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "sandbox": {
     "pages": ["test"],
     "content_security_policy": "sandbox;"

extensions/test/data/manifest_tests/sandbox_dictionary_1.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "sandbox": {
     "pages": ["test"]
   },

extensions/test/data/manifest_tests/sandbox_dictionary_2.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "sandbox": {
     "pages": ["test"]
   },

extensions/test/data/manifest_tests/sandbox_invalid_type.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "sandbox": {
     "pages": ["test"]
   },

extensions/test/data/manifest_tests/unsandboxed_csp.json

 {
   "name": "test",
   "version": "0.1",
-  "manifest_version": 2,
+  "manifest_version": 3,
   "sandbox": {
     "pages": ["test"]
   },