[IP Protection] Use ProxyChain from HttpProxySocketParams

This CL adds ProxyChain and the proxy chain index into
HttpProxySocketParams and uses it in the calls to
NetworkServiceProxyDelegate::OnTunnelHeadersReceived via
the proxy client sockets.

This CL also adds a test of the connect jobs in the case where
the SocketParams objects have been constructed to support
nested proxies. Some associated changes were necessary to get
this passing, specifically modifying some DCHECKs and adding
a secure_dns_policy member to HttpProxySocketParams.

As part of this, I also refactored some of the unit tests,
such as ssl_connect_job_unittest.cc where we now create
params similarly to how it's done in
http_proxy_connect_job_unittest.cc. In both I was able
to re-use existing helper methods for a DoH test.

I dont think this CL introduces any changes in
functionality.

Bug: 1491092
Change-Id: Icd20413aa0c350d33b79db2708377f5bbf929a41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4979730
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Andrew Williams <awillia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1217290}

net/base/proxy_delegate.h

@@ -72,18 +72,19 @@ class NET_EXPORT ProxyDelegate {
     OnFallback(ProxyChain(bad_server), net_error);
   }
 
-  void OnBeforeTunnelRequestServerOnly(const ProxyServer& proxy_server,
+  void OnBeforeTunnelRequestServerOnly(const ProxyChain& proxy_chain,
+                                       size_t proxy_chain_index,
                                        HttpRequestHeaders* extra_headers) {
-    DCHECK(!proxy_server.is_direct());
-    OnBeforeTunnelRequest(ProxyChain(proxy_server), /*chain_index=*/0,
-                          extra_headers);
+    DCHECK(!proxy_chain.is_direct());
+    OnBeforeTunnelRequest(proxy_chain, proxy_chain_index, extra_headers);
   }
 
   Error OnTunnelHeadersReceivedServerOnly(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t proxy_chain_index,
       const HttpResponseHeaders& response_headers) {
-    return OnTunnelHeadersReceived(ProxyChain(proxy_server),
-                                   /*chain_index=*/0, response_headers);
+    return OnTunnelHeadersReceived(proxy_chain, proxy_chain_index,
+                                   response_headers);
   }
 };
 

net/base/test_proxy_delegate.cc

