Allow extenions to override the default content_security_policy, but require

the explicit policy to meet a minimum security threshold.
Review URL: http://codereview.chromium.org/8773028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112656 0039d316-1c4b-4281-b951-d872f2087c98

chrome/chrome_common.gypi

@@ -103,6 +103,8 @@
         'common/content_settings_types.h',
         'common/custom_handlers/protocol_handler.cc',
         'common/custom_handlers/protocol_handler.h',
+        'common/extensions/csp_validator.cc',
+        'common/extensions/csp_validator.h',
         'common/extensions/extension.cc',
         'common/extensions/extension.h',
         'common/extensions/extension_action.cc',

chrome/chrome_tests.gypi

@@ -1941,6 +1941,7 @@
         'common/content_settings_helper_unittest.cc',
         'common/content_settings_pattern_parser_unittest.cc',
         'common/content_settings_pattern_unittest.cc',
+        'common/extensions/csp_validator_unittest.cc',
         'common/extensions/extension_action_unittest.cc',
         'common/extensions/extension_file_util_unittest.cc',
         'common/extensions/extension_icon_set_unittest.cc',

chrome/common/extensions/csp_validator.cc

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/extensions/csp_validator.h"
+
+#include "base/string_split.h"
+#include "base/string_tokenizer.h"
+#include "base/string_util.h"
+
+namespace extensions {
+
+namespace csp_validator {
+
+namespace {
+
+const char kDefaultSrc[] = "default-src";
+const char kScriptSrc[] = "script-src";
+const char kObjectSrc[] = "object-src";
+
+struct DirectiveStatus {
+  explicit DirectiveStatus(const char* name)
+    : directive_name(name)
+    , seen_in_policy(false)
+    , is_secure(false) {
+  }
+
+  const char* directive_name;
+  bool seen_in_policy;
+  bool is_secure;
+};
+
+bool HasOnlySecureTokens(StringTokenizer& tokenizer) {
+  while (tokenizer.GetNext()) {
+    std::string source = tokenizer.token();
+    StringToLowerASCII(&source);
+
+    if (EndsWith(source, "*", true))
+      return false;
+
+    // We might need to relax this whitelist over time.
+    if (source == "'self'" ||
+        source == "'none'" ||
+        StartsWithASCII(source, "https://", true) ||
+        StartsWithASCII(source, "chrome://", true) ||
+        StartsWithASCII(source, "chrome-extension://", true)) {
+      continue;
+    }
+
+    return false;
+  }
+
+  return true;  // Empty values default to 'none', which is secure.
+}
+
+// Returns true if |directive_name| matches |status.directive_name|.
+bool UpdateStatus(const std::string& directive_name,
+                  StringTokenizer& tokenizer,
+                  DirectiveStatus* status) {
+  if (status->seen_in_policy)
+    return false;
+  if (directive_name != status->directive_name)
+    return false;
+  status->seen_in_policy = true;
+  status->is_secure = HasOnlySecureTokens(tokenizer);
+  return true;
+}
+
+}  //  namespace
+
+bool ContentSecurityPolicyIsLegal(const std::string& policy) {
+  // We block these characters to prevent HTTP header injection when
+  // representing the content security policy as an HTTP header.
+  const char kBadChars[] = {'\r', '\n', '\0'};
+
+  return policy.find_first_of(kBadChars, 0, arraysize(kBadChars)) ==
+      std::string::npos;
+}
+
+bool ContentSecurityPolicyIsSecure(const std::string& policy) {
+  // See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.
+  std::vector<std::string> directives;
+  base::SplitString(policy, ';', &directives);
+
+  DirectiveStatus default_src_status(kDefaultSrc);
+  DirectiveStatus script_src_status(kScriptSrc);
+  DirectiveStatus object_src_status(kObjectSrc);
+
+  for (size_t i = 0; i < directives.size(); ++i) {
+    std::string& input = directives[i];
+    StringTokenizer tokenizer(input, " \t\r\n");
+    if (!tokenizer.GetNext())
+      continue;
+
+    std::string directive_name = tokenizer.token();
+    StringToLowerASCII(&directive_name);
+
+    if (UpdateStatus(directive_name, tokenizer, &default_src_status))
+      continue;
+    if (UpdateStatus(directive_name, tokenizer, &script_src_status))
+      continue;
+    if (UpdateStatus(directive_name, tokenizer, &object_src_status))
+      continue;
+  }
+
+  if (script_src_status.seen_in_policy && !script_src_status.is_secure)
+    return false;
+
+  if (object_src_status.seen_in_policy && !object_src_status.is_secure)
+    return false;
+
+  if (default_src_status.seen_in_policy && !default_src_status.is_secure) {
+    return script_src_status.seen_in_policy &&
+           object_src_status.seen_in_policy;
+  }
+
+  return default_src_status.seen_in_policy ||
+      (script_src_status.seen_in_policy && object_src_status.seen_in_policy);
+}
+
+}  // csp_validator
+
+}  // extensions

