paivana

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

commit a8b943313330695fb48a8ebb271784a64cbeaba8
parent 20f568902c43f0bdb1d676b01154fa2d5daffa2d
Author: Christian Grothoff <christian@grothoff.org>
Date:   Fri,  7 Aug 2026 14:02:40 +0200

improve reverse HTTP standards conformance (options, timeouts, etc.)

Diffstat:
MREADME | 36+++++++++++++++++++++++++++++++++++-
Msrc/backend/paivana-httpd_reverse.c | 613+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 622 insertions(+), 27 deletions(-)

diff --git a/README b/README @@ -173,7 +173,23 @@ upstream rather than replacing it. Paivana reads both the RFC 7239 `Forwarded` header and the de-facto `X-Forwarded-*` ones, preferring `Forwarded` where both are present, and emits both upstream — the standardized one for origins that speak -it, the de-facto ones for the many that do not. +it, the de-facto ones for the many that do not. Of the `X-Forwarded-*` +family it emits `-For`, `-Proto`, `-Host` and `-Port`; the port is +taken from the front end's `X-Forwarded-Port` under `-f`, and otherwise +from the authority in `Host` when that names one. + +What the origin sees as `Host` is **not** what the client sent: it is +the authority of `DESTINATION_BASE_URL`, because that is the name +Paivana connects to. The client's own value survives as +`X-Forwarded-Host` (and as the `host` parameter of `Forwarded`). An +origin doing virtual hosting must therefore be configured for the +`DESTINATION_BASE_URL` authority, and an origin that generates absolute +URLs should be told to build them from `X-Forwarded-Host` / +`X-Forwarded-Proto` / `X-Forwarded-Port`. If it builds them from +`Host` instead, its `Location` values will name Paivana's view of the +origin — an internal host and port, which Paivana relays unchanged. +That leaks the internal name, and points the client straight at the +origin wherever the client can route to it, bypassing the paywall. `-f` is only safe if the server in front **overwrites** the forwarding headers rather than appending to whatever the client sent. Otherwise a @@ -313,6 +329,24 @@ configured. The MHD daemon is not started until paywall templates have been fetched from the merchant backend asynchronously. +Because the whole response is buffered before anything is sent on, a +1xx interim response cannot be forwarded — RFC 9110 §15.2 asks a proxy +to forward them, and Paivana instead drops them. `103 Early Hints` +therefore does not reach clients through Paivana. Its header fields +are dropped with it rather than being merged into the final response, +which is the part that would be actively harmful. Trailer fields are +dropped for the same reason (RFC 9110 §6.5.1 forbids merging them into +the header section). + +An origin that accepts the connection but does not finish a response +within 60 seconds yields `504 Gateway Timeout`; one that cannot be +reached at all yields `502 Bad Gateway`. The distinction matters +because caches and monitoring retry the former and not the latter. + +`OPTIONS` carrying `Max-Forwards: 0` is answered by Paivana itself with +an `Allow` list, as RFC 9110 §7.6.2 requires of an intermediary; any +larger value is decremented before the request is passed on. + License ------- diff --git a/src/backend/paivana-httpd_reverse.c b/src/backend/paivana-httpd_reverse.c @@ -57,6 +57,45 @@ */ #define PH_NO_CONTENT_LENGTH UINT64_MAX +/** + * The methods `configure_curl_method()' is willing to forward, in the + * form RFC 9110 §10.2.1 wants for an `Allow' field: sent on the 405 + * for anything else, and on the OPTIONS we answer ourselves. + */ +#define PH_ALLOWED_METHODS \ + "GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS" + +/** + * How long the origin has to complete a response before we give up on + * it and answer 504. + */ +#define PH_UPSTREAM_TIMEOUT \ + GNUNET_TIME_relative_multiply (GNUNET_TIME_UNIT_SECONDS, 60) + +/** + * Value for CURLOPT_TIMEOUT, in seconds. + * + * Deliberately *longer* than #PH_UPSTREAM_TIMEOUT rather than equal to + * it. libcurl reports a timeout the same way it reports a refused + * connection or a TLS failure -- CURLINFO_RESPONSE_CODE of 0, with the + * CURLcode not surfaced by libgnunetcurl -- so the only way to tell + * "the origin was slow" (504, and worth retrying) from "the origin was + * not there" (502, and not) is to have our own deadline expire first. + * Setting both to 60 s would make which of them fires a race. This one + * remains as a backstop: if it ever fires, our task did not run, and + * 502 is then the honest answer. + */ +#define PH_CURL_TIMEOUT_S 90L + +/** + * Value for CURLOPT_CONNECTTIMEOUT, in seconds. + * + * Must stay below #PH_UPSTREAM_TIMEOUT for the same reason: a connect + * that hangs is a 502, and it has to be able to say so before the + * gateway-timeout deadline claims it. + */ +#define PH_CURL_CONNECT_TIMEOUT_S 30L + /** * State machine for HTTP requests (per request). MHD invokes the @@ -300,6 +339,62 @@ struct HttpRequest bool head_request; /** + * Has the blank line ending the final response's header section + * arrived? Everything `curl_check_hdr()' is handed after that is a + * TRAILER field: libcurl delivers those through the same callback + * (it flags them CURLH_TRAILER, which the callback cannot see) and + * there is no CURLOPT_* to suppress them. + * + * RFC 9110 §6.5.1: "A recipient MUST NOT merge a trailer field into + * a header section unless the recipient understands the + * corresponding header field definition and that definition + * explicitly permits and defines how trailer field values can be + * safely merged." Practically no field definition does; Set-Cookie + * and Cache-Control certainly do not. We also strip `Trailer' as + * hop-by-hop, so the client would not even be told which fields had + * been trailers. Hence: drop them. + */ + bool headers_complete; + + /** + * Are we inside the header section of a 1xx interim response? + * + * libcurl reports the headers of *every* response it receives, + * including interim ones, through the same callback. Merging those + * into the final response is how a 103 Early Hints `Link' -- which + * origins commonly build out of the request path -- reappears on the + * final response, where it is subject to entirely different + * processing (preload fetches, CSP, cookie jars). + * + * RFC 9110 §15.2 says a proxy MUST forward 1xx responses; our fully + * buffered model cannot, since MHD is handed one finished response. + * Dropping them is therefore a known deviation -- but dropping is + * what the status code degrades to gracefully, and merging is not. + */ + bool interim_response; + + /** + * Did #PH_UPSTREAM_TIMEOUT expire on this request? Decides whether + * #REQUEST_STATE_PROXY_DOWNLOAD_FAILED becomes a 504 or a 502. + */ + bool upstream_timed_out; + + /** + * Deadline for the upstream request, armed while @e job is + * outstanding. See #PH_UPSTREAM_TIMEOUT. + */ + struct GNUNET_SCHEDULER_Task *timeout_task; + + /** + * Ready-made `Max-Forwards: N' line to send upstream in place of the + * client's, with N one less than the value we received; NULL when + * the client sent none or the method is not OPTIONS. `con_val_iter' + * drops the raw header whenever this is set, so the origin sees + * exactly one, decremented (RFC 9110 §7.6.2). + */ + char *max_forwards; + + /** * Concatenated value of the client's Via header(s), if any. Per * RFC 9110 §7.6.3 a proxy must *append* its own entry to this * list, not replace it; we capture the inbound value here before @@ -329,6 +424,17 @@ struct HttpRequest char *client_xfp; /** + * Value of the client's `X-Forwarded-Port` header, if any. Like + * @e client_xfp this names a single value, so the first one wins. + * `con_val_iter' drops every `X-Forwarded-*' on the way in, and + * `append_forwarded_headers' used to re-emit only -For, -Proto and + * -Host -- so a port the front-end had told us about was consumed + * for our own base URL and then withheld from the origin, which + * then generated absolute URLs on the wrong port. + */ + char *client_xfport; + + /** * Value of the client's `X-Forwarded-Host` header, if any. Used * in place of the client's `Host:` when * #PH_respect_forwarded_headers is set. Not a list header: the @@ -382,6 +488,13 @@ static struct HttpRequest *hr_tail; static struct MHD_Response *curl_failure_response; /** + * Response we return when the upstream did not answer in time. + * Distinct from #curl_failure_response so the two can carry the status + * codes RFC 9110 §15.6.3 and §15.6.5 actually assign them. + */ +static struct MHD_Response *timeout_failure_response; + +/** * Response we return if the HTTP method is not allowed. */ static struct MHD_Response *method_failure_response; @@ -396,6 +509,12 @@ static struct MHD_Response *upload_failure_response; */ static struct MHD_Response *internal_failure_response; +/** + * Response we return to an OPTIONS request we may not forward, that + * is one whose `Max-Forwards' had reached zero. + */ +static struct MHD_Response *options_response; + /** * Create HTML response using @a body @@ -432,6 +551,12 @@ PAIVANA_HTTPD_reverse_init (void) "<body><h1>502 Bad Gateway</h1>" "<p>The upstream server could not be reached.</p>" "</body></html>\n"; + static const char *timeout_failure_body = + "<!DOCTYPE html>\n" + "<html><head><title>Gateway Timeout</title></head>" + "<body><h1>504 Gateway Timeout</h1>" + "<p>The upstream server did not respond in time.</p>" + "</body></html>\n"; static const char *internal_failure_body = "<!DOCTYPE html>\n" "<html><head><title>Internal server failure</title></head>" @@ -458,6 +583,13 @@ PAIVANA_HTTPD_reverse_init (void) GNUNET_break (0); return false; } + timeout_failure_response + = make_html_response (timeout_failure_body); + if (NULL == timeout_failure_response) + { + GNUNET_break (0); + return false; + } upload_failure_response = make_html_response (upload_failure_body); if (NULL == upload_failure_response) @@ -475,8 +607,24 @@ PAIVANA_HTTPD_reverse_init (void) GNUNET_break (MHD_YES == MHD_add_response_header (method_failure_response, MHD_HTTP_HEADER_ALLOW, - "GET, HEAD, POST, PUT, PATCH," - " DELETE, OPTIONS")); + PH_ALLOWED_METHODS)); + /* Answer for an OPTIONS that arrives with `Max-Forwards: 0': we are + then the final recipient and must describe ourselves, not ask the + origin (RFC 9110 §7.6.2). */ + options_response + = MHD_create_response_from_buffer_static (0, + ""); + if (NULL == options_response) + { + GNUNET_break (0); + return false; + } + /* MHD derives `Content-Length: 0' from the zero-size body itself, + and rejects an explicit one here. */ + GNUNET_break (MHD_YES == + MHD_add_response_header (options_response, + MHD_HTTP_HEADER_ALLOW, + PH_ALLOWED_METHODS)); internal_failure_response = make_html_response (internal_failure_body); if (NULL == internal_failure_response) @@ -529,6 +677,16 @@ PAIVANA_HTTPD_reverse_shutdown (void) MHD_destroy_response (internal_failure_response); internal_failure_response = NULL; } + if (NULL != options_response) + { + MHD_destroy_response (options_response); + options_response = NULL; + } + if (NULL != timeout_failure_response) + { + MHD_destroy_response (timeout_failure_response); + timeout_failure_response = NULL; + } } @@ -697,6 +855,13 @@ collect_proxy_state (void *cls, hr->client_xfh = GNUNET_strdup (value); return MHD_YES; } + if (0 == strcasecmp (PH_HEADER_X_FORWARDED_PORT, + key)) + { + if (NULL == hr->client_xfport) + hr->client_xfport = GNUNET_strdup (value); + return MHD_YES; + } if (0 == strcasecmp (MHD_HTTP_HEADER_VIA, key)) target = &hr->client_via; @@ -721,6 +886,95 @@ collect_proxy_state (void *cls, /** + * Is @a txt a port number we are willing to pass on? + * + * @param txt candidate text + * @return true if @a txt is 1..5 digits naming a port in 1..65535 + */ +static bool +valid_port_text (const char *txt) +{ + size_t len = strlen (txt); + unsigned long port; + + if ( (0 == len) || + (len > 5) ) + return false; + for (size_t i = 0; i < len; i++) + if (! isdigit ((unsigned char) txt[i])) + return false; + port = strtoul (txt, + NULL, + 10); + return (port > 0) && (port <= 65535); +} + + +/** + * Return the port of the authority @a host, if it carries one. + * + * Understands the bracketed form an IPv6 literal needs (RFC 3986 + * §3.2.2), where the colons of the address itself must not be mistaken + * for the port separator. + * + * @param host authority, such as "example.com:8443" or "[::1]:8443" + * @return pointer into @a host just past the ':', or NULL if there is + * no usable port + */ +static const char * +host_port_suffix (const char *host) +{ + const char *colon; + + if ('[' == host[0]) + { + const char *close = strchr (host, + ']'); + + if (NULL == close) + return NULL; + colon = (':' == close[1]) ? close + 1 : NULL; + } + else + { + colon = strchr (host, + ':'); + /* More than one colon and no brackets is a bare IPv6 literal, + which has no port to find. */ + if ( (NULL != colon) && + (NULL != strchr (colon + 1, + ':')) ) + return NULL; + } + if (NULL == colon) + return NULL; + return valid_port_text (colon + 1) ? colon + 1 : NULL; +} + + +/** + * Discard every response header accumulated for @a hr so far. + * + * @param[in,out] hr request whose collected headers to release + */ +static void +free_response_headers (struct HttpRequest *hr) +{ + struct HttpResponseHeader *header; + + while (NULL != (header = hr->header_head)) + { + GNUNET_CONTAINER_DLL_remove (hr->header_head, + hr->header_tail, + header); + GNUNET_free (header->type); + GNUNET_free (header->value); + GNUNET_free (header); + } +} + + +/** * Transform _one_ CURL header (gotten from the request) into * MHD format and put it into the response headers list; mostly * copies the headers, but makes special adjustments based on @@ -742,17 +996,81 @@ curl_check_hdr (void *buffer, struct HttpRequest *hr = cls; struct HttpResponseHeader *header; size_t bytes = size * nmemb; + size_t len; char *ndup; const char *hdr_type; char *hdr_val; char *tok; + /* Past the end of the final header section: a trailer field. See + @e headers_complete. */ + if (hr->headers_complete) + return bytes; /* Raw line is not guaranteed to be null-terminated. */ ndup = GNUNET_malloc (bytes + 1); memcpy (ndup, buffer, bytes); ndup[bytes] = '\0'; + /* Trim the line terminator here rather than leaving it to the value + parsing below: the blank line and the status line both have to be + recognised before anything is treated as a field line. */ + len = strlen (ndup); + while ( (len > 0) && + ( ('\r' == ndup[len - 1]) || + ('\n' == ndup[len - 1]) ) ) + ndup[--len] = '\0'; + if (0 == len) + { + /* Blank line: end of a header section. Which one it ended is what + decides whether trailers follow (final) or another response is + still to come (interim). */ + if (hr->interim_response) + hr->interim_response = false; + else + hr->headers_complete = true; + GNUNET_free (ndup); + return bytes; + } + if (0 == strncasecmp (ndup, + "HTTP/", + strlen ("HTTP/"))) + { + /* A status line, not a field line. Recognising it is what keeps + `HTTP/1.1 500 Error: foo' from being parsed as a header whose + name is "HTTP/1.1 500 Error" -- MHD rejects that name, so the + only visible effect used to be a remotely triggerable + assertion in the log. */ + unsigned int code = 0; + const char *sp = strchr (ndup, ' '); + + if (NULL != sp) + code = (unsigned int) strtoul (sp + 1, + NULL, + 10); + hr->interim_response = ( (code >= 100) && + (code < MHD_HTTP_OK) ); + if (! hr->interim_response) + { + /* Start of a final response's header section. Discard anything + accumulated so far: if a 1xx preceded us its fields belong to + it alone, and libcurl can also replay a full exchange (for + instance an authentication round) through this callback. */ + free_response_headers (hr); + hr->upstream_content_length = PH_NO_CONTENT_LENGTH; + GNUNET_free (hr->upstream_connection); + hr->upstream_connection = NULL; + } + GNUNET_free (ndup); + return bytes; + } + if (hr->interim_response) + { + PAIVANA_LOG_DEBUG ("Not merging interim-response header line `%s'\n", + ndup); + GNUNET_free (ndup); + return bytes; + } hdr_type = strtok (ndup, ":"); if (NULL == hdr_type) { @@ -856,6 +1174,46 @@ status_allows_body (unsigned int code) /** + * Task run when the origin did not complete a response within + * #PH_UPSTREAM_TIMEOUT. + * + * Exists so that a slow origin can be told apart from an unreachable + * one: both reach `curl_download_cb()' as a response code of 0, and + * RFC 9110 gives them different status codes (§15.6.5 504 "did not + * receive a timely response from an upstream server" versus §15.6.3 + * 502). Monitoring and CDN retry policies act on the difference -- a + * 504 is retried, a 502 usually is not. + * + * @param cls the `struct HttpRequest *` that ran out of time + */ +static void +upstream_timeout (void *cls) +{ + struct HttpRequest *hr = cls; + + hr->timeout_task = NULL; + hr->upstream_timed_out = true; + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Upstream did not answer for `%s' within %s\n", + hr->url, + GNUNET_STRINGS_relative_time_to_string (PH_UPSTREAM_TIMEOUT, + true)); + if (NULL != hr->job) + { + GNUNET_CURL_job_cancel (hr->job); + hr->job = NULL; + } + hr->state = REQUEST_STATE_PROXY_DOWNLOAD_FAILED; + if (GNUNET_YES == hr->suspended) + { + hr->suspended = GNUNET_NO; + MHD_resume_connection (hr->con); + TALER_MHD_daemon_trigger (); + } +} + + +/** * Handle response payload data from cURL. * * @param cls our `struct HttpRequest *` @@ -872,6 +1230,11 @@ curl_download_cb (void *cls, struct HttpRequest *hr = cls; hr->job = NULL; + if (NULL != hr->timeout_task) + { + GNUNET_SCHEDULER_cancel (hr->timeout_task); + hr->timeout_task = NULL; + } if (GNUNET_YES == hr->suspended) { hr->suspended = GNUNET_NO; @@ -945,6 +1308,45 @@ curl_download_cb (void *cls, } hr->own_response = true; hr->response_code = response_code; + /* Restore the upstream's Content-Length where it describes a + representation rather than the bytes that follow. + `curl_check_hdr()' drops it unconditionally, which is right for a + 200 (MHD regenerates it from the buffer) and wrong here: the + buffer is legitimately empty while the field must still state the + length the equivalent GET would have had. RFC 9110 §8.6 makes + that a MUST NOT in both directions -- a HEAD response must not + carry a length that differs from the GET's, and neither must a + 304 differ from the 200 it stands in for -- and MHD synthesises + `Content-Length: 0', which is exactly such a difference. + + MHD_RF_HEAD_ONLY_RESPONSE is what makes MHD accept a declared + length with no body behind it; without it MHD overrides us. */ + if ( (PH_NO_CONTENT_LENGTH != hr->upstream_content_length) && + (0 == body_size) /* MHD_RF_HEAD_ONLY_RESPONSE requires it */ && + ( (hr->head_request) || + (MHD_HTTP_NOT_MODIFIED == (unsigned int) response_code) ) ) + { + char clbuf[24]; + + GNUNET_snprintf (clbuf, + sizeof (clbuf), + "%llu", + (unsigned long long) hr->upstream_content_length); + if (MHD_YES != + MHD_set_response_options (hr->response, + MHD_RF_HEAD_ONLY_RESPONSE, + MHD_RO_END)) + { + GNUNET_break (0); + } + else if (MHD_YES != + MHD_add_response_header (hr->response, + MHD_HTTP_HEADER_CONTENT_LENGTH, + clbuf)) + { + GNUNET_break (0); + } + } /* RFC 9110 §7.6.1: drop the headers named by the upstream's Connection list. Deferred to here (rather than done in `curl_check_hdr()') because Connection may arrive after the @@ -979,10 +1381,21 @@ curl_download_cb (void *cls, "Adding MHD response header %s->%s\n", header->type, header->value); - GNUNET_break (MHD_YES == - MHD_add_response_header (hr->response, - header->type, - header->value)); + if (MHD_YES != + MHD_add_response_header (hr->response, + header->type, + header->value)) + { + /* MHD validates the field name (RFC 9112 §5 token) and the + value; a malformed one is the upstream's fault, not ours, so + GNUNET_break_op and not GNUNET_break -- the latter let an + eccentric origin fill the log with "Assertion failed". */ + GNUNET_break_op (0); + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Upstream `%s' sent a header MHD rejected: `%s'\n", + hr->url, + header->type); + } } if ( (REQUEST_STATE_PROXY_DOWNLOAD_STARTED == hr->state) || (REQUEST_STATE_PROXY_UPLOAD_STARTED == hr->state) ) @@ -1258,6 +1671,13 @@ con_val_iter (void *cls, /* libcurl manages Expect: 100-continue on its own. */ return MHD_YES; } + if ( (NULL != hr->max_forwards) && + (0 == strcasecmp (MHD_HTTP_HEADER_MAX_FORWARDS, + key)) ) + { + /* Replaced by the decremented copy `configure_curl_method' built. */ + return MHD_YES; + } if (is_hop_by_hop_header (key)) return MHD_YES; /* RFC 9110 §7.6.1: suppress any header named by the client's @@ -1328,8 +1748,6 @@ PAIVANA_HTTPD_reverse_create (struct MHD_Connection *connection, void PAIVANA_HTTPD_reverse_cleanup (struct HttpRequest *hr) { - struct HttpResponseHeader *header; - if (NULL != hr->curl) { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, @@ -1338,6 +1756,11 @@ PAIVANA_HTTPD_reverse_cleanup (struct HttpRequest *hr) hr->curl = NULL; hr->io_len = 0; } + if (NULL != hr->timeout_task) + { + GNUNET_SCHEDULER_cancel (hr->timeout_task); + hr->timeout_task = NULL; + } if (NULL != hr->job) { GNUNET_CURL_job_cancel (hr->job); @@ -1355,17 +1778,7 @@ PAIVANA_HTTPD_reverse_cleanup (struct HttpRequest *hr) reference we never held. */ MHD_destroy_response (hr->response); - for (header = hr->header_head; - header != NULL; - header = hr->header_head) - { - GNUNET_CONTAINER_DLL_remove (hr->header_head, - hr->header_tail, - header); - GNUNET_free (header->type); - GNUNET_free (header->value); - GNUNET_free (header); - } + free_response_headers (hr); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Proxying of '%s' completely done\n", hr->url); @@ -1376,6 +1789,8 @@ PAIVANA_HTTPD_reverse_cleanup (struct HttpRequest *hr) GNUNET_free (hr->client_xff); GNUNET_free (hr->client_xfp); GNUNET_free (hr->client_xfh); + GNUNET_free (hr->client_xfport); + GNUNET_free (hr->max_forwards); GNUNET_free (hr->client_forwarded); GNUNET_free (hr->client_connection); GNUNET_free (hr->upstream_connection); @@ -1511,6 +1926,44 @@ buffer_upload_chunk (struct HttpRequest *hr, /** + * Arrange for a buffered request body to be forwarded under @a meth, + * and set @a hr->state to match. + * + * For the methods whose curl option is otherwise bodyless + * (CURLOPT_HTTPGET, CURLOPT_NOBODY, or a bare CURLOPT_CUSTOMREQUEST), + * turning on CURLOPT_POST is what enables the read callback; + * CURLOPT_CUSTOMREQUEST then restores the verb on the wire. This is + * the same shape DELETE has always used. With no body buffered + * nothing is changed, so the common case still goes out as a plain + * GET. + * + * @param[in,out] hr the request + * @param meth HTTP method to keep on the wire + */ +static void +forward_body_verbatim (struct HttpRequest *hr, + const char *meth) +{ + if (0 == hr->io_len) + { + hr->state = REQUEST_STATE_PROXY_DOWNLOAD_STARTED; + return; + } + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, + "Forwarding a %llu byte request body with %s\n", + (unsigned long long) hr->io_len, + meth); + curl_easy_setopt (hr->curl, + CURLOPT_POST, + 1L); + curl_easy_setopt (hr->curl, + CURLOPT_CUSTOMREQUEST, + meth); + hr->state = REQUEST_STATE_PROXY_UPLOAD_STARTED; +} + + +/** * Choose the curl options for the HTTP method we're proxying and * set the next proxy state accordingly. Queues an error response * and returns the corresponding MHD_Result for unsupported methods; @@ -1533,10 +1986,18 @@ configure_curl_method (struct HttpRequest *hr, if (0 == strcasecmp (meth, MHD_HTTP_METHOD_GET)) { - hr->state = REQUEST_STATE_PROXY_DOWNLOAD_STARTED; curl_easy_setopt (hr->curl, CURLOPT_HTTPGET, 1L); + /* A GET body is unusual and its semantics are undefined (RFC 9110 + §9.3.1), but it is not forbidden and real APIs use it -- + GraphQL-over-GET, several search endpoints. Dropping it while + `con_val_iter()' still forwards the client's Content-Type left + the origin with a request advertising content that never + arrived, and the client with a parse error it could not + explain. */ + forward_body_verbatim (hr, + MHD_HTTP_METHOD_GET); return MHD_YES; } if (0 == strcasecmp (meth, @@ -1553,13 +2014,16 @@ configure_curl_method (struct HttpRequest *hr, if (0 == strcasecmp (meth, MHD_HTTP_METHOD_HEAD)) { - hr->state = REQUEST_STATE_PROXY_DOWNLOAD_STARTED; /* The upstream still states the length the equivalent GET would have had; nothing follows it (RFC 9110 §9.3.2). */ hr->head_request = true; curl_easy_setopt (hr->curl, CURLOPT_NOBODY, 1L); + /* HEAD must be forwarded with whatever body the equivalent GET + carried, or the origin cannot answer the question HEAD asks. */ + forward_body_verbatim (hr, + MHD_HTTP_METHOD_HEAD); return MHD_YES; } if (0 == strcasecmp (meth, @@ -1614,10 +2078,62 @@ configure_curl_method (struct HttpRequest *hr, if (0 == strcasecmp (meth, MHD_HTTP_METHOD_OPTIONS)) { - hr->state = REQUEST_STATE_PROXY_DOWNLOAD_STARTED; + const char *mf; + curl_easy_setopt (hr->curl, CURLOPT_CUSTOMREQUEST, "OPTIONS"); + /* RFC 9110 §7.6.2: "Each intermediary that receives a TRACE or + OPTIONS request containing a Max-Forwards header field MUST + check and update its value prior to forwarding ... If the + received value is zero (0), the intermediary MUST NOT forward + the request; instead, the intermediary MUST respond as the final + recipient." TRACE we reject outright, so OPTIONS is the only + method this can reach. */ + mf = MHD_lookup_connection_value (con, + MHD_HEADER_KIND, + MHD_HTTP_HEADER_MAX_FORWARDS); + if (NULL != mf) + { + char *endptr; + unsigned long long hops; + + errno = 0; + hops = strtoull (mf, + &endptr, + 10); + if ( (0 != errno) || + (endptr == mf) || + ('\0' != *endptr) ) + { + /* Not a valid Max-Forwards. Neither forwarding it nor acting + as the final recipient is defined for that, so treat it as + the client's error rather than guessing. */ + GNUNET_break_op (0); + curl_easy_cleanup (hr->curl); + hr->curl = NULL; + return MHD_queue_response (con, + MHD_HTTP_BAD_REQUEST, + method_failure_response); + } + if (0 == hops) + { + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, + "Answering OPTIONS as the final recipient:" + " Max-Forwards is exhausted\n"); + curl_easy_cleanup (hr->curl); + hr->curl = NULL; + return MHD_queue_response (con, + MHD_HTTP_OK, + options_response); + } + GNUNET_asprintf (&hr->max_forwards, + "%s: %llu", + MHD_HTTP_HEADER_MAX_FORWARDS, + hops - 1); + } + forward_body_verbatim (hr, + MHD_HTTP_METHOD_OPTIONS); return MHD_YES; } /* TRACE leaks headers back to the client; CONNECT is for TLS @@ -1815,6 +2331,9 @@ append_forwarded_headers (struct HttpRequest *hr, hdr); GNUNET_free (hdr); } + if (NULL != hr->max_forwards) + hr->headers = curl_slist_append (hr->headers, + hr->max_forwards); proto = forwarded_proto (hr, con, &owned_proto); @@ -1850,6 +2369,41 @@ append_forwarded_headers (struct HttpRequest *hr, hdr); GNUNET_free (hdr); } + { + /* `con_val_iter' drops every X-Forwarded-* on the way in, so + whatever the front end told us about the port has to be put back + here or the origin never learns it. The documented nginx recipe + sends `X-Forwarded-Port', and `$host' in `X-Forwarded-Host' + omits the port -- so without this an origin behind + https://example.com:8443 reconstructs https://example.com/ and + every absolute URL it generates points at port 443. */ + const char *fport = NULL; + + if (PH_respect_forwarded_headers) + fport = hr->client_xfport; + if ( (NULL != fport) && + (! valid_port_text (fport)) ) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Ignoring unusable %s value `%s'\n", + PH_HEADER_X_FORWARDED_PORT, + fport); + fport = NULL; + } + if ( (NULL == fport) && + (NULL != fhost) ) + fport = host_port_suffix (fhost); + if (NULL != fport) + { + GNUNET_asprintf (&hdr, + "%s: %s", + PH_HEADER_X_FORWARDED_PORT, + fport); + hr->headers = curl_slist_append (hr->headers, + hdr); + GNUNET_free (hdr); + } + } /* RFC 7239. The element we add describes the hop we are completing: `for` is the peer we accepted from, `by` the interface we accepted on, and `proto`/`host` what the client used. A Unix-domain peer @@ -2021,10 +2575,10 @@ start_curl_request (struct HttpRequest *hr, 0); curl_easy_setopt (hr->curl, CURLOPT_CONNECTTIMEOUT, - 60L); + PH_CURL_CONNECT_TIMEOUT_S); curl_easy_setopt (hr->curl, CURLOPT_TIMEOUT, - 60L); + PH_CURL_TIMEOUT_S); curl_easy_setopt (hr->curl, CURLOPT_NOSIGNAL, 1L); @@ -2118,6 +2672,9 @@ start_curl_request (struct HttpRequest *hr, MHD_HTTP_BAD_GATEWAY, curl_failure_response); } + hr->timeout_task = GNUNET_SCHEDULER_add_delayed (PH_UPSTREAM_TIMEOUT, + &upstream_timeout, + hr); return MHD_YES; } @@ -2264,8 +2821,12 @@ PAIVANA_HTTPD_reverse (struct HttpRequest *hr, con); case REQUEST_STATE_PROXY_DOWNLOAD_FAILED: return MHD_queue_response (con, - MHD_HTTP_BAD_GATEWAY, - curl_failure_response); + hr->upstream_timed_out + ? MHD_HTTP_GATEWAY_TIMEOUT + : MHD_HTTP_BAD_GATEWAY, + hr->upstream_timed_out + ? timeout_failure_response + : curl_failure_response); } GNUNET_assert (0); /* unreachable */ }