Преглед на файлове

fix(amneziawg): stop losing an inbound and its server keys on the API path

Two saves that the panel UI never makes, but the documented REST API does.

A client whose allowedIPs normalized to empty passed validation, then
InstanceFromInbound skipped the peer and dropped the whole instance when it was
the only one. Nothing logged it, so an enabled inbound simply never opened its
socket. Refuse an enabled peer with no address, naming the client, the way the
injection and collision checks already do.

The server keypair was regenerated whenever a payload omitted privateKey, which
invalidates every client config already distributed, and a payload carrying only
privateKey left publicKey empty so rendered configs got a blank "PublicKey =".
An omitted key now means unchanged: the stored pair is carried forward, a
half-supplied pair has its public half derived, and generation is reserved for
an inbound that has no stored keys at all. UpdateInbound loads the stored row
before normalizing so those keys are available.

Closes #6407
Sanaei преди 8 часа
родител
ревизия
705b291d34
променени са 3 файла, в които са добавени 125 реда и са изтрити 18 реда
  1. 7 7
      internal/web/service/inbound.go
  2. 42 5
      internal/web/service/inbound_amneziawg.go
  3. 76 6
      internal/web/service/inbound_amneziawg_test.go

+ 7 - 7
internal/web/service/inbound.go

@@ -1044,7 +1044,7 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo
 	if err := s.normalizeMtprotoXrayPort(inbound, ""); err != nil {
 		return inbound, false, err
 	}
-	if err := s.normalizeAmneziaWGSettings(inbound); err != nil {
+	if err := s.normalizeAmneziaWGSettings(inbound, ""); err != nil {
 		return inbound, false, err
 	}
 	if inbound.NodeID != nil && !isNodeEligibleProtocol(inbound.Protocol) {
@@ -1565,7 +1565,12 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
 		return inbound, false, err
 	}
 	s.normalizeMtprotoSecret(inbound)
-	if err := s.normalizeAmneziaWGSettings(inbound); err != nil {
+
+	oldInbound, err := s.GetInbound(inbound.Id)
+	if err != nil {
+		return inbound, false, err
+	}
+	if err := s.normalizeAmneziaWGSettings(inbound, oldInbound.Settings); err != nil {
 		return inbound, false, err
 	}
 	inbound.SubSortIndex = normalizeSubSortIndex(inbound.SubSortIndex)
@@ -1581,11 +1586,6 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
 			}
 		}
 	}
-
-	oldInbound, err := s.GetInbound(inbound.Id)
-	if err != nil {
-		return inbound, false, err
-	}
 	// Grandfather a row that was already stored incomplete so it stays editable;
 	// only a save that breaks a previously valid TLS block is refused.
 	if !s.FromNodeSync {

+ 42 - 5
internal/web/service/inbound_amneziawg.go

@@ -159,11 +159,45 @@ func fillAmneziaWGServerKeys(server *amneziawg.ServerSettings) error {
 	return nil
 }
 
+// resolveAmneziaWGServerKeys settles the server keypair for a save. An omitted
+// key means "unchanged", never "mint a new one": rotating it silently
+// invalidates every client config already handed out.
+func resolveAmneziaWGServerKeys(server *amneziawg.ServerSettings, oldSettings string) error {
+	if server.PrivateKey == "" {
+		storedPriv, storedPub := storedAmneziaWGServerKeys(oldSettings)
+		if storedPriv == "" {
+			return fillAmneziaWGServerKeys(server)
+		}
+		server.PrivateKey, server.PublicKey = storedPriv, storedPub
+	}
+	if server.PublicKey == "" {
+		pub, err := wgutil.PublicKeyFromPrivate(server.PrivateKey)
+		if err != nil {
+			return fmt.Errorf("amneziawg: derive server public key: %w", err)
+		}
+		server.PublicKey = pub
+	}
+	return nil
+}
+
+// storedAmneziaWGServerKeys returns the keypair already saved for this inbound.
+// oldSettings is empty on a first save, and need not be valid AmneziaWG JSON.
+func storedAmneziaWGServerKeys(oldSettings string) (priv, pub string) {
+	if strings.TrimSpace(oldSettings) == "" {
+		return "", ""
+	}
+	var prev amneziawg.InboundSettings
+	if err := json.Unmarshal([]byte(oldSettings), &prev); err != nil || prev.Server == nil {
+		return "", ""
+	}
+	return prev.Server.PrivateKey, prev.Server.PublicKey
+}
+
 // normalizeAmneziaWGSettings ensures an AmneziaWG inbound's settings have a
 // valid server block, generating one (fresh obfuscation params + keypair) on
 // first save and validating a manually-edited one so a bad entry can't bring
 // the interface down on the next apply. A no-op for every other protocol.
-func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound) error {
+func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound, oldSettings string) error {
 	if inbound.Protocol != model.AmneziaWG {
 		return nil
 	}
@@ -193,10 +227,8 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound) erro
 			return err
 		}
 		parsed.Server = server
