commit ebe53cf61e21b00a987f42a3dd7ca23d0dbc84e8
parent 526d63c925ee11f34a3ee96ea923144a4a039e67
Author: Christian Grothoff <christian@grothoff.org>
Date: Fri, 7 Aug 2026 00:04:24 +0200
add timeout on merchant backend request and only Vary on language if we actually have multiple templates that could apply
Diffstat:
2 files changed, 204 insertions(+), 22 deletions(-)
diff --git a/src/backend/paivana-httpd_pay.c b/src/backend/paivana-httpd_pay.c
@@ -37,6 +37,36 @@ struct PayRequest;
#include "taler/merchant/get-private-orders-ORDER_ID.h"
/**
+ * How long we give the merchant backend to answer the
+ * `GET /private/orders/$ORDER_ID' behind one client's redemption.
+ *
+ * A bound is needed at all because the endpoint is unauthenticated:
+ * every syntactically valid POST suspends an MHD connection and issues
+ * a backend query before any payment has been shown to exist, so
+ * without one, a wedged backend pins a suspended connection per
+ * request forever. MHD_OPTION_CONNECTION_TIMEOUT does not apply to
+ * suspended connections, so the deadline has to come from us.
+ *
+ * It is passed as TALER_MERCHANT_get_private_order_option_timeout(),
+ * which does two things at once: it caps the request client-side
+ * (CURLOPT_TIMEOUT_MS, so an unreachable or hung backend is bounded
+ * here too) and it sets `timeout_ms' on the URL, asking the backend to
+ * long-poll for that long before reporting an order as unpaid.
+ *
+ * The long poll is wanted, not merely tolerated. By the time the
+ * client posts here it has already seen the backend confirm the
+ * payment, so an order that still reads "unpaid" means either a client
+ * that is lying -- replaying an order ID it never paid -- or one that
+ * raced a state change that is about to land. Waiting a few seconds
+ * settles the race in the honest client's favour, and the dishonest
+ * one pays for it with a bounded wait and then a 409. Keep it short:
+ * this is the interval an attacker can pin a connection for.
+ */
+#define MERCHANT_ORDER_TIMEOUT \
+ GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 5)
+
+
+/**
* Handle for processing actual payment.
*/
struct PayRequest
@@ -384,7 +414,19 @@ order_status_cb (struct PayRequest *ph,
ph->response_status = MHD_HTTP_SEE_OTHER;
}
break;
+ case MHD_HTTP_UNAUTHORIZED:
case MHD_HTTP_FORBIDDEN:
+ /* Our `MERCHANT_ACCESS_TOKEN' is wrong: the operator's problem, not
+ the client's, hence 500 and not a 4xx. UNAUTHORIZED is the case
+ that actually fires -- taler-merchant-httpd_auth.c answers a bad
+ bearer token with 401 -- and without it this landed in the
+ default branch below, telling the operator that a protocol
+ incompatibility should be reported to us. */
+ GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
+ "Merchant backend at `%s' rejected our credentials (HTTP"
+ " %u); check MERCHANT_ACCESS_TOKEN\n",
+ PH_merchant_base_url,
+ osr->hr.http_status);
ph->response = TALER_MHD_make_error (TALER_EC_PAIVANA_BACKEND_REFUSED,
NULL);
ph->response_status = MHD_HTTP_INTERNAL_SERVER_ERROR;
@@ -394,6 +436,46 @@ order_status_cb (struct PayRequest *ph,
ph->order_id);
ph->response_status = MHD_HTTP_NOT_FOUND;
break;
+ case 0:
+ /* No HTTP status at all. The merchant client library reports this
+ both for a request that never completed and for one whose reply
+ it could not make sense of, with the same error code; @e reply is
+ what tells them apart, being NULL only in the former case. */
+ if (NULL != osr->hr.reply)
+ {
+ GNUNET_break_op (0);
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Merchant backend at `%s' sent an unusable reply for"
+ " order `%s'\n",
+ PH_merchant_base_url,
+ ph->order_id);
+ ph->response = TALER_MHD_make_error (TALER_EC_PAIVANA_BACKEND_ERROR,
+ ph->order_id);
+ ph->response_status = MHD_HTTP_BAD_GATEWAY;
+ break;
+ }
+ /* Nothing came back: the request hit #MERCHANT_ORDER_TIMEOUT, or
+ the backend was unreachable. Either way the client has waited
+ for us rather than been answered, which is what separates 504
+ from the 502 above. */
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Merchant backend at `%s' did not answer for order `%s'"
+ " within %s; giving up on this redemption\n",
+ PH_merchant_base_url,
+ ph->order_id,
+ GNUNET_STRINGS_relative_time_to_string (MERCHANT_ORDER_TIMEOUT,
+ true));
+ /* GENERIC_TIMEOUT's hint ("trying again might help") is the one
+ that is true for the client here; GET_ORDER_FAILED says "this
+ should never happen, consult the logs", which is wrong advice
+ for a backend that was merely slow. A dedicated
+ PAIVANA_BACKEND_TIMEOUT would be better still, but that is a
+ GANA registration and so its own change in another
+ repository. */
+ ph->response = TALER_MHD_make_error (TALER_EC_GENERIC_TIMEOUT,
+ ph->order_id);
+ ph->response_status = MHD_HTTP_GATEWAY_TIMEOUT;
+ break;
default:
{
char code[20];
@@ -500,7 +582,9 @@ PAIVANA_HTTPD_payment_handle (struct PayRequest *ph,
TALER_MERCHANT_get_private_order_set_options (
ph->co,
TALER_MERCHANT_get_private_order_option_session_id (
- paivana_id)));
+ paivana_id),
+ TALER_MERCHANT_get_private_order_option_timeout (
+ MERCHANT_ORDER_TIMEOUT)));
GNUNET_free (paivana_id);
}
GNUNET_CONTAINER_DLL_insert (ph_head,
diff --git a/src/backend/paivana-httpd_templates.c b/src/backend/paivana-httpd_templates.c
@@ -46,11 +46,15 @@ struct Template;
/**
* Maximum number of rendered paywall responses we cache per template.
- * The cache is keyed on the client-supplied Accept-Language /
- * Accept-Encoding headers; without a bound an attacker could send
+ *
+ * The key is derived from the client-supplied Accept-Language and
+ * Accept-Encoding headers, so without a bound an attacker could send
* unlimited distinct header values and grow the cache without limit
- * (memory-exhaustion DoS on the cheap pre-payment path). When the
- * cap is reached we evict the oldest entry (FIFO).
+ * (memory-exhaustion DoS on the cheap pre-payment path). The key is
+ * normalised to what we can actually serve — see cache_key_language()
+ * and TALER_MHD_can_compress() — which is what keeps every request
+ * after the first few a *hit*; this cap only backstops that. On
+ * reaching it we evict the least recently used entry.
*/
#define MAX_RESPONSE_CACHE_ENTRIES 128
@@ -96,9 +100,9 @@ struct ResponseCacheEntry
char *lang;
/**
- * Accept-Encoding of the response.
+ * True if @e paywall carries a deflate-compressed body.
*/
- char *ae;
+ bool deflate;
/**
* Paywall response for these request parameters.
@@ -417,6 +421,46 @@ js_string_literal (const char *s)
/**
+ * Number of installed `paywall.*.must' templates, counted once by
+ * count_paywall_templates(). UINT_MAX until then.
+ */
+static unsigned int paywall_template_count = UINT_MAX;
+
+
+/**
+ * Count the installed paywall templates.
+ *
+ * TALER_TEMPLATING_build() picks among them by matching the client's
+ * `Accept-Language' against the language tag in each file name, and
+ * does not report which one it chose. We do not need to know: if there
+ * is only one to choose from — the shipped configuration, which ships
+ * `paywall.en.must' and nothing else — then every `Accept-Language'
+ * whatsoever produces the same body, and the header can be dropped from
+ * the cache key entirely.
+ *
+ * @param cls unused
+ * @param filename file found in the template directory
+ * @return #GNUNET_OK to continue the scan
+ */
+static enum GNUNET_GenericReturnValue
+count_paywall_template (void *cls,
+ const char *filename)
+{
+ const char *base;
+
+ (void) cls;
+ base = strrchr (filename,
+ '/');
+ base = (NULL == base) ? filename : base + 1;
+ if (0 == strncmp (base,
+ "paywall.",
+ strlen ("paywall.")))
+ paywall_template_count++;
+ return GNUNET_OK;
+}
+
+
+/**
* The `Content-Security-Policy' for the paywall page, built once from
* #PH_merchant_base_url. NULL until first needed.
*/
@@ -497,6 +541,59 @@ get_paywall_csp (void)
/**
+ * Return the language component of the render cache key for @a conn.
+ *
+ * With a single installed paywall template there is nothing to
+ * negotiate, so the key does not depend on `Accept-Language' at all and
+ * the whole header — an unauthenticated client's free choice, on the
+ * pre-payment path — stops being able to force a fresh Mustache render
+ * per distinct value. With several installed we cannot tell which one
+ * the templating library picked (it has no accessor for that; the
+ * proper fix belongs there), so we fall back to keying on the raw
+ * header and accept the amplification for that configuration.
+ *
+ * @param conn connection to derive the key component for
+ * @return the key component, or NULL if `Accept-Language' does not
+ * affect the rendered body
+ */
+static const char *
+cache_key_language (struct MHD_Connection *conn)
+{
+ if (UINT_MAX == paywall_template_count)
+ {
+ char *dir;
+ char *tdir;
+
+ paywall_template_count = 0;
+ dir = GNUNET_OS_installation_get_path (PAIVANA_project_data (),
+ GNUNET_OS_IPK_DATADIR);
+ GNUNET_asprintf (&tdir,
+ "%stemplates",
+ dir);
+ GNUNET_free (dir);
+ if (0 > GNUNET_DISK_directory_scan (tdir,
+ &count_paywall_template,
+ NULL))
+ {
+ /* Cannot tell; assume the worst and keep the old key. */
+ GNUNET_break (0);
+ paywall_template_count = 2;
+ }
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "%u paywall template(s) installed in `%s'\n",
+ paywall_template_count,
+ tdir);
+ GNUNET_free (tdir);
+ }
+ if (2 > paywall_template_count)
+ return NULL;
+ return MHD_lookup_connection_value (conn,
+ MHD_HEADER_KIND,
+ MHD_HTTP_HEADER_ACCEPT_LANGUAGE);
+}
+
+
+/**
* Try to initialize the paywall response.
*
* @param conn connection to create the response for
@@ -509,23 +606,24 @@ load_paywall (struct MHD_Connection *conn,
{
struct MHD_Response *reply;
const char *lang;
- const char *ae;
+ bool deflate;
unsigned int http_status = MHD_HTTP_PAYMENT_REQUIRED;
- lang = MHD_lookup_connection_value (conn,
- MHD_HEADER_KIND,
- MHD_HTTP_HEADER_ACCEPT_LANGUAGE);
- ae = MHD_lookup_connection_value (conn,
- MHD_HEADER_KIND,
- MHD_HTTP_HEADER_ACCEPT_ENCODING);
+ lang = cache_key_language (conn);
+ /* `Accept-Encoding' reaches the body through exactly this predicate
+ (templating_api.c), so keying on its result rather than on the
+ header text is not an approximation: it is the decision itself,
+ and it has two outcomes instead of unboundedly many. */
+ deflate = (TALER_MHD_CT_DEFLATE ==
+ TALER_MHD_can_compress (conn,
+ TALER_MHD_CT_DEFLATE));
for (struct ResponseCacheEntry *pos = t->rce_head;
NULL != pos;
pos = pos->next)
{
if ( (eq (lang,
pos->lang)) &&
- (eq (ae,
- pos->ae) ) )
+ (deflate == pos->deflate) )
{
if (t->rce_head != pos)
{
@@ -678,9 +776,12 @@ load_paywall (struct MHD_Connection *conn,
{
struct ResponseCacheEntry *rce;
- while (t->rce_length > MAX_RESPONSE_CACHE_ENTRIES)
+ /* '>=', not '>': the insert below is what takes us to the cap, so
+ testing '>' left the steady state one entry above it. */
+ while (t->rce_length >= MAX_RESPONSE_CACHE_ENTRIES)
{
- /* Evict the oldest entry (inserted at head, so tail is oldest). */
+ /* Evict the least recently used entry; the hit path above
+ promotes to the head, so the tail is the coldest. */
struct ResponseCacheEntry *old = t->rce_tail;
GNUNET_CONTAINER_DLL_remove (t->rce_head,
@@ -689,15 +790,13 @@ load_paywall (struct MHD_Connection *conn,
GNUNET_assert (t->rce_length > 0);
t->rce_length--;
MHD_destroy_response (old->paywall);
- GNUNET_free (old->ae);
GNUNET_free (old->lang);
GNUNET_free (old);
}
rce = GNUNET_new (struct ResponseCacheEntry);
if (NULL != lang)
rce->lang = GNUNET_strdup (lang);
- if (NULL != ae)
- rce->ae = GNUNET_strdup (ae);
+ rce->deflate = deflate;
rce->paywall = reply;
rce->http_status = http_status;
t->rce_length++;
@@ -1141,7 +1240,6 @@ PAIVANA_HTTPD_unload_templates ()
t->rce_tail,
rce);
MHD_destroy_response (rce->paywall);
- GNUNET_free (rce->ae);
GNUNET_free (rce->lang);
GNUNET_free (rce);
}
