From f6a233220ab329800f90bcc9ead2fbd4033358dc Mon Sep 17 00:00:00 2001 From: 25f2002654 Date: Sat, 24 Oct 2026 08:36:49 +0530 Subject: [PATCH] perf(auth): look up api tokens by their hash index (#6915) Every bearer-authenticated request, including each node-sync call, loaded all enabled api_tokens rows and compared each stored SHA-256 hash in constant time. Tokens are stored hashed, so the database can compare hashes directly: a timing leak there reveals only a stored hash, which cannot authenticate. The lookup is now one indexed seek on idx_api_tokens_token, which AutoMigrate creates from the model tag on new and existing databases; scope and expiry are still checked afterwards. --- internal/database/index_tags_test.go | 3 +- internal/database/model/model.go | 2 +- internal/web/service/panel/api_token.go | 29 ++++------- internal/web/service/panel/api_token_test.go | 54 ++++++++++++++++++++ 4 files changed, 66 insertions(+), 22 deletions(-) diff --git a/internal/database/index_tags_test.go b/internal/database/index_tags_test.go index 6dd7ce73c..b5c291d76 100644 --- a/internal/database/index_tags_test.go +++ b/internal/database/index_tags_test.go @@ -21,7 +21,7 @@ func TestAutoMigrateCreatesHotPathIndexes(t *testing.T) { if err != nil { t.Fatalf("open sqlite: %v", err) } - if err := db.AutoMigrate(&model.ClientRecord{}, &xray.ClientTraffic{}, &model.ClientGlobalTraffic{}); err != nil { + if err := db.AutoMigrate(&model.ClientRecord{}, &xray.ClientTraffic{}, &model.ClientGlobalTraffic{}, &model.ApiToken{}); err != nil { t.Fatalf("automigrate: %v", err) } @@ -33,6 +33,7 @@ func TestAutoMigrateCreatesHotPathIndexes(t *testing.T) { {&xray.ClientTraffic{}, "idx_client_traffics_inbound"}, {&xray.ClientTraffic{}, "idx_client_traffics_renew"}, {&model.ClientGlobalTraffic{}, "idx_client_global_email"}, + {&model.ApiToken{}, "idx_api_tokens_token"}, } for _, c := range cases { if !db.Migrator().HasIndex(c.model, c.index) { diff --git a/internal/database/model/model.go b/internal/database/model/model.go index f5dcd781d..e8a3ebc1a 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -178,7 +178,7 @@ func IsNodeSyncApiScope(s string) bool { type ApiToken struct { Id int `json:"id" gorm:"primaryKey;autoIncrement"` Name string `json:"name" gorm:"uniqueIndex;not null"` - Token string `json:"token" gorm:"not null"` // SHA-256 hash; the plaintext is shown only once at creation + Token string `json:"token" gorm:"not null;index:idx_api_tokens_token"` // SHA-256 hash; the plaintext is shown only once at creation Enabled bool `json:"enabled" gorm:"default:true"` CreatedAt int64 `json:"createdAt" gorm:"autoCreateTime"` Scope string `json:"scope" gorm:"not null;default:admin"` diff --git a/internal/web/service/panel/api_token.go b/internal/web/service/panel/api_token.go index 7ff8427aa..eda5fcd8f 100644 --- a/internal/web/service/panel/api_token.go +++ b/internal/web/service/panel/api_token.go @@ -1,7 +1,6 @@ package panel import ( - "crypto/subtle" "errors" "strings" "time" @@ -256,36 +255,26 @@ func requireExpectedScope(expectedScope string) (string, error) { return scope, nil } -// MatchToken returns the enabled, non-expired api_token row whose stored -// SHA-256 hash matches the presented bearer value, or (nil,false). The loop -// scans every enabled row with constant-time compares, then applies expiry and -// scope checks to avoid treating corrupt values as admin. +// MatchToken seeks the enabled row by the presented value's SHA-256 (comparing hashes leaks nothing +// usable), then checks scope and expiry so a corrupt value is never treated as admin. func (s *ApiTokenService) MatchToken(presented string) (*model.ApiToken, bool) { if presented == "" { return nil, false } db := database.GetDB() - var rows []*model.ApiToken - if err := db.Model(model.ApiToken{}).Where("enabled = ?", true).Find(&rows).Error; err != nil { + var row model.ApiToken + if err := db.Model(model.ApiToken{}). + Where("token = ? AND enabled = ?", crypto.HashTokenSHA256(presented), true). + First(&row).Error; err != nil { return nil, false } - presentedHash := []byte(crypto.HashTokenSHA256(presented)) - var matched *model.ApiToken - for _, r := range rows { - if subtle.ConstantTimeCompare([]byte(r.Token), presentedHash) == 1 { - matched = r - } - } - if matched == nil { + if !model.IsKnownApiScope(row.Scope) { return nil, false } - if !model.IsKnownApiScope(matched.Scope) { + if row.ExpiresAt != 0 && nowMilli() >= row.ExpiresAt { return nil, false } - if matched.ExpiresAt != 0 && nowMilli() >= matched.ExpiresAt { - return nil, false - } - return matched, true + return &row, true } // Match is the legacy boolean form for callers that do not need scope. diff --git a/internal/web/service/panel/api_token_test.go b/internal/web/service/panel/api_token_test.go index 28933df20..e82118e8a 100644 --- a/internal/web/service/panel/api_token_test.go +++ b/internal/web/service/panel/api_token_test.go @@ -2,6 +2,7 @@ package panel import ( "errors" + "fmt" "strings" "testing" @@ -276,3 +277,56 @@ func TestRecreateByNameKeepsOneToken(t *testing.T) { t.Fatalf("token rows = %d, want 1", count) } } + +// maxRowsPerQuery reports the largest number of rows any single SELECT inside +// fn returned, which is how a table scan shows up. +func maxRowsPerQuery(t *testing.T, fn func()) int64 { + t.Helper() + db := database.GetDB() + var max int64 + cb := "test:count_api_token_rows" + if err := db.Callback().Query().After("gorm:query").Register(cb, func(tx *gorm.DB) { + if tx.RowsAffected > max { + max = tx.RowsAffected + } + }); err != nil { + t.Fatalf("register query callback: %v", err) + } + defer func() { + if err := db.Callback().Query().Remove(cb); err != nil { + t.Errorf("remove query callback: %v", err) + } + }() + fn() + return max +} + +// Every authenticated request looks its bearer token up. The lookup must read +// the matching row, not every enabled token. +func TestMatchTokenReadsOnlyTheMatchingTokenRow(t *testing.T) { + setupAPITokenTestDB(t) + svc := &ApiTokenService{} + + tokens := make([]string, 0, 25) + for i := range 25 { + created, err := svc.Create(fmt.Sprintf("token-%d", i), model.ApiScopeAdmin, 0) + if err != nil { + t.Fatalf("create token %d: %v", i, err) + } + tokens = append(tokens, created.Token) + } + + rows := maxRowsPerQuery(t, func() { + matched, ok := svc.MatchToken(tokens[7]) + if !ok { + t.Fatal("the created token must match") + } + if matched.Name != "token-7" { + t.Fatalf("matched %q, want token-7", matched.Name) + } + }) + + if rows > 1 { + t.Fatalf("the lookup read %d token rows; it must read only the matching one", rows) + } +}