From bdee0a2068944bbfb543ceed566688cdf3a35c75 Mon Sep 17 00:00:00 2001 From: Kuzz007 Date: Mon, 27 Jul 2026 15:26:10 +0300 Subject: [PATCH] fix(xray): force a full restart for TPROXY inbounds, never hot-add them Real incident: an AmneziaWG inbound with RouteThroughXray enabled lost all internet on that connection after a migration. Root-caused on the live box -- iptables TPROXY counters were incrementing (packets correctly redirected to 127.0.0.1:63110), but nothing was actually listening there (ss showed nothing on that port) until a full `systemctl restart x-ui`, after which the bridge came up immediately. Xray-core's gRPC AddInbound reports success for a new sockopt.tproxy inbound (internal/amneziawg's own Xray egress bridge is the only kind this fork ever generates) but doesn't reliably bind a working listener for it outside of process startup -- the bridge silently never comes up, and RouteThroughXray traffic goes nowhere until the next full restart happens to occur for an unrelated reason. diffInbounds already has this exact defensive pattern for REALITY inbounds ("a gRPC remove+add does not reliably rebuild the REALITY authenticator"), just never extended to TPROXY, and only in the already-existing-then-changed branch -- the "brand new inbound" branch had no such guard at all, which is exactly the path a freshly-enabled RouteThroughXray bridge takes. Added inboundUsesTproxy and wired it into both branches. Co-Authored-By: Claude Sonnet 5 --- internal/xray/hot_diff.go | 32 +++++++++++++++++++++ internal/xray/hot_diff_test.go | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/internal/xray/hot_diff.go b/internal/xray/hot_diff.go index d43898aee..73dbc20d3 100644 --- a/internal/xray/hot_diff.go +++ b/internal/xray/hot_diff.go @@ -132,6 +132,10 @@ func diffInbounds(oldCfg, newCfg *Config, diff *HotDiff) bool { logger.Debug("hot diff: inbound [", oldIb.Tag, "] REALITY configuration changed; a gRPC remove+add does not reliably rebuild the REALITY authenticator, forcing a full restart") return false } + if exists && (inboundUsesTproxy(oldIb) || inboundUsesTproxy(newIb)) { + logger.Debug("hot diff: inbound [", oldIb.Tag, "] is a TPROXY target; a gRPC add reports success but does not reliably bind a working listener, forcing a full restart instead of a hot swap") + return false + } diff.RemovedInboundTags = append(diff.RemovedInboundTags, oldIb.Tag) if exists { raw, err := json.Marshal(newIb) @@ -149,6 +153,10 @@ func diffInbounds(oldCfg, newCfg *Config, diff *HotDiff) bool { if newIb.Tag == apiTag || newIb.Tag == "api" { return false } + if inboundUsesTproxy(newIb) { + logger.Debug("hot diff: new inbound [", newIb.Tag, "] is a TPROXY target (e.g. internal/amneziawg's Xray egress bridge); a gRPC add reports success but does not reliably bind a working listener, forcing a full restart instead of a hot add") + return false + } raw, err := json.Marshal(newIb) if err != nil { return false @@ -264,6 +272,30 @@ func inboundUsesReality(ib *InboundConfig) bool { return stream.Security == "reality" } +// inboundUsesTproxy reports whether an inbound's streamSettings mark it as a +// TPROXY target (internal/amneziawg's own Xray egress bridge -- see +// service.amneziawgEgressStreamSettings -- is the only inbound kind this +// fork ever generates with sockopt.tproxy set). Confirmed directly against a +// real Xray-core instance: adding this kind of inbound through the gRPC +// HandlerService reports success, but no listener actually ends up bound on +// the configured port, so TPROXY-redirected traffic silently goes nowhere +// until the next full restart. Forcing a restart here is the same +// defensive pattern already used for REALITY inbounds above. +func inboundUsesTproxy(ib *InboundConfig) bool { + if ib == nil || len(ib.StreamSettings) == 0 { + return false + } + var stream struct { + Sockopt struct { + Tproxy string `json:"tproxy"` + } `json:"sockopt"` + } + if err := json.Unmarshal(ib.StreamSettings, &stream); err != nil { + return false + } + return stream.Sockopt.Tproxy != "" && stream.Sockopt.Tproxy != "off" +} + func inboundHasReverseClient(ib *InboundConfig) bool { if ib == nil { return false diff --git a/internal/xray/hot_diff_test.go b/internal/xray/hot_diff_test.go index 2d464492b..03644311b 100644 --- a/internal/xray/hot_diff_test.go +++ b/internal/xray/hot_diff_test.go @@ -385,3 +385,54 @@ func TestComputeHotDiff_RealityClientOnlyChangeStaysHot(t *testing.T) { t.Fatalf("expected user b added via AlterInbound, got %+v", diff.AddedUsers) } } + +// TestComputeHotDiff_NewTproxyInboundNeedsRestart reproduces a real incident: +// enabling RouteThroughXray on an AmneziaWG inbound while Xray is already +// running adds a brand-new dokodemo-door bridge with sockopt.tproxy set. +// Xray-core's gRPC AddInbound reports success for this but never actually +// binds a working listener, so TPROXY-redirected peer traffic silently goes +// nowhere until the next full restart -- confirmed directly on a real box +// (iptables TPROXY counters incrementing, but `ss` showing nothing listening +// on the bridge port; the listener only appeared after `systemctl restart +// x-ui`). This must force a restart instead of a hot add. +func TestComputeHotDiff_NewTproxyInboundNeedsRestart(t *testing.T) { + oldCfg := makeHotConfig() + newCfg := makeHotConfig() + newCfg.InboundConfigs = append(newCfg.InboundConfigs, InboundConfig{ + Listen: json_util.RawMessage(`"127.0.0.1"`), + Port: 63110, + Protocol: "dokodemo-door", + Tag: "in-443-udp", + Settings: json_util.RawMessage(`{"allowedNetwork":"tcp,udp","followRedirect":true}`), + StreamSettings: json_util.RawMessage(`{"sockopt":{"tproxy":"tproxy"}}`), + }) + + if _, ok := ComputeHotDiff(oldCfg, newCfg); ok { + t.Fatal("adding a new TPROXY-sockopt inbound must force a full restart, not a gRPC hot add") + } +} + +// TestComputeHotDiff_TproxyStreamChangeNeedsRestart mirrors the REALITY +// stream-change test above: an existing TPROXY bridge whose port changed +// (e.g. the AmneziaWG inbound's own id-derived egress port shifted) must not +// be hot-swapped either, for the same reliability reason. +func TestComputeHotDiff_TproxyStreamChangeNeedsRestart(t *testing.T) { + tproxyIb := InboundConfig{ + Listen: json_util.RawMessage(`"127.0.0.1"`), + Port: 63110, + Protocol: "dokodemo-door", + Tag: "in-443-udp", + Settings: json_util.RawMessage(`{"allowedNetwork":"tcp,udp","followRedirect":true}`), + StreamSettings: json_util.RawMessage(`{"sockopt":{"tproxy":"tproxy"}}`), + } + oldCfg := makeHotConfig() + oldCfg.InboundConfigs = append(oldCfg.InboundConfigs, tproxyIb) + newCfg := makeHotConfig() + changedIb := tproxyIb + changedIb.Port = 63111 + newCfg.InboundConfigs = append(newCfg.InboundConfigs, changedIb) + + if _, ok := ComputeHotDiff(oldCfg, newCfg); ok { + t.Fatal("a TPROXY bridge's port change must force a full restart, not a gRPC hot swap") + } +}