-	} else if parsed.Server.PrivateKey == "" {
-		if err := fillAmneziaWGServerKeys(parsed.Server); err != nil {
-			return err
-		}
+	} else if err := resolveAmneziaWGServerKeys(parsed.Server, oldSettings); err != nil {
+		return err
 	}
 	parsed.Server.HeaderProtectionKey = strings.TrimSpace(parsed.Server.HeaderProtectionKey)
 	for _, f := range []*string{
@@ -265,6 +297,11 @@ func (s *InboundService) normalizeAmneziaWGSettings(inbound *model.Inbound) erro
 		if err != nil {
 			return fmt.Errorf("amneziawg: client %q: %w", c.Email, err)
 		}
+		// An enabled peer with no address is skipped by InstanceFromInbound, and
+		// if it was the only one the whole inbound never starts, silently.
+		if c.Enable && len(normalized) == 0 {
+			return fmt.Errorf("amneziawg: client %q: allowedIPs is required", c.Email)
+		}
 		c.AllowedIPs = normalized
 	}
 

+ 76 - 6
internal/web/service/inbound_amneziawg_test.go

@@ -14,8 +14,18 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/internal/database"
 	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
 	"github.com/mhsanaei/3x-ui/v3/internal/logger"
+	wgutil "github.com/mhsanaei/3x-ui/v3/internal/util/wireguard"
 )
 
+// A real X25519 pair, so PublicKeyFromPrivate agrees with the stored value.
+var awgTestPrivateKey, awgTestPublicKey = func() (string, string) {
+	priv, pub, err := wgutil.GenerateWireguardKeypair()
+	if err != nil {
+		panic(err)
+	}
+	return priv, pub
+}()
+
 func TestCheckForwardedPortsConflict_EmptySpecNoConflict(t *testing.T) {
 	setupConflictDB(t)
 	svc := &InboundService{}
@@ -134,7 +144,7 @@ func TestNormalizeAmneziaWGSettings_GeneratesFull31Set(t *testing.T) {
 	setupConflictDB(t)
 	svc := &InboundService{}
 	inbound := &model.Inbound{Protocol: model.AmneziaWG, Port: 51820, Settings: ""}
-	if err := svc.normalizeAmneziaWGSettings(inbound); err != nil {
+	if err := svc.normalizeAmneziaWGSettings(inbound, ""); err != nil {
 		t.Fatalf("normalize empty settings: %v", err)
 	}
 
@@ -188,7 +198,7 @@ func TestNormalizeAmneziaWGSettings_RejectsBad31Values(t *testing.T) {
 			Port:     51820,
 			Settings: `{"server":{"privateKey":"x","publicKey":"y","subnetIp":"10.8.1.0","subnetCidr":24,` + c.snippet + `},"clients":[]}`,
 		}
-		if err := svc.normalizeAmneziaWGSettings(inbound); err == nil {
+		if err := svc.normalizeAmneziaWGSettings(inbound, ""); err == nil {
 			t.Errorf("%s must be rejected", c.name)
 		}
 	}
@@ -203,7 +213,7 @@ func TestNormalizeAmneziaWGSettings_CanonicalizesRangeValues(t *testing.T) {
 		Settings: `{"server":{"privateKey":"x","publicKey":"y","subnetIp":"10.8.1.0","subnetCidr":24,` +
 			`"rekeyAfterTime":"110 - 140","rejectAfterTime":"190-250","keepaliveTimeout":"   "},"clients":[]}`,
 	}
-	if err := svc.normalizeAmneziaWGSettings(inbound); err != nil {
+	if err := svc.normalizeAmneziaWGSettings(inbound, ""); err != nil {
 		t.Fatalf("normalize: %v", err)
 	}
 	var parsed amneziawg.InboundSettings
@@ -245,7 +255,7 @@ func TestNormalizeAmneziaWGSettings_RejectsInjectedClientAllowedIPs(t *testing.T
 			`"clients":[{"email":"a@x","enable":true,"publicKey":"pk",` +
 			`"allowedIPs":["10.8.1.2/32\n[Interface]\nPostUp = touch /tmp/pwned"]}]}`,
 	}
-	err := svc.normalizeAmneziaWGSettings(inbound)
+	err := svc.normalizeAmneziaWGSettings(inbound, "")
 	if err == nil {
 		t.Fatalf("an allowedIPs entry carrying a config-injection payload must be rejected; settings became:\n%s", inbound.Settings)
 	}
@@ -263,7 +273,7 @@ func TestNormalizeAmneziaWGSettings_CanonicalizesClientAllowedIPs(t *testing.T)
 		Settings: `{"server":{"privateKey":"x","publicKey":"y","subnetIp":"10.8.1.0","subnetCidr":24},` +
 			`"clients":[{"email":"a@x","enable":true,"publicKey":"pk","allowedIPs":[" 10.8.1.2 "]}]}`,
 	}
-	if err := svc.normalizeAmneziaWGSettings(inbound); err != nil {
+	if err := svc.normalizeAmneziaWGSettings(inbound, ""); err != nil {
 		t.Fatalf("normalize: %v", err)
 	}
 	var parsed amneziawg.InboundSettings
@@ -375,7 +385,7 @@ func TestNormalizeAmneziaWGSettingsKeepsClearedDNS(t *testing.T) {
 		t.Fatalf("marshal settings: %v", err)
 	}
 	inbound := &model.Inbound{Protocol: model.AmneziaWG, Settings: string(bs)}
-	if err := (&InboundService{}).normalizeAmneziaWGSettings(inbound); err != nil {
+	if err := (&InboundService{}).normalizeAmneziaWGSettings(inbound, ""); err != nil {
 		t.Fatalf("normalizeAmneziaWGSettings: %v", err)
 	}
 	for _, key := range []string{`"primaryDns"`, `"secondaryDns"`} {
@@ -384,3 +394,63 @@ func TestNormalizeAmneziaWGSettingsKeepsClearedDNS(t *testing.T) {
 		}
 	}
 }
+
+// An enabled peer with no address is skipped by InstanceFromInbound, and when it
+// is the only one the entire inbound never starts, with nothing logged anywhere.
+func TestNormalizeAmneziaWGSettings_RejectsEmptyClientAllowedIPs(t *testing.T) {
+	setupConflictDB(t)
+	svc := &InboundService{}
+	inbound := &model.Inbound{Protocol: model.AmneziaWG, Port: 51823, Settings: `{
+		"server": {"privateKey":"` + awgTestPrivateKey + `","publicKey":"` + awgTestPublicKey + `","subnetIp":"10.8.1.0","subnetCidr":24},
+		"clients": [{"email":"ghost","enable":true,"publicKey":"` + awgTestPublicKey + `","allowedIPs":[]}]
+	}`}
+	err := svc.normalizeAmneziaWGSettings(inbound, "")
+	if err == nil || !strings.Contains(err.Error(), "allowedIPs is required") {
+		t.Fatalf("err = %v, want an allowedIPs refusal naming the client", err)
+	}
+	if !strings.Contains(fmt.Sprint(err), "ghost") {
+		t.Fatalf("error must name the offending client, got %v", err)
+	}
+}
+
+// Omitting the server keys on update means "unchanged": minting a fresh pair
+// invalidates every client config already distributed, with no warning.
+func TestNormalizeAmneziaWGSettings_KeepsStoredServerKeysWhenOmitted(t *testing.T) {
+	setupConflictDB(t)
+	svc := &InboundService{}
+	stored := `{"server":{"privateKey":"` + awgTestPrivateKey + `","publicKey":"` + awgTestPublicKey + `","subnetIp":"10.8.1.0","subnetCidr":24}}`
+
+	inbound := &model.Inbound{Protocol: model.AmneziaWG, Port: 51824, Settings: `{"server":{"subnetIp":"10.8.1.0","subnetCidr":24,"randomTrailers":true}}`}
+	if err := svc.normalizeAmneziaWGSettings(inbound, stored); err != nil {
+		t.Fatalf("normalize: %v", err)
+	}
+	var parsed amneziawg.InboundSettings
+	if err := json.Unmarshal([]byte(inbound.Settings), &parsed); err != nil || parsed.Server == nil {
+		t.Fatalf("normalized settings must carry a server block (err=%v): %s", err, inbound.Settings)
+	}
+	if parsed.Server.PrivateKey != awgTestPrivateKey || parsed.Server.PublicKey != awgTestPublicKey {
+		t.Fatalf("server keypair was rotated by an unrelated edit: private=%q public=%q", parsed.Server.PrivateKey, parsed.Server.PublicKey)
+	}
+}
+
+// A payload carrying only the private half used to pass straight through, so
+// every rendered client config got "PublicKey = " with nothing after it.
+func TestNormalizeAmneziaWGSettings_DerivesServerPublicKeyFromPrivate(t *testing.T) {
+	setupConflictDB(t)
+	svc := &InboundService{}
+	inbound := &model.Inbound{Protocol: model.AmneziaWG, Port: 51825, Settings: `{"server":{"privateKey":"` + awgTestPrivateKey + `","subnetIp":"10.8.1.0","subnetCidr":24}}`}
+	if err := svc.normalizeAmneziaWGSettings(inbound, ""); err != nil {
+		t.Fatalf("normalize: %v", err)
+	}
+	var parsed amneziawg.InboundSettings
+	if err := json.Unmarshal([]byte(inbound.Settings), &parsed); err != nil || parsed.Server == nil {
+		t.Fatalf("normalized settings must carry a server block (err=%v): %s", err, inbound.Settings)
+	}
+	want, err := wgutil.PublicKeyFromPrivate(awgTestPrivateKey)
+	if err != nil {
+		t.Fatalf("derive expected key: %v", err)
+	}
+	if parsed.Server.PublicKey != want {
+		t.Fatalf("server publicKey = %q, want %q derived from the supplied private key", parsed.Server.PublicKey, want)
+	}
+}