commit 10949b07c2635129bf3517386878604c8ed84aed
parent fd46bcdead8ef87bf13e95ee11433004a9db68d7
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 23 Apr 2026 23:07:04 +0200
fix Via and Connection header handling
Diffstat:
2 files changed, 210 insertions(+), 23 deletions(-)
diff --git a/src/backend/paivana-httpd_reverse.c b/src/backend/paivana-httpd_reverse.c
@@ -232,6 +232,24 @@ struct HttpRequest
* response on the "final" access-handler call.
*/
bool reject_upload;
+
+ /**
+ * Concatenated value of the client's Via header(s), if any. Per
+ * RFC 9110 §7.6.3 a proxy must *append* its own entry to this
+ * list, not replace it; we capture the inbound value here before
+ * iterating over the other headers and emit our appended Via when
+ * building the curl request.
+ */
+ char *client_via;
+
+ /**
+ * Concatenated value of the client's Connection header(s), if
+ * any. Per RFC 9110 §7.6.1 this lists additional header names
+ * that are connection-specific and must not be forwarded; we
+ * consult this list in `con_val_iter` to filter such headers out
+ * of the upstream request.
+ */
+ char *client_connection;
};
@@ -334,11 +352,9 @@ 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).
- *
- * FIXME: Note that we *additionally* must remove all headers listed
- * within the "Connection" header (for which we should keep that
- * header around separately and to another pass after getting all the
- * headers).
+ * The client may name *additional* hop-by-hop headers in its
+ * Connection header; those are handled separately in
+ * `connection_lists_header()`.
*
* @param name header field name
* @return true if @a name is a hop-by-hop header
@@ -366,6 +382,108 @@ 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.
+ *
+ * @param list comma-separated list of header names, may be NULL
+ * @param name header name to look for
+ * @return true if @a list names @a name
+ */
+static bool
+connection_lists_header (const char *list,
+ const char *name)
+{
+ size_t namelen;
+ const char *p;
+
+ if (NULL == list)
+ return false;
+ namelen = strlen (name);
+ p = list;
+ while ('\0' != *p)
+ {
+ const char *comma;
+ size_t len;
+
+ while ( (' ' == *p) ||
+ ('\t' == *p) ||
+ (',' == *p) )
+ p++;
+ if ('\0' == *p)
+ break;
+ comma = strchr (p, ',');
+ len = (NULL == comma) ? strlen (p) : (size_t) (comma - p);
+ while ( (len > 0) &&
+ ( (' ' == p[len - 1]) ||
+ ('\t' == p[len - 1]) ) )
+ len--;
+ if ( (len == namelen) &&
+ (0 == strncasecmp (p, name, namelen)) )
+ return true;
+ if (NULL == comma)
+ break;
+ p = comma + 1;
+ }
+ return false;
+}
+
+
+/**
+ * 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
+ * Connection (RFC 9110 §7.6.1). Multiple values for the same
+ * header are joined with ", ", matching the list-header combining
+ * rule.
+ *
+ * @param cls our `struct HttpRequest *`
+ * @param kind value kind (unused)
+ * @param key header field name
+ * @param value header field value
+ * @return #MHD_YES to continue iteration
+ */
+static enum MHD_Result
+collect_proxy_state (void *cls,
+ enum MHD_ValueKind kind,
+ const char *key,
+ const char *value)
+{
+ struct HttpRequest *hr = cls;
+ char **target;
+
+ (void) kind;
+ if (NULL == value)
+ return MHD_YES;
+ if (0 == strcasecmp (MHD_HTTP_HEADER_VIA,
+ key))
+ target = &hr->client_via;
+ else if (0 == strcasecmp (MHD_HTTP_HEADER_CONNECTION,
+ key))
+ target = &hr->client_connection;
+ else
+ return MHD_YES;
+ if (NULL == *target)
+ {
+ *target = GNUNET_strdup (value);
+ }
+ else
+ {
+ char *combined;
+
+ GNUNET_asprintf (&combined,
+ "%s, %s",
+ *target,
+ value);
+ GNUNET_free (*target);
+ *target = combined;
+ }
+ return MHD_YES;
+}
+
+
/* *************** HTTP handling with cURL ***************** */
@@ -812,17 +930,27 @@ con_val_iter (void *cls,
}
if (is_hop_by_hop_header (key))
return MHD_YES;
+ /* RFC 9110 §7.6.1: suppress any header named by the client's
+ Connection list (additional hop-by-hop headers). */
+ if (connection_lists_header (hr->client_connection,
+ key))
+ return MHD_YES;
if ( (0 == strncasecmp ("X-Forwarded-",
key,
strlen ("X-Forwarded-"))) ||
(0 == strcasecmp (MHD_HTTP_HEADER_FORWARDED,
- key)) ||
- (0 == strcasecmp (MHD_HTTP_HEADER_VIA,
key)) )
{
/* We will replace these with our own below. */
- /* FIXME: we should RFC 9110 says we should APPEND
- our existence to the Via members, not replace it! */
+ return MHD_YES;
+ }
+ if (0 == strcasecmp (MHD_HTTP_HEADER_VIA,
+ key))
+ {
+ /* The client's Via was captured in `collect_proxy_state` and
+ will be emitted below with our own entry appended (RFC 9110
+ §7.6.3). Drop the raw header here so we don't forward it
+ twice. */
return MHD_YES;
}
GNUNET_asprintf (&hdr,
@@ -1005,6 +1133,8 @@ PAIVANA_HTTPD_reverse_cleanup (struct HttpRequest *hr)
GNUNET_free (hr->url);
GNUNET_free (hr->io_buf);
+ GNUNET_free (hr->client_via);
+ GNUNET_free (hr->client_connection);
GNUNET_CONTAINER_DLL_remove (hr_head,
hr_tail,
hr);
@@ -1359,15 +1489,21 @@ PAIVANA_HTTPD_reverse (struct HttpRequest *hr,
curl_failure_response);
}
+ /* First pass: collect Via / Connection so `con_val_iter` can
+ honor them (append to Via; drop headers named by Connection). */
+ MHD_get_connection_values (con,
+ MHD_HEADER_KIND,
+ &collect_proxy_state,
+ hr);
MHD_get_connection_values (con,
MHD_HEADER_KIND,
&con_val_iter,
hr);
- /* Add standard reverse-proxy forwarding headers. We always
- overwrite any client-supplied X-Forwarded-* / Forwarded /
- Via (filtered out in con_val_iter) so upstream sees our
- view of the connection, not anything the client made up. */
+ /* Add standard reverse-proxy forwarding headers. X-Forwarded-*
+ and Forwarded are replaced with our view of the connection
+ (filtered out in con_val_iter); Via is *appended to* the
+ client's existing chain per RFC 9110 §7.6.3. */
{
const union MHD_ConnectionInfo *ci;
char *hdr;
@@ -1403,7 +1539,8 @@ PAIVANA_HTTPD_reverse (struct HttpRequest *hr,
GNUNET_asprintf (&hdr,
"X-Forwarded-For: %s",
ip);
- hr->headers = curl_slist_append (hr->headers, hdr);
+ hr->headers = curl_slist_append (hr->headers,
+ hdr);
GNUNET_free (hdr);
}
}
@@ -1412,7 +1549,8 @@ PAIVANA_HTTPD_reverse (struct HttpRequest *hr,
GNUNET_asprintf (&hdr,
"X-Forwarded-Proto: %s",
proto);
- hr->headers = curl_slist_append (hr->headers, hdr);
+ hr->headers = curl_slist_append (hr->headers,
+ hdr);
GNUNET_free (hdr);
fhost = MHD_lookup_connection_value (con,
MHD_HEADER_KIND,
@@ -1422,23 +1560,34 @@ PAIVANA_HTTPD_reverse (struct HttpRequest *hr,
GNUNET_asprintf (&hdr,
"X-Forwarded-Host: %s",
fhost);
- hr->headers = curl_slist_append (hr->headers, hdr);
+ hr->headers = curl_slist_append (hr->headers,
+ hdr);
GNUNET_free (hdr);
}
- /* Via: pseudonym + protocol-version (RFC 7230 5.7.1);
- MHD hands us e.g. "HTTP/1.1" but Via wants just "1.1". */
+ /* Via: pseudonym + protocol-version (RFC 9110 §7.6.3);
+ MHD hands us e.g. "HTTP/1.1" but Via wants just "1.1".
+ If the client already carried a Via chain, prepend it so
+ the upstream sees the full trace of proxies. */
{
- /* FIXME: we should append our Via, not replace... */
const char *via_ver = "1.1";
if ( (NULL != ver) &&
(0 == strncasecmp (ver, "HTTP/", 5)) )
via_ver = ver + 5;
- GNUNET_asprintf (&hdr,
- "Via: %s paivana",
- via_ver);
+ if (NULL != hr->client_via)
+ GNUNET_asprintf (&hdr,
+ "%s: %s, %s paivana",
+ MHD_HTTP_HEADER_VIA,
+ hr->client_via,
+ via_ver);
+ else
+ GNUNET_asprintf (&hdr,
+ "%s: %s paivana",
+ MHD_HTTP_HEADER_VIA,
+ via_ver);
}
- hr->headers = curl_slist_append (hr->headers, hdr);
+ hr->headers = curl_slist_append (hr->headers,
+ hdr);
GNUNET_free (hdr);
}
diff --git a/src/tests/test_reverse_proxy.sh b/src/tests/test_reverse_proxy.sh
@@ -354,6 +354,44 @@ function run_battery() {
fail "upstream did not see Via: paivana"
ok
+ # RFC 9110 §7.6.3: client's Via chain must be preserved and our
+ # pseudonym *appended* to it, not replaced.
+ msg "[$label] client Via is preserved and paivana is appended"
+ curl -sS -H 'Via: 1.1 alpha.example, 2.0 beta.example' \
+ -o "$TMPDIR/body" "$(PAIVANA_URL /echo-headers)" 2>"$TMPDIR/err" \
+ || fail "curl: $(cat "$TMPDIR/err")"
+ local via
+ via="$(grep -i '^via:' "$TMPDIR/body" | tr -d '\r')"
+ [ -n "$via" ] || fail "no Via header at upstream"
+ # Expect: "Via: 1.1 alpha.example, 2.0 beta.example, 1.1 paivana"
+ case "$via" in
+ *"alpha.example"*"beta.example"*paivana*) ;;
+ *) fail "Via not appended correctly: '$via'";;
+ esac
+ ok
+
+ # RFC 9110 §7.6.1: headers named in the client's Connection
+ # header are hop-by-hop and must not be forwarded upstream.
+ msg "[$label] headers named in Connection: are stripped"
+ curl -sS \
+ -H 'Connection: X-Custom-Hop, X-Other-Hop' \
+ -H 'X-Custom-Hop: must-not-forward' \
+ -H 'X-Other-Hop: neither' \
+ -H 'X-Keep: keep-this' \
+ -o "$TMPDIR/body" "$(PAIVANA_URL /echo-headers)" 2>"$TMPDIR/err" \
+ || fail "curl: $(cat "$TMPDIR/err")"
+ if grep -qi '^x-custom-hop:' "$TMPDIR/body";
+ then
+ fail "X-Custom-Hop leaked to upstream (Connection list ignored)"
+ fi
+ if grep -qi '^x-other-hop:' "$TMPDIR/body";
+ then
+ fail "X-Other-Hop leaked to upstream (Connection list ignored)"
+ fi
+ grep -qi '^x-keep:.*keep-this' "$TMPDIR/body" || \
+ fail "X-Keep (not named in Connection) was incorrectly dropped"
+ ok
+
# Custom request header must be forwarded.
msg "[$label] custom request header X-Test is forwarded"
curl -sS -H 'X-Test: dingbat-42' \
