diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c530d09..4a49406 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -93,6 +93,8 @@ jobs: key: vtun-${{ hashFiles('server/build-vtund.sh') }} - name: Build and run the unit tests under qemu + env: + GITHUB_TOKEN: ${{ github.token }} # toolchain digest lookup (ci/build-musl.sh) run: QEMU=1 ci/build-musl.sh ${{ matrix.target }} dist - name: Upload binaries diff --git a/CHANGELOG.md b/CHANGELOG.md index f3887ec..a464d84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,27 @@ The version lives in `include/vtesse/version.h`. Binaries are published only as [`latest`](https://github.com/compot-dev/vtesse/releases/tag/latest) release, rebuilt from every push to `main`; this file is the change history. +## 1.12 — 2026-10-15 + +Fixes from the first Qodo review of the whole codebase (#34, PR #35): + +- lwIP backend: a DHCP renewal replaces the classless routes (or removes them) even + though the address stays the same; before, only the first lease set them. +- The keepalive timeout is refreshed only by complete frames: a peer that trickles + bytes of a frame it never finishes no longer keeps a dead session alive. +- `vt_create()` rejects negative timer settings; the reconnect backoff cannot overflow. +- SOCKS5: an unfinished handshake is dropped after the connect timeout (15 s), so idle + clients cannot hold every connection slot. +- `vtesse-cli`: `--seconds` and the `--fetch` port are checked (`--seconds 1m` used to + mean "no limit"); the `stop` hook runs only after earlier hooks finished, so a DHCP + client started by a late `up` hook is not left running. +- `vtesse-udhcpc.script` installs on-link classless routes (gateway 0.0.0.0) without `via`. +- `S99vtesse` stop/status check the process (`-x`), so a stale PID file cannot make + them signal or report another process. +- CI: unit-test failures under QEMU fail the build (`| tail -1` hid them); toolchain + archives are checked against SHA256 before their compiler runs; the bench self-test + checks that ECHO_REQ really is the first frame. + ## 1.11 — 2026-10-15 - The Android APK of every new version is posted to the OpenIPC Telegram dev channel diff --git a/bench/selftest.py b/bench/selftest.py index 85a21a9..fadfe5e 100755 --- a/bench/selftest.py +++ b/bench/selftest.py @@ -91,6 +91,24 @@ def wait_flag(s, want, seconds): return False +def first_frame_flag(s, seconds): + """Return the flag of the first frame (0 for a data frame), or None on timeout/EOF.""" + s.settimeout(seconds) + buf = b"" + try: + while len(buf) < 2: + c = s.recv(2 - len(buf)) + if not c: + return None + buf += c + except socket.timeout: + return None + hdr = struct.unpack(">H", buf)[0] + if hdr & 0xF000 == ECHO_REQ: + s.sendall(struct.pack(">H", ECHO_REP)) + return hdr & 0xF000 + + def dhcp_offer(s, mac, seconds): """Send DHCPDISCOVER, return (seconds until OFFER, raw frame) or (None, None).""" t0 = time.time() @@ -129,7 +147,7 @@ def main(): check("greeting is exactly 50 bytes, NUL padded", len(g[0]) == 50 and g[0].startswith(b"VTUN server ver ") and g[0].rstrip(b"\0").endswith(b"\n")) check("main session flags ", reply == "OK FLAGS: \n", repr(reply)) - check("ECHO_REQ is the first frame after OK FLAGS", wait_flag(s, ECHO_REQ, 2)) + check("ECHO_REQ is the first frame after OK FLAGS", first_frame_flag(s, 2) == ECHO_REQ) time.sleep(0.5) # the server brings the tap up asynchronously dt, fr = dhcp_offer(s, bytes.fromhex(S["main"][0]), 10) check("DHCP OFFER through the tunnel", fr is not None, "after %.1fs" % dt if dt else "") diff --git a/ci/Dockerfile.cross b/ci/Dockerfile.cross index 3ef8378..bf63bbc 100644 --- a/ci/Dockerfile.cross +++ b/ci/Dockerfile.cross @@ -1,6 +1,6 @@ # Cross build environment for ci/build-musl.sh (Bootlin musl toolchains + qemu-user). FROM debian:bookworm-slim RUN apt-get update \ - && apt-get install -y --no-install-recommends make curl ca-certificates xz-utils file qemu-user \ + && apt-get install -y --no-install-recommends make curl ca-certificates xz-utils file qemu-user jq \ && rm -rf /var/lib/apt/lists/* WORKDIR /src diff --git a/ci/build-musl.sh b/ci/build-musl.sh index 00e276c..a31ac82 100755 --- a/ci/build-musl.sh +++ b/ci/build-musl.sh @@ -20,6 +20,10 @@ set -eu TARGET=$1 OUT=${2:-dist} BOOTLIN_VERSION=${BOOTLIN_VERSION:-stable-2026.08-1} +# The archives are checked before their compiler is run: SHA256 pinned below for the +# default Bootlin and OpenWrt versions (TOOLCHAIN_SHA256 overrides it, required with +# another version); the OpenIPC toolchain is a rolling release asset, checked +# against the digest GitHub reports for it. CACHE=${TOOLCHAIN_CACHE:-$HOME/.cache/vtesse-toolchains} ROOT=$(cd "$(dirname "$0")/.." && pwd) @@ -29,23 +33,32 @@ OPENWRT_VERSION=${OPENWRT_VERSION:-23.05.5} OPENWRT=https://downloads.openwrt.org/releases/$OPENWRT_VERSION/targets SUBDIR= # toolchain directory inside the archive, if not its top level case "$TARGET" in - arm32) URL=$OPENIPC/toolchain.hisilicon-hi3516cv100.tgz; DIRNAME=arm-openipc-linux-musleabi_sdk-buildroot; QEMU_BIN=qemu-arm ;; + arm32) URL=$OPENIPC/toolchain.hisilicon-hi3516cv100.tgz; DIRNAME=arm-openipc-linux-musleabi_sdk-buildroot; QEMU_BIN=qemu-arm + SHA=github ;; mipsel|mips) # OpenWrt toolchains: musl, soft-float (24kc), as on the routers themselves if [ "$TARGET" = mipsel ]; then OW=ramips/mt7621; OWA=mipsel_24kc; QEMU_BIN=qemu-mipsel - else OW=ath79/generic; OWA=mips_24kc; QEMU_BIN=qemu-mips; fi + SHA=bb08478a46869009a2a8b85f88e266cc6b6d3ccefc34de61907ead2a986e08c9 + else OW=ath79/generic; OWA=mips_24kc; QEMU_BIN=qemu-mips + SHA=6cf197b2705307057548e65158396ca4d26b343f557bf81c97fd5643978cc991; fi + [ "$OPENWRT_VERSION" = 23.05.5 ] || SHA= DIRNAME=openwrt-toolchain-$OPENWRT_VERSION-$(echo $OW | tr / -)_gcc-12.3.0_musl.Linux-x86_64 URL=$OPENWRT/$OW/$DIRNAME.tar.xz SUBDIR=/toolchain-${OWA}_gcc-12.3.0_musl ;; *) case "$TARGET" in - x86_64) TC=x86-64; QEMU_BIN=qemu-x86_64 ;; - armv5) TC=armv5-eabi; QEMU_BIN=qemu-arm ;; - armv7) TC=armv7-eabihf; QEMU_BIN=qemu-arm ;; - arm64) TC=aarch64; QEMU_BIN=qemu-aarch64 ;; + x86_64) TC=x86-64; QEMU_BIN=qemu-x86_64 + SHA=78d3a4683d6ac47b5ee73bd5bce210b55eb93dff1b137c61298af97eb0d2b5a6 ;; + armv5) TC=armv5-eabi; QEMU_BIN=qemu-arm + SHA=a7db571eaa0f1ba2eea9a3932e8ad64bfaf33bfb7e7b96dd09702fe3ecedcf1a ;; + armv7) TC=armv7-eabihf; QEMU_BIN=qemu-arm + SHA=9147bafae4aa272321a3c6440d04d83b7e23411b2d344f875541d84c4444ba9b ;; + arm64) TC=aarch64; QEMU_BIN=qemu-aarch64 + SHA=b388c480a48e8e9f9b99e3d14e69219c4d61e5a2424a82faecb88a015b781a60 ;; *) echo "unknown target: $TARGET" >&2; exit 2 ;; esac DIRNAME=$TC--musl--$BOOTLIN_VERSION + [ "$BOOTLIN_VERSION" = stable-2026.08-1 ] || SHA= URL=$BOOTLIN/$TC/tarballs/$DIRNAME.tar.xz ;; esac @@ -56,6 +69,20 @@ if [ ! -d "$DIR/bin" ]; then # download first (with retries), then extract: a truncated stream piped # into tar looks like a corrupt archive curl -fsSL --retry 3 -o "$CACHE/toolchain.tar" "$URL" + SHA=${TOOLCHAIN_SHA256:-$SHA} + if [ "$SHA" = github ]; then + # authenticated when a token is available: anonymous calls share a 60/hour limit per IP + command -v jq >/dev/null || + { echo "jq is needed to verify $URL (or set TOOLCHAIN_SHA256)" >&2; rm -f "$CACHE/toolchain.tar"; exit 1; } + TOKEN=${GITHUB_TOKEN:-${GH_TOKEN:-}} + REL=$(curl -fsSL --retry 3 ${TOKEN:+-H "Authorization: Bearer $TOKEN"} \ + "https://api.github.com/repos/OpenIPC/firmware/releases/tags/toolchain") || + { echo "GitHub API request for the toolchain digest failed (rate limit?)" >&2; rm -f "$CACHE/toolchain.tar"; exit 1; } + SHA=$(printf '%s' "$REL" | jq -r --arg n "${URL##*/}" '.assets[] | select(.name == $n) | .digest // "" | sub("^sha256:"; "")') + fi + [ -n "$SHA" ] || { echo "no SHA256 for $URL: set TOOLCHAIN_SHA256" >&2; rm -f "$CACHE/toolchain.tar"; exit 1; } + echo "$SHA $CACHE/toolchain.tar" | sha256sum -c - >/dev/null || + { echo "SHA256 mismatch for $URL" >&2; rm -f "$CACHE/toolchain.tar"; exit 1; } tar -xf "$CACHE/toolchain.tar" -C "$CACHE" rm -f "$CACHE/toolchain.tar" [ -x "$DIR/relocate-sdk.sh" ] && "$DIR/relocate-sdk.sh" >/dev/null 2>&1 || true @@ -94,7 +121,9 @@ fi if [ "${QEMU:-0}" = 1 ]; then for t in test_proto test_route test_client; do echo "== $QEMU_BIN $t" - "$QEMU_BIN" "build-$TARGET-full/$t" | tail -1 + # no pipefail in sh: keep the test's own exit status, not that of tail + out=$("$QEMU_BIN" "build-$TARGET-full/$t" 2>&1) || { echo "$out"; echo "$t failed under $QEMU_BIN" >&2; exit 1; } + echo "$out" | tail -1 done "$QEMU_BIN" "$OUT/vtesse-$TARGET" --version "$QEMU_BIN" "$OUT/vtesse-tap-$TARGET" --version diff --git a/cli/vtesse_cli.c b/cli/vtesse_cli.c index aa419ca..1e12240 100644 --- a/cli/vtesse_cli.c +++ b/cli/vtesse_cli.c @@ -14,6 +14,7 @@ */ #include #include +#include #include #include #include @@ -315,7 +316,11 @@ static int parse_url(const char *url) fetch.port = 80; colon = strchr(host, ':'); if (colon) { - fetch.port = (uint16_t)atoi(colon + 1); + char *end; + long port = strtol(colon + 1, &end, 10); + if (end == colon + 1 || *end || port < 1 || port > 65535) + return -1; + fetch.port = (uint16_t)port; host[colon - host] = '\0'; } snprintf(fetch.path, sizeof(fetch.path), "%s", slash ? slash : "/"); @@ -515,16 +520,78 @@ static void reap_children(void) ; } +/* Hooks still running, by PID: as PID 1 (in a container) we also inherit the DHCP + * client the hook daemonized, so waiting for "any child" would never return. */ +static pid_t hook_pids[16]; + +/* 1: the hook has exited (or was already reaped), 0: still running. EINTR is + * retried, so an interrupted check never counts as an exit. */ +static int hook_done(pid_t pid) +{ + pid_t r; + + while ((r = waitpid(pid, NULL, WNOHANG)) < 0 && errno == EINTR) + ; + return r != 0; /* pid, or -1 with ECHILD */ +} + +/* Waits for the hooks started so far: "stop" must not overtake an earlier "up" that + * has yet to start the DHCP client (it would be left running after we exit). A hook + * still running after HOOK_WAIT_MS is terminated, so shutdown cannot hang on it. */ +#define HOOK_WAIT_MS 5000 + +static void wait_hooks(void) +{ + uint64_t deadline = nc_now_ms() + HOOK_WAIT_MS; + size_t i; + + for (i = 0; i < sizeof(hook_pids) / sizeof(hook_pids[0]); i++) { + while (hook_pids[i] > 0) { + if (hook_done(hook_pids[i])) { + hook_pids[i] = 0; /* finished, or already reaped */ + } else if (nc_now_ms() >= deadline) { + uint64_t grace = nc_now_ms() + 1000; + printf("[%8.3f] hook: pid %d still running, terminated\n", elapsed(t_start), + (int)hook_pids[i]); + /* the whole group: also a udhcpc still in the hook's foreground */ + kill(-hook_pids[i], SIGTERM); + while (!hook_done(hook_pids[i]) && nc_now_ms() < grace) { + struct timespec ts = { 0, 20 * 1000000L }; + nanosleep(&ts, NULL); + } + kill(-hook_pids[i], SIGKILL); + while (waitpid(hook_pids[i], NULL, 0) < 0 && errno == EINTR) + ; + hook_pids[i] = 0; + } else { + struct timespec ts = { 0, 20 * 1000000L }; + nanosleep(&ts, NULL); + } + } + } +} + /* Runs "$hook " asynchronously with VTESSE_* in the environment. */ static void run_hook(const char *event) { pid_t pid; + size_t slot; reap_children(); if (!hook) return; + /* a free slot first: every hook we start must be one shutdown can wait for */ + for (slot = 0; slot < sizeof(hook_pids) / sizeof(hook_pids[0]); slot++) + if (hook_pids[slot] <= 0 || hook_done(hook_pids[slot])) + break; + if (slot == sizeof(hook_pids) / sizeof(hook_pids[0])) { + printf("[%8.3f] hook: too many hooks still running, \"%s\" not started\n", + elapsed(t_start), event); + return; + } pid = fork(); if (pid == 0) { + setpgid(0, 0); /* own process group, so a stuck hook can be ended as a whole */ setenv("VTESSE_EVENT", event, 1); setenv("VTESSE_IFNAME", nc_tap_ifname(tap), 1); setenv("VTESSE_LINK", nc_tap_link(tap) == TUNNEL_LINK_L3 ? "tun" : "ether", 1); @@ -534,6 +601,10 @@ static void run_hook(const char *event) } if (pid < 0) printf("[%8.3f] hook: fork failed: %s\n", elapsed(t_start), strerror(errno)); + else { + setpgid(pid, pid); /* also here: no race with a kill() before the child ran */ + hook_pids[slot] = pid; + } } static void on_tap_event(void *user, int ev, int tev, int err) @@ -598,8 +669,16 @@ int main(int argc, char **argv) for (i = 1; i < argc; i++) { if (!strcmp(argv[i], "--config") && i + 1 < argc) config = argv[++i]; - else if (!strcmp(argv[i], "--seconds") && i + 1 < argc) - seconds = atof(argv[++i]); + else if (!strcmp(argv[i], "--seconds") && i + 1 < argc) { + const char *arg = argv[++i]; + char *end; + seconds = strtod(arg, &end); + /* "1m" or "-5" must not mean "forever"; inf or 1e30 would overflow the deadline */ + if (end == arg || *end || !(seconds > 0) || !(seconds <= 1e9)) { + fprintf(stderr, "--seconds: expected a positive number of seconds up to 1e9, got '%s'\n", arg); + return 2; + } + } else if (!strcmp(argv[i], "-v")) verbose = 1; else if (!strcmp(argv[i], "--id-from") && i + 1 < argc) @@ -889,9 +968,10 @@ int main(int argc, char **argv) nc_tap_ifname(tap), ts.frames_in, ts.frames_out, ts.dropped); if (nc_tap_link(tap) == TUNNEL_LINK_NONE && vt_is_up(vt)) rc = 1; /* interface creation failed */ + wait_hooks(); run_hook("stop"); + wait_hooks(); nc_tap_destroy(tap); - reap_children(); } #endif t.ops->close(vt); diff --git a/include/vtesse/version.h b/include/vtesse/version.h index bf444d1..40064cf 100644 --- a/include/vtesse/version.h +++ b/include/vtesse/version.h @@ -3,6 +3,6 @@ /* MAJOR.MM. Shown by --version and in the title of the "latest" release (CI); * record each bump in CHANGELOG.md. */ -#define VTESSE_VERSION "1.11" +#define VTESSE_VERSION "1.12" #endif diff --git a/netcore/proxy.c b/netcore/proxy.c index b9c5cab..15b265e 100644 --- a/netcore/proxy.c +++ b/netcore/proxy.c @@ -549,6 +549,8 @@ static void accept_on(nc_proxy_t *px, int which) if (which == 0) { c->socks = 1; c->state = C_GREETING; + /* the handshake gets the connect timeout too: idle clients must not hold slots */ + c->deadline = nc_now_ms() + (uint64_t)px->cfg.connect_timeout_ms; } else { const nc_forward_t *f = &px->cfg.forwards[which - 1]; c->dst = f->ip; @@ -639,7 +641,7 @@ static int px_next_timeout(void *ctx) for (i = 0; i < px->nconns; i++) { const conn_t *c = px->conns[i]; - if (c->state == C_CONNECTING && c->deadline && (!next || c->deadline < next)) + if (c->state <= C_CONNECTING && !c->dead && c->deadline && (!next || c->deadline < next)) next = c->deadline; } if (!next) @@ -659,6 +661,10 @@ static void px_timer(void *ctx) struct tcp_pcb *pcb = c->pcb; c->deadline = 0; tcp_abort(pcb); /* calls on_err(ERR_ABRT) -> "connect timeout" */ + } else if (c->state < C_CONNECTING && !c->dead && c->deadline && now >= c->deadline) { + logf_(px, "socks: handshake timeout"); + c->deadline = 0; + c->dead = 1; } } service(px); diff --git a/netcore/stack.c b/netcore/stack.c index 7dab90a..975d5a5 100644 --- a/netcore/stack.c +++ b/netcore/stack.c @@ -35,6 +35,8 @@ static struct { nc_routes_t pending; /* from the last DHCPACK */ int pending_valid; /* 121 (2) or 249 (1) seen, 0 none */ u32_t pending_xid; + int lease_used; /* dhcp->lease_used seen last: it restarts at 0 on every ACK */ + int dhcp_state; /* dhcp->state seen last */ uint8_t txbuf[FRAME_MAX]; } st; @@ -116,9 +118,13 @@ void nc_hook_dhcp_parse_option(struct netif *netif, struct dhcp *dhcp, u8_t stat int prio = option == 121 ? 2 : option == 249 ? 1 : 0; (void)dhcp; - (void)state; if (netif != &st.netif || !prio || msg_type != DHCP_ACK) return; + /* lwIP parses every reply but accepts an ACK only in these states: the options of + * an ACK it ignores (e.g. a duplicate while BOUND) must not reach the routes */ + if (state != DHCP_STATE_REQUESTING && state != DHCP_STATE_REBOOTING && + state != DHCP_STATE_RENEWING && state != DHCP_STATE_REBINDING) + return; if (pbuf_copy_partial(p, buf, len, offset) != len) return; if (nc_routes_parse_classless(buf, len, &rt) != 0) @@ -207,21 +213,47 @@ static err_t netif_init_cb(struct netif *n) return ERR_OK; } +/* Routes of the DHCPACK that made the lease current: option 121/249 if it had them. */ +static void apply_dhcp_routes(const struct dhcp *d) +{ + int have121 = st.pending_valid && d && st.pending_xid == d->xid; + nc_routes_t none; + + memset(&none, 0, sizeof(none)); + /* RFC 3442: with classless routes the router option is ignored */ + apply_routes(have121 ? &st.pending : &none, !have121 || st.cfg.opt3_default_route); + st.pending_valid = 0; + st.lease_used = d ? d->lease_used : 0; + st.dhcp_state = DHCP_STATE_BOUND; +} + +/* A renewal or a reboot after a reconnect keeps the address, so status_cb is not + * called, and the DHCPREQUEST reuses the xid. A new ACK shows as a return to BOUND + * (from RENEWING, REBINDING or REBOOTING), or, if no check saw the state in between, + * as lease_used going back: dhcp_bind() restarts it at 0 on every ACK. Its routes + * replace the old ones (none if it had none). */ +static void check_renewal(void) +{ + const struct dhcp *d = netif_dhcp_data(&st.netif); + int prev = st.dhcp_state; + + if (!d || st.cfg.static_ip) + return; + st.dhcp_state = d->state; + if (!st.bound || d->state != DHCP_STATE_BOUND) + return; + if (prev != DHCP_STATE_BOUND || d->lease_used < st.lease_used) + apply_dhcp_routes(d); + st.lease_used = d->lease_used; +} + static void status_cb(struct netif *n) { int has_addr = netif_is_up(n) && !ip4_addr_isany_val(*netif_ip4_addr(n)); if (has_addr && !st.bound) { - if (!st.cfg.static_ip) { - struct dhcp *d = netif_dhcp_data(n); - int have121 = st.pending_valid && d && st.pending_xid == d->xid; - nc_routes_t none; - - memset(&none, 0, sizeof(none)); - /* RFC 3442: with classless routes the router option is ignored */ - apply_routes(have121 ? &st.pending : &none, !have121 || st.cfg.opt3_default_route); - st.pending_valid = 0; - } + if (!st.cfg.static_ip) + apply_dhcp_routes(netif_dhcp_data(n)); st.bound = 1; emit(NC_STACK_EV_ADDR_BOUND, 0, 0); } else if (!has_addr && st.bound) { @@ -258,6 +290,7 @@ static void on_rx(void *user, const uint8_t *pkt, size_t len) pbuf_take(p, pkt, (u16_t)len); if (st.netif.input(p, &st.netif) != ERR_OK) pbuf_free(p); + check_renewal(); } static void on_tunnel_event(void *user, int ev, int err) @@ -311,6 +344,7 @@ static void timer_run(void *ctx) { (void)ctx; sys_check_timeouts(); + check_renewal(); } nc_source_t nc_stack_timer_source(void) diff --git a/scripts/S99vtesse b/scripts/S99vtesse index 8e1134e..7770c0f 100755 --- a/scripts/S99vtesse +++ b/scripts/S99vtesse @@ -24,7 +24,8 @@ start() { stop() { printf "Stopping vtesse: " - start-stop-daemon -K -q -p "$PIDFILE" && echo "OK" || echo "not running" + # -x: a stale PID file must not make us signal another process that reused the PID + start-stop-daemon -K -q -p "$PIDFILE" -x "$DAEMON" && echo "OK" || echo "not running" rm -f "$PIDFILE" } @@ -33,7 +34,7 @@ case "$1" in stop) stop ;; restart) stop; sleep 1; start ;; status) - if [ -f "$PIDFILE" ] && kill -0 "$(cat "$PIDFILE")" 2>/dev/null; then + if [ -f "$PIDFILE" ] && start-stop-daemon -K -t -q -p "$PIDFILE" -x "$DAEMON"; then echo "vtesse is running (pid $(cat "$PIDFILE"))" else echo "vtesse is not running" diff --git a/scripts/vtesse-udhcpc.script b/scripts/vtesse-udhcpc.script index 1d8c5fc..07cf4b8 100755 --- a/scripts/vtesse-udhcpc.script +++ b/scripts/vtesse-udhcpc.script @@ -17,7 +17,11 @@ STATE=/var/run/vtesse-routes.$interface routes_del() { [ -f "$STATE" ] || return 0 while read -r dst gw; do - ip route del "$dst" via "$gw" dev "$interface" 2>/dev/null + if [ "$gw" = 0.0.0.0 ]; then + ip route del "$dst" dev "$interface" 2>/dev/null + else + ip route del "$dst" via "$gw" dev "$interface" 2>/dev/null + fi done < "$STATE" rm -f "$STATE" } @@ -28,7 +32,11 @@ routes_add() { if [ -n "$list" ]; then set -- $list while [ $# -ge 2 ]; do - ip route add "$1" via "$2" dev "$interface" && echo "$1 $2" >> "$STATE" + if [ "$2" = 0.0.0.0 ]; then # on-link destination (RFC 3442) + ip route add "$1" dev "$interface" && echo "$1 $2" >> "$STATE" + else + ip route add "$1" via "$2" dev "$interface" && echo "$1 $2" >> "$STATE" + fi shift 2 done elif [ -n "$router" ] && [ "$VTESSE_DEFAULT_ROUTE" = 1 ]; then diff --git a/tests/test_client.c b/tests/test_client.c index bf3c540..70efe3b 100644 --- a/tests/test_client.c +++ b/tests/test_client.c @@ -445,6 +445,38 @@ static void test_keepalive_timeout(void) server_status(pid); } +static void script_partial_frame(int lfd) +{ + int fd = accept(lfd, NULL, NULL); + uint8_t hdr[2] = { 0x00, 100 }; /* a 100-byte data frame that never completes */ + int i; + + srv_handshake(fd, "OK FLAGS: \n"); + srv_write(fd, hdr, sizeof(hdr)); + for (i = 0; i < 20; i++) { /* one byte every 100 ms, no ECHO_REP */ + sleep_ms(100); + srv_write(fd, "x", 1); + } + close(fd); +} + +/* Bytes of an unfinished frame are not a sign of life (Qodo on #35). */ +static void test_partial_frame_timeout(void) +{ + uint16_t port; + pid_t pid = start_server(script_partial_frame, &port); + rec_t r; + vt_t *vt = make_client(port, &r, 1); + + vt_start(vt); + CHECK(run_until(vt, pred_up, &r, 2000)); + CHECK(run_until(vt, pred_failed, &r, 1500)); + CHECK_EQ_INT(r.errs[r.nev - 1], TUNNEL_ERR_KEEPALIVE); + vt_destroy(vt); + kill(pid, SIGTERM); + server_status(pid); +} + static void script_reconnect(int lfd) { int fd = accept(lfd, NULL, NULL); @@ -544,6 +576,16 @@ static void test_config_validation(void) cfg.password = ""; CHECK(vt_create(&cfg, &err) == NULL); CHECK_EQ_INT(err, TUNNEL_ERR_CONFIG); + /* negative tuning values would wrap in the deadline arithmetic */ + cfg.password = "x"; + cfg.auth = VT_AUTH_XOR; + cfg.ka_interval_ms = -1; + CHECK(vt_create(&cfg, &err) == NULL); + CHECK_EQ_INT(err, TUNNEL_ERR_CONFIG); + cfg.ka_interval_ms = 0; + cfg.reconnect_max_ms = -5; + CHECK(vt_create(&cfg, &err) == NULL); + CHECK_EQ_INT(err, TUNNEL_ERR_CONFIG); } int main(void) @@ -558,6 +600,7 @@ int main(void) RUN(test_bad_greeting); RUN(test_handshake_timeout); RUN(test_keepalive_timeout); + RUN(test_partial_frame_timeout); RUN(test_reconnect); return UNIT_SUMMARY(); } diff --git a/vtun/vtun_client.c b/vtun/vtun_client.c index 7e68b6d..d31adf6 100644 --- a/vtun/vtun_client.c +++ b/vtun/vtun_client.c @@ -119,9 +119,10 @@ static void fail(vt_t *vt, int err) vt->backoff_ms = vt->cfg.reconnect_min_ms; vt->state = ST_WAIT_RECONNECT; vt->reconnect_at = now + (uint64_t)vt->backoff_ms; - vt->backoff_ms *= 2; - if (vt->backoff_ms > vt->cfg.reconnect_max_ms) + if (vt->backoff_ms > vt->cfg.reconnect_max_ms / 2) /* no int overflow when doubling */ vt->backoff_ms = vt->cfg.reconnect_max_ms; + else + vt->backoff_ms *= 2; emit(vt, TUNNEL_EV_DOWN, err); } @@ -296,6 +297,7 @@ static void handle_mesg(vt_t *vt, char *m) static size_t handle_frames(vt_t *vt, const uint8_t *p, size_t len) { size_t used = 0; + int progress = 0; /* a complete frame: bytes of an unfinished one do not keep the peer alive */ while (vt->state == ST_ESTABLISHED) { uint16_t hdr, flen, fl; @@ -306,6 +308,7 @@ static size_t handle_frames(vt_t *vt, const uint8_t *p, size_t len) vt->skip -= n; if (vt->skip) break; + progress = 1; continue; } if (len - used < 2) @@ -315,6 +318,7 @@ static size_t handle_frames(vt_t *vt, const uint8_t *p, size_t len) flen = hdr & VT_FSIZE_MASK; if (fl) { used += 2; + progress = 1; if (fl & VT_CONN_CLOSE) { fail(vt, TUNNEL_ERR_PEER_CLOSED); break; @@ -333,7 +337,10 @@ static size_t handle_frames(vt_t *vt, const uint8_t *p, size_t len) if (flen && vt->rx_cb) vt->rx_cb(vt->rx_user, p + used + 2, flen); used += 2u + flen; + progress = 1; } + if (progress) + vt->last_rx = nc_now_ms(); return used; } @@ -421,7 +428,6 @@ static void op_on_readable(void *ctx) return; } vt->in_len += (size_t)n; - vt->last_rx = nc_now_ms(); process_input(vt); if (vt->fd < 0) return; @@ -594,6 +600,11 @@ vt_t *vt_create(const vt_cfg_t *cfg, int *err) !vt_session_name_valid(cfg->session) || !cfg->password || !*cfg->password || cfg->auth != VT_AUTH_XOR) return NULL; + /* 0 means the default; negative values would wrap in the unsigned deadline arithmetic */ + if (cfg->handshake_timeout_ms < 0 || cfg->ka_interval_ms < 0 || cfg->ka_count < 0 || + cfg->data_delay_ms < 0 || cfg->reconnect_min_ms < 0 || cfg->reconnect_max_ms < 0 || + cfg->out_buffer_size < 0) + return NULL; *err = TUNNEL_ERR_NOMEM; vt = calloc(1, sizeof(*vt));