diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 828e73caa..d24ec8294 100644 --- a/internal/web/service/inbound.go +++ b/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 { diff --git a/internal/web/service/inbound_amneziawg.go b/internal/web/service/inbound_amneziawg.go index a829e60c9..d5844c461 100644 --- a/internal/web/service/inbound_amneziawg.go +++ b/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 } diff --git a/internal/web/service/inbound_amneziawg_test.go b/internal/web/service/inbound_amneziawg_test.go index f7c378d68..9be7ca6b3 100644 --- a/internal/web/service/inbound_amneziawg_test.go +++ b/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) + } +}