@@ -23,12 +23,22 @@ void TestProxyDelegate::VerifyOnTunnelHeadersReceived(
     const ProxyChain& proxy_chain,
     size_t chain_index,
     const std::string& response_header_name,
-    const std::string& response_header_value) const {
-  EXPECT_EQ(proxy_chain, on_tunnel_headers_received_proxy_chain_);
-  EXPECT_EQ(chain_index, on_tunnel_headers_received_chain_index_);
-  ASSERT_NE(on_tunnel_headers_received_headers_.get(), nullptr);
-  EXPECT_TRUE(on_tunnel_headers_received_headers_->HasHeaderValue(
-      response_header_name, response_header_value));
+    const std::string& response_header_value,
+    size_t call_index) const {
+  ASSERT_LT(call_index, on_tunnel_headers_received_proxy_chains_.size());
+  ASSERT_LT(call_index, on_tunnel_headers_received_chain_indices_.size());
+  ASSERT_LT(call_index, on_tunnel_headers_received_headers_.size());
+
+  EXPECT_EQ(proxy_chain,
+            on_tunnel_headers_received_proxy_chains_.at(call_index));
+  EXPECT_EQ(chain_index,
+            on_tunnel_headers_received_chain_indices_.at(call_index));
+
+  scoped_refptr<HttpResponseHeaders> response_headers =
+      on_tunnel_headers_received_headers_.at(call_index);
+  ASSERT_NE(response_headers.get(), nullptr);
+  EXPECT_TRUE(response_headers->HasHeaderValue(response_header_name,
+                                               response_header_value));
 }
 
 void TestProxyDelegate::OnResolveProxy(
@@ -47,22 +57,22 @@ void TestProxyDelegate::OnBeforeTunnelRequest(
     HttpRequestHeaders* extra_headers) {
   on_before_tunnel_request_called_ = true;
   if (extra_headers) {
-    // TODO(crbug.com/1491092): Include the entire chain in the header.
-    extra_headers->SetHeader("Foo",
-                             ProxyServerToProxyUri(proxy_chain.proxy_server()));
+    extra_headers->SetHeader(
+        kTestHeaderName,
+        ProxyServerToProxyUri(proxy_chain.GetProxyServer(chain_index)));
   }
 }
 
 Error TestProxyDelegate::OnTunnelHeadersReceived(
     const ProxyChain& proxy_chain,
     size_t chain_index,
     const HttpResponseHeaders& response_headers) {
-  EXPECT_EQ(on_tunnel_headers_received_headers_.get(), nullptr);
-  on_tunnel_headers_received_headers_ =
-      base::MakeRefCounted<HttpResponseHeaders>(response_headers.raw_headers());
+  on_tunnel_headers_received_headers_.push_back(
+      base::MakeRefCounted<HttpResponseHeaders>(
+          response_headers.raw_headers()));
 
-  on_tunnel_headers_received_proxy_chain_ = proxy_chain;
-  on_tunnel_headers_received_chain_index_ = chain_index;
+  on_tunnel_headers_received_proxy_chains_.push_back(proxy_chain);
+  on_tunnel_headers_received_chain_indices_.push_back(chain_index);
   return OK;
 }
 

net/base/test_proxy_delegate.h

@@ -6,6 +6,7 @@
 #define NET_BASE_TEST_PROXY_DELEGATE_H_
 
 #include <string>
+#include <vector>
 
 #include "base/memory/scoped_refptr.h"
 #include "net/base/proxy_chain.h"
@@ -22,15 +23,24 @@ class TestProxyDelegate : public ProxyDelegate {
   TestProxyDelegate();
   ~TestProxyDelegate() override;
 
+  constexpr static char kTestHeaderName[] = "Foo";
+  // Note: `kTestSpdyHeaderName` should be a lowercase version of
+  // `kTestHeaderName`.
+  constexpr static char kTestSpdyHeaderName[] = "foo";
+
   bool on_before_tunnel_request_called() const {
     return on_before_tunnel_request_called_;
   }
 
-  void VerifyOnTunnelHeadersReceived(
-      const ProxyChain& proxy_chain,
-      size_t chain_index,
-      const std::string& response_header_name,
-      const std::string& response_header_value) const;
+  size_t on_tunnel_headers_received_call_count() {
+    return on_tunnel_headers_received_headers_.size();
+  }
+
+  void VerifyOnTunnelHeadersReceived(const ProxyChain& proxy_chain,
+                                     size_t chain_index,
+                                     const std::string& response_header_name,
+                                     const std::string& response_header_value,
+                                     size_t call_index = 0) const;
 
   // ProxyDelegate implementation:
   void OnResolveProxy(const GURL& url,
@@ -49,9 +59,10 @@ class TestProxyDelegate : public ProxyDelegate {
 
  private:
   bool on_before_tunnel_request_called_ = false;
-  ProxyChain on_tunnel_headers_received_proxy_chain_;
-  size_t on_tunnel_headers_received_chain_index_;
-  scoped_refptr<HttpResponseHeaders> on_tunnel_headers_received_headers_;
+  std::vector<ProxyChain> on_tunnel_headers_received_proxy_chains_;
+  std::vector<size_t> on_tunnel_headers_received_chain_indices_;
+  std::vector<scoped_refptr<HttpResponseHeaders>>
+      on_tunnel_headers_received_headers_;
 };
 
 }  // namespace net

net/http/http_proxy_client_socket.cc

@@ -16,7 +16,6 @@
 #include "net/base/io_buffer.h"
 #include "net/base/proxy_chain.h"
 #include "net/base/proxy_delegate.h"
-#include "net/base/proxy_server.h"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_log_util.h"
 #include "net/http/http_network_session.h"
@@ -36,7 +35,8 @@ HttpProxyClientSocket::HttpProxyClientSocket(
     std::unique_ptr<StreamSocket> socket,
     const std::string& user_agent,
     const HostPortPair& endpoint,
-    const ProxyServer& proxy_server,
+    const ProxyChain& proxy_chain,
+    size_t proxy_chain_index,
     scoped_refptr<HttpAuthController> http_auth_controller,
     ProxyDelegate* proxy_delegate,
     const NetworkTrafficAnnotationTag& traffic_annotation)
@@ -45,7 +45,8 @@ HttpProxyClientSocket::HttpProxyClientSocket(
       socket_(std::move(socket)),
       endpoint_(endpoint),
       auth_(std::move(http_auth_controller)),
-      proxy_server_(proxy_server),
+      proxy_chain_(proxy_chain),
+      proxy_chain_index_(proxy_chain_index),
       proxy_delegate_(proxy_delegate),
       traffic_annotation_(traffic_annotation),
       net_log_(socket_->NetLog()) {
@@ -353,9 +354,8 @@ int HttpProxyClientSocket::DoSendRequest() {
 
     if (proxy_delegate_) {
       HttpRequestHeaders proxy_delegate_headers;
-      // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
-      proxy_delegate_->OnBeforeTunnelRequestServerOnly(proxy_server_,
-                                                       &proxy_delegate_headers);
+      proxy_delegate_->OnBeforeTunnelRequestServerOnly(
+          proxy_chain_, proxy_chain_index_, &proxy_delegate_headers);
       extra_headers.MergeFrom(proxy_delegate_headers);
     }
 
@@ -406,9 +406,8 @@ int HttpProxyClientSocket::DoReadHeadersComplete(int result) {
       response_.headers.get());
 
   if (proxy_delegate_) {
-    // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
     int rv = proxy_delegate_->OnTunnelHeadersReceivedServerOnly(
-        proxy_server_, *response_.headers);
+        proxy_chain_, proxy_chain_index_, *response_.headers);
     if (rv != OK) {
       DCHECK_NE(ERR_IO_PENDING, rv);
       return rv;

net/http/http_proxy_client_socket.h

@@ -16,7 +16,7 @@
 #include "net/base/completion_repeating_callback.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_export.h"
-#include "net/base/proxy_server.h"
+#include "net/base/proxy_chain.h"
 #include "net/http/http_auth_controller.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
@@ -43,7 +43,8 @@ class NET_EXPORT_PRIVATE HttpProxyClientSocket : public ProxyClientSocket {
   HttpProxyClientSocket(std::unique_ptr<StreamSocket> socket,
                         const std::string& user_agent,
                         const HostPortPair& endpoint,
-                        const ProxyServer& proxy_server,
+                        const ProxyChain& proxy_chain,
+                        size_t proxy_chain_index,
                         scoped_refptr<HttpAuthController> http_auth_controller,
                         ProxyDelegate* proxy_delegate,
                         const NetworkTrafficAnnotationTag& traffic_annotation);
@@ -154,7 +155,8 @@ class NET_EXPORT_PRIVATE HttpProxyClientSocket : public ProxyClientSocket {
   std::string request_line_;
   HttpRequestHeaders request_headers_;
 
-  const ProxyServer proxy_server_;
+  const ProxyChain proxy_chain_;
+  const size_t proxy_chain_index_;
 
   // This delegate must outlive this proxy client socket.
   raw_ptr<ProxyDelegate> proxy_delegate_;

net/http/http_proxy_client_socket_fuzzer.cc

@@ -18,6 +18,8 @@
 #include "net/base/auth.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/network_anonymization_key.h"
+#include "net/base/proxy_chain.h"
+#include "net/base/proxy_server.h"
 #include "net/base/test_completion_callback.h"
 #include "net/http/http_auth_cache.h"
 #include "net/http/http_auth_handler_basic.h"
@@ -66,10 +68,10 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   // is HTTPS.
   net::HttpProxyClientSocket socket(
       std::move(fuzzed_socket), "Bond/007", net::HostPortPair("foo", 80),
-      net::ProxyServer(net::ProxyServer::SCHEME_HTTP,
-                       net::HostPortPair("proxy", 42)),
-      auth_controller.get(), nullptr /* proxy_delegate */,
-      TRAFFIC_ANNOTATION_FOR_TESTS);
+      net::ProxyChain(net::ProxyServer::SCHEME_HTTP,
+                      net::HostPortPair("proxy", 42)),
+      /*proxy_chain_index=*/0, auth_controller.get(),
+      /*proxy_delegate=*/nullptr, TRAFFIC_ANNOTATION_FOR_TESTS);
   int result = socket.Connect(callback.callback());
   result = callback.GetResult(result);
 

net/http/http_proxy_client_socket_unittest.cc

@@ -7,7 +7,7 @@
 #include "build/build_config.h"
 #include "net/base/address_list.h"
 #include "net/base/host_port_pair.h"
-#include "net/base/proxy_server.h"
+#include "net/base/proxy_chain.h"
 #include "net/socket/next_proto.h"
 #include "net/socket/socket_tag.h"
 #include "net/socket/socket_test_util.h"
@@ -28,7 +28,8 @@ TEST(HttpProxyClientSocketTest, Tag) {
   // |socket| takes ownership of |tagging_sock|, but the test keeps a non-owning
   // pointer to it.
   HttpProxyClientSocket socket(
-      std::move(tagging_sock), /*user_agent=*/"", HostPortPair(), ProxyServer(),
+      std::move(tagging_sock), /*user_agent=*/"", HostPortPair(), ProxyChain(),
+      /*proxy_chain_index=*/0,
       /*http_auth_controller=*/nullptr,
       /*proxy_delegate=*/nullptr, TRAFFIC_ANNOTATION_FOR_TESTS);