commit d4aa5afad1f6a03ad38d1727f147339581d7425c
parent 33e36b26edb3b057c833b93b92c094a6e37e7f88
Author: Christian Grothoff <christian@grothoff.org>
Date: Tue, 4 Aug 2026 17:10:24 +0200
normalize IP addresses before hashing for cookie
Diffstat:
6 files changed, 226 insertions(+), 53 deletions(-)
diff --git a/README b/README
@@ -154,7 +154,7 @@ Source layout
paivana-httpd_daemon.c MHD daemon startup
paivana_pd.c GNUnet project-data descriptor
src/include/platform.h GNUnet-style platform header (include first)
- src/tests/ Automated reverse-proxy tests
+ src/tests/ Automated reverse-proxy and unit tests
contrib/paywall.en.must.j2 Default Mustache paywall template source (Jinja2)
doc/prebuilt/ Git submodule: taler-docs (man pages)
diff --git a/src/backend/meson.build b/src/backend/meson.build
@@ -11,6 +11,10 @@ paivana_httpd_SOURCES = [
'paivana_pd.c',
]
+# Shared with unit tests in src/tests, which link individual
+# compilation units rather than the whole daemon.
+paivana_backend_inc = include_directories('.')
+
paivana_httpd_exe = executable(
'paivana-httpd',
paivana_httpd_SOURCES,
diff --git a/src/backend/paivana-httpd_helper.c b/src/backend/paivana-httpd_helper.c
@@ -23,10 +23,118 @@
* @file paivana-httpd_helper.c
* @brief helper functions
*/
+#include "platform.h"
#include "paivana-httpd.h"
#include "paivana-httpd_helper.h"
#include <taler/taler_mhd_lib.h>
+/**
+ * Store IPv6 address @a a6 in @a ca / @a ca_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
+ */
+static void
+store_v6 (const struct in6_addr *a6,
+ void **ca,
+ size_t *ca_len)
+{
+ if (IN6_IS_ADDR_V4MAPPED (a6))
+ {
+ *ca = GNUNET_memdup (&a6->s6_addr[12],
+ sizeof (struct in_addr));
+ *ca_len = sizeof (struct in_addr);
+ return;
+ }
+ *ca = GNUNET_memdup (a6,
+ sizeof (*a6));
+ *ca_len = sizeof (*a6);
+}
+
+
+bool
+PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
+ void **ca,
+ size_t *ca_len)
+{
+ const char *start = xff;
+ const char *end;
+ size_t len;
+ /* Long enough for any address inet_pton() accepts. */
+ char addr[INET6_ADDRSTRLEN];
+ struct in_addr a4;
+ struct in6_addr a6;
+
+ *ca = NULL;
+ *ca_len = 0;
+ /* Use first part before ',', getting rid of whitespace
+ at start or end of the substring. */
+ while ( (' ' == *start) ||
+ ('\t' == *start) )
+ start++;
+ end = strchr (start,
+ ',');
+ len = (NULL != end)
+ ? (size_t) (end - start)
+ : strlen (start);
+ while ( (len > 0) &&
+ ( (' ' == start[len - 1]) ||
+ ('\t' == start[len - 1]) ) )
+ len--;
+ if ( (0 == len) ||
+ (len >= sizeof (addr)) )
+ {
+ GNUNET_break_op (0);
+ return false;
+ }
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Client address is based on X-Forwarded-For: `%.*s'\n",
+ (int) len,
+ start);
+ memcpy (addr,
+ start,
+ len);
+ addr[len] = '\0';
+ /* What we return has to be the *same bytes* the socket branch would
+ have produced for this client, or the cookie MAC -- which covers
+ the client address -- silently stops matching as soon as a
+ request arrives without the header, or through a proxy that
+ spells the address differently ("::1" and "0:0:0:0:0:0:0:1" are
+ one host). Hence the address is parsed into its binary form
+ rather than carried around as text. A token that is not an
+ address at all (a port suffix, brackets, an RFC 7239 obfuscated
+ identifier, a hostname) has no such form and is refused. */
+ if (1 == inet_pton (AF_INET,
+ addr,
+ &a4))
+ {
+ *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;
+}
+
+
bool
PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
void **ca,
@@ -34,8 +142,6 @@ PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
{
const union MHD_ConnectionInfo *ci;
const struct sockaddr *sa;
- const void *ip;
- size_t ip_len;
*ca = NULL;
*ca_len = 0;
@@ -47,39 +153,9 @@ PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
MHD_HEADER_KIND,
"X-Forwarded-For");
if (NULL != xff)
- {
- const char *start = xff;
- const char *end;
- size_t len;
-
- /* Use first part before ',', getting rid of whitespace
- at start or end of the substring. */
- while ( (' ' == *start) ||
- ('\t' == *start) )
- start++;
- end = strchr (start,
- ',');
- len = (NULL != end)
- ? (size_t) (end - start)
- : strlen (start);
- while ( (len > 0) &&
- ( (' ' == start[len - 1]) ||
- ('\t' == start[len - 1]) ) )
- len--;
- if (0 == len)
- {
- GNUNET_break_op (0);
- return false;
- }
- GNUNET_log (GNUNET_ERROR_TYPE_INFO,
- "Client address is based on X-Forwarded-For: `%.*s'\n",
- (int) len,
- start);
- *ca = GNUNET_strndup (start,
- len);
- *ca_len = len;
- return true;
- }
+ return PAIVANA_HTTPD_parse_forwarded_for (xff,
+ ca,
+ ca_len);
/* No header present: fall through to the socket address. */
}
ci = MHD_get_connection_info (connection,
@@ -89,21 +165,23 @@ PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
switch (sa->sa_family)
{
case AF_INET:
- ip_len = sizeof (struct in_addr);
- ip = &((const struct sockaddr_in *) sa)->sin_addr;
- break;
+ *ca = GNUNET_memdup (&((const struct sockaddr_in *) sa)->sin_addr,
+ sizeof (struct in_addr));
+ *ca_len = sizeof (struct in_addr);
+ return true;
case AF_INET6:
- ip_len = sizeof (struct in6_addr);
- ip = &((const struct sockaddr_in6 *) sa)->sin6_addr;
- break;
+ /* 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;
default:
+ /* AF_UNIX in particular: there is no client address to bind the
+ cookie to. */
GNUNET_break (0);
return false;
}
- *ca = GNUNET_memdup (ip,
- ip_len);
- *ca_len = ip_len;
- return true;
}
diff --git a/src/backend/paivana-httpd_helper.h b/src/backend/paivana-httpd_helper.h
@@ -34,16 +34,47 @@
/**
* Obtain the client address of @a connection
*
+ * 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.
+ *
* @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
+ * @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)
*/
bool
PAIVANA_HTTPD_get_client_address (struct MHD_Connection *connection,
void **ca,
size_t *ca_len);
+
+/**
+ * Extract the client address from an `X-Forwarded-For` header value.
+ *
+ * Uses the first element of @a xff (the client as seen by the
+ * outermost proxy) and returns it in the same binary form
+ * #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.
+ *
+ * @param xff value of the `X-Forwarded-For` header
+ * @param[out] ca where to write the client address
+ * @param[out] ca_len number of bytes in @a ca
+ * @return true on success, false if the first element is not a bare
+ * IPv4 or IPv6 address
+ */
+bool
+PAIVANA_HTTPD_parse_forwarded_for (const char *xff,
+ void **ca,
+ size_t *ca_len);
+
/**
* Determine the Base URL that the client made the HTTP request to.
* The URL returned will be without the trailing '/'.
diff --git a/src/tests/README b/src/tests/README
@@ -1,10 +1,17 @@
-paivana reverse-proxy tests
-===========================
+paivana tests
+=============
-This directory contains an integration test suite for the reverse-proxy
-side of paivana-httpd. All tests run paivana-httpd with `-n`
-(paywall disabled) so no merchant backend is required: they only
-verify that the proxy correctly forwards HTTP requests and responses.
+This directory contains two test programs:
+
+ reverse_proxy an integration suite for the reverse-proxy side of
+ paivana-httpd, driven by test_reverse_proxy.sh
+ client_address a unit test for the client address the access
+ cookie is keyed on (test_client_address.c)
+
+The integration suite runs paivana-httpd with `-n` (paywall disabled)
+so no merchant backend is required: it only verifies that the proxy
+correctly forwards HTTP requests and responses. Everything below
+describes that suite except the "client_address" section at the end.
What gets built
---------------
@@ -157,6 +164,34 @@ Cross-cutting tests (run once):
there keeps reading, so the socket
never stays full.
+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.
+
+The access cookie is an HMAC over (expiration, website, client
+address). A host therefore has to produce the *same bytes* however
+paivana learns its address, or the cookie it was issued silently stops
+verifying and the visitor is asked to pay again. The test asserts:
+
+ - an X-Forwarded-For value and the socket address of the same host
+ yield identical bytes (including ::ffff:a.b.c.d from a dual-stack
+ listener versus a.b.c.d from a proxy),
+ - alternative spellings of one address are one identity
+ ("::1" / "0:0:0:0:0:0:0:1", upper/lower case hex),
+ - a value that is not a bare IP address is refused rather than
+ turned into an identity of its own (port suffixes, brackets, RFC
+ 7239 "unknown"/"_hidden", hostnames, zone ids, junk),
+ - a cookie issued for one host is not accepted for another.
+
+It links paivana-httpd_helper.c and paivana-httpd_cookie.c directly
+and supplies the daemon globals itself, so it needs no MHD connection
+and no merchant backend. The integration suite cannot cover any of
+this: with `-n` the cookie path is never reached, so the client
+address is never computed.
+
Environment variables
---------------------
diff --git a/src/tests/meson.build b/src/tests/meson.build
@@ -26,6 +26,31 @@ early_response_upstream = executable(
install: false,
)
+# Unit test for the client-address canonicalisation that the access
+# cookie is keyed on. Links the two backend compilation units it
+# exercises; the daemon's globals are supplied by the test itself.
+test_client_address = executable(
+ 'test_client_address',
+ [
+ 'test_client_address.c',
+ '../backend/paivana-httpd_helper.c',
+ '../backend/paivana-httpd_cookie.c',
+ ],
+ dependencies: [
+ talerutil_dep,
+ talermhd_dep,
+ gnunetutil_dep,
+ gcrypt_dep,
+ mhd_dep,
+ json_dep,
+ curl_dep,
+ ],
+ include_directories: [incdir, configuration_inc, paivana_backend_inc],
+ install: false,
+)
+
+test('client_address', test_client_address)
+
test_deps = [
upstream_mhd,
pipeline_client,
