paivana

HTTP paywall reverse proxy
Log | Files | Refs | Submodules | README | LICENSE

commit cb59489c4f245950fdc789a719d5cddac1418291
parent f59d992032e2e2b698790e560d9518a0f280c728
Author: Christian Grothoff <christian@grothoff.org>
Date:   Thu,  6 Aug 2026 19:23:50 +0200

clean up forwarding header chain parsing and handling

Diffstat:
Msrc/backend/paivana-httpd.c | 43++++++++++++++++++++++++++++++++++++++-----
Msrc/backend/paivana-httpd_helper.c | 2048+++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
Msrc/backend/paivana-httpd_helper.h | 257+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------
Msrc/tests/README | 79+++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
Msrc/tests/meson.build | 2+-
Msrc/tests/test_client_address.c | 1387+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------
Msrc/tests/test_reverse_proxy.sh | 9+++++++++
7 files changed, 2759 insertions(+), 1066 deletions(-)

diff --git a/src/backend/paivana-httpd.c b/src/backend/paivana-httpd.c @@ -109,11 +109,19 @@ static struct GNUNET_CURL_RescheduleContext *proxy_ctx_rc; * 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. + * yields a valid pointer to an empty list rather than an error; + * - an entry that is not terminated by ';' -- the last one, when the + * operator wrote the separators but not the terminator -- is dropped + * without a word, and so is anything after the final ';'. * - * 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. + * The first two 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. The third + * is worse, because it succeeds: a single missing ';' would leave the + * clients behind an unlisted proxy sharing that proxy's address as + * their identity, i.e. sharing one paid cookie. So the entries that + * came back are counted against the ';' that went in, and the value + * has to end in one. * * @param c configuration to read from * @param option name of the option @@ -128,6 +136,8 @@ load_trusted_proxies (const struct GNUNET_CONFIGURATION_Handle *c, char *opt; bool v6 = (0 != strcmp (option, "TRUSTED_PROXIES")); + unsigned int want = 0; + size_t len; *count = 0; if (GNUNET_OK != @@ -136,6 +146,13 @@ load_trusted_proxies (const struct GNUNET_CONFIGURATION_Handle *c, option, &opt)) return true; /* not configured at all: fine */ + len = strlen (opt); + while ( (len > 0) && + ( (' ' == opt[len - 1]) || + ('\t' == opt[len - 1]) ) ) + opt[--len] = '\0'; + for (const char *p = strchr (opt, ';'); NULL != p; p = strchr (p + 1, ';')) + want++; if (v6) { PH_trusted_proxies6 = GNUNET_STRINGS_parse_ipv6_policy (opt); @@ -150,8 +167,24 @@ load_trusted_proxies (const struct GNUNET_CONFIGURATION_Handle *c, while (0 != PH_trusted_proxies4[*count].network.s_addr) (*count)++; } - if (0 == *count) + if ( (0 == *count) || + (*count != want) || + (0 == len) || + (';' != opt[len - 1]) ) { + if ( (0 != *count) && + (*count != want) ) + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Only %u of the %u entries of `%s' were understood;" + " refusing to trust a prefix of the list\n", + *count, + want, + option); + else if (0 != *count) + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "`%s' does not end in the ';' that terminates the last" + " entry; anything after the final one is dropped\n", + option); GNUNET_log_config_invalid ( GNUNET_ERROR_TYPE_ERROR, "paivana", diff --git a/src/backend/paivana-httpd_helper.c b/src/backend/paivana-httpd_helper.c @@ -29,321 +29,351 @@ #include <taler/taler_mhd_lib.h> /** - * Store IPv6 address @a a6 in @a ca / @a ca_len. + * Longest node identifier we keep. RFC 7239 §6 has + * `node = nodename [ ":" node-port ]` with + * `nodename = ... / "[" IPv6address "]"`, so the longest one that can + * name an address is `[` + 45 + `]` + `:` + 5 = 53 bytes -- more than + * INET6_ADDRSTRLEN, which is why the port cannot be stripped after a + * length check against that. + */ +#define PH_NODE_MAX 64 + +/** + * Longest authority we accept in a `host` parameter or an + * `X-Forwarded-Host`. A DNS name is at most 253 bytes and an IPv6 + * literal with brackets and port 53; anything longer is not an + * authority. + */ +#define PH_HOST_MAX 256 + +/** + * Longest `Forwarded` parameter value we parse. The values we care + * about are a node, a scheme or an authority; the rest (`by`, + * extensions) only has to fit for the element to be understood at all. + */ +#define PH_VALUE_MAX 512 + + +/** + * One hop of a forwarding chain, as reported by the hop to its right. + */ +struct Element +{ + + /** + * Node identifier as it appeared in the header, for logging; "" if + * it did not fit. + */ + char node[PH_NODE_MAX]; + + /** + * Authority from the element's `host` parameter, or "" if it had + * none we could use. + */ + char host[PH_HOST_MAX]; + + /** + * Binary form of @e node, in the representation the socket branch + * produces for the same host. + */ + unsigned char addr[sizeof (struct in6_addr)]; + + /** + * Number of bytes in @e addr; 0 if @e node names no address. + */ + size_t addr_len; + + /** + * Did the element carry a `proto` we understood? + */ + bool have_proto; + + /** + * Was that `proto` https? + */ + bool https; +}; + + +/** + * Store IPv6 address @a a6 in @a out / @a out_len. * * An IPv4-mapped address (::ffff:a.b.c.d) is unwrapped to the four * bytes a plain IPv4 peer would have yielded: it names the same host, * and the two spellings must not produce two identities. * * @param a6 address to store - * @param[out] ca where to write the allocated address - * @param[out] ca_len set to the number of bytes in @a ca + * @param[out] out buffer of at least `sizeof (struct in6_addr)` bytes + * @param[out] out_len set to the number of bytes written to @a out */ static void -store_v6 (const struct in6_addr *a6, - void **ca, - size_t *ca_len) +pack_v6 (const struct in6_addr *a6, + unsigned char *out, + size_t *out_len) { if (IN6_IS_ADDR_V4MAPPED (a6)) { - *ca = GNUNET_memdup (&a6->s6_addr[12], - sizeof (struct in_addr)); - *ca_len = sizeof (struct in_addr); + memcpy (out, + &a6->s6_addr[12], + sizeof (struct in_addr)); + *out_len = sizeof (struct in_addr); return; } - *ca = GNUNET_memdup (a6, - sizeof (*a6)); - *ca_len = sizeof (*a6); + memcpy (out, + a6, + sizeof (*a6)); + *out_len = sizeof (*a6); } /** - * Parse a single `X-Forwarded-For` element into binary form. + * Is @a c a `tchar`, i.e. legal in a token (RFC 9110 §5.6.2)? * - * 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 + * @param c character to test + * @return true if @a c may appear in a token */ static bool -parse_element (const char *tok, - size_t len, - void **ca, - size_t *ca_len) +is_tchar (char c) { - /* Long enough for any address inet_pton() accepts. */ - char addr[INET6_ADDRSTRLEN]; - struct in_addr a4; - struct in6_addr a6; - - *ca = NULL; - *ca_len = 0; - if ( (0 == len) || - (len >= sizeof (addr)) ) - { - GNUNET_break_op (0); + if ('\0' == c) return false; - } - memcpy (addr, - tok, - len); - addr[len] = '\0'; - if (1 == inet_pton (AF_INET, - addr, - &a4)) - { - *ca = GNUNET_memdup (&a4, - sizeof (a4)); - *ca_len = sizeof (a4); - return true; - } - if (1 == inet_pton (AF_INET6, - addr, - &a6)) - { - store_v6 (&a6, - ca, - ca_len); - return true; - } - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Malformed X-Forwarded-For client address `%s'\n", - addr); - GNUNET_break_op (0); - return false; + return ( (c >= 'a') && (c <= 'z') ) || + ( (c >= 'A') && (c <= 'Z') ) || + ( (c >= '0') && (c <= '9') ) || + (NULL != strchr ("!#$%&'*+-.^_`|~", + c)); } /** - * Locate element number @a idx (counting from 0 at the left) of the - * comma-separated list @a xff, with surrounding whitespace stripped. + * May @a c appear in an unquoted RFC 7239 §4 `value`? * - * @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 + * The grammar says `token`, but proxies in the field -- the Apache + * snippet we ship among them -- emit an unquoted authority, and `:` + * is not a `tchar`. Refusing those outright would throw away the + * whole chain a correctly configured front server sent, so anything + * that is not a delimiter, whitespace or a control character is read + * as part of the value. That keeps the element boundaries exactly + * where the grammar puts them, which is the part that matters; whether + * the value is one we can *use* is then decided per parameter. + * + * @param c character to test + * @return true if @a c continues an unquoted value */ -static const char * -nth_element (const char *xff, - unsigned int idx, - size_t *len) +static bool +is_value_char (char c) { - const char *p = xff; + unsigned char u = (unsigned char) c; - for (unsigned int i = 0; /* until return */; i++) - { - const char *comma; - size_t l; + return (u > 0x20) && + (0x7F != u) && + (';' != u) && + (',' != u) && + ('"' != u); +} - 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; - } + +/** + * Is @a c a `qdtext`, i.e. legal unescaped inside a quoted-string + * (RFC 9110 §5.6.4)? + * + * @param c character to test + * @return true if @a c may appear unescaped + */ +static bool +is_qdtext (char c) +{ + unsigned char u = (unsigned char) c; + + return ('\t' == u) || + (' ' == u) || + (0x21 == u) || + ( (u >= 0x23) && (u <= 0x5B) ) || + ( (u >= 0x5D) && (u <= 0x7E) ) || + (u >= 0x80); } /** - * Count the elements of the comma-separated list @a xff. + * May @a c follow a backslash in a quoted-pair (RFC 9110 §5.6.4)? * - * @param xff header value to scan - * @return number of elements (at least 1 for a non-empty string) + * @param c character to test + * @return true if @a c may be escaped */ -static unsigned int -count_elements (const char *xff) +static bool +is_escapable (char c) { - unsigned int n = 1; + unsigned char u = (unsigned char) c; - for (const char *p = strchr (xff, ','); NULL != p; p = strchr (p + 1, ',')) - n++; - return n; + return ('\t' == u) || + ( (u >= 0x20) && (u <= 0x7E) ) || + (u >= 0x80); +} + + +/** + * Skip optional whitespace (RFC 9110 §5.6.3). + * + * @param p position to start at + * @return first position that is neither SP nor HTAB + */ +static const char * +skip_ows (const char *p) +{ + while ( (' ' == *p) || + ('\t' == *p) ) + p++; + return p; } /** - * Locate element number @a idx of the RFC 7239 `Forwarded` list - * @a fwd and return the value of its @a name parameter, with any - * surrounding quotes removed. - * - * Commas and semicolons inside a quoted string are data, not - * delimiters (RFC 7239 §4 via RFC 9110 §5.6.4), so the scan has to - * track quoting rather than reach for strchr(). - * - * A quoted-pair escape is taken verbatim: none of the values we care - * about (an address, a scheme, an authority) can legitimately contain - * one, so anything that needed unescaping will fail the checks the - * caller applies anyway. - * - * @param fwd header value to scan - * @param idx which element to read, counting from 0 at the left - * @param name parameter to look for, e.g. "for" - * @param[out] out buffer for the extracted value + * Read one RFC 7239 §4 `value` -- a token or a quoted-string -- from + * @a pp into @a out, advancing @a pp past it. + * + * The quoted-string is unescaped here, which is the only place that + * may happen: a caller handed the raw bytes could not tell a `,` that + * was data from one that was a delimiter. + * + * @param[in,out] pp position to read from; advanced past the value + * @param[out] out buffer for the unescaped value * @param out_size number of bytes in @a out - * @return true if element @a idx exists and carries @a name + * @return true if a well-formed value was read */ static bool -nth_forwarded_param (const char *fwd, - unsigned int idx, - const char *name, - char *out, - size_t out_size) -{ - const char *p = fwd; - const size_t namelen = strlen (name); - unsigned int elem = 0; - bool in_quotes = false; - const char *pair = NULL; - - for (; ; p++) +parse_value (const char **pp, + char *out, + size_t out_size) +{ + const char *p = *pp; + size_t len = 0; + + if ('"' == *p) { - if (in_quotes) + p++; + while ('"' != *p) { - if ('\0' == *p) + char c = *p; + + if ('\0' == c) { + /* An unterminated quoted-string is not a value at all, and + reading on would run off the end of the header. */ GNUNET_break_op (0); return false; } - if ('\\' == *p) + if ('\\' == c) { - if ('\0' == p[1]) + c = p[1]; + if (! is_escapable (c)) + { + GNUNET_break_op (0); return false; + } p++; - continue; } - if ('"' == *p) - in_quotes = false; - continue; - } - if ('"' == *p) - { - in_quotes = true; - continue; - } - if ( (',' != *p) && - (';' != *p) && - ('\0' != *p) ) - { - /* Skip the whitespace that follows a delimiter: "a, b" starts - its second element at 'b', not at the space. */ - if ( (NULL == pair) && - (' ' != *p) && - ('\t' != *p) ) - pair = p; - continue; + else if (! is_qdtext (c)) + { + GNUNET_break_op (0); + return false; + } + if (len + 1 >= out_size) + { + GNUNET_break_op (0); + return false; + } + out[len++] = c; + p++; } - /* End of one parameter. Keep it if it is the one we were asked - for, in the element we were asked for. */ - if ( (NULL != pair) && - (elem == idx) && - (0 == strncasecmp (pair, - name, - namelen)) ) + p++; /* closing DQUOTE */ + } + else + { + while (is_value_char (*p)) { - const char *v = pair + namelen; - - while ( (' ' == *v) || - ('\t' == *v) ) - v++; - if ('=' == *v) + if (len + 1 >= out_size) { - size_t len; - - v++; - while ( (' ' == *v) || - ('\t' == *v) ) - v++; - len = (size_t) (p - v); - while ( (len > 0) && - ( (' ' == v[len - 1]) || - ('\t' == v[len - 1]) ) ) - len--; - if ( (len >= 2) && - ('"' == v[0]) && - ('"' == v[len - 1]) ) - { - v++; - len -= 2; - } - if (len >= out_size) - return false; - memcpy (out, - v, - len); - out[len] = '\0'; - return true; + GNUNET_break_op (0); + return false; } + out[len++] = *p++; } - if ('\0' == *p) - return false; - if (',' == *p) + if (0 == len) { - if (elem == idx) - return false; /* element ended without the parameter */ - elem++; + /* `for=` with nothing after it is not a `value`. */ + GNUNET_break_op (0); + return false; } - pair = NULL; } + out[len] = '\0'; + *pp = p; + return true; } /** - * Reduce an RFC 7239 node identifier to a bare address by removing - * brackets and any port, in place. + * Parse the RFC 7239 §6 node identifier @a node 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. * - * RFC 7239 §6 permits `[2001:db8::1]:443` and `203.0.113.7:80`; a - * port is not part of the identity of a host and the cookie must not - * depend on it. + * Brackets and any port are removed first: a port is not part of the + * identity of a host and the cookie must not depend on it. A node + * that is not an address at all -- the `unknown` of §6.3, an + * obfuscated identifier, a hostname -- has no such form and is + * reported as naming none. * - * @param[in,out] node identifier to trim + * @param node identifier to parse + * @param[out] addr buffer of at least `sizeof (struct in6_addr)` bytes + * @param[out] addr_len set to the number of bytes written, 0 if + * @a node names no address */ static void -strip_node_port (char *node) +node_to_addr (const char *node, + unsigned char *addr, + size_t *addr_len) { + char tmp[PH_NODE_MAX]; + const char *a; size_t len = strlen (node); + struct in_addr a4; + struct in6_addr a6; - if ( (len > 0) && - ('[' == node[0]) ) + *addr_len = 0; + if ( (0 == len) || + (len >= sizeof (tmp)) ) + return; + memcpy (tmp, + node, + len + 1); + if ('[' == tmp[0]) { - char *close = strchr (node, + char *close = strchr (tmp, ']'); if (NULL == close) + { + /* RFC 7239 §6 requires the closing bracket. */ + GNUNET_break_op (0); return; + } *close = '\0'; - memmove (node, - node + 1, - strlen (node + 1) + 1); - return; + if ( ('\0' != close[1]) && + (':' != close[1]) ) + { + GNUNET_break_op (0); + return; + } + a = &tmp[1]; } + else { - char *colon = strchr (node, + char *colon = strchr (tmp, ':'); /* Only a *single* colon can be a port separator; more than one @@ -353,252 +383,482 @@ strip_node_port (char *node) (NULL == strchr (colon + 1, ':')) ) *colon = '\0'; + a = tmp; + } + if (1 == inet_pton (AF_INET, + a, + &a4)) + { + memcpy (addr, + &a4, + sizeof (a4)); + *addr_len = sizeof (a4); + return; } + if (1 == inet_pton (AF_INET6, + a, + &a6)) + pack_v6 (&a6, + addr, + addr_len); } /** - * Read the `for=` of element @a idx as a bare address. + * Is @a c legal in an RFC 3986 §3.2.2 `reg-name` as we accept them? * - * @param fwd header value to scan - * @param idx which element to read - * @param[out] out buffer for the address - * @param out_size number of bytes in @a out - * @return true if element @a idx exists and carries a `for=` + * Deliberately narrower than the grammar: percent-encoding and the + * sub-delims have no business in a host we are about to concatenate + * into the string the access cookie is keyed on and the templates' + * regular expressions are matched against. + * + * @param c character to test + * @return true if @a c may appear in a host name */ static bool -nth_forwarded_for (const char *fwd, - unsigned int idx, - char *out, - size_t out_size) -{ - if (! nth_forwarded_param (fwd, - idx, - "for", - out, - out_size)) - return false; - strip_node_port (out); - return true; -} - - -char * -PAIVANA_HTTPD_forwarded_param (const char *fwd, - const char *name) +is_host_char (char c) { - /* Long enough for an authority; anything longer is not one. */ - char buf[256]; - - /* The leftmost element describes the connection the client itself - made, which is what `proto` and `host` are being asked about. - With a single proxy in front -- the ordinary case -- there is only - one element and the question does not arise. */ - if (! nth_forwarded_param (fwd, - 0, - name, - buf, - sizeof (buf))) - return NULL; - if ('\0' == buf[0]) - return NULL; - return GNUNET_strdup (buf); + return ( (c >= 'a') && (c <= 'z') ) || + ( (c >= 'A') && (c <= 'Z') ) || + ( (c >= '0') && (c <= '9') ) || + ('-' == c) || + ('.' == c) || + ('_' == c); } /** - * Count the elements of the RFC 7239 list @a fwd, respecting quoting. + * Is @a p an RFC 3986 §3.2.3 port naming one that can be connected to? * - * @param fwd header value to scan - * @return number of elements (at least 1 for a non-empty string) + * @param p text to check + * @return true if @a p is a usable port number */ -static unsigned int -count_forwarded_elements (const char *fwd) +static bool +valid_port_text (const char *p) { - unsigned int n = 1; - bool in_quotes = false; + unsigned long v = 0; + size_t len = strlen (p); - for (const char *p = fwd; '\0' != *p; p++) + if ( (0 == len) || + (len > 5) ) + return false; + for (size_t i = 0; i < len; i++) { - if (in_quotes) - { - if ('\\' == *p) - { - if ('\0' == p[1]) - break; - p++; - continue; - } - if ('"' == *p) - in_quotes = false; - continue; - } - if ('"' == *p) - in_quotes = true; - else if (',' == *p) - n++; + if ( (p[i] < '0') || + (p[i] > '9') ) + return false; + v = v * 10 + (unsigned long) (p[i] - '0'); } - return n; + return ( (v > 0) && + (v < 65536) ); } -char * -PAIVANA_HTTPD_forwarded_for_chain (const char *fwd) +/** + * Is @a h an authority we are willing to build a base URL from, i.e. + * an RFC 3986 §3.2 `host [ ":" port ]`? + * + * @param h text to check + * @return true if @a h is such an authority + */ +static bool +valid_host (const char *h) { - struct GNUNET_Buffer buf = { 0 }; - unsigned int n = count_forwarded_elements (fwd); + size_t len = strlen (h); + const char *port; - for (unsigned int i = 0; i < n; i++) + if ( (0 == len) || + (len >= PH_HOST_MAX) ) + return false; + if ('[' == h[0]) { - char node[INET6_ADDRSTRLEN]; - struct in_addr a4; + /* RFC 3986 §3.2.2: an IPv6 literal is bracketed, and RFC 5952 §6 + asks for exactly that spelling in a URI. */ + char tmp[INET6_ADDRSTRLEN]; + const char *close = strchr (h, + ']'); struct in6_addr a6; + size_t hl; - if (! nth_forwarded_for (fwd, - i, - node, - sizeof (node))) - { - GNUNET_buffer_clear (&buf); - return NULL; - } - /* All or nothing: X-Forwarded-For has no way to say "this hop had - no address", so an element whose `for` is RFC 7239's "unknown", - an obfuscated identifier, or anything else that is not an - address cannot be represented. Dropping it would silently - shift every position to its left, which is worse than declining - to translate the header at all. */ - if ( (1 != inet_pton (AF_INET, - node, - &a4)) && - (1 != inet_pton (AF_INET6, - node, - &a6)) ) - { - GNUNET_buffer_clear (&buf); - return NULL; - } - if (0 != i) - GNUNET_buffer_write_str (&buf, - ", "); - GNUNET_buffer_write_str (&buf, - node); + if (NULL == close) + return false; + hl = (size_t) (close - h) - 1; + if ( (0 == hl) || + (hl >= sizeof (tmp)) ) + return false; + memcpy (tmp, + h + 1, + hl); + tmp[hl] = '\0'; + if (1 != inet_pton (AF_INET6, + tmp, + &a6)) + return false; + if ('\0' == close[1]) + return true; + if (':' != close[1]) + return false; + port = close + 2; } - return GNUNET_buffer_reap_str (&buf); + else + { + const char *colon = strchr (h, + ':'); + size_t hl = (NULL != colon) + ? (size_t) (colon - h) + : len; + + if (0 == hl) + return false; + for (size_t i = 0; i < hl; i++) + if (! is_host_char (h[i])) + return false; + if (NULL == colon) + return true; + port = colon + 1; + } + return valid_port_text (port); } -bool -PAIVANA_HTTPD_parse_forwarded (const char *fwd, - void **ca, - size_t *ca_len) +/** + * Does the authority @a h already carry a port? + * + * @param h authority that passed #valid_host() + * @return true if a port is present + */ +static bool +host_has_port (const char *h) { - unsigned int n; - char node[INET6_ADDRSTRLEN]; + const char *p = ('[' == h[0]) + ? strchr (h, + ']') + : h; - *ca = NULL; - *ca_len = 0; - n = count_forwarded_elements (fwd); - if (! PH_have_trusted_proxies) + if (NULL == p) + return false; + return (NULL != strchr (p, + ':')); +} + + +/** + * Parse one RFC 7239 §4 `forwarded-element` from @a pp into @a e, + * advancing @a pp to the `,` or NUL that ended it. + * + * @param[in,out] pp position to read from + * @param[out] e element to fill in + * @return true if the element was well-formed + */ +static bool +parse_forwarded_element (const char **pp, + struct Element *e) +{ + const char *p = *pp; + bool have_for = false; + bool have_host = false; + bool have_proto = false; + + memset (e, + 0, + sizeof (*e)); + while (true) { - if (! nth_forwarded_for (fwd, - 0, - node, - sizeof (node))) + char name[32]; + char value[PH_VALUE_MAX]; + size_t nlen = 0; + + p = skip_ows (p); + if (';' == *p) + { + /* RFC 7239 §4 permits an empty forwarded-pair. */ + p++; + continue; + } + if ( (',' == *p) || + ('\0' == *p) ) + break; + while (is_tchar (*p)) + { + if (nlen + 1 >= sizeof (name)) + { + GNUNET_break_op (0); + return false; + } + name[nlen++] = *p++; + } + if (0 == nlen) { GNUNET_break_op (0); return false; } - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Client address is based on Forwarded: `%s'\n", - node); - return parse_element (node, - strlen (node), - ca, - ca_len); - } - /* Same walk as for X-Forwarded-For; see there. */ - for (unsigned int i = n; i > 0; i--) - { - if (! nth_forwarded_for (fwd, - i - 1, - node, - sizeof (node))) + name[nlen] = '\0'; + p = skip_ows (p); + if ('=' != *p) { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Forwarded element without a usable `for'; not looking" - " further left in the chain\n"); + GNUNET_break_op (0); return false; } - if (! parse_element (node, - strlen (node), - ca, - ca_len)) + p++; + p = skip_ows (p); + if (! parse_value (&p, + value, + sizeof (value))) return false; - if (! PAIVANA_HTTPD_is_trusted_proxy (*ca, - *ca_len)) + /* RFC 7239 §4: "Each parameter MUST NOT occur more than once per + field-value." A second one would leave which of the two we + believe up to the direction we happen to scan in. */ + if (0 == strcasecmp (name, + "for")) { - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Client address is based on Forwarded: `%s'\n", - node); - return true; + if (have_for) + { + GNUNET_break_op (0); + return false; + } + have_for = true; + if (strlen (value) < sizeof (e->node)) + memcpy (e->node, + value, + strlen (value) + 1); + node_to_addr (value, + e->addr, + &e->addr_len); } - GNUNET_free (*ca); - *ca = NULL; - *ca_len = 0; - } - if (! nth_forwarded_for (fwd, - 0, - node, - sizeof (node))) - { - GNUNET_break (0); - return false; - } - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "All Forwarded elements are trusted proxies; using the leftmost" - " `%s' as the client\n", - node); - return parse_element (node, - strlen (node), - ca, - ca_len); -} - - -char * -PAIVANA_HTTPD_forwarded_node (const void *ca, - size_t ca_len) -{ - char buf[INET6_ADDRSTRLEN]; - char *ret; - - if ( (NULL == ca) || - (0 == ca_len) ) + else if (0 == strcasecmp (name, + "host")) + { + if (have_host) + { + GNUNET_break_op (0); + return false; + } + have_host = true; + if (valid_host (value)) + memcpy (e->host, + value, + strlen (value) + 1); + else + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Ignoring unusable Forwarded host `%s'\n", + value); + } + else if (0 == strcasecmp (name, + "proto")) + { + if (have_proto) + { + GNUNET_break_op (0); + return false; + } + have_proto = true; + /* Anything but the two schemes we can serve would end up in a + Location header and in the string the cookie is keyed on. */ + if (0 == strcasecmp (value, + "https")) + { + e->have_proto = true; + e->https = true; + } + else if (0 == strcasecmp (value, + "http")) + { + e->have_proto = true; + } + else + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Ignoring unusable Forwarded proto `%s'\n", + value); + } + } + /* `by` and extension parameters are none of our business, but had + to be parsed to find the end of the element. */ + p = skip_ows (p); + if (';' != *p) + break; + p++; + } + if ( (',' != *p) && + ('\0' != *p) ) { - /* RFC 7239 §6.3 provides exactly this for a hop whose predecessor - has no address we can name -- a Unix-domain peer, here. */ - return GNUNET_strdup ("unknown"); + GNUNET_break_op (0); + return false; } - if (sizeof (struct in_addr) == ca_len) + *pp = p; + return true; +} + + +/** + * Was this header present at all? + * + * @param lines NULL-terminated array of field line values, or NULL + * @return true if there is at least one field line + */ +static bool +have_lines (const char *const *lines) +{ + return ( (NULL != lines) && + (NULL != lines[0]) ); +} + + +/** + * Parse every `Forwarded` field line of @a lines into @a el, in order. + * + * RFC 9110 §5.3 makes repeated field lines of a list-based field one + * list, and RFC 7239 §4 explicitly blesses a proxy adding a field line + * of its own instead of extending the last one -- so reading only the + * first line would let a client's element outrank the trusted proxy's. + * + * @param lines NULL-terminated array of field line values + * @param[out] el array of #PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS + * elements to fill in + * @return number of elements parsed, 0 if the header is unusable + */ +static unsigned int +parse_forwarded_lines (const char *const *lines, + struct Element *el) +{ + unsigned int n = 0; + + for (unsigned int i = 0; NULL != lines[i]; i++) { - GNUNET_assert (NULL != inet_ntop (AF_INET, - ca, - buf, - sizeof (buf))); - return GNUNET_strdup (buf); + const char *p = lines[i]; + + while (true) + { + p = skip_ows (p); + if ('\0' == *p) + break; + if (',' == *p) + { + /* RFC 9110 §5.6.1.2: an empty list element is to be ignored. */ + p++; + continue; + } + if (PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS == n) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Forwarded chain longer than %u elements; ignoring it\n", + (unsigned int) PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS); + GNUNET_break_op (0); + return 0; + } + if (! parse_forwarded_element (&p, + &el[n])) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Malformed Forwarded header; ignoring it\n"); + return 0; + } + n++; + if (',' == *p) + p++; + } } - GNUNET_assert (sizeof (struct in6_addr) == ca_len); - GNUNET_assert (NULL != inet_ntop (AF_INET6, - ca, - buf, - sizeof (buf))); - /* RFC 7239 §6: an IPv6 identifier is bracketed, and the brackets - force the whole thing to be a quoted-string. */ - GNUNET_asprintf (&ret, - "\"[%s]\"", - buf); - return ret; + return n; +} + + +/** + * Parse every `X-Forwarded-For` field line of @a lines into @a el, in + * order. + * + * The de-facto header has no grammar beyond "comma-separated + * addresses"; an element that is not a bare address has no binary form + * and is recorded as naming none. + * + * @param lines NULL-terminated array of field line values + * @param[out] el array of #PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS + * elements to fill in + * @return number of elements parsed, 0 if the header is unusable + */ +static unsigned int +parse_xff_lines (const char *const *lines, + struct Element *el) +{ + unsigned int n = 0; + + for (unsigned int i = 0; NULL != lines[i]; i++) + { + const char *p = lines[i]; + + while (true) + { + const char *comma; + struct Element *e; + size_t len; + + p = skip_ows (p); + comma = strchr (p, + ','); + len = (NULL != comma) + ? (size_t) (comma - p) + : strlen (p); + while ( (len > 0) && + ( (' ' == p[len - 1]) || + ('\t' == p[len - 1]) ) ) + len--; + if (0 == len) + { + if (NULL == comma) + break; + p = comma + 1; + continue; + } + if (PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS == n) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "X-Forwarded-For chain longer than %u elements;" + " ignoring it\n", + (unsigned int) PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS); + GNUNET_break_op (0); + return 0; + } + e = &el[n++]; + memset (e, + 0, + sizeof (*e)); + if (len < sizeof (e->node)) + { + struct in_addr a4; + struct in6_addr a6; + + memcpy (e->node, + p, + len); + e->node[len] = '\0'; + if (1 == inet_pton (AF_INET, + e->node, + &a4)) + { + memcpy (e->addr, + &a4, + sizeof (a4)); + e->addr_len = sizeof (a4); + } + else if (1 == inet_pton (AF_INET6, + e->node, + &a6)) + { + pack_v6 (&a6, + e->addr, + &e->addr_len); + } + } + if (0 == e->addr_len) + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "X-Forwarded-For element `%.*s' is not a bare address\n", + (int) len, + p); + if (NULL == comma) + break; + p = comma + 1; + } + } + return n; } @@ -656,99 +916,422 @@ PAIVANA_HTTPD_is_trusted_proxy (const void *ca, } -bool -PAIVANA_HTTPD_parse_forwarded_for (const char *xff, - void **ca, - size_t *ca_len) +/** + * Walk the chain @a el from the right and return the index of the + * element that speaks for the client. + * + * We arrive here already standing on the peer we accepted from, which + * is trusted because `-f` says something in front of us is. Each step + * leftwards is permitted only by the node we are stepping over: that + * node wrote the element to its left, so unless *it* is one of ours, + * that element is hearsay. The first node we may not step over is + * therefore as far back as the chain can be believed, and is the + * client. + * + * @param el chain, leftmost element first + * @param n number of elements in @a el, at least 1 + * @return index into @a el + */ +static unsigned int +select_element (const struct Element *el, + unsigned int n) { - unsigned int n; + for (unsigned int i = n; i > 1; i--) + { + const struct Element *e = &el[i - 1]; + + if (0 == e->addr_len) + return i - 1; /* nothing further left is reachable */ + if (! PAIVANA_HTTPD_is_trusted_proxy (e->addr, + e->addr_len)) + return i - 1; /* the client */ + } + /* Every hop is a proxy we trust; the leftmost is all we have. */ + return 0; +} + + +/** + * Copy the leftmost element of the first field line of @a lines into + * @a out, with whitespace stripped. + * + * The de-facto `X-Forwarded-*` headers other than `-For` are + * single-valued in practice, but a chain of proxies may still have + * turned one into a list; its leftmost element is the one describing + * the client's own request. + * + * @param lines NULL-terminated array of field line values, or NULL + * @param[out] out buffer for the value + * @param out_size number of bytes in @a out + * @return true if a non-empty value was found + */ +static bool +first_value (const char *const *lines, + char *out, + size_t out_size) +{ + const char *p; + const char *comma; size_t len; - const char *tok; - *ca = NULL; - *ca_len = 0; - n = count_elements (xff); - if (! PH_have_trusted_proxies) + if (! have_lines (lines)) + return false; + p = skip_ows (lines[0]); + comma = strchr (p, + ','); + len = (NULL != comma) + ? (size_t) (comma - p) + : strlen (p); + while ( (len > 0) && + ( (' ' == p[len - 1]) || + ('\t' == p[len - 1]) ) ) + len--; + if ( (0 == len) || + (len >= out_size) ) + return false; + memcpy (out, + p, + len); + out[len] = '\0'; + return true; +} + + +void +PAIVANA_HTTPD_client_clear (struct PAIVANA_HTTPD_Client *cl) +{ + GNUNET_free (cl->ca); + GNUNET_free (cl->proto); + GNUNET_free (cl->host); + memset (cl, + 0, + sizeof (*cl)); +} + + +bool +PAIVANA_HTTPD_resolve_forwarding (const struct PAIVANA_HTTPD_Forwarding *fi, + struct PAIVANA_HTTPD_Client *cl) +{ + struct Element el[PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS]; + unsigned int n = 0; + bool from_forwarded = false; + + memset (cl, + 0, + sizeof (*cl)); + if (fi->respect_forwarded) { - /* 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) + /* RFC 7239 is the standardized form and says more than the + de-facto headers do, so it wins where both are present -- and + the reverse-proxy configurations we ship set it. A proxy that + emits both should agree with itself; if it does not, we would + rather be predictable than clever. */ + if (have_lines (fi->forwarded)) { - GNUNET_break_op (0); - return false; + if (have_lines (fi->xff)) + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Both Forwarded and X-Forwarded-For present; using" + " Forwarded\n"); + from_forwarded = true; + n = parse_forwarded_lines (fi->forwarded, + el); + } + else if (have_lines (fi->xff)) + { + n = parse_xff_lines (fi->xff, + el); } - 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--) + if (0 != n) { - tok = nth_element (xff, - i - 1, - &len); - if (NULL == tok) + unsigned int sel = select_element (el, + n); + + if (0 != el[sel].addr_len) { - GNUNET_break (0); - return false; + cl->ca = GNUNET_memdup (el[sel].addr, + el[sel].addr_len); + cl->ca_len = el[sel].addr_len; + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Client address is based on %s: `%s'\n", + from_forwarded + ? MHD_HTTP_HEADER_FORWARDED + : PH_HEADER_X_FORWARDED_FOR, + el[sel].node); } - if (! parse_element (tok, - len, - ca, - ca_len)) + else { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Unparseable X-Forwarded-For element; not looking further" - " left in the chain\n"); - return false; + /* RFC 7239 §6.3 `unknown`, an obfuscated identifier, a name we + cannot turn into bytes: legal, but nothing to bind a cookie + to. The peer is what we know for certain, and a header the + client controls must never be able to take that away. */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Forwarding chain names no client address at `%s';" + " falling back to the socket peer\n", + el[sel].node); } - if (! PAIVANA_HTTPD_is_trusted_proxy (*ca, - *ca_len)) + if (from_forwarded) { - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Client address is based on X-Forwarded-For: `%.*s'\n", - (int) len, - tok); - return true; + /* Same element, same author: the hop that reported this node + also reported the connection its predecessor made to it. */ + if (el[sel].have_proto) + cl->proto = GNUNET_strdup (el[sel].https + ? "https" + : "http"); + if ('\0' != el[sel].host[0]) + cl->host = GNUNET_strdup (el[sel].host); } - 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) + if ( (NULL == cl->ca) && + (NULL != fi->peer) && + (0 != fi->peer_len) ) { - GNUNET_break (0); - return false; + cl->ca = GNUNET_memdup (fi->peer, + fi->peer_len); + cl->ca_len = fi->peer_len; } - 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); + if (fi->respect_forwarded) + { + char buf[PH_HOST_MAX]; + + if ( (NULL == cl->proto) && + (first_value (fi->xfp, + buf, + sizeof (buf))) ) + { + if (0 == strcasecmp (buf, + "https")) + cl->proto = GNUNET_strdup ("https"); + else if (0 == strcasecmp (buf, + "http")) + cl->proto = GNUNET_strdup ("http"); + else + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Ignoring unusable %s value `%s'\n", + PH_HEADER_X_FORWARDED_PROTO, + buf); + } + if ( (NULL == cl->host) && + (first_value (fi->xfh, + buf, + sizeof (buf))) ) + { + if (valid_host (buf)) + cl->host = GNUNET_strdup (buf); + else + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Ignoring unusable %s value `%s'\n", + PH_HEADER_X_FORWARDED_HOST, + buf); + } + /* An authority carries at most one port. nginx's + `X-Forwarded-Host $http_host` includes one and its + `X-Forwarded-Port` repeats it; appending regardless would yield + "example.com:8443:8443", which is no authority at all. */ + if ( (NULL != cl->host) && + (! host_has_port (cl->host)) && + (first_value (fi->xfport, + buf, + sizeof (buf))) ) + { + if (! valid_port_text (buf)) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Ignoring unusable %s value `%s'\n", + PH_HEADER_X_FORWARDED_PORT, + buf); + } + else + { + unsigned int port = (unsigned int) strtoul (buf, + NULL, + 10); + bool https = ( (NULL != cl->proto) && + (0 == strcmp (cl->proto, + "https")) ); + + /* Re-rendered rather than echoed: "0443" is a valid spelling + of 443 and must not reach the URL as itself. The default + port for the scheme is left off, as RFC 3986 §6.2.3 asks. */ + if (port != (https ? 443U : 80U)) + { + char *hp; + + GNUNET_asprintf (&hp, + "%s:%u", + cl->host, + port); + GNUNET_free (cl->host); + cl->host = hp; + } + } + } + } + return (NULL != cl->ca); +} + + +char * +PAIVANA_HTTPD_forwarded_param (const char *fwd, + const char *name) +{ + struct Element el[PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS]; + const char *lines[] = { + fwd, + NULL + }; + + if (0 == parse_forwarded_lines (lines, + el)) + return NULL; + /* The leftmost element describes the connection the client itself + made, which is what `proto` and `host` are being asked about. + With a single proxy in front -- the ordinary case -- there is only + one element and the question does not arise. */ + if (0 == strcasecmp (name, + "proto")) + return el[0].have_proto + ? GNUNET_strdup (el[0].https + ? "https" + : "http") + : NULL; + if (0 == strcasecmp (name, + "host")) + return ('\0' != el[0].host[0]) + ? GNUNET_strdup (el[0].host) + : NULL; + GNUNET_break (0); /* no other parameter is validated here */ + return NULL; +} + + +char * +PAIVANA_HTTPD_forwarded_value (const char *v) +{ + struct GNUNET_Buffer buf = { 0 }; + bool token = ('\0' != v[0]); + + for (const char *p = v; '\0' != *p; p++) + if (! is_tchar (*p)) + { + token = false; + break; + } + if (token) + return GNUNET_strdup (v); + GNUNET_buffer_write (&buf, + "\"", + 1); + for (const char *p = v; '\0' != *p; p++) + { + if ( ('"' == *p) || + ('\\' == *p) ) + { + GNUNET_buffer_write (&buf, + "\\", + 1); + } + else if (! is_qdtext (*p)) + { + /* A control character has no quoted-pair either, so there is no + way to say this at all. */ + GNUNET_break_op (0); + GNUNET_buffer_clear (&buf); + return NULL; + } + GNUNET_buffer_write (&buf, + p, + 1); + } + GNUNET_buffer_write (&buf, + "\"", + 1); + return GNUNET_buffer_reap_str (&buf); +} + + +char * +PAIVANA_HTTPD_forwarded_node (const void *ca, + size_t ca_len) +{ + char buf[INET6_ADDRSTRLEN]; + char bracketed[INET6_ADDRSTRLEN + 2]; + + if ( (NULL == ca) || + (0 == ca_len) ) + { + /* RFC 7239 §6.3 provides exactly this for a hop whose predecessor + has no address we can name -- a Unix-domain peer, here. */ + return GNUNET_strdup ("unknown"); + } + if (sizeof (struct in_addr) == ca_len) + { + GNUNET_assert (NULL != inet_ntop (AF_INET, + ca, + buf, + sizeof (buf))); + return PAIVANA_HTTPD_forwarded_value (buf); + } + GNUNET_assert (sizeof (struct in6_addr) == ca_len); + GNUNET_assert (NULL != inet_ntop (AF_INET6, + ca, + buf, + sizeof (buf))); + /* RFC 7239 §6: an IPv6 identifier is bracketed, and the brackets + force the whole thing to be a quoted-string. */ + GNUNET_snprintf (bracketed, + sizeof (bracketed), + "[%s]", + buf); + return PAIVANA_HTTPD_forwarded_value (bracketed); +} + + +char * +PAIVANA_HTTPD_forwarded_for_chain (const char *fwd) +{ + struct Element el[PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS]; + struct GNUNET_Buffer buf = { 0 }; + const char *lines[] = { + fwd, + NULL + }; + unsigned int n; + + n = parse_forwarded_lines (lines, + el); + if (0 == n) + return NULL; + for (unsigned int i = 0; i < n; i++) + { + char node[INET6_ADDRSTRLEN]; + + /* All or nothing: X-Forwarded-For has no way to say "this hop had + no address", so an element whose `for` is RFC 7239's "unknown", + an obfuscated identifier, or anything else that is not an + address cannot be represented. Dropping it would silently + shift every position to its left, which is worse than declining + to translate the header at all. */ + if (0 == el[i].addr_len) + { + GNUNET_buffer_clear (&buf); + return NULL; + } + GNUNET_assert (NULL != + inet_ntop ( (sizeof (struct in_addr) == el[i].addr_len) + ? AF_INET + : AF_INET6, + el[i].addr, + node, + sizeof (node))); + if (0 != i) + GNUNET_buffer_write_str (&buf, + ", "); + GNUNET_buffer_write_str (&buf, + node); + } + return GNUNET_buffer_reap_str (&buf); } @@ -756,20 +1339,19 @@ PAIVANA_HTTPD_parse_forwarded_for (const char *xff, * 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 + * @param[out] addr buffer of at least `sizeof (struct in6_addr)` bytes + * @param[out] addr_len set to the number of bytes written, 0 for a + * peer that has no IP address */ -static bool +static void socket_address (struct MHD_Connection *connection, - void **ca, - size_t *ca_len) + unsigned char *addr, + size_t *addr_len) { const union MHD_ConnectionInfo *ci; const struct sockaddr *sa; - *ca = NULL; - *ca_len = 0; + *addr_len = 0; ci = MHD_get_connection_info (connection, MHD_CONNECTION_INFO_CLIENT_ADDRESS); GNUNET_assert (NULL != ci); @@ -777,106 +1359,270 @@ socket_address (struct MHD_Connection *connection, switch (sa->sa_family) { case AF_INET: - *ca = GNUNET_memdup (&((const struct sockaddr_in *) sa)->sin_addr, - sizeof (struct in_addr)); - *ca_len = sizeof (struct in_addr); - return true; + memcpy (addr, + &((const struct sockaddr_in *) sa)->sin_addr, + sizeof (struct in_addr)); + *addr_len = sizeof (struct in_addr); + return; case AF_INET6: /* A dual-stack listener hands us ::ffff:a.b.c.d for an IPv4 - peer; store_v6() folds that back to the IPv4 form. */ - store_v6 (&((const struct sockaddr_in6 *) sa)->sin6_addr, - ca, - ca_len); - return true; + peer; pack_v6() folds that back to the IPv4 form. */ + pack_v6 (&((const struct sockaddr_in6 *) sa)->sin6_addr, + addr, + addr_len); + return; default: /* AF_UNIX: no address exists. */ - return false; + return; } } +/** + * The field lines of the forwarding headers of one request, as + * collected from MHD. + */ +struct HeaderLines +{ + + /** + * `Forwarded` field lines, NULL-terminated once collected. + */ + const char **forwarded; + + /** + * `X-Forwarded-For` field lines. + */ + const char **xff; + + /** + * `X-Forwarded-Proto` field lines. + */ + const char **xfp; + + /** + * `X-Forwarded-Host` field lines. + */ + const char **xfh; + + /** + * `X-Forwarded-Port` field lines. + */ + const char **xfport; + + /** + * Number of entries in @e forwarded. + */ + unsigned int forwarded_len; + + /** + * Number of entries in @e xff. + */ + unsigned int xff_len; + + /** + * Number of entries in @e xfp. + */ + unsigned int xfp_len; + + /** + * Number of entries in @e xfh. + */ + unsigned int xfh_len; + + /** + * Number of entries in @e xfport. + */ + unsigned int xfport_len; +}; + + +/** + * Collect one header field line into the arrays of @a cls. + * + * @param cls a `struct HeaderLines *` + * @param kind header kind, always #MHD_HEADER_KIND here + * @param key field name + * @param value field line value + * @return #MHD_YES to keep iterating + */ +static enum MHD_Result +collect_header (void *cls, + enum MHD_ValueKind kind, + const char *key, + const char *value) +{ + struct HeaderLines *hl = cls; + + (void) kind; + if ( (NULL == key) || + (NULL == value) ) + return MHD_YES; + if (0 == strcasecmp (key, + MHD_HTTP_HEADER_FORWARDED)) + GNUNET_array_append (hl->forwarded, + hl->forwarded_len, + value); + else if (0 == strcasecmp (key, + PH_HEADER_X_FORWARDED_FOR)) + GNUNET_array_append (hl->xff, + hl->xff_len, + value); + else if (0 == strcasecmp (key, + PH_HEADER_X_FORWARDED_PROTO)) + GNUNET_array_append (hl->xfp, + hl->xfp_len, + value); + else if (0 == strcasecmp (key, + PH_HEADER_X_FORWARDED_HOST)) + GNUNET_array_append (hl->xfh, + hl->xfh_len, + value); + else if (0 == strcasecmp (key, + PH_HEADER_X_FORWARDED_PORT)) + GNUNET_array_append (hl->xfport, + hl->xfport_len, + value); + return MHD_YES; +} + + +/** + * Collect the forwarding header field lines of @a connection. + * + * MHD stores repeated field lines separately and + * MHD_lookup_connection_value() returns only one of them, so iterating + * is the only way to see them all -- and RFC 9110 §5.3 says all of + * them together are the field value. The values are borrowed from MHD + * and live as long as the connection; only the arrays are ours. + * + * @param connection connection to read from + * @param[out] hl where to collect; release with #free_headers() + */ +static void +collect_headers (struct MHD_Connection *connection, + struct HeaderLines *hl) +{ + memset (hl, + 0, + sizeof (*hl)); + MHD_get_connection_values (connection, + MHD_HEADER_KIND, + &collect_header, + hl); + if (0 != hl->forwarded_len) + GNUNET_array_append (hl->forwarded, + hl->forwarded_len, + NULL); + if (0 != hl->xff_len) + GNUNET_array_append (hl->xff, + hl->xff_len, + NULL); + if (0 != hl->xfp_len) + GNUNET_array_append (hl->xfp, + hl->xfp_len, + NULL); + if (0 != hl->xfh_len) + GNUNET_array_append (hl->xfh, + hl->xfh_len, + NULL); + if (0 != hl->xfport_len) + GNUNET_array_append (hl->xfport, + hl->xfport_len, + NULL); +} + + +/** + * Release the arrays of @a hl. + * + * @param[in,out] hl what #collect_headers() filled in + */ +static void +free_headers (struct HeaderLines *hl) +{ + GNUNET_array_grow (hl->forwarded, + hl->forwarded_len, + 0); + GNUNET_array_grow (hl->xff, + hl->xff_len, + 0); + GNUNET_array_grow (hl->xfp, + hl->xfp_len, + 0); + GNUNET_array_grow (hl->xfh, + hl->xfh_len, + 0); + GNUNET_array_grow (hl->xfport, + hl->xfport_len, + 0); +} + + +/** + * Run the forwarding walk for @a connection. + * + * @param connection connection to resolve + * @param[out] cl what we concluded; to be released with + * #PAIVANA_HTTPD_client_clear() + * @return true if a client address was determined + */ +static bool +resolve_connection (struct MHD_Connection *connection, + struct PAIVANA_HTTPD_Client *cl) +{ + struct HeaderLines hl; + struct PAIVANA_HTTPD_Forwarding fi = { 0 }; + unsigned char peer[sizeof (struct in6_addr)]; + size_t peer_len; + bool ret; + + socket_address (connection, + peer, + &peer_len); + collect_headers (connection, + &hl); + fi.peer = (0 != peer_len) + ? peer + : NULL; + fi.peer_len = peer_len; + fi.respect_forwarded = (0 != PH_respect_forwarded_headers); + fi.forwarded = hl.forwarded; + fi.xff = hl.xff; + fi.xfp = hl.xfp; + fi.xfh = hl.xfh; + fi.xfport = hl.xfport; + ret = PAIVANA_HTTPD_resolve_forwarding (&fi, + cl); + free_headers (&hl); + return ret; +} + + 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) + struct PAIVANA_HTTPD_Client cl; + bool ret; + + ret = resolve_connection (connection, + &cl); + *ca = cl.ca; + *ca_len = cl.ca_len; + cl.ca = NULL; + cl.ca_len = 0; + PAIVANA_HTTPD_client_clear (&cl); + if (! ret) { - 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)) ) - { - const char *fwd; - - /* RFC 7239 is the standardized form and says more than the - de-facto header does, so it wins where both are present. A - proxy that emits both should agree with itself; if it does - not, we would rather be predictable than clever. */ - fwd = MHD_lookup_connection_value (connection, - MHD_HEADER_KIND, - MHD_HTTP_HEADER_FORWARDED); - xff = MHD_lookup_connection_value (connection, - MHD_HEADER_KIND, - PH_HEADER_X_FORWARDED_FOR); - if ( (NULL != fwd) && - (NULL != xff) ) - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Both Forwarded and X-Forwarded-For present; using" - " Forwarded\n"); - if (NULL != fwd) - { - GNUNET_free (*ca); - *ca = NULL; - *ca_len = 0; - return PAIVANA_HTTPD_parse_forwarded (fwd, - ca, - ca_len); - } - if (NULL != xff) - { - GNUNET_free (*ca); - *ca = NULL; - *ca_len = 0; - return PAIVANA_HTTPD_parse_forwarded_for (xff, - ca, - ca_len); - } - /* Neither 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. */ + /* Only a peer that has no address at all gets here, i.e. AF_UNIX + with no forwarding header to stand in for it. The shipped + packaging serves over a Unix socket and passes -f for exactly + this reason. */ GNUNET_break (0); - return false; } - return true; + return ret; } @@ -884,8 +1630,8 @@ bool PAIVANA_HTTPD_get_base_url (struct MHD_Connection *connection, struct GNUNET_Buffer *buf) { - const char *forwarded_host; - bool is_https; + struct PAIVANA_HTTPD_Client cl; + const char *host; GNUNET_buffer_clear (buf); if (NULL != PH_base_url) @@ -894,79 +1640,61 @@ PAIVANA_HTTPD_get_base_url (struct MHD_Connection *connection, PH_base_url); return true; } - /* TALER_mhd_is_https() reports https "either directly or via - proxy", i.e. it also believes an inbound X-Forwarded-Proto. That - is only a truthful answer when something trusted set that header; - otherwise any client could decide the scheme of the URLs we - generate for it. Unless we are behind a trusted proxy, go by the - transport: MHD reports a TLS session or it does not. */ - is_https = PH_respect_forwarded_headers - ? (GNUNET_YES == TALER_mhd_is_https (connection)) - : (NULL != MHD_get_connection_info (connection, - MHD_CONNECTION_INFO_PROTOCOL)); - if (is_https) + /* run() refuses to start without BASE_URL unless -f was given: with + no proxy in front of us the Host header is unverified client + input and reconstructing a URL from it is not something we may + do. So everything below is the behind-a-trusted-proxy case. */ + GNUNET_assert (0 != PH_respect_forwarded_headers); + (void) resolve_connection (connection, + &cl); + if (NULL != cl.proto) + { GNUNET_buffer_write_str (buf, - "https://"); - else + cl.proto); GNUNET_buffer_write_str (buf, - "http://"); - forwarded_host = PH_respect_forwarded_headers - ? MHD_lookup_connection_value (connection, - MHD_HEADER_KIND, - PH_HEADER_X_FORWARDED_HOST) - : NULL; - if (NULL != forwarded_host) + "://"); + } + else { - const char *forwarded_port; - + /* The proxy said nothing about the scheme, so go by the transport: + MHD reports a TLS session or it does not. (TALER_mhd_is_https() + would consult X-Forwarded-Proto again, without applying the + walk's verdict on whether it may be believed.) */ GNUNET_buffer_write_str (buf, - forwarded_host); - /* Only reached when PH_respect_forwarded_headers is set. */ - forwarded_port = MHD_lookup_connection_value (connection, - MHD_HEADER_KIND, - PH_HEADER_X_FORWARDED_PORT); - if ( (NULL != forwarded_port) && - (0 != strcmp (forwarded_port, - is_https ? "443" : "80") ) ) - { - unsigned int port; - char c; - - if ( (1 != - sscanf (forwarded_port, - "%u%c", - &port, - &c)) || - (0 == port) || - (65536 <= port) ) - { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Invalid X-Forwarded-Port value `%s'. Ignored\n", - forwarded_port); - } - else - { - GNUNET_buffer_write_str (buf, - ":"); - GNUNET_buffer_write_str (buf, - forwarded_port); - } - } + (NULL != + MHD_get_connection_info ( + connection, + MHD_CONNECTION_INFO_PROTOCOL)) + ? "https://" + : "http://"); } - else + host = cl.host; + if (NULL == host) { - const char *host; - + /* No `Forwarded` host and no X-Forwarded-Host: the proxy passed + the client's Host through, and -f says it vouches for it. */ host = MHD_lookup_connection_value (connection, MHD_HEADER_KIND, MHD_HTTP_HEADER_HOST); if (NULL == host) { - GNUNET_break (0); + /* RFC 9112 §3.2 requires a Host on every HTTP/1.1 request. */ + GNUNET_break_op (0); + PAIVANA_HTTPD_client_clear (&cl); + return false; + } + if (! valid_host (host)) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Refusing to build a base URL from Host `%s'\n", + host); + GNUNET_break_op (0); + PAIVANA_HTTPD_client_clear (&cl); return false; } - GNUNET_buffer_write_str (buf, - host); } + GNUNET_buffer_write_str (buf, + host); + PAIVANA_HTTPD_client_clear (&cl); return true; } diff --git a/src/backend/paivana-httpd_helper.h b/src/backend/paivana-httpd_helper.h @@ -32,78 +32,180 @@ /** - * Obtain the client address of @a connection + * Longest forwarding chain we are willing to look at. * - * The address is returned in binary form: 4 bytes (`struct in_addr`) - * for IPv4, 16 bytes (`struct in6_addr`) for IPv6. That - * representation is canonical and does not depend on where the - * address was taken from -- X-Forwarded-For or the socket -- which - * matters because it feeds the access-cookie MAC: two requests from - * the same host have to yield the same bytes or the cookie stops - * verifying. + * A real chain has single digits; MHD's default connection pool would + * otherwise let a client hand us some 2500 elements per request, and + * anything that touches each of them is work we do on the one thread + * that also drives all outbound traffic. A longer chain is treated + * as malformed, i.e. as if no forwarding header had been sent. + */ +#define PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS 32 + + +/** + * Everything about a request that the forwarding layer may look at. * - * @param connection HTTP client connection - * @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 no address could be determined - * (in particular for a non-IP socket, or for an X-Forwarded-For - * value that is not a bare IP address) + * All header members are arrays of the *field line* values MHD + * reported for that field name, in the order they arrived, terminated + * by a NULL entry (NULL for "field not present at all"). RFC 9110 + * §5.3 makes repeated field lines of a list-valued field one list, so + * every line has to be considered, not just the first. */ -bool -PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection, - void **ca, - size_t *ca_len); +struct PAIVANA_HTTPD_Forwarding +{ + + /** + * Address of the peer we accepted the connection from, in the binary + * form used throughout, or NULL for a peer that has none (AF_UNIX). + */ + const void *peer; + + /** + * Number of bytes in @e peer. + */ + size_t peer_len; + + /** + * Are we behind a trusted reverse proxy, i.e. was `-f` given? If + * not, none of the header members below is looked at at all. + */ + bool respect_forwarded; + + /** + * Field lines of the `Forwarded` header (RFC 7239). + */ + const char *const *forwarded; + + /** + * Field lines of the `X-Forwarded-For` header. + */ + const char *const *xff; + + /** + * Field lines of the `X-Forwarded-Proto` header. + */ + const char *const *xfp; + + /** + * Field lines of the `X-Forwarded-Host` header. + */ + const char *const *xfh; + + /** + * Field lines of the `X-Forwarded-Port` header. + */ + const char *const *xfport; +}; /** - * Extract the client address from an `X-Forwarded-For` header value. - * - * 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 selected element is not a - * bare IPv4 or IPv6 address + * What the forwarding layer concluded about the client. + */ +struct PAIVANA_HTTPD_Client +{ + + /** + * Client address in binary form: 4 bytes (`struct in_addr`) for + * IPv4, 16 bytes (`struct in6_addr`) for IPv6. NULL only if the + * peer had no address and no header supplied one. + */ + void *ca; + + /** + * Number of bytes in @e ca. + */ + size_t ca_len; + + /** + * Scheme the client itself used, "http" or "https", or NULL if + * nothing we trust said. + */ + char *proto; + + /** + * Authority the client itself addressed (host, optionally with a + * port), or NULL if nothing we trust said. + */ + char *host; +}; + + +/** + * Determine who the client is, and what it thinks it connected to. + * + * This is the one walk over the forwarding chain; both the address the + * access cookie is keyed on and the base URL we reconstruct come out + * of it, so that the two can never disagree about which hop was + * believed. + * + * Given `-f` we are behind a trusted reverse proxy, so the peer we + * accepted from is trusted implicitly and `TRUSTED_PROXIES` names the + * *additional* hops further out that may also speak for their + * predecessor. The chain is therefore walked from the right: we step + * leftwards over a node for as long as that node is one we trust, and + * the first node that is not is the client. Nothing to the left of an + * untrusted node is attributable to anyone, and nothing to the left of + * a node that names no address (RFC 7239 §6.3 `unknown`, an obfuscated + * identifier, a hostname) can be reached at all. + * + * The scheme and authority are taken from the same element the address + * was: that element was written by a hop we trust and describes the + * connection its predecessor -- the client -- made. Where the chain + * says nothing, the de-facto `X-Forwarded-Proto` / `-Host` / `-Port` + * headers are consulted instead, and they too only when `-f` says + * something in front of us is entitled to have set them. + * + * Anything the client controls that we cannot believe falls back to + * the peer: a malformed, empty or over-long chain never removes the + * one address we know for certain. + * + * @param fi what the request carried + * @param[out] cl what we concluded; to be released with + * #PAIVANA_HTTPD_client_clear() whatever the return value + * @return true if a client address was determined, false only if the + * peer has no address and no header supplied one */ bool -PAIVANA_HTTPD_parse_forwarded_for (const char *xff, - void **ca, - size_t *ca_len); +PAIVANA_HTTPD_resolve_forwarding (const struct PAIVANA_HTTPD_Forwarding *fi, + struct PAIVANA_HTTPD_Client *cl); /** - * Extract the client address from a `Forwarded` header value - * (RFC 7239). + * Release the members of @a cl and zero it. * - * The standardized spelling of #PAIVANA_HTTPD_parse_forwarded_for(), - * and selects an element by the same rule. Only the `for=` parameter - * of each element is of interest here; a node identifier that is not - * an address — the `unknown` of §6.3, an obfuscated `_secret`, or a - * hostname — is not one we can bind a cookie to and ends the walk. - * Ports are stripped: RFC 7239 §6 permits `for="[2001:db8::1]:443"`, - * and the port is not part of the host's identity. + * @param[in,out] cl result of #PAIVANA_HTTPD_resolve_forwarding() + */ +void +PAIVANA_HTTPD_client_clear (struct PAIVANA_HTTPD_Client *cl); + + +/** + * Obtain the client address of @a connection. * - * @param fwd value of the `Forwarded` header + * A thin adapter over #PAIVANA_HTTPD_resolve_forwarding(): it reads + * the socket peer and the forwarding field lines off @a connection and + * hands them to that function, which is where the policy lives. + * + * The address is returned in binary form: 4 bytes (`struct in_addr`) + * for IPv4, 16 bytes (`struct in6_addr`) for IPv6. That + * representation is canonical and does not depend on where the + * address was taken from -- a forwarding header or the socket -- which + * matters because it feeds the access-cookie MAC: two requests from + * the same host have to yield the same bytes or the cookie stops + * verifying. + * + * @param connection HTTP client connection * @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 no usable address could be found + * @return true on success, false if no address could be determined, + * which for a request that arrived over IP cannot happen: only a + * non-IP socket with nothing to fall back on gets here */ bool -PAIVANA_HTTPD_parse_forwarded (const char *fwd, - void **ca, - size_t *ca_len); +PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection, + void **ca, + size_t *ca_len); /** @@ -127,15 +229,27 @@ PAIVANA_HTTPD_forwarded_for_chain (const char *fwd); /** * Read a parameter of the leftmost element of a `Forwarded` header - * value, e.g. `proto` or `host`. + * value. Only `proto` and `host` are supported. * * The leftmost element describes the connection the client itself * made, which is what those two are being asked about. * + * The result is *validated*, not merely unquoted: `proto` is one of + * the two literals "http" and "https", and `host` is an RFC 3986 §3.2 + * authority. Neither can therefore contain a `;`, `,`, `"` or space, + * which is what makes it safe to splice into a header we build. A + * value that does not pass is reported as absent. + * + * Note that a `host` may still contain `:` and `[`/`]`, which are not + * `tchar` (RFC 9110 §5.6.2): pass it through + * #PAIVANA_HTTPD_forwarded_value() before emitting it inside a + * `Forwarded` element, though it may be used as-is as the value of an + * `X-Forwarded-Host`. + * * @param fwd value of the `Forwarded` header - * @param name parameter to look for - * @return the value with any quoting removed, or NULL if the header - * does not carry it; to be freed by the caller + * @param name parameter to look for, "proto" or "host" + * @return the value, or NULL if the header does not carry a usable + * one; to be freed by the caller */ char * PAIVANA_HTTPD_forwarded_param (const char *fwd, @@ -143,6 +257,24 @@ PAIVANA_HTTPD_forwarded_param (const char *fwd, /** + * Render @a v as an RFC 7239 §4 `value`, i.e. as a bare token where + * that is legal and as an escaped quoted-string otherwise. + * + * Splicing a value into a `Forwarded` element without this is how a + * `;` or `,` in it becomes a parameter or an element of its own, and + * an origin that trusts us reads what the client wrote as something we + * said. + * + * @param v value to render + * @return the token or quoted-string, or NULL if @a v contains bytes + * that no `Forwarded` value may carry (a control character other + * than HTAB); to be freed by the caller + */ +char * +PAIVANA_HTTPD_forwarded_value (const char *v); + + +/** * Render @a ca as an RFC 7239 `for=` node identifier, quoting and * bracketing an IPv6 address as §6 requires. * @@ -178,6 +310,13 @@ PAIVANA_HTTPD_is_trusted_proxy (const void *ca, * Determine the Base URL that the client made the HTTP request to. * The URL returned will be without the trailing '/'. * + * `BASE_URL` wins if it is configured. Otherwise the scheme and + * authority come from #PAIVANA_HTTPD_resolve_forwarding(), i.e. from + * the same element of the forwarding chain the client address came + * from, so that the string the cookie is keyed on and the address it + * is keyed on were reported by the same trusted hop. Where that says + * nothing we go by the transport and the `Host` header. + * * @param connection client connection used * @param[out] buf where to write the base URL; buffer will be cleared * and must be reaped by caller. diff --git a/src/tests/README b/src/tests/README @@ -220,9 +220,14 @@ Cross-cutting tests (run once): The client_address unit test ---------------------------- -`test_client_address.c` covers PAIVANA_HTTPD_parse_forwarded_for() -and, through it, the byte representation of the client address that -PAIVANA_HTTPD_get_client_address() hands to the cookie MAC. +`test_client_address.c` covers PAIVANA_HTTPD_resolve_forwarding(), +the single walk over the forwarding chain that decides both the client +address PAIVANA_HTTPD_get_client_address() hands to the cookie MAC and +the scheme and authority PAIVANA_HTTPD_get_base_url() rebuilds the +website string from. That function is deliberately pure — it takes +the socket peer, the ordered field lines of each forwarding header and +the trust configuration, and nothing else — so the whole policy is +reachable without an MHD connection; the MHD half is a thin adapter. The access cookie is an HMAC over (expiration, website, client address). A host therefore has to produce the *same bytes* however @@ -239,34 +244,56 @@ 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 further group covers the RFC 7239 `Forwarded` parser: quoting, -bracketed and unbracketed IPv6, port suffixes, parameters in any -order, commas inside quoted strings (which are data, not element -boundaries), node identifiers that are not addresses ("unknown", -obfuscated "_secret", hostnames), the same right-to-left walk under a -trusted-proxy policy, and the rendering back out — as a `for=` -identifier and as an X-Forwarded-For chain. +A table-driven group then covers the walk itself. `-f` means "we are +behind a trusted reverse proxy", so the socket peer is trusted +implicitly and TRUSTED_PROXIES / TRUSTED_PROXIES6 name the *additional* +hops further out; the walk steps leftwards over a node only while that +node is trusted and stops at the first one that is not. The rows +cover: + + - without `-f`, the socket peer wins even with every forwarding + header present, + - a single proxy and a single element, in both spellings, + - two and three proxies with only some of them listed, and an + untrusted node in the middle, which stops the walk where it should, + - a chain of nothing but trusted hops, where the leftmost is all + there is, + - IPv4, bracketed IPv6, IPv6 with a port, RFC 7239 §6.3 "unknown" + and an obfuscated identifier, + - repeated field lines of one header and a field line that is itself + a list (RFC 9110 §5.3), + - quoted strings with escapes, and an unterminated one, which used + to be read past the end of the header, + - malformed, empty and whitespace-only headers, all of which fall + back to the socket peer rather than losing it, + - `Forwarded` winning where both headers are present, + - a chain of 2500 elements, which is refused outright rather than + walked: every element used to be located by rescanning the header + from byte 0. + +A second table covers what the base URL is built from: `proto=` and +`host=` taken from the same element the address came from, the +X-Forwarded-Proto/-Host/-Port fallbacks, an X-Forwarded-Host that +already carries a port together with an X-Forwarded-Port (which must +not yield "example.com:8443:8443"), ports re-rendered rather than +echoed, and hosts and schemes that are refused because they are not +one. + +A further group covers the rendering back out — a `for=` identifier, +an X-Forwarded-For chain, and RFC 7239 §4 values, where a parameter +that would otherwise splice a second forwarded-element into a header +we build is either quoted or reported as absent. 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" +about spaces, 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 list terminator, and an address of the wrong family), and the one +way they return "usable, but not what was written" — a final entry +without its ';', or anything after the last ';', is dropped and the +prefix reported as success, which is why the loader counts separators. +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. diff --git a/src/tests/meson.build b/src/tests/meson.build @@ -116,5 +116,5 @@ test( 'PAIVANA_HTTPD': paivana_httpd_exe.full_path(), }, depends: test_deps, - timeout: 120, + timeout: 300, ) diff --git a/src/tests/test_client_address.c b/src/tests/test_client_address.c @@ -20,21 +20,29 @@ /** * @file test_client_address.c - * @brief tests that the client address paivana derives from - * X-Forwarded-For is the same binary value it derives from the - * connection's socket address + * @brief tests the forwarding walk: who the client is, and what it + * thinks it connected to * - * The access cookie is an HMAC over (expiration, website, client - * address). If a host yields one byte string when it reaches paivana - * through a proxy and a different one when it reaches paivana - * directly -- or when two proxies spell its address differently -- - * then the cookie it was issued stops verifying, with no error - * anywhere: it just looks unpaid again. So the representation has to - * be canonical, which is what this test pins down. + * Two things come out of #PAIVANA_HTTPD_resolve_forwarding(), and both + * are load-bearing: * - * The integration suite cannot cover this: it runs paivana with -n, - * where the cookie path (and hence the client address) is never - * reached at all. + * - The client address. The access cookie is an HMAC over + * (expiration, website, client address). If a host yields one byte + * string when it reaches paivana through a proxy and a different one + * when it reaches paivana directly -- or when two proxies spell its + * address differently -- then the cookie it was issued stops + * verifying, with no error anywhere: it just looks unpaid again. So + * the representation has to be canonical, which is what this test + * pins down. And if a client can *choose* that address, one paid + * cookie admits everybody, which is what the trust walk prevents. + * + * - The scheme and authority the base URL is rebuilt from, which the + * very same walk has to decide, or the cookie ends up keyed on a + * website string one hop reported and an address another one did. + * + * The integration suite cannot cover any of this: it runs paivana with + * -n, where the cookie path is never reached at all, and it has no way + * to present a forwarding chain from a chosen peer. */ #include "platform.h" #include <gnunet/gnunet_util_lib.h> @@ -70,6 +78,84 @@ static unsigned int failures; /** + * One row of the table-driven forwarding tests. + * + * The header members are the *field lines* of that header, in the + * order they arrived; leaving one empty means the header was absent. + */ +struct Case +{ + + /** + * What this row is about, for the log. + */ + const char *name; + + /** + * Socket peer, in presentation form, or NULL for a peer without an + * address (AF_UNIX). + */ + const char *peer; + + /** + * `TRUSTED_PROXIES`, or NULL. + */ + const char *v4; + + /** + * `TRUSTED_PROXIES6`, or NULL. + */ + const char *v6; + + /** + * `Forwarded` field lines. + */ + const char *fwd[4]; + + /** + * `X-Forwarded-For` field lines. + */ + const char *xff[4]; + + /** + * `X-Forwarded-Proto` field lines. + */ + const char *xfp[2]; + + /** + * `X-Forwarded-Host` field lines. + */ + const char *xfh[2]; + + /** + * `X-Forwarded-Port` field lines. + */ + const char *xfport[2]; + + /** + * Expected client address in presentation form, or NULL if none + * should be found. + */ + const char *want_ca; + + /** + * Expected scheme, or NULL for "nothing we trust said". + */ + const char *want_proto; + + /** + * Expected authority, or NULL for "nothing we trust said". + */ + const char *want_host; + + /** + * Was `-f` given? + */ + bool respect; +}; + + +/** * Print @a len bytes of @a p as hex into @a out. * * @param p bytes to render @@ -92,10 +178,9 @@ tohex (const void *p, /** - * The binary address the socket branch of - * #PAIVANA_HTTPD_get_client_address() would produce for a peer whose - * address is @a literal. Mirrors that branch rather than calling it, - * as calling it would need a live MHD connection. + * The binary address the socket branch of the resolver produces for a + * peer whose address is @a literal. Mirrors that branch rather than + * calling it, as calling it would need a live MHD connection. * * @param literal address of the peer, in presentation form * @param[out] ca where to write the allocated address @@ -181,6 +266,201 @@ cookie_survives (const void *mint_ca, /** + * 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; + } +} + + +/** + * Resolve @a c and check the outcome against what it expects. + * + * @param c case to run + */ +static void +run_case (const struct Case *c) +{ + struct PAIVANA_HTTPD_Forwarding fi = { 0 }; + struct PAIVANA_HTTPD_Client cl; + void *peer = NULL; + void *want = NULL; + size_t peer_len = 0; + size_t want_len = 0; + bool ok = true; + + set_policy (c->v4, + c->v6); + PH_respect_forwarded_headers = c->respect ? 1 : 0; + if (NULL != c->peer) + socket_address (c->peer, + &peer, + &peer_len); + fi.peer = peer; + fi.peer_len = peer_len; + fi.respect_forwarded = c->respect; + fi.forwarded = c->fwd; + fi.xff = c->xff; + fi.xfp = c->xfp; + fi.xfh = c->xfh; + fi.xfport = c->xfport; + if (PAIVANA_HTTPD_resolve_forwarding (&fi, + &cl) != (NULL != c->want_ca)) + ok = false; + if (NULL != c->want_ca) + { + socket_address (c->want_ca, + &want, + &want_len); + if ( (want_len != cl.ca_len) || + (NULL == cl.ca) || + (0 != memcmp (want, + cl.ca, + want_len)) ) + ok = false; + /* The identity has to be the same bytes however it was learned, + or a cookie minted on one path stops verifying on the next. */ + else if (! cookie_survives (cl.ca, + cl.ca_len, + want, + want_len)) + ok = false; + } + else if (NULL != cl.ca) + { + ok = false; + } + if ( ( (NULL == cl.proto) != (NULL == c->want_proto) ) || + ( (NULL != cl.proto) && + (0 != strcmp (cl.proto, + c->want_proto)) ) ) + ok = false; + if ( ( (NULL == cl.host) != (NULL == c->want_host) ) || + ( (NULL != cl.host) && + (0 != strcmp (cl.host, + c->want_host)) ) ) + ok = false; + if (! ok) + { + char ghex[2 * sizeof (struct in6_addr) + 1]; + char ehex[2 * sizeof (struct in6_addr) + 1]; + + tohex (cl.ca, + cl.ca_len, + ghex); + tohex (want, + want_len, + ehex); + fprintf (stderr, + "FAIL: %s\n" + " address %s, want %s (%s)\n" + " proto %s, want %s\n" + " host %s, want %s\n", + c->name, + ghex, + ehex, + (NULL != c->want_ca) ? c->want_ca : "(none)", + (NULL != cl.proto) ? cl.proto : "(none)", + (NULL != c->want_proto) ? c->want_proto : "(none)", + (NULL != cl.host) ? cl.host : "(none)", + (NULL != c->want_host) ? c->want_host : "(none)"); + failures++; + } + else + { + fprintf (stderr, + " ok: %s\n", + c->name); + } + PAIVANA_HTTPD_client_clear (&cl); + GNUNET_free (peer); + GNUNET_free (want); + set_policy (NULL, + NULL); +} + + +/** + * Run every case in the NULL-name-terminated table @a tab. + * + * @param tab cases to run + */ +static void +run_cases (const struct Case *tab) +{ + for (unsigned int i = 0; NULL != tab[i].name; i++) + run_case (&tab[i]); +} + + +/** + * Resolve @a xff as the only `X-Forwarded-For` field line of a request + * from a peer with no address of its own, so that whatever comes out + * came out of the header. + * + * @param xff header value + * @param[out] ca where to write the allocated address + * @param[out] ca_len set to the number of bytes in @a ca + * @return true if an address was found + */ +static bool +resolve_xff (const char *xff, + void **ca, + size_t *ca_len) +{ + struct PAIVANA_HTTPD_Forwarding fi = { 0 }; + struct PAIVANA_HTTPD_Client cl; + const char *lines[] = { + xff, + NULL + }; + bool ret; + + PH_respect_forwarded_headers = 1; + fi.respect_forwarded = true; + fi.xff = lines; + ret = PAIVANA_HTTPD_resolve_forwarding (&fi, + &cl); + *ca = cl.ca; + *ca_len = cl.ca_len; + cl.ca = NULL; + cl.ca_len = 0; + PAIVANA_HTTPD_client_clear (&cl); + return ret; +} + + +/** * Check that @a xff yields exactly the address a direct connection * from @a peer would, and that a cookie issued on one path is * accepted on the other. @@ -202,9 +482,9 @@ same_host (const char *xff, socket_address (peer, &sa, &sa_len); - if (! PAIVANA_HTTPD_parse_forwarded_for (xff, - &fa, - &fa_len)) + if (! resolve_xff (xff, + &fa, + &fa_len)) { fprintf (stderr, "FAIL: X-Forwarded-For `%s' rejected, want the address of %s\n", @@ -258,8 +538,10 @@ same_host (const char *xff, /** - * Check that @a xff is refused: it is not a bare IP address, so there - * is no canonical form for it and it must not become an identity. + * Check that @a xff names no address: it is not a bare IP address, so + * there is no canonical form for it and it must not become an + * identity. With no socket address to fall back on either, nothing + * comes out at all. * * @param xff `X-Forwarded-For` value to reject */ @@ -269,9 +551,9 @@ refused (const char *xff) void *ca; size_t ca_len; - if (PAIVANA_HTTPD_parse_forwarded_for (xff, - &ca, - &ca_len)) + if (resolve_xff (xff, + &ca, + &ca_len)) { char hex[2 * sizeof (struct in6_addr) + 1]; @@ -319,12 +601,12 @@ same_identity (const char *a, size_t ca_len; size_t cb_len; - GNUNET_assert (PAIVANA_HTTPD_parse_forwarded_for (a, - &ca, - &ca_len)); - GNUNET_assert (PAIVANA_HTTPD_parse_forwarded_for (b, - &cb, - &cb_len)); + GNUNET_assert (resolve_xff (a, + &ca, + &ca_len)); + GNUNET_assert (resolve_xff (b, + &cb, + &cb_len)); if ( (ca_len != cb_len) || (0 != memcmp (ca, cb, @@ -353,109 +635,6 @@ 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 @@ -495,125 +674,69 @@ trusted_is (const char *addr, /** - * Check that @a fwd, walked under the policy currently installed, - * names @a want as the client. + * Check that parameter @a name of @a fwd reads as @a want (NULL for + * "not present, or not usable"). * * @param fwd `Forwarded` value - * @param want expected client address in presentation form + * @param name parameter to read + * @param want expected value, or NULL */ static void -fwd_client_is (const char *fwd, - const char *want) +fwd_param_is (const char *fwd, + const char *name, + 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 (fwd, - &got, - &got_len)) - { - fprintf (stderr, - "FAIL: Forwarded `%s' rejected, want client %s\n", - fwd, - 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 *got; - tohex (got, - got_len, - ghex); + got = PAIVANA_HTTPD_forwarded_param (fwd, + name); + if ( ( (NULL == got) != (NULL == want) ) || + ( (NULL != got) && + (0 != strcmp (got, + want)) ) ) + { fprintf (stderr, - "FAIL: Forwarded `%s' gives %s, want %s\n", + "FAIL: `%s' %s=%s, want %s\n", fwd, - ghex, - want); + name, + (NULL != got) ? got : "(none)", + (NULL != want) ? want : "(none)"); failures++; } else { fprintf (stderr, - " ok: Forwarded `%s' -> %s\n", + " ok: `%s' %s=%s\n", fwd, - want); + name, + (NULL != got) ? got : "(none)"); } GNUNET_free (got); - GNUNET_free (exp); } /** - * Check that @a fwd yields no client address. + * Check that @a v renders as the RFC 7239 §4 value @a want (NULL if + * it cannot be rendered at all). * - * @param fwd `Forwarded` value to reject + * @param v value to render + * @param want expected rendering, or NULL */ static void -fwd_refused (const char *fwd) -{ - void *ca; - size_t ca_len; - - if (PAIVANA_HTTPD_parse_forwarded (fwd, - &ca, - &ca_len)) - { - char hex[2 * sizeof (struct in6_addr) + 1]; - - tohex (ca, - ca_len, - hex); - fprintf (stderr, - "FAIL: Forwarded `%s' accepted as %s, want refusal\n", - fwd, - hex); - failures++; - GNUNET_free (ca); - return; - } - fprintf (stderr, - " ok: Forwarded `%s' refused\n", - fwd); -} - - -/** - * Check that parameter @a name of @a fwd reads as @a want (NULL for - * "not present"). - * - * @param fwd `Forwarded` value - * @param name parameter to read - * @param want expected value, or NULL - */ -static void -fwd_param_is (const char *fwd, - const char *name, - const char *want) +value_is (const char *v, + const char *want) { char *got; - got = PAIVANA_HTTPD_forwarded_param (fwd, - name); + got = PAIVANA_HTTPD_forwarded_value (v); if ( ( (NULL == got) != (NULL == want) ) || ( (NULL != got) && (0 != strcmp (got, want)) ) ) { fprintf (stderr, - "FAIL: `%s' %s=%s, want %s\n", - fwd, - name, + "FAIL: value of `%s' is `%s', want `%s'\n", + v, (NULL != got) ? got : "(none)", (NULL != want) ? want : "(none)"); failures++; @@ -621,9 +744,8 @@ fwd_param_is (const char *fwd, else { fprintf (stderr, - " ok: `%s' %s=%s\n", - fwd, - name, + " ok: value of `%s' is `%s'\n", + v, (NULL != got) ? got : "(none)"); } GNUNET_free (got); @@ -712,6 +834,634 @@ chain_is (const char *fwd, /** + * Which hop the walk stops at, in every shape of chain we can be + * handed. The socket peer is trusted implicitly whenever -f is set -- + * that is what the flag means -- and TRUSTED_PROXIES names the + * additional hops further out that may speak for their predecessor. + */ +static void +test_walk (void) +{ + static const struct Case tab[] = { + { + .name = "without -f the socket peer wins, headers or not", + .peer = "203.0.113.7", + .respect = false, + .fwd = { "for=1.2.3.4;proto=https;host=evil.example.com" }, + .xff = { "5.6.7.8" }, + .xfp = { "https" }, + .xfh = { "evil.example.com" }, + .want_ca = "203.0.113.7" + }, + { + .name = "without -f not even a truthful header is read", + .peer = "198.51.100.9", + .respect = false, + .xff = { "203.0.113.7" }, + .want_ca = "198.51.100.9" + }, + { + .name = "-f, one proxy (the peer), one element: the client", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, one element, Forwarded spelling", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, no TRUSTED_PROXIES: only the peer's own report" + " counts, so a client cannot prepend itself an identity", + .peer = "10.0.0.1", + .respect = true, + .xff = { "1.2.3.4, 203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, two proxies, the outer one trusted: the walk steps" + " over it and stops at the client", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .xff = { "203.0.113.7, 10.0.0.2" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, three proxies, all trusted", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .xff = { "203.0.113.7, 10.0.0.2, 10.0.0.3" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, untrusted node in the middle: the walk stops there", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .xff = { "1.2.3.4, 203.0.113.7, 10.0.0.2" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, the peer is trusted implicitly even where" + " TRUSTED_PROXIES does not cover it", + .peer = "192.0.2.5", + .v4 = "10.0.0.0/8;", + .respect = true, + .xff = { "203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, every element is one of ours: the leftmost is all" + " we have", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .xff = { "10.0.0.9, 10.0.0.2" }, + .want_ca = "10.0.0.9" + }, + { + .name = "-f, IPv6 client, bracketed node", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=\"[2001:db8::1]\"" }, + .want_ca = "2001:db8::1" + }, + { + .name = "-f, IPv6 client with a port: the port is not part of" + " the identity", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=\"[2001:0db8:0000:0000:0000:0000:0000:0001]:47011\"" }, + .want_ca = "2001:db8::1" + }, + { + .name = "-f, IPv4 node with a port", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=\"203.0.113.7:8080\"" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, IPv6 trusted proxy, IPv6 policy", + .peer = "2001:db8::2", + .v6 = "2001:db8::/32;", + .respect = true, + .fwd = { "for=\"[2001:db9::1]\", for=\"[2001:db8::3]\"" }, + .want_ca = "2001:db9::1" + }, + { + .name = "-f, for=unknown (RFC 7239 §6.3): back to the peer", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=unknown" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, obfuscated node: back to the peer", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=_hidden" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, unknown to the left of a trusted proxy stops the" + " walk without losing the peer", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .fwd = { "for=203.0.113.7, for=unknown, for=10.0.0.2" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, element without a for= at all", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "proto=https;host=e.com" }, + .want_ca = "10.0.0.1", + .want_proto = "https", + .want_host = "e.com" + }, + /* RFC 9110 §5.3: repeated field lines of a list-based field are + one list. A proxy adding a line of its own instead of extending + the last one is explicitly permitted by RFC 7239 §4, and reading + only the first would let the client's element outrank it. */ + { + .name = "-f, two Forwarded field lines: both are the header", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=1.2.3.4", "for=203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, three X-Forwarded-For field lines", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .xff = { "1.2.3.4", "203.0.113.7", "10.0.0.2" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, a field line that is itself a list", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .fwd = { "for=1.2.3.4, for=203.0.113.7", "for=10.0.0.2" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, Forwarded wins where both headers are present", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7" }, + .xff = { "1.2.3.4" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, X-Forwarded-For is read when Forwarded is absent", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + /* A quoted-string is data: a ',' or ';' inside one does not end an + element or a parameter (RFC 7239 §4 via RFC 9110 §5.6.4). */ + { + .name = "-f, comma inside a quoted string is not a delimiter", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "host=\"a,b\";for=203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f, quoted-pair escapes are unescaped, not counted", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "by=\"x\\\"y;for=1.2.3.4\";for=203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + /* An unterminated quoted-string used to be read past the end of + the header. It is malformed, and malformed means the peer. */ + { + .name = "-f, unterminated quoted string: malformed, back to the" + " peer", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=\"unterminated" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, unterminated quoted string in a later element", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7, for=\"1.2.3.4" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, backslash at the very end of the header", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=\"1.2.3.4\\" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, empty Forwarded field line: back to the peer", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, whitespace-only field line", + .peer = "10.0.0.1", + .respect = true, + .fwd = { " \t " }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, whitespace-only X-Forwarded-For", + .peer = "10.0.0.1", + .respect = true, + .xff = { " " }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, a parameter without a value", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, a duplicate parameter (RFC 7239 §4 forbids it)", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7;for=1.2.3.4" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, garbage where a forwarded-pair should be", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=1.2.3.4 nonsense" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f, X-Forwarded-For element that is not an address", + .peer = "10.0.0.1", + .respect = true, + .xff = { "garbage" }, + .want_ca = "10.0.0.1" + }, + { + .name = "-f over a Unix socket: the header is all there is", + .peer = NULL, + .respect = true, + .fwd = { "for=203.0.113.7" }, + .want_ca = "203.0.113.7" + }, + { + .name = "-f over a Unix socket, unusable header: nothing at all", + .peer = NULL, + .respect = true, + .fwd = { "for=unknown" }, + .want_ca = NULL + }, + { + .name = "a dual-stack listener folds ::ffff:a.b.c.d to IPv4", + .peer = "::ffff:203.0.113.7", + .respect = false, + .want_ca = "203.0.113.7" + }, + { .name = NULL } + }; + + run_cases (tab); +} + + +/** + * Where the scheme and authority of the base URL come from. They are + * decided by the same walk as the address, so that the string the + * cookie is keyed on and the address it is keyed on were reported by + * the same hop. + */ +static void +test_base (void) +{ + static const struct Case tab[] = { + { + .name = "Forwarded proto=/host= are read, not only the" + " X-Forwarded-* spellings", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7;proto=https;host=example.com" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com" + }, + /* The element the Apache snippet in debian/ builds: an unquoted + authority (`:` is not a tchar, but that is what mod_headers + emits) and an unbracketed IPv6 node. */ + { + .name = "the shipped Apache element, with a port in the host", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7;proto=https;host=example.com:8443" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com:8443" + }, + { + .name = "the shipped Apache element, IPv6 client, unbracketed", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=2001:db8::1;proto=https;host=example.com" }, + .want_ca = "2001:db8::1", + .want_proto = "https", + .want_host = "example.com" + }, + { + .name = "proto= and host= come from the element the address" + " came from", + .peer = "10.0.0.1", + .v4 = "10.0.0.0/8;", + .respect = true, + .fwd = { "for=203.0.113.7;proto=https;host=example.com," + " for=10.0.0.2;proto=http;host=inner.example.net" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com" + }, + { + .name = "X-Forwarded-Proto/-Host fill in where Forwarded is" + " silent", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7" }, + .xfp = { "https" }, + .xfh = { "example.com" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com" + }, + { + .name = "without -f neither is believed", + .peer = "10.0.0.1", + .respect = false, + .xfp = { "https" }, + .xfh = { "evil.example.com" }, + .want_ca = "10.0.0.1" + }, + { + .name = "X-Forwarded-Port is appended when the host has none", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfp = { "https" }, + .xfh = { "example.com" }, + .xfport = { "8443" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com:8443" + }, + { + .name = "X-Forwarded-Host with a port plus X-Forwarded-Port does" + " not yield example.com:8443:8443", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfp = { "https" }, + .xfh = { "example.com:8443" }, + .xfport = { "8443" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com:8443" + }, + { + .name = "the default port for the scheme is left off", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfp = { "https" }, + .xfh = { "example.com" }, + .xfport = { "443" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com" + }, + { + .name = "a port is re-rendered, not echoed: 0443 is 443", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfp = { "https" }, + .xfh = { "example.com" }, + .xfport = { "0443" }, + .want_ca = "203.0.113.7", + .want_proto = "https", + .want_host = "example.com" + }, + { + .name = "a port that is not a number is ignored", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfh = { "example.com" }, + .xfport = { "+80" }, + .want_ca = "203.0.113.7", + .want_host = "example.com" + }, + { + .name = "an out-of-range port is ignored", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfh = { "example.com" }, + .xfport = { "70000" }, + .want_ca = "203.0.113.7", + .want_host = "example.com" + }, + { + .name = "an IPv6 X-Forwarded-Host keeps its brackets and takes" + " the port outside them", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfh = { "[2001:db8::1]" }, + .xfport = { "8080" }, + .want_ca = "203.0.113.7", + .want_host = "[2001:db8::1]:8080" + }, + { + .name = "a host that is not an authority is refused", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7;host=\"a, for=1.2.3.4\"" }, + .want_ca = "203.0.113.7" + }, + { + .name = "a host with a path is refused", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfh = { "example.com/evil" }, + .want_ca = "203.0.113.7" + }, + { + .name = "a scheme we cannot serve is refused", + .peer = "10.0.0.1", + .respect = true, + .fwd = { "for=203.0.113.7;proto=javascript" }, + .want_ca = "203.0.113.7" + }, + { + .name = "the leftmost element of a listed X-Forwarded-Proto is" + " the client's", + .peer = "10.0.0.1", + .respect = true, + .xff = { "203.0.113.7" }, + .xfp = { "https, http" }, + .want_ca = "203.0.113.7", + .want_proto = "https" + }, + { .name = NULL } + }; + + run_cases (tab); +} + + +/** + * A chain long enough to hurt. Every element used to be found by + * rescanning the header from byte 0, so a client could buy Θ(n·len) of + * the daemon's only thread for the price of one request. The walk is + * a single pass now, and declines outright to look at more elements + * than any real deployment has. + */ +static void +test_long_chain (void) +{ + struct PAIVANA_HTTPD_Forwarding fi = { 0 }; + struct PAIVANA_HTTPD_Client cl; + struct GNUNET_Buffer buf = { 0 }; + struct GNUNET_TIME_Absolute start; + struct GNUNET_TIME_Relative dur; + void *peer; + size_t peer_len; + char *big; + const char *lines[2] = { + NULL, + NULL + }; + unsigned int rounds = 200; + + socket_address ("10.0.0.1", + &peer, + &peer_len); + set_policy ("10.0.0.0/8;", + NULL); + PH_respect_forwarded_headers = 1; + fi.peer = peer; + fi.peer_len = peer_len; + fi.respect_forwarded = true; + + /* Exactly the cap: still a chain, still walked. */ + for (unsigned int i = 0; + i < PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS; + i++) + { + char elem[64]; + + if (0 != i) + GNUNET_buffer_write_str (&buf, + ", "); + GNUNET_snprintf (elem, + sizeof (elem), + "for=10.0.0.%u", + 1 + i); + GNUNET_buffer_write_str (&buf, + elem); + } + big = GNUNET_buffer_reap_str (&buf); + lines[0] = big; + fi.forwarded = lines; + (void) PAIVANA_HTTPD_resolve_forwarding (&fi, + &cl); + if ( (sizeof (struct in_addr) != cl.ca_len) || + (0 != memcmp (cl.ca, + "\x0a\x00\x00\x01", + 4)) ) + { + fprintf (stderr, + "FAIL: a chain of exactly %u trusted hops should resolve to" + " its leftmost element\n", + (unsigned int) PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS); + failures++; + } + else + { + fprintf (stderr, + " ok: a chain of exactly %u elements is walked\n", + (unsigned int) PAIVANA_HTTPD_MAX_FORWARDED_ELEMENTS); + } + PAIVANA_HTTPD_client_clear (&cl); + GNUNET_free (big); + + /* Everything an MHD connection pool can hold. The answer is the + peer -- we decline to look -- and saying so has to stay linear in + the length of the header. */ + for (unsigned int i = 0; i < 2500; i++) + { + if (0 != i) + GNUNET_buffer_write_str (&buf, + ", "); + GNUNET_buffer_write_str (&buf, + "for=10.0.0.1"); + } + big = GNUNET_buffer_reap_str (&buf); + lines[0] = big; + fi.forwarded = lines; + start = GNUNET_TIME_absolute_get (); + for (unsigned int r = 0; r < rounds; r++) + { + (void) PAIVANA_HTTPD_resolve_forwarding (&fi, + &cl); + if ( (peer_len != cl.ca_len) || + (NULL == cl.ca) || + (0 != memcmp (cl.ca, + peer, + peer_len)) ) + { + fprintf (stderr, + "FAIL: an over-long chain must fall back to the socket" + " peer\n"); + failures++; + PAIVANA_HTTPD_client_clear (&cl); + break; + } + PAIVANA_HTTPD_client_clear (&cl); + } + dur = GNUNET_TIME_absolute_get_duration (start); + fprintf (stderr, + " ok: 2500 elements over %u bytes, %u times, refused in %s\n", + (unsigned int) strlen (big), + rounds, + GNUNET_STRINGS_relative_time_to_string (dur, + GNUNET_YES)); + GNUNET_free (big); + GNUNET_free (peer); + set_policy (NULL, + NULL); +} + + +/** * 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 @@ -723,6 +1473,7 @@ test_policy_parser (void) { struct GNUNET_STRINGS_IPv4NetworkPolicy *p4; struct GNUNET_STRINGS_IPv6NetworkPolicy *p6; + unsigned int n; /* The trailing ';' is a terminator, not a separator: without it nothing parses at all. */ @@ -752,6 +1503,51 @@ test_policy_parser (void) fprintf (stderr, " ok: unterminated IPv6 policy refused\n"); } + /* A missing terminator on the LAST entry drops only that entry, and + the parser reports success for the prefix it did understand -- + which is why load_trusted_proxies() counts the ';' itself. */ + p4 = GNUNET_STRINGS_parse_ipv4_policy ("10.0.0.0/8;192.168.0.0/16"); + n = 0; + if (NULL != p4) + while (0 != p4[n].network.s_addr) + n++; + if ( (NULL == p4) || + (1 != n) ) + { + fprintf (stderr, + "FAIL: a partially terminated IPv4 policy no longer parses" + " to its first entry -- load_trusted_proxies() can stop" + " counting separators\n"); + failures++; + } + else + { + fprintf (stderr, + " ok: `10.0.0.0/8;192.168.0.0/16' silently yields 1 of 2" + " entries (rejected at startup)\n"); + } + GNUNET_free (p4); + /* Likewise trailing garbage after the last ';'. */ + p4 = GNUNET_STRINGS_parse_ipv4_policy ("10.0.0.0/8;garbage"); + n = 0; + if (NULL != p4) + while (0 != p4[n].network.s_addr) + n++; + if ( (NULL == p4) || + (1 != n) ) + { + fprintf (stderr, + "FAIL: trailing garbage in an IPv4 policy no longer slips" + " through the parser\n"); + failures++; + } + else + { + fprintf (stderr, + " ok: trailing garbage is silently dropped (rejected at" + " startup)\n"); + } + GNUNET_free (p4); /* 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. */ @@ -890,12 +1686,9 @@ main (int argc, a proxy in front of it reports the plain IPv4 address. */ same_host ("203.0.113.7", "::ffff:203.0.113.7"); - /* The first element is the client; the rest are proxies. */ - same_host ("203.0.113.7, 198.51.100.9, 198.51.100.10", - "203.0.113.7"); - same_host (" 203.0.113.7 ,198.51.100.9", + same_host (" 203.0.113.7 ", "203.0.113.7"); - same_host ("\t2001:db8::1\t,198.51.100.9", + same_host ("\t2001:db8::1\t", "2001:db8::1"); fprintf (stderr, @@ -930,40 +1723,16 @@ main (int argc, "2001:0db8:0000:0000:0000:0000:0000:0001:0002:0003:0004:0005:0006"); fprintf (stderr, - "-- RFC 7239 Forwarded --\n"); - fwd_client_is ("for=203.0.113.7", - "203.0.113.7"); - fwd_client_is ("For=203.0.113.7", /* case-insensitive */ - "203.0.113.7"); - fwd_client_is ("for=\"203.0.113.7\"", /* quoted */ - "203.0.113.7"); - fwd_client_is ("for=\"[2001:db8::1]\"", /* §6: bracketed */ - "2001:db8::1"); - fwd_client_is ("for=\"[2001:db8::1]:443\"", /* ...with a port */ - "2001:db8::1"); - fwd_client_is ("for=203.0.113.7:8080", /* v4 with a port */ - "203.0.113.7"); - fwd_client_is ("for=2001:db8::1", /* unbracketed v6 */ - "2001:db8::1"); - fwd_client_is ("proto=https;for=203.0.113.7;host=e.com", /* not first */ - "203.0.113.7"); - fwd_client_is ("for = 203.0.113.7", /* space around '=' */ - "203.0.113.7"); - /* No policy: the leftmost element is the client. */ - fwd_client_is ("for=203.0.113.7, for=198.51.100.9", - "203.0.113.7"); - /* A comma inside a quoted string is data, not an element boundary - (RFC 7239 §4 via RFC 9110 §5.6.4). */ - fwd_client_is ("host=\"a,b\";for=203.0.113.7", - "203.0.113.7"); - fwd_refused ("for=unknown"); /* §6.3: no address */ - fwd_refused ("for=_hidden"); /* §6.3: obfuscated */ - fwd_refused ("for=client.example.com"); /* a name, not an address */ - fwd_refused ("proto=https;host=e.com"); /* no `for' at all */ - fwd_refused (""); - fwd_refused ("for="); - fwd_refused ("for=\"[2001:db8::1\""); /* unclosed bracket */ - fwd_refused ("for=\"unterminated"); /* unclosed quote */ + "-- the walk --\n"); + test_walk (); + + fprintf (stderr, + "-- the scheme and authority the base URL is built from --\n"); + test_base (); + + fprintf (stderr, + "-- a chain long enough to hurt --\n"); + test_long_chain (); fprintf (stderr, "-- Forwarded parameters and node rendering --\n"); @@ -976,6 +1745,10 @@ main (int argc, fwd_param_is ("for=203.0.113.7;host=\"e.com:8443\"", "host", "e.com:8443"); + /* Unquoted, as mod_headers writes it. */ + fwd_param_is ("for=203.0.113.7;host=e.com:8443", + "host", + "e.com:8443"); fwd_param_is ("for=203.0.113.7", "proto", NULL); @@ -983,6 +1756,32 @@ main (int argc, fwd_param_is ("proto=https, proto=http", "proto", "https"); + /* What comes back is validated, so that a caller splicing it into a + header it builds cannot be made to emit a second + forwarded-element on the client's behalf. */ + fwd_param_is ("for=10.0.0.1;host=\"a, for=1.2.3.4\"", + "host", + NULL); + fwd_param_is ("for=10.0.0.1;proto=\"https;by=x\"", + "proto", + NULL); + fwd_param_is ("for=10.0.0.1;host=\"e.com/../evil\"", + "host", + NULL); + value_is ("203.0.113.7", + "203.0.113.7"); + value_is ("[2001:db8::1]", + "\"[2001:db8::1]\""); + value_is ("e.com:8443", + "\"e.com:8443\""); + value_is ("a\"b", + "\"a\\\"b\""); + value_is ("a\\b", + "\"a\\\\b\""); + value_is ("a, for=1.2.3.4", + "\"a, for=1.2.3.4\""); + value_is ("a\nb", + NULL); node_is ("203.0.113.7", "203.0.113.7"); node_is ("2001:db8::1", @@ -996,20 +1795,8 @@ main (int argc, /* Not every element is an address, so no chain can be rendered. */ chain_is ("for=unknown, for=10.0.0.1", NULL); - - fprintf (stderr, - "-- walking a Forwarded chain under a policy --\n"); - set_policy ("10.0.0.0/8;", NULL); - fwd_client_is ("for=203.0.113.7, for=10.0.0.1", - "203.0.113.7"); - fwd_client_is ("for=1.2.3.4, for=203.0.113.7, for=10.0.0.1", - "203.0.113.7"); - fwd_client_is ("for=unknown, for=203.0.113.7, for=10.0.0.1", - "203.0.113.7"); - fwd_client_is ("for=10.0.0.1, for=10.0.0.2", - "10.0.0.1"); - fwd_refused ("for=unknown, for=10.0.0.1"); - set_policy (NULL, NULL); + chain_is ("for=\"unterminated", + NULL); fprintf (stderr, "-- the GNUnet policy parsers behave as the loader assumes --\n"); @@ -1036,38 +1823,6 @@ main (int argc, 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; @@ -1075,12 +1830,12 @@ main (int argc, size_t al; size_t bl; - GNUNET_assert (PAIVANA_HTTPD_parse_forwarded_for ("203.0.113.7", - &a, - &al)); - GNUNET_assert (PAIVANA_HTTPD_parse_forwarded_for ("203.0.113.8", - &b, - &bl)); + GNUNET_assert (resolve_xff ("203.0.113.7", + &a, + &al)); + GNUNET_assert (resolve_xff ("203.0.113.8", + &b, + &bl)); if (cookie_survives (a, al, b, @@ -1099,6 +1854,8 @@ main (int argc, GNUNET_free (b); } + set_policy (NULL, + NULL); if (0 != failures) { fprintf (stderr, diff --git a/src/tests/test_reverse_proxy.sh b/src/tests/test_reverse_proxy.sh @@ -1176,6 +1176,12 @@ function test_forwarded_unix_rfc7239() { # 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. +# +# Worse than "nothing usable" is "some of it": a list whose LAST entry +# has no ';' loses that entry and reports success for the rest, so a +# single typo would leave every client behind the unlisted proxy +# sharing that proxy's address -- and one paid cookie. The loader +# counts the entries it got back against the ';' that went in. ###################################################################### # Start paivana with an extra config line and report whether it came @@ -1212,7 +1218,10 @@ function test_trusted_proxies_config() { 'TRUSTED_PROXIES = 0.0.0.0/0;' \ 'TRUSTED_PROXIES = ::1;' \ 'TRUSTED_PROXIES = garbage;' \ + 'TRUSTED_PROXIES = 10.0.0.0/8;192.168.0.0/16' \ + 'TRUSTED_PROXIES = 10.0.0.0/8;garbage' \ 'TRUSTED_PROXIES6 = 2001:db8::/32' \ + 'TRUSTED_PROXIES6 = 2001:db8::/32;fe80::/10' \ 'TRUSTED_PROXIES6 = ::/0;' \ 'TRUSTED_PROXIES6 = 2001:db8::/32; fe80::/10;' do