diff --git a/overlays/default.nix b/overlays/default.nix index 114df6b..000bd19 100644 --- a/overlays/default.nix +++ b/overlays/default.nix @@ -15,6 +15,7 @@ }; }; + patches = import ./patches; test-exclusions = import ./test-exclusions.nix; x86-64-v3-workarounds = import ./x86-64-v3-workarounds.nix; diff --git a/overlays/patches/default.nix b/overlays/patches/default.nix new file mode 100644 index 0000000..1ffc6e6 --- /dev/null +++ b/overlays/patches/default.nix @@ -0,0 +1,3 @@ +_final: prev: { + gnutls = import ./gnutls { inherit (prev) gnutls; }; +} diff --git a/overlays/patches/gnutls/README.md b/overlays/patches/gnutls/README.md new file mode 100644 index 0000000..c286028 --- /dev/null +++ b/overlays/patches/gnutls/README.md @@ -0,0 +1,160 @@ +# GnuTLS UDP server readiness + +Under load, the test client can start before `gnutls-serv` binds its UDP +socket, and the first handshake fails with `Connection refused`. +`serv-udp.sh` currently waits a fixed four seconds; elapsed time does not +establish server readiness. `udp-server-readiness.patch` replaces that wait +with polling for the local IPv4 UDP endpoint. + +## Scope and waiting behavior + +The patch changes the existing `wait_udp_server()` and adds a new +`check_if_udp_port_bound()` beside it in `tests/scripts/common.sh`. +`serv-udp.sh` is its only caller in 3.8.13. The TCP helpers `wait_server()` +and `wait_for_port()`, including their existing sleeps, are unchanged. +Both original DTLS handshake checks remain unchanged. + +Each iteration checks process liveness and the socket **before sleeping**. +A ready socket returns immediately. An unsuccessful check sleeps two +seconds only if another attempt remains: at most 90 attempts, consistent +with the existing `wait_server()` budget implemented by `wait_for_port()`, +with no sleep after the final check. Server exit fails early; exhausting the +budget fails and terminates the server. No handshake is retried, and no +protocol timeout is changed. Once bound, the kernel can queue datagrams +while the server is scheduled; the probe itself sends no packets. + +The existing `have_port_finder()` prefers `ss`, then `netstat`. If neither +exists, it prints `neither ss nor netstat found` and exits **77 (skip)**. +In the normal test flow, port selection calls it before launching a server. +The probe runs in a subshell so that, even if this skip occurs after launch, +the waiting helper can terminate and reap the server before exiting 77. + +## Why an IPv4 socket is expected + +This is specific to the server used by this test, not a general rule that +IPv6 sockets cannot serve IPv4 clients. The client explicitly uses +`127.0.0.1`. The server's `--udp` path calls `udp_server()`, which calls +`listen_socket(..., SOCK_DGRAM)`. That function iterates the wildcard +addresses returned by `getaddrinfo(NULL, ..., AI_PASSIVE)`: + +| Server build / Linux setting | Binding behavior | +| --- | --- | +| IPv6 enabled, `net.ipv6.bindv6only=0` | Requests `IPV6_V6ONLY=1` on the IPv6 socket, binds `[::]:PORT`, and separately binds `0.0.0.0:PORT`. It overrides the system's dual-stack default. | +| IPv6 enabled, `net.ipv6.bindv6only=1` | The same explicit socket option and separate IPv4/IPv6 binds. | +| `HAVE_IPV6` undefined | Skips every address family except `AF_INET`; only the IPv4 wildcard is attempted. | + +`udp_server()` uses `wait_for_connection()`, which puts **every listener** +from that list into `select()` and returns a readable socket for `recvfrom()`; +it does not permanently choose one socket based on `getaddrinfo()` order. + +The first two cases were traced with the actual GnuTLS 3.8.13 binary in +separate Linux network namespaces: `setsockopt(IPV6_V6ONLY, [1])` and both +UDP binds returned success under each setting. The no-IPv6 case was checked +in source, not by building a second binary. The same bind implementation +was checked directly on GitLab master. + +Thus, successful normal startup for this invocation provides an explicit +IPv4 socket; a lone IPv6 wildcard is not the expected success path. +There is one portability caveat: upstream discards the return value of +`setsockopt(IPV6_V6ONLY)`. On a platform where that call fails and the server +ends up with only a dual-stack socket, this helper would time out despite +IPv4 reachability. Such a platform needs additional handling before this +patch can claim support. Blindly accepting every IPv6 wildcard would also +accept IPv6-only sockets before the separate IPv4 bind finishes. + +Source: [`src/serv.c`, `listen_socket()`](https://gitlab.com/gnutls/gnutls/-/blob/master/src/serv.c#L937), +[`src/udp-serv.c`](https://gitlab.com/gnutls/gnutls/-/blob/master/src/udp-serv.c), +and [`tests/serv-udp.sh`](https://gitlab.com/gnutls/gnutls/-/blob/master/tests/serv-udp.sh). + +## Port matching and ownership limit + +Only `-an` is passed to the socket-listing tool: BSD `netstat -u` selects +Unix-domain sockets, whereas Linux `netstat -u` selects UDP. The parser +handles the extra state column in `ss`, Linux colon-separated endpoints, +and BSD dot-separated endpoints, including `*.PORT`. It matches the full +local port and rejects TCP, IPv6 entries, peer ports, and longer numbers. + +A live PID plus a bound port does **not** prove that PID owns the socket. +Existing `GETPORT` selection checks for an unused port and uses a test +port-lock directory; `launch_bare_server()` also calls +`wait_for_free_port()` before starting the process. These are advisory: +the launcher does not enforce the latter's result, and another process +can bind between the check and launch. The patch does not close that race +or add nonportable PID parsing. An unrelated process can satisfy the +socket check; the real handshakes remain the functional check and may +fail (or reach the wrong server). This is a startup-order fix, not a +socket-ownership guarantee. + +## Reproduction and focused checks + +Apply the patch to an unpacked source tree, then run the companion checks +with Python's standard library and a shell: + +```sh +patch --fuzz=0 -d /path/to/gnutls -p1 < udp-server-readiness.patch +SHELL=/bin/sh python3 verify-readiness.py /path/to/gnutls/tests/scripts/common.sh -v +``` + +Set `NETSTAT=/path/to/netstat` to exercise one outside `PATH`. The checks +cover Linux/BSD output samples, false matches, immediate readiness, +missing tools, process exit, timeout cleanup, and real IPv4 UDP sockets +whose bind is delayed six seconds. The missing-tools fixture is skipped +if an absolute fallback `ss` path cannot be hidden with `PATH`. Native +BSD execution remains untested. + +To reproduce with GnuTLS itself, run `tests/serv-udp.sh` with `SERV` pointing +to a wrapper that sleeps six seconds, then `exec`s `gnutls-serv` with all +arguments. Set `CLI` to the matching `gnutls-cli`, `srcdir` to the source +`tests` directory, and `abs_top_builddir` to a writable build directory. +With GnuTLS 3.8.13, the original helper failed the first handshake with +`Connection refused`; the patched helper passed both with the same binaries. + +## GnuTLS submission + +Development and merge requests are on [GitLab](https://gitlab.com/gnutls/gnutls). +[`CONTRIBUTING.md` on master](https://gitlab.com/gnutls/gnutls/-/blob/master/CONTRIBUTING.md) +was read directly for this review. It requires the contributor's DCO +`Signed-off-by`, successful and failure test coverage, consistent coding +style, and adequate documentation; GitLab CI runs for merge requests. +Its commenting guidance asks for comments explaining non-obvious behavior +or protocol expectations. It does not prescribe an additional special +test-suite comment. The patch now explains its IPv4 binding assumption +next to the probe. + +The submission will contain the shell patch, without the Python verifier +or a new Python test dependency. The existing `serv-udp.sh` supplies the +functional success check. Running it through the six-second startup +wrapper supplies a reproducible regression case: it fails before the fix +and passes after it. The local verifier was used to validate socket-output +parsing and the helper's success, process-exit, skip-cleanup, and timeout +branches. Those branch checks are local evidence, not new automated +coverage in the upstream suite; the MR must state that distinction. + +No dedicated unit-test harness for these shell helpers was found in the +3.8.13 tests inspected. That does not establish that Python cannot be used +upstream; keeping this submission dependency-free is a scope choice. Use +the existing test and before/after reproduction as the submission's +coverage argument, retaining the platform limitations above. Apply the +patch in an upstream checkout and include those results with the +contributor's own sign-off. No MR or sign-off has been created. + +## Local NixOS integration and build results + +`overlays/default.nix` imports the `overlays/patches` overlay, which loads +`gnutls/default.nix` to apply the patch and keep `serv-udp.sh` enabled. +The patch itself has no Nix dependencies and applies to 3.8.13 and GitLab +master without fuzz. + +The final patch was rebuilt with: + +```sh +nix build --no-link -L .#nixosConfigurations.jeeves.pkgs.gnutls +``` + +That x86-64-v3 build passed: 927 tests, 796 passes, 131 existing skips, +zero failures/errors, and `PASS: serv-udp.sh`. The patch bytes in the built +derivation were compared with the repository artifact; both have SHA-256 +`59013d47fd446f2dd065012a2259ccc1898fedc8a053a630e13efa0076368760`. +All seven local checks passed, including skip cleanup and exactly 90 +probes with 89 sleeps on timeout. The six-second before/after reproduction +was also repeated successfully with the final helper. diff --git a/overlays/patches/gnutls/default.nix b/overlays/patches/gnutls/default.nix new file mode 100644 index 0000000..0fe01ef --- /dev/null +++ b/overlays/patches/gnutls/default.nix @@ -0,0 +1,6 @@ +{ gnutls }: +gnutls.overrideAttrs (old: { + # Keep the UDP handshake test enabled on loaded builders by waiting for + # the server to bind its socket. Kept as a standalone patch for upstream. + patches = (old.patches or [ ]) ++ [ ./udp-server-readiness.patch ]; +}) diff --git a/overlays/patches/gnutls/udp-server-readiness.patch b/overlays/patches/gnutls/udp-server-readiness.patch new file mode 100644 index 0000000..dae4946 --- /dev/null +++ b/overlays/patches/gnutls/udp-server-readiness.patch @@ -0,0 +1,70 @@ +Subject: [PATCH] tests: wait for the UDP server socket before connecting + +A fixed four-second sleep does not guarantee that gnutls-serv has bound +its UDP socket on a busy builder. Poll the local IPv4 UDP endpoint using +the existing ss/netstat discovery, with the same retry budget as the TCP +helper. Fail early if the server exits, and retain the original handshake +checks in serv-udp.sh. + +Use flags common to ss and BSD/Linux netstat. Match the local endpoint +and complete port number, excluding TCP, IPv6-only and peer endpoints. + +--- a/tests/scripts/common.sh ++++ b/tests/scripts/common.sh +@@ -185,10 +185,55 @@ + fi + } + ++check_if_udp_port_bound() { ++ local PORT=$1 ++ have_port_finder ++ # Use only -an, which is shared by ss and BSD/Linux netstat. UDP has ++ # no LISTEN state. Match the local IPv4 endpoint, not a peer port or ++ # a longer port number. serv-udp.sh connects to 127.0.0.1; ++ # listen_socket() in serv.c binds IPv4 separately and requests ++ # IPV6_V6ONLY=1 for its IPv6 socket. ++ $PFCMD -an | awk -v port="$PORT" ' ++ $1 == "udp" || $1 == "udp4" { ++ # ss includes a state column; netstat does not. ++ address = ($2 == "UNCONN" || $2 == "ESTAB") ? $5 : $4 ++ if (address ~ ("^[0-9.]+[.:]" port "$") || ++ address == "*." port) ++ found = 1 ++ } ++ END { exit !found } ++ ' ++} ++ + wait_udp_server() { + local PID=$1 ++ local ret + trap "test -n \"${PID}\" && kill ${PID};exit 1" 1 15 2 +- sleep 4 ++ local i=0 ++ # Use the same retry budget as wait_for_port(), but also stop if the ++ # server exits before binding its socket. ++ while test $i -lt 90; do ++ if ! kill -0 "$PID" 2>/dev/null; then ++ fail "" "UDP server $PID exited before binding port $PORT" ++ fi ++ # Contain have_port_finder's exit so a skip also stops the server. ++ if (check_if_udp_port_bound "$PORT"); then ++ return 0 ++ else ++ ret=$? ++ if test "$ret" = 77; then ++ kill "$PID" 2>/dev/null || : ++ wait "$PID" 2>/dev/null || : ++ exit 77 ++ fi ++ fi ++ i=$((i + 1)) ++ if test $i -lt 90; then ++ echo "try $i: waiting for UDP port $PORT" ++ sleep 2 ++ fi ++ done ++ fail "$PID" "UDP server $PORT did not come up" + } + + create_testdir() { diff --git a/overlays/patches/gnutls/verify-readiness.py b/overlays/patches/gnutls/verify-readiness.py new file mode 100755 index 0000000..04ea39c --- /dev/null +++ b/overlays/patches/gnutls/verify-readiness.py @@ -0,0 +1,180 @@ +#!/usr/bin/env python3 +"""Exercise patched common.sh without building GnuTLS (Python standard library only). + +Usage: python3 verify-readiness.py /path/to/patched/tests/scripts/common.sh +Set SHELL to test another shell, and NETSTAT to test a netstat outside PATH. +""" + +# Use unittest so this upstream companion tool needs no pytest installation. +# ruff: noqa: PT009 + +import os +import shutil +import socket +import subprocess +import sys +import tempfile +import time +import unittest +from pathlib import Path + +COMMON = str(Path(sys.argv.pop(1)).resolve()) +SHELL = os.environ.get("SHELL", "/bin/sh") + + +class ReadinessTests(unittest.TestCase): + """Check endpoint parsing and the server startup lifecycle.""" + + def setUp(self) -> None: + """Create a socket-listing fixture for each check.""" + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) + self.fixture = self.root / "sockets" + self.fixture.write_text("") + self.finder = self.root / "port-finder" + self.finder.write_text('#!/bin/sh\ncat "$SOCKET_FIXTURE"\n') + self.finder.chmod(0o755) + + def run_shell(self, body: str, **env: str) -> subprocess.CompletedProcess[str]: + """Source the actual helper and run a shell scenario.""" + return subprocess.run( + [SHELL, "-c", '. "$COMMON"\n' + body], + env={ + **os.environ, + "COMMON": COMMON, + "SOCKET_FIXTURE": str(self.fixture), + "PFCMD": str(self.finder), + "PORT": "12345", + **env, + }, + capture_output=True, + text=True, + timeout=20, + check=False, + ) + + def test_socket_formats_and_false_matches(self) -> None: + """Accept IPv4 UDP local endpoints and reject unrelated sockets.""" + cases = [ + ("udp UNCONN 0 0 0.0.0.0:12345 0.0.0.0:*", True), + ("udp UNCONN 0 0 127.0.0.1:12345 0.0.0.0:*", True), + ("udp 0 0 0.0.0.0:12345 0.0.0.0:*", True), + ("udp4 0 0 *.12345 *.*", True), + ("udp 0 0 127.0.0.1.12345 *.*", True), + ("udp 0 0 *.12345 *.*", True), + ("udp UNCONN 0 0 0.0.0.0:123456 0.0.0.0:*", False), + ("udp 0 0 0.0.0.0:123456 0.0.0.0:*", False), + ("udp ESTAB 0 0 127.0.0.1:54321 127.0.0.1:12345", False), + ("udp 0 0 127.0.0.1:54321 127.0.0.1:12345", False), + ("tcp LISTEN 0 128 0.0.0.0:12345 0.0.0.0:*", False), + ("tcp 0 0 0.0.0.0:12345 0.0.0.0:* LISTEN", False), + ("udp UNCONN 0 0 [::]:12345 [::]:*", False), + ("udp UNCONN 0 0 *:12345 *:*", False), + ("udp6 0 0 :::12345 :::*", False), + ("udp6 0 0 *.12345 *.*", False), + ("", False), + ] + for row, ready in cases: + with self.subTest(row=row): + self.fixture.write_text(row + "\n") + result = self.run_shell('check_if_udp_port_bound "$PORT"') + self.assertEqual(result.returncode, 0 if ready else 1, result.stderr) + + def test_exited_server_fails_immediately(self) -> None: + """Fail without sleeping when the server has already exited.""" + result = self.run_shell( + 'true &\npid=$!\nwait "$pid"\nsleep() { echo "unexpected sleep" >&2; }\nwait_udp_server "$pid"' + ) + self.assertEqual(result.returncode, 1) + self.assertIn("exited before binding", result.stderr) + self.assertNotIn("unexpected sleep", result.stderr) + + def test_ready_socket_does_not_sleep(self) -> None: + """Check readiness before the first sleep.""" + self.fixture.write_text("udp UNCONN 0 0 0.0.0.0:12345 0.0.0.0:*\n") + result = self.run_shell('sleep() { echo "unexpected sleep" >&2; }\nwait_udp_server "$$"') + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("unexpected sleep", result.stderr) + + def test_missing_port_finders_skip(self) -> None: + """Skip and stop the live server when no finder is available.""" + # have_port_finder also tries these paths independently of PATH. + if any(os.access(f"{directory}/ss", os.X_OK) for directory in ("/sbin", "/usr/sbin", "/usr/local/sbin")): + self.skipTest("an absolute ss path cannot be hidden by this PATH-only fixture") + with subprocess.Popen(["sleep", "60"]) as server: + try: + result = self.run_shell( + 'unset PFCMD\nPATH=/nonexistent\nwait_udp_server "$SERVER_PID"', + SERVER_PID=str(server.pid), + ) + self.assertEqual(result.returncode, 77) + self.assertIn("neither ss nor netstat found", result.stderr) + server.wait(timeout=3) + self.assertLess(server.returncode, 0) + finally: + if server.poll() is None: + server.kill() + + def test_timeout_is_bounded_and_cleans_up(self) -> None: + """Stop polling after the retry budget and terminate the server.""" + # Only accelerate the polling delay; keep a real live server process. + self.finder.write_text('#!/bin/sh\necho probe >&2\ncat "$SOCKET_FIXTURE"\n') + with subprocess.Popen(["sleep", "60"]) as server: + try: + result = self.run_shell( + 'sleep() { echo polling-sleep; }\nwait_udp_server "$SERVER_PID"', + SERVER_PID=str(server.pid), + ) + self.assertEqual(result.returncode, 1) + self.assertIn("did not come up", result.stderr) + self.assertEqual(result.stderr.count("probe\n"), 90) + self.assertEqual(result.stdout.count("polling-sleep"), 89) + server.wait(timeout=3) + self.assertLess(server.returncode, 0) + finally: + if server.poll() is None: + server.kill() + + def test_server_exits_while_waiting(self) -> None: + """Detect a startup failure that happens after polling begins.""" + result = self.run_shell('sleep 1 &\npid=$!\nwait_udp_server "$pid"') + self.assertEqual(result.returncode, 1) + self.assertIn("exited before binding", result.stderr) + self.assertIn("waiting for UDP port", result.stdout) + + def test_real_socket_delayed_beyond_four_seconds(self) -> None: + """Wait for a real delayed bind with each installed port finder.""" + finders = [shutil.which("ss"), os.environ.get("NETSTAT") or shutil.which("netstat")] + finders = [finder for finder in finders if finder] + if not finders: + self.skipTest("neither ss nor netstat available") + for finder in finders: + with self.subTest(finder=finder): + with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as sock: + sock.bind(("127.0.0.1", 0)) + port = sock.getsockname()[1] + code = ( + "import socket,time,sys; time.sleep(6); " + "s=socket.socket(socket.AF_INET,socket.SOCK_DGRAM); " + "s.bind(('127.0.0.1',int(sys.argv[1]))); time.sleep(30)" + ) + with subprocess.Popen([sys.executable, "-c", code, str(port)]) as server: + try: + started = time.monotonic() + result = self.run_shell( + 'wait_udp_server "$SERVER_PID"', + SERVER_PID=str(server.pid), + PORT=str(port), + PFCMD=finder, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertGreaterEqual(time.monotonic() - started, 6) + self.assertIsNone(server.poll()) + finally: + server.terminate() + server.wait(timeout=3) + + +if __name__ == "__main__": + unittest.main() diff --git a/overlays/test-exclusions.nix b/overlays/test-exclusions.nix index 1c66c41..ab8089c 100644 --- a/overlays/test-exclusions.nix +++ b/overlays/test-exclusions.nix @@ -9,14 +9,6 @@ # architecture-dependent floating-point differences whose risk we accept for # our workloads. Keep these exceptions visible until their causes are fixed. _final: prev: { - gnutls = prev.gnutls.overrideAttrs (old: { - # This test uses a fixed four-second sleep instead of checking UDP - # readiness; the client saw no listener in the x86-64-v3 build. - postPatch = (old.postPatch or "") + '' - sed '2iexit 77' -i tests/serv-udp.sh - ''; - }); - prometheus = prev.prometheus.overrideAttrs ( old: let @@ -83,15 +75,15 @@ _final: prev: { ''; }); - scipy = pythonPrev.scipy.overridePythonAttrs (old: { - # x86-64-v3 FFT implementations produce rounding differences outside - # these tests' strict tolerances. We accept the numerical-precision - # risk for our workloads. - disabledTests = (old.disabledTests or [ ]) ++ [ - "test_roundtrip_float32" - "test_roundtrip_scaling" - ]; - }); + # scipy = pythonPrev.scipy.overridePythonAttrs (old: { + # # x86-64-v3 FFT implementations produce rounding differences outside + # # these tests' strict tolerances. We accept the numerical-precision + # # risk for our workloads. + # disabledTests = (old.disabledTests or [ ]) ++ [ + # "test_roundtrip_float32" + # "test_roundtrip_scaling" + # ]; + # }); sentry-sdk = pythonPrev.sentry-sdk.overridePythonAttrs (old: { # This test globally mocks threading.current_thread while another