From 2a536c0e3475442d4fffe6915df54e61a02e68f4 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Sun, 6 Sep 2026 00:43:34 +0100 Subject: [PATCH 1/2] Cover the protocols in-process, and keep the droplet for the network path The droplet e2e never exercised muxing or UDP. It predates the mux, and it has no UDP coverage at all -- it drives curl, which only opens TCP -- so the two things this transport now does differently on the wire had never been run end to end. That gap is the reason for this change; the maintenance argument follows from it. TestTwiddleCarriesTCPAndUDPOverOneTunnel stands up both boxes in-process and drives TCP and UDP through the client's SOCKS5 inbound: two HTTP requests (the second proving the tunnel is reused, which under muxing means a second stream rather than a second opening) and two datagrams echoed back through UoT. It needs no droplet, no Docker and no credentials, because what it tests is two lantern-box instances talking to each other -- which is what the remote host was standing in for in every assertion except the network path itself. Corrupting the ticket fails both halves, so it is the tunnel under test and not a leak past it. The droplet job moves to nightly plus manual dispatch. What it uniquely covers is a REAL remote host -- routing, MTU, a cover site over the actual internet -- and that is worth keeping for a transport whose job is surviving a network path. What it is not worth is gating every PR: it costs a droplet per run, needs DO_API_TOKEN, takes minutes, and fails on things unrelated to the change under review. A 422 from DigitalOcean's API reddened two boards on 2026-09-05, neither a code fault, and I misread the first as a rotated credential. No workflow change was needed for the PR side: test/e2e already runs in `make test`, so the new coverage lands on every PR as it is. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017gn6KuHUL766qQNn8m1fu1 --- .github/workflows/e2e.yaml | 32 +++-- test/e2e/testdata/twiddle_client.json | 23 ++++ test/e2e/testdata/twiddle_server.json | 15 +++ test/e2e/twiddle_test.go | 162 ++++++++++++++++++++++++++ 4 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 test/e2e/testdata/twiddle_client.json create mode 100644 test/e2e/testdata/twiddle_server.json create mode 100644 test/e2e/twiddle_test.go diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 34eb0b48..5bad5e69 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -2,16 +2,28 @@ name: E2E Protocol Tests on: workflow_dispatch: - # Run on every PR — stacked PRs against feature branches (e.g. the Unbounded - # outbound stack) should still get live-machine coverage before they merge - # into main. The path filter keeps doc-only PRs from consuming droplets. - pull_request: - paths: - - '**.go' - - 'go.mod' - - 'go.sum' - - 'Dockerfile' - - '.github/workflows/e2e.yaml' + + # Nightly rather than per-PR. + # + # What this job uniquely tests is a REAL remote host: actual routing, actual + # MTU, a cover site reached over the actual internet. That is worth having for + # a transport whose whole job is surviving a network path, and it is not + # something an in-process test can stand in for. + # + # What it is not worth is gating every PR. It costs a droplet per run, needs + # DO_API_TOKEN, takes minutes, and fails on things that have nothing to do with + # the change under review -- a 422 from DigitalOcean's API cost two red boards + # on 2026-09-05, neither of them a code fault. + # + # Protocol coverage on PRs is test/e2e instead: in-process boxes talking to + # each other over loopback, which is what the droplet was standing in for in + # every assertion except the network path itself. Those run in `make test`. + # + # Trigger a run by hand (Actions > E2E Protocol Tests > Run workflow) whenever + # a change touches the wire: a new protocol, a transport's framing, or anything + # that would behave differently against a real peer. + schedule: + - cron: '0 7 * * *' env: DROPLET_NAME: "e2e-lantern-box-${{ github.run_id }}" diff --git a/test/e2e/testdata/twiddle_client.json b/test/e2e/testdata/twiddle_client.json new file mode 100644 index 00000000..dc59b763 --- /dev/null +++ b/test/e2e/testdata/twiddle_client.json @@ -0,0 +1,23 @@ +{ + "log": {"disabled": true, "level": "trace", "output": "stdout"}, + "inbounds": [ + { + "type": "mixed", + "tag": "mixed-in", + "listen": "127.0.0.1", + "listen_port": 1080 + } + ], + "outbounds": [ + { + "type": "twiddle", + "tag": "twiddle-out", + "server": "127.0.0.1", + "server_port": 9005, + "ticket": "TICKET", + "psk": "PSK", + "cover_sni": "www.cloudflare.com", + "hello_pool": "HELLO_POOL" + } + ] +} diff --git a/test/e2e/testdata/twiddle_server.json b/test/e2e/testdata/twiddle_server.json new file mode 100644 index 00000000..bfe40310 --- /dev/null +++ b/test/e2e/testdata/twiddle_server.json @@ -0,0 +1,15 @@ +{ + "log": {"disabled": true, "level": "trace", "output": "stdout"}, + "inbounds": [ + { + "type": "twiddle", + "tag": "twiddle-in", + "listen": "127.0.0.1", + "listen_port": 9005, + "ticket_key": "TICKET_KEY", + "masquerade_upstream": "www.cloudflare.com:443", + "ticket_max_age": "24h" + } + ], + "outbounds": [{"type": "direct", "tag": "direct"}] +} diff --git a/test/e2e/twiddle_test.go b/test/e2e/twiddle_test.go new file mode 100644 index 00000000..a7ea2467 --- /dev/null +++ b/test/e2e/twiddle_test.go @@ -0,0 +1,162 @@ +package e2e + +import ( + "context" + "encoding/base64" + "encoding/hex" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" + + sbox "github.com/sagernet/sing-box" + "github.com/sagernet/sing-box/option" + "github.com/sagernet/sing/common/json" + M "github.com/sagernet/sing/common/metadata" + N "github.com/sagernet/sing/common/network" + "github.com/sagernet/sing/protocol/socks" + "github.com/stretchr/testify/require" + + box "github.com/getlantern/lantern-box" + lboption "github.com/getlantern/lantern-box/option" + tw "github.com/getlantern/twiddle" +) + +// TestTwiddleCarriesTCPAndUDPOverOneTunnel exercises what the droplet e2e never +// has: the multiplexed outbound, and UDP through it. +// +// The shell e2e drives curl through a client box, which only ever opens TCP -- +// it has no UDP coverage at all. And it predates muxing, so the one thing this +// transport now does differently on the wire (N inner destinations over one +// outer twiddle connection, UDP included via UoT) has never been run end to end. +// +// Both boxes run in-process, so this needs no droplet, no Docker and no +// credentials: what is actually under test is two lantern-box instances talking +// to each other, which is exactly what the remote host was standing in for. +func TestTwiddleCarriesTCPAndUDPOverOneTunnel(t *testing.T) { + ctx := context.Background() + boxCtx := box.BaseContext() + + key, err := tw.NewTicketKey() + require.NoError(t, err) + cover, err := tw.CoverFor("www.cloudflare.com") + require.NoError(t, err) + cred, err := key.Issue(1, cover.TicketLen) + require.NoError(t, err) + + // The embedded pool would do, but naming it keeps the test honest about + // which hellos are on the wire. + pool := tw.FormatPool(tw.DefaultPool()[:1]) + + serverPort := freePort(t) + clientPort := freePort(t) + + serverOpts := twiddleOptions(boxCtx, t, "testdata/twiddle_server.json", map[string]string{ + "TICKET_KEY": hex.EncodeToString(key[:]), + }) + clientOpts := twiddleOptions(boxCtx, t, "testdata/twiddle_client.json", map[string]string{ + "TICKET": base64.StdEncoding.EncodeToString(cred.Ticket), + "PSK": hex.EncodeToString(cred.PSK[:]), + "HELLO_POOL": strings.TrimSpace(pool), + }) + serverOpts.Inbounds[0].Options.(*lboption.TwiddleInboundOptions).ListenPort = serverPort + clientOpts.Inbounds[0].Options.(*option.HTTPMixedInboundOptions).ListenPort = clientPort + clientOpts.Outbounds[0].Options.(*lboption.TwiddleOutboundOptions).ServerPort = serverPort + + serverBox, err := sbox.New(sbox.Options{Context: boxCtx, Options: serverOpts}) + require.NoError(t, err) + require.NoError(t, serverBox.Start()) + t.Cleanup(func() { serverBox.Close() }) + + clientBox, err := sbox.New(sbox.Options{Context: boxCtx, Options: clientOpts}) + require.NoError(t, err) + require.NoError(t, clientBox.Start()) + t.Cleanup(func() { clientBox.Close() }) + + client := socks.NewClient( + N.SystemDialer, M.ParseSocksaddrHostPort("127.0.0.1", clientPort), socks.Version5, "", "") + + t.Run("tcp", func(t *testing.T) { + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "through the tunnel") + })) + defer origin.Close() + + hc := &http.Client{ + Timeout: 20 * time.Second, + Transport: &http.Transport{ + DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + return client.DialContext(ctx, network, M.ParseSocksaddr(addr)) + }, + }, + } + // Twice: the second request proves the tunnel is reusable, which under + // muxing means a second stream rather than a second twiddle opening. + for i := range 2 { + resp, err := hc.Get(origin.URL) + require.NoErrorf(t, err, "request %d", i+1) + body, err := io.ReadAll(resp.Body) + resp.Body.Close() + require.NoError(t, err) + require.Equal(t, "through the tunnel", string(body)) + } + }) + + t.Run("udp", func(t *testing.T) { + // A UDP echo origin: what comes back proves the datagram made the whole + // round trip through UoT and the mux, not merely that a socket opened. + origin, err := net.ListenPacket("udp", "127.0.0.1:0") + require.NoError(t, err) + defer origin.Close() + go func() { + buf := make([]byte, 2048) + for { + n, addr, err := origin.ReadFrom(buf) + if err != nil { + return + } + _, _ = origin.WriteTo(buf[:n], addr) + } + }() + + conn, err := client.ListenPacket(ctx, M.ParseSocksaddr(origin.LocalAddr().String())) + require.NoError(t, err, "SOCKS5 UDP associate through twiddle") + defer conn.Close() + require.NoError(t, conn.SetDeadline(time.Now().Add(20*time.Second))) + + target := M.ParseSocksaddr(origin.LocalAddr().String()).UDPAddr() + for i, payload := range [][]byte{ + []byte("first datagram"), + []byte(strings.Repeat("x", 1200)), + } { + _, err = conn.WriteTo(payload, target) + require.NoErrorf(t, err, "datagram %d", i+1) + + buf := make([]byte, 2048) + n, _, err := conn.ReadFrom(buf) + require.NoErrorf(t, err, "datagram %d reply", i+1) + require.Equal(t, payload, buf[:n]) + } + }) +} + +// twiddleOptions reads a config and substitutes the credentials generated for +// this run, so nothing secret-shaped is checked in and every run is independent. +func twiddleOptions(ctx context.Context, t *testing.T, path string, subs map[string]string) option.Options { + t.Helper() + raw, err := os.ReadFile(path) + require.NoError(t, err) + body := string(raw) + for k, v := range subs { + require.Containsf(t, body, k, "%s has no %s placeholder", path, k) + body = strings.ReplaceAll(body, k, v) + } + opts, err := json.UnmarshalExtendedContext[option.Options](ctx, []byte(body)) + require.NoError(t, err) + return opts +} From 5e61538aed585d4d6fc5f0bb432f6eadb091dad9 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Sun, 6 Sep 2026 00:50:59 +0100 Subject: [PATCH 2/2] address review: make the muxing claim falsifiable The comment said the second request proved the tunnel was reused under muxing. Nothing checked either half of that, and both halves were wrong. http.Transport caches idle connections, so with keep-alives on the second Get took the first request's connection back and no second stream was opened at all -- the loop was not exercising what the comment described. DisableKeepAlives forces a real second inner dial, and innerDials counts them so the loop cannot quietly stop testing anything again. And nothing counted OUTER connections, which is the property the test is named for. A regression opening a fresh twiddle tunnel per destination serves every request correctly, so it would have passed while the thing muxing exists for had been lost. countingRelay sits between the client box and the egress and counts; the test now requires three inner connections -- two TCP and the UoT carrier -- to have crossed one outer tunnel. Removing the session reuse from ensureSession now fails it. Before, it passed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017gn6KuHUL766qQNn8m1fu1 --- test/e2e/twiddle_test.go | 62 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/test/e2e/twiddle_test.go b/test/e2e/twiddle_test.go index a7ea2467..24af59c3 100644 --- a/test/e2e/twiddle_test.go +++ b/test/e2e/twiddle_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "os" "strings" + "sync/atomic" "testing" "time" @@ -66,7 +67,8 @@ func TestTwiddleCarriesTCPAndUDPOverOneTunnel(t *testing.T) { }) serverOpts.Inbounds[0].Options.(*lboption.TwiddleInboundOptions).ListenPort = serverPort clientOpts.Inbounds[0].Options.(*option.HTTPMixedInboundOptions).ListenPort = clientPort - clientOpts.Outbounds[0].Options.(*lboption.TwiddleOutboundOptions).ServerPort = serverPort + relay := startCountingRelay(t, serverPort) + clientOpts.Outbounds[0].Options.(*lboption.TwiddleOutboundOptions).ServerPort = relay.port serverBox, err := sbox.New(sbox.Options{Context: boxCtx, Options: serverOpts}) require.NoError(t, err) @@ -87,16 +89,20 @@ func TestTwiddleCarriesTCPAndUDPOverOneTunnel(t *testing.T) { })) defer origin.Close() + var innerDials atomic.Int64 hc := &http.Client{ Timeout: 20 * time.Second, Transport: &http.Transport{ + // Without this the transport hands the second request its idle + // connection from the first, so no second stream is ever opened + // and the loop below proves nothing about reuse. + DisableKeepAlives: true, DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + innerDials.Add(1) return client.DialContext(ctx, network, M.ParseSocksaddr(addr)) }, }, } - // Twice: the second request proves the tunnel is reusable, which under - // muxing means a second stream rather than a second twiddle opening. for i := range 2 { resp, err := hc.Get(origin.URL) require.NoErrorf(t, err, "request %d", i+1) @@ -105,6 +111,8 @@ func TestTwiddleCarriesTCPAndUDPOverOneTunnel(t *testing.T) { require.NoError(t, err) require.Equal(t, "through the tunnel", string(body)) } + require.EqualValues(t, 2, innerDials.Load(), + "each request must open its own inner connection, or the loop tests nothing") }) t.Run("udp", func(t *testing.T) { @@ -143,6 +151,12 @@ func TestTwiddleCarriesTCPAndUDPOverOneTunnel(t *testing.T) { require.Equal(t, payload, buf[:n]) } }) + + // Three inner connections have now crossed the relay -- two TCP and one UoT + // carrier for the UDP association -- and muxing means they shared a single + // outer twiddle connection. One opening on the wire, not three. + require.EqualValues(t, 1, relay.conns.Load(), + "TCP and UDP must share one outer twiddle tunnel; a fresh opening per destination is the fingerprint muxing exists to remove") } // twiddleOptions reads a config and substitutes the credentials generated for @@ -160,3 +174,45 @@ func twiddleOptions(ctx context.Context, t *testing.T, path string, subs map[str require.NoError(t, err) return opts } + +// countingRelay sits between the client box and the twiddle inbound and counts +// the OUTER connections opened through it. +// +// Without it the muxing claim is unfalsifiable from out here: a regression that +// opened a fresh twiddle tunnel per inner destination still serves every +// request correctly, so the test would pass while the property it exists for +// had been lost. Counting is what makes "one tunnel" an assertion rather than a +// comment. +type countingRelay struct { + port uint16 + conns atomic.Int64 +} + +func startCountingRelay(t *testing.T, upstreamPort uint16) *countingRelay { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { ln.Close() }) + + r := &countingRelay{port: uint16(ln.Addr().(*net.TCPAddr).Port)} + go func() { + for { + down, err := ln.Accept() + if err != nil { + return + } + r.conns.Add(1) + go func() { + defer down.Close() + up, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", upstreamPort)) + if err != nil { + return + } + defer up.Close() + go io.Copy(up, down) + io.Copy(down, up) + }() + } + }() + return r +}