chrome/common/extensions/csp_validator.h

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_COMMON_EXTENSIONS_CSP_VALIDATOR_H_
+#define CHROME_COMMON_EXTENSIONS_CSP_VALIDATOR_H_
+#pragma once
+
+#include <string>
+
+namespace extensions {
+
+namespace csp_validator {
+
+// Checks whether the given |policy| is legal for use in the extension system.
+// This check just ensures that the policy doesn't contain any characters that
+// will cause problems when we transmit the policy in an HTTP header.
+bool ContentSecurityPolicyIsLegal(const std::string& policy);
+
+// Checks whether the given |policy| meets the minimum security requirements
+// for use in the extension system. The philosophy behind our minimum
+// requirements is that an XSS vulnerability in the extension should not be
+// able to execute script, even in the precense of an active network attacker.
+// Specifically, 'unsafe-inline' and 'unsafe-eval' are forbidden, as is
+// script or object inclusion from insecure schemes. Also, the use of * is
+// forbidden for scripts and objects.
+bool ContentSecurityPolicyIsSecure(const std::string& policy);
+
+}  // namespace csp_validator
+
+}  // namespace extensions
+
+#endif  // CHROME_COMMON_EXTENSIONS_CSP_VALIDATOR_H_

chrome/common/extensions/csp_validator_unittest.cc

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/extensions/csp_validator.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using namespace extensions::csp_validator;
+
+TEST(ExtensionCSPValidator, IsLegal) {
+  EXPECT_TRUE(ContentSecurityPolicyIsLegal("foo"));
+  EXPECT_TRUE(ContentSecurityPolicyIsLegal(
+      "default-src 'self'; script-src http://www.google.com"));
+  EXPECT_FALSE(ContentSecurityPolicyIsLegal(
+      "default-src 'self';\nscript-src http://www.google.com"));
+  EXPECT_FALSE(ContentSecurityPolicyIsLegal(
+      "default-src 'self';\rscript-src http://www.google.com"));
+}
+
+TEST(ExtensionCSPValidator, IsSecure) {
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(""));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure("img-src https://google.com"));
+
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure("default-src *"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure("default-src 'self'"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure("default-src 'none'"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' ftp://google.com"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' https://google.com"));
+
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src *; default-src 'self'"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self'; default-src *"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self'; default-src *; script-src *; script-src 'self'"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self'; default-src *; script-src 'self'; script-src *"));
+
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src *; script-src 'self'"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src *; script-src 'self'; img-src 'self'"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src *; script-src 'self'; object-src 'self'"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "script-src 'self'; object-src 'self'"));
+
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure("default-src 'unsafe-inline'"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure("default-src 'unsafe-eval'"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'unsafe-inline' 'none'"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' http://google.com"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' https://google.com"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' chrome://resources"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' chrome-extension://aabbcc"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' https:"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' http:"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' https://*"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' *"));
+  EXPECT_FALSE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' google.com"));
+  EXPECT_TRUE(ContentSecurityPolicyIsSecure(
+      "default-src 'self' https://*.google.com"));
+}

chrome/common/extensions/extension.cc

@@ -25,6 +25,7 @@
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/extensions/csp_validator.h"
 #include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_error_utils.h"
@@ -50,6 +51,9 @@ namespace keys = extension_manifest_keys;
 namespace values = extension_manifest_values;
 namespace errors = extension_manifest_errors;
 
+using extensions::csp_validator::ContentSecurityPolicyIsLegal;
+using extensions::csp_validator::ContentSecurityPolicyIsSecure;
+
 namespace {
 
 const int kModernManifestVersion = 1;
@@ -2240,21 +2244,23 @@ bool Extension::InitFromValue(const DictionaryValue& source, int flags,
       *error = errors::kInvalidContentSecurityPolicy;
       return false;
     }
-    // We block these characters to prevent HTTP header injection when
-    // representing the content security policy as an HTTP header.
-    const char kBadCSPCharacters[] = {'\r', '\n', '\0'};
-    if (content_security_policy.find_first_of(kBadCSPCharacters, 0,
-                                              arraysize(kBadCSPCharacters)) !=
-        std::string::npos) {
+    if (!ContentSecurityPolicyIsLegal(content_security_policy)) {
       *error = errors::kInvalidContentSecurityPolicy;
       return false;
     }
+    if (manifest_version_ >= 2 &&
+        !ContentSecurityPolicyIsSecure(content_security_policy)) {
+      *error = errors::kInvalidContentSecurityPolicy;
+      return false;
+    }
+
     content_security_policy_ = content_security_policy;
   } else if (manifest_version_ >= 2) {
     // Manifest version 2 introduced a default Content-Security-Policy.
     // TODO(abarth): Should we continue to let extensions override the
     //               default Content-Security-Policy?
     content_security_policy_ = kDefaultContentSecurityPolicy;
+    CHECK(ContentSecurityPolicyIsSecure(content_security_policy_));
   }
 
   // Initialize devtools page url (optional).

chrome/test/data/extensions/api_test/content_security_policy/manifest.json

@@ -4,5 +4,5 @@
   "manifest_version": 2,
   "description": "Sanity check that content_security_policy works for extensions. The majority of this is implemented (and tested) in WebKit, but we have a sanity test here just to make sure the integration with Chromium keeps working.",
   "background_page": "test.html",
-  "content_security_policy": "script-src 'self'"
+  "content_security_policy": "script-src 'self'; object-src 'none'"
 }