CSP: move 'upgrade-insecure-request' out of |directives|.

Up to very recently, the CSP mojo struct only contained directives using
a list of CSP sources, except for 'upgrade-insecure-requests', which
used an empty list.

In the next few weeks. Two new directives will be added:
- treat-as-public-address (https://crbug.com/1042164)
- sandbox (https://crbug.com/1041376)

So there will be 3 kind of directives:
  1) The ones containing a set of CSPSources.
  2) The ones with no data.
  3) The WebSandboxPolicy one.

[Question]
Can we continue storing everything in the map |directive|?
|directive| is: map: CSPDirectiveName -> CSPSourceList.

To continue [A], we could make |directives| to store a mojo union.
union {
  CSPSourceList source_list
  WebSandboxFlags sandbox_flags
  void none.
}

or we could stop storing everything in |directives| [B].
Instead, we could just add more attributes.

It is not clear to me what in between [A] and [B] to be preferable. This
patch tries [B]. Please let me know if you think otherwise.

[A] would gives:
~~~
- csp->directives[kSandbox].get_sandbox_flag();
- csp->directives[FrameAncestors].get_source_list();
~~~

[B] would gives:
~~
- csp->sandbox
- csp->directives[FrameAncestors];
~~

cons of [B] is that we need to update all the conversions from/to this
CSP struct in between non-blink/blink-public/blink-private.

Bug: 1041376
Change-Id: I8ed473bca2d47beaab82187a68204cc10643e0ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2165113Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763439}

content/renderer/content_security_policy_util.cc

@@ -42,6 +42,7 @@ network::mojom::ContentSecurityPolicyPtr BuildContentSecurityPolicy(
     auto name = network::ToCSPDirectiveName(directive.name.Utf8());
     policy->directives[name] = BuildCSPSourceList(directive.source_list);
   }
+  policy->upgrade_insecure_requests = policy_in.upgrade_insecure_requests;
 
   for (const blink::WebString& endpoint : policy_in.report_endpoints)
     policy->report_endpoints.push_back(endpoint.Utf8());

services/network/public/cpp/content_security_policy/content_security_policy.cc

@@ -32,7 +32,6 @@ static CSPDirectiveName CSPFallback(CSPDirectiveName directive) {
   switch (directive) {
     case CSPDirectiveName::DefaultSrc:
     case CSPDirectiveName::FormAction:
-    case CSPDirectiveName::UpgradeInsecureRequests:
     case CSPDirectiveName::NavigateTo:
     case CSPDirectiveName::FrameAncestors:
       return CSPDirectiveName::Unknown;
@@ -78,7 +77,6 @@ const char* ErrorMessage(CSPDirectiveName directive) {
     case CSPDirectiveName::ChildSrc:
     case CSPDirectiveName::DefaultSrc:
     case CSPDirectiveName::Unknown:
-    case CSPDirectiveName::UpgradeInsecureRequests:
       NOTREACHED();
       return nullptr;
   };
@@ -554,7 +552,7 @@ bool CheckContentSecurityPolicy(const mojom::ContentSecurityPolicyPtr& policy,
 bool ShouldUpgradeInsecureRequest(
     const std::vector<mojom::ContentSecurityPolicyPtr>& policies) {
   for (const auto& policy : policies) {
-    if (policy->directives.count(CSPDirectiveName::UpgradeInsecureRequests))
+    if (policy->upgrade_insecure_requests)
       return true;
   }
 
@@ -587,8 +585,6 @@ CSPDirectiveName ToCSPDirectiveName(const std::string& name) {
     return CSPDirectiveName::FrameSrc;
   if (name == "form-action")
     return CSPDirectiveName::FormAction;
-  if (name == "upgrade-insecure-requests")
-    return CSPDirectiveName::UpgradeInsecureRequests;
   if (name == "navigate-to")
     return CSPDirectiveName::NavigateTo;
   if (name == "frame-ancestors")
@@ -606,8 +602,6 @@ std::string ToString(CSPDirectiveName name) {
       return "frame-src";
     case CSPDirectiveName::FormAction:
       return "form-action";
-    case CSPDirectiveName::UpgradeInsecureRequests:
-      return "upgrade-insecure-requests";
     case CSPDirectiveName::NavigateTo:
       return "navigate-to";
     case CSPDirectiveName::FrameAncestors:

services/network/public/cpp/content_security_policy/content_security_policy_unittest.cc

@@ -485,7 +485,7 @@ TEST(ContentSecurityPolicy, ShouldUpgradeInsecureRequest) {
   EXPECT_FALSE(ShouldUpgradeInsecureRequest(policies));
 
   policies.push_back(mojom::ContentSecurityPolicy::New());
-  policies[0]->directives[mojom::CSPDirectiveName::UpgradeInsecureRequests];
+  policies[0]->upgrade_insecure_requests = true;
 
   EXPECT_TRUE(ShouldUpgradeInsecureRequest(policies));
 }
@@ -494,7 +494,7 @@ TEST(ContentSecurityPolicy, ShouldUpgradeInsecureRequest) {
 TEST(ContentSecurityPolicy, UpgradeInsecureRequests) {
   std::vector<mojom::ContentSecurityPolicyPtr> policies;
   policies.push_back(mojom::ContentSecurityPolicy::New());
-  policies[0]->directives[mojom::CSPDirectiveName::UpgradeInsecureRequests];
+  policies[0]->upgrade_insecure_requests = true;
 
   struct {
     std::string input;

services/network/public/mojom/content_security_policy.mojom

@@ -78,7 +78,6 @@ enum CSPDirectiveName {
   ChildSrc,
   FrameSrc,
   FormAction,
-  UpgradeInsecureRequests,
   NavigateTo,
   FrameAncestors,
 };
@@ -86,6 +85,11 @@ enum CSPDirectiveName {
 struct ContentSecurityPolicy {
   map<CSPDirectiveName, CSPSourceList> directives;
 
+  // Spec: https://www.w3.org/TR/upgrade-insecure-requests/
+  bool upgrade_insecure_requests = false;
+  // TODO(https://crbug.com/1042164) Add 'treat-as-public-address'.
+  // TODO(https://crbug.com/1041376) Add 'sandbox'.
+
   ContentSecurityPolicyHeader header;
 
   // Whether this CSP policy uses the new reporting API.

third_party/blink/public/platform/web_content_security_policy_struct.h

@@ -77,6 +77,7 @@ struct WebContentSecurityPolicy {
   network::mojom::ContentSecurityPolicyType disposition;
   network::mojom::ContentSecurityPolicySource source;
   WebVector<WebContentSecurityPolicyDirective> directives;
+  bool upgrade_insecure_requests;
   WebVector<WebString> report_endpoints;
   WebString header;
   bool use_reporting_api;

third_party/blink/renderer/core/exported/local_frame_client_impl.cc

@@ -199,8 +199,6 @@ WebString ConvertToPublic(
       return "frame-src";
     case CSPDirectiveName::FormAction:
       return "form-action";
-    case CSPDirectiveName::UpgradeInsecureRequests:
-      return "upgrade-insecure-requests";
     case CSPDirectiveName::NavigateTo:
       return "navigate-to";
     case CSPDirectiveName::FrameAncestors:
@@ -223,9 +221,13 @@ WebContentSecurityPolicy ConvertToPublic(
                        ConvertToPublic(std::move(directive.value))};
   }
 
-  return {policy->header->type,         policy->header->source,
-          std::move(directives),        std::move(policy->report_endpoints),
-          policy->header->header_value, policy->use_reporting_api};
+  return {policy->header->type,
+          policy->header->source,
+          std::move(directives),
+          policy->upgrade_insecure_requests,
+          std::move(policy->report_endpoints),
+          policy->header->header_value,
+          policy->use_reporting_api};
 }
 
 }  // namespace

third_party/blink/renderer/core/frame/csp/csp_directive_list.cc

@@ -1635,13 +1635,7 @@ CSPDirectiveList::ExposeForNavigationalChecks() const {
                            navigate_to_->ExposeForNavigationalChecks());
   }
 
-  if (upgrade_insecure_requests_) {
-    auto empty_source_list = network::mojom::blink::CSPSourceList::New(
-        WTF::Vector<network::mojom::blink::CSPSourcePtr>(), false, false,
-        false);
-    policy->directives.Set(CSPDirectiveName::UpgradeInsecureRequests,
-                           std::move(empty_source_list));
-  }
+  policy->upgrade_insecure_requests = upgrade_insecure_requests_;
 
   return policy;
 }

third_party/blink/renderer/platform/network/http_parsers.cc

@@ -92,10 +92,11 @@ blink::ContentSecurityPolicyPtr ConvertToBlink(
   policy->header = ConvertToBlink(std::move(policy_in->header));
   policy->use_reporting_api = policy_in->use_reporting_api;
 
-  for (auto& directive : policy_in->directives) {
-    policy->directives.insert(ConvertToBlink(directive.first),
-                              ConvertToBlink(std::move(directive.second)));
+  for (auto& list : policy_in->directives) {
+    policy->directives.insert(ConvertToBlink(list.first),
+                              ConvertToBlink(std::move(list.second)));
   }
+  policy->upgrade_insecure_requests = policy_in->upgrade_insecure_requests;
 
   for (auto& endpoint : policy_in->report_endpoints)
     policy->report_endpoints.push_back(String::FromUTF8(endpoint));

third_party/blink/renderer/platform/network/http_parsers_test.cc

@@ -687,8 +687,9 @@ TEST(HTTPParsersTest, ParseContentSecurityPolicyDirectiveName) {
       "Content-Security-Policy: frame-src 'none'\r\n"
       "Content-Security-Policy: child-src 'none'\r\n"
       "Content-Security-Policy: script-src 'none'\r\n"
-      "Content-Security-Policy: default-src 'none'\r\n");
-  EXPECT_EQ(8u, policies.size());
+      "Content-Security-Policy: default-src 'none'\r\n"
+      "Content-Security-Policy: upgrade-insecure-requests\r\n");
+  EXPECT_EQ(9u, policies.size());
   // frame-ancestors
   EXPECT_EQ(1u, policies[0]->directives.size());
   // sandbox. TODO(https://crbug.com/1041376) Implement this.
@@ -705,6 +706,8 @@ TEST(HTTPParsersTest, ParseContentSecurityPolicyDirectiveName) {
   EXPECT_EQ(0u, policies[6]->directives.size());
   // default-src. Not parsed.
   EXPECT_EQ(0u, policies[7]->directives.size());
+  // upgrade-insecure-policies. Not parsed.
+  EXPECT_EQ(false, policies[8]->upgrade_insecure_requests);
 }
 
 TEST(HTTPParsersTest, ParseContentSecurityPolicyReportTo) {