fix(xray): force a full restart for password-auth SOCKS5 hot-apply

Real production incident: editing a client under an AmneziaWG inbound
left its embedded SOCKS5 relay's settings byte-different (a new account
list), and Xray's gRPC remove+add hot swap silently dropped the account
for a peer whose email contained non-ASCII characters -- its tunnel kept
handshaking fine but all its traffic got rejected at the SOCKS5 layer,
while every other peer on the same relay was unaffected. A full restart
(reading the same JSON straight from disk) always produced the correct
account list. socks isn't in userDiffableProtocols (that only covers
vless/vmess/trojan's clients+email shape, not accounts+user), so any
settings drift on this inbound fell through to the generic remove+add
path. Forces a restart instead, the same defensive choice already made
for REALITY and TPROXY -- scoped to auth:"password" specifically so the
other, noauth SOCKS5 bridges (panel/node/mtproto egress) keep the cheaper
hot path.
This commit is contained in:
Kuzz007
2026-08-03 11:55:19 +03:00
parent 3a8738c958
commit d36c1217c8
2 changed files with 118 additions and 0 deletions
+43
View File
@@ -136,6 +136,10 @@ func diffInbounds(oldCfg, newCfg *Config, diff *HotDiff) bool {
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
}
if exists && (inboundUsesSocksAccounts(oldIb) || inboundUsesSocksAccounts(newIb)) {
logger.Debug("hot diff: inbound [", oldIb.Tag, "] is a password-auth SOCKS5 inbound (e.g. internal/amneziawgnet's per-peer relay); a gRPC remove+add reports success but was observed in production to silently drop an account, 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)
@@ -157,6 +161,10 @@ func diffInbounds(oldCfg, newCfg *Config, diff *HotDiff) bool {
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
}
if inboundUsesSocksAccounts(newIb) {
logger.Debug("hot diff: new inbound [", newIb.Tag, "] is a password-auth SOCKS5 inbound (e.g. internal/amneziawgnet's per-peer relay); forcing a full restart instead of a hot add, same reasoning as the existing-inbound case above")
return false
}
raw, err := json.Marshal(newIb)
if err != nil {
return false
@@ -296,6 +304,41 @@ func inboundUsesTproxy(ib *InboundConfig) bool {
return stream.Sockopt.Tproxy != "" && stream.Sockopt.Tproxy != "off"
}
// inboundUsesSocksAccounts reports whether an inbound is a password-auth
// SOCKS5 inbound with one or more named accounts -- the shape
// internal/amneziawgnet's per-inbound relay (internal/web/service/xray.go's
// injectAmneziawgnetSocks) is the only generator of in this fork; every
// other SOCKS5 bridge this fork builds (panel egress, per-node egress,
// mtproto egress) uses "noauth" with no per-account identity at all, so this
// check can't accidentally rope in one of those lower-churn bridges.
//
// Real production incident, not a theoretical concern: a single client
// edit under an AmneziaWG inbound left this inbound's settings unchanged in
// every way relevant to accounts.user was already correct in the freshly
// regenerated config, yet the account for a peer whose email contained
// non-ASCII characters silently vanished from the running Xray process
// after a gRPC remove+add hot swap -- while a full process restart (reading
// the same JSON straight from disk) always produced the correct account
// list. socks isn't in userDiffableProtocols (that only covers vless/vmess/
// trojan, which use a wholly different clients+email shape, not
// accounts+user), so without this check any settings drift on this inbound
// -- even one unrelated to the account list itself -- falls through to the
// generic remove+add path and can reproduce the same silent drop. Forcing a
// full restart here is the same defensive choice already made above for
// REALITY and TPROXY.
func inboundUsesSocksAccounts(ib *InboundConfig) bool {
if ib == nil || ib.Protocol != "socks" || len(ib.Settings) == 0 {
return false
}
var settings struct {
Auth string `json:"auth"`
}
if err := json.Unmarshal(ib.Settings, &settings); err != nil {
return false
}
return settings.Auth == "password"
}
func inboundHasReverseClient(ib *InboundConfig) bool {
if ib == nil {
return false
+75
View File
@@ -436,3 +436,78 @@ func TestComputeHotDiff_TproxyStreamChangeNeedsRestart(t *testing.T) {
t.Fatal("a TPROXY bridge's port change must force a full restart, not a gRPC hot swap")
}
}
// TestComputeHotDiff_SocksAccountsSettingsChangeNeedsRestart reproduces a
// real incident: editing one client under an AmneziaWG inbound left its
// relay's settings.json byte-different (a new account list) while Xray was
// already running. A gRPC remove+add reported success but silently dropped
// an account with a non-ASCII email; a full restart always produced the
// correct account list. This must force a restart, not a hot swap.
func TestComputeHotDiff_SocksAccountsSettingsChangeNeedsRestart(t *testing.T) {
relayIb := InboundConfig{
Listen: json_util.RawMessage(`"127.0.0.1"`),
Port: 65110,
Protocol: "socks",
Tag: "in-443-udp",
Settings: json_util.RawMessage(`{"auth":"password","udp":true,"accounts":[{"user":"Роутер_awg","pass":"p"}]}`),
}
oldCfg := makeHotConfig()
oldCfg.InboundConfigs = append(oldCfg.InboundConfigs, relayIb)
newCfg := makeHotConfig()
changedIb := relayIb
changedIb.Settings = json_util.RawMessage(`{"auth":"password","udp":true,"accounts":[{"user":"Роутер_awg","pass":"p"},{"user":"Майфун🛟","pass":"p"}]}`)
newCfg.InboundConfigs = append(newCfg.InboundConfigs, changedIb)
if _, ok := ComputeHotDiff(oldCfg, newCfg); ok {
t.Fatal("a password-auth SOCKS5 relay's account-list change must force a full restart, not a gRPC hot swap")
}
}
// TestComputeHotDiff_NewSocksAccountsInboundNeedsRestart mirrors the TPROXY
// new-inbound test above: a brand-new AmneziaWG relay inbound must also
// force a restart, for the same reliability reason.
func TestComputeHotDiff_NewSocksAccountsInboundNeedsRestart(t *testing.T) {
oldCfg := makeHotConfig()
newCfg := makeHotConfig()
newCfg.InboundConfigs = append(newCfg.InboundConfigs, InboundConfig{
Listen: json_util.RawMessage(`"127.0.0.1"`),
Port: 65110,
Protocol: "socks",
Tag: "in-443-udp",
Settings: json_util.RawMessage(`{"auth":"password","udp":true,"accounts":[{"user":"Майфун🛟","pass":"p"}]}`),
})
if _, ok := ComputeHotDiff(oldCfg, newCfg); ok {
t.Fatal("adding a new password-auth SOCKS5 relay inbound must force a full restart, not a gRPC hot add")
}
}
// TestComputeHotDiff_NoauthSocksBridgeStaysHot confirms the check above is
// scoped to password-auth accounts specifically: this fork's other SOCKS5
// bridges (panel egress, per-node egress, mtproto egress) use "noauth" with
// no per-account identity, have no history of this failure mode, and must
// keep using the ordinary remove+add hot path rather than pay for an
// unnecessary restart on every port/tag change.
func TestComputeHotDiff_NoauthSocksBridgeStaysHot(t *testing.T) {
bridgeIb := InboundConfig{
Listen: json_util.RawMessage(`"127.0.0.1"`),
Port: 62790,
Protocol: "socks",
Tag: "panel-egress",
Settings: json_util.RawMessage(`{"auth":"noauth","udp":false}`),
}
oldCfg := makeHotConfig()
oldCfg.InboundConfigs = append(oldCfg.InboundConfigs, bridgeIb)
newCfg := makeHotConfig()
changedIb := bridgeIb
changedIb.Port = 62791
newCfg.InboundConfigs = append(newCfg.InboundConfigs, changedIb)
diff, ok := ComputeHotDiff(oldCfg, newCfg)
if !ok {
t.Fatal("a noauth SOCKS5 bridge's port change should stay hot-appliable")
}
if len(diff.RemovedInboundTags) != 1 || len(diff.AddedInbounds) != 1 {
t.Fatalf("expected a plain remove+add for the changed bridge, got %+v", diff)
}
}