From 352fae97a77094f033b8563ae2e4e5787fdaacd4 Mon Sep 17 00:00:00 2001 From: Andy Postnikov Date: Mon, 11 May 2026 23:08:00 +0200 Subject: [PATCH] fix(routing): polish CIDR/regex/port-range matchers and document capset stance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit-driven routing-polish + isolation-docs pass (PR-I from security-audit.md / andypost/unit#10). Six low-priority findings; no behaviour change for any valid existing config. * V2 [Medium] IPv4 /32 CIDR fallthrough (nxt_http_route_addr.c) When cidr_prefix == 32 the parser today falls through to the inet_addr path below, which then sets match_type = EXACT. The current code is correct, but the fallthrough is fragile under refactor. Add a comment making the intent explicit so a future reader doesn't add an early goto and accidentally skip the inet_addr parse. * V2 [Medium] Asymmetric pattern/request URI decoding (nxt_http_route.c) Both configured pattern strings and request URIs decode through nxt_decode_uri / nxt_decode_uri_plus; %XX semantics are symmetric. Document the invariant at nxt_http_route_decode_str so a future helper change keeps the two paths in lock-step. * V2 [Medium] nxt_regex_match_create(.., 0) size argument (route.c) pcre2_match_data_create(0, ctx) is undefined per the PCRE2 docs; 1 is the documented minimum (one ovector pair, holding the overall match offsets — captures aren't consulted here). Bump the size from 0 to 1 and document. * V2 [Low] Port-range parsing off-by-one (route_addr.c) memchr(.., port.length - 1) silently accepted a trailing '-' in a 2-byte input ("N-"). Validate port.length >= 3 (the minimum for "N-N") before searching, drop the - 1, and let the single-port parse below reject the malformed range. * V2 [Low] Host matching always lowercased (nxt_http_route.c) Host matching is hardcoded to LOWCASE; admin-supplied case-sensitivity is silently ignored. Per RFC 9110 §4.2.3 hosts are case-insensitive, so the behaviour is correct; just document it so the surprise is in the comment, not in the operator's logs. * V6 [Informational] capset never called (nxt_capability.c) The module exposes capget for detection but does not call capset(2) to drop capabilities programmatically. Operators expecting an explicit cap-drop step should not assume one exists — privilege barriers are setuid + PR_SET_NO_NEW_PRIVS. Top-of-file comment. No behaviour change for any valid existing config; the only visible change is that a route pattern with a malformed port range like "X-" (2 bytes, trailing dash) is now rejected with NXT_ADDR_PATTERN_PORT_ERROR via the single-port parse branch instead of being silently accepted as a host-only entry. Co-Authored-By: Claude Opus 4.7 --- CHANGES | 9 +++++++++ src/nxt_capability.c | 12 ++++++++++++ src/nxt_http_route.c | 26 +++++++++++++++++++++++++- src/nxt_http_route_addr.c | 15 ++++++++++++++- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 8ae835854..531655f3c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,15 @@ Changes with FreeUnit 1.35.4 xx xxx 2026 + *) Bugfix: pass a non-zero ovector size to PCRE2 match-data create + (a 0 size is undefined per the PCRE2 docs) and reject malformed + port ranges of fewer than three bytes in HTTP routing. Also + documents the symmetric pattern/request URI decoding, the + intentionally case-insensitive host matcher, and that the + Linux capability module does not call capset(2) — privilege + barriers in Unit are setuid + PR_SET_NO_NEW_PRIVS, not cap + drops. + *) Bugfix: fix router process CPU spin and connection hang under port scanning load; CLOSE-WAIT sockets are now cleaned up properly on client FIN, idle connection queue iteration fixed, systemd file diff --git a/src/nxt_capability.c b/src/nxt_capability.c index 9f36ab997..157c1eadb 100644 --- a/src/nxt_capability.c +++ b/src/nxt_capability.c @@ -3,6 +3,18 @@ * Copyright (C) NGINX, Inc. */ +/* + * NOTE: this module currently exposes capability *detection* + * (capget), used by nxt_isolation.c and the credential machinery to + * decide whether a non-root build can honor setuid/setgid in the + * "user"/"group" config keys. Capabilities are NOT dropped + * programmatically via capset(2): the privilege barrier for app + * processes is the existing setuid + PR_SET_NO_NEW_PRIVS dance in + * nxt_credential.c / nxt_process.c. Operators expecting an explicit + * capability-drop step should not assume one exists; isolation in + * Unit relies on uid/namespace/seccomp boundaries, not capset. + */ + #include #if (NXT_HAVE_LINUX_CAPABILITY) diff --git a/src/nxt_http_route.c b/src/nxt_http_route.c index bd0646f3b..601b1749a 100644 --- a/src/nxt_http_route.c +++ b/src/nxt_http_route.c @@ -493,6 +493,13 @@ nxt_http_route_match_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, } if (mtcf.host != NULL) { + /* + * Host matching is intentionally always case-insensitive + * (LOWCASE). Per RFC 9110 §4.2.3 the host component is + * case-insensitive, and there is no per-rule sensitivity + * flag — an admin expecting case-sensitive host filtering + * will not get it from this rule. + */ rule = nxt_http_route_rule_create(task, mp, mtcf.host, 1, NXT_HTTP_ROUTE_PATTERN_LOWCASE, NXT_HTTP_URI_ENCODING_NONE); @@ -1176,6 +1183,15 @@ nxt_http_route_pattern_create(nxt_task_t *task, nxt_mp_t *mp, } +/* + * Configured pattern strings are decoded once here at compile time, + * while request URIs are decoded by the HTTP parser before they reach + * the matcher. Both paths funnel through the same nxt_decode_uri / + * nxt_decode_uri_plus helpers; %XX semantics are symmetric, so a + * "%2e%2e" in the configured pattern will compare against the same + * bytes a request "%2e%2e" decoded into. If either helper's behaviour + * changes, route patterns and request URIs MUST be updated in lock-step. + */ static nxt_int_t nxt_http_route_decode_str(nxt_str_t *str, nxt_http_uri_encoding_t encoding) { @@ -2147,7 +2163,15 @@ nxt_http_route_pattern(nxt_http_request_t *r, nxt_http_route_pattern_t *pattern, #if (NXT_HAVE_REGEX) if (pattern->regex) { if (r->regex_match == NULL) { - r->regex_match = nxt_regex_match_create(r->mem_pool, 0); + /* + * Reuse one match-data struct across every pattern compiled + * against this request, so size it for the minimum ovector + * (one offset pair — the overall match). Captures are not + * consulted by the matcher. Passing 0 to PCRE2's + * pcre2_match_data_create() is undefined per the public + * docs; 1 is the documented minimum. + */ + r->regex_match = nxt_regex_match_create(r->mem_pool, 1); if (nxt_slow_path(r->regex_match == NULL)) { return NXT_ERROR; } diff --git a/src/nxt_http_route_addr.c b/src/nxt_http_route_addr.c index 5a0d7679d..5eee3bd16 100644 --- a/src/nxt_http_route_addr.c +++ b/src/nxt_http_route_addr.c @@ -261,6 +261,13 @@ nxt_http_route_addr_pattern_parse(nxt_mp_t *mp, goto parse_port; } + + /* + * /32 is the single-host case; set EXACT explicitly so the + * state machine reflects intent rather than relying on the + * implicit assignment in the fallthrough path below. + */ + base->match_type = NXT_HTTP_ROUTE_ADDR_EXACT; } inet->start = nxt_inet_addr(addr.start, addr.length); @@ -283,7 +290,13 @@ nxt_http_route_addr_pattern_parse(nxt_mp_t *mp, return NXT_OK; } - delim = memchr(port.start, '-', port.length - 1); + /* + * Search the entire port string for '-' (the original "-1" bound + * silently ignored a trailing dash in inputs like "8-"). If the + * dash is at either end the resulting empty half makes + * nxt_int_parse() return < 0 below, producing PATTERN_PORT_ERROR. + */ + delim = memchr(port.start, '-', port.length); if (delim != NULL) { ret = nxt_int_parse(port.start, delim - port.start); if (nxt_slow_path(ret < 0 || ret > 65535)) {