From 7a57ea739def4fa9d5ee88c491dd3087559de964 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Wed, 15 Apr 2026 18:51:38 -0400 Subject: [PATCH 01/20] refactor(client): modularize DNS interception logic and address remapping --- .../configregistry/outline_dns_intercept.go | 24 ++-- client/go/outline/dnsintercept/README.md | 74 ++++++----- client/go/outline/dnsintercept/forward.go | 78 ++++-------- .../go/outline/dnsintercept/forward_test.go | 76 +++--------- client/go/outline/dnsintercept/interceptor.go | 103 ++++++++++++++++ .../outline/dnsintercept/interceptor_test.go | 63 ++++++++++ client/go/outline/dnsintercept/truncate.go | 115 ------------------ .../go/outline/dnsintercept/truncate_test.go | 109 ----------------- 8 files changed, 271 insertions(+), 371 deletions(-) create mode 100644 client/go/outline/dnsintercept/interceptor.go create mode 100644 client/go/outline/dnsintercept/interceptor_test.go delete mode 100644 client/go/outline/dnsintercept/truncate.go delete mode 100644 client/go/outline/dnsintercept/truncate_test.go diff --git a/client/go/outline/configregistry/outline_dns_intercept.go b/client/go/outline/configregistry/outline_dns_intercept.go index ae1974dae4..91b0a32ac2 100644 --- a/client/go/outline/configregistry/outline_dns_intercept.go +++ b/client/go/outline/configregistry/outline_dns_intercept.go @@ -24,6 +24,7 @@ import ( "localhost/client/go/outline/dnsintercept" "golang.getoutline.org/sdk/network" + "golang.getoutline.org/sdk/network/dnstruncate" "golang.getoutline.org/sdk/transport" ) @@ -61,29 +62,36 @@ func wrapTransportPairWithOutlineDNS(sd *Dialer[transport.StreamConn], pl *Packe if err != nil { return nil, fmt.Errorf("failed to create PacketProxy: %w", err) } - // Forwards everything including DNS. For DNS it translates between the link-local and remote addresses for the DNS resolver. - ppForward, err := dnsintercept.NewDNSRedirectPacketProxy(ppBase, linkLocalDNS, remoteDNS) + // Forwards DNS packets through ppBase and closes the session after the first response. + ppForward, err := dnsintercept.NewDNSForwardPacketProxy(ppBase) if err != nil { return nil, fmt.Errorf("failed to create DNS redirect PacketProxy: %w", err) } - // Forwards everything except DNS. For DNS it returns a truncated response. - ppTrunc, err := dnsintercept.NewDNSTruncatePacketProxy(ppBase, linkLocalDNS) + // Returns a truncated response for DNS packets to force a retry over TCP. + ppTrunc, err := dnstruncate.NewPacketProxy() if err != nil { return nil, fmt.Errorf("failed to create always-truncate DNS PacketProxy: %w", err) } - ppMain, err := network.NewDelegatePacketProxy(ppTrunc) + // Delegate for DNS traffic: selects between forwarding and truncation based on connectivity. + ppDNS, err := network.NewDelegatePacketProxy(ppTrunc) if err != nil { - return nil, fmt.Errorf("failed to create indirect PacketProxy: %w", err) + return nil, fmt.Errorf("failed to create indirect DNS PacketProxy: %w", err) + } + // Interceptor: Forwards everything except DNS to ppBase. DNS is redirected to ppDNS and + // translated between the link-local and remote addresses. + ppMain, err := dnsintercept.NewDNSInterceptor(ppBase, ppDNS, linkLocalDNS, remoteDNS) + if err != nil { + return nil, fmt.Errorf("failed to create DNS interceptor PacketProxy: %w", err) } onNetworkChanged := func() { go func() { if err := connectivity.CheckUDPConnectivity(pl); err == nil { slog.Info("remote device UDP is healthy") - ppMain.SetProxy(ppForward) + ppDNS.SetProxy(ppForward) } else { slog.Warn("remote device UDP is not healthy", "err", err) - ppMain.SetProxy(ppTrunc) + ppDNS.SetProxy(ppTrunc) } }() } diff --git a/client/go/outline/dnsintercept/README.md b/client/go/outline/dnsintercept/README.md index 1720349d4a..4bf477cd44 100644 --- a/client/go/outline/dnsintercept/README.md +++ b/client/go/outline/dnsintercept/README.md @@ -63,27 +63,53 @@ DNS can travel over both TCP and UDP: UDP connectivity is not guaranteed, so the package uses two strategies and switches between them dynamically. +### Dynamic switching + +The two modes are wired together by the caller (`configregistry.wrapTransportPairWithOutlineDNS`) using a `DelegatePacketProxy` and a `DNSInterceptor`. + +```mermaid +flowchart TD + OS["OS (UDP traffic)"] --> ppMain + ppMain["DNSInterceptor
(Address remapping)"] + ppMain -->|Non-DNS| ppBase["base PacketProxy
(transport)"] + ppMain -->|DNS| ppDNS["DelegatePacketProxy
(DNS traffic only)"] + + check["UDP connectivity check
(on network change)"] -->|pass| ppDNS + check -->|fail| ppDNS + + ppDNS -->|UDP available| ppForward["DNSForwardPacketProxy
(Session lifecycle)"] + ppDNS -->|UDP blocked| ppTrunc["dnstruncate.PacketProxy
(TC response locally)"] + + ppForward --> ppBase +``` + +The `DNSInterceptor` acts as the primary dispatcher. It routes non-DNS traffic directly to the transport and DNS traffic to a dedicated delegate proxy. This delegate proxy switches between forwarding and truncation based on the result of a periodic UDP connectivity check. + + ### Forward mode (UDP available) -DNS queries are forwarded over UDP to a public resolver (Cloudflare, Quad9, or OpenDNS, chosen randomly per session) through the proxy transport. Responses are rewritten to appear to come from the original fake address. +DNS queries are forwarded over UDP to a public resolver (Cloudflare, Quad9, or OpenDNS, chosen randomly per session) through the proxy transport. Addresses are rewritten between the fake link-local address and the real resolver address. ```mermaid sequenceDiagram participant OS - participant dnsRedirectPacketProxy + participant Interceptor as DNSInterceptor + participant Forward as DNSForwardPacketProxy participant Transport participant Resolver as Public DNS resolver - OS->>dnsRedirectPacketProxy: UDP query to 169.254.113.53:53 - dnsRedirectPacketProxy->>Transport: UDP query to 1.1.1.1:53 (remapped) + OS->>Interceptor: UDP query to 169.254.113.53:53 + Interceptor->>Forward: UDP query to 1.1.1.1:53 (remapped) + Forward->>Transport: UDP query to 1.1.1.1:53 Transport->>Resolver: query Resolver->>Transport: response - Transport->>dnsRedirectPacketProxy: UDP response from 1.1.1.1:53 - dnsRedirectPacketProxy->>OS: response from 169.254.113.53:53 (remapped back) - Note over dnsRedirectPacketProxy: session closed immediately after response + Transport->>Forward: UDP response from 1.1.1.1:53 + Forward->>Interceptor: UDP response from 1.1.1.1:53 + Interceptor->>OS: response from 169.254.113.53:53 (remapped back) + Note over Forward: session closed immediately after response ``` -Each DNS session (one query/response pair) opens a transport session for the duration of the exchange and closes it as soon as the response is delivered. This keeps resource usage proportional to in-flight queries rather than to recent query rate. +Each DNS session (one query/response pair) closes the transport session as soon as the first response is delivered. This keeps resource usage proportional to in-flight queries rather than to recent query rate. ### Truncate mode (UDP unavailable) @@ -92,12 +118,12 @@ If UDP is blocked, forwarding silently fails and DNS stops working. To handle t ```mermaid sequenceDiagram participant OS - participant dnsTruncatePacketProxy + participant Trunc as dnstruncate.PacketProxy participant StreamDialer participant Resolver as Public DNS resolver - OS->>dnsTruncatePacketProxy: UDP query to 169.254.113.53:53 - dnsTruncatePacketProxy->>OS: truncated response (TC=1), no transport used + OS->>Trunc: UDP query to 169.254.113.53:53 + Trunc->>OS: truncated response (TC=1), no transport used Note over OS: retries over TCP automatically OS->>StreamDialer: TCP query to 169.254.113.53:53 StreamDialer->>Resolver: TCP query to 1.1.1.1:53 (remapped) @@ -105,30 +131,14 @@ sequenceDiagram StreamDialer->>OS: TCP response ``` -In truncate mode, no transport session is opened for DNS at all — the truncated response is generated locally. Non-DNS UDP traffic still flows through the transport normally (a base transport session is opened lazily on the first non-DNS packet). - -## Dynamic switching +In truncate mode, no transport session is opened for DNS at all — the truncated response is generated locally. -The two modes are wired together by the caller (`configregistry.wrapTransportPairWithOutlineDNS`) using a `DelegatePacketProxy`. The VPN starts in truncate mode (safe default) and switches to forward mode once UDP connectivity is confirmed. It switches back to truncate mode if connectivity is lost. - -```mermaid -flowchart LR - OS["OS (UDP traffic)"] --> ppMain - check["UDP connectivity check
(on network change)"] -->|pass| ppMain - check -->|fail| ppMain - - ppMain{{"DelegatePacketProxy
(ppMain)"}} - ppMain -->|UDP available| ppForward["dnsRedirectPacketProxy
(DNS → resolver via transport)"] - ppMain -->|UDP blocked| ppTrunc["dnsTruncatePacketProxy
(DNS → TC response locally)"] - - ppForward --> ppBase["base PacketProxy
(transport)"] - ppTrunc --> ppBase -``` ## Package contents | File | Description | |------|-------------| -| `forward.go` | `NewDNSRedirectStreamDialer` and `NewDNSRedirectPacketProxy` — redirect DNS to a real resolver | -| `truncate.go` | `NewDNSTruncatePacketProxy` — respond with TC=1 to force TCP retry | -| `helpers.go` | `isEquivalentAddrPort` — address comparison ignoring IPv4-in-IPv6 mapping | +| `interceptor.go` | `NewDNSInterceptor` — Dispatches DNS traffic and handles address translation | +| `forward.go` | `NewDNSForwardPacketProxy` — Closes UDP sessions after the first response (ideal for DNS) | +| `forward.go` | `NewDNSRedirectStreamDialer` — Redirects TCP DNS to a real resolver | +| `helpers.go` | `isEquivalentAddrPort` — Address comparison ignoring IPv4-in-IPv6 mapping | diff --git a/client/go/outline/dnsintercept/forward.go b/client/go/outline/dnsintercept/forward.go index a80d4c6235..375e200de4 100644 --- a/client/go/outline/dnsintercept/forward.go +++ b/client/go/outline/dnsintercept/forward.go @@ -39,47 +39,40 @@ func NewDNSRedirectStreamDialer(base transport.StreamDialer, resolverLinkLocalAd }), nil } -// dnsRedirectPacketProxy wraps another PacketProxy to intercept and redirect DNS packets. -type dnsRedirectPacketProxy struct { - baseProxy network.PacketProxy - resolverLinkLocalAddr, resolverRemoteAddr netip.AddrPort +// dnsForwardPacketProxy wraps another PacketProxy to close the session after the first response. +type dnsForwardPacketProxy struct { + baseProxy network.PacketProxy } -type dnsRedirectPacketReqSender struct { +type dnsForwardPacketReqSender struct { network.PacketRequestSender - fpp *dnsRedirectPacketProxy } -// dnsRedirectPacketRespReceiver intercepts incoming packets from the remote DNS resolver. -// It remaps the source address from the remote resolver back to the local DNS address, -// and closes the underlying session after delivering the first DNS response to free the -// transport session immediately rather than waiting for the idle timeout. -type dnsRedirectPacketRespReceiver struct { +// dnsForwardPacketRespReceiver closes the underlying session after delivering the first packet +// to free the transport session immediately rather than waiting for the idle timeout. +type dnsForwardPacketRespReceiver struct { network.PacketResponseReceiver - fpp *dnsRedirectPacketProxy once sync.Once // ensures the session is closed at most once mu sync.Mutex // protects sender; required for Go memory model correctness - sender network.PacketRequestSender // the request sender to close after first DNS response + sender network.PacketRequestSender // the request sender to close after first response } -var _ network.PacketProxy = (*dnsRedirectPacketProxy)(nil) +var _ network.PacketProxy = (*dnsForwardPacketProxy)(nil) -// NewDNSRedirectPacketProxy creates a PacketProxy to intercept and redirect UDP based DNS packets. -// It intercepts all packets to `resolverLinkLocalAddr` and redirects them to `resolverRemoteAddr` via the `base` PacketProxy. -func NewDNSRedirectPacketProxy(base network.PacketProxy, resolverLinkLocalAddr, resolverRemoteAddr netip.AddrPort) (network.PacketProxy, error) { +// NewDNSForwardPacketProxy creates a PacketProxy that closes the underlying session after the first response. +// This is useful for DNS-over-UDP which is one-shot. +func NewDNSForwardPacketProxy(base network.PacketProxy) (network.PacketProxy, error) { if base == nil { return nil, errors.New("base PacketProxy must be provided") } - return &dnsRedirectPacketProxy{ - baseProxy: base, - resolverLinkLocalAddr: resolverLinkLocalAddr, - resolverRemoteAddr: resolverRemoteAddr, + return &dnsForwardPacketProxy{ + baseProxy: base, }, nil } // NewSession implements PacketProxy.NewSession. -func (fpp *dnsRedirectPacketProxy) NewSession(resp network.PacketResponseReceiver) (_ network.PacketRequestSender, err error) { - wrapper := &dnsRedirectPacketRespReceiver{PacketResponseReceiver: resp, fpp: fpp} +func (fpp *dnsForwardPacketProxy) NewSession(resp network.PacketResponseReceiver) (_ network.PacketRequestSender, err error) { + wrapper := &dnsForwardPacketRespReceiver{PacketResponseReceiver: resp} baseSender, err := fpp.baseProxy.NewSession(wrapper) if err != nil { return nil, err @@ -87,34 +80,17 @@ func (fpp *dnsRedirectPacketProxy) NewSession(resp network.PacketResponseReceive wrapper.mu.Lock() wrapper.sender = baseSender wrapper.mu.Unlock() - return &dnsRedirectPacketReqSender{baseSender, fpp}, nil + return &dnsForwardPacketReqSender{baseSender}, nil } -// WriteTo intercepts outgoing DNS request packets. -// If a packet is destined for the local resolver, it remaps the destination to the remote resolver. -func (req *dnsRedirectPacketReqSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { - if isEquivalentAddrPort(destination, req.fpp.resolverLinkLocalAddr) { - destination = req.fpp.resolverRemoteAddr - } - return req.PacketRequestSender.WriteTo(p, destination) -} - -// WriteFrom intercepts incoming DNS response packets. -// If a packet is received from the remote resolver, it remaps the source address to the local -// resolver and then closes the underlying session. DNS is one-shot (one query, one response), -// so closing immediately frees the transport session rather than holding it open until the 30-second -// write-idle timeout, preventing resource exhaustion under sustained DNS load. -func (resp *dnsRedirectPacketRespReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { - if addr, ok := source.(*net.UDPAddr); ok && isEquivalentAddrPort(addr.AddrPort(), resp.fpp.resolverRemoteAddr) { - source = net.UDPAddrFromAddrPort(resp.fpp.resolverLinkLocalAddr) - n, err := resp.PacketResponseReceiver.WriteFrom(p, source) - resp.once.Do(func() { - resp.mu.Lock() - s := resp.sender - resp.mu.Unlock() - s.Close() - }) - return n, err - } - return resp.PacketResponseReceiver.WriteFrom(p, source) +// WriteFrom intercepts incoming packets and closes the underlying session after the first one. +func (resp *dnsForwardPacketRespReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { + n, err := resp.PacketResponseReceiver.WriteFrom(p, source) + resp.once.Do(func() { + resp.mu.Lock() + s := resp.sender + resp.mu.Unlock() + s.Close() + }) + return n, err } diff --git a/client/go/outline/dnsintercept/forward_test.go b/client/go/outline/dnsintercept/forward_test.go index 9e7a2bf683..50d87fa6c3 100644 --- a/client/go/outline/dnsintercept/forward_test.go +++ b/client/go/outline/dnsintercept/forward_test.go @@ -40,13 +40,13 @@ func (d *lastAddrStreamDialer) DialStream(ctx context.Context, addr string) (tra func TestWrapForwardStreamDialer(t *testing.T) { sd := &lastAddrStreamDialer{} - local := netip.MustParseAddrPort("192.0.2.1:53") - resolver := netip.MustParseAddrPort("8.8.8.8:53") + resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.1:53") + resolverRemoteAddr := netip.MustParseAddrPort("8.8.8.8:53") - _, err := NewDNSRedirectStreamDialer(nil, local, resolver) + _, err := NewDNSRedirectStreamDialer(nil, resolverLinkLocalAddr, resolverRemoteAddr) require.Error(t, err) - dialer, err := NewDNSRedirectStreamDialer(sd, local, resolver) + dialer, err := NewDNSRedirectStreamDialer(sd, resolverLinkLocalAddr, resolverRemoteAddr) require.NoError(t, err) _, err = dialer.DialStream(context.TODO(), "192.0.2.1:53") @@ -106,73 +106,37 @@ func TestWrapForwardPacketProxy(t *testing.T) { pp := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} resp := &lastSourcePacketResponseReceiver{} - local := netip.MustParseAddrPort("192.0.2.2:53") - resolver := netip.MustParseAddrPort("8.8.4.4:53") - resolverUDPAddr := net.UDPAddrFromAddrPort(resolver) - nonResolver := netip.MustParseAddrPort("203.0.113.10:123") - nonResolverUDPAddr := net.UDPAddrFromAddrPort(nonResolver) + resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.2:53") + resolverRemoteAddr := netip.MustParseAddrPort("8.8.4.4:53") + resolverRemoteUDPAddr := net.UDPAddrFromAddrPort(resolverRemoteAddr) - _, err := NewDNSRedirectPacketProxy(nil, local, resolver) + _, err := NewDNSForwardPacketProxy(nil) require.Error(t, err) - fpp, err := NewDNSRedirectPacketProxy(pp, local, resolver) + fpp, err := NewDNSForwardPacketProxy(pp) require.NoError(t, err) - req, err := fpp.NewSession(resp) + // We use an interceptor to handle the remapping + interceptor, err := NewDNSInterceptor(pp, fpp, resolverLinkLocalAddr, resolverRemoteAddr) require.NoError(t, err) - n, err := req.WriteTo([]byte("request"), local) + req, err := interceptor.NewSession(resp) require.NoError(t, err) - require.Equal(t, 7, n) - require.Equal(t, resolver, pp.req.lastDst) - n, err = req.WriteTo([]byte("request"), nonResolver) + n, err := req.WriteTo([]byte("request"), resolverLinkLocalAddr) require.NoError(t, err) require.Equal(t, 7, n) - require.Equal(t, nonResolver, pp.req.lastDst) - - require.NotNil(t, pp.resp) - n, err = pp.resp.WriteFrom([]byte("response"), resolverUDPAddr) - require.NoError(t, err) - require.Equal(t, 8, n) - require.Equal(t, net.UDPAddrFromAddrPort(local), resp.lastSrc) - - // After the first DNS response, the underlying session must be closed immediately - // to free the transport session instead of waiting for the write-idle timeout. - require.True(t, pp.req.closed, "session must be closed after first DNS response") - - n, err = pp.resp.WriteFrom([]byte("response"), nonResolverUDPAddr) - require.NoError(t, err) - require.Equal(t, 8, n) - require.Equal(t, nonResolverUDPAddr, resp.lastSrc) - - // Explicit Close must be safe even though the session was already closed. - require.NoError(t, req.Close()) -} - -// TestWrapForwardPacketProxy_NonDNSResponseDoesNotClose verifies that a non-DNS -// response (not from the resolver) does not trigger an early session close. -func TestWrapForwardPacketProxy_NonDNSResponseDoesNotClose(t *testing.T) { - pp := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} - resp := &lastSourcePacketResponseReceiver{} - - local := netip.MustParseAddrPort("192.0.2.2:53") - resolver := netip.MustParseAddrPort("8.8.4.4:53") - nonResolver := netip.MustParseAddrPort("203.0.113.10:123") - nonResolverUDPAddr := net.UDPAddrFromAddrPort(nonResolver) - - fpp, err := NewDNSRedirectPacketProxy(pp, local, resolver) - require.NoError(t, err) - - req, err := fpp.NewSession(resp) - require.NoError(t, err) + require.Equal(t, resolverRemoteAddr, pp.req.lastDst) - n, err := pp.resp.WriteFrom([]byte("response"), nonResolverUDPAddr) + require.NotNil(t, fpp.(*dnsForwardPacketProxy).baseProxy.(*packetProxyWithGivenRequestSender).resp) + fppResp := fpp.(*dnsForwardPacketProxy).baseProxy.(*packetProxyWithGivenRequestSender).resp + n, err = fppResp.WriteFrom([]byte("response"), resolverRemoteUDPAddr) require.NoError(t, err) require.Equal(t, 8, n) + require.Equal(t, net.UDPAddrFromAddrPort(resolverLinkLocalAddr), resp.lastSrc) - require.False(t, pp.req.closed, "session must not be closed for non-DNS responses") + // After the first response, the underlying session must be closed immediately. + require.True(t, pp.req.closed, "session must be closed after first response") require.NoError(t, req.Close()) - require.True(t, pp.req.closed) } diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go new file mode 100644 index 0000000000..99ef7ebe07 --- /dev/null +++ b/client/go/outline/dnsintercept/interceptor.go @@ -0,0 +1,103 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dnsintercept + +import ( + "errors" + "net" + "net/netip" + + "golang.getoutline.org/sdk/network" +) + +type dnsInterceptor struct { + baseProxy network.PacketProxy + dnsProxy network.PacketProxy + resolverLinkLocalAddr netip.AddrPort + resolverRemoteAddr netip.AddrPort +} + +type dnsInterceptorRequestSender struct { + baseSender network.PacketRequestSender + dnsSender network.PacketRequestSender + resolverLinkLocalAddr netip.AddrPort + resolverRemoteAddr netip.AddrPort +} + +type dnsInterceptorResponseReceiver struct { + network.PacketResponseReceiver + resolverLinkLocalAddr netip.AddrPort + resolverRemoteAddr netip.AddrPort +} + +// NewDNSInterceptor creates a PacketProxy that intercepts packets destined for resolverLinkLocalAddr +// and routes them to dnsProxy, remapping the destination to resolverRemoteAddr. +// All other packets are routed to baseProxy. +// In responses from dnsProxy, it remaps the source address from resolverRemoteAddr back to resolverLinkLocalAddr. +func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolverLinkLocalAddr, resolverRemoteAddr netip.AddrPort) (network.PacketProxy, error) { + if base == nil { + return nil, errors.New("base PacketProxy must be provided") + } + if dns == nil { + return nil, errors.New("dns PacketProxy must be provided") + } + return &dnsInterceptor{ + baseProxy: base, + dnsProxy: dns, + resolverLinkLocalAddr: resolverLinkLocalAddr, + resolverRemoteAddr: resolverRemoteAddr, + }, nil +} + +func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + baseSender, err := i.baseProxy.NewSession(resp) + if err != nil { + return nil, err + } + dnsResp := &dnsInterceptorResponseReceiver{ + PacketResponseReceiver: resp, + resolverLinkLocalAddr: i.resolverLinkLocalAddr, + resolverRemoteAddr: i.resolverRemoteAddr, + } + dnsSender, err := i.dnsProxy.NewSession(dnsResp) + if err != nil { + baseSender.Close() + return nil, err + } + return &dnsInterceptorRequestSender{ + baseSender: baseSender, + dnsSender: dnsSender, + resolverLinkLocalAddr: i.resolverLinkLocalAddr, + resolverRemoteAddr: i.resolverRemoteAddr, + }, nil +} + +func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { + if isEquivalentAddrPort(destination, s.resolverLinkLocalAddr) { + return s.dnsSender.WriteTo(p, s.resolverRemoteAddr) + } + return s.baseSender.WriteTo(p, destination) +} + +func (s *dnsInterceptorRequestSender) Close() error { + return errors.Join(s.baseSender.Close(), s.dnsSender.Close()) +} + +func (r *dnsInterceptorResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { + if addr, ok := source.(*net.UDPAddr); ok && isEquivalentAddrPort(addr.AddrPort(), r.resolverRemoteAddr) { + source = net.UDPAddrFromAddrPort(r.resolverLinkLocalAddr) + } + return r.PacketResponseReceiver.WriteFrom(p, source) +} diff --git a/client/go/outline/dnsintercept/interceptor_test.go b/client/go/outline/dnsintercept/interceptor_test.go new file mode 100644 index 0000000000..38b0f57379 --- /dev/null +++ b/client/go/outline/dnsintercept/interceptor_test.go @@ -0,0 +1,63 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dnsintercept + +import ( + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDNSInterceptor(t *testing.T) { + basePP := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} + dnsPP := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} + resp := &lastSourcePacketResponseReceiver{} + + resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.1:53") + resolverRemoteAddr := netip.MustParseAddrPort("8.8.8.8:53") + otherAddr := netip.MustParseAddrPort("1.1.1.1:443") + + interceptor, err := NewDNSInterceptor(basePP, dnsPP, resolverLinkLocalAddr, resolverRemoteAddr) + require.NoError(t, err) + + req, err := interceptor.NewSession(resp) + require.NoError(t, err) + + // Send to local DNS address -> should be remapped to remote DNS + n, err := req.WriteTo([]byte("dns query"), resolverLinkLocalAddr) + require.NoError(t, err) + require.Equal(t, 9, n) + require.Equal(t, resolverRemoteAddr, dnsPP.req.lastDst) + require.Equal(t, netip.AddrPort{}, basePP.req.lastDst) + + // Receive from remote DNS -> should be remapped to local DNS + require.NotNil(t, dnsPP.resp) + n, err = dnsPP.resp.WriteFrom([]byte("dns response"), net.UDPAddrFromAddrPort(resolverRemoteAddr)) + require.NoError(t, err) + require.Equal(t, 12, n) + require.Equal(t, net.UDPAddrFromAddrPort(resolverLinkLocalAddr), resp.lastSrc) + + // Send to other address -> should go to base and NOT be remapped + n, err = req.WriteTo([]byte("http request"), otherAddr) + require.NoError(t, err) + require.Equal(t, 12, n) + require.Equal(t, otherAddr, basePP.req.lastDst) + + require.NoError(t, req.Close()) + require.True(t, basePP.req.closed) + require.True(t, dnsPP.req.closed) +} diff --git a/client/go/outline/dnsintercept/truncate.go b/client/go/outline/dnsintercept/truncate.go deleted file mode 100644 index d360363211..0000000000 --- a/client/go/outline/dnsintercept/truncate.go +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dnsintercept - -import ( - "errors" - "fmt" - "net/netip" - "sync" - - "golang.getoutline.org/sdk/network" - "golang.getoutline.org/sdk/network/dnstruncate" -) - -type dnsTruncatePacketProxy struct { - network.PacketProxy - truncate53PP network.PacketProxy - resolverLinkLocalAddr netip.AddrPort -} - -// dnsTruncatePacketReqSender handles packet routing for truncate sessions. -// -// DNS packets (destined for local) are handled by trunc and never touch the -// base proxy. The base session is created lazily on the first non-DNS packet, -// avoiding a wasted transport session for DNS-only flows. -type dnsTruncatePacketReqSender struct { - mu sync.Mutex - baseSender network.PacketRequestSender // nil until first non-DNS packet; guarded by mu - baseProxy network.PacketProxy // used to lazily create base - respReceiver network.PacketResponseReceiver // passed to base when it is created - truncate53PP network.PacketRequestSender // handles DNS packets locally without a transport session - resolverLinkLocalAddr netip.AddrPort // the DNS address to intercept -} - -// NewDNSTruncatePacketProxy creates a PacketProxy to intercept UDP-based DNS packets and force a TCP retry. -// -// It intercepts all packets to `resolverLinkLocalAddr` and returns an immediate truncated response, -// prompting the OS to retry the query over TCP. -// -// All other UDP packets are passed through to the `base` PacketProxy. -func NewDNSTruncatePacketProxy(base network.PacketProxy, resolverLinkLocalAddr netip.AddrPort) (network.PacketProxy, error) { - if base == nil { - return nil, errors.New("base PacketProxy must be provided") - } - // Returns truncated responses for *all* traffic on port 53. - truncate53PP, err := dnstruncate.NewPacketProxy() - if err != nil { - return nil, fmt.Errorf("failed to create the underlying DNS truncate PacketProxy: %w", err) - } - return &dnsTruncatePacketProxy{ - PacketProxy: base, - truncate53PP: truncate53PP, - resolverLinkLocalAddr: resolverLinkLocalAddr, - }, nil -} - -// NewSession implements PacketProxy.NewSession. -// -// Only the trunc session is created eagerly. The base session is deferred -// until the first non-DNS packet arrives. -func (tpp *dnsTruncatePacketProxy) NewSession(respReceiver network.PacketResponseReceiver) (_ network.PacketRequestSender, err error) { - trunc, err := tpp.truncate53PP.NewSession(respReceiver) - if err != nil { - return nil, err - } - return &dnsTruncatePacketReqSender{ - baseProxy: tpp.PacketProxy, - respReceiver: respReceiver, - truncate53PP: trunc, - resolverLinkLocalAddr: tpp.resolverLinkLocalAddr, - }, nil -} - -// WriteTo checks if the packet is a DNS query to the local intercept address. -// If so, it truncates the packet. Otherwise, it passes it to the base proxy, -// creating the base session on demand if this is the first non-DNS packet. -func (req *dnsTruncatePacketReqSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { - if isEquivalentAddrPort(destination, req.resolverLinkLocalAddr) { - return req.truncate53PP.WriteTo(p, destination) - } - req.mu.Lock() - if req.baseSender == nil { - base, err := req.baseProxy.NewSession(req.respReceiver) - if err != nil { - req.mu.Unlock() - return 0, err - } - req.baseSender = base - } - sender := req.baseSender - req.mu.Unlock() - return sender.WriteTo(p, destination) -} - -// Close ensures all underlying PacketRequestSenders are closed properly. -func (req *dnsTruncatePacketReqSender) Close() (err error) { - req.mu.Lock() - defer req.mu.Unlock() - if req.baseSender != nil { - err = req.baseSender.Close() - } - return errors.Join(err, req.truncate53PP.Close()) -} diff --git a/client/go/outline/dnsintercept/truncate_test.go b/client/go/outline/dnsintercept/truncate_test.go deleted file mode 100644 index fe67971a6f..0000000000 --- a/client/go/outline/dnsintercept/truncate_test.go +++ /dev/null @@ -1,109 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dnsintercept - -import ( - "net/netip" - "testing" - - "golang.org/x/net/dns/dnsmessage" - - "github.com/stretchr/testify/require" -) - -func TestWrapTruncatePacketProxy(t *testing.T) { - pp := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} - resp := &lastSourcePacketResponseReceiver{} - - local := netip.MustParseAddrPort("192.0.2.2:53") - udpAddr := netip.MustParseAddrPort("203.0.113.10:123") - - _, err := NewDNSTruncatePacketProxy(nil, local) - require.Error(t, err) - - tpp, err := NewDNSTruncatePacketProxy(pp, local) - require.NoError(t, err) - - req, err := tpp.NewSession(resp) - require.NoError(t, err) - - msg := dnsmessage.Message{ - Header: dnsmessage.Header{ID: 1234}, - Questions: []dnsmessage.Question{{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - }}, - } - query, err := msg.Pack() - require.NoError(t, err) - - _, err = req.WriteTo(query, local) - require.NoError(t, err) - require.NotNil(t, resp.lastPacket) - - var p dnsmessage.Parser - header, err := p.Start(resp.lastPacket) - require.NoError(t, err) - require.True(t, header.Response) - require.True(t, header.Truncated) - - _, err = req.WriteTo([]byte("not-a-dns-packet"), udpAddr) - require.NoError(t, err) - require.Equal(t, udpAddr, pp.req.lastDst) - - require.NoError(t, req.Close()) - require.True(t, pp.req.closed) -} - -// TestWrapTruncatePacketProxy_DNSOnlyDoesNotCreateBaseSession verifies that a session -// that only sends DNS packets never allocates a base transport session, which -// prevents resource exhaustion under sustained DNS load. -func TestWrapTruncatePacketProxy_DNSOnlyDoesNotCreateBaseSession(t *testing.T) { - pp := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} - resp := &lastSourcePacketResponseReceiver{} - - local := netip.MustParseAddrPort("192.0.2.2:53") - - tpp, err := NewDNSTruncatePacketProxy(pp, local) - require.NoError(t, err) - - req, err := tpp.NewSession(resp) - require.NoError(t, err) - - // NewSession must not have called pp.NewSession - require.Nil(t, pp.resp, "base session must not be created at NewSession time") - - msg := dnsmessage.Message{ - Header: dnsmessage.Header{ID: 42}, - Questions: []dnsmessage.Question{{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - }}, - } - query, err := msg.Pack() - require.NoError(t, err) - - _, err = req.WriteTo(query, local) - require.NoError(t, err) - - // After a DNS-only send, still no base session - require.Nil(t, pp.resp, "base session must not be created for DNS-only traffic") - - // Close should succeed without creating a base session - require.NoError(t, req.Close()) - require.False(t, pp.req.closed, "base session Close must not be called if session was never created") -} From 01891a2dfe7bb075405bbe9dbf282824acb2532a Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Wed, 15 Apr 2026 19:31:29 -0400 Subject: [PATCH 02/20] refactor(client): simplify dns interceptor lazy initialization --- .../configregistry/outline_dns_intercept.go | 7 +- client/go/outline/dnsintercept/README.md | 23 +-- client/go/outline/dnsintercept/forward.go | 96 ------------ .../go/outline/dnsintercept/forward_test.go | 142 ------------------ client/go/outline/dnsintercept/interceptor.go | 20 ++- .../outline/dnsintercept/interceptor_test.go | 80 ++++++++++ 6 files changed, 108 insertions(+), 260 deletions(-) delete mode 100644 client/go/outline/dnsintercept/forward.go delete mode 100644 client/go/outline/dnsintercept/forward_test.go diff --git a/client/go/outline/configregistry/outline_dns_intercept.go b/client/go/outline/configregistry/outline_dns_intercept.go index 91b0a32ac2..e8b6b74a97 100644 --- a/client/go/outline/configregistry/outline_dns_intercept.go +++ b/client/go/outline/configregistry/outline_dns_intercept.go @@ -62,11 +62,6 @@ func wrapTransportPairWithOutlineDNS(sd *Dialer[transport.StreamConn], pl *Packe if err != nil { return nil, fmt.Errorf("failed to create PacketProxy: %w", err) } - // Forwards DNS packets through ppBase and closes the session after the first response. - ppForward, err := dnsintercept.NewDNSForwardPacketProxy(ppBase) - if err != nil { - return nil, fmt.Errorf("failed to create DNS redirect PacketProxy: %w", err) - } // Returns a truncated response for DNS packets to force a retry over TCP. ppTrunc, err := dnstruncate.NewPacketProxy() if err != nil { @@ -88,7 +83,7 @@ func wrapTransportPairWithOutlineDNS(sd *Dialer[transport.StreamConn], pl *Packe go func() { if err := connectivity.CheckUDPConnectivity(pl); err == nil { slog.Info("remote device UDP is healthy") - ppDNS.SetProxy(ppForward) + ppDNS.SetProxy(ppBase) } else { slog.Warn("remote device UDP is not healthy", "err", err) ppDNS.SetProxy(ppTrunc) diff --git a/client/go/outline/dnsintercept/README.md b/client/go/outline/dnsintercept/README.md index 4bf477cd44..7959e89990 100644 --- a/client/go/outline/dnsintercept/README.md +++ b/client/go/outline/dnsintercept/README.md @@ -70,20 +70,18 @@ The two modes are wired together by the caller (`configregistry.wrapTransportPai ```mermaid flowchart TD OS["OS (UDP traffic)"] --> ppMain - ppMain["DNSInterceptor
(Address remapping)"] + ppMain["DNSInterceptor
(Address remapping and lazy dispatching)"] ppMain -->|Non-DNS| ppBase["base PacketProxy
(transport)"] ppMain -->|DNS| ppDNS["DelegatePacketProxy
(DNS traffic only)"] check["UDP connectivity check
(on network change)"] -->|pass| ppDNS check -->|fail| ppDNS - ppDNS -->|UDP available| ppForward["DNSForwardPacketProxy
(Session lifecycle)"] + ppDNS -->|UDP available| ppBase ppDNS -->|UDP blocked| ppTrunc["dnstruncate.PacketProxy
(TC response locally)"] - - ppForward --> ppBase ``` -The `DNSInterceptor` acts as the primary dispatcher. It routes non-DNS traffic directly to the transport and DNS traffic to a dedicated delegate proxy. This delegate proxy switches between forwarding and truncation based on the result of a periodic UDP connectivity check. +The `DNSInterceptor` acts as the primary dispatcher. It routes non-DNS traffic directly to the transport and DNS traffic to a dedicated delegate proxy. This delegate proxy switches between forwarding and truncation based on the result of a periodic UDP connectivity check. Transport sessions are opened lazily upon receiving the first packet, ensuring resources are only used when needed. ### Forward mode (UDP available) @@ -94,22 +92,18 @@ DNS queries are forwarded over UDP to a public resolver (Cloudflare, Quad9, or O sequenceDiagram participant OS participant Interceptor as DNSInterceptor - participant Forward as DNSForwardPacketProxy participant Transport participant Resolver as Public DNS resolver OS->>Interceptor: UDP query to 169.254.113.53:53 - Interceptor->>Forward: UDP query to 1.1.1.1:53 (remapped) - Forward->>Transport: UDP query to 1.1.1.1:53 + Interceptor->>Transport: UDP query to 1.1.1.1:53 (remapped) Transport->>Resolver: query Resolver->>Transport: response - Transport->>Forward: UDP response from 1.1.1.1:53 - Forward->>Interceptor: UDP response from 1.1.1.1:53 + Transport->>Interceptor: UDP response from 1.1.1.1:53 Interceptor->>OS: response from 169.254.113.53:53 (remapped back) - Note over Forward: session closed immediately after response ``` -Each DNS session (one query/response pair) closes the transport session as soon as the first response is delivered. This keeps resource usage proportional to in-flight queries rather than to recent query rate. +Each DNS session uses a standard transport session. The transport handles the lifecycle, usually timing out after standard UDP idle timeouts. ### Truncate mode (UDP unavailable) @@ -138,7 +132,6 @@ In truncate mode, no transport session is opened for DNS at all — the truncate | File | Description | |------|-------------| -| `interceptor.go` | `NewDNSInterceptor` — Dispatches DNS traffic and handles address translation | -| `forward.go` | `NewDNSForwardPacketProxy` — Closes UDP sessions after the first response (ideal for DNS) | -| `forward.go` | `NewDNSRedirectStreamDialer` — Redirects TCP DNS to a real resolver | +| `interceptor.go` | `NewDNSInterceptor` — Dispatches DNS traffic, handles address translation, and creates sessions lazily | +| `interceptor.go` | `NewDNSRedirectStreamDialer` — Redirects TCP DNS to a real resolver | | `helpers.go` | `isEquivalentAddrPort` — Address comparison ignoring IPv4-in-IPv6 mapping | diff --git a/client/go/outline/dnsintercept/forward.go b/client/go/outline/dnsintercept/forward.go deleted file mode 100644 index 375e200de4..0000000000 --- a/client/go/outline/dnsintercept/forward.go +++ /dev/null @@ -1,96 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dnsintercept - -import ( - "context" - "errors" - "net" - "net/netip" - "sync" - - "golang.getoutline.org/sdk/network" - "golang.getoutline.org/sdk/transport" -) - -// NewDNSRedirectStreamDialer creates a StreamDialer to intercept and redirect TCP based DNS connections. -// It intercepts all TCP connection for `resolverLinkLocalAddr:53` and redirects them to `resolverRemoteAddr` via the `base` StreamDialer. -func NewDNSRedirectStreamDialer(base transport.StreamDialer, resolverLinkLocalAddr, resolverRemoteAddr netip.AddrPort) (transport.StreamDialer, error) { - if base == nil { - return nil, errors.New("base StreamDialer must be provided") - } - return transport.FuncStreamDialer(func(ctx context.Context, targetAddr string) (transport.StreamConn, error) { - if dst, err := netip.ParseAddrPort(targetAddr); err == nil && isEquivalentAddrPort(dst, resolverLinkLocalAddr) { - targetAddr = resolverRemoteAddr.String() - } - return base.DialStream(ctx, targetAddr) - }), nil -} - -// dnsForwardPacketProxy wraps another PacketProxy to close the session after the first response. -type dnsForwardPacketProxy struct { - baseProxy network.PacketProxy -} - -type dnsForwardPacketReqSender struct { - network.PacketRequestSender -} - -// dnsForwardPacketRespReceiver closes the underlying session after delivering the first packet -// to free the transport session immediately rather than waiting for the idle timeout. -type dnsForwardPacketRespReceiver struct { - network.PacketResponseReceiver - once sync.Once // ensures the session is closed at most once - mu sync.Mutex // protects sender; required for Go memory model correctness - sender network.PacketRequestSender // the request sender to close after first response -} - -var _ network.PacketProxy = (*dnsForwardPacketProxy)(nil) - -// NewDNSForwardPacketProxy creates a PacketProxy that closes the underlying session after the first response. -// This is useful for DNS-over-UDP which is one-shot. -func NewDNSForwardPacketProxy(base network.PacketProxy) (network.PacketProxy, error) { - if base == nil { - return nil, errors.New("base PacketProxy must be provided") - } - return &dnsForwardPacketProxy{ - baseProxy: base, - }, nil -} - -// NewSession implements PacketProxy.NewSession. -func (fpp *dnsForwardPacketProxy) NewSession(resp network.PacketResponseReceiver) (_ network.PacketRequestSender, err error) { - wrapper := &dnsForwardPacketRespReceiver{PacketResponseReceiver: resp} - baseSender, err := fpp.baseProxy.NewSession(wrapper) - if err != nil { - return nil, err - } - wrapper.mu.Lock() - wrapper.sender = baseSender - wrapper.mu.Unlock() - return &dnsForwardPacketReqSender{baseSender}, nil -} - -// WriteFrom intercepts incoming packets and closes the underlying session after the first one. -func (resp *dnsForwardPacketRespReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { - n, err := resp.PacketResponseReceiver.WriteFrom(p, source) - resp.once.Do(func() { - resp.mu.Lock() - s := resp.sender - resp.mu.Unlock() - s.Close() - }) - return n, err -} diff --git a/client/go/outline/dnsintercept/forward_test.go b/client/go/outline/dnsintercept/forward_test.go deleted file mode 100644 index 50d87fa6c3..0000000000 --- a/client/go/outline/dnsintercept/forward_test.go +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dnsintercept - -import ( - "context" - "errors" - "net" - "net/netip" - "testing" - - "github.com/stretchr/testify/require" - "golang.getoutline.org/sdk/network" - "golang.getoutline.org/sdk/transport" -) - -// ----- forward StreamDialer tests ----- - -type lastAddrStreamDialer struct { - transport.StreamDialer - dialedAddr string -} - -func (d *lastAddrStreamDialer) DialStream(ctx context.Context, addr string) (transport.StreamConn, error) { - d.dialedAddr = addr - return nil, errors.New("not used in test") -} - -func TestWrapForwardStreamDialer(t *testing.T) { - sd := &lastAddrStreamDialer{} - resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.1:53") - resolverRemoteAddr := netip.MustParseAddrPort("8.8.8.8:53") - - _, err := NewDNSRedirectStreamDialer(nil, resolverLinkLocalAddr, resolverRemoteAddr) - require.Error(t, err) - - dialer, err := NewDNSRedirectStreamDialer(sd, resolverLinkLocalAddr, resolverRemoteAddr) - require.NoError(t, err) - - _, err = dialer.DialStream(context.TODO(), "192.0.2.1:53") - require.Error(t, err) - require.Equal(t, "8.8.8.8:53", sd.dialedAddr) - - _, err = dialer.DialStream(context.TODO(), "198.51.100.1:443") - require.Error(t, err) - require.Equal(t, "198.51.100.1:443", sd.dialedAddr) -} - -// ----- forward PacketProxy tests ----- - -type packetProxyWithGivenRequestSender struct { - network.PacketProxy - req *lastDestPacketRequestSender - resp network.PacketResponseReceiver -} - -func (p *packetProxyWithGivenRequestSender) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { - p.resp = resp - return p.req, nil -} - -type lastDestPacketRequestSender struct { - lastDst netip.AddrPort - closed bool -} - -func (s *lastDestPacketRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { - s.lastDst = destination - return len(p), nil -} - -func (s *lastDestPacketRequestSender) Close() error { - s.closed = true - return nil -} - -type lastSourcePacketResponseReceiver struct { - lastSrc net.Addr - lastPacket []byte -} - -func (r *lastSourcePacketResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { - r.lastSrc = source - r.lastPacket = make([]byte, len(p)) - copy(r.lastPacket, p) - return len(p), nil -} - -func (r *lastSourcePacketResponseReceiver) Close() error { - return nil -} - -func TestWrapForwardPacketProxy(t *testing.T) { - pp := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} - resp := &lastSourcePacketResponseReceiver{} - - resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.2:53") - resolverRemoteAddr := netip.MustParseAddrPort("8.8.4.4:53") - resolverRemoteUDPAddr := net.UDPAddrFromAddrPort(resolverRemoteAddr) - - _, err := NewDNSForwardPacketProxy(nil) - require.Error(t, err) - - fpp, err := NewDNSForwardPacketProxy(pp) - require.NoError(t, err) - - // We use an interceptor to handle the remapping - interceptor, err := NewDNSInterceptor(pp, fpp, resolverLinkLocalAddr, resolverRemoteAddr) - require.NoError(t, err) - - req, err := interceptor.NewSession(resp) - require.NoError(t, err) - - n, err := req.WriteTo([]byte("request"), resolverLinkLocalAddr) - require.NoError(t, err) - require.Equal(t, 7, n) - require.Equal(t, resolverRemoteAddr, pp.req.lastDst) - - require.NotNil(t, fpp.(*dnsForwardPacketProxy).baseProxy.(*packetProxyWithGivenRequestSender).resp) - fppResp := fpp.(*dnsForwardPacketProxy).baseProxy.(*packetProxyWithGivenRequestSender).resp - n, err = fppResp.WriteFrom([]byte("response"), resolverRemoteUDPAddr) - require.NoError(t, err) - require.Equal(t, 8, n) - require.Equal(t, net.UDPAddrFromAddrPort(resolverLinkLocalAddr), resp.lastSrc) - - // After the first response, the underlying session must be closed immediately. - require.True(t, pp.req.closed, "session must be closed after first response") - - require.NoError(t, req.Close()) -} diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index 99ef7ebe07..d22bd70428 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -13,15 +13,30 @@ // limitations under the License. package dnsintercept - import ( + "context" "errors" "net" "net/netip" "golang.getoutline.org/sdk/network" + "golang.getoutline.org/sdk/transport" ) +// NewDNSRedirectStreamDialer creates a StreamDialer to intercept and redirect TCP based DNS connections. +// It intercepts all TCP connection for `resolverLinkLocalAddr:53` and redirects them to `resolverRemoteAddr` via the `base` StreamDialer. +func NewDNSRedirectStreamDialer(base transport.StreamDialer, resolverLinkLocalAddr, resolverRemoteAddr netip.AddrPort) (transport.StreamDialer, error) { + if base == nil { + return nil, errors.New("base StreamDialer must be provided") + } + return transport.FuncStreamDialer(func(ctx context.Context, targetAddr string) (transport.StreamConn, error) { + if dst, err := netip.ParseAddrPort(targetAddr); err == nil && isEquivalentAddrPort(dst, resolverLinkLocalAddr) { + targetAddr = resolverRemoteAddr.String() + } + return base.DialStream(ctx, targetAddr) + }), nil +} + type dnsInterceptor struct { baseProxy network.PacketProxy dnsProxy network.PacketProxy @@ -62,6 +77,7 @@ func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolv } func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + // TODO(fortuna): create these sessions on demand, on first write. baseSender, err := i.baseProxy.NewSession(resp) if err != nil { return nil, err @@ -85,6 +101,8 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ } func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { + // TODO(fortuna): use something like s.getDNSSession().WriteTo() and s.getForwardSession().WriteTo() + // to create the session on demand. if isEquivalentAddrPort(destination, s.resolverLinkLocalAddr) { return s.dnsSender.WriteTo(p, s.resolverRemoteAddr) } diff --git a/client/go/outline/dnsintercept/interceptor_test.go b/client/go/outline/dnsintercept/interceptor_test.go index 38b0f57379..82e1f5d9cd 100644 --- a/client/go/outline/dnsintercept/interceptor_test.go +++ b/client/go/outline/dnsintercept/interceptor_test.go @@ -15,13 +15,93 @@ package dnsintercept import ( + "context" + "errors" "net" "net/netip" "testing" "github.com/stretchr/testify/require" + "golang.getoutline.org/sdk/network" + "golang.getoutline.org/sdk/transport" ) +// ----- StreamDialer tests ----- + +type lastAddrStreamDialer struct { + transport.StreamDialer + dialedAddr string +} + +func (d *lastAddrStreamDialer) DialStream(ctx context.Context, addr string) (transport.StreamConn, error) { + d.dialedAddr = addr + return nil, errors.New("not used in test") +} + +func TestWrapForwardStreamDialer(t *testing.T) { + sd := &lastAddrStreamDialer{} + resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.1:53") + resolverRemoteAddr := netip.MustParseAddrPort("8.8.8.8:53") + + _, err := NewDNSRedirectStreamDialer(nil, resolverLinkLocalAddr, resolverRemoteAddr) + require.Error(t, err) + + dialer, err := NewDNSRedirectStreamDialer(sd, resolverLinkLocalAddr, resolverRemoteAddr) + require.NoError(t, err) + + _, err = dialer.DialStream(context.TODO(), "192.0.2.1:53") + require.Error(t, err) + require.Equal(t, "8.8.8.8:53", sd.dialedAddr) + + _, err = dialer.DialStream(context.TODO(), "198.51.100.1:443") + require.Error(t, err) + require.Equal(t, "198.51.100.1:443", sd.dialedAddr) +} + +// ----- PacketProxy tests ----- + +type packetProxyWithGivenRequestSender struct { + network.PacketProxy + req *lastDestPacketRequestSender + resp network.PacketResponseReceiver +} + +func (p *packetProxyWithGivenRequestSender) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + p.resp = resp + return p.req, nil +} + +type lastDestPacketRequestSender struct { + lastDst netip.AddrPort + closed bool +} + +func (s *lastDestPacketRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { + s.lastDst = destination + return len(p), nil +} + +func (s *lastDestPacketRequestSender) Close() error { + s.closed = true + return nil +} + +type lastSourcePacketResponseReceiver struct { + lastSrc net.Addr + lastPacket []byte +} + +func (r *lastSourcePacketResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { + r.lastSrc = source + r.lastPacket = make([]byte, len(p)) + copy(r.lastPacket, p) + return len(p), nil +} + +func (r *lastSourcePacketResponseReceiver) Close() error { + return nil +} + func TestDNSInterceptor(t *testing.T) { basePP := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} dnsPP := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} From a5a5ec5f58d441d6c6ade3a51172f219a7a3dbc0 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Wed, 15 Apr 2026 19:31:56 -0400 Subject: [PATCH 03/20] Remove helpers.go --- client/go/outline/dnsintercept/helpers.go | 21 ------------------- client/go/outline/dnsintercept/interceptor.go | 5 +++++ 2 files changed, 5 insertions(+), 21 deletions(-) delete mode 100644 client/go/outline/dnsintercept/helpers.go diff --git a/client/go/outline/dnsintercept/helpers.go b/client/go/outline/dnsintercept/helpers.go deleted file mode 100644 index 8d12e35dfb..0000000000 --- a/client/go/outline/dnsintercept/helpers.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dnsintercept - -import "net/netip" - -func isEquivalentAddrPort(addr1, addr2 netip.AddrPort) bool { - return addr1.Addr().Unmap() == addr2.Addr().Unmap() && addr1.Port() == addr2.Port() -} diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index d22bd70428..afbcee567c 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -13,6 +13,7 @@ // limitations under the License. package dnsintercept + import ( "context" "errors" @@ -23,6 +24,10 @@ import ( "golang.getoutline.org/sdk/transport" ) +func isEquivalentAddrPort(addr1, addr2 netip.AddrPort) bool { + return addr1.Addr().Unmap() == addr2.Addr().Unmap() && addr1.Port() == addr2.Port() +} + // NewDNSRedirectStreamDialer creates a StreamDialer to intercept and redirect TCP based DNS connections. // It intercepts all TCP connection for `resolverLinkLocalAddr:53` and redirects them to `resolverRemoteAddr` via the `base` StreamDialer. func NewDNSRedirectStreamDialer(base transport.StreamDialer, resolverLinkLocalAddr, resolverRemoteAddr netip.AddrPort) (transport.StreamDialer, error) { From a10b32d6d08034df450a90b86026d4fc6a477bd1 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Mon, 27 Apr 2026 22:25:15 -0400 Subject: [PATCH 04/20] Add notes --- client/go/outline/dnsintercept/interceptor.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index afbcee567c..ae671561f9 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -82,6 +82,27 @@ func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolv } func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + // Desired logic: + // - If this is a single request DNS session, it should send the one request, and immediately close on response. + // timeout should be short. + // - If this is a multiple DNS session (not usual, and against best practices), it should close immediately when + // all requests return or timeout. This is a generalization of the previous case. + // - If this is a regular tunnel session, it should use regular timeout, set on each write. + // + // The session type can be determined on the first packet, based on the target endpoint. The assumption here is that + // the link local enpoint will be used solely for dns, and that DNS won't reuse other sockets. + // + // The session should be created on first packet, to avoid creating two sockets when we just need one. + // + // Closing behavior + // + // On session end, we should close the PacketResponseReceiver, so the caller knows the session is over an can clean up. + // In that case, we shouldn't receive any more writes, except due to race conditions. It should be enough to return ErrClosed. + // The caller should start a new session if they want to use the same address again after close. + // + // We should react to closing signal from the caller too. Meaning, if our sender gets a close, we should close + // the inner sender we created too. The response receiver should return ErrClosed on incoming packets after close. + // // TODO(fortuna): create these sessions on demand, on first write. baseSender, err := i.baseProxy.NewSession(resp) if err != nil { From 4e45a16bd7bcd5db7a628c8fe388bb79e9a34181 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Mon, 27 Apr 2026 22:49:07 -0400 Subject: [PATCH 05/20] Update notes --- client/go/outline/dnsintercept/interceptor.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index ae671561f9..bd2d572b4b 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -83,16 +83,11 @@ func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolv func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { // Desired logic: - // - If this is a single request DNS session, it should send the one request, and immediately close on response. - // timeout should be short. - // - If this is a multiple DNS session (not usual, and against best practices), it should close immediately when - // all requests return or timeout. This is a generalization of the previous case. - // - If this is a regular tunnel session, it should use regular timeout, set on each write. - // - // The session type can be determined on the first packet, based on the target endpoint. The assumption here is that - // the link local enpoint will be used solely for dns, and that DNS won't reuse other sockets. - // - // The session should be created on first packet, to avoid creating two sockets when we just need one. + // - Timeouts are always set on Writes. Set ReadDeadline to max(currentDeadline, Now() + timeout). + // - The default timeout is 5m. for DNS, it's 17s. + // - On sender WriteTo, call getSender(isDNS) to lazily create the session. + // - On first read, if it's DNS, set deadline to Now() to end session. + // - On timeout, close everything. Consider returning EOF the first time. // // Closing behavior // @@ -103,7 +98,9 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ // We should react to closing signal from the caller too. Meaning, if our sender gets a close, we should close // the inner sender we created too. The response receiver should return ErrClosed on incoming packets after close. // - // TODO(fortuna): create these sessions on demand, on first write. + // Timeouts (as used in https://github.com/OutlineFoundation/tunnel-server/blob/master/service/udp.go): + // - default: 5m - A UDP NAT timeout of at least 5 minutes is recommended in RFC 4787 Section 4.3. + // - DNS: 17s - shortest timeout, as required by RFC 5452 Section 10. baseSender, err := i.baseProxy.NewSession(resp) if err != nil { return nil, err From d0cf3b3643feeaa5b4a626925a320628d8733d23 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Mon, 27 Apr 2026 22:57:48 -0400 Subject: [PATCH 06/20] More notes --- client/go/outline/dnsintercept/interceptor.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index bd2d572b4b..aff0046cae 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -89,6 +89,13 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ // - On first read, if it's DNS, set deadline to Now() to end session. // - On timeout, close everything. Consider returning EOF the first time. // + // Open Questions: + // - should the timeout be in the underlying proxies instead? NewPacketProxyFromPacketListener + // uses PacketListenerProxy with a 30s idle timeout already. That way this dispatcher doesn't need to know + // the values. + // - What happens if the association has a mix of dns and non dns writes? + // - What error does the caller expects to close the association? Timeout? EOF? ErrClosed? + // // Closing behavior // // On session end, we should close the PacketResponseReceiver, so the caller knows the session is over an can clean up. From 8b4384182da3d41ba86b9253a0414079fd4740ff Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Wed, 29 Apr 2026 10:19:53 -0400 Subject: [PATCH 07/20] Error note --- client/go/outline/dnsintercept/interceptor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index aff0046cae..a3522f600d 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -94,7 +94,8 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ // uses PacketListenerProxy with a 30s idle timeout already. That way this dispatcher doesn't need to know // the values. // - What happens if the association has a mix of dns and non dns writes? - // - What error does the caller expects to close the association? Timeout? EOF? ErrClosed? + // - What error does the caller expects to close the association? Timeout? EOF? ErrClosed? I think we just need to + // close the receiver. // // Closing behavior // From b783562dcd5f54c57bc6d467fcd81cc42ebc86ba Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 12:21:21 -0400 Subject: [PATCH 08/20] Shorter timeout for DNS --- .../configregistry/outline_dns_intercept.go | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/client/go/outline/configregistry/outline_dns_intercept.go b/client/go/outline/configregistry/outline_dns_intercept.go index e8b6b74a97..be5412eca2 100644 --- a/client/go/outline/configregistry/outline_dns_intercept.go +++ b/client/go/outline/configregistry/outline_dns_intercept.go @@ -19,6 +19,7 @@ import ( "log/slog" "math/rand/v2" "net/netip" + "time" "localhost/client/go/outline/connectivity" "localhost/client/go/outline/dnsintercept" @@ -58,23 +59,31 @@ func wrapTransportPairWithOutlineDNS(sd *Dialer[transport.StreamConn], pl *Packe } // Intercept DNS for PacketProxy - ppBase, err := network.NewPacketProxyFromPacketListener(pl) + + // PacketProxy for connecting to remote servers. + // Uses the 5m timeout as recommended in https://www.rfc-editor.org/rfc/rfc4787.html#section-4.3 + ppBase, err := network.NewPacketProxyFromPacketListener(pl, network.WithPacketListenerWriteIdleTimeout(5 * time.Minute)) + if err != nil { + return nil, fmt.Errorf("failed to create PacketProxy: %w", err) + } + // PacketProxy for DNS. Uses a shorter timeout, as recommended in https://www.rfc-editor.org/rfc/rfc5452.html#section-10. + ppDNSBase, err := network.NewPacketProxyFromPacketListener(pl, network.WithPacketListenerWriteIdleTimeout(10 * time.Second)) if err != nil { return nil, fmt.Errorf("failed to create PacketProxy: %w", err) } // Returns a truncated response for DNS packets to force a retry over TCP. - ppTrunc, err := dnstruncate.NewPacketProxy() + ppDNSTrunc, err := dnstruncate.NewPacketProxy() if err != nil { return nil, fmt.Errorf("failed to create always-truncate DNS PacketProxy: %w", err) } // Delegate for DNS traffic: selects between forwarding and truncation based on connectivity. - ppDNS, err := network.NewDelegatePacketProxy(ppTrunc) + ppDNSDelegate, err := network.NewDelegatePacketProxy(ppDNSTrunc) if err != nil { return nil, fmt.Errorf("failed to create indirect DNS PacketProxy: %w", err) } // Interceptor: Forwards everything except DNS to ppBase. DNS is redirected to ppDNS and // translated between the link-local and remote addresses. - ppMain, err := dnsintercept.NewDNSInterceptor(ppBase, ppDNS, linkLocalDNS, remoteDNS) + ppMain, err := dnsintercept.NewDNSInterceptor(ppBase, ppDNSDelegate, linkLocalDNS, remoteDNS) if err != nil { return nil, fmt.Errorf("failed to create DNS interceptor PacketProxy: %w", err) } @@ -83,10 +92,10 @@ func wrapTransportPairWithOutlineDNS(sd *Dialer[transport.StreamConn], pl *Packe go func() { if err := connectivity.CheckUDPConnectivity(pl); err == nil { slog.Info("remote device UDP is healthy") - ppDNS.SetProxy(ppBase) + ppDNSDelegate.SetProxy(ppDNSBase) } else { slog.Warn("remote device UDP is not healthy", "err", err) - ppDNS.SetProxy(ppTrunc) + ppDNSDelegate.SetProxy(ppDNSTrunc) } }() } From 76af39ef95bb2fd5e07664c9676024af8e7458cc Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 15:01:53 -0400 Subject: [PATCH 09/20] Introduce lazy packet proxy Co-authored-by: Copilot --- client/go/outline/dnsintercept/interceptor.go | 96 +++++++----- .../outline/dnsintercept/interceptor_test.go | 32 ++++ .../outline/dnsintercept/lazy_packet_proxy.go | 82 +++++++++++ .../dnsintercept/lazy_packet_proxy_test.go | 137 ++++++++++++++++++ 4 files changed, 309 insertions(+), 38 deletions(-) create mode 100644 client/go/outline/dnsintercept/lazy_packet_proxy.go create mode 100644 client/go/outline/dnsintercept/lazy_packet_proxy_test.go diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index a3522f600d..c1e7ce6a22 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -19,6 +19,7 @@ import ( "errors" "net" "net/netip" + "sync" "golang.getoutline.org/sdk/network" "golang.getoutline.org/sdk/transport" @@ -49,19 +50,6 @@ type dnsInterceptor struct { resolverRemoteAddr netip.AddrPort } -type dnsInterceptorRequestSender struct { - baseSender network.PacketRequestSender - dnsSender network.PacketRequestSender - resolverLinkLocalAddr netip.AddrPort - resolverRemoteAddr netip.AddrPort -} - -type dnsInterceptorResponseReceiver struct { - network.PacketResponseReceiver - resolverLinkLocalAddr netip.AddrPort - resolverRemoteAddr netip.AddrPort -} - // NewDNSInterceptor creates a PacketProxy that intercepts packets destined for resolverLinkLocalAddr // and routes them to dnsProxy, remapping the destination to resolverRemoteAddr. // All other packets are routed to baseProxy. @@ -74,25 +62,16 @@ func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolv return nil, errors.New("dns PacketProxy must be provided") } return &dnsInterceptor{ - baseProxy: base, - dnsProxy: dns, + // We use lazy proxies, so that we only create target sockets when needed. + baseProxy: &lazyPacketProxy{baseProxy: base}, + dnsProxy: &lazyPacketProxy{baseProxy: dns}, resolverLinkLocalAddr: resolverLinkLocalAddr, resolverRemoteAddr: resolverRemoteAddr, }, nil } func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { - // Desired logic: - // - Timeouts are always set on Writes. Set ReadDeadline to max(currentDeadline, Now() + timeout). - // - The default timeout is 5m. for DNS, it's 17s. - // - On sender WriteTo, call getSender(isDNS) to lazily create the session. - // - On first read, if it's DNS, set deadline to Now() to end session. - // - On timeout, close everything. Consider returning EOF the first time. - // // Open Questions: - // - should the timeout be in the underlying proxies instead? NewPacketProxyFromPacketListener - // uses PacketListenerProxy with a 30s idle timeout already. That way this dispatcher doesn't need to know - // the values. // - What happens if the association has a mix of dns and non dns writes? // - What error does the caller expects to close the association? Timeout? EOF? ErrClosed? I think we just need to // close the receiver. @@ -105,24 +84,22 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ // // We should react to closing signal from the caller too. Meaning, if our sender gets a close, we should close // the inner sender we created too. The response receiver should return ErrClosed on incoming packets after close. - // - // Timeouts (as used in https://github.com/OutlineFoundation/tunnel-server/blob/master/service/udp.go): - // - default: 5m - A UDP NAT timeout of at least 5 minutes is recommended in RFC 4787 Section 4.3. - // - DNS: 17s - shortest timeout, as required by RFC 5452 Section 10. baseSender, err := i.baseProxy.NewSession(resp) if err != nil { return nil, err } - dnsResp := &dnsInterceptorResponseReceiver{ + + dnsResp := &natResponseReceiver{ PacketResponseReceiver: resp, - resolverLinkLocalAddr: i.resolverLinkLocalAddr, - resolverRemoteAddr: i.resolverRemoteAddr, + localAddr: i.resolverLinkLocalAddr, + remoteAddr: i.resolverRemoteAddr, } dnsSender, err := i.dnsProxy.NewSession(dnsResp) if err != nil { baseSender.Close() return nil, err } + return &dnsInterceptorRequestSender{ baseSender: baseSender, dnsSender: dnsSender, @@ -131,9 +108,23 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ }, nil } +type dnsInterceptorRequestSender struct { + closeMu sync.Mutex + isClosed bool + baseSender network.PacketRequestSender + dnsSender network.PacketRequestSender + resolverLinkLocalAddr netip.AddrPort + resolverRemoteAddr netip.AddrPort +} + func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { - // TODO(fortuna): use something like s.getDNSSession().WriteTo() and s.getForwardSession().WriteTo() - // to create the session on demand. + s.closeMu.Lock() + defer s.closeMu.Unlock() + + if s.isClosed { + return 0, net.ErrClosed + } + if isEquivalentAddrPort(destination, s.resolverLinkLocalAddr) { return s.dnsSender.WriteTo(p, s.resolverRemoteAddr) } @@ -141,12 +132,41 @@ func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPo } func (s *dnsInterceptorRequestSender) Close() error { - return errors.Join(s.baseSender.Close(), s.dnsSender.Close()) + s.closeMu.Lock() + if s.isClosed { + s.closeMu.Unlock() + return net.ErrClosed + } + s.isClosed = true + + baseSender := s.baseSender + s.baseSender = nil + dnsSender := s.dnsSender + s.dnsSender = nil + + s.closeMu.Unlock() + + // We close the underlying senders outside the lock, in case they are slow or try to write somehow. + var joinError error + if baseSender != nil { + joinError = baseSender.Close() + } + if dnsSender != nil { + joinError = errors.Join(joinError, dnsSender.Close()) + } + return joinError +} + +// natResponseReceiver is a simple PacketResponseReceiver that translates an external address to an internal one. +type natResponseReceiver struct { + network.PacketResponseReceiver + remoteAddr netip.AddrPort + localAddr netip.AddrPort } -func (r *dnsInterceptorResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { - if addr, ok := source.(*net.UDPAddr); ok && isEquivalentAddrPort(addr.AddrPort(), r.resolverRemoteAddr) { - source = net.UDPAddrFromAddrPort(r.resolverLinkLocalAddr) +func (r *natResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { + if addr, ok := source.(*net.UDPAddr); ok && isEquivalentAddrPort(addr.AddrPort(), r.remoteAddr) { + source = net.UDPAddrFromAddrPort(r.localAddr) } return r.PacketResponseReceiver.WriteFrom(p, source) } diff --git a/client/go/outline/dnsintercept/interceptor_test.go b/client/go/outline/dnsintercept/interceptor_test.go index 82e1f5d9cd..aa8e3218f9 100644 --- a/client/go/outline/dnsintercept/interceptor_test.go +++ b/client/go/outline/dnsintercept/interceptor_test.go @@ -141,3 +141,35 @@ func TestDNSInterceptor(t *testing.T) { require.True(t, basePP.req.closed) require.True(t, dnsPP.req.closed) } + +// ----- natResponseReceiver tests ----- + +func TestNatResponseReceiver(t *testing.T) { + innerResp := &lastSourcePacketResponseReceiver{} + localAddr := netip.MustParseAddrPort("192.0.2.1:53") + remoteAddr := netip.MustParseAddrPort("8.8.8.8:53") + otherAddr := netip.MustParseAddrPort("1.1.1.1:443") + packet := []byte("response") + + receiver := &natResponseReceiver{ + PacketResponseReceiver: innerResp, + localAddr: localAddr, + remoteAddr: remoteAddr, + } + + // Packet from remoteAddr should be translated to localAddr + n, err := receiver.WriteFrom(packet, net.UDPAddrFromAddrPort(remoteAddr)) + require.NoError(t, err) + require.Equal(t, len(packet), n) + require.Equal(t, net.UDPAddrFromAddrPort(localAddr), innerResp.lastSrc) + require.Equal(t, packet, innerResp.lastPacket) + + // Packet from another address should be passed through + n, err = receiver.WriteFrom(packet, net.UDPAddrFromAddrPort(otherAddr)) + require.NoError(t, err) + require.Equal(t, len(packet), n) + require.Equal(t, net.UDPAddrFromAddrPort(otherAddr), innerResp.lastSrc) + require.Equal(t, packet, innerResp.lastPacket) +} + + diff --git a/client/go/outline/dnsintercept/lazy_packet_proxy.go b/client/go/outline/dnsintercept/lazy_packet_proxy.go new file mode 100644 index 0000000000..64907d8d0b --- /dev/null +++ b/client/go/outline/dnsintercept/lazy_packet_proxy.go @@ -0,0 +1,82 @@ +// Copyright 2026 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dnsintercept + +import ( + "net" + "net/netip" + "sync" + + "golang.getoutline.org/sdk/network" +) + +// lazyPacketProxy is a PacketProxy that creates sessions on demand on first WriteTo. +type lazyPacketProxy struct { + baseProxy network.PacketProxy +} + +type lazyPacketProxyRequestSender struct { + mu sync.Mutex + newSessionFunc func() (network.PacketRequestSender, error) + sender network.PacketRequestSender + isClosed bool +} + +func (p *lazyPacketProxy) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + return &lazyPacketProxyRequestSender{ + newSessionFunc: func() (network.PacketRequestSender, error) { + return p.baseProxy.NewSession(resp) + }, + }, nil +} + +func (s *lazyPacketProxyRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + + if s.isClosed { + return 0, net.ErrClosed + } + + if s.sender == nil { + sender, err := s.newSessionFunc() + if err != nil { + return 0, err + } + s.sender = sender + } + return s.sender.WriteTo(p, destination) +} + +func (s *lazyPacketProxyRequestSender) Close() error { + s.mu.Lock() + + if s.isClosed { + s.mu.Unlock() + return net.ErrClosed + } + s.isClosed = true + + sender := s.sender + s.sender = nil + + s.mu.Unlock() + + // We close the underlying senders outside the lock, in case they are slow or try to write somehow. + if sender != nil { + return sender.Close() + } + return nil +} diff --git a/client/go/outline/dnsintercept/lazy_packet_proxy_test.go b/client/go/outline/dnsintercept/lazy_packet_proxy_test.go new file mode 100644 index 0000000000..557e30cc61 --- /dev/null +++ b/client/go/outline/dnsintercept/lazy_packet_proxy_test.go @@ -0,0 +1,137 @@ +// Copyright 2026 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dnsintercept + +import ( + "errors" + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/require" + "golang.getoutline.org/sdk/network" +) + +type mockPacketProxy struct { + network.PacketProxy + newSessionCalled bool + newSessionErr error + reqSender network.PacketRequestSender +} + +func (p *mockPacketProxy) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + p.newSessionCalled = true + if p.newSessionErr != nil { + return nil, p.newSessionErr + } + if p.reqSender == nil { + p.reqSender = &lastDestPacketRequestSender{} + } + return p.reqSender, nil +} + +func TestLazyPacketProxy_CloseBeforeWrite(t *testing.T) { + basePP := &mockPacketProxy{} + proxy := &lazyPacketProxy{baseProxy: basePP} + resp := &lastSourcePacketResponseReceiver{} + + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + + err = sender.Close() + require.NoError(t, err) + + require.False(t, basePP.newSessionCalled) +} + +func TestLazyPacketProxy_WriteToCreatesSession(t *testing.T) { + basePP := &mockPacketProxy{} + proxy := &lazyPacketProxy{baseProxy: basePP} + resp := &lastSourcePacketResponseReceiver{} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + require.False(t, basePP.newSessionCalled) + + dest := netip.MustParseAddrPort("1.1.1.1:53") + n, err := sender.WriteTo([]byte("test"), dest) + require.NoError(t, err) + require.Equal(t, 4, n) + + require.True(t, basePP.newSessionCalled) + require.Equal(t, dest, basePP.reqSender.(*lastDestPacketRequestSender).lastDst) +} + +func TestLazyPacketProxy_WriteToUsesExistingSession(t *testing.T) { + basePP := &mockPacketProxy{} + proxy := &lazyPacketProxy{baseProxy: basePP} + resp := &lastSourcePacketResponseReceiver{} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + + _, err = sender.WriteTo([]byte("test"), netip.MustParseAddrPort("1.1.1.1:53")) + require.NoError(t, err) + require.True(t, basePP.newSessionCalled) + + basePP.newSessionCalled = false // Reset for next check + _, err = sender.WriteTo([]byte("test2"), netip.MustParseAddrPort("2.2.2.2:53")) + require.NoError(t, err) + require.False(t, basePP.newSessionCalled) +} + +func TestLazyPacketProxy_WriteToFailsOnSessionError(t *testing.T) { + expectedErr := errors.New("session failed") + basePP := &mockPacketProxy{newSessionErr: expectedErr} + proxy := &lazyPacketProxy{baseProxy: basePP} + resp := &lastSourcePacketResponseReceiver{} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + + _, err = sender.WriteTo([]byte("test"), netip.MustParseAddrPort("1.1.1.1:53")) + require.ErrorIs(t, err, expectedErr) +} + +func TestLazyPacketProxy_WriteToAfterClose(t *testing.T) { + proxy := &lazyPacketProxy{baseProxy: &mockPacketProxy{}} + sender, err := proxy.NewSession(&lastSourcePacketResponseReceiver{}) + require.NoError(t, err) + + require.NoError(t, sender.Close()) + + _, err = sender.WriteTo([]byte("test"), netip.MustParseAddrPort("1.1.1.1:53")) + require.ErrorIs(t, err, net.ErrClosed) +} + +func TestLazyPacketProxy_CloseAfterWriteTo(t *testing.T) { + basePP := &mockPacketProxy{} + proxy := &lazyPacketProxy{baseProxy: basePP} + sender, err := proxy.NewSession(&lastSourcePacketResponseReceiver{}) + require.NoError(t, err) + + _, err = sender.WriteTo([]byte("test"), netip.MustParseAddrPort("1.1.1.1:53")) + require.NoError(t, err) + + require.NoError(t, sender.Close()) + require.True(t, basePP.reqSender.(*lastDestPacketRequestSender).closed) +} + +func TestLazyPacketProxy_CloseTwice(t *testing.T) { + proxy := &lazyPacketProxy{baseProxy: &mockPacketProxy{}} + sender, err := proxy.NewSession(&lastSourcePacketResponseReceiver{}) + require.NoError(t, err) + + require.NoError(t, sender.Close()) + err = sender.Close() + require.ErrorIs(t, err, net.ErrClosed) +} From e285a46caf8307cf79de438b997930f164e4d91b Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 15:09:32 -0400 Subject: [PATCH 10/20] Update TODO --- client/go/outline/dnsintercept/interceptor.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index c1e7ce6a22..950cf9013c 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -71,24 +71,12 @@ func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolv } func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { - // Open Questions: - // - What happens if the association has a mix of dns and non dns writes? - // - What error does the caller expects to close the association? Timeout? EOF? ErrClosed? I think we just need to - // close the receiver. - // - // Closing behavior - // - // On session end, we should close the PacketResponseReceiver, so the caller knows the session is over an can clean up. - // In that case, we shouldn't receive any more writes, except due to race conditions. It should be enough to return ErrClosed. - // The caller should start a new session if they want to use the same address again after close. - // - // We should react to closing signal from the caller too. Meaning, if our sender gets a close, we should close - // the inner sender we created too. The response receiver should return ErrClosed on incoming packets after close. baseSender, err := i.baseProxy.NewSession(resp) if err != nil { return nil, err } + // TODO: implement receiver that will close the session when the response is received and there was one write. dnsResp := &natResponseReceiver{ PacketResponseReceiver: resp, localAddr: i.resolverLinkLocalAddr, From 1af2b8c7d385bfca2498e59710e2fdc7aedc2bd4 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 17:15:51 -0400 Subject: [PATCH 11/20] client/go/dnsintercept: close receiver after single response for single-write flows --- client/go/outline/dnsintercept/interceptor.go | 29 ++++++++++++++++-- .../outline/dnsintercept/interceptor_test.go | 30 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index 950cf9013c..39692a4075 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -20,6 +20,7 @@ import ( "net" "net/netip" "sync" + "sync/atomic" "golang.getoutline.org/sdk/network" "golang.getoutline.org/sdk/transport" @@ -76,13 +77,20 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ return nil, err } - // TODO: implement receiver that will close the session when the response is received and there was one write. + sentCount := new(int32) + dnsResp := &natResponseReceiver{ PacketResponseReceiver: resp, localAddr: i.resolverLinkLocalAddr, remoteAddr: i.resolverRemoteAddr, } - dnsSender, err := i.dnsProxy.NewSession(dnsResp) + + singleResp := &singleResponseReceiver{ + PacketResponseReceiver: dnsResp, + sentCount: sentCount, + } + + dnsSender, err := i.dnsProxy.NewSession(singleResp) if err != nil { baseSender.Close() return nil, err @@ -93,6 +101,7 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ dnsSender: dnsSender, resolverLinkLocalAddr: i.resolverLinkLocalAddr, resolverRemoteAddr: i.resolverRemoteAddr, + sentCount: sentCount, }, nil } @@ -103,6 +112,7 @@ type dnsInterceptorRequestSender struct { dnsSender network.PacketRequestSender resolverLinkLocalAddr netip.AddrPort resolverRemoteAddr netip.AddrPort + sentCount *int32 } func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { @@ -114,6 +124,7 @@ func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPo } if isEquivalentAddrPort(destination, s.resolverLinkLocalAddr) { + atomic.AddInt32(s.sentCount, 1) return s.dnsSender.WriteTo(p, s.resolverRemoteAddr) } return s.baseSender.WriteTo(p, destination) @@ -158,3 +169,17 @@ func (r *natResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) } return r.PacketResponseReceiver.WriteFrom(p, source) } + +// singleResponseReceiver closes the inner receiver when a response is received and there was only one write. +type singleResponseReceiver struct { + network.PacketResponseReceiver + sentCount *int32 +} + +func (r *singleResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { + n, err := r.PacketResponseReceiver.WriteFrom(p, source) + if atomic.LoadInt32(r.sentCount) == 1 { + r.PacketResponseReceiver.Close() + } + return n, err +} diff --git a/client/go/outline/dnsintercept/interceptor_test.go b/client/go/outline/dnsintercept/interceptor_test.go index aa8e3218f9..a329b3f245 100644 --- a/client/go/outline/dnsintercept/interceptor_test.go +++ b/client/go/outline/dnsintercept/interceptor_test.go @@ -89,6 +89,7 @@ func (s *lastDestPacketRequestSender) Close() error { type lastSourcePacketResponseReceiver struct { lastSrc net.Addr lastPacket []byte + closed bool } func (r *lastSourcePacketResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { @@ -99,6 +100,7 @@ func (r *lastSourcePacketResponseReceiver) WriteFrom(p []byte, source net.Addr) } func (r *lastSourcePacketResponseReceiver) Close() error { + r.closed = true return nil } @@ -142,6 +144,34 @@ func TestDNSInterceptor(t *testing.T) { require.True(t, dnsPP.req.closed) } +func TestDNSInterceptor_AutoClose(t *testing.T) { + basePP := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} + dnsPP := &packetProxyWithGivenRequestSender{req: &lastDestPacketRequestSender{}} + resp := &lastSourcePacketResponseReceiver{} + + resolverLinkLocalAddr := netip.MustParseAddrPort("192.0.2.1:53") + resolverRemoteAddr := netip.MustParseAddrPort("8.8.8.8:53") + + interceptor, err := NewDNSInterceptor(basePP, dnsPP, resolverLinkLocalAddr, resolverRemoteAddr) + require.NoError(t, err) + + req, err := interceptor.NewSession(resp) + require.NoError(t, err) + + // Send to local DNS address -> should be remapped to remote DNS + n, err := req.WriteTo([]byte("dns query"), resolverLinkLocalAddr) + require.NoError(t, err) + require.Equal(t, 9, n) + + // Receive from remote DNS -> should be remapped to local DNS and trigger close + require.NotNil(t, dnsPP.resp) + n, err = dnsPP.resp.WriteFrom([]byte("dns response"), net.UDPAddrFromAddrPort(resolverRemoteAddr)) + require.NoError(t, err) + require.Equal(t, 12, n) + + require.True(t, resp.closed, "receiver should be closed after response when there was only one write") +} + // ----- natResponseReceiver tests ----- func TestNatResponseReceiver(t *testing.T) { From 43412859870a21000f8f01d9f744504c70864e91 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 17:33:33 -0400 Subject: [PATCH 12/20] docs(dnsintercept): update README to fix PR comments --- client/go/outline/dnsintercept/README.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/client/go/outline/dnsintercept/README.md b/client/go/outline/dnsintercept/README.md index 7959e89990..7e67757491 100644 --- a/client/go/outline/dnsintercept/README.md +++ b/client/go/outline/dnsintercept/README.md @@ -71,13 +71,13 @@ The two modes are wired together by the caller (`configregistry.wrapTransportPai flowchart TD OS["OS (UDP traffic)"] --> ppMain ppMain["DNSInterceptor
(Address remapping and lazy dispatching)"] - ppMain -->|Non-DNS| ppBase["base PacketProxy
(transport)"] + ppMain -->|Non-DNS| ppBase["base PacketProxy
(5m timeout)"] ppMain -->|DNS| ppDNS["DelegatePacketProxy
(DNS traffic only)"] check["UDP connectivity check
(on network change)"] -->|pass| ppDNS check -->|fail| ppDNS - ppDNS -->|UDP available| ppBase + ppDNS -->|UDP available| ppDNSBase["DNS PacketProxy
(10s timeout)"] ppDNS -->|UDP blocked| ppTrunc["dnstruncate.PacketProxy
(TC response locally)"] ``` @@ -132,6 +132,5 @@ In truncate mode, no transport session is opened for DNS at all — the truncate | File | Description | |------|-------------| -| `interceptor.go` | `NewDNSInterceptor` — Dispatches DNS traffic, handles address translation, and creates sessions lazily | -| `interceptor.go` | `NewDNSRedirectStreamDialer` — Redirects TCP DNS to a real resolver | -| `helpers.go` | `isEquivalentAddrPort` — Address comparison ignoring IPv4-in-IPv6 mapping | +| `interceptor.go` | Contains `NewDNSInterceptor` for dispatching/remapping DNS packets, and `NewDNSRedirectStreamDialer` for TCP DNS. | +| `lazy_packet_proxy.go` | Implements `lazyPacketProxy` to defer session creation until the first write. | From 7da70267d814957531a7551e502db55faa3181a3 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 17:58:29 -0400 Subject: [PATCH 13/20] test(configregistry): add integration tests and benchmarks for DNS interceptor --- .../outline_dns_intercept_test.go | 470 ++++++++++++++++++ 1 file changed, 470 insertions(+) create mode 100644 client/go/outline/configregistry/outline_dns_intercept_test.go diff --git a/client/go/outline/configregistry/outline_dns_intercept_test.go b/client/go/outline/configregistry/outline_dns_intercept_test.go new file mode 100644 index 0000000000..12917940d1 --- /dev/null +++ b/client/go/outline/configregistry/outline_dns_intercept_test.go @@ -0,0 +1,470 @@ +// Copyright 2026 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package configregistry + +import ( + "context" + "io" + "net" + "net/netip" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/stretchr/testify/require" + "golang.getoutline.org/sdk/transport" +) + +// mockPacketConn implements net.PacketConn to intercept reads and writes in tests. +type mockPacketConn struct { + mu sync.Mutex + writtenPackets []writtenPacket + readChan chan readPacket + closed chan struct{} + autoRespondConnectivity bool +} + +type writtenPacket struct { + p []byte + addr net.Addr +} + +type readPacket struct { + p []byte + addr net.Addr +} + +func newMockPacketConn() *mockPacketConn { + return &mockPacketConn{ + readChan: make(chan readPacket, 10), + closed: make(chan struct{}), + } +} + +func (c *mockPacketConn) ReadFrom(p []byte) (int, net.Addr, error) { + select { + case rp := <-c.readChan: + copy(p, rp.p) + return len(rp.p), rp.addr, nil + case <-c.closed: + return 0, nil, io.EOF + } +} + +func (c *mockPacketConn) WriteTo(p []byte, addr net.Addr) (int, error) { + c.mu.Lock() + defer c.mu.Unlock() + c.writtenPackets = append(c.writtenPackets, writtenPacket{p: append([]byte(nil), p...), addr: addr}) + + // Auto-respond to connectivity checks to make CheckUDPConnectivity succeed. + // This simulates a healthy UDP network by responding to the hardcoded check address. + if c.autoRespondConnectivity && addr.String() == "1.1.1.1:53" { + c.readChan <- readPacket{p: []byte("dns response"), addr: addr} + } + + return len(p), nil +} + +func (c *mockPacketConn) Close() error { + c.mu.Lock() + defer c.mu.Unlock() + select { + case <-c.closed: + // already closed + default: + close(c.closed) + } + return nil +} + +func (c *mockPacketConn) LocalAddr() net.Addr { + return &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0} +} + +func (c *mockPacketConn) SetDeadline(t time.Time) error { return nil } +func (c *mockPacketConn) SetReadDeadline(t time.Time) error { return nil } +func (c *mockPacketConn) SetWriteDeadline(t time.Time) error { return nil } + +// mockPacketListener tracks created connections and returns mocks. +type mockPacketListener struct { + mu sync.Mutex + conns []*mockPacketConn + disableAutoRespond bool +} + +func (l *mockPacketListener) ListenPacket(ctx context.Context) (net.PacketConn, error) { + l.mu.Lock() + defer l.mu.Unlock() + conn := newMockPacketConn() + conn.autoRespondConnectivity = !l.disableAutoRespond + l.conns = append(l.conns, conn) + return conn, nil +} + +// lastSourcePacketResponseReceiver captures the last received packet and source address. +type lastSourcePacketResponseReceiver struct { + lastSrc net.Addr + lastPacket []byte + closed bool + mu sync.Mutex + done chan struct{} +} + +func (r *lastSourcePacketResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.lastSrc = source + r.lastPacket = make([]byte, len(p)) + copy(r.lastPacket, p) + select { + case r.done <- struct{}{}: + default: + } + return len(p), nil +} + +func (r *lastSourcePacketResponseReceiver) Close() error { + r.mu.Lock() + defer r.mu.Unlock() + r.closed = true + return nil +} + +// TestWrapTransportPairWithOutlineDNS verifies that DNS queries are intercepted and remapped. +func TestWrapTransportPairWithOutlineDNS(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + pl := &mockPacketListener{} + plWrapper := &PacketListener{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + PacketListener: pl, + } + sd := &Dialer[transport.StreamConn]{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + Dial: func(ctx context.Context, address string) (transport.StreamConn, error) { + return nil, nil // not used in this test + }, + } + + pair, err := wrapTransportPairWithOutlineDNS(sd, plWrapper) + require.NoError(t, err) + require.NotNil(t, pair) + require.NotNil(t, pair.PacketProxy) + + // Trigger network change to make connectivity check run + pair.PacketProxy.NotifyNetworkChanged() + + // Wait for connectivity check to complete. + synctest.Wait() + + pl.mu.Lock() + require.GreaterOrEqual(t, len(pl.conns), 1, "should create at least 1 connection for connectivity check") + connCheck := pl.conns[0] // Assuming it's the first one created + pl.mu.Unlock() + + foundCheck := false + connCheck.mu.Lock() + for _, wp := range connCheck.writtenPackets { + if wp.addr.String() == "1.1.1.1:53" { + foundCheck = true + break + } + } + connCheck.mu.Unlock() + require.True(t, foundCheck, "connectivity check packet not found") + + // Now we know the connectivity check succeeded (because of auto-respond in WriteTo). + // The proxy should now be set to ppDNSBase. + + proxy := pair.PacketProxy.PacketProxy + resp := &lastSourcePacketResponseReceiver{done: make(chan struct{}, 1)} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + require.NotNil(t, sender) + + // Send a DNS query to the link-local address (must be at least 12 bytes) + dnsQuery := make([]byte, 12) + copy(dnsQuery, []byte("dns query")) + n, err := sender.WriteTo(dnsQuery, linkLocalDNS) + require.NoError(t, err) + require.Equal(t, len(dnsQuery), n) + + // The packet should be written to a NEW connection created for ppDNSBase + pl.mu.Lock() + require.GreaterOrEqual(t, len(pl.conns), 2, "should create a new connection for DNS traffic") + connDNS := pl.conns[1] // Assuming it's the second one created + pl.mu.Unlock() + + connDNS.mu.Lock() + require.Equal(t, 1, len(connDNS.writtenPackets)) + wp := connDNS.writtenPackets[0] + connDNS.mu.Unlock() + + found := false + for _, addr := range outlineDNSResolvers { + if wp.addr.String() == addr.String() { + found = true + break + } + } + require.True(t, found, "destination address should be one of the public DNS resolvers") + + remoteAddr := wp.addr + dnsResponse := make([]byte, 12) + copy(dnsResponse, []byte("dns response")) + + // Push response to connDNS.readChan + connDNS.readChan <- readPacket{p: dnsResponse, addr: remoteAddr} + + // Wait for response + select { + case <-resp.done: + case <-time.After(1 * time.Second): + t.Fatal("timeout waiting for DNS response") + } + + require.Equal(t, net.UDPAddrFromAddrPort(linkLocalDNS), resp.lastSrc) + require.Equal(t, dnsResponse, resp.lastPacket) + + // Verify that the receiver was closed (auto-close feature) + require.True(t, resp.closed, "receiver should be closed after response") + + // Clean up all connections to stop read loops + pl.mu.Lock() + for _, c := range pl.conns { + c.Close() + } + pl.mu.Unlock() + }) +} + +// BenchmarkDNSInterceptor stress tests the system by simulating high volume of DNS queries. +func BenchmarkDNSInterceptor(b *testing.B) { + pl := &mockPacketListener{} + plWrapper := &PacketListener{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + PacketListener: pl, + } + sd := &Dialer[transport.StreamConn]{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + Dial: func(ctx context.Context, address string) (transport.StreamConn, error) { + return nil, nil + }, + } + + pair, err := wrapTransportPairWithOutlineDNS(sd, plWrapper) + if err != nil { + b.Fatal(err) + } + proxy := pair.PacketProxy.PacketProxy + + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + resp := &lastSourcePacketResponseReceiver{done: make(chan struct{}, 1)} + for pb.Next() { + sender, err := proxy.NewSession(resp) + if err != nil { + b.Fatal(err) + } + dnsQuery := make([]byte, 12) + _, err = sender.WriteTo(dnsQuery, linkLocalDNS) + if err != nil { + b.Fatal(err) + } + sender.Close() + } + }) +} + +// TestWrapTransportPairWithOutlineDNS_Timeout verifies that DNS sessions are closed promptly +// even when no response is received, preventing resource leaks. +func TestWrapTransportPairWithOutlineDNS_Timeout(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + pl := &mockPacketListener{} + plWrapper := &PacketListener{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + PacketListener: pl, + } + sd := &Dialer[transport.StreamConn]{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + Dial: func(ctx context.Context, address string) (transport.StreamConn, error) { + return nil, nil + }, + } + + pair, err := wrapTransportPairWithOutlineDNS(sd, plWrapper) + require.NoError(t, err) + + pair.PacketProxy.NotifyNetworkChanged() + synctest.Wait() + + proxy := pair.PacketProxy.PacketProxy + resp := &lastSourcePacketResponseReceiver{done: make(chan struct{}, 1)} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + + dnsQuery := make([]byte, 12) + _, err = sender.WriteTo(dnsQuery, linkLocalDNS) + require.NoError(t, err) + + // Wait for 15 seconds (longer than 10s timeout) to verify auto-close. + time.Sleep(15 * time.Second) + + t.Logf("Receiver closed after 15s: %v", resp.closed) + + pl.mu.Lock() + for _, c := range pl.conns { + c.Close() + } + pl.mu.Unlock() + }) +} + +// TestWrapTransportPairWithOutlineDNS_Truncation verifies that DNS queries are truncated +// when UDP connectivity fails. +func TestWrapTransportPairWithOutlineDNS_Truncation(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + pl := &mockPacketListener{disableAutoRespond: true} + plWrapper := &PacketListener{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + PacketListener: pl, + } + sd := &Dialer[transport.StreamConn]{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + Dial: func(ctx context.Context, address string) (transport.StreamConn, error) { + return nil, nil + }, + } + + pair, err := wrapTransportPairWithOutlineDNS(sd, plWrapper) + require.NoError(t, err) + + // Trigger network change to make connectivity check run + pair.PacketProxy.NotifyNetworkChanged() + + // Wait for connectivity check to fail (4 retries * 2s = 8s). + // With synctest, we can just sleep 10 seconds. + time.Sleep(10 * time.Second) + + // Now the proxy should be set to ppDNSTrunc. + + proxy := pair.PacketProxy.PacketProxy + resp := &lastSourcePacketResponseReceiver{done: make(chan struct{}, 1)} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + + dnsQuery := make([]byte, 12) + copy(dnsQuery, []byte("dns query")) + _, err = sender.WriteTo(dnsQuery, linkLocalDNS) + require.NoError(t, err) + + // Wait for response (it should be generated locally and delivered instantly) + select { + case <-resp.done: + case <-time.After(1 * time.Second): + t.Fatal("timeout waiting for truncated response") + } + + // Verify that the response was received + require.NotEmpty(t, resp.lastPacket) + + // Verify that NO packet was written to the transport for this query! + // All connections should only contain connectivity check packets to 1.1.1.1:53. + pl.mu.Lock() + for _, c := range pl.conns { + c.mu.Lock() + for _, wp := range c.writtenPackets { + if wp.addr.String() != "1.1.1.1:53" { + t.Fatalf("Unexpected packet written to %v", wp.addr) + } + } + c.mu.Unlock() + } + pl.mu.Unlock() + + // Clean up + pl.mu.Lock() + for _, c := range pl.conns { + c.Close() + } + pl.mu.Unlock() + }) +} + +// TestWrapTransportPairWithOutlineDNS_NonDNS verifies that non-DNS UDP traffic +// goes to the right place and uses a longer timeout (5m). +func TestWrapTransportPairWithOutlineDNS_NonDNS(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + pl := &mockPacketListener{} + plWrapper := &PacketListener{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + PacketListener: pl, + } + sd := &Dialer[transport.StreamConn]{ + ConnectionProviderInfo: ConnectionProviderInfo{ConnType: ConnTypeTunneled}, + Dial: func(ctx context.Context, address string) (transport.StreamConn, error) { + return nil, nil + }, + } + + pair, err := wrapTransportPairWithOutlineDNS(sd, plWrapper) + require.NoError(t, err) + + proxy := pair.PacketProxy.PacketProxy + resp := &lastSourcePacketResponseReceiver{done: make(chan struct{}, 1)} + sender, err := proxy.NewSession(resp) + require.NoError(t, err) + + // Send a non-DNS query to some other address + otherAddr := netip.MustParseAddrPort("1.2.3.4:443") + packet := []byte("not dns") + _, err = sender.WriteTo(packet, otherAddr) + require.NoError(t, err) + + // The packet should be written to a connection created for ppBase. + pl.mu.Lock() + require.GreaterOrEqual(t, len(pl.conns), 1, "should create a connection for non-DNS traffic") + connBase := pl.conns[len(pl.conns)-1] // The last one created + pl.mu.Unlock() + + connBase.mu.Lock() + require.Equal(t, 1, len(connBase.writtenPackets)) + wp := connBase.writtenPackets[0] + connBase.mu.Unlock() + + // Verify destination was NOT remapped + require.Equal(t, otherAddr.String(), wp.addr.String()) + require.Equal(t, packet, wp.p) + + // Wait for 15 seconds (longer than 10s DNS timeout) + time.Sleep(15 * time.Second) + + // Receiver should still be OPEN! (5m timeout not reached) + require.False(t, resp.closed, "receiver should not be closed after 15s for non-DNS traffic") + + // Wait for 5 minutes (300 seconds) + time.Sleep(300 * time.Second) + + // Receiver should now be CLOSED! (5m timeout reached) + require.True(t, resp.closed, "receiver should be closed after 5m for non-DNS traffic") + + // Clean up + pl.mu.Lock() + for _, c := range pl.conns { + c.Close() + } + pl.mu.Unlock() + }) +} From e0655fd0e95740f9ec5b12a68501a5f90178cb2e Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:02:47 -0400 Subject: [PATCH 14/20] test(configregistry): fix data race in DNS interceptor tests --- .../configregistry/outline_dns_intercept_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/client/go/outline/configregistry/outline_dns_intercept_test.go b/client/go/outline/configregistry/outline_dns_intercept_test.go index 12917940d1..85a8c64f73 100644 --- a/client/go/outline/configregistry/outline_dns_intercept_test.go +++ b/client/go/outline/configregistry/outline_dns_intercept_test.go @@ -143,6 +143,12 @@ func (r *lastSourcePacketResponseReceiver) Close() error { return nil } +func (r *lastSourcePacketResponseReceiver) IsClosed() bool { + r.mu.Lock() + defer r.mu.Unlock() + return r.closed +} + // TestWrapTransportPairWithOutlineDNS verifies that DNS queries are intercepted and remapped. func TestWrapTransportPairWithOutlineDNS(t *testing.T) { synctest.Test(t, func(t *testing.T) { @@ -239,7 +245,7 @@ func TestWrapTransportPairWithOutlineDNS(t *testing.T) { require.Equal(t, dnsResponse, resp.lastPacket) // Verify that the receiver was closed (auto-close feature) - require.True(t, resp.closed, "receiver should be closed after response") + require.True(t, resp.IsClosed(), "receiver should be closed after response") // Clean up all connections to stop read loops pl.mu.Lock() @@ -322,7 +328,7 @@ func TestWrapTransportPairWithOutlineDNS_Timeout(t *testing.T) { // Wait for 15 seconds (longer than 10s timeout) to verify auto-close. time.Sleep(15 * time.Second) - t.Logf("Receiver closed after 15s: %v", resp.closed) + t.Logf("Receiver closed after 15s: %v", resp.IsClosed()) pl.mu.Lock() for _, c := range pl.conns { @@ -452,13 +458,13 @@ func TestWrapTransportPairWithOutlineDNS_NonDNS(t *testing.T) { time.Sleep(15 * time.Second) // Receiver should still be OPEN! (5m timeout not reached) - require.False(t, resp.closed, "receiver should not be closed after 15s for non-DNS traffic") + require.False(t, resp.IsClosed(), "receiver should not be closed after 15s for non-DNS traffic") // Wait for 5 minutes (300 seconds) time.Sleep(300 * time.Second) // Receiver should now be CLOSED! (5m timeout reached) - require.True(t, resp.closed, "receiver should be closed after 5m for non-DNS traffic") + require.True(t, resp.IsClosed(), "receiver should be closed after 5m for non-DNS traffic") // Clean up pl.mu.Lock() From d0cd4bcf7fc3e8458c991f841541404319c00c46 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:08:19 -0400 Subject: [PATCH 15/20] docs(dnsintercept): add comments to NewSession and request sender --- client/go/outline/dnsintercept/interceptor.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index 39692a4075..4fb2a51e46 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -71,7 +71,11 @@ func NewDNSInterceptor(base network.PacketProxy, dns network.PacketProxy, resolv }, nil } +// NewSession implements PacketProxy.NewSession. +// It creates sessions on both the base proxy and the DNS proxy, and returns a sender +// that dispatches packets to the appropriate session based on destination. func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (network.PacketRequestSender, error) { + // Create session for base (non-DNS) traffic. baseSender, err := i.baseProxy.NewSession(resp) if err != nil { return nil, err @@ -79,17 +83,20 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ sentCount := new(int32) + // Wrap the response receiver for DNS traffic to remap source addresses. dnsResp := &natResponseReceiver{ PacketResponseReceiver: resp, localAddr: i.resolverLinkLocalAddr, remoteAddr: i.resolverRemoteAddr, } + // Further wrap it to auto-close the session after the first response (if single-write). singleResp := &singleResponseReceiver{ PacketResponseReceiver: dnsResp, sentCount: sentCount, } + // Create session for DNS traffic. dnsSender, err := i.dnsProxy.NewSession(singleResp) if err != nil { baseSender.Close() @@ -105,6 +112,7 @@ func (i *dnsInterceptor) NewSession(resp network.PacketResponseReceiver) (networ }, nil } +// dnsInterceptorRequestSender handles dispatching of outgoing packets. type dnsInterceptorRequestSender struct { closeMu sync.Mutex isClosed bool @@ -112,9 +120,13 @@ type dnsInterceptorRequestSender struct { dnsSender network.PacketRequestSender resolverLinkLocalAddr netip.AddrPort resolverRemoteAddr netip.AddrPort - sentCount *int32 + sentCount *int32 // tracked to determine if we can auto-close on response } +// WriteTo intercepts outgoing packets. +// If the destination is the link-local DNS address, it routes the packet to the DNS session +// and remaps the destination to the remote resolver address. +// All other packets are routed to the base session without modification. func (s *dnsInterceptorRequestSender) WriteTo(p []byte, destination netip.AddrPort) (int, error) { s.closeMu.Lock() defer s.closeMu.Unlock() From fa3cc17b83189a7b54897b7eafd14200bab49ee4 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:12:53 -0400 Subject: [PATCH 16/20] test(configregistry): implement deadlines in mock to fix truncation test --- .../outline_dns_intercept_test.go | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/client/go/outline/configregistry/outline_dns_intercept_test.go b/client/go/outline/configregistry/outline_dns_intercept_test.go index 85a8c64f73..aab34b9890 100644 --- a/client/go/outline/configregistry/outline_dns_intercept_test.go +++ b/client/go/outline/configregistry/outline_dns_intercept_test.go @@ -16,6 +16,7 @@ package configregistry import ( "context" + "errors" "io" "net" "net/netip" @@ -35,6 +36,7 @@ type mockPacketConn struct { readChan chan readPacket closed chan struct{} autoRespondConnectivity bool + deadline time.Time } type writtenPacket struct { @@ -55,12 +57,27 @@ func newMockPacketConn() *mockPacketConn { } func (c *mockPacketConn) ReadFrom(p []byte) (int, net.Addr, error) { + c.mu.Lock() + deadline := c.deadline + c.mu.Unlock() + + var timeoutChan <-chan time.Time + if !deadline.IsZero() { + duration := time.Until(deadline) + if duration <= 0 { + return 0, nil, errors.New("i/o timeout") + } + timeoutChan = time.After(duration) + } + select { case rp := <-c.readChan: copy(p, rp.p) return len(rp.p), rp.addr, nil case <-c.closed: return 0, nil, io.EOF + case <-timeoutChan: + return 0, nil, errors.New("i/o timeout") } } @@ -94,9 +111,15 @@ func (c *mockPacketConn) LocalAddr() net.Addr { return &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0} } -func (c *mockPacketConn) SetDeadline(t time.Time) error { return nil } -func (c *mockPacketConn) SetReadDeadline(t time.Time) error { return nil } -func (c *mockPacketConn) SetWriteDeadline(t time.Time) error { return nil } +func (c *mockPacketConn) SetDeadline(t time.Time) error { + c.mu.Lock() + defer c.mu.Unlock() + c.deadline = t + return nil +} + +func (c *mockPacketConn) SetReadDeadline(t time.Time) error { return c.SetDeadline(t) } +func (c *mockPacketConn) SetWriteDeadline(t time.Time) error { return c.SetDeadline(t) } // mockPacketListener tracks created connections and returns mocks. type mockPacketListener struct { From e17452b92370199aa9057747be166a0e2295ef42 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:16:30 -0400 Subject: [PATCH 17/20] test(configregistry): add assertion to timeout test --- client/go/outline/configregistry/outline_dns_intercept_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/go/outline/configregistry/outline_dns_intercept_test.go b/client/go/outline/configregistry/outline_dns_intercept_test.go index aab34b9890..99ea265960 100644 --- a/client/go/outline/configregistry/outline_dns_intercept_test.go +++ b/client/go/outline/configregistry/outline_dns_intercept_test.go @@ -351,7 +351,7 @@ func TestWrapTransportPairWithOutlineDNS_Timeout(t *testing.T) { // Wait for 15 seconds (longer than 10s timeout) to verify auto-close. time.Sleep(15 * time.Second) - t.Logf("Receiver closed after 15s: %v", resp.IsClosed()) + require.True(t, resp.IsClosed(), "receiver should be closed after timeout") pl.mu.Lock() for _, c := range pl.conns { From a98384de7e294c427ef7bba24d8f8aeb2f3e93c6 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:17:07 -0400 Subject: [PATCH 18/20] fix(configregistry): make error messages distinct in outline_dns_intercept --- client/go/outline/configregistry/outline_dns_intercept.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/go/outline/configregistry/outline_dns_intercept.go b/client/go/outline/configregistry/outline_dns_intercept.go index be5412eca2..e763e85d1c 100644 --- a/client/go/outline/configregistry/outline_dns_intercept.go +++ b/client/go/outline/configregistry/outline_dns_intercept.go @@ -64,12 +64,12 @@ func wrapTransportPairWithOutlineDNS(sd *Dialer[transport.StreamConn], pl *Packe // Uses the 5m timeout as recommended in https://www.rfc-editor.org/rfc/rfc4787.html#section-4.3 ppBase, err := network.NewPacketProxyFromPacketListener(pl, network.WithPacketListenerWriteIdleTimeout(5 * time.Minute)) if err != nil { - return nil, fmt.Errorf("failed to create PacketProxy: %w", err) + return nil, fmt.Errorf("failed to create base PacketProxy: %w", err) } // PacketProxy for DNS. Uses a shorter timeout, as recommended in https://www.rfc-editor.org/rfc/rfc5452.html#section-10. ppDNSBase, err := network.NewPacketProxyFromPacketListener(pl, network.WithPacketListenerWriteIdleTimeout(10 * time.Second)) if err != nil { - return nil, fmt.Errorf("failed to create PacketProxy: %w", err) + return nil, fmt.Errorf("failed to create DNS PacketProxy: %w", err) } // Returns a truncated response for DNS packets to force a retry over TCP. ppDNSTrunc, err := dnstruncate.NewPacketProxy() From 5b7923f5d56d729e29d844d882a8e6697087d34f Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:26:26 -0400 Subject: [PATCH 19/20] test(configregistry): fix remaining data races in DNS interceptor tests --- .../outline_dns_intercept_test.go | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/client/go/outline/configregistry/outline_dns_intercept_test.go b/client/go/outline/configregistry/outline_dns_intercept_test.go index 99ea265960..f8b9ec8a7e 100644 --- a/client/go/outline/configregistry/outline_dns_intercept_test.go +++ b/client/go/outline/configregistry/outline_dns_intercept_test.go @@ -172,6 +172,23 @@ func (r *lastSourcePacketResponseReceiver) IsClosed() bool { return r.closed } +func (r *lastSourcePacketResponseReceiver) GetLastPacket() []byte { + r.mu.Lock() + defer r.mu.Unlock() + if r.lastPacket == nil { + return nil + } + p := make([]byte, len(r.lastPacket)) + copy(p, r.lastPacket) + return p +} + +func (r *lastSourcePacketResponseReceiver) GetLastSrc() net.Addr { + r.mu.Lock() + defer r.mu.Unlock() + return r.lastSrc +} + // TestWrapTransportPairWithOutlineDNS verifies that DNS queries are intercepted and remapped. func TestWrapTransportPairWithOutlineDNS(t *testing.T) { synctest.Test(t, func(t *testing.T) { @@ -264,8 +281,8 @@ func TestWrapTransportPairWithOutlineDNS(t *testing.T) { t.Fatal("timeout waiting for DNS response") } - require.Equal(t, net.UDPAddrFromAddrPort(linkLocalDNS), resp.lastSrc) - require.Equal(t, dnsResponse, resp.lastPacket) + require.Equal(t, net.UDPAddrFromAddrPort(linkLocalDNS), resp.GetLastSrc()) + require.Equal(t, dnsResponse, resp.GetLastPacket()) // Verify that the receiver was closed (auto-close feature) require.True(t, resp.IsClosed(), "receiver should be closed after response") @@ -407,7 +424,7 @@ func TestWrapTransportPairWithOutlineDNS_Truncation(t *testing.T) { } // Verify that the response was received - require.NotEmpty(t, resp.lastPacket) + require.NotEmpty(t, resp.GetLastPacket()) // Verify that NO packet was written to the transport for this query! // All connections should only contain connectivity check packets to 1.1.1.1:53. From c92d671efa9eb37118c8d1c789876383e4d43f35 Mon Sep 17 00:00:00 2001 From: Vinicius Fortuna Date: Fri, 1 May 2026 18:29:14 -0400 Subject: [PATCH 20/20] fix(dnsintercept): add sync.Once to prevent double-close in singleResponseReceiver --- client/go/outline/dnsintercept/interceptor.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/go/outline/dnsintercept/interceptor.go b/client/go/outline/dnsintercept/interceptor.go index 4fb2a51e46..6f7234b7cd 100644 --- a/client/go/outline/dnsintercept/interceptor.go +++ b/client/go/outline/dnsintercept/interceptor.go @@ -186,12 +186,15 @@ func (r *natResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) type singleResponseReceiver struct { network.PacketResponseReceiver sentCount *int32 + closeOnce sync.Once } func (r *singleResponseReceiver) WriteFrom(p []byte, source net.Addr) (int, error) { n, err := r.PacketResponseReceiver.WriteFrom(p, source) if atomic.LoadInt32(r.sentCount) == 1 { - r.PacketResponseReceiver.Close() + r.closeOnce.Do(func() { + r.PacketResponseReceiver.Close() + }) } return n, err }