commit 3e741b0e9f467efad354ce16379f9a1e85d4c504
parent fdd2b9dd38a08d1b51977be63bf884b8becdc71a
Author: Christian Grothoff <christian@grothoff.org>
Date: Tue, 4 Aug 2026 15:52:13 +0200
do not forward Connection header from server back to client either
Diffstat:
6 files changed, 187 insertions(+), 17 deletions(-)
diff --git a/src/backend/paivana-httpd_reverse.c b/src/backend/paivana-httpd_reverse.c
@@ -275,6 +275,17 @@ struct HttpRequest
* of the upstream request.
*/
char *client_connection;
+
+ /**
+ * Concatenated value of the upstream's Connection header(s), if
+ * any. The symmetric counterpart of @e client_connection for the
+ * response direction: the names listed here must not be relayed to
+ * the client. Collected in `curl_check_hdr()` and applied to
+ * @e header_head in `curl_download_cb()` — it cannot be applied as
+ * the headers stream in, since Connection may arrive *after* the
+ * headers it names.
+ */
+ char *upstream_connection;
};
@@ -434,7 +445,7 @@ PAIVANA_HTTPD_reverse_shutdown (void)
/**
* Is @a name a hop-by-hop HTTP header name that a proxy must not
* forward (RFC 9110 section 7.6.1 and RFC 7230, section 6.1).
- * The client may name *additional* hop-by-hop headers in its
+ * Either peer may name *additional* hop-by-hop headers in its
* Connection header; those are handled separately in
* `connection_lists_header()`.
*
@@ -467,8 +478,10 @@ is_hop_by_hop_header (const char *name)
/**
* Return true if @a name appears (case-insensitively) as a
* comma-separated element of @a list. Used to honor RFC 9110
- * §7.6.1: the client's Connection header lists additional
- * hop-by-hop header names that must not be forwarded.
+ * §7.6.1: a Connection header lists additional hop-by-hop header
+ * names that must not be forwarded. Applied to the client's
+ * Connection list on the request side and to the upstream's on the
+ * response side.
*
* @param list comma-separated list of header names, may be NULL
* @param name header name to look for
@@ -514,6 +527,35 @@ connection_lists_header (const char *list,
/**
+ * Append @a value to the list header accumulated at @a target,
+ * joining with ", " per the list-header combining rule of RFC 9110
+ * §5.3. Takes ownership of nothing; @a target is (re)allocated.
+ *
+ * @param[in,out] target where the combined value is kept, `*target`
+ * may be NULL (in which case @a value is simply duplicated)
+ * @param value value to append
+ */
+static void
+append_list_value (char **target,
+ const char *value)
+{
+ char *combined;
+
+ if (NULL == *target)
+ {
+ *target = GNUNET_strdup (value);
+ return;
+ }
+ GNUNET_asprintf (&combined,
+ "%s, %s",
+ *target,
+ value);
+ GNUNET_free (*target);
+ *target = combined;
+}
+
+
+/**
* Pre-iteration collector: records the client's Via and Connection
* headers on @a hr so the subsequent forwarding pass can append to
* Via (RFC 9110 §7.6.3) and honor the hop-by-hop names listed in
@@ -547,21 +589,8 @@ collect_proxy_state (void *cls,
target = &hr->client_connection;
else
return MHD_YES;
- if (NULL == *target)
- {
- *target = GNUNET_strdup (value);
- }
- else
- {
- char *combined;
-
- GNUNET_asprintf (&combined,
- "%s, %s",
- *target,
+ append_list_value (target,
value);
- GNUNET_free (*target);
- *target = combined;
- }
return MHD_YES;
}
@@ -636,6 +665,14 @@ curl_check_hdr (void *buffer,
GNUNET_free (ndup);
return bytes;
}
+ /* Remember the upstream's Connection list; the headers it names
+ are connection-specific and get stripped from `header_head' in
+ `curl_download_cb()' once the full header block is in (RFC 9110
+ §7.6.1). The header itself is hop-by-hop and dropped below. */
+ if (0 == strcasecmp (hdr_type,
+ MHD_HTTP_HEADER_CONNECTION))
+ append_list_value (&hr->upstream_connection,
+ hdr_val);
/* Skip hop-by-hop headers. In particular Transfer-Encoding
must not leak through: libcurl has already dechunked the
body for us and MHD will decide whether to re-chunk. */
@@ -699,6 +736,32 @@ curl_download_cb (void *cls,
return;
}
hr->response_code = response_code;
+ /* RFC 9110 §7.6.1: drop the headers named by the upstream's
+ Connection list. Deferred to here (rather than done in
+ `curl_check_hdr()') because Connection may arrive after the
+ headers it names. */
+ {
+ struct HttpResponseHeader *nxt;
+
+ for (struct HttpResponseHeader *header = hr->header_head;
+ NULL != header;
+ header = nxt)
+ {
+ nxt = header->next;
+ if (! connection_lists_header (hr->upstream_connection,
+ header->type))
+ continue;
+ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+ "Dropping connection-specific response header %s\n",
+ header->type);
+ GNUNET_CONTAINER_DLL_remove (hr->header_head,
+ hr->header_tail,
+ header);
+ GNUNET_free (header->type);
+ GNUNET_free (header->value);
+ GNUNET_free (header);
+ }
+ }
for (struct HttpResponseHeader *header = hr->header_head;
NULL != header;
header = header->next)
@@ -984,6 +1047,7 @@ PAIVANA_HTTPD_reverse_cleanup (struct HttpRequest *hr)
GNUNET_free (hr->io_buf);
GNUNET_free (hr->client_via);
GNUNET_free (hr->client_connection);
+ GNUNET_free (hr->upstream_connection);
GNUNET_CONTAINER_DLL_remove (hr_head,
hr_tail,
hr);
diff --git a/src/tests/test_reverse_proxy.sh b/src/tests/test_reverse_proxy.sh
@@ -401,6 +401,28 @@ function run_battery() {
fail "upstream did not see X-Test: dingbat-42"
ok
+ # RFC 9110 §7.6.1, response direction: headers named in the
+ # *upstream's* Connection header are equally hop-by-hop and must
+ # not be relayed to the client. The upstream emits one such
+ # header before the Connection line and one after it, so this
+ # also pins that the filter is applied to the complete header
+ # block rather than as the headers stream in.
+ msg "[$label] headers named in upstream Connection: are stripped"
+ curl -sS -D "$TMPDIR/hdrs" -o /dev/null \
+ "$(PAIVANA_URL /conn-response)" 2>"$TMPDIR/err" \
+ || fail "curl: $(cat "$TMPDIR/err")"
+ if grep -qi '^x-hop-before:' "$TMPDIR/hdrs";
+ then
+ fail "X-Hop-Before leaked to client (upstream Connection ignored)"
+ fi
+ if grep -qi '^x-hop-after:' "$TMPDIR/hdrs";
+ then
+ fail "X-Hop-After leaked to client (upstream Connection ignored)"
+ fi
+ grep -qi '^x-keep-resp:.*survivor' "$TMPDIR/hdrs" || \
+ fail "X-Keep-Resp (not named in Connection) was incorrectly dropped"
+ ok
+
# Response header passthrough: upstream sets X-Upstream.
msg "[$label] upstream response header X-Upstream is forwarded"
curl -sS -D "$TMPDIR/hdrs" -o /dev/null "$(PAIVANA_URL /hello)" 2>"$TMPDIR/err" \
diff --git a/src/tests/upstream_go.go b/src/tests/upstream_go.go
@@ -134,6 +134,20 @@ func echoHeaders(w http.ResponseWriter, r *http.Request) {
}
}
+// connResponse names two of its own response headers in Connection;
+// a conforming proxy must strip both (RFC 9110 §7.6.1) but keep
+// X-Keep-Resp.
+func connResponse(w http.ResponseWriter, r *http.Request) {
+ setHeaders(w)
+ w.Header().Set("X-Hop-Before", "must-not-leak")
+ w.Header().Set("Connection", "X-Hop-Before, X-Hop-After")
+ w.Header().Set("X-Hop-After", "must-not-leak")
+ w.Header().Set("X-Keep-Resp", "survivor")
+ w.Header().Set("Content-Type", "text/plain")
+ w.WriteHeader(200)
+ fmt.Fprint(w, "conn\n")
+}
+
func deleteItem(w http.ResponseWriter, r *http.Request) {
setHeaders(w)
w.WriteHeader(204)
@@ -171,6 +185,8 @@ func route(w http.ResponseWriter, r *http.Request) {
deleteItem(w, r)
case r.URL.Path == "/echo-headers":
echoHeaders(w, r)
+ case r.URL.Path == "/conn-response":
+ connResponse(w, r)
default:
w.Header().Set("Content-Type", "text/plain")
w.WriteHeader(404)
diff --git a/src/tests/upstream_mhd.c b/src/tests/upstream_mhd.c
@@ -248,6 +248,36 @@ handler (void *cls,
return ret;
}
+ /* GET /conn-response — reply naming two of our own response
+ headers in Connection; a conforming proxy must strip both
+ (RFC 9110 section 7.6.1) but keep X-Keep-Resp. */
+ if ( (0 == strcmp (method,
+ MHD_HTTP_METHOD_GET)) &&
+ (0 == strcmp (url,
+ "/conn-response")) )
+ {
+ struct MHD_Response *r = make_text_response ("conn\n");
+ enum MHD_Result ret;
+
+ MHD_add_response_header (r,
+ "X-Hop-Before",
+ "must-not-leak");
+ MHD_add_response_header (r,
+ MHD_HTTP_HEADER_CONNECTION,
+ "X-Hop-Before, X-Hop-After");
+ MHD_add_response_header (r,
+ "X-Hop-After",
+ "must-not-leak");
+ MHD_add_response_header (r,
+ "X-Keep-Resp",
+ "survivor");
+ ret = MHD_queue_response (con,
+ MHD_HTTP_OK,
+ r);
+ MHD_destroy_response (r);
+ return ret;
+ }
+
/* GET /status/NNN */
if ( (0 == strcmp (method,
MHD_HTTP_METHOD_GET)) &&
diff --git a/src/tests/upstream_py.py b/src/tests/upstream_py.py
@@ -89,6 +89,25 @@ class Handler(BaseHTTPRequestHandler):
time.sleep(ms / 1000.0)
self._send_text(200, "slept\n")
return
+ if self.path == "/conn-response":
+ # Name two of our own response headers in Connection: a
+ # conforming proxy must strip both (RFC 9110 §7.6.1) but
+ # keep X-Keep-Resp. X-Hop-Before is deliberately emitted
+ # *before* the Connection header that names it, X-Hop-After
+ # *after* it, so the proxy cannot get away with a
+ # single-pass filter.
+ body = b"conn\n"
+ self.send_response(200)
+ self.send_header("X-Upstream", UPSTREAM_NAME)
+ self.send_header("X-Hop-Before", "must-not-leak")
+ self.send_header("Connection", "X-Hop-Before, X-Hop-After")
+ self.send_header("X-Hop-After", "must-not-leak")
+ self.send_header("X-Keep-Resp", "survivor")
+ self.send_header("Content-Type", "text/plain")
+ self.send_header("Content-Length", str(len(body)))
+ self.end_headers()
+ self.wfile.write(body)
+ return
if self.path == "/echo-headers":
parts = []
for k, v in self.headers.items():
diff --git a/src/tests/upstream_rs.rs b/src/tests/upstream_rs.rs
@@ -147,6 +147,25 @@ fn handle(req: &Request, stream: &mut TcpStream) {
send_response(stream, 200, "OK", "text/plain", b"slept\n", &[]);
return;
}
+ if req.path == "/conn-response" && req.method == "GET" {
+ // Name two of our own response headers in Connection; a
+ // conforming proxy must strip both (RFC 9110 §7.6.1) but keep
+ // X-Keep-Resp. X-Hop-Before is emitted *before* the
+ // Connection header that names it and X-Hop-After *after* it,
+ // so a single-pass filter cannot catch both.
+ let body = b"conn\n";
+ let head = format!(
+ "HTTP/1.1 200 OK\r\nX-Upstream: {}\r\nX-Hop-Before: must-not-leak\r\n\
+ Connection: X-Hop-Before, X-Hop-After\r\nX-Hop-After: must-not-leak\r\n\
+ X-Keep-Resp: survivor\r\nContent-Type: text/plain\r\n\
+ Content-Length: {}\r\n\r\n",
+ UPSTREAM,
+ body.len()
+ );
+ let _ = stream.write_all(head.as_bytes());
+ let _ = stream.write_all(body);
+ return;
+ }
if req.path == "/echo-headers" && req.method == "GET" {
let mut b = String::new();
for (k, v) in &req.headers {
