mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-08-09 21:00:58 +00:00
fix(xray): stop the runtime user API from crashing xray-core
Exercising the whole XrayAPI surface against a real xray-core 26.7.28 (the version go.mod pins) turned up a way for ordinary panel activity to kill the core process, plus two smaller mismatches with what the core actually does. buildUserAccount picked the shadowsocks account type by falling through to a 2022 account whenever the cipher was not one of six hardcoded names. xray's legacy and 2022 inbounds cast the account they are handed without checking (proxy/shadowsocks/validator.go, proxy/shadowsocks_2022/inbound_multi.go), so the wrong type is not an error — it panics the core and drops every connection on the server. The fallback was reachable without any misconfiguration: autoRenewClients hands AddUser the client object straight out of the inbound's settings, where the cipher lives under "method", never "cipher", so every auto-renewed client on a legacy-cipher shadowsocks inbound took xray down. The xray-valid aead_* aliases hit it too. The cipher is now read from either key, matched with the same table (and case-insensitivity) the core's own conf package uses, and an unrecognized one is an error instead of a guess. The legacy shadowsocks validator is also the only one that accepts a second user under an email it already holds, and RemoveUser then drops just one of them — a disabled or expired client kept connecting. AddUser now drops the email first on that account type so a single removal fully revokes the client. GetTraffic skipped every stat the first time it saw it. xray creates a counter on a user's first use, so that dropped a new client's traffic for a whole polling interval, as did the counter reset after a core restart. Only the first poll of a process is a baseline now; later, unseen and rewound counters both count from zero. Also fixes three unchecked settings["method"].(string) assertions that panic the panel on a shadowsocks inbound whose settings carry no method, and bounds TestRoute's port so an out-of-range value cannot wrap into the uint32 the core is asked about. Tests: api_users_e2e_test.go drives add/remove for every protocol against a real core and asserts it survives each one (skipped unless XRAY_E2E_BINARY is set); the account-type, traffic-delta and renew paths get unit coverage.
This commit is contained in:
@@ -476,7 +476,7 @@ func (s *ClientService) addInboundClient(inboundSvc *InboundService, data *model
|
||||
}
|
||||
cipher := ""
|
||||
if oldInbound.Protocol == "shadowsocks" {
|
||||
cipher = oldSettings["method"].(string)
|
||||
cipher, _ = oldSettings["method"].(string)
|
||||
}
|
||||
err1 := rt.AddUser(context.Background(), oldInbound, map[string]any{
|
||||
"email": client.Email,
|
||||
@@ -858,7 +858,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
if clients[0].Enable {
|
||||
cipher := ""
|
||||
if oldInbound.Protocol == "shadowsocks" {
|
||||
cipher = oldSettings["method"].(string)
|
||||
cipher, _ = oldSettings["method"].(string)
|
||||
}
|
||||
err1 := rt.AddUser(context.Background(), oldInbound, map[string]any{
|
||||
"email": clients[0].Email,
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/database"
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/xray"
|
||||
)
|
||||
|
||||
// TestAPIUserFromClientCarriesShadowsocksCipher pins what the renew path must
|
||||
// hand the runtime API. A shadowsocks client object holds no cipher of its own,
|
||||
// and xray's legacy and 2022 inbounds take different account types that they
|
||||
// cast without checking — an account built for the wrong one panics the core.
|
||||
func TestAPIUserFromClientCarriesShadowsocksCipher(t *testing.T) {
|
||||
client := map[string]any{"email": "a@x", "password": "pw", "method": "aes-256-gcm"}
|
||||
|
||||
user := apiUserFromClient(client, "aes-256-gcm")
|
||||
if got := user["cipher"]; got != "aes-256-gcm" {
|
||||
t.Fatalf("cipher = %v, want the inbound method", got)
|
||||
}
|
||||
if _, polluted := client["cipher"]; polluted {
|
||||
t.Fatal("the stored client object was mutated; the API-only cipher would be persisted into the inbound settings")
|
||||
}
|
||||
|
||||
user["email"] = "changed@x"
|
||||
if client["email"] != "a@x" {
|
||||
t.Fatal("the API user shares storage with the stored client object")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPIUserFromClientWithoutCipher(t *testing.T) {
|
||||
client := map[string]any{"email": "a@x", "id": "11111111-1111-1111-1111-111111111111"}
|
||||
user := apiUserFromClient(client, "")
|
||||
if _, ok := user["cipher"]; ok {
|
||||
t.Fatal("a non-shadowsocks client must not gain a cipher key")
|
||||
}
|
||||
if user["id"] != client["id"] {
|
||||
t.Fatalf("id = %v, want it copied from the client", user["id"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestAutoRenewShadowsocksKeepsSettingsClean renews a shadowsocks client and
|
||||
// checks the inbound settings the panel writes back: the cipher the API needs
|
||||
// must not leak into a stored client object, where it would end up in the
|
||||
// generated xray config as a per-user key.
|
||||
func TestAutoRenewShadowsocksKeepsSettingsClean(t *testing.T) {
|
||||
setupBulkDB(t)
|
||||
svc := &InboundService{}
|
||||
db := database.GetDB()
|
||||
|
||||
past := time.Now().Add(-48 * time.Hour).UnixMilli()
|
||||
clients := []model.Client{
|
||||
{Email: "ss@x", Password: "pw", Enable: false, Reset: 30, ExpiryTime: past},
|
||||
}
|
||||
|
||||
settings := map[string]any{
|
||||
"method": "aes-256-gcm",
|
||||
"network": "tcp,udp",
|
||||
"clients": []map[string]any{
|
||||
{"email": "ss@x", "password": "pw", "method": "aes-256-gcm", "enable": false, "expiryTime": past, "reset": 30},
|
||||
},
|
||||
}
|
||||
raw, err := json.MarshalIndent(settings, "", " ")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ib := mkInbound(t, 30011, model.Shadowsocks, string(raw))
|
||||
if err := svc.clientService.SyncInbound(nil, ib.Id, clients); err != nil {
|
||||
t.Fatalf("SyncInbound: %v", err)
|
||||
}
|
||||
if err := db.Create(&[]xray.ClientTraffic{
|
||||
{InboundId: ib.Id, Email: "ss@x", Enable: false, Up: 10, Down: 20, Reset: 30, ExpiryTime: past},
|
||||
}).Error; err != nil {
|
||||
t.Fatalf("seed client_traffics: %v", err)
|
||||
}
|
||||
|
||||
if _, count, err := svc.autoRenewClients(db); err != nil {
|
||||
t.Fatalf("autoRenewClients: %v", err)
|
||||
} else if count != 1 {
|
||||
t.Fatalf("renewed count = %d, want 1", count)
|
||||
}
|
||||
|
||||
var stored model.Inbound
|
||||
if err := db.Where("id = ?", ib.Id).First(&stored).Error; err != nil {
|
||||
t.Fatalf("read inbound: %v", err)
|
||||
}
|
||||
var parsed map[string]any
|
||||
if err := json.Unmarshal([]byte(stored.Settings), &parsed); err != nil {
|
||||
t.Fatalf("unmarshal stored settings: %v", err)
|
||||
}
|
||||
storedClients, _ := parsed["clients"].([]any)
|
||||
if len(storedClients) != 1 {
|
||||
t.Fatalf("stored clients = %d, want 1", len(storedClients))
|
||||
}
|
||||
client, _ := storedClients[0].(map[string]any)
|
||||
if _, polluted := client["cipher"]; polluted {
|
||||
t.Fatalf("the renewed client was persisted with an API-only cipher key: %+v", client)
|
||||
}
|
||||
if client["method"] != "aes-256-gcm" {
|
||||
t.Fatalf("the client's method was lost: %+v", client)
|
||||
}
|
||||
if enabled, _ := client["enable"].(bool); !enabled {
|
||||
t.Fatalf("the renewed client was not re-enabled: %+v", client)
|
||||
}
|
||||
}
|
||||
@@ -287,6 +287,23 @@ func (s *InboundService) adjustTraffics(tx *gorm.DB, dbClientTraffics []*xray.Cl
|
||||
return dbClientTraffics, newExpiryByEmail, nil
|
||||
}
|
||||
|
||||
// apiUserFromClient prepares a stored client object for the runtime AddUser
|
||||
// call. The copy matters twice over: the stored object keeps being mutated and
|
||||
// marshalled back into the inbound's settings, which must not gain an API-only
|
||||
// key, and shadowsocks clients carry no cipher of their own — it lives on the
|
||||
// inbound, and without it the API cannot tell which of xray's two shadowsocks
|
||||
// account types the running inbound expects.
|
||||
func apiUserFromClient(client map[string]any, cipher string) map[string]any {
|
||||
user := maps.Clone(client)
|
||||
if user == nil {
|
||||
user = map[string]any{}
|
||||
}
|
||||
if cipher != "" {
|
||||
user["cipher"] = cipher
|
||||
}
|
||||
return user
|
||||
}
|
||||
|
||||
func (s *InboundService) autoRenewClients(tx *gorm.DB) (bool, int64, error) {
|
||||
// check for time expired
|
||||
var traffics []*xray.ClientTraffic
|
||||
@@ -367,6 +384,10 @@ func (s *InboundService) autoRenewClients(tx *gorm.DB) (bool, int64, error) {
|
||||
if len(clients) == 0 {
|
||||
continue
|
||||
}
|
||||
cipher := ""
|
||||
if inbounds[inbound_index].Protocol == model.Shadowsocks {
|
||||
cipher, _ = settings["method"].(string)
|
||||
}
|
||||
for client_index := range clients {
|
||||
c := clients[client_index].(map[string]any)
|
||||
email, _ := c["email"].(string)
|
||||
@@ -393,7 +414,7 @@ func (s *InboundService) autoRenewClients(tx *gorm.DB) (bool, int64, error) {
|
||||
}{
|
||||
protocol: string(inbounds[inbound_index].Protocol),
|
||||
tag: inbounds[inbound_index].Tag,
|
||||
client: c,
|
||||
client: apiUserFromClient(c, cipher),
|
||||
})
|
||||
}
|
||||
clients[client_index] = any(c)
|
||||
@@ -603,7 +624,7 @@ func (s *InboundService) resetClientTrafficLocked(id int, clientEmail string) (b
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
cipher = oldSettings["method"].(string)
|
||||
cipher, _ = oldSettings["method"].(string)
|
||||
}
|
||||
err1 := rt.AddUser(context.Background(), inbound, map[string]any{
|
||||
"email": client.Email,
|
||||
|
||||
Reference in New Issue
Block a user