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.
This commit is contained in:
25f2002654
2026-10-24 05:06:49 +02:00
committed by GitHub
parent 9f0f2b3802
commit f6a233220a
4 changed files with 66 additions and 22 deletions
+2 -1
View File
@@ -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) {
+1 -1
View File
@@ -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"`
+9 -20
View File
@@ -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.
@@ -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)
}
}