paivana

commit d2d7c542df491ff82f594b39f6ffd47613389d3a
parent cb59489c4f245950fdc789a719d5cddac1418291
Author: Christian Grothoff <christian@grothoff.org>
Date:   Thu,  6 Aug 2026 19:25:04 +0200

clean up forwarding header chain parsing and handling

Diffstat:
M
src/backend/paivana-httpd.h
 | 
38
+++++++++++++++++++++-----------------
M
src/backend/paivana-httpd_reverse.c
 | 
27
++++++++++++++++++++++++---

2 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/src/backend/paivana-httpd.h b/src/backend/paivana-httpd.h
@@ -129,28 +129,32 @@ extern int PH_global_cookie;
 extern int PH_no_check;
 
 /**
- * If set, believe the "X-Forwarded-For" request header when deciding
- * the client address (falling back to the socket address when the
- * header is absent).  Only enable this when paivana-httpd is itself
- * behind a reverse proxy that overwrites that header — otherwise
- * clients can spoof their address.
+ * If set, we are behind a reverse proxy: the socket peer is a proxy we
+ * trust, and the forwarding headers ("Forwarded" and the "X-Forwarded-*"
+ * family) are read to recover what the client actually did.  Unset, no
+ * forwarding header is consulted at all and the peer *is* the client.
+ * Only enable it when something in front overwrites those headers
+ * rather than appending to what the client sent.
  *
- * On its own this trusts *every* hop, including the leftmost entry,
- * which is whatever the client wrote.  #PH_trusted_proxies4 /
- * #PH_trusted_proxies6 narrow that to named networks and should be
- * preferred; see PAIVANA_HTTPD_is_trusted_proxy().
+ * This flag alone extends trust exactly one hop, to the peer, so with
+ * no #PH_trusted_proxies4 / #PH_trusted_proxies6 the client is the
+ * RIGHTMOST element of the chain -- the only one the peer vouches for.
+ * Naming further networks there extends the walk leftwards, one hop per
+ * trusted node; see PAIVANA_HTTPD_resolve_forwarding().
+ *
+ * It also decides whether `BASE_URL` is optional: the flag is the
+ * assertion that the proxy enforced a correct "Host", which is what
+ * makes reconstructing our own URL from the request safe.
  */
 extern int PH_respect_forwarded_headers;
 
 /**
  * Networks whose members are reverse proxies we trust to report the
  * client address truthfully, from the `TRUSTED_PROXIES` configuration
- * option.  NULL if unconfigured.  Together with
- * #PH_trusted_proxies6 this replaces the blanket trust of
- * #PH_respect_forwarded_headers: an "X-Forwarded-For" is only
- * consulted if the peer that sent it is listed here, and only the
- * entries contributed by listed proxies are skipped when looking for
- * the client.
+ * option.  NULL if unconfigured.  These are the hops BEYOND the socket
+ * peer: #PH_respect_forwarded_headers already trusts the peer, and each
+ * network named here lets the walk step one element further left, past
+ * a node it matches.  The first node not covered is the client.
  */
 extern struct GNUNET_STRINGS_IPv4NetworkPolicy *PH_trusted_proxies4;
 
@@ -161,8 +165,8 @@ extern struct GNUNET_STRINGS_IPv6NetworkPolicy *PH_trusted_proxies6;
 
 /**
  * True if either #PH_trusted_proxies4 or #PH_trusted_proxies6 was
- * configured.  Distinguishes "trust nothing" (no policy given, fall
- * back to #PH_respect_forwarded_headers) from "trust exactly these".
+ * configured.  Distinguishes "no hop beyond the peer is trusted" from
+ * "these networks are, in addition to the peer".
  */
 extern bool PH_have_trusted_proxies;
 
diff --git a/src/backend/paivana-httpd_reverse.c b/src/backend/paivana-httpd_reverse.c
@@ -1840,6 +1840,8 @@ append_forwarded_headers (struct HttpRequest *hr,
   {
     char *elem;
     char *node;
+    char *qproto;
+    char *qhost = NULL;
     void *ca = NULL;
     size_t ca_len = 0;
 
@@ -1870,17 +1872,36 @@ append_forwarded_headers (struct HttpRequest *hr,
       node = PAIVANA_HTTPD_forwarded_node (NULL,
                                            0);
     }
+    /* `proto` and `host` may both be raw client input: without -f they
+       come from the Host header, with it from X-Forwarded-Proto /
+       X-Forwarded-Host, neither of which we validated.  Splicing them
+       in as-is would let a ';' in either open a parameter of ours, or
+       a ',' an element of ours, in a header the origin reads as
+       paivana's own statement about the hop -- so render them as an
+       RFC 7239 section 4 `value` (token, or escaped quoted-string).
+       PAIVANA_HTTPD_forwarded_value() returns NULL only for a control
+       character, which no `value` production can carry at all; drop
+       the parameter rather than emit something unparseable. */
+    qproto = PAIVANA_HTTPD_forwarded_value (proto);
     if (NULL != fhost)
+      qhost = PAIVANA_HTTPD_forwarded_value (fhost);
+    if (NULL == qproto)
+      GNUNET_asprintf (&elem,
+                       "for=%s;by=_paivana",
+                       node);
+    else if (NULL != qhost)
       GNUNET_asprintf (&elem,
                        "for=%s;by=_paivana;proto=%s;host=%s",
                        node,
-                       proto,
-                       fhost);
+                       qproto,
+                       qhost);
     else
       GNUNET_asprintf (&elem,
                        "for=%s;by=_paivana;proto=%s",
                        node,
-                       proto);
+                       qproto);
+    GNUNET_free (qproto);
+    GNUNET_free (qhost);
     GNUNET_free (node);
     if ( (PH_respect_forwarded_headers) &&
          (NULL != hr->client_forwarded) )