Sfoglia il codice sorgente

fix(clients): keep per-peer keys when a client spans several tunnel inbounds

A client attached to several WireGuard/AmneziaWG inbounds is that many
independent peers, each with its own keypair, preshared key and tunnel address.
The client edit form can only represent one peer, so Update's per-inbound loop
stamped that single field set onto every attached inbound: every peer ended up
with identical keys and one inbound's address, and the tunnels on all the other
nodes stopped working with no way to recover the overwritten values from the
panel. The only guard covered AllowedIPs, and only for AmneziaWG.

When more than one tunnel inbound is in scope and the caller sent no per-inbound
override, clear the shared peer fields so UpdateInboundClient's existing
carry-forward preserves each inbound's own. A scoped update (?inboundIds=) still
narrows to one inbound and edits it normally.

Closes #6372
Sanaei 9 ore fa
parent
commit
65c5580e7d

+ 25 - 0
internal/web/service/client_crud.go

@@ -648,6 +648,11 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model
 		}
 	}
 
+	tunnelCount, tcErr := tunnelInboundCount(inboundIds)
+	if tcErr != nil {
+		return false, tcErr
+	}
+
 	// Built before any inbound is written, as in Create: fillProtocolDefaults
 	// mints the shared credentials on the first inbound, later ones reuse them.
 	applies := make([]inboundApply, 0, len(inboundIds))
@@ -673,6 +678,13 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model
 		clientForInbound := updated
 		if ips, ok := updated.AllowedIPsByInbound[ibId]; ok {
 			clientForInbound.AllowedIPs = ips
+		} else if tunnelCount > 1 && (inbound.Protocol == model.WireGuard || inbound.Protocol == model.AmneziaWG) {
+			// One shared peer field set cannot describe several peers: broadcast
+			// it and they all end up with the same keys and tunnel address.
+			clientForInbound.AllowedIPs = nil
+			clientForInbound.PrivateKey = ""
+			clientForInbound.PublicKey = ""
+			clientForInbound.PreSharedKey = ""
 		} else if !addressesFitAmneziaWGInbound(clientForInbound.AllowedIPs, inbound) {
 			// A single shared AllowedIPs field (the common case for a caller
 			// that never sends AllowedIPsByInbound) must never overwrite an
@@ -906,6 +918,19 @@ func (s *ClientService) hasTunnelAttachment(inboundSvc *InboundService, inboundI
 	return false
 }
 
+// tunnelInboundCount reports how many of inboundIds are WireGuard/AmneziaWG,
+// i.e. how many independent peers one shared field set would be written to.
+func tunnelInboundCount(inboundIds []int) (int64, error) {
+	if len(inboundIds) == 0 {
+		return 0, nil
+	}
+	var n int64
+	err := database.GetDB().Model(&model.Inbound{}).
+		Where("id IN ? AND protocol IN ?", inboundIds, []model.Protocol{model.WireGuard, model.AmneziaWG}).
+		Count(&n).Error
+	return n, err
+}
+
 // addressesFitAmneziaWGInbound reports whether every entry in addrs falls
 // inside ib's own configured subnet(s). AmneziaWG only: its kernel interface
 // Address is exactly that subnet, so an address inherited from elsewhere (an

+ 98 - 0
internal/web/service/client_update_tunnel_peers_test.go

@@ -0,0 +1,98 @@
+package service
+
+import (
+	"encoding/base64"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	wgutil "github.com/mhsanaei/3x-ui/v3/internal/util/wireguard"
+)
+
+func wgTestKeypair(t *testing.T, seed byte) (priv, pub string) {
+	t.Helper()
+	raw := make([]byte, 32)
+	for i := range raw {
+		raw[i] = seed
+	}
+	priv = base64.StdEncoding.EncodeToString(raw)
+	pub, err := wgutil.PublicKeyFromPrivate(priv)
+	if err != nil {
+		t.Fatalf("derive public key: %v", err)
+	}
+	return priv, pub
+}
+
+func inboundPeer(t *testing.T, inboundSvc *InboundService, ibId int, email string) model.Client {
+	t.Helper()
+	ib, err := inboundSvc.GetInbound(ibId)
+	if err != nil {
+		t.Fatalf("GetInbound %d: %v", ibId, err)
+	}
+	clients, err := inboundSvc.GetClients(ib)
+	if err != nil {
+		t.Fatalf("GetClients %d: %v", ibId, err)
+	}
+	for i := range clients {
+		if clients[i].Email == email {
+			return clients[i]
+		}
+	}
+	t.Fatalf("email %q not found on inbound %d", email, ibId)
+	return model.Client{}
+}
+
+// A client on several WireGuard inbounds is several independent peers, each
+// with its own keypair and tunnel address. The edit form can only carry one
+// field set, so a save that broadcasts it leaves every peer but one with keys
+// and an address belonging to a different node, breaking those tunnels.
+func TestUpdateDoesNotBroadcastPeerCredentialsAcrossTunnelInbounds(t *testing.T) {
+	setupBulkDB(t)
+	inboundSvc := &InboundService{}
+	svc := &ClientService{}
+
+	const email = "multi@wg"
+	privA, pubA := wgTestKeypair(t, 0x11)
+	privB, pubB := wgTestKeypair(t, 0x22)
+
+	peerA := model.Client{
+		Email: email, SubID: "sub-multi", Enable: true,
+		PrivateKey: privA, PublicKey: pubA, AllowedIPs: []string{"10.10.151.5/32"},
+	}
+	peerB := model.Client{
+		Email: email, SubID: "sub-multi", Enable: true,
+		PrivateKey: privB, PublicKey: pubB, AllowedIPs: []string{"10.10.152.5/32"},
+	}
+
+	ibA := mkInbound(t, 51821, model.WireGuard, clientsSettings(t, []model.Client{peerA}))
+	if err := svc.SyncInbound(nil, ibA.Id, []model.Client{peerA}); err != nil {
+		t.Fatalf("seed inbound A linkage: %v", err)
+	}
+	ibB := mkInbound(t, 51822, model.WireGuard, clientsSettings(t, []model.Client{peerB}))
+	if err := svc.SyncInbound(nil, ibB.Id, []model.Client{peerB}); err != nil {
+		t.Fatalf("seed inbound B linkage: %v", err)
+	}
+	recId := lookupClientRecord(t, email).Id
+
+	// What the client edit form sends: inbound A's peer fields, once, for
+	// a save that only meant to change an unrelated field.
+	updated := model.Client{
+		Email: email, Enable: true, Comment: "renamed",
+		PrivateKey: privA, PublicKey: pubA, AllowedIPs: []string{"10.10.151.5/32"},
+	}
+	if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil {
+		t.Fatalf("Update: %v", err)
+	}
+
+	gotB := inboundPeer(t, inboundSvc, ibB.Id, email)
+	if gotB.PrivateKey != privB || gotB.PublicKey != pubB {
+		t.Fatalf("inbound B peer keys were overwritten with inbound A's: private=%q public=%q", gotB.PrivateKey, gotB.PublicKey)
+	}
+	if len(gotB.AllowedIPs) != 1 || gotB.AllowedIPs[0] != "10.10.152.5/32" {
+		t.Fatalf("inbound B AllowedIPs = %v, want unchanged [10.10.152.5/32]", gotB.AllowedIPs)
+	}
+
+	gotA := inboundPeer(t, inboundSvc, ibA.Id, email)
+	if gotA.PrivateKey != privA || len(gotA.AllowedIPs) != 1 || gotA.AllowedIPs[0] != "10.10.151.5/32" {
+		t.Fatalf("inbound A peer must keep its own values, got %+v", gotA)
+	}
+}