Send TCS polling secret in X-Tsunami-TCS-Secret header (CWE-598 client-side half)

plugin/src/main/java/com/google/tsunami/plugin/TcsClient.java

@@ -39,6 +39,14 @@ public final class TcsClient {
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
   private static final JsonFormat.Parser jsonParser = JsonFormat.parser();
 
+  // Per-scan correlation secret used to be sent in the polling URL query string
+  // ('/?secret=<raw>'), but URL query strings are routinely captured by web-server access
+  // logs, cloud LB logs, browser history / Referer, ps-output of debugging curl invocations,
+  // and CDN analytics — none of which are appropriate channels for a secret. The polling
+  // server (callback-server) accepts the secret via this header instead. See
+  // https://www.rfc-editor.org/rfc/rfc9110#section-17.9 (Sensitive Information in URIs).
+  private static final String SECRET_HEADER_NAME = "X-Tsunami-TCS-Secret";
+
   private final String callbackAddress;
   private final int callbackPort;
   private final String pollingBaseUrl;
@@ -127,9 +135,12 @@ public boolean hasOobLog(String secretString) {
 
   private Optional<PollingResult> sendPollingRequest(String secretString) {
     HttpRequest request =
-        HttpRequest.get(
-                String.format("%s/?secret=%s", removeTrailingSlashes(pollingBaseUrl), secretString))
-            .setHeaders(HttpHeaders.builder().addHeader("Cache-Control", "no-cache").build())
+        HttpRequest.get(removeTrailingSlashes(pollingBaseUrl) + "/")
+            .setHeaders(
+                HttpHeaders.builder()
+                    .addHeader("Cache-Control", "no-cache")
+                    .addHeader(SECRET_HEADER_NAME, secretString)
+                    .build())
             .build();
     try {
       HttpResponse response = httpClient.send(request);

plugin/src/test/java/com/google/tsunami/plugin/TcsClientTest.java

@@ -29,6 +29,7 @@
 import javax.inject.Inject;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.mockwebserver.RecordedRequest;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -154,8 +155,11 @@ public void isVulnerable_sendsValidPollingRequest() throws IOException, Interrup
 
     client.hasOobLog(SECRET);
 
-    assertThat(mockWebServer.takeRequest().getPath())
-        .isEqualTo(String.format("/?secret=%s", SECRET));
+    RecordedRequest recordedRequest = mockWebServer.takeRequest();
+    // Secret travels in a request header, not in the URL query string. The path is "/"
+    // with no query parameters; the X-Tsunami-TCS-Secret header carries the raw secret.
+    assertThat(recordedRequest.getPath()).isEqualTo("/");
+    assertThat(recordedRequest.getHeader("X-Tsunami-TCS-Secret")).isEqualTo(SECRET);
     mockWebServer.shutdown();
   }
 

plugin_server/py/plugin/tcs_client.py

@@ -11,6 +11,14 @@
 import polling_pb2
 
 
+# Per-scan correlation secret used to be sent in the polling URL query string
+# ('/?secret=<raw>'), but URL query strings are routinely captured by web-server access
+# logs, cloud LB logs, browser history / Referer, ps-output of debugging curl invocations,
+# and CDN analytics — none of which are appropriate channels for a secret. The polling
+# server (callback-server) accepts the secret via this header instead.
+_SECRET_HEADER_NAME = 'X-Tsunami-TCS-Secret'
+
+
 class TcsClient:
   """Client use for communicating with Tsunami Callback Server."""
 
@@ -116,12 +124,13 @@ def _send_polling_request(
     return None
 
   def _build_polling_request(self, secret_string: str) -> HttpRequest:
-    url = '%s/?secret=%s' % (self.polling_base_url, secret_string)
+    url = '%s/' % self.polling_base_url
     return (
         HttpRequest.get(url)
         .set_headers(
             HttpHeaders.builder()
             .add_header('Cache-Control', 'no-cache')
+            .add_header(_SECRET_HEADER_NAME, secret_string)
             .build()
         )
         .build()

plugin_server/py/plugin/tcs_client_test.py

@@ -82,30 +82,32 @@ def test_tcs_client_with_invalid_port_raises_error(self):
   @requests_mock.mock()
   def test_has_oob_log_sends_polling_request(self, mock):
     body = '{ "has_dns_interaction":false, "has_http_interaction":true}'
-    mock.register_uri(
-        'GET', '%s/?secret=%s' % (URL, SECRET), content=body.encode('utf-8')
-    )
+    mock.register_uri('GET', '%s/' % URL, content=body.encode('utf-8'))
     client = TcsClient(DOMAIN, PORT, URL, self.http_client)
     self.assertTrue(client.has_oob_log(SECRET))
+    # Secret travels in a request header, not in the URL query string. The path is "/"
+    # with no query parameters; the X-Tsunami-TCS-Secret header carries the raw secret.
+    self.assertEqual(mock.last_request.qs, {})
+    self.assertEqual(
+        mock.last_request.headers.get('X-Tsunami-TCS-Secret'), SECRET
+    )
 
   @requests_mock.mock()
   def test_has_oob_log_with_no_logs_returns_false(self, mock):
     body = '{ "has_dns_interaction":false, "has_http_interaction":false}'
-    mock.register_uri(
-        'GET', '%s/?secret=%s' % (URL, SECRET), content=body.encode('utf-8')
-    )
+    mock.register_uri('GET', '%s/' % URL, content=body.encode('utf-8'))
     client = TcsClient(DOMAIN, PORT, URL, self.http_client)
     self.assertFalse(client.has_oob_log(SECRET))
 
   @requests_mock.mock()
   def test_has_oob_log_with_no_response_returns_false(self, mock):
-    mock.register_uri('GET', '%s/?secret=%s' % (URL, SECRET))
+    mock.register_uri('GET', '%s/' % URL)
     client = TcsClient(DOMAIN, PORT, URL, self.http_client)
     self.assertFalse(client.has_oob_log(SECRET))
 
   @requests_mock.mock()
   def test_has_oob_log_with_unsuccessful_response_returns_false(self, mock):
-    mock.register_uri('GET', '%s/?secret=%s' % (URL, SECRET), status_code=500)
+    mock.register_uri('GET', '%s/' % URL, status_code=500)
     client = TcsClient(DOMAIN, PORT, URL, self.http_client)
     self.assertFalse(client.has_oob_log(SECRET))