mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-05 09:57:14 +00:00
Attach: never inherit an address that doesn't fit the target inbound
hasTunnelAttachment (from the earlier fix, commit 51067f16) only
asked "does this identity have ANY tunnel attachment", treating that
as license to reuse its stored address verbatim on every inbound
being attached. Real production case this missed: an identity's
stored address came from WireGuard's own fallback subnet
(10.0.0.0/24, used when that inbound has no other clients to infer a
base from), then got attached to a second, AmneziaWG inbound
configured for a completely different subnet (10.8.1.0/24).
defaultAmneziaWGClients's already-set-AllowedIPs branch only checks
for collisions, never subnet membership, so the mismatched address
was accepted silently -- producing a peer that can never actually
connect, since an AmneziaWG address must fall inside the kernel
interface's own configured subnet to be routable at all.
Add addressesFitAmneziaWGInbound, checked per inbound inside Attach's
loop: if the inherited address doesn't fit the SPECIFIC inbound being
attached, clear it just for that one so it gets a fresh, valid
allocation instead, while other already-attached inbounds keep their
existing values. WireGuard has no equivalent strict subnet
requirement (allocateWireguardAddress can widen to a fallback pool
for it), so this only ever constrains AmneziaWG targets.
This commit is contained in:
@@ -54,3 +54,44 @@ func TestHasTunnelAttachmentDetectsWireguardOrAmneziaWG(t *testing.T) {
|
||||
t.Error("an AmneziaWG inbound must count as a tunnel attachment")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAddressesFitAmneziaWGInbound is a regression test for a real
|
||||
// production bug: hasTunnelAttachment only asked "does this identity have
|
||||
// ANY tunnel attachment", not "is the address it would inherit actually
|
||||
// valid for THIS inbound" -- so an identity whose stored address came from
|
||||
// WireGuard's own fallback subnet (10.0.0.0/24, used when that inbound has
|
||||
// no other clients to infer a base from) got that exact address silently
|
||||
// carried over onto a second, AmneziaWG inbound configured for a completely
|
||||
// different subnet (10.8.1.0/24). defaultAmneziaWGClients's already-set
|
||||
// branch only checks for collisions, not subnet membership, so the mismatch
|
||||
// was accepted with no error -- producing a peer that can never actually
|
||||
// connect (an AmneziaWG address must fall inside the kernel interface's own
|
||||
// configured subnet to be routable at all). addressesFitAmneziaWGInbound is
|
||||
// the check Attach now runs per inbound before deciding whether to keep an
|
||||
// inherited address or force a fresh allocation.
|
||||
func TestAddressesFitAmneziaWGInbound(t *testing.T) {
|
||||
setupConflictDB(t)
|
||||
seedInboundConflict(t, "awg-1", "0.0.0.0", 443, model.AmneziaWG, ``, `{"server":{"subnetIp":"10.8.1.0","subnetCidr":24},"clients":[]}`)
|
||||
seedInboundConflict(t, "wg-1", "0.0.0.0", 51820, model.WireGuard, ``, `{"clients":[]}`)
|
||||
|
||||
var awgInbound, wgInbound model.Inbound
|
||||
if err := database.GetDB().Where("tag = ?", "awg-1").First(&awgInbound).Error; err != nil {
|
||||
t.Fatalf("read seeded awg row: %v", err)
|
||||
}
|
||||
if err := database.GetDB().Where("tag = ?", "wg-1").First(&wgInbound).Error; err != nil {
|
||||
t.Fatalf("read seeded wg row: %v", err)
|
||||
}
|
||||
|
||||
if !addressesFitAmneziaWGInbound(nil, &awgInbound) {
|
||||
t.Error("no addresses at all must trivially fit (Attach's own fresh-allocate path)")
|
||||
}
|
||||
if !addressesFitAmneziaWGInbound([]string{"10.0.0.2/32"}, &wgInbound) {
|
||||
t.Error("WireGuard has no strict subnet requirement -- must never be rejected here")
|
||||
}
|
||||
if addressesFitAmneziaWGInbound([]string{"10.0.0.2/32"}, &awgInbound) {
|
||||
t.Fatal("the real bug: a WireGuard-fallback-subnet address must NOT be accepted as fitting an AmneziaWG inbound configured for a different subnet")
|
||||
}
|
||||
if !addressesFitAmneziaWGInbound([]string{"10.8.1.21/32"}, &awgInbound) {
|
||||
t.Error("an address genuinely inside the awg inbound's own configured subnet must fit")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/netip"
|
||||
"strings"
|
||||
"time"
|
||||
"unicode"
|
||||
@@ -620,6 +621,59 @@ func (s *ClientService) hasTunnelAttachment(inboundSvc *InboundService, inboundI
|
||||
return false
|
||||
}
|
||||
|
||||
// addressesFitAmneziaWGInbound reports whether every entry in addrs parses
|
||||
// as a host address inside ib's own configured subnet(s). Only AmneziaWG is
|
||||
// checked -- unlike WireGuard (allocateWireguardAddress can widen out to a
|
||||
// /16 fallback pool for it), an AmneziaWG peer's address must fall inside
|
||||
// the kernel interface's own configured Address subnet to be routable at
|
||||
// all (see allocateWireguardAddress's allowWidening doc comment), so
|
||||
// inheriting an address from an unrelated subnet isn't just cosmetically
|
||||
// wrong for AmneziaWG, it produces a peer that can never actually connect.
|
||||
// Real case this guards against: an identity's stored address came from
|
||||
// WireGuard's own fallback subnet (10.0.0.0/24, used when that inbound has
|
||||
// no other clients to infer a base from) and gets attached to a second,
|
||||
// AmneziaWG inbound whose configured subnet is something else entirely
|
||||
// (e.g. 10.8.1.0/24) -- addressesFitAmneziaWGInbound catches that mismatch
|
||||
// so Attach can allocate fresh for this inbound instead of silently
|
||||
// persisting an unroutable peer.
|
||||
func addressesFitAmneziaWGInbound(addrs []string, ib *model.Inbound) bool {
|
||||
if ib.Protocol != model.AmneziaWG || len(addrs) == 0 {
|
||||
return true
|
||||
}
|
||||
v4Base, v6Base, err := defaultAmneziaWGSubnetBases(ib.Settings)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
bases := make([]netip.Prefix, 0, 2)
|
||||
for _, base := range []string{v4Base, v6Base} {
|
||||
if base == "" {
|
||||
continue
|
||||
}
|
||||
prefix, pErr := netip.ParsePrefix(base)
|
||||
if pErr != nil {
|
||||
return false
|
||||
}
|
||||
bases = append(bases, prefix)
|
||||
}
|
||||
for _, a := range addrs {
|
||||
host := wireguardHostAddr(a)
|
||||
if !host.IsValid() {
|
||||
return false
|
||||
}
|
||||
fits := false
|
||||
for _, prefix := range bases {
|
||||
if prefix.Contains(host) {
|
||||
fits = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !fits {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func (s *ClientService) Attach(inboundSvc *InboundService, id int, inboundIds []int) (bool, error) {
|
||||
existing, err := s.GetByID(id)
|
||||
if err != nil {
|
||||
@@ -669,6 +723,9 @@ func (s *ClientService) Attach(inboundSvc *InboundService, id int, inboundIds []
|
||||
return needRestart, getErr
|
||||
}
|
||||
copyClient := *clientWire
|
||||
if !addressesFitAmneziaWGInbound(copyClient.AllowedIPs, inbound) {
|
||||
copyClient.AllowedIPs = nil
|
||||
}
|
||||
if err := s.fillProtocolDefaults(©Client, inbound); err != nil {
|
||||
return needRestart, err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user