send each credential only to its own hop in write_request (#2579)

* send each credential only to its own hop in write_request

An SSLClient behind a proxy sent Proxy-Authorization inside the TLS tunnel, where the origin reads it, and sent the origin's Authorization on the CONNECT request the proxy reads. Attach each only on the message its hop actually reads.

* Keep default headers off the CONNECT request

set_default_headers() is typically used for origin credentials such as
Authorization, Cookie or API keys, but they were also attached to the
CONNECT request an SSLClient sends to its proxy, in plaintext before the
TLS tunnel exists. Default headers now go only on requests the origin
reads, the same split the previous commit makes for set_basic_auth and
set_bearer_token_auth.

Claude-Session: https://claude.ai/code/session_01JYPWKpbp4a881EdpEf2xSi

* Simplify per-hop credential handling and its tests

Flatten the Authorization insertion in write_request into one guard with
an else-if (Basic already took precedence over Bearer), and shorten the
comments around it. Fold DefaultHeadersStayOffConnect into the
CredentialsStayWithTheirHop helper, which now takes the list of headers
that must reach only the origin.

Claude-Session: https://claude.ai/code/session_01JYPWKpbp4a881EdpEf2xSi

---------

Co-authored-by: yhirose <yuji.hirose.bug@gmail.com>

httplib.h

@@ -14929,8 +14929,12 @@ inline Result ClientImpl::send_(Request &&req) {
 inline void ClientImpl::prepare_default_headers(Request &r, bool for_stream,
                                                 const std::string &ct) {
   (void)for_stream;
-  for (const auto &header : default_headers_) {
-    if (!r.has_header(header.first)) { r.headers.insert(header); }
+  // Default headers are meant for the origin and may carry its credentials, so
+  // keep them off the CONNECT request the proxy reads.
+  if (r.method != "CONNECT") {
+    for (const auto &header : default_headers_) {
+      if (!r.has_header(header.first)) { r.headers.insert(header); }
+    }
   }
 
   // RFC 9110 5.3 recommends sending control data such as Host first, so
@@ -15611,24 +15615,24 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req,
     }
   }
 
-  if (!basic_auth_password_.empty() || !basic_auth_username_.empty()) {
-    if (!req.has_header("Authorization")) {
+  // A CONNECT request is read by the proxy; everything sent through the tunnel
+  // it opens is read by the origin. Each credential goes only to its own hop.
+  auto is_connect = req.method == "CONNECT";
+
+  if (!is_connect && !req.has_header("Authorization")) {
+    if (!basic_auth_password_.empty() || !basic_auth_username_.empty()) {
       req.headers.insert(make_basic_authentication_header(
           basic_auth_username_, basic_auth_password_, false));
-    }
-  }
-
-  if (!bearer_token_auth_token_.empty()) {
-    if (!req.has_header("Authorization")) {
+    } else if (!bearer_token_auth_token_.empty()) {
       req.headers.insert(make_bearer_token_authentication_header(
           bearer_token_auth_token_, false));
     }
   }
 
-  // Proxy-Authorization is only sent when the proxy is actually used for
-  // this target — otherwise NO_PROXY-matched requests would leak proxy
-  // credentials directly to the destination server.
-  if (is_proxy_enabled_for_host(host_)) {
+  // Proxy-Authorization is only sent when the proxy reads this message —
+  // otherwise NO_PROXY-matched requests, and requests inside a TLS tunnel,
+  // would leak proxy credentials to the destination server.
+  if (is_proxy_enabled_for_host(host_) && (!is_ssl() || is_connect)) {
     if (!proxy_basic_auth_username_.empty() &&
         !proxy_basic_auth_password_.empty() &&
         !req.has_header("Proxy-Authorization")) {

test/test.cc

@@ -25353,6 +25353,11 @@ public:
 
   int port() const { return port_; }
   int connect_hits() const { return connect_hits_.load(); }
+  // Head of the last CONNECT request, as the proxy saw it on the wire.
+  std::string connect_request() const {
+    std::lock_guard<std::mutex> lock(connect_request_mutex_);
+    return connect_request_;
+  }
 
 private:
   void run() {
@@ -25379,6 +25384,10 @@ private:
         continue;
       }
       connect_hits_++;
+      {
+        std::lock_guard<std::mutex> lock(connect_request_mutex_);
+        connect_request_ = req;
+      }
 
       const char *ok = "HTTP/1.1 200 Connection established\r\n\r\n";
       ::send(client_fd, ok, std::strlen(ok), 0);
@@ -25428,10 +25437,92 @@ private:
   std::thread th_;
   std::atomic<bool> stop_{false};
   std::atomic<int> connect_hits_{0};
+  mutable std::mutex connect_request_mutex_;
+  std::string connect_request_;
 };
 
+// Sends one request through the CONNECT proxy to the TLS origin with a
+// credential configured for each hop, and checks that each credential reaches
+// only the hop it was configured for: the proxy's on the CONNECT request, the
+// origin's (every header in origin_headers) on the tunnelled request.
+void CredentialsStayWithTheirHop(
+    const std::function<void(SSLClient &)> &set_credentials,
+    const std::string &scheme,
+    const std::vector<std::string> &origin_headers = {"Authorization"}) {
+  std::atomic<int> origin_hits{0};
+  std::atomic<bool> origin_saw_proxy_authz{false};
+  std::atomic<bool> origin_saw_headers{false};
+
+  ScopedSSLServer origin;
+  origin.svr().Get(".*", [&](const Request &req, Response &res) {
+    origin_hits++;
+    origin_saw_proxy_authz = req.has_header("Proxy-Authorization");
+    origin_saw_headers =
+        std::all_of(origin_headers.begin(), origin_headers.end(),
+                    [&](const std::string &h) { return req.has_header(h); });
+    res.set_content("ok", "text/plain");
+  });
+  origin.listen();
+
+  ScopedConnectProxy proxy(origin.port());
+  ASSERT_NE(0, proxy.port());
+
+  // Pinned to 127.0.0.1 for the same reason as the test below.
+  SSLClient cli("127.0.0.1", origin.port());
+  cli.enable_server_certificate_verification(false);
+  cli.set_proxy("127.0.0.1", proxy.port());
+  set_credentials(cli);
+
+  auto res = cli.Get("/x");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::OK_200, res->status);
+  EXPECT_EQ(1, origin_hits.load());
+  EXPECT_EQ(1, proxy.connect_hits());
+
+  EXPECT_TRUE(origin_saw_headers.load());
+  EXPECT_FALSE(origin_saw_proxy_authz.load())
+      << "Proxy-Authorization must not be sent inside the tunnel";
+
+  auto connect_req = proxy.connect_request();
+  EXPECT_NE(std::string::npos,
+            connect_req.find("\r\nProxy-Authorization: " + scheme + " "));
+  for (const auto &h : origin_headers) {
+    EXPECT_EQ(std::string::npos, connect_req.find("\r\n" + h + ": "))
+        << h << " must not be sent to the proxy on CONNECT";
+  }
+}
+
 } // namespace proxy_tunnel_test
 
+TEST(ProxyTunnelTest, BasicCredentialsStayWithTheirHop) {
+  proxy_tunnel_test::CredentialsStayWithTheirHop(
+      [](SSLClient &cli) {
+        cli.set_proxy_basic_auth("proxy-user", "proxy-pass");
+        cli.set_basic_auth("origin-user", "origin-pass");
+      },
+      "Basic");
+}
+
+TEST(ProxyTunnelTest, BearerCredentialsStayWithTheirHop) {
+  proxy_tunnel_test::CredentialsStayWithTheirHop(
+      [](SSLClient &cli) {
+        cli.set_proxy_bearer_token_auth("proxy-token");
+        cli.set_bearer_token_auth("origin-token");
+      },
+      "Bearer");
+}
+
+TEST(ProxyTunnelTest, DefaultHeadersStayOffConnect) {
+  proxy_tunnel_test::CredentialsStayWithTheirHop(
+      [](SSLClient &cli) {
+        cli.set_proxy_basic_auth("proxy-user", "proxy-pass");
+        cli.set_default_headers({{"Authorization", "Bearer origin-token"},
+                                 {"Cookie", "sid=origin-session"},
+                                 {"X-Api-Key", "origin-key"}});
+      },
+      "Basic", {"Authorization", "Cookie", "X-Api-Key"});
+}
+
 TEST(ProxyTunnelTest, OriginReturning407InsideTunnelDoesNotLeakProxyDigest) {
   // Origin inside a CONNECT tunnel replying 407 must not trigger the digest
   // retry; otherwise proxy creds would be sent to the origin.