Restrict app sandbox's CSP to disallow loading web content in them.

extensions/common/csp_validator.cc

 // Copyright 2013 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 "extensions/common/csp_validator.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 10 matching lines @@
 
 namespace extensions {
 
 namespace csp_validator {
 
 namespace {
 
 const char kDefaultSrc[] = "default-src";
 const char kScriptSrc[] = "script-src";
 const char kObjectSrc[] = "object-src";
+const char kFrameSrc[] = "frame-src";
+const char kChildSrc[] = "child-src";
+
+const char kDirectiveSeparator[] = ";";
+
 const char kPluginTypes[] = "plugin-types";
 
 const char kObjectSrcDefaultDirective[] = "object-src 'self';";
 const char kScriptSrcDefaultDirective[] =
     "script-src 'self' chrome-extension-resource:;";
 
+const char kAppSandboxSubframeSrcDefaultDirective[] = "child-src 'self';";
+const char kAppSandboxScriptSrcDefaultDirective[] = "script-src 'self';";
+
 const char kSandboxDirectiveName[] = "sandbox";
 const char kAllowSameOriginToken[] = "allow-same-origin";
 const char kAllowTopNavigation[] = "allow-top-navigation";
 
 // This is the list of plugin types which are fully sandboxed and are safe to
 // load up in an extension, regardless of the URL they are navigated to.
 const char* const kSandboxedPluginTypes[] = {
   "application/pdf",
   "application/x-google-chrome-pdf",
   "application/x-pnacl"
@@ skipping 222 matching lines @@
   return policy.find_first_of(kBadChars, 0, arraysize(kBadChars)) ==
       std::string::npos;
 }
 
 std::string SanitizeContentSecurityPolicy(
     const std::string& policy,
     int options,
     std::vector<InstallWarning>* warnings) {
   // See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.
   std::vector<std::string> directives = base::SplitString(
-      policy, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+      policy, kDirectiveSeparator, base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
 
   DirectiveStatus default_src_status(kDefaultSrc);
   DirectiveStatus script_src_status(kScriptSrc);
   DirectiveStatus object_src_status(kObjectSrc);
 
   bool allow_insecure_object_src =
       AllowedToHaveInsecureObjectSrc(options, directives);
 
   std::vector<std::string> sane_csp_parts;
   std::vector<InstallWarning> default_src_csp_warnings;
   for (size_t i = 0; i < directives.size(); ++i) {
     std::string& input = directives[i];
     base::StringTokenizer tokenizer(input, " \t\r\n");
     if (!tokenizer.GetNext())
       continue;
 
     std::string directive_name = base::ToLowerASCII(tokenizer.token_piece());
     if (UpdateStatus(directive_name, &tokenizer, &default_src_status, options,
                      &sane_csp_parts, &default_src_csp_warnings))
       continue;
     if (UpdateStatus(directive_name, &tokenizer, &script_src_status, options,
                      &sane_csp_parts, warnings))
       continue;
     if (!allow_insecure_object_src &&
         UpdateStatus(directive_name, &tokenizer, &object_src_status, options,
                      &sane_csp_parts, warnings))
       continue;
 
     // Pass the other CSP directives as-is without further validation.
-    sane_csp_parts.push_back(input + ";");
+    sane_csp_parts.push_back(input + kDirectiveSeparator);
   }
 
   if (default_src_status.seen_in_policy) {
     if (!script_src_status.seen_in_policy ||
         !object_src_status.seen_in_policy) {
       // Insecure values in default-src are only relevant if either script-src
       // or object-src is omitted.
       if (warnings)
         warnings->insert(warnings->end(),
                          default_src_csp_warnings.begin(),
@@ skipping 10 matching lines @@
       sane_csp_parts.push_back(kObjectSrcDefaultDirective);
       if (warnings)
         warnings->push_back(CSPInstallWarning(ErrorUtils::FormatErrorMessage(
             manifest_errors::kInvalidCSPMissingSecureSrc, kObjectSrc)));
     }
   }
 
   return base::JoinString(sane_csp_parts, " ");
 }
 
+std::string GetEffectiveSandoxedPageCSP(const std::string& policy) {

[Devlin, 2016/12/09 15:46:01]

I kind of wonder if it makes sense to have a more general parsing for CSP in
some kind of object that's easily modifiable.  E.g.,

class CSP {
 public:
  CSP(std::string);
  std::string ToString();
  // accessors and modifiers
};

And then use that both here and in SanitizeCSP.  WDYT?  It seems like it would
be fairly simple and would make a lot of this code more readable, but I haven't
tried coding it up so maybe I'm missing something.

(There may even already be this somewhere?  But I couldn't find it.  Maybe
Charlie knows.)

[Charlie Reis, 2016/12/09 19:55:07]

I haven't seen one, but I agree that avoiding a bunch of custom parsing logic
for each use case is a good idea.  alexmos@ and mkwst@ are more familiar with
CSP and might know.

[alexmos, 2016/12/12 19:04:11]

I haven't seen one either, and I agree it's a good idea to have a common place
for parsing CSP.  We even have a bug filed for this: https://crbug.com/376522;
the motivation there is to move some of CSP enforcement to the browser process. 
mkwst/arthursonzogni are adding another little bit of CSP frame_ancestors
parsing in ancestor_throttle in https://codereview.chromium.org/2488743003/,
which could also use this.

[lazyboy, 2016/12/14 00:49:05]

I've introduced a CSPEnforcer class that can sanitize csp for apps sandbox and
(existing) extensions.

+  // See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.
+  std::vector<std::string> effective_csp_parts;
+  bool seen_default_source = false;
+  bool seen_subframe_source = false;
+  bool seen_script_source = false;
+  for (const std::string& directive : base::SplitString(
+           policy, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL)) {
+    base::StringTokenizer tokenizer(directive, " \t\r\n");
+    if (!tokenizer.GetNext())
+      continue;
+
+    std::string directive_name = base::ToLowerASCII(tokenizer.token_piece());
+    if (directive_name == kDefaultSrc) {
+      seen_default_source = true;
+    } else if (directive_name == kChildSrc || directive_name == kFrameSrc) {
+      seen_subframe_source = true;
+    } else if (directive_name == kScriptSrc) {
+      seen_script_source = true;
+    } else {
+      // Keep this directive as is.
+      effective_csp_parts.push_back(directive + kDirectiveSeparator);
+      continue;
+    }
+
+    effective_csp_parts.push_back(directive_name);
+    bool seen_self_or_none = false;
+    while (tokenizer.GetNext()) {
+      std::string source_lower = base::ToLowerASCII(tokenizer.token());
+      seen_self_or_none |= source_lower == "'none'" || source_lower == "'self'";
+
+      // Keyword directive sources are surrounded with quotes, e.g. 'self',
+      // 'sha256-...', 'unsafe-eval', 'nonce-...'. These do not specify a remote
+      // host or '*', so keep them and restrict the rest.
+      if (source_lower.size() > 1u && source_lower[0] == '\'' &&
+          source_lower[source_lower.size() - 1] == '\'') {

[Devlin, 2016/12/09 15:46:01]

When we see an insecure pattern, we could add an install warning to let
developers know that it will be ignored.

[lazyboy, 2016/12/14 00:49:05]

Done.

+        effective_csp_parts.push_back(source_lower);
+      }
+    }
+
+    if (!seen_self_or_none)
+      effective_csp_parts.push_back("'self'");
+
+    // Add ";" at the end of the directive.
+    effective_csp_parts.back().append(kDirectiveSeparator);
+  }
+
+  // If subframes, scripts were not restricted in |effective_csp_parts|, add the
+  // restriction.
+  if (!seen_subframe_source && !seen_default_source)
+    effective_csp_parts.push_back(kAppSandboxSubframeSrcDefaultDirective);
+  if (!seen_script_source && !seen_default_source)
+    effective_csp_parts.push_back(kAppSandboxScriptSrcDefaultDirective);
+
+  return base::JoinString(effective_csp_parts, " ");
+}
+
 bool ContentSecurityPolicyIsSandboxed(
     const std::string& policy, Manifest::Type type) {
   // See http://www.w3.org/TR/CSP/#parse-a-csp-policy for parsing algorithm.
   bool seen_sandbox = false;
   for (const std::string& input : base::SplitString(
            policy, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL)) {
     base::StringTokenizer tokenizer(input, " \t\r\n");
     if (!tokenizer.GetNext())
       continue;
 
@@ skipping 17 matching lines @@
       }
     }
   }
 
   return seen_sandbox;
 }
 
 }  // namespace csp_validator
 
 }  // namespace extensions