Block private network requests from insecure contexts.

This CL blocks private network requests coming from insecure contexts when the
network feature kBlockInsecurePrivateNetworkRequests is enabled. That feature
is renamed to reflect the name change in the spec. Right now, the feature is
disabled by default.

Bug: 986744
Change-Id: I78c1aa26d1e385ba8b10e13e08545ce8aada7867
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2260423Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Auto-Submit: Titouan Rigoudy <titouan@chromium.org>
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795531}

content/browser/frame_host/navigation_request.cc

@@ -122,6 +122,7 @@
 #include "third_party/blink/public/mojom/web_feature/web_feature.mojom.h"
 #include "third_party/blink/public/platform/resource_request_blocked_reason.h"
 #include "third_party/blink/public/platform/web_mixed_content_context_type.h"
+#include "url/origin.h"
 #include "url/url_constants.h"
 
 namespace content {
@@ -4182,6 +4183,14 @@ void NavigationRequest::ReadyToCommitNavigation(CommitPageType type) {
                        : empty_csp);
     commit_params_->ip_address_space = ip_address_space;
     client_security_state_->ip_address_space = ip_address_space;
+
+    // TODO(crbug.com/986744): Check whether this is the correct way of
+    // detecting a secure context, amend if not. The current origin being
+    // trustworthy is a necessary condition for secure contexts, but it might
+    // not be sufficient.
+    client_security_state_->is_web_secure_context =
+        network::IsOriginPotentiallyTrustworthy(
+            url::Origin::Create(common_params_->url));
   }
 
   if (appcache_handle_) {

content/child/runtime_features.cc

@@ -350,7 +350,8 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
   // function and using feature string name with EnableFeatureFromString.
   const RuntimeFeatureToChromiumFeatureMap<const char*>
       runtimeFeatureNameToChromiumFeatureMapping[] = {
-          {"AddressSpace", network::features::kBlockNonSecureExternalRequests,
+          {"AddressSpace",
+           network::features::kBlockInsecurePrivateNetworkRequests,
            kEnableOnly},
           {"AllowContentInitiatedDataUrlNavigations",
            features::kAllowContentInitiatedDataUrlNavigations,

content/public/test/url_loader_interceptor.cc

@@ -542,8 +542,8 @@ URLLoaderInterceptor::ServeFilesFromDirectoryAtOrigin(
 }
 
 void URLLoaderInterceptor::WriteResponse(
-    const std::string& headers,
-    const std::string& body,
+    base::StringPiece headers,
+    base::StringPiece body,
     network::mojom::URLLoaderClient* client,
     base::Optional<net::SSLInfo> ssl_info) {
   net::HttpResponseInfo info;
@@ -555,17 +555,7 @@ void URLLoaderInterceptor::WriteResponse(
   response->ssl_info = std::move(ssl_info);
   client->OnReceiveResponse(std::move(response));
 
-  uint32_t bytes_written = body.size();
-  mojo::DataPipe data_pipe(body.size());
-  CHECK_EQ(MOJO_RESULT_OK,
-           data_pipe.producer_handle->WriteData(
-               body.data(), &bytes_written, MOJO_WRITE_DATA_FLAG_ALL_OR_NONE));
-  client->OnStartLoadingResponseBody(std::move(data_pipe.consumer_handle));
-
-  network::URLLoaderCompletionStatus status;
-  status.decoded_body_length = body.size();
-  status.error_code = net::OK;
-  client->OnComplete(status);
+  CHECK_EQ(WriteResponseBody(body, client), MOJO_RESULT_OK);
 }
 
 void URLLoaderInterceptor::WriteResponse(
@@ -599,6 +589,41 @@ void URLLoaderInterceptor::WriteResponse(
   WriteResponse(headers_str, ReadFile(file_path), client, std::move(ssl_info));
 }
 
+MojoResult URLLoaderInterceptor::WriteResponseBody(
+    base::StringPiece body,
+    network::mojom::URLLoaderClient* client) {
+  mojo::ScopedDataPipeProducerHandle producer_handle;
+  mojo::ScopedDataPipeConsumerHandle consumer_handle;
+
+  MojoCreateDataPipeOptions options;
+  options.struct_size = sizeof(MojoCreateDataPipeOptions);
+  options.flags = MOJO_CREATE_DATA_PIPE_FLAG_NONE;
+  options.element_num_bytes = 1;
+  options.capacity_num_bytes = body.size();
+
+  MojoResult result =
+      CreateDataPipe(&options, &producer_handle, &consumer_handle);
+  if (result != MOJO_RESULT_OK) {
+    return result;
+  }
+
+  uint32_t bytes_written = body.size();
+  result = producer_handle->WriteData(body.data(), &bytes_written,
+                                      MOJO_WRITE_DATA_FLAG_ALL_OR_NONE);
+  if (result != MOJO_RESULT_OK) {
+    return result;
+  }
+
+  client->OnStartLoadingResponseBody(std::move(consumer_handle));
+
+  network::URLLoaderCompletionStatus status;
+  status.decoded_body_length = body.size();
+  status.error_code = net::OK;
+  client->OnComplete(status);
+
+  return MOJO_RESULT_OK;
+}
+
 void URLLoaderInterceptor::CreateURLLoaderFactoryForRenderProcessHost(
     mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
     int process_id,

content/public/test/url_loader_interceptor.h

@@ -12,6 +12,7 @@
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/strings/string_piece.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -109,8 +110,8 @@ class URLLoaderInterceptor {
   // Helper methods for use when intercepting.
   // Writes the given response body, header, and SSL Info to |client|.
   static void WriteResponse(
-      const std::string& headers,
-      const std::string& body,
+      base::StringPiece headers,
+      base::StringPiece body,
       network::mojom::URLLoaderClient* client,
       base::Optional<net::SSLInfo> ssl_info = base::nullopt);
 
@@ -135,6 +136,11 @@ class URLLoaderInterceptor {
       const std::string* headers = nullptr,
       base::Optional<net::SSLInfo> ssl_info = base::nullopt);
 
+  // Attempts to write |body| to |client| and complete the load with status OK.
+  // client->OnReceiveResponse() must have been called prior to this.
+  static MojoResult WriteResponseBody(base::StringPiece body,
+                                      network::mojom::URLLoaderClient* client);
+
   // Returns an interceptor that (as long as it says alive) will intercept
   // requests to |url| and fail them using the provided |error|.
   // |ready_callback| is optional and avoids the use of RunLoop, see

net/base/net_error_list.h

@@ -127,6 +127,11 @@ NET_ERROR(BLOCKED_BY_CSP, -30)
 // The request was blocked because of no H/2 or QUIC session.
 NET_ERROR(H2_OR_QUIC_REQUIRED, -31)
 
+// The request was blocked because it is a private network request coming from
+// an insecure context in a less private IP address space. This is used to
+// enforce CORS-RFC1918: https://wicg.github.io/cors-rfc1918.
+NET_ERROR(INSECURE_PRIVATE_NETWORK_REQUEST, -32)
+
 // A connection was closed (corresponding to a TCP FIN).
 NET_ERROR(CONNECTION_CLOSED, -100)
 

services/network/public/cpp/features.cc

@@ -110,13 +110,13 @@ const base::Feature kCrossOriginEmbedderPolicy{
 const base::Feature kCrossOriginIsolated{"CrossOriginIsolated",
                                          base::FEATURE_DISABLED_BY_DEFAULT};
 
-// When kBlockNonSecureExternalRequests is enabled, requests initiated from a
-// pubic network may only target a private network if the initiating context
-// is secure.
+// When kBlockInsecurePrivateNetworkRequests is enabled, requests initiated
+// from a less-private network may only target a more-private network if the
+// initiating context is secure.
 //
 // https://wicg.github.io/cors-rfc1918/#integration-fetch
-const base::Feature kBlockNonSecureExternalRequests{
-    "BlockNonSecureExternalRequests", base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kBlockInsecurePrivateNetworkRequests{
+    "BlockInsecurePrivateNetworkRequests", base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Enables or defaults splittup up server (not proxy) entries in the
 // HttpAuthCache.

services/network/public/cpp/features.h

@@ -45,7 +45,7 @@ extern const base::Feature kCrossOriginEmbedderPolicy;
 COMPONENT_EXPORT(NETWORK_CPP)
 extern const base::Feature kCrossOriginIsolated;
 COMPONENT_EXPORT(NETWORK_CPP)
-extern const base::Feature kBlockNonSecureExternalRequests;
+extern const base::Feature kBlockInsecurePrivateNetworkRequests;
 COMPONENT_EXPORT(NETWORK_CPP)
 extern const base::Feature kSplitAuthCacheByNetworkIsolationKey;
 COMPONENT_EXPORT(NETWORK_CPP)

services/network/public/mojom/network_context.mojom

@@ -580,6 +580,7 @@ struct ClientSecurityState {
   CrossOriginEmbedderPolicy cross_origin_embedder_policy;
 
   // Whether the initiator of the requests is in a web secure context.
+  // See: https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts
   bool is_web_secure_context = false;
 
   // The initiator's IP AddressSpace.

services/network/url_loader.cc

@@ -28,10 +28,10 @@
 #include "base/trace_event/trace_event.h"
 #include "mojo/public/cpp/system/simple_watcher.h"
 #include "net/base/elements_upload_data_stream.h"
-#include "net/base/ip_endpoint.h"
 #include "net/base/isolation_info.h"
 #include "net/base/load_flags.h"
 #include "net/base/mime_sniffer.h"
+#include "net/base/transport_info.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/base/upload_file_element_reader.h"
 #include "net/cookies/canonical_cookie.h"
@@ -54,6 +54,7 @@
 #include "services/network/public/cpp/cross_origin_resource_policy.h"
 #include "services/network/public/cpp/empty_url_loader_client.h"
 #include "services/network/public/cpp/header_util.h"
+#include "services/network/public/cpp/ip_address_space_util.h"
 #include "services/network/public/cpp/net_adapters.h"
 #include "services/network/public/cpp/network_switches.h"
 #include "services/network/public/cpp/origin_policy.h"
@@ -981,6 +982,56 @@ void URLLoader::ResumeReadingBodyFromNet() {
   }
 }
 
+int URLLoader::OnConnected(net::URLRequest* url_request,
+                           const net::TransportInfo& info) {
+  DCHECK_EQ(url_request, url_request_.get());
+
+  DVLOG(1) << "Connection obtained for URL request to " << url_request->url()
+           << ": " << info;
+
+  // We use this opportunity to check if the request initiator should be allowed
+  // to make requests to the remote endpoint.
+  // See the CORS-RFC1918 spec: https://wicg.github.io/cors-rfc1918.
+
+  const mojom::ClientSecurityStatePtr& security_state =
+      factory_params_->client_security_state;
+  if (!security_state) {
+    DVLOG(1) << "Skipping CORS-RFC1918 check: missing client security state.";
+    return net::OK;
+  }
+
+  if (!base::FeatureList::IsEnabled(
+          network::features::kBlockInsecurePrivateNetworkRequests)) {
+    DVLOG(1) << "Skipping CORS-RFC1918 check: feature disabled.";
+    return net::OK;
+  }
+
+  // Now that the request endpoint's address has been resolved, check if this
+  // request should be blocked. We block requests to subresources in IP address
+  // spaces less public than that of the parent resource, when those requests
+  // are initiated from insecure contexts. This prevents malicious public
+  // websites from making requests to someone's printer, for example.
+
+  const mojom::IPAddressSpace remote_address_space =
+      IPAddressToIPAddressSpace(info.endpoint.address());
+  const bool is_endpoint_less_public = IsLessPublicAddressSpace(
+      remote_address_space, security_state->ip_address_space);
+
+  DVLOG(1) << "Performing CORS-RFC1918 check: client address space: "
+           << security_state->ip_address_space
+           << ", is_secure_context: " << security_state->is_web_secure_context
+           << ", remote address space: " << remote_address_space;
+
+  if (is_endpoint_less_public && !security_state->is_web_secure_context) {
+    DVLOG(1) << "CORS-RFC1918 check failed: "
+             << "blocking insecure private network request.";
+    return net::ERR_INSECURE_PRIVATE_NETWORK_REQUEST;
+  }
+
+  DVLOG(1) << "CORS-RFC1918 check succeeded.";
+  return net::OK;
+}
+
 void URLLoader::OnReceivedRedirect(net::URLRequest* url_request,
                                    const net::RedirectInfo& redirect_info,
                                    bool* defer_redirect) {

services/network/url_loader.h

@@ -46,7 +46,9 @@
 
 namespace net {
 class HttpResponseHeaders;
+class IPEndPoint;
 struct RedirectInfo;
+struct TransportInfo;
 class URLRequestContext;
 }  // namespace net
 
@@ -134,6 +136,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
   void ResumeReadingBodyFromNet() override;
 
   // net::URLRequest::Delegate implementation:
+  int OnConnected(net::URLRequest* url_request,
+                  const net::TransportInfo& info) override;
   void OnReceivedRedirect(net::URLRequest* url_request,
                           const net::RedirectInfo& redirect_info,
                           bool* defer_redirect) override;