diff --git a/luci-app-homeproxy-pro/Makefile b/luci-app-homeproxy-pro/Makefile index 6b1cddec7..9b6995141 100644 --- a/luci-app-homeproxy-pro/Makefile +++ b/luci-app-homeproxy-pro/Makefile @@ -41,7 +41,7 @@ LUCI_DEPENDS:= \ PKG_NAME:=luci-app-homeproxy-pro PKG_VERSION:=28.10.1.14 -PKG_RELEASE:=23 +PKG_RELEASE:=24 LUCI_BASENAME:=homeproxy-pro diff --git a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/firewall_post.ut b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/firewall_post.ut index bf27f9de5..133582c2d 100755 --- a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/firewall_post.ut +++ b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/firewall_post.ut @@ -26,7 +26,7 @@ */ import { readfile } from 'fs'; import { cursor } from 'uci'; -import { isEmpty, isValidCIDR } from '/etc/homeproxy-pro/scripts/homeproxy-pro.uc'; +import { isEmpty, isValidCIDR, udp_transport_verdict } from '/etc/homeproxy-pro/scripts/homeproxy-pro.uc'; import { ipv4_to_nftarr, mac_to_nftarr, @@ -288,15 +288,38 @@ if (match(proxy_mode, /redirect/)) { self_mark = mark_value_or(uci.get(cfgname, 'infra', 'self_mark'), resolve_mark('100')); redirect_port = port_or(uci.get(cfgname, 'infra', 'redirect_port'), '5331'); } -/* Whether UDP can actually be proxied in this configuration: there is a - * UDP-capable node, or the user took the decision over with custom rules. - * The UDP tproxy chain already uses exactly this condition, and the QUIC reject - * has to agree with it - it keyed off `routing_mode !== 'custom'` only, so a +/* Whether QUIC has to be rejected, and why. + * + * This used to ask "is there a UDP node?" and that question has been wrong in + * both directions. + * + * Wrong by too much: it keyed off `routing_mode !== 'custom'` alone, so a * router with `main_udp_node` set still rejected QUIC on UDP 443 (measured on * the device: 1722 QUIC packets rejected in one sample) and browsers that had - * learned HTTP/3 fell back to TCP needlessly. Now the reject is emitted only - * when UDP really is not proxyable. */ -const udp_proxyable = (outbound_udp_node !== 'nil' || routing_mode === 'custom'); + * already learned HTTP/3 fell back to TCP needlessly. + * + * Wrong by too little, which is what the current form is: "a UDP node exists" + * says nothing about what that node's transport is. `main_udp_node = same` + * with a TCP-transport main node is the shape that slips through - the UI + * label reads "Same as main node" and nothing tells the user the consequence. + * The UDP is proxied, yes, but it is tunnelled over TCP, and QUIC over a TCP + * tunnel stalls: measured with both nodes on the SAME server and only the + * transport differing (anytls TCP:443 vs hysteria2 QUIC/UDP:50466), UDP came + * back at 174 ms +/- 1 ms through hysteria2 and produced a 568 ms spike with + * 37 ms jitter and outright loss through anytls. + * + * So the gate is now the transport, resolved by udp_transport_verdict() in + * homeproxy-pro.uc - one definition, shared with the diagnostic report so the log + * and the rule cannot disagree. + * + * NOTE the deliberate asymmetry with the tproxy chain below and in the UDP + * tproxy block: those still key off the plain "is there a UDP node" test, + * because they decide whether UDP is PROXIED at all. This gate decides + * whether QUIC is WORTH proxying. A TCP-transport UDP node still proxies + * games, voice and everything else; only QUIC is turned away, so the browser + * falls straight back to a TCP path that the same tunnel carries far better. */ +const udp_transport = udp_transport_verdict(cfgname, uci); +const udp_quic_rejected = !udp_transport.native; if (match(proxy_mode, /tproxy/)) if (outbound_udp_node !== 'nil' || routing_mode === 'custom') { @@ -1022,17 +1045,17 @@ chain homeproxy_mangle_tun { {% if (v6_handled): %} ip6 daddr != @homeproxy_gfw_list_v6 counter return {% endif /* v6_handled */ %} - {% if (!udp_proxyable): %} - udp dport { 80, 443 } counter reject comment "!{{ cfgname }}: Block QUIC (not proxyable)" - {% endif /* udp_proxyable */ %} + {% if (udp_quic_rejected): %} + udp dport { 80, 443 } counter reject comment "!{{ cfgname }}: Block QUIC (UDP path is not UDP-native)" + {% endif /* udp_quic_rejected */ %} {% elif (routing_mode === 'bypass_mainland_china' || bypass_cn_traffic === '1'): %} ip daddr @homeproxy_mainland_addr_v4 counter return {% if (v6_handled): %} ip6 daddr @homeproxy_mainland_addr_v6 counter return {% endif /* v6_handled */ %} - {% if (!udp_proxyable): %} - udp dport { 80, 443 } counter reject comment "!{{ cfgname }}: Block QUIC (not proxyable)" - {% endif /* udp_proxyable */ %} + {% if (udp_quic_rejected): %} + udp dport { 80, 443 } counter reject comment "!{{ cfgname }}: Block QUIC (UDP path is not UDP-native)" + {% endif /* udp_quic_rejected */ %} {% elif (routing_mode === 'proxy_mainland_china'): %} ip daddr != @homeproxy_mainland_addr_v4 counter return {% if (v6_handled): %} diff --git a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/generate_client.uc b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/generate_client.uc index 48c6df8b0..d235f10b3 100755 --- a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/generate_client.uc +++ b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/generate_client.uc @@ -30,7 +30,7 @@ import { lstat, mkdtemp, readfile, writefile } from 'fs'; import { Loader } from './config/loader.uc'; import { generate } from './generator/client.uc'; import { rule_set_tags } from './generator/common.uc'; -import { removeBlankAttrs, isEmpty, isValidCIDR, probeRuleSetFile, ruleSetFormatFromPath, shellQuote, validateRuleSetPath, validation, HP_DIR, RUN_DIR, UCICONFIG_DIR } from './homeproxy-pro.uc'; +import { removeBlankAttrs, isEmpty, isValidCIDR, probeRuleSetFile, ruleSetFormatFromPath, shellQuote, udp_transport_verdict, validateRuleSetPath, validation, HP_DIR, RUN_DIR, UCICONFIG_DIR } from './homeproxy-pro.uc'; /* Resolve the GenerationContext inputs. This is the only impure step on the * client generation path, and it is deliberately here rather than under @@ -129,6 +129,39 @@ function export_node_addresses(config) { dump(RUN_DIR + '/node-addr-domains.txt', domains); } +/* Export the UDP transport verdict for the shell runtime to read. + * + * runtime/health.sh wants to say out loud why QUIC was or was not rejected, + * and it is a shell script: it cannot call into homeproxy-pro.uc, because that + * module imports luci.http and a plain `ucode -e` outside rpcd does not + * resolve it. Re-implementing the judgement in shell would duplicate the + * UDP_NATIVE_PROTOCOLS set, and a duplicated set is a set that drifts - the + * firewall would reject QUIC on one list while the log explained another. + * + * So the verdict is computed once, here, and written out. The consumer is + * a one-line read, and a stale file only costs a stale log line. + * + * Format is one token per line, deliberately not prose: the shell reads the + * flag with `read`, and the reason is a single line safe to print as-is. */ +function export_udp_transport() { + const v = udp_transport_verdict('homeproxy-pro'); + + /* join(sep, list), not list.join(sep): ucode will not let an array literal + * be the receiver of a method call, and `['a'].join('\n')` fails to parse + * as "call join on the literal" - it raises "left-hand side is not a + * function" at the closing bracket. The dump() helper above already + * spells it the other way for the same reason. */ + const body = join('\n', [ + v.native ? '1' : '0', + v.configured ? '1' : '0', + v.protocol, + v.reason + ]); + + if (writefile(RUN_DIR + '/udp-transport.txt', body + '\n') == null) + warn(sprintf('homeproxy-pro: could not write %s\n', RUN_DIR + '/udp-transport.txt')); +} + function resolve_env(dm) { const routing_mode = dm.general.routing_mode || 'bypass_mainland_china'; const ubus = connect(); @@ -380,6 +413,7 @@ if (system('sing-box check --config ' + shellQuote(tmp)) !== 0) { * dnsmasq snippets read the exported list, and they must never be pointed at * addresses from a configuration that does not start. */ export_node_addresses(config); +export_udp_transport(); if (system('mv -f ' + shellQuote(tmp) + ' ' + shellQuote(RUN_DIR) + '/sing-box-c.json') !== 0) { system('rm -rf ' + shellQuote(work_dir)); diff --git a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/homeproxy-pro.uc b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/homeproxy-pro.uc index 6358551b2..5dae6ff15 100644 --- a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/homeproxy-pro.uc +++ b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/homeproxy-pro.uc @@ -5,6 +5,7 @@ */ import { access, lstat, mkdtemp, open, rmdir, unlink } from 'fs'; +import { cursor } from 'uci'; import { urldecode_params } from 'luci.http'; /* Global variables start */ @@ -701,6 +702,157 @@ export function strToTime(str) { return match(str, /[a-zA-Z]$/) ? str : (str + 's'); }; +/* Which node transports carry UDP as their own wire format. + * + * QUIC inside a tunnel whose own transport is TCP is the worst of both + * worlds: the QUIC layer retransmits on a schedule that assumes it owns the + * path, and the TCP layer underneath retransmits on a schedule that assumes + * nothing above it is doing that. Measured on a router where both candidate + * nodes were the SAME server and only the transport differed (anytls on + * TCP:443 vs hysteria2 on QUIC/UDP:50466), DNS-over-UDP through the tunnel + * came back at 174 ms +/- 1 ms through hysteria2 and produced a 568 ms spike + * with 37 ms jitter and outright loss through anytls. Same server, same + * path - the variable really was the transport, not the node. + * + * A set rather than a list so membership is an object lookup and an unknown + * protocol name is simply absent rather than accidentally matching. + * + * This is the TRANSPORT, not the sing-box build feature: the `with_quic` + * marker in htdocs/.../homeproxy-pro.js records whether the binary was compiled + * with QUIC support, which is a different question and the wrong thing to + * test here. */ +export const UDP_NATIVE_PROTOCOLS = { + 'hysteria': true, + 'hysteria2': true, + 'tuic': true, + 'wireguard': true +}; + +/* Decide whether the UDP leaving this router has a UDP-native path, and say + * why. Used by firewall_post.ut to decide whether QUIC has to be rejected, + * and by the diagnostic report to explain it - one definition, two + * consumers, because a second copy of this judgement is a copy that drifts. + * + * Note what this is NOT: a capability check. A TCP-transport node still + * proxies UDP perfectly well, sing-box will tunnel it. The question is only + * whether putting QUIC on that path is a good idea, so `native: false` means + * "QUIC would perform badly", never "UDP cannot be proxied". + * + * Returns { native, configured, protocol, reason }: + * native - true when the effective UDP path is UDP end to end + * configured - false when there is no UDP node at all (nothing to judge) + * protocol - the resolved node type, for the log + * reason - one sentence, safe to print + * + * WHY THIS READS UCI RATHER THAN TAKING A MODEL, given guard 7 keeps the + * semantic layers off the cursor: the three consumers of this judgement - + * the fw4 template, the RPC diagnostic report and (through the exported file) + * the shell runtime - do not share a data model. firewall_post.ut already + * reads main_node/main_udp_node straight from UCI for the tproxy chain, so + * this is not a new dependency in that file. The alternative - resolving it + * in generator/context.uc from the Loader's model and exporting it - would + * make the firewall depend on a value the generator produced, which is the + * coupling firewall_post.ut's own header warns against when it reads + * node-addr-ips.txt instead of re-deriving node selection. One function, + * three callers, no second opinion: that is the property worth protecting + * here. The uci module is available everywhere this runs (the CI toolchain + * builds ucode with -DUCI_SUPPORT=ON). + */ +export function udp_transport_verdict(cfgname, cursor_arg) { + /* The cursor is a parameter so a caller that already has one - the fw4 + * template and the RPC report both do - reuses it instead of opening a + * second view on the same config, and so the unit test can point this at + * a sandbox directory. Callers on the client path pass nothing and get + * the default confdir. */ + const uci = cursor_arg ?? cursor(); + uci.load(cfgname); + + const routing_mode = uci.get(cfgname, 'config', 'routing_mode') || 'bypass_mainland_china'; + + /* Custom routing mode is the user driving the rule graph by hand. They + * have already said what carries what and this function cannot read it, + * so it must not second-guess them - that is the same reasoning the QUIC + * gate used when it keyed on `routing_mode === 'custom'` alone. */ + if (routing_mode === 'custom') + return { + native: true, + configured: true, + protocol: 'custom', + reason: 'custom routing mode: the rule graph decides what carries UDP' + }; + + const udp_node = uci.get(cfgname, 'config', 'main_udp_node') || 'nil'; + const main_node = uci.get(cfgname, 'config', 'main_node') || 'nil'; + + const node_type = (sec) => (isEmpty(sec) || sec === 'nil') + ? null + : (uci.get(cfgname, sec, 'type') || null); + + /* `same` is the case this function was written for. It is also the + * silent one: the UI label reads "Same as main node", and nothing + * anywhere tells the user that a TCP-transport main node turns every UDP + * flow into a TCP tunnel. */ + if (udp_node === 'same') { + const type = node_type(main_node); + if (type === null) + return { + native: false, configured: false, protocol: 'nil', + reason: 'UDP follows the main node and no main node is set' + }; + + return (UDP_NATIVE_PROTOCOLS[type] === true) + ? { native: true, configured: true, protocol: type, + reason: sprintf('UDP follows the main node (%s), which is UDP-native', type) } + : { native: false, configured: true, protocol: type, + reason: sprintf('UDP follows the main node (%s), which is a TCP transport - QUIC would be tunnelled over TCP', type) }; + } + + /* The url-test group picks a member per connection, so "all of them" is + * the only answer that holds for every packet: a single TCP member is + * enough to put QUIC back on the stalling path whenever it happens to be + * the selected one. */ + if (udp_node === 'urltest') { + const members = uci.get(cfgname, 'config', 'main_udp_urltest_nodes'); + if (isEmpty(members)) + return { + native: false, configured: false, protocol: 'urltest', + reason: 'the UDP url-test group has no members' + }; + + let offender = null; + for (let sec in members) { + const type = node_type(sec); + if (UDP_NATIVE_PROTOCOLS[type] !== true && offender === null) + offender = type || 'unknown'; + } + + return (offender === null) + ? { native: true, configured: true, protocol: 'urltest', + reason: sprintf('every member of the UDP url-test group is UDP-native (%d members)', length(members)) } + : { native: false, configured: true, protocol: offender, + reason: sprintf('the UDP url-test group contains a TCP-transport member (%s) - QUIC would be tunnelled over TCP whenever it is selected', offender) }; + } + + if (udp_node === 'nil') + return { + native: false, configured: false, protocol: 'nil', + reason: 'UDP is disabled: no UDP node is configured' + }; + + const type = node_type(udp_node); + if (type === null) + return { + native: false, configured: false, protocol: 'nil', + reason: sprintf('the UDP node "%s" does not exist', udp_node) + }; + + return (UDP_NATIVE_PROTOCOLS[type] === true) + ? { native: true, configured: true, protocol: type, + reason: sprintf('the UDP node is %s, which is UDP-native', type) } + : { native: false, configured: true, protocol: type, + reason: sprintf('the UDP node is %s, which is a TCP transport - QUIC would be tunnelled over TCP', type) }; +}; + /* Turn a UCI list of port strings into the int array sing-box wants * (e.g. the WireGuard `reserved` list, a routing rule's `port` / * `source_port`). Returns null for anything that is not a non-empty diff --git a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/debug_report.uc b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/debug_report.uc index c43b15eb2..c96c9fd8b 100644 --- a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/debug_report.uc +++ b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/debug_report.uc @@ -43,7 +43,7 @@ import { popen } from 'fs'; -import { HP_DIR, RUN_DIR, shellQuote } from '../homeproxy-pro.uc'; +import { HP_DIR, RUN_DIR, shellQuote, udp_transport_verdict } from '../homeproxy-pro.uc'; export const DEBUG_REPORT_PATH = RUN_DIR + '/debug.log'; @@ -274,6 +274,18 @@ function configSummary(uci) { (v == null || v === '') ? '_(unset)_' : (isSecretKey(sw[1]) ? maskValue(v) : v)); } + /* The QUIC verdict, because it is the one conclusion in this report that + * is derived rather than reported: the switches table shows + * `main_udp_node`, and `main_udp_node` on its own does not say whether + * QUIC was rejected by the firewall or why. udp_transport_verdict() is + * the same function firewall_post.ut gates the nft rule with, so this + * line describes the rule that is actually loaded rather than a second + * opinion about it. */ + const udp = udp_transport_verdict('homeproxy-pro', uci); + out += sprintf('\n- UDP path: **%s** - %s\n', + udp.native ? 'UDP-native' : (udp.configured ? 'not UDP-native' : 'UDP not configured'), + udp.reason); + return out; }; diff --git a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/health.sh b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/health.sh index aef4d5cc7..a975b1135 100644 --- a/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/health.sh +++ b/luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/health.sh @@ -339,6 +339,47 @@ hp_probe_tcp() { return 1 } +# hp_report_udp_transport +# Say out loud, on every start, whether QUIC is being rejected and why. +# +# This is a shell script and the judgement lives in ucode (udp_transport_verdict +# in homeproxy-pro.uc), which this file cannot call into: homeproxy-pro.uc imports +# luci.http, which a plain `ucode -e` outside rpcd does not resolve. So the +# generator computes it once and drops it in RUN_DIR, and this reads that file. +# Re-implementing the rule here instead would duplicate the protocol set, and +# the log would then explain a different decision from the one nft actually has. +# +# The file is written by generate_client.uc at every generation. A missing +# file means generation has not run yet, which is not this function's problem +# to report - it stays silent rather than guessing. +hp_report_udp_transport() { + # Same inline default as runtime/dns.sh uses for its run-dir arguments: + # this file is sourced by init.d/homeproxy-pro but is also exercised by + # tests/runtime/ without it. + local run_dir="${HP_RUN_DIR:-/var/run/homeproxy-pro}" + local f="${run_dir}/udp-transport.txt" + local native configured proto reason + + [ -r "$f" ] || return 0 + + { + read -r native + read -r configured + read -r proto + read -r reason + } < "$f" + + [ -n "$native" ] || return 0 + + if [ "$native" = "1" ]; then + log "UDP path is UDP-native (${proto}): QUIC is proxied as-is." + elif [ "$configured" = "1" ]; then + log "Warning: ${reason}. QUIC on UDP 80/443 is rejected so browsers fall back to TCP; every other UDP flow is still proxied." + else + log "UDP path: ${reason}. QUIC on UDP 80/443 is rejected." + fi +} + # hp_run_probes