commit 9cd622d3b04c7d4a15b85c9f7a7af4200bf3d1f1
parent 44abe13cc8bc06fe657ac9a66b5759d22ffbf7a8
Author: Christian Grothoff <christian@grothoff.org>
Date: Tue, 4 Aug 2026 18:33:03 +0200
add TRUSTED_PROXIES policy for the forwarded client address
-f on its own believes every element of X-Forwarded-For, and
parse_forwarded_for() took the leftmost -- the one furthest from us and
closest to the client. Whether that is the real client therefore
depended entirely on the server in front overwriting the header:
against a proxy that appends (Apache's mod_proxy does, and nginx passes
a client-supplied value straight through unless configured otherwise) a
client could name any address it liked and so choose the identity its
access cookie is bound to.
New option TRUSTED_PROXIES / TRUSTED_PROXIES6 names the networks entitled to
speak for a client. The chain is then walked from the right with those
hops skipped; the first element that is not one of them is as far back
as the header can be believed. An element that does not parse ends the
walk, since nothing to its left is attributable to a trusted proxy.
X-Forwarded-For is ignored outright if the peer that connected is not
itself listed. A Unix-domain peer is trusted: it has no address to
match, is by construction on this host, and access to the socket is
governed by UNIXPATH_MODE.
Without a policy the old leftmost-element behaviour stands, with a
startup warning.
Diffstat:
8 files changed, 998 insertions(+), 62 deletions(-)
diff --git a/README b/README
@@ -94,6 +94,12 @@ Paivana reads an INI-style `.conf` file. The only section used is
(X-Forwarded-Host / Host / X-Forwarded-Port) if absent.
SECRET Stable secret for cookie MAC and Paivana ID derivation.
A random nonce is generated on every startup if absent.
+ TRUSTED_PROXIES
+ IPv4 networks whose members are reverse proxies trusted to
+ report the client address in `X-Forwarded-For`. Only
+ consulted with `-f`. See "Trusted proxies" below.
+ TRUSTED_PROXIES6
+ IPv6 counterpart of TRUSTED_PROXIES.
SERVE `tcp` (default) or `unix` (Unix-domain socket) or `systemd`
(systemd socket activation).
PORT TCP port, used when SERVE = tcp.
@@ -176,6 +182,44 @@ Apache (requires mod_proxy, mod_proxy_http and mod_headers):
Ready-made versions of both are shipped in `debian/etc/`.
+
+Trusted proxies
+---------------
+
+`-f` on its own believes every hop in `X-Forwarded-For`, including the
+leftmost entry — which is whatever the client wrote, unless the server
+in front overwrites the header. `TRUSTED_PROXIES` and
+`TRUSTED_PROXIES6` say which addresses may speak for a client, so that
+correctness no longer rests on the front server's configuration alone:
+
+ [paivana]
+ TRUSTED_PROXIES = 10.0.0.0/8;192.168.0.0/16;
+ TRUSTED_PROXIES6 = 2001:db8::/32;
+
+The chain is then walked from the right, skipping listed proxies; the
+first element that is not one of them is the client. A client that
+prepends entries of its own cannot promote them, because the walk stops
+at the address the trusted proxy actually reported. If the peer that
+connected is not itself listed, `X-Forwarded-For` is ignored entirely.
+A Unix-domain peer is always trusted: it has no address to match, is by
+construction on this host, and access is governed by `UNIXPATH_MODE`.
+
+Syntax notes, inherited from GNUnet's network-policy parser:
+
+ - entries are separated *and terminated* by `;` — a missing trailing
+ semicolon means nothing is parsed;
+ - `TRUSTED_PROXIES6` does not tolerate spaces between entries
+ (`TRUSTED_PROXIES` does);
+ - `0.0.0.0/0` and `::/0` cannot be expressed: they are
+ indistinguishable from the end of the list.
+
+Anything that parses to an empty list is refused at startup rather
+than silently trusting nobody.
+
+Put IPv4 proxies in `TRUSTED_PROXIES`, not in `TRUSTED_PROXIES6` as
+`::ffff:a.b.c.d`: addresses are folded to their IPv4 form before
+matching, so a mapped entry would never be hit.
+
Set `BASE_URL` in the configuration file to the public HTTPS URL so
that redirects and cookie domains are correct.
diff --git a/src/backend/paivana-httpd.c b/src/backend/paivana-httpd.c
@@ -57,6 +57,12 @@ int PH_no_check;
int PH_respect_forwarded_headers;
+struct GNUNET_STRINGS_IPv4NetworkPolicy *PH_trusted_proxies4;
+
+struct GNUNET_STRINGS_IPv6NetworkPolicy *PH_trusted_proxies6;
+
+bool PH_have_trusted_proxies;
+
unsigned long long PH_request_buffer_max = 1024 * 1024;
int PH_global_ret;
@@ -83,6 +89,83 @@ static struct GNUNET_CURL_RescheduleContext *ctx_rc;
/**
+ * Load one of the `TRUSTED_PROXIES` options.
+ *
+ * The GNUnet policy parsers are lenient in ways that matter here, so
+ * a non-NULL return is not on its own evidence that anything was
+ * understood:
+ *
+ * - the list is terminated by an all-zero entry, so a network of
+ * 0.0.0.0/0 or ::/0 *is* the terminator and silently truncates
+ * everything after it. "Trust everyone" is therefore inexpressible
+ * — and would be a strange thing to write anyway;
+ * - a v6 address handed to the v4 parser (or a stray port policy)
+ * yields a valid pointer to an empty list rather than an error.
+ *
+ * Both come out as "parsed, but nothing usable", which we reject:
+ * quietly trusting nobody would send every visitor to the socket
+ * address, and the operator would have no hint why.
+ *
+ * @param c configuration to read from
+ * @param option name of the option
+ * @param[out] count set to the number of usable entries
+ * @return false if the option is present but unusable
+ */
+static bool
+load_trusted_proxies (const struct GNUNET_CONFIGURATION_Handle *c,
+ const char *option,
+ unsigned int *count)
+{
+ char *opt;
+ bool v6 = (0 != strcmp (option,
+ "TRUSTED_PROXIES"));
+
+ *count = 0;
+ if (GNUNET_OK !=
+ GNUNET_CONFIGURATION_get_value_string (c,
+ "paivana",
+ option,
+ &opt))
+ return true; /* not configured at all: fine */
+ if (v6)
+ {
+ PH_trusted_proxies6 = GNUNET_STRINGS_parse_ipv6_policy (opt);
+ if (NULL != PH_trusted_proxies6)
+ while (! GNUNET_is_zero (&PH_trusted_proxies6[*count].network))
+ (*count)++;
+ }
+ else
+ {
+ PH_trusted_proxies4 = GNUNET_STRINGS_parse_ipv4_policy (opt);
+ if (NULL != PH_trusted_proxies4)
+ while (0 != PH_trusted_proxies4[*count].network.s_addr)
+ (*count)++;
+ }
+ if (0 == *count)
+ {
+ GNUNET_log_config_invalid (
+ GNUNET_ERROR_TYPE_ERROR,
+ "paivana",
+ option,
+ v6
+ ? "not a usable IPv6 network list; entries are separated *and*"
+ " terminated by ';' and must not contain spaces, e.g."
+ " \"2001:db8::/32;fe80::/10;\" (note that ::/0 is indistinguishable"
+ " from the end of the list and cannot be used)"
+ : "not a usable IPv4 network list; entries are separated *and*"
+ " terminated by ';', e.g. \"10.0.0.0/8;192.168.0.0/16;\""
+ " (note that 0.0.0.0/0 is indistinguishable from the end of the"
+ " list and cannot be used)");
+ GNUNET_free (opt);
+ return false;
+ }
+ GNUNET_free (opt);
+ PH_have_trusted_proxies = true;
+ return true;
+}
+
+
+/**
* Task run on shutdown
*
* @param cls closure
@@ -101,6 +184,8 @@ do_shutdown (void *cls)
TALER_TEMPLATING_done ();
GNUNET_free (PH_target_server_base_url);
GNUNET_free (PH_target_server_unixpath);
+ GNUNET_free (PH_trusted_proxies4);
+ GNUNET_free (PH_trusted_proxies6);
GNUNET_free (PH_merchant_base_url);
GNUNET_free (PH_base_url);
if (PH_have_whitelist_ex)
@@ -191,6 +276,47 @@ run (void *cls,
GNUNET_SCHEDULER_shutdown ();
return;
}
+ {
+ unsigned int n4;
+ unsigned int n6;
+
+ if ( (! load_trusted_proxies (c,
+ "TRUSTED_PROXIES",
+ &n4)) ||
+ (! load_trusted_proxies (c,
+ "TRUSTED_PROXIES6",
+ &n6)) )
+ {
+ PH_global_ret = EXIT_NOTCONFIGURED;
+ GNUNET_SCHEDULER_shutdown ();
+ return;
+ }
+ if (PH_have_trusted_proxies)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Trusting %u IPv4 and %u IPv6 network(s) as reverse proxies\n",
+ n4,
+ n6);
+ if (! PH_respect_forwarded_headers)
+ {
+ /* The policy says which proxies may speak for a client; it is
+ the -f flag that says we listen at all. Configuring one
+ without the other is a mistake in either direction, but only
+ this one leaves the policy inert. */
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "TRUSTED_PROXIES configured but -f/--respect-forwarded-headers"
+ " is not set; forwarded headers are ignored entirely\n");
+ }
+ }
+ else if (PH_respect_forwarded_headers)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "-f/--respect-forwarded-headers is set without TRUSTED_PROXIES:"
+ " every hop in X-Forwarded-For is believed, including the"
+ " leftmost entry, which the client itself controls unless the"
+ " proxy in front overwrites the header\n");
+ }
+ }
GNUNET_CONFIGURATION_get_value_filename (
c,
"paivana",
diff --git a/src/backend/paivana-httpd.h b/src/backend/paivana-httpd.h
@@ -104,15 +104,44 @@ extern int PH_global_cookie;
extern int PH_no_check;
/**
- * If set, derive the client address from the leftmost entry of the
- * "X-Forwarded-For" request header (falling back to the socket
- * address only when the header is absent). Only enable this when
- * paivana-httpd is itself behind a trusted reverse proxy that
- * sanitizes that header — otherwise clients can spoof their address.
+ * 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.
+ *
+ * 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().
*/
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.
+ */
+extern struct GNUNET_STRINGS_IPv4NetworkPolicy *PH_trusted_proxies4;
+
+/**
+ * IPv6 counterpart of #PH_trusted_proxies4, from `TRUSTED_PROXIES6`.
+ */
+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".
+ */
+extern bool PH_have_trusted_proxies;
+
+/**
* Value to return from main()
*/
extern int PH_global_ret;
diff --git a/src/backend/paivana-httpd_helper.c b/src/backend/paivana-httpd_helper.c
@@ -57,14 +57,31 @@ store_v6 (const struct in6_addr *a6,
}
-bool
-PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
- void **ca,
- size_t *ca_len)
+/**
+ * Parse a single `X-Forwarded-For` element into binary form.
+ *
+ * What we return has to be the *same bytes* the socket branch would
+ * have produced for this client, or the cookie MAC -- which covers
+ * the client address -- silently stops matching as soon as a request
+ * arrives without the header, or through a proxy that spells the
+ * address differently ("::1" and "0:0:0:0:0:0:0:1" are one host).
+ * Hence the address is parsed into its binary form rather than
+ * carried around as text. A token that is not an address at all (a
+ * port suffix, brackets, an RFC 7239 obfuscated identifier, a
+ * hostname) has no such form and is refused.
+ *
+ * @param tok start of the element
+ * @param len number of bytes in @a tok
+ * @param[out] ca where to write the allocated address
+ * @param[out] ca_len set to the number of bytes in @a ca
+ * @return true on success
+ */
+static bool
+parse_element (const char *tok,
+ size_t len,
+ void **ca,
+ size_t *ca_len)
{
- const char *start = xff;
- const char *end;
- size_t len;
/* Long enough for any address inet_pton() accepts. */
char addr[INET6_ADDRSTRLEN];
struct in_addr a4;
@@ -72,43 +89,16 @@ PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
*ca = NULL;
*ca_len = 0;
- /* Use first part before ',', getting rid of whitespace
- at start or end of the substring. */
- while ( (' ' == *start) ||
- ('\t' == *start) )
- start++;
- end = strchr (start,
- ',');
- len = (NULL != end)
- ? (size_t) (end - start)
- : strlen (start);
- while ( (len > 0) &&
- ( (' ' == start[len - 1]) ||
- ('\t' == start[len - 1]) ) )
- len--;
if ( (0 == len) ||
(len >= sizeof (addr)) )
{
GNUNET_break_op (0);
return false;
}
- GNUNET_log (GNUNET_ERROR_TYPE_INFO,
- "Client address is based on X-Forwarded-For: `%.*s'\n",
- (int) len,
- start);
memcpy (addr,
- start,
+ tok,
len);
addr[len] = '\0';
- /* What we return has to be the *same bytes* the socket branch would
- have produced for this client, or the cookie MAC -- which covers
- the client address -- silently stops matching as soon as a
- request arrives without the header, or through a proxy that
- spells the address differently ("::1" and "0:0:0:0:0:0:0:1" are
- one host). Hence the address is parsed into its binary form
- rather than carried around as text. A token that is not an
- address at all (a port suffix, brackets, an RFC 7239 obfuscated
- identifier, a hostname) has no such form and is refused. */
if (1 == inet_pton (AF_INET,
addr,
&a4))
@@ -135,29 +125,236 @@ PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
}
+/**
+ * Locate element number @a idx (counting from 0 at the left) of the
+ * comma-separated list @a xff, with surrounding whitespace stripped.
+ *
+ * @param xff header value to scan
+ * @param idx which element to return
+ * @param[out] len set to the length of the element
+ * @return start of the element, or NULL if there are @a idx or fewer
+ */
+static const char *
+nth_element (const char *xff,
+ unsigned int idx,
+ size_t *len)
+{
+ const char *p = xff;
+
+ for (unsigned int i = 0; /* until return */; i++)
+ {
+ const char *comma;
+ size_t l;
+
+ while ( (' ' == *p) ||
+ ('\t' == *p) )
+ p++;
+ comma = strchr (p,
+ ',');
+ l = (NULL != comma)
+ ? (size_t) (comma - p)
+ : strlen (p);
+ while ( (l > 0) &&
+ ( (' ' == p[l - 1]) ||
+ ('\t' == p[l - 1]) ) )
+ l--;
+ if (i == idx)
+ {
+ *len = l;
+ return p;
+ }
+ if (NULL == comma)
+ return NULL;
+ p = comma + 1;
+ }
+}
+
+
+/**
+ * Count the elements of the comma-separated list @a xff.
+ *
+ * @param xff header value to scan
+ * @return number of elements (at least 1 for a non-empty string)
+ */
+static unsigned int
+count_elements (const char *xff)
+{
+ unsigned int n = 1;
+
+ for (const char *p = strchr (xff, ','); NULL != p; p = strchr (p + 1, ','))
+ n++;
+ return n;
+}
+
+
bool
-PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
- void **ca,
- size_t *ca_len)
+PAIVANA_HTTPD_is_trusted_proxy (const void *ca,
+ size_t ca_len)
{
- const union MHD_ConnectionInfo *ci;
- const struct sockaddr *sa;
+ if (! PH_have_trusted_proxies)
+ return false;
+ if (sizeof (struct in_addr) == ca_len)
+ {
+ const struct in_addr *a4 = ca;
+
+ if (NULL == PH_trusted_proxies4)
+ return false;
+ /* The list is terminated by an all-zero entry; GNUnet does not
+ hand out a count. See load_trusted_proxies(). */
+ for (unsigned int i = 0;
+ 0 != PH_trusted_proxies4[i].network.s_addr;
+ i++)
+ if ( (a4->s_addr & PH_trusted_proxies4[i].netmask.s_addr) ==
+ (PH_trusted_proxies4[i].network.s_addr &
+ PH_trusted_proxies4[i].netmask.s_addr) )
+ return true;
+ return false;
+ }
+ if (sizeof (struct in6_addr) == ca_len)
+ {
+ const struct in6_addr *a6 = ca;
+
+ if (NULL == PH_trusted_proxies6)
+ return false;
+ for (unsigned int i = 0;
+ ! GNUNET_is_zero (&PH_trusted_proxies6[i].network);
+ i++)
+ {
+ const struct in6_addr *net = &PH_trusted_proxies6[i].network;
+ const struct in6_addr *mask = &PH_trusted_proxies6[i].netmask;
+ bool match = true;
+
+ for (unsigned int j = 0; j < sizeof (struct in6_addr); j++)
+ if ( (a6->s6_addr[j] & mask->s6_addr[j]) !=
+ (net->s6_addr[j] & mask->s6_addr[j]) )
+ {
+ match = false;
+ break;
+ }
+ if (match)
+ return true;
+ }
+ return false;
+ }
+ GNUNET_break (0);
+ return false;
+}
+
+
+bool
+PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
+ void **ca,
+ size_t *ca_len)
+{
+ unsigned int n;
+ size_t len;
+ const char *tok;
*ca = NULL;
*ca_len = 0;
- if (PH_respect_forwarded_headers)
+ n = count_elements (xff);
+ if (! PH_have_trusted_proxies)
{
- const char *xff;
-
- xff = MHD_lookup_connection_value (connection,
- MHD_HEADER_KIND,
- PH_HEADER_X_FORWARDED_FOR);
- if (NULL != xff)
- return PAIVANA_HTTPD_parse_forwarded_for (xff,
- ca,
- ca_len);
- /* No header present: fall through to the socket address. */
+ /* No policy: the leftmost element is taken as the client, which
+ means trusting every hop -- including whatever the client
+ itself wrote, unless the proxy in front overwrites the header.
+ load_trusted_proxies() warns about this at startup. */
+ tok = nth_element (xff,
+ 0,
+ &len);
+ if (NULL == tok)
+ {
+ GNUNET_break_op (0);
+ return false;
+ }
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Client address is based on X-Forwarded-For: `%.*s'\n",
+ (int) len,
+ tok);
+ return parse_element (tok,
+ len,
+ ca,
+ ca_len);
+ }
+ /* Walk right to left, discarding the hops we ourselves put trust in;
+ the first element that is *not* one of our proxies is as far back
+ as the chain can be believed, and is therefore the client. An
+ element we cannot parse ends the walk for the same reason: nothing
+ to its left is attributable to a trusted proxy. Should every
+ element be trusted, the leftmost one is all that is left to
+ report. */
+ for (unsigned int i = n; i > 0; i--)
+ {
+ tok = nth_element (xff,
+ i - 1,
+ &len);
+ if (NULL == tok)
+ {
+ GNUNET_break (0);
+ return false;
+ }
+ if (! parse_element (tok,
+ len,
+ ca,
+ ca_len))
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Unparseable X-Forwarded-For element; not looking further"
+ " left in the chain\n");
+ return false;
+ }
+ if (! PAIVANA_HTTPD_is_trusted_proxy (*ca,
+ *ca_len))
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Client address is based on X-Forwarded-For: `%.*s'\n",
+ (int) len,
+ tok);
+ return true;
+ }
+ GNUNET_free (*ca);
+ *ca = NULL;
+ *ca_len = 0;
+ }
+ /* Every hop is a proxy we trust; the leftmost is the best we have. */
+ tok = nth_element (xff,
+ 0,
+ &len);
+ if (NULL == tok)
+ {
+ GNUNET_break (0);
+ return false;
}
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "All X-Forwarded-For elements are trusted proxies; using the"
+ " leftmost `%.*s' as the client\n",
+ (int) len,
+ tok);
+ return parse_element (tok,
+ len,
+ ca,
+ ca_len);
+}
+
+
+/**
+ * Store the address of the peer we accepted @a connection from.
+ *
+ * @param connection HTTP client connection
+ * @param[out] ca where to write the allocated address
+ * @param[out] ca_len set to the number of bytes in @a ca
+ * @return true on success, false for a peer that has no IP address
+ */
+static bool
+socket_address (struct MHD_Connection *connection,
+ void **ca,
+ size_t *ca_len)
+{
+ const union MHD_ConnectionInfo *ci;
+ const struct sockaddr *sa;
+
+ *ca = NULL;
+ *ca_len = 0;
ci = MHD_get_connection_info (connection,
MHD_CONNECTION_INFO_CLIENT_ADDRESS);
GNUNET_assert (NULL != ci);
@@ -177,11 +374,71 @@ PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
ca_len);
return true;
default:
- /* AF_UNIX in particular: there is no client address to bind the
- cookie to. */
+ /* AF_UNIX: no address exists. */
+ return false;
+ }
+}
+
+
+bool
+PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
+ void **ca,
+ size_t *ca_len)
+{
+ bool have_peer;
+
+ *ca = NULL;
+ *ca_len = 0;
+ have_peer = socket_address (connection,
+ ca,
+ ca_len);
+ if (PH_respect_forwarded_headers)
+ {
+ const char *xff;
+
+ /* A forwarding header only means anything if it came from
+ something entitled to set it. With a policy configured, that is
+ decided here, once, for the peer we actually accepted from:
+ otherwise any client could open a connection and hand us a chain
+ naming whoever it liked. A Unix-domain peer is taken as
+ entitled -- it has no address to match, is by construction on
+ this host, and reaching the socket at all is governed by
+ UNIXPATH_MODE. GNUnet's own service ACLs treat AF_UNIX the same
+ way. */
+ if ( (! PH_have_trusted_proxies) ||
+ (! have_peer) ||
+ (PAIVANA_HTTPD_is_trusted_proxy (*ca,
+ *ca_len)) )
+ {
+ xff = MHD_lookup_connection_value (connection,
+ MHD_HEADER_KIND,
+ PH_HEADER_X_FORWARDED_FOR);
+ if (NULL != xff)
+ {
+ GNUNET_free (*ca);
+ *ca = NULL;
+ *ca_len = 0;
+ return PAIVANA_HTTPD_parse_forwarded_for (xff,
+ ca,
+ ca_len);
+ }
+ /* No header present: the socket address stands. */
+ }
+ else
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Ignoring X-Forwarded-For: peer is not a trusted proxy\n");
+ }
+ }
+ if (! have_peer)
+ {
+ /* AF_UNIX without -f: there is nothing to bind the cookie to. The
+ shipped packaging serves over a Unix socket and passes -f for
+ exactly this reason. */
GNUNET_break (0);
return false;
}
+ return true;
}
diff --git a/src/backend/paivana-httpd_helper.h b/src/backend/paivana-httpd_helper.h
@@ -58,23 +58,49 @@ PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
/**
* Extract the client address from an `X-Forwarded-For` header value.
*
- * Uses the first element of @a xff (the client as seen by the
- * outermost proxy) and returns it in the same binary form
+ * Returns it in the same binary form
* #PAIVANA_HTTPD_get_client_address() derives from a socket, so that
* the two agree for a given host. Exposed separately from that
* function so it can be tested without an MHD connection.
*
+ * Which element of the chain names the client depends on what we have
+ * been told to trust. With `TRUSTED_PROXIES` configured, the chain is
+ * walked from the right and the hops we trust are skipped; the first
+ * element that is not one of our proxies is as far back as the header
+ * can be believed. Without such a policy the leftmost element is
+ * used, which trusts every hop — including whatever the client itself
+ * wrote, unless the proxy in front overwrites the header.
+ *
* @param xff value of the `X-Forwarded-For` header
* @param[out] ca where to write the client address
* @param[out] ca_len number of bytes in @a ca
- * @return true on success, false if the first element is not a bare
- * IPv4 or IPv6 address
+ * @return true on success, false if the selected element is not a
+ * bare IPv4 or IPv6 address
*/
bool
PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
void **ca,
size_t *ca_len);
+
+/**
+ * Is @a ca the address of a reverse proxy we trust to report the
+ * client address truthfully, i.e. one covered by `TRUSTED_PROXIES` or
+ * `TRUSTED_PROXIES6`?
+ *
+ * Always false if no policy was configured: "trust nothing in
+ * particular" and "trust everything" are deliberately different
+ * answers, and only the former can be stated safely by default.
+ *
+ * @param ca address in the binary form used throughout, i.e. 4 bytes
+ * for IPv4 and 16 for IPv6
+ * @param ca_len number of bytes in @a ca
+ * @return true if @a ca is within a configured trusted network
+ */
+bool
+PAIVANA_HTTPD_is_trusted_proxy (const void *ca,
+ size_t ca_len);
+
/**
* Determine the Base URL that the client made the HTTP request to.
* The URL returned will be without the trailing '/'.
diff --git a/src/tests/README b/src/tests/README
@@ -139,6 +139,11 @@ Forwarding-header tests (run once):
X-Forwarded-For is emitted at all.
This is the only case that reaches the
address-less code paths.
+ TRUSTED_PROXIES startup a policy that parses to nothing usable
+ (missing trailing ';', a /0 network, an
+ address of the wrong family, junk) must
+ abort startup rather than silently
+ trusting nobody; usable ones must start.
Cross-cutting tests (run once):
@@ -218,6 +223,30 @@ verifying and the visitor is asked to pay again. The test asserts:
7239 "unknown"/"_hidden", hostnames, zone ids, junk),
- a cookie issued for one host is not accepted for another.
+It also covers the trusted-proxy policy (TRUSTED_PROXIES /
+TRUSTED_PROXIES6):
+
+ - which addresses fall inside a configured policy, including that an
+ IPv4-mapped address is matched against the *IPv4* list,
+ - that walking the chain from the right and skipping trusted hops
+ picks the address the trusted proxy reported, so a client cannot
+ promote an entry it prepended itself,
+ - that an unparseable element ends the walk rather than letting the
+ search continue past it,
+ - that with no policy configured the leftmost element is used and
+ nothing is trusted.
+
+A separate group pins the behaviour of GNUnet's
+GNUNET_STRINGS_parse_ipv{4,6}_policy() that load_trusted_proxies()
+compensates for: the mandatory trailing ';', the v4/v6 disagreement
+about spaces, and the two ways those parsers return "nothing usable"
+without returning NULL (a /0 network, which is indistinguishable from
+the list terminator, and an address of the wrong family). If upstream
+ever fixes these, this group is what says so.
+
+The startup validation built on top of that is in the integration
+suite instead, since it is about whether the daemon comes up.
+
It links paivana-httpd_helper.c and paivana-httpd_cookie.c directly
and supplies the daemon globals itself, so it needs no MHD connection
and no merchant backend. The integration suite cannot cover any of
diff --git a/src/tests/test_client_address.c b/src/tests/test_client_address.c
@@ -48,6 +48,9 @@
int PH_respect_forwarded_headers;
char *PH_base_url;
int PH_global_cookie;
+struct GNUNET_STRINGS_IPv4NetworkPolicy *PH_trusted_proxies4;
+struct GNUNET_STRINGS_IPv6NetworkPolicy *PH_trusted_proxies6;
+bool PH_have_trusted_proxies;
/**
* Cookie key, normally set up by paivana-httpd.c from the SECRET
@@ -349,6 +352,298 @@ same_identity (const char *a,
}
+/**
+ * Install a trusted-proxy policy for the checks that follow, or clear
+ * it when both arguments are NULL.
+ *
+ * The GNUnet policy parsers are lenient enough that "returned
+ * non-NULL" is not the same as "understood something" -- see
+ * load_trusted_proxies() in paivana-httpd.c -- so this asserts that
+ * usable entries actually came back.
+ *
+ * @param v4 IPv4 policy string, or NULL
+ * @param v6 IPv6 policy string, or NULL
+ */
+static void
+set_policy (const char *v4,
+ const char *v6)
+{
+ GNUNET_free (PH_trusted_proxies4);
+ GNUNET_free (PH_trusted_proxies6);
+ PH_trusted_proxies4 = NULL;
+ PH_trusted_proxies6 = NULL;
+ PH_have_trusted_proxies = false;
+ if (NULL != v4)
+ {
+ PH_trusted_proxies4 = GNUNET_STRINGS_parse_ipv4_policy (v4);
+ GNUNET_assert (NULL != PH_trusted_proxies4);
+ GNUNET_assert (0 != PH_trusted_proxies4[0].network.s_addr);
+ PH_have_trusted_proxies = true;
+ }
+ if (NULL != v6)
+ {
+ PH_trusted_proxies6 = GNUNET_STRINGS_parse_ipv6_policy (v6);
+ GNUNET_assert (NULL != PH_trusted_proxies6);
+ GNUNET_assert (! GNUNET_is_zero (&PH_trusted_proxies6[0].network));
+ PH_have_trusted_proxies = true;
+ }
+}
+
+
+/**
+ * Check that @a xff, walked under the policy currently installed,
+ * names @a want as the client.
+ *
+ * @param xff `X-Forwarded-For` value
+ * @param want expected client address in presentation form
+ */
+static void
+client_is (const char *xff,
+ const char *want)
+{
+ void *got;
+ void *exp;
+ size_t got_len;
+ size_t exp_len;
+
+ socket_address (want,
+ &exp,
+ &exp_len);
+ if (! PAIVANA_HTTPD_parse_forwarded_for (xff,
+ &got,
+ &got_len))
+ {
+ fprintf (stderr,
+ "FAIL: `%s' rejected, want the client to be %s\n",
+ xff,
+ want);
+ failures++;
+ GNUNET_free (exp);
+ return;
+ }
+ if ( (got_len != exp_len) ||
+ (0 != memcmp (got,
+ exp,
+ got_len)) )
+ {
+ char ghex[2 * sizeof (struct in6_addr) + 1];
+ char ehex[2 * sizeof (struct in6_addr) + 1];
+
+ tohex (got,
+ got_len,
+ ghex);
+ tohex (exp,
+ exp_len,
+ ehex);
+ fprintf (stderr,
+ "FAIL: `%s' gives %s, want %s (%s)\n",
+ xff,
+ ghex,
+ ehex,
+ want);
+ failures++;
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: `%s' -> %s\n",
+ xff,
+ want);
+ }
+ GNUNET_free (got);
+ GNUNET_free (exp);
+}
+
+
+/**
+ * Check that @a addr is (or is not) inside the installed policy.
+ *
+ * @param addr address in presentation form
+ * @param want expected verdict
+ */
+static void
+trusted_is (const char *addr,
+ bool want)
+{
+ void *ca;
+ size_t ca_len;
+ bool got;
+
+ socket_address (addr,
+ &ca,
+ &ca_len);
+ got = PAIVANA_HTTPD_is_trusted_proxy (ca,
+ ca_len);
+ if (got != want)
+ {
+ fprintf (stderr,
+ "FAIL: %s trusted=%s, want %s\n",
+ addr,
+ got ? "true" : "false",
+ want ? "true" : "false");
+ failures++;
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: %s trusted=%s\n",
+ addr,
+ got ? "true" : "false");
+ }
+ GNUNET_free (ca);
+}
+
+
+/**
+ * Pin the behaviour of the GNUnet policy parsers that
+ * load_trusted_proxies() has to compensate for. These are not our
+ * functions, and their edge cases are what the configuration loader
+ * rejects on the operator's behalf; if upstream ever changes them,
+ * this is where it shows up.
+ */
+static void
+test_policy_parser (void)
+{
+ struct GNUNET_STRINGS_IPv4NetworkPolicy *p4;
+ struct GNUNET_STRINGS_IPv6NetworkPolicy *p6;
+
+ /* The trailing ';' is a terminator, not a separator: without it
+ nothing parses at all. */
+ p4 = GNUNET_STRINGS_parse_ipv4_policy ("10.0.0.0/8");
+ if (NULL != p4)
+ {
+ fprintf (stderr,
+ "FAIL: unterminated IPv4 policy accepted\n");
+ failures++;
+ GNUNET_free (p4);
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: unterminated IPv4 policy refused\n");
+ }
+ p6 = GNUNET_STRINGS_parse_ipv6_policy ("2001:db8::/32");
+ if (NULL != p6)
+ {
+ fprintf (stderr,
+ "FAIL: unterminated IPv6 policy accepted\n");
+ failures++;
+ GNUNET_free (p6);
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: unterminated IPv6 policy refused\n");
+ }
+ /* The list is terminated by an all-zero entry, so a /0 network is
+ indistinguishable from the end of it: "trust everyone" parses to
+ "trust no one", taking any later entries with it. */
+ p4 = GNUNET_STRINGS_parse_ipv4_policy ("0.0.0.0/0;10.0.0.0/8;");
+ if ( (NULL == p4) ||
+ (0 != p4[0].network.s_addr) )
+ {
+ fprintf (stderr,
+ "FAIL: 0.0.0.0/0 no longer swallows the IPv4 list --"
+ " load_trusted_proxies() can stop apologising for it\n");
+ failures++;
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: 0.0.0.0/0 yields an empty list (rejected at startup)\n");
+ }
+ GNUNET_free (p4);
+ p6 = GNUNET_STRINGS_parse_ipv6_policy ("::/0;2001:db8::/32;");
+ if ( (NULL == p6) ||
+ (! GNUNET_is_zero (&p6[0].network)) )
+ {
+ fprintf (stderr,
+ "FAIL: ::/0 no longer swallows the IPv6 list\n");
+ failures++;
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: ::/0 yields an empty list (rejected at startup)\n");
+ }
+ GNUNET_free (p6);
+ /* An IPv6 address in the IPv4 key returns a valid pointer to an
+ empty list rather than an error, which is why the loader counts
+ entries instead of just checking for NULL. */
+ p4 = GNUNET_STRINGS_parse_ipv4_policy ("::1;");
+ if ( (NULL == p4) ||
+ (0 != p4[0].network.s_addr) )
+ {
+ fprintf (stderr,
+ "FAIL: an IPv6 address in the IPv4 key no longer parses to"
+ " an empty list\n");
+ failures++;
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: IPv6 in the IPv4 key yields an empty list\n");
+ }
+ GNUNET_free (p4);
+ /* Whitespace between entries: tolerated by the v4 parser, fatal to
+ the v6 one. Documented in the loader's error message. */
+ p4 = GNUNET_STRINGS_parse_ipv4_policy ("10.0.0.0/8; 192.168.0.0/16;");
+ if (NULL == p4)
+ {
+ fprintf (stderr,
+ "FAIL: IPv4 policy with spaces refused\n");
+ failures++;
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: IPv4 policy tolerates spaces between entries\n");
+ }
+ GNUNET_free (p4);
+ p6 = GNUNET_STRINGS_parse_ipv6_policy ("2001:db8::/32; fe80::/10;");
+ if (NULL != p6)
+ {
+ fprintf (stderr,
+ "FAIL: IPv6 policy with spaces accepted -- the loader's"
+ " error message says otherwise\n");
+ failures++;
+ GNUNET_free (p6);
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: IPv6 policy refuses spaces between entries\n");
+ }
+ /* Out-of-range prefix lengths are caught. */
+ p4 = GNUNET_STRINGS_parse_ipv4_policy ("10.0.0.0/33;");
+ if (NULL != p4)
+ {
+ fprintf (stderr,
+ "FAIL: IPv4 /33 accepted\n");
+ failures++;
+ GNUNET_free (p4);
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: IPv4 /33 refused\n");
+ }
+ p6 = GNUNET_STRINGS_parse_ipv6_policy ("2001:db8::/129;");
+ if (NULL != p6)
+ {
+ fprintf (stderr,
+ "FAIL: IPv6 /129 accepted\n");
+ failures++;
+ GNUNET_free (p6);
+ }
+ else
+ {
+ fprintf (stderr,
+ " ok: IPv6 /129 refused\n");
+ }
+}
+
+
int
main (int argc,
char *const *argv)
@@ -418,6 +713,62 @@ main (int argc,
"2001:0db8:0000:0000:0000:0000:0000:0001:0002:0003:0004:0005:0006");
fprintf (stderr,
+ "-- the GNUnet policy parsers behave as the loader assumes --\n");
+ test_policy_parser ();
+
+ fprintf (stderr,
+ "-- matching against a trusted-proxy policy --\n");
+ set_policy ("10.0.0.0/8;192.168.0.0/16;",
+ "2001:db8::/32;");
+ trusted_is ("10.0.0.1", true);
+ trusted_is ("10.255.255.255", true);
+ trusted_is ("11.0.0.1", false);
+ trusted_is ("192.168.0.1", true);
+ trusted_is ("192.169.0.1", false);
+ trusted_is ("2001:db8::1", true);
+ trusted_is ("2001:db9::1", false);
+ trusted_is ("203.0.113.7", false);
+ /* An IPv4-mapped address is folded to 4 bytes before it gets here,
+ so it is matched against the IPv4 list -- which is why IPv4
+ proxies belong in TRUSTED_PROXIES and not in TRUSTED_PROXIES6 as
+ ::ffff:10.0.0.1. */
+ trusted_is ("::ffff:10.0.0.1", true);
+ set_policy (NULL, NULL);
+ trusted_is ("10.0.0.1", false); /* no policy: nothing is trusted */
+
+ fprintf (stderr,
+ "-- walking the chain under a policy --\n");
+ set_policy ("10.0.0.0/8;", NULL);
+ /* The rightmost hops are ours; the first one that is not is the
+ client, however many entries the client prepended itself. */
+ client_is ("203.0.113.7, 10.0.0.1",
+ "203.0.113.7");
+ client_is ("203.0.113.7, 10.0.0.1, 10.0.0.2",
+ "203.0.113.7");
+ client_is ("1.2.3.4, 203.0.113.7, 10.0.0.1",
+ "203.0.113.7");
+ /* A single untrusted entry is the client. */
+ client_is ("203.0.113.7",
+ "203.0.113.7");
+ /* Nothing but our own proxies: the leftmost is all we have. */
+ client_is ("10.0.0.1, 10.0.0.2",
+ "10.0.0.1");
+ /* A forged element to the left of a trusted proxy cannot promote
+ itself: the walk stops at the first untrusted hop from the right,
+ which is the address the trusted proxy actually reported. */
+ client_is ("unknown, 203.0.113.7, 10.0.0.1",
+ "203.0.113.7");
+ /* An element we cannot parse ends the walk: nothing further left is
+ attributable to a proxy we trust. */
+ refused ("garbage, 10.0.0.1");
+ refused ("203.0.113.7:80, 10.0.0.1");
+ set_policy (NULL, NULL);
+ /* Without a policy the leftmost element is the client, forged or
+ not -- the reason the startup warning exists. */
+ client_is ("1.2.3.4, 203.0.113.7, 10.0.0.1",
+ "1.2.3.4");
+
+ fprintf (stderr,
"-- distinct hosts stay distinct --\n");
{
void *a;
diff --git a/src/tests/test_reverse_proxy.sh b/src/tests/test_reverse_proxy.sh
@@ -1009,6 +1009,79 @@ function test_forwarded_unix() {
}
######################################################################
+# TRUSTED_PROXIES configuration validation.
+#
+# The GNUnet policy parsers accept several things that mean "nothing
+# usable" without saying so -- a missing terminator, a /0 network
+# (which is indistinguishable from the list terminator), an address of
+# the wrong family. Quietly trusting nobody would send every visitor
+# to the socket address with no hint why, so the loader refuses to
+# start instead. These cases pin that.
+######################################################################
+
+# Start paivana with an extra config line and report whether it came
+# up. Echoes "started" or "refused".
+function paivana_with_config_line() {
+ local line="$1"
+ local cfg="$TMPDIR/trusted.conf"
+ sed -e "s|@DEST@|http://127.0.0.1:$MHD_PORT|g" \
+ -e "s|@PORT@|$PAIVANA_PORT|g" \
+ "$SRCDIR/test_reverse_proxy.conf.in" > "$cfg"
+ printf '%s\n' "$line" >> "$cfg"
+ local log="$LOGDIR/trusted.log"
+ ( exec "$PAIVANA_HTTPD" -c "$cfg" -n -f -L ERROR ) >"$log" 2>&1 &
+ local pid=$!
+ if wait_for_port 127.0.0.1 "$PAIVANA_PORT";
+ then
+ kill -TERM "$pid" 2>/dev/null
+ wait "$pid" 2>/dev/null
+ echo "started"
+ return
+ fi
+ kill -TERM "$pid" 2>/dev/null
+ wait "$pid" 2>/dev/null
+ echo "refused"
+}
+
+function test_trusted_proxies_config() {
+ stop_paivana
+ local r
+
+ # Bad values must be refused loudly rather than silently ignored.
+ for bad in \
+ 'TRUSTED_PROXIES = 10.0.0.0/8' \
+ 'TRUSTED_PROXIES = 0.0.0.0/0;' \
+ 'TRUSTED_PROXIES = ::1;' \
+ 'TRUSTED_PROXIES = garbage;' \
+ 'TRUSTED_PROXIES6 = 2001:db8::/32' \
+ 'TRUSTED_PROXIES6 = ::/0;' \
+ 'TRUSTED_PROXIES6 = 2001:db8::/32; fe80::/10;'
+ do
+ msg "startup refused: $bad"
+ r="$(paivana_with_config_line "$bad")"
+ [ "$r" = "refused" ] || \
+ fail "paivana started with an unusable policy ($bad)"
+ ok
+ done
+
+ # ...and good ones must of course still start.
+ for good in \
+ 'TRUSTED_PROXIES = 10.0.0.0/8;192.168.0.0/16;' \
+ 'TRUSTED_PROXIES = 127.0.0.1;' \
+ 'TRUSTED_PROXIES6 = 2001:db8::/32;fe80::/10;' \
+ 'TRUSTED_PROXIES6 = ::1;'
+ do
+ msg "startup accepted: $good"
+ r="$(paivana_with_config_line "$good")"
+ [ "$r" = "started" ] || \
+ fail "paivana refused a usable policy ($good); log:\n$(cat "$LOGDIR/trusted.log")"
+ ok
+ done
+
+ start_paivana "http://127.0.0.1:$MHD_PORT"
+}
+
+######################################################################
# Drive the tests.
######################################################################
@@ -1035,6 +1108,7 @@ test_pipelined
test_forwarded_no_flag
test_forwarded_with_flag
test_forwarded_unix
+test_trusted_proxies_config
stop_paivana
