From 4a6cd2e26a7982ae679f4fca36c3bec919428f39 Mon Sep 17 00:00:00 2001 From: thivindu Date: Mon, 5 Jan 2026 17:02:42 +0530 Subject: [PATCH 1/9] Hash API key using argon2id --- gateway/docker-compose.yaml | 10 +- gateway/gateway-builder/go.mod | 5 +- gateway/gateway-builder/go.sum | 1 + gateway/gateway-controller/go.mod | 2 +- gateway/gateway-controller/go.sum | 1 + .../pkg/apikeyxds/apikey_manager.go | 8 +- .../pkg/apikeyxds/apikey_snapshot.go | 13 +- .../gateway-controller/pkg/models/api_key.go | 25 +- .../pkg/storage/apikey_store.go | 123 +++++++--- .../gateway-controller/pkg/storage/memory.go | 85 +------ .../gateway-controller/pkg/utils/api_key.go | 217 +++++++++++++++--- .../pkg/utils/api_key_hash_test.go | 122 ++++++++++ gateway/policies/api-key-auth/v0.1.0/go.mod | 2 +- gateway/policies/api-key-auth/v0.1.0/go.sum | 2 + gateway/policy-engine/go.mod | 1 + gateway/policy-engine/go.sum | 1 + sdk/gateway/policy/v1alpha/api_key.go | 135 +++++++---- .../policy/v1alpha/api_key_hash_test.go | 197 ++++++++++++++++ sdk/go.mod | 2 +- 19 files changed, 738 insertions(+), 214 deletions(-) create mode 100644 gateway/gateway-controller/pkg/utils/api_key_hash_test.go create mode 100644 gateway/policies/api-key-auth/v0.1.0/go.sum create mode 100644 sdk/gateway/policy/v1alpha/api_key_hash_test.go diff --git a/gateway/docker-compose.yaml b/gateway/docker-compose.yaml index f44ed0ab6..2eee17e65 100644 --- a/gateway/docker-compose.yaml +++ b/gateway/docker-compose.yaml @@ -19,7 +19,7 @@ services: gateway-controller: container_name: gateway-controller - image: ghcr.io/wso2/api-platform/gateway-controller:0.3.0-SNAPSHOT + image: ghcr.io/wso2/api-platform/gateway-controller:latest mem_limit: 128m mem_reservation: 128m cpus: 0.1 @@ -38,14 +38,14 @@ services: - ./configs/config.yaml:/app/config/config.yaml:ro - ./gateway-controller/certificates:/app/certificates # Read-write for dynamic certificate management - ./gateway-controller/listener-certs:/app/listener-certs:ro # Read-only for HTTPS listener certificates - extra_hosts: - - "host.docker.internal:host-gateway" +# extra_hosts: +# - "host.docker.internal:host-gateway" networks: - gateway-network policy-engine: container_name: policy-engine - image: ghcr.io/wso2/api-platform/policy-engine:0.3.0-SNAPSHOT + image: ghcr.io/wso2/api-platform/policy-engine:latest mem_limit: 128m mem_reservation: 128m cpus: 0.2 @@ -59,7 +59,7 @@ services: router: container_name: router - image: ghcr.io/wso2/api-platform/gateway-router:0.3.0-SNAPSHOT + image: ghcr.io/wso2/api-platform/gateway-router:latest mem_limit: 256m mem_reservation: 256m cpus: 0.2 diff --git a/gateway/gateway-builder/go.mod b/gateway/gateway-builder/go.mod index 083ff34bc..9b9901f04 100644 --- a/gateway/gateway-builder/go.mod +++ b/gateway/gateway-builder/go.mod @@ -8,7 +8,10 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -require gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect +require ( + golang.org/x/crypto v0.46.0 // indirect + gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect +) // Local module replacements for Docker builds replace github.com/wso2/api-platform/sdk => ../../sdk diff --git a/gateway/gateway-builder/go.sum b/gateway/gateway-builder/go.sum index 2c8a73fa1..04655ef8b 100644 --- a/gateway/gateway-builder/go.sum +++ b/gateway/gateway-builder/go.sum @@ -7,6 +7,7 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= +golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= golang.org/x/mod v0.30.0 h1:fDEXFVZ/fmCKProc/yAXXUijritrDzahmwwefnjoPFk= golang.org/x/mod v0.30.0/go.mod h1:lAsf5O2EvJeSFMiBxXDki7sCgAxEUcZHXoXMKT4GJKc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/gateway/gateway-controller/go.mod b/gateway/gateway-controller/go.mod index 0a91f3273..fed654a02 100644 --- a/gateway/gateway-controller/go.mod +++ b/gateway/gateway-controller/go.mod @@ -20,6 +20,7 @@ require ( github.com/wso2/api-platform/sdk v0.0.0 github.com/xeipuuv/gojsonschema v1.2.0 go.uber.org/zap v1.27.1 + golang.org/x/crypto v0.46.0 google.golang.org/grpc v1.78.0 google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v3 v3.0.1 @@ -79,7 +80,6 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/arch v0.23.0 // indirect - golang.org/x/crypto v0.46.0 // indirect golang.org/x/net v0.48.0 // indirect golang.org/x/sys v0.39.0 // indirect golang.org/x/text v0.32.0 // indirect diff --git a/gateway/gateway-controller/go.sum b/gateway/gateway-controller/go.sum index 1b05d8ba4..5986c2000 100644 --- a/gateway/gateway-controller/go.sum +++ b/gateway/gateway-controller/go.sum @@ -80,6 +80,7 @@ github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.13-0.20220915233716-71ac16282d12 h1:9Nu54bhS/H/Kgo2/7xNSUuC5G28VR8ljfrLKU2G4IjU= +github.com/json-iterator/go v1.1.13-0.20220915233716-71ac16282d12/go.mod h1:TBzl5BIHNXfS9+C35ZyJaklL7mLDbgUkcgXzSLa8Tk0= github.com/juju/gnuflag v0.0.0-20171113085948-2ce1bb71843d/go.mod h1:2PavIy+JPciBPrBUjwbNvtwB6RQlve+hkpll6QSNmOE= github.com/klauspost/cpuid/v2 v2.3.0 h1:S4CRMLnYUhGeDFDqkGriYKdfoFlDnMtqTiI/sFzhA9Y= github.com/klauspost/cpuid/v2 v2.3.0/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= diff --git a/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go b/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go index 3ae9db085..e9165fa8f 100644 --- a/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go +++ b/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go @@ -76,7 +76,7 @@ func (asm *APIKeyStateManager) RevokeAPIKey(apiId, apiName, apiVersion, apiKeyVa zap.String("correlation_id", correlationID)) // Revoke the API key and update the snapshot - if err := asm.snapshotManager.RevokeAPIKey(apiKeyValue); err != nil { + if err := asm.snapshotManager.RevokeAPIKey(apiId, apiKeyValue); err != nil { asm.logger.Error("Failed to revoke API key and update snapshot", zap.String("api_key_value", asm.MaskAPIKey(apiKeyValue)), zap.Error(err)) @@ -84,6 +84,7 @@ func (asm *APIKeyStateManager) RevokeAPIKey(apiId, apiName, apiVersion, apiKeyVa } asm.logger.Info("Successfully revoked API key and updated policy engine state", + zap.String("api_id", apiId), zap.String("api_key_value", asm.MaskAPIKey(apiKeyValue)), zap.String("correlation_id", correlationID)) @@ -113,11 +114,6 @@ func (asm *APIKeyStateManager) RemoveAPIKeysByAPI(apiId, apiName, apiVersion, co return nil } -// GetAPIKeyByValue retrieves an API key by its value -func (asm *APIKeyStateManager) GetAPIKeyByValue(value string) (*models.APIKey, bool) { - return asm.store.GetByValue(value) -} - // GetAPIKeyByID retrieves an API key by its ID func (asm *APIKeyStateManager) GetAPIKeyByID(id string) (*models.APIKey, bool) { return asm.store.GetByID(id) diff --git a/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go b/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go index 4b3eafe64..2aa11ea84 100644 --- a/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go +++ b/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go @@ -133,12 +133,17 @@ func (sm *APIKeySnapshotManager) StoreAPIKey(apiKey *models.APIKey) error { } // RevokeAPIKey revokes an API key and updates the snapshot -func (sm *APIKeySnapshotManager) RevokeAPIKey(apiKeyValue string) error { - sm.logger.Info("Revoking API key", zap.String("api_key_value", MaskAPIKey(apiKeyValue))) +func (sm *APIKeySnapshotManager) RevokeAPIKey(apiId, apiKeyValue string) error { + sm.logger.Info("Revoking API key", + zap.String("api_id", apiId), + zap.String("api_key_value", MaskAPIKey(apiKeyValue))) // Revoke in the API key store - if !sm.store.Revoke(apiKeyValue) { - return fmt.Errorf("API key not found: %s", MaskAPIKey(apiKeyValue)) + if !sm.store.Revoke(apiId, apiKeyValue) { + sm.logger.Warn("API key not found for revocation", + zap.String("api_id", apiId), + zap.String("api_key_value", MaskAPIKey(apiKeyValue))) + return nil } // Update the snapshot to reflect the new state diff --git a/gateway/gateway-controller/pkg/models/api_key.go b/gateway/gateway-controller/pkg/models/api_key.go index 0045bd349..70055d708 100644 --- a/gateway/gateway-controller/pkg/models/api_key.go +++ b/gateway/gateway-controller/pkg/models/api_key.go @@ -33,18 +33,19 @@ const ( // APIKey represents an API key for an API type APIKey struct { - ID string `json:"id" db:"id"` - Name string `json:"name" db:"name"` - APIKey string `json:"api_key" db:"api_key"` - APIId string `json:"apiId" db:"apiId"` - Operations string `json:"operations" db:"operations"` - Status APIKeyStatus `json:"status" db:"status"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - CreatedBy string `json:"created_by" db:"created_by"` - UpdatedAt time.Time `json:"updated_at" db:"updated_at"` - ExpiresAt *time.Time `json:"expires_at" db:"expires_at"` - Unit *string `json:"-" db:"expires_in_unit"` - Duration *int `json:"-" db:"expires_in_duration"` + ID string `json:"id" db:"id"` + Name string `json:"name" db:"name"` + APIKey string `json:"api_key" db:"api_key"` // Stores hashed API key + PlainAPIKey string `json:"-" db:"-"` // Temporary field for plain API key (not persisted) + APIId string `json:"apiId" db:"apiId"` + Operations string `json:"operations" db:"operations"` + Status APIKeyStatus `json:"status" db:"status"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + CreatedBy string `json:"created_by" db:"created_by"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + ExpiresAt *time.Time `json:"expires_at" db:"expires_at"` + Unit *string `json:"-" db:"expires_in_unit"` + Duration *int `json:"-" db:"expires_in_duration"` } // IsValid checks if the API key is valid (active and not expired) diff --git a/gateway/gateway-controller/pkg/storage/apikey_store.go b/gateway/gateway-controller/pkg/storage/apikey_store.go index 3dabe1764..f6b257b24 100644 --- a/gateway/gateway-controller/pkg/storage/apikey_store.go +++ b/gateway/gateway-controller/pkg/storage/apikey_store.go @@ -19,18 +19,23 @@ package storage import ( + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "strings" "sync" "sync/atomic" "github.com/wso2/api-platform/gateway/gateway-controller/pkg/models" "go.uber.org/zap" + "golang.org/x/crypto/argon2" ) // APIKeyStore manages API keys in memory with thread-safe operations type APIKeyStore struct { mu sync.RWMutex apiKeys map[string]*models.APIKey // key: API key ID - apiKeysByValue map[string]*models.APIKey // key: API key value apiKeysByAPI map[string][]*models.APIKey // key: API ID resourceVersion int64 logger *zap.Logger @@ -39,10 +44,9 @@ type APIKeyStore struct { // NewAPIKeyStore creates a new API key store func NewAPIKeyStore(logger *zap.Logger) *APIKeyStore { return &APIKeyStore{ - apiKeys: make(map[string]*models.APIKey), - apiKeysByValue: make(map[string]*models.APIKey), - apiKeysByAPI: make(map[string][]*models.APIKey), - logger: logger, + apiKeys: make(map[string]*models.APIKey), + apiKeysByAPI: make(map[string][]*models.APIKey), + logger: logger, } } @@ -53,13 +57,11 @@ func (s *APIKeyStore) Store(apiKey *models.APIKey) { // Remove old entry if updating if existing, exists := s.apiKeys[apiKey.ID]; exists { - delete(s.apiKeysByValue, existing.APIKey) s.removeFromAPIMapping(existing) } // Store the API key s.apiKeys[apiKey.ID] = apiKey - s.apiKeysByValue[apiKey.APIKey] = apiKey s.addToAPIMapping(apiKey) s.logger.Debug("Stored API key", @@ -77,15 +79,6 @@ func (s *APIKeyStore) GetByID(id string) (*models.APIKey, bool) { return apiKey, exists } -// GetByValue retrieves an API key by its value -func (s *APIKeyStore) GetByValue(value string) (*models.APIKey, bool) { - s.mu.RLock() - defer s.mu.RUnlock() - - apiKey, exists := s.apiKeysByValue[value] - return apiKey, exists -} - // GetByAPI retrieves all API keys for a specific API func (s *APIKeyStore) GetByAPI(apiId string) []*models.APIKey { s.mu.RLock() @@ -110,24 +103,40 @@ func (s *APIKeyStore) GetAll() []*models.APIKey { return result } -// Revoke marks an API key as revoked -func (s *APIKeyStore) Revoke(apiKeyValue string) bool { +// Revoke marks an API key as revoked by finding it through hash comparison +func (s *APIKeyStore) Revoke(apiId, plainAPIKeyValue string) bool { s.mu.Lock() defer s.mu.Unlock() - apiKey, exists := s.apiKeysByValue[apiKeyValue] + // Get all API keys for the specified API + apiKeys, exists := s.apiKeysByAPI[apiId] if !exists { + s.logger.Debug("No API keys found for API", + zap.String("api_id", apiId)) return false } - // Update status to revoked - apiKey.Status = models.APIKeyStatusRevoked + // Find the API key by comparing plain text key against stored hashes + for _, apiKey := range apiKeys { + // Compare plain text key with stored hashed key using Argon2id + err := compareArgon2id(plainAPIKeyValue, apiKey.APIKey) + if err == nil { + // Hash matches - this is our target API key + apiKey.Status = models.APIKeyStatusRevoked + + s.logger.Debug("Revoked API key", + zap.String("id", apiKey.ID), + zap.String("name", apiKey.Name), + zap.String("api_id", apiKey.APIId)) + + return true + } + } - s.logger.Debug("Revoked API key", - zap.String("id", apiKey.ID), - zap.String("api_id", apiKey.APIId)) + s.logger.Debug("API key not found for revocation", + zap.String("api_id", apiId)) - return true + return false } // RemoveByID removes an API key by its ID @@ -141,7 +150,6 @@ func (s *APIKeyStore) RemoveByID(id string) bool { } delete(s.apiKeys, id) - delete(s.apiKeysByValue, apiKey.APIKey) s.removeFromAPIMapping(apiKey) s.logger.Debug("Removed API key", @@ -161,7 +169,6 @@ func (s *APIKeyStore) RemoveByAPI(apiId string) int { for _, apiKey := range apiKeys { delete(s.apiKeys, apiKey.ID) - delete(s.apiKeysByValue, apiKey.APIKey) } delete(s.apiKeysByAPI, apiId) @@ -211,3 +218,65 @@ func (s *APIKeyStore) removeFromAPIMapping(apiKey *models.APIKey) { delete(s.apiKeysByAPI, apiKey.APIId) } } + +// compareArgon2id validates a plain text key against an Argon2id hash +func compareArgon2id(apiKey, hashedAPIKey string) error { + if apiKey == "" { + return fmt.Errorf("plain API key cannot be empty") + } + if hashedAPIKey == "" { + return fmt.Errorf("hashed API key cannot be empty") + } + + parts := strings.Split(hashedAPIKey, "$") + if len(parts) != 6 || parts[1] != "argon2id" { + return fmt.Errorf("invalid argon2id hash format") + } + + // parts[2] -> v=19 + var version int + if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil { + return err + } + if version != argon2.Version { + return fmt.Errorf("unsupported argon2 version: %d", version) + } + + // parts[3] -> m=65536,t=3,p=4 + var mem uint32 + var iters uint32 + var threads uint8 + var t, m, p uint32 + if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &m, &t, &p); err != nil { + return err + } + mem = m + iters = t + threads = uint8(p) + + // decode salt and hash (try RawStd then Std) + salt, err := decodeBase64(parts[4]) + if err != nil { + return err + } + hash, err := decodeBase64(parts[5]) + if err != nil { + return err + } + + derived := argon2.IDKey([]byte(apiKey), salt, iters, mem, threads, uint32(len(hash))) + if subtle.ConstantTimeCompare(derived, hash) == 1 { + return nil + } + return errors.New("API key mismatch") +} + +// decodeBase64 decodes a base64 string, trying RawStdEncoding first, then StdEncoding +func decodeBase64(s string) ([]byte, error) { + b, err := base64.RawStdEncoding.DecodeString(s) + if err == nil { + return b, nil + } + // try StdEncoding as a fallback + return base64.StdEncoding.DecodeString(s) +} diff --git a/gateway/gateway-controller/pkg/storage/memory.go b/gateway/gateway-controller/pkg/storage/memory.go index 5971445d0..849f3544b 100644 --- a/gateway/gateway-controller/pkg/storage/memory.go +++ b/gateway/gateway-controller/pkg/storage/memory.go @@ -41,7 +41,6 @@ type ConfigStore struct { templateIdByHandle map[string]string // API Keys storage - apiKeys map[string]*models.APIKey // Key: API key value → Value: APIKey apiKeysByAPI map[string][]*models.APIKey // Key: "configID" → Value: slice of APIKeys } @@ -55,7 +54,6 @@ func NewConfigStore() *ConfigStore { TopicManager: NewTopicManager(), templates: make(map[string]*models.StoredLLMProviderTemplate), templateIdByHandle: make(map[string]string), - apiKeys: make(map[string]*models.APIKey), apiKeysByAPI: make(map[string][]*models.APIKey), } } @@ -471,65 +469,32 @@ func (cs *ConfigStore) StoreAPIKey(apiKey *models.APIKey) error { // Check if an API key with the same apiId and name already exists existingKeys, apiIdExists := cs.apiKeysByAPI[apiKey.APIId] var existingKeyIndex = -1 - var oldAPIKeyValue string if apiIdExists { for i, existingKey := range existingKeys { if existingKey.Name == apiKey.Name { existingKeyIndex = i - oldAPIKeyValue = existingKey.APIKey break } } } - // Check if the new API key value already exists (but with different apiId/name) - if _, keyExists := cs.apiKeys[apiKey.APIKey]; keyExists && oldAPIKeyValue != apiKey.APIKey { - return ErrConflict - } + // Note: We no longer check for API key value collisions because: + // 1. API key values are now hashed, making direct comparison impossible + // 2. API keys use 256-bit cryptographic random generation, making collisions highly unlikely + // 3. Any extremely rare hash collisions will be handled at the database level with constraints if existingKeyIndex >= 0 { - // Update existing API key - // Remove old API key value from apiKeys map if it's different - if oldAPIKeyValue != apiKey.APIKey { - delete(cs.apiKeys, oldAPIKeyValue) - } - // Update the existing entry in apiKeysByAPI cs.apiKeysByAPI[apiKey.APIId][existingKeyIndex] = apiKey - - // Store by new API key value - cs.apiKeys[apiKey.APIKey] = apiKey } else { // Insert new API key - // Check if API key value already exists - if _, exists := cs.apiKeys[apiKey.APIKey]; exists { - return ErrConflict - } - - // Store by API key value - cs.apiKeys[apiKey.APIKey] = apiKey - - // Store by API apiId cs.apiKeysByAPI[apiKey.APIId] = append(cs.apiKeysByAPI[apiKey.APIId], apiKey) } return nil } -// GetAPIKeyByKey retrieves an API key by its key value -func (cs *ConfigStore) GetAPIKeyByKey(key string) (*models.APIKey, error) { - cs.mu.RLock() - defer cs.mu.RUnlock() - - apiKey, exists := cs.apiKeys[key] - if !exists { - return nil, ErrNotFound - } - - return apiKey, nil -} - // GetAPIKeysByAPI retrieves all API keys for a specific API func (cs *ConfigStore) GetAPIKeysByAPI(apiId string) ([]*models.APIKey, error) { cs.mu.RLock() @@ -566,53 +531,16 @@ func (cs *ConfigStore) GetAPIKeyByName(apiId, name string) (*models.APIKey, erro return nil, ErrNotFound } -// RemoveAPIKey removes an API key from the in-memory cache -func (cs *ConfigStore) RemoveAPIKey(apiKey string) error { - cs.mu.Lock() - defer cs.mu.Unlock() - - // Get the API key first to find its API association - key, exists := cs.apiKeys[apiKey] - if !exists { - return ErrNotFound - } - - // Remove from main map - delete(cs.apiKeys, apiKey) - - // Remove from API-specific map - if apiKeys, exists := cs.apiKeysByAPI[key.APIId]; exists { - for i, k := range apiKeys { - if k.APIKey == apiKey { - // Remove from slice - cs.apiKeysByAPI[key.APIId] = append(apiKeys[:i], apiKeys[i+1:]...) - break - } - } - // Clean up empty slices - if len(cs.apiKeysByAPI[key.APIId]) == 0 { - delete(cs.apiKeysByAPI, key.APIId) - } - } - - return nil -} - // RemoveAPIKeysByAPI removes all API keys for a specific API func (cs *ConfigStore) RemoveAPIKeysByAPI(apiId string) error { cs.mu.Lock() defer cs.mu.Unlock() - apiKeys, exists := cs.apiKeysByAPI[apiId] + _, exists := cs.apiKeysByAPI[apiId] if !exists { return nil // No keys to remove } - // Remove from main map - for _, key := range apiKeys { - delete(cs.apiKeys, key.APIKey) - } - // Remove from API-specific map delete(cs.apiKeysByAPI, apiId) @@ -646,9 +574,6 @@ func (cs *ConfigStore) RemoveAPIKeyByName(apiId, name string) error { return ErrNotFound } - // Remove from main apiKeys map - delete(cs.apiKeys, targetAPIKey.APIKey) - // Remove from apiKeysByAPI slice cs.apiKeysByAPI[apiId] = append(apiKeys[:targetIndex], apiKeys[targetIndex+1:]...) diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index 1ef2041b3..9b43497d9 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -20,6 +20,8 @@ package utils import ( "crypto/rand" + "crypto/subtle" + "encoding/base64" "encoding/hex" "encoding/json" "errors" @@ -34,6 +36,7 @@ import ( "github.com/wso2/api-platform/gateway/gateway-controller/pkg/models" "github.com/wso2/api-platform/gateway/gateway-controller/pkg/storage" "go.uber.org/zap" + "golang.org/x/crypto/argon2" ) // APIKeyGenerationParams contains parameters for API key generation operations @@ -117,7 +120,15 @@ func NewAPIKeyService(store *storage.ConfigStore, db storage.Storage, xdsManager } } -const APIKeyPrefix = "apip_" +const ( + APIKeyPrefix = "apip_" + // Argon2id parameters (recommended for production security) + argon2Time = 1 // Number of iterations + argon2Memory = 64 * 1024 // Memory usage in KiB (64 MiB) + argon2Threads = 4 // Number of threads + argon2KeyLen = 32 // Length of derived key in bytes + argon2SaltLen = 16 // Length of salt in bytes +) // GenerateAPIKey handles the complete API key generation process func (s *APIKeyService) GenerateAPIKey(params APIKeyGenerationParams) (*APIKeyGenerationResult, error) { @@ -259,21 +270,43 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo return nil, fmt.Errorf("API configuration handle '%s' not found", params.Handle) } - apiKeyValue := strings.TrimSpace(params.Request.ApiKey) - if apiKeyValue == "" { + providedAPIKeyValue := strings.TrimSpace(params.Request.ApiKey) + if providedAPIKeyValue == "" { logger.Warn("API key value is required for revocation", zap.String("handle", params.Handle), zap.String("correlation_id", params.CorrelationID)) return nil, fmt.Errorf("API key value is required for revocation") } - // Get the API key by its value - apiKey, err := s.store.GetAPIKeyByKey(apiKeyValue) + // Get all API keys for this API and find the one that matches the plain text key + var apiKey *models.APIKey + var matchedKey *models.APIKey + + // First, try to get API keys from memory store + apiKeys, err := s.store.GetAPIKeysByAPI(config.ID) if err != nil { - logger.Debug("API key not found for revocation", - zap.String("handle", params.Handle), - zap.String("correlation_id", params.CorrelationID)) - //return nil + // If memory store fails, try database + if s.db != nil { + apiKeys, err = s.db.GetAPIKeysByAPI(config.ID) + if err != nil { + logger.Debug("Failed to get API keys for revocation", + zap.Error(err), + zap.String("handle", params.Handle), + zap.String("correlation_id", params.CorrelationID)) + // Continue with revocation for security reasons (don't leak info) + } + } + } + + // Find the API key that matches the provided plain text key + if apiKeys != nil { + for _, key := range apiKeys { + if s.ValidateAPIKey(providedAPIKeyValue, key.APIKey) { + matchedKey = key + apiKey = key + break + } + } } result := &APIKeyRevocationResult{ @@ -327,8 +360,8 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo } } - // Remove the API key from memory store - if err := s.store.RemoveAPIKey(apiKeyValue); err != nil { + // Remove the API key from memory store by name (since we have the matched key) + if err := s.store.RemoveAPIKeyByName(config.ID, apiKey.Name); err != nil { logger.Error("Failed to remove API key from memory store", zap.Error(err), zap.String("handle", params.Handle), @@ -349,8 +382,8 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo // Remove the API key from database (complete removal) // Note: This is cleanup only - the revocation is already complete - if s.db != nil { - if err := s.db.DeleteAPIKey(apiKeyValue); err != nil { + if s.db != nil && matchedKey != nil { + if err := s.db.RemoveAPIKeyAPIAndName(config.ID, matchedKey.Name); err != nil { logger.Warn("Failed to remove API key from database, but revocation was successful", zap.Error(err), zap.String("handle", params.Handle), @@ -375,15 +408,16 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo apiVersion := apiConfig.Version logger.Info("Removing API key from policy engine", zap.String("handle", params.Handle), - zap.String("api key", s.MaskAPIKey(apiKeyValue)), + zap.String("api key", s.MaskAPIKey(providedAPIKeyValue)), zap.String("api_name", apiName), zap.String("api_version", apiVersion), zap.String("user", user.UserID), zap.String("correlation_id", params.CorrelationID)) - // Send the API key revocation to the policy engine via xDS + // Send the plain API key revocation to the policy engine via xDS + // The policy engine will find and revoke the matching hashed key if s.xdsManager != nil { - if err := s.xdsManager.RevokeAPIKey(apiId, apiName, apiVersion, apiKeyValue, params.CorrelationID); err != nil { + if err := s.xdsManager.RevokeAPIKey(apiId, apiName, apiVersion, providedAPIKeyValue, params.CorrelationID); err != nil { logger.Error("Failed to remove API key from policy engine", zap.Error(err), zap.String("correlation_id", params.CorrelationID)) @@ -393,7 +427,7 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo logger.Info("API key revoked successfully", zap.String("handle", params.Handle), - zap.String("api key", s.MaskAPIKey(apiKeyValue)), + zap.String("api key", s.MaskAPIKey(providedAPIKeyValue)), zap.String("user", user.UserID), zap.String("correlation_id", params.CorrelationID)) @@ -664,11 +698,17 @@ func (s *APIKeyService) generateAPIKeyFromRequest(handle string, request *api.AP id := uuid.New().String() // Generate 32 random bytes for the API key - apiKeyValue, err := s.generateAPIKeyValue() + plainAPIKeyValue, err := s.generateAPIKeyValue() if err != nil { return nil, err } + // Hash the API key for storage and policy engine + hashedAPIKeyValue, err := s.hashAPIKey(plainAPIKeyValue) + if err != nil { + return nil, fmt.Errorf("failed to hash API key: %w", err) + } + // Set name - use provided name or generate a default one name := fmt.Sprintf("%s-key-%s", handle, id[:8]) // Default name if request.Name != nil && strings.TrimSpace(*request.Name) != "" { @@ -716,10 +756,10 @@ func (s *APIKeyService) generateAPIKeyFromRequest(handle string, request *api.AP expiresAt = &expiry } - return &models.APIKey{ + apiKey := &models.APIKey{ ID: id, Name: name, - APIKey: apiKeyValue, + APIKey: hashedAPIKeyValue, // Store hashed key in database and policy engine APIId: config.ID, Operations: operations, Status: models.APIKeyStatusActive, @@ -729,7 +769,13 @@ func (s *APIKeyService) generateAPIKeyFromRequest(handle string, request *api.AP ExpiresAt: expiresAt, Unit: unit, Duration: duration, - }, nil + } + + // Temporarily store the plain key for response generation + // This field is not persisted and only used for returning to user + apiKey.PlainAPIKey = plainAPIKeyValue + + return apiKey, nil } // generateOperationsString creates a string array from operations in format "METHOD path" @@ -766,12 +812,21 @@ func (s *APIKeyService) buildAPIKeyResponse(key *models.APIKey, handle string) a } } + // Use PlainAPIKey for response if available, otherwise mask the hashed key + var responseAPIKey *string + if key.PlainAPIKey != "" { + responseAPIKey = &key.PlainAPIKey + } else { + // For existing keys where PlainAPIKey is not available, don't return the hashed key + responseAPIKey = nil + } + return api.APIKeyGenerationResponse{ Status: "success", Message: "API key generated successfully", ApiKey: &api.APIKey{ Name: key.Name, - ApiKey: &key.APIKey, + ApiKey: responseAPIKey, // Return plain key only during generation/rotation ApiId: handle, Operations: key.Operations, Status: api.APIKeyStatus(key.Status), @@ -786,11 +841,17 @@ func (s *APIKeyService) buildAPIKeyResponse(key *models.APIKey, handle string) a func (s *APIKeyService) generateRotatedAPIKey(existingKey *models.APIKey, request api.APIKeyRotationRequest, user string, logger *zap.Logger) (*models.APIKey, error) { // Generate new API key value - newAPIKeyValue, err := s.generateAPIKeyValue() + plainAPIKeyValue, err := s.generateAPIKeyValue() if err != nil { return nil, err } + // Hash the new API key for storage + hashedAPIKeyValue, err := s.hashAPIKey(plainAPIKeyValue) + if err != nil { + return nil, fmt.Errorf("failed to hash rotated API key: %w", err) + } + now := time.Now() // Determine expiration settings based on request and existing key @@ -873,10 +934,10 @@ func (s *APIKeyService) generateRotatedAPIKey(existingKey *models.APIKey, reques } // Create the rotated API key - return &models.APIKey{ + rotatedKey := &models.APIKey{ ID: existingKey.ID, Name: existingKey.Name, - APIKey: newAPIKeyValue, + APIKey: hashedAPIKeyValue, // Store hashed key APIId: existingKey.APIId, Operations: existingKey.Operations, Status: models.APIKeyStatusActive, @@ -886,7 +947,12 @@ func (s *APIKeyService) generateRotatedAPIKey(existingKey *models.APIKey, reques ExpiresAt: expiresAt, Unit: unit, Duration: duration, - }, nil + } + + // Temporarily store the plain key for response generation + rotatedKey.PlainAPIKey = plainAPIKeyValue + + return rotatedKey, nil } // canRevokeAPIKey determines if a user can revoke a specific API key @@ -1026,3 +1092,102 @@ func (s *APIKeyService) isAdmin(user *commonmodels.AuthContext) bool { func (s *APIKeyService) isDeveloper(user *commonmodels.AuthContext) bool { return slices.Contains(user.Roles, "developer") } + +// hashAPIKey securely hashes an API key using Argon2id +// Returns the hashed API key that should be stored in database and policy engine +func (s *APIKeyService) hashAPIKey(plainAPIKey string) (string, error) { + if plainAPIKey == "" { + return "", fmt.Errorf("API key cannot be empty") + } + + // Generate random salt + salt := make([]byte, argon2SaltLen) + if _, err := rand.Read(salt); err != nil { + return "", fmt.Errorf("failed to generate salt: %w", err) + } + + // Generate hash using Argon2id + hash := argon2.IDKey([]byte(plainAPIKey), salt, argon2Time, argon2Memory, argon2Threads, argon2KeyLen) + + // Encode salt and hash using base64 + saltEncoded := base64.RawStdEncoding.EncodeToString(salt) + hashEncoded := base64.RawStdEncoding.EncodeToString(hash) + + // Format: $argon2id$v=19$m=65536,t=1,p=4$$ + hashedKey := fmt.Sprintf("$argon2id$v=19$m=%d,t=%d,p=%d$%s$%s", + argon2Memory, argon2Time, argon2Threads, saltEncoded, hashEncoded) + + return hashedKey, nil +} + +// ValidateAPIKey is a public method to validate API keys for external use +// Returns true if the plain API key matches the hash, false otherwise +func (s *APIKeyService) ValidateAPIKey(providedAPIKey, storedAPIKey string) bool { + if providedAPIKey == "" { + return false + } + if storedAPIKey == "" { + return false + } + if strings.HasPrefix(storedAPIKey, "$argon2id$") { + err := s.compareArgon2id(providedAPIKey, storedAPIKey) + return err == nil + } + return false +} + +// compareArgon2id parses an encoded Argon2id hash and compares it to the provided password. +// Expected format: $argon2id$v=19$m=65536,t=3,p=4$$ +func (s *APIKeyService) compareArgon2id(apiKey, encoded string) error { + parts := strings.Split(encoded, "$") + if len(parts) != 6 || parts[1] != "argon2id" { + return fmt.Errorf("invalid argon2id hash format") + } + + // parts[2] -> v=19 + var version int + if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil { + return err + } + if version != argon2.Version { + return fmt.Errorf("unsupported argon2 version: %d", version) + } + + // parts[3] -> m=65536,t=3,p=4 + var mem uint32 + var iters uint32 + var threads uint8 + var t, m, p uint32 + if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &m, &t, &p); err != nil { + return err + } + mem = m + iters = t + threads = uint8(p) + + // decode salt and hash (try RawStd then Std) + salt, err := decodeBase64(parts[4]) + if err != nil { + return err + } + hash, err := decodeBase64(parts[5]) + if err != nil { + return err + } + + derived := argon2.IDKey([]byte(apiKey), salt, iters, mem, threads, uint32(len(hash))) + if subtle.ConstantTimeCompare(derived, hash) == 1 { + return nil + } + return errors.New("API key mismatch") +} + +// decodeBase64 decodes a base64 string, trying RawStdEncoding first, then StdEncoding +func decodeBase64(s string) ([]byte, error) { + b, err := base64.RawStdEncoding.DecodeString(s) + if err == nil { + return b, nil + } + // try StdEncoding as a fallback + return base64.StdEncoding.DecodeString(s) +} diff --git a/gateway/gateway-controller/pkg/utils/api_key_hash_test.go b/gateway/gateway-controller/pkg/utils/api_key_hash_test.go new file mode 100644 index 000000000..fcbe87293 --- /dev/null +++ b/gateway/gateway-controller/pkg/utils/api_key_hash_test.go @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package utils + +import ( + "strings" + "testing" +) + +func TestAPIKeyHashing(t *testing.T) { + service := &APIKeyService{} + + // Test API key generation and hashing + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Test hashing + hashedKey, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key: %v", err) + } + + // Verify the hashed key is different from plain key + if hashedKey == plainKey { + t.Error("Hashed key should be different from plain key") + } + + // Verify the hash starts with argon2id prefix + if !strings.HasPrefix(hashedKey, "$argon2id$v=19$") { + t.Error("Hashed key should start with $argon2id$v=19$ prefix") + } + + // Test validation with correct key + valid := service.ValidateAPIKey(plainKey, hashedKey) + if !valid { + t.Error("Validation should succeed with correct plain key") + } + + // Test validation with incorrect key + wrongKey := "apip_wrong123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + valid = service.ValidateAPIKey(wrongKey, hashedKey) + if valid { + t.Error("Validation should fail with incorrect plain key") + } + + // Test empty keys + _, err = service.hashAPIKey("") + if err == nil { + t.Error("Hashing empty key should return error") + } + + valid = service.ValidateAPIKey("", hashedKey) + if valid { + t.Error("Validation should fail with empty plain key") + } + + valid = service.ValidateAPIKey(plainKey, "") + if valid { + t.Error("Validation should fail with empty hash") + } +} + +func TestAPIKeyHashParameters(t *testing.T) { + service := &APIKeyService{} + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + hashedKey, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key: %v", err) + } + + // Verify the format includes the expected Argon2id parameters + expectedParams := "m=65536,t=1,p=4" + if !strings.Contains(hashedKey, expectedParams) { + t.Errorf("Hash should contain parameters %s, got: %s", expectedParams, hashedKey) + } +} + +func TestAPIKeyHashDeterminism(t *testing.T) { + service := &APIKeyService{} + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Generate two hashes of the same key + hash1, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key (1): %v", err) + } + + hash2, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key (2): %v", err) + } + + // Hashes should be different due to random salt + if hash1 == hash2 { + t.Error("Two hashes of the same key should be different (Argon2id uses random salt)") + } + + // But both should validate against the same plain key + if !service.ValidateAPIKey(plainKey, hash1) { + t.Error("First hash should validate correctly") + } + + if !service.ValidateAPIKey(plainKey, hash2) { + t.Error("Second hash should validate correctly") + } +} diff --git a/gateway/policies/api-key-auth/v0.1.0/go.mod b/gateway/policies/api-key-auth/v0.1.0/go.mod index 7ccc5b367..84d2868f5 100644 --- a/gateway/policies/api-key-auth/v0.1.0/go.mod +++ b/gateway/policies/api-key-auth/v0.1.0/go.mod @@ -1,5 +1,5 @@ module github.com/policy-engine/policies/api-key-auth -go 1.23.0 +go 1.25.1 require github.com/wso2/api-platform/sdk v0.3.0 diff --git a/gateway/policies/api-key-auth/v0.1.0/go.sum b/gateway/policies/api-key-auth/v0.1.0/go.sum new file mode 100644 index 000000000..0c64e1414 --- /dev/null +++ b/gateway/policies/api-key-auth/v0.1.0/go.sum @@ -0,0 +1,2 @@ +github.com/wso2/api-platform/sdk v0.3.0 h1:OmZv0Kltc/fOtgRdsMikhodQAWZG+lVjNPtOZxl/2OQ= +github.com/wso2/api-platform/sdk v0.3.0/go.mod h1:byr46IKr+KyUuPT7hm/Si+KosOtLQt5tjMbHFhexQgM= diff --git a/gateway/policy-engine/go.mod b/gateway/policy-engine/go.mod index bfff24c75..14b778e59 100644 --- a/gateway/policy-engine/go.mod +++ b/gateway/policy-engine/go.mod @@ -43,6 +43,7 @@ require ( go.opentelemetry.io/otel/metric v1.39.0 // indirect go.opentelemetry.io/proto/otlp v1.9.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect + golang.org/x/crypto v0.46.0 // indirect golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 // indirect golang.org/x/net v0.48.0 // indirect golang.org/x/sys v0.39.0 // indirect diff --git a/gateway/policy-engine/go.sum b/gateway/policy-engine/go.sum index 114edf514..970756d6f 100644 --- a/gateway/policy-engine/go.sum +++ b/gateway/policy-engine/go.sum @@ -96,6 +96,7 @@ go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 h1:R84qjqJb5nVJMxqWYb3np9L5ZsaDtB+a39EqjV0JSUM= golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0/go.mod h1:S9Xr4PYopiDyqSyp5NjCrhFrqg6A5zA2E/iPHPhqnS8= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= diff --git a/sdk/gateway/policy/v1alpha/api_key.go b/sdk/gateway/policy/v1alpha/api_key.go index 022b03bcd..59c390bd0 100644 --- a/sdk/gateway/policy/v1alpha/api_key.go +++ b/sdk/gateway/policy/v1alpha/api_key.go @@ -1,12 +1,16 @@ package policyv1alpha import ( + "crypto/subtle" + "encoding/base64" "encoding/json" "errors" "fmt" "strings" "sync" "time" + + "golang.org/x/crypto/argon2" ) type APIKey struct { @@ -61,14 +65,12 @@ var ( type APIkeyStore struct { mu sync.RWMutex // Protects concurrent access // API Keys storage - apiKeys map[string]*APIKey // Key: API key value → Value: APIKey apiKeysByAPI map[string][]*APIKey // Key: "API ID" → Value: slice of APIKeys } // NewAPIkeyStore creates a new in-memory API key store func NewAPIkeyStore() *APIkeyStore { return &APIkeyStore{ - apiKeys: make(map[string]*APIKey), apiKeysByAPI: make(map[string][]*APIKey), } } @@ -93,46 +95,26 @@ func (aks *APIkeyStore) StoreAPIKey(apiId string, apiKey *APIKey) error { // Check if an API key with the same apiId and name already exists existingKeys, apiIdExists := aks.apiKeysByAPI[apiId] var existingKeyIndex = -1 - var oldAPIKeyValue string if apiIdExists { for i, existingKey := range existingKeys { if existingKey.Name == apiKey.Name { existingKeyIndex = i - oldAPIKeyValue = existingKey.APIKey break } } } - // Check if the new API key value already exists (but with different apiId/name) - if _, keyExists := aks.apiKeys[apiKey.APIKey]; keyExists && oldAPIKeyValue != apiKey.APIKey { - return ErrConflict - } + // Note: We no longer check for API key value collisions because: + // 1. API key values are now hashed, making direct comparison impossible + // 2. API keys use 256-bit cryptographic random generation, making collisions highly unlikely + // 3. Any extremely rare hash collisions will be handled at the database level with constraints if existingKeyIndex >= 0 { - // Update existing API key - // Remove old API key value from apiKeys map if it's different - if oldAPIKeyValue != apiKey.APIKey { - delete(aks.apiKeys, oldAPIKeyValue) - } - // Update the existing entry in apiKeysByAPI aks.apiKeysByAPI[apiId][existingKeyIndex] = apiKey - - // Store by new API key value - aks.apiKeys[apiKey.APIKey] = apiKey } else { // Insert new API key - // Check if API key value already exists - if _, exists := aks.apiKeys[apiKey.APIKey]; exists { - return ErrConflict - } - - // Store by API key value - aks.apiKeys[apiKey.APIKey] = apiKey - - // Store by API ID aks.apiKeysByAPI[apiId] = append(aks.apiKeysByAPI[apiId], apiKey) } @@ -140,7 +122,7 @@ func (aks *APIkeyStore) StoreAPIKey(apiId string, apiKey *APIKey) error { } // ValidateAPIKey validates the provided API key against the internal APIkey store -func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, apiKey string) (bool, error) { +func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, providedAPIKey string) (bool, error) { aks.mu.Lock() defer aks.mu.Unlock() @@ -150,13 +132,18 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, api return false, ErrNotFound } - // Find the API key with the matching key value + // Find the API key that matches the provided plain text key (by comparing against hashed values) var targetAPIKey *APIKey for _, ak := range apiKeys { - if ak.APIKey == apiKey { - targetAPIKey = ak - break + // Compare provided plain text key with stored hashed key using Argon2id + if strings.HasPrefix(ak.APIKey, "$argon2id$") { + err := compareArgon2id(providedAPIKey, ak.APIKey) + if err == nil { + // Hash matches - this is our target API key + targetAPIKey = ak + break + } } } @@ -202,8 +189,8 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, api return false, nil } -// RevokeAPIKey revokes a specific API key by API key value -func (aks *APIkeyStore) RevokeAPIKey(apiId, apiKeyValue string) error { +// RevokeAPIKey revokes a specific API key by plain text API key value +func (aks *APIkeyStore) RevokeAPIKey(apiId, plainAPIKeyValue string) error { aks.mu.Lock() defer aks.mu.Unlock() @@ -215,12 +202,14 @@ func (aks *APIkeyStore) RevokeAPIKey(apiId, apiKeyValue string) error { return nil } - // Find the API key with the matching key value + // Find the API key with the matching hashed key value var targetAPIKey *APIKey var targetIndex = -1 for i, apiKey := range apiKeys { - if apiKey.APIKey == apiKeyValue { + // Compare plain text key with stored hashed key using Argon2id + err := compareArgon2id(plainAPIKeyValue, apiKey.APIKey) + if err == nil { targetAPIKey = apiKey targetIndex = i break @@ -235,9 +224,6 @@ func (aks *APIkeyStore) RevokeAPIKey(apiId, apiKeyValue string) error { // Set status to revoked targetAPIKey.Status = Revoked - // Remove from main apiKeys map - delete(aks.apiKeys, apiKeyValue) - // Remove from apiKeysByAPI slice aks.apiKeysByAPI[apiId] = append(apiKeys[:targetIndex], apiKeys[targetIndex+1:]...) @@ -254,16 +240,11 @@ func (aks *APIkeyStore) RemoveAPIKeysByAPI(apiId string) error { aks.mu.Lock() defer aks.mu.Unlock() - apiKeys, exists := aks.apiKeysByAPI[apiId] + _, exists := aks.apiKeysByAPI[apiId] if !exists { return nil // No keys to remove } - // Remove from main map - for _, key := range apiKeys { - delete(aks.apiKeys, key.APIKey) - } - // Remove from API-specific map delete(aks.apiKeysByAPI, apiId) @@ -275,16 +256,70 @@ func (aks *APIkeyStore) ClearAll() error { aks.mu.Lock() defer aks.mu.Unlock() - // Clear the main API keys map - aks.apiKeys = make(map[string]*APIKey) - // Clear the API-specific keys map aks.apiKeysByAPI = make(map[string][]*APIKey) return nil } -// compositeKey creates a composite key from name and version -func compositeKey(name, version string) string { - return fmt.Sprintf("%s:%s", name, version) +// compareArgon2id validates a plain text key against an Argon2id hash +func compareArgon2id(apiKey, hashedAPIKey string) error { + if apiKey == "" { + return fmt.Errorf("plain API key cannot be empty") + } + if hashedAPIKey == "" { + return fmt.Errorf("hashed API key cannot be empty") + } + + parts := strings.Split(hashedAPIKey, "$") + if len(parts) != 6 || parts[1] != "argon2id" { + return fmt.Errorf("invalid argon2id hash format") + } + + // parts[2] -> v=19 + var version int + if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil { + return err + } + if version != argon2.Version { + return fmt.Errorf("unsupported argon2 version: %d", version) + } + + // parts[3] -> m=65536,t=3,p=4 + var mem uint32 + var iters uint32 + var threads uint8 + var t, m, p uint32 + if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &m, &t, &p); err != nil { + return err + } + mem = m + iters = t + threads = uint8(p) + + // decode salt and hash (try RawStd then Std) + salt, err := decodeBase64(parts[4]) + if err != nil { + return err + } + hash, err := decodeBase64(parts[5]) + if err != nil { + return err + } + + derived := argon2.IDKey([]byte(apiKey), salt, iters, mem, threads, uint32(len(hash))) + if subtle.ConstantTimeCompare(derived, hash) == 1 { + return nil + } + return errors.New("API key mismatch") +} + +// decodeBase64 decodes a base64 string, trying RawStdEncoding first, then StdEncoding +func decodeBase64(s string) ([]byte, error) { + b, err := base64.RawStdEncoding.DecodeString(s) + if err == nil { + return b, nil + } + // try StdEncoding as a fallback + return base64.StdEncoding.DecodeString(s) } diff --git a/sdk/gateway/policy/v1alpha/api_key_hash_test.go b/sdk/gateway/policy/v1alpha/api_key_hash_test.go new file mode 100644 index 000000000..86f202a5e --- /dev/null +++ b/sdk/gateway/policy/v1alpha/api_key_hash_test.go @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package policyv1alpha + +import ( + "crypto/rand" + "encoding/base64" + "fmt" + "testing" + "time" + + "golang.org/x/crypto/argon2" +) + +func TestAPIKeyHashedValidation(t *testing.T) { + store := NewAPIkeyStore() + + // Create a plain text API key (69 bytes like real generated keys) + plainAPIKey := "apip_88f8399ef29761f92f4f6d2dbd6dcd78399b3bcb8c53417cb23726e5780ac215" + + // Hash the API key using Argon2id (simulating what the gateway controller does) + salt := make([]byte, 16) + _, err := rand.Read(salt) + if err != nil { + t.Fatalf("Failed to generate salt: %v", err) + } + + hash := argon2.IDKey([]byte(plainAPIKey), salt, 1, 64*1024, 4, 32) + saltEncoded := base64.RawStdEncoding.EncodeToString(salt) + hashEncoded := base64.RawStdEncoding.EncodeToString(hash) + hashedAPIKey := fmt.Sprintf("$argon2id$v=19$m=%d,t=%d,p=%d$%s$%s", + 64*1024, 1, 4, saltEncoded, hashEncoded) + + // Create API key with hashed value + apiKey := &APIKey{ + ID: "test-id-1", + Name: "test-key", + APIKey: hashedAPIKey, // Store hashed key + APIId: "api-123", + Operations: "[\"*\"]", + Status: Active, + CreatedAt: time.Now(), + CreatedBy: "test-user", + UpdatedAt: time.Now(), + ExpiresAt: nil, + } + + // Store the API key + err = store.StoreAPIKey("api-123", apiKey) + if err != nil { + t.Fatalf("Failed to store API key: %v", err) + } + + // Test validation with correct plain text key + valid, err := store.ValidateAPIKey("api-123", "/test", "GET", plainAPIKey) + if err != nil { + t.Fatalf("Validation failed with error: %v", err) + } + if !valid { + t.Error("Validation should succeed with correct plain text API key") + } +} + +func TestAPIKeyHashedValidationFailures(t *testing.T) { + store := NewAPIkeyStore() + + plainAPIKey := "apip_88f8399ef29761f92f4f6d2dbd6dcd78399b3bcb8c53417cb23726e5780ac215" + + // Hash the API key using Argon2id + salt := make([]byte, 16) + _, err := rand.Read(salt) + if err != nil { + t.Fatalf("Failed to generate salt: %v", err) + } + + hash := argon2.IDKey([]byte(plainAPIKey), salt, 1, 64*1024, 4, 32) + saltEncoded := base64.RawStdEncoding.EncodeToString(salt) + hashEncoded := base64.RawStdEncoding.EncodeToString(hash) + hashedAPIKey := fmt.Sprintf("$argon2id$v=19$m=%d,t=%d,p=%d$%s$%s", + 64*1024, 1, 4, saltEncoded, hashEncoded) + + apiKey := &APIKey{ + ID: "test-id-2", + Name: "test-key-2", + APIKey: hashedAPIKey, + APIId: "api-456", + Operations: "[\"*\"]", + Status: Active, + CreatedAt: time.Now(), + CreatedBy: "test-user", + UpdatedAt: time.Now(), + ExpiresAt: nil, + } + + err = store.StoreAPIKey("api-456", apiKey) + if err != nil { + t.Fatalf("Failed to store API key: %v", err) + } + + // Test validation with wrong plain text key + wrongKey := "apip_wrong399ef29761f92f4f6d2dbd6dcd78399b3bcb8c53417cb23726e5780ac999" + valid, err := store.ValidateAPIKey("api-456", "/test", "GET", wrongKey) + if err != nil { + if err != ErrNotFound { + t.Fatalf("Expected ErrNotFound, got: %v", err) + } + } + if valid { + t.Error("Validation should fail with incorrect plain text API key") + } + + // Test validation with non-existent API + valid, err = store.ValidateAPIKey("non-existent-api", "/test", "GET", plainAPIKey) + if err == nil { + t.Error("Expected error for non-existent API") + } + if valid { + t.Error("Validation should fail for non-existent API") + } +} + +func TestAPIKeyHashedRevocation(t *testing.T) { + store := NewAPIkeyStore() + + plainAPIKey := "apip_revoke399ef29761f92f4f6d2dbd6dcd78399b3bcb8c53417cb23726e5780ac215" + + // Hash the API key using Argon2id + salt := make([]byte, 16) + _, err := rand.Read(salt) + if err != nil { + t.Fatalf("Failed to generate salt: %v", err) + } + + hash := argon2.IDKey([]byte(plainAPIKey), salt, 1, 64*1024, 4, 32) + saltEncoded := base64.RawStdEncoding.EncodeToString(salt) + hashEncoded := base64.RawStdEncoding.EncodeToString(hash) + hashedAPIKey := fmt.Sprintf("$argon2id$v=19$m=%d,t=%d,p=%d$%s$%s", + 64*1024, 1, 4, saltEncoded, hashEncoded) + + apiKey := &APIKey{ + ID: "test-id-3", + Name: "revoke-test-key", + APIKey: hashedAPIKey, + APIId: "api-789", + Operations: "[\"*\"]", + Status: Active, + CreatedAt: time.Now(), + CreatedBy: "test-user", + UpdatedAt: time.Now(), + ExpiresAt: nil, + } + + err = store.StoreAPIKey("api-789", apiKey) + if err != nil { + t.Fatalf("Failed to store API key: %v", err) + } + + // Verify key works before revocation + valid, err := store.ValidateAPIKey("api-789", "/test", "GET", plainAPIKey) + if err != nil { + t.Fatalf("Validation failed before revocation: %v", err) + } + if !valid { + t.Error("API key should be valid before revocation") + } + + // Revoke the API key using plain text key + err = store.RevokeAPIKey("api-789", plainAPIKey) + if err != nil { + t.Fatalf("Failed to revoke API key: %v", err) + } + + // Verify key no longer works after revocation + valid, err = store.ValidateAPIKey("api-789", "/test", "GET", plainAPIKey) + if err != nil && err != ErrNotFound { + t.Fatalf("Unexpected error during validation after revocation: %v", err) + } + if valid { + t.Error("API key should be invalid after revocation") + } +} diff --git a/sdk/go.mod b/sdk/go.mod index 077f58ef7..220606ac3 100644 --- a/sdk/go.mod +++ b/sdk/go.mod @@ -8,6 +8,7 @@ require ( github.com/milvus-io/milvus/client/v2 v2.6.1 github.com/milvus-io/milvus/pkg/v2 v2.6.7 github.com/redis/go-redis/v9 v9.8.0 + golang.org/x/crypto v0.46.0 ) require ( @@ -105,7 +106,6 @@ require ( go.uber.org/automaxprocs v1.5.3 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.1 // indirect - golang.org/x/crypto v0.46.0 // indirect golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 // indirect golang.org/x/net v0.48.0 // indirect golang.org/x/sync v0.19.0 // indirect From 35b482f991d2640e95f7187f7fe1303f514f6f30 Mon Sep 17 00:00:00 2001 From: thivindu Date: Thu, 8 Jan 2026 21:11:47 +0530 Subject: [PATCH 2/9] Support sha256, bcrypt for API key hashing --- gateway/configs/config.yaml | 12 + gateway/docker-compose.yaml | 10 +- .../gateway-controller/cmd/controller/main.go | 4 +- .../pkg/api/handlers/handlers.go | 3 +- .../pkg/config/api_key_hashing_test.go | 109 +++ .../gateway-controller/pkg/config/config.go | 67 +- .../pkg/constants/constants.go | 5 + .../pkg/storage/apikey_store.go | 93 ++- .../gateway-controller/pkg/utils/api_key.go | 196 ++++- .../pkg/utils/api_key_hash_test.go | 698 +++++++++++++++++- sdk/gateway/policy/v1alpha/api_key.go | 105 ++- 11 files changed, 1215 insertions(+), 87 deletions(-) create mode 100644 gateway/gateway-controller/pkg/config/api_key_hashing_test.go diff --git a/gateway/configs/config.yaml b/gateway/configs/config.yaml index 1fb4f4dd8..d99757c0e 100644 --- a/gateway/configs/config.yaml +++ b/gateway/configs/config.yaml @@ -217,6 +217,18 @@ gateway_controller: # Expected issuer value in the JWT `iss` claim issuer: "" + # API key hashing configuration + # Used to hash API keys before storing/comparing them + # By default, API keys hashing is enabled with SHA-256 algorithm + # Supported algorithms: "sha256", "bcrypt", "argon2id" + # We recommend using a strong hashing algorithm like argon2id for production deployments + # if `api_key_hashing.enabled` is set to false, API keys will be stored and compared in plain text (not recommended for production) + api_key_hashing: + # Enable or disable API key hashing + enabled: true + # Hashing algorithm: "sha256", "bcrypt", "argon2id" + algorithm: sha256 + # Logging configuration logging: # Log level: "debug", "info", "warn", or "error" diff --git a/gateway/docker-compose.yaml b/gateway/docker-compose.yaml index 2eee17e65..f44ed0ab6 100644 --- a/gateway/docker-compose.yaml +++ b/gateway/docker-compose.yaml @@ -19,7 +19,7 @@ services: gateway-controller: container_name: gateway-controller - image: ghcr.io/wso2/api-platform/gateway-controller:latest + image: ghcr.io/wso2/api-platform/gateway-controller:0.3.0-SNAPSHOT mem_limit: 128m mem_reservation: 128m cpus: 0.1 @@ -38,14 +38,14 @@ services: - ./configs/config.yaml:/app/config/config.yaml:ro - ./gateway-controller/certificates:/app/certificates # Read-write for dynamic certificate management - ./gateway-controller/listener-certs:/app/listener-certs:ro # Read-only for HTTPS listener certificates -# extra_hosts: -# - "host.docker.internal:host-gateway" + extra_hosts: + - "host.docker.internal:host-gateway" networks: - gateway-network policy-engine: container_name: policy-engine - image: ghcr.io/wso2/api-platform/policy-engine:latest + image: ghcr.io/wso2/api-platform/policy-engine:0.3.0-SNAPSHOT mem_limit: 128m mem_reservation: 128m cpus: 0.2 @@ -59,7 +59,7 @@ services: router: container_name: router - image: ghcr.io/wso2/api-platform/gateway-router:latest + image: ghcr.io/wso2/api-platform/gateway-router:0.3.0-SNAPSHOT mem_limit: 256m mem_reservation: 256m cpus: 0.2 diff --git a/gateway/gateway-controller/cmd/controller/main.go b/gateway/gateway-controller/cmd/controller/main.go index 84f9ca684..958d92457 100644 --- a/gateway/gateway-controller/cmd/controller/main.go +++ b/gateway/gateway-controller/cmd/controller/main.go @@ -300,8 +300,8 @@ func main() { router.Use(gin.Recovery()) // Initialize API server with the configured validator and API key manager - apiServer := handlers.NewAPIServer(configStore, db, snapshotManager, policyManager, log, cpClient, - policyDefinitions, templateDefinitions, validator, &cfg.GatewayController.Router, apiKeyXDSManager) + apiServer := handlers.NewAPIServer(configStore, db, snapshotManager, policyManager, log, cpClient, policyDefinitions, + templateDefinitions, validator, &cfg.GatewayController.Router, apiKeyXDSManager, &cfg.GatewayController.APIKeyHashing) // Register API routes (includes certificate management endpoints from OpenAPI spec) api.RegisterHandlers(router, apiServer) diff --git a/gateway/gateway-controller/pkg/api/handlers/handlers.go b/gateway/gateway-controller/pkg/api/handlers/handlers.go index 0634bd4ab..42c180f7a 100644 --- a/gateway/gateway-controller/pkg/api/handlers/handlers.go +++ b/gateway/gateway-controller/pkg/api/handlers/handlers.go @@ -82,6 +82,7 @@ func NewAPIServer( validator config.Validator, routerConfig *config.RouterConfig, apiKeyXDSManager *apikeyxds.APIKeyStateManager, + apiKeyHashingConfig *config.APIKeyHashingConfig, ) *APIServer { deploymentService := utils.NewAPIDeploymentService(store, db, snapshotManager, validator, routerConfig) server := &APIServer{ @@ -97,7 +98,7 @@ func NewAPIServer( mcpDeploymentService: utils.NewMCPDeploymentService(store, db, snapshotManager), llmDeploymentService: utils.NewLLMDeploymentService(store, db, snapshotManager, templateDefinitions, deploymentService, routerConfig), - apiKeyService: utils.NewAPIKeyService(store, db, apiKeyXDSManager), + apiKeyService: utils.NewAPIKeyService(store, db, apiKeyXDSManager, apiKeyHashingConfig), apiKeyXDSManager: apiKeyXDSManager, controlPlaneClient: controlPlaneClient, routerConfig: routerConfig, diff --git a/gateway/gateway-controller/pkg/config/api_key_hashing_test.go b/gateway/gateway-controller/pkg/config/api_key_hashing_test.go new file mode 100644 index 000000000..4692192ec --- /dev/null +++ b/gateway/gateway-controller/pkg/config/api_key_hashing_test.go @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package config + +import ( + "testing" + + "github.com/wso2/api-platform/gateway/gateway-controller/pkg/constants" +) + +func TestValidateAPIKeyHashingConfig(t *testing.T) { + tests := []struct { + name string + enabled bool + algorithm string + expectError bool + }{ + { + name: "hashing disabled", + enabled: false, + algorithm: "", + expectError: false, + }, + { + name: "hashing disabled with algorithm set", + enabled: false, + algorithm: constants.HashingAlgorithmSHA256, + expectError: false, + }, + { + name: "hashing enabled without algorithm - should default to SHA256", + enabled: true, + algorithm: "", + expectError: false, + }, + { + name: "hashing enabled with valid SHA256 algorithm", + enabled: true, + algorithm: constants.HashingAlgorithmSHA256, + expectError: false, + }, + { + name: "hashing enabled with valid bcrypt algorithm", + enabled: true, + algorithm: constants.HashingAlgorithmBcrypt, + expectError: false, + }, + { + name: "hashing enabled with valid Argon2id algorithm", + enabled: true, + algorithm: constants.HashingAlgorithmArgon2ID, + expectError: false, + }, + { + name: "hashing enabled with invalid algorithm", + enabled: true, + algorithm: "invalid-algorithm", + expectError: true, + }, + { + name: "hashing enabled with case-insensitive valid algorithm", + enabled: true, + algorithm: "SHA256", // uppercase + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a minimal config with API key hashing settings + config := &Config{ + GatewayController: GatewayController{ + APIKeyHashing: APIKeyHashingConfig{ + Enabled: tt.enabled, + Algorithm: tt.algorithm, + }, + }, + } + + err := config.validateAPIKeyHashingConfig() + + if tt.expectError { + if err == nil { + t.Errorf("Expected error but got none") + } + } else { + if err != nil { + t.Errorf("Expected no error but got: %v", err) + } + } + }) + } +} diff --git a/gateway/gateway-controller/pkg/config/config.go b/gateway/gateway-controller/pkg/config/config.go index 90fe7a1f8..86b2c3910 100644 --- a/gateway/gateway-controller/pkg/config/config.go +++ b/gateway/gateway-controller/pkg/config/config.go @@ -49,15 +49,16 @@ type AnalyticsConfig struct { // GatewayController holds the main configuration sections for the gateway-controller type GatewayController struct { - Server ServerConfig `koanf:"server"` - Storage StorageConfig `koanf:"storage"` - Router RouterConfig `koanf:"router"` - Logging LoggingConfig `koanf:"logging"` - ControlPlane ControlPlaneConfig `koanf:"controlplane"` - PolicyServer PolicyServerConfig `koanf:"policyserver"` - Policies PoliciesConfig `koanf:"policies"` - LLM LLMConfig `koanf:"llm"` - Auth AuthConfig `koanf:"auth"` + Server ServerConfig `koanf:"server"` + Storage StorageConfig `koanf:"storage"` + Router RouterConfig `koanf:"router"` + Logging LoggingConfig `koanf:"logging"` + ControlPlane ControlPlaneConfig `koanf:"controlplane"` + PolicyServer PolicyServerConfig `koanf:"policyserver"` + Policies PoliciesConfig `koanf:"policies"` + LLM LLMConfig `koanf:"llm"` + Auth AuthConfig `koanf:"auth"` + APIKeyHashing APIKeyHashingConfig `koanf:"api_key_hashing"` } // AuthConfig holds authentication related configuration @@ -294,6 +295,12 @@ type ControlPlaneConfig struct { InsecureSkipVerify bool `koanf:"insecure_skip_verify"` // Skip TLS certificate verification (default: true for dev) } +// APIKeyHashingConfig represents the configuration for API key hashing +type APIKeyHashingConfig struct { + Enabled bool `koanf:"enabled"` // Whether API key hashing is enabled + Algorithm string `koanf:"algorithm"` // Hashing algorithm to use +} + // LoadConfig loads configuration from file, environment variables, and defaults // Priority: Environment variables > Config file > Defaults func LoadConfig(configPath string) (*Config, error) { @@ -486,6 +493,10 @@ func defaultConfig() *Config { PollingInterval: 15 * time.Minute, InsecureSkipVerify: true, }, + APIKeyHashing: APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmSHA256, + }, }, Analytics: AnalyticsConfig{ Enabled: false, @@ -640,6 +651,11 @@ func (c *Config) Validate() error { return err } + // Validate API key hashing configuration + if err := c.validateAPIKeyHashingConfig(); err != nil { + return err + } + return nil } @@ -1086,6 +1102,39 @@ func (c *Config) validateAuthConfig() error { return nil } +// validateControlPlaneConfig validates the control plane configuration +func (c *Config) validateAPIKeyHashingConfig() error { + // If hashing is disabled, skip validation + if !c.GatewayController.APIKeyHashing.Enabled { + return nil + } + + // If hashing is enabled but no algorithm is provided, default to SHA256 + if c.GatewayController.APIKeyHashing.Algorithm == "" { + c.GatewayController.APIKeyHashing.Algorithm = constants.HashingAlgorithmSHA256 + return nil + } + + // If hashing is enabled and algorithm is provided, validate it's one of the supported ones + validAlgorithms := []string{ + constants.HashingAlgorithmSHA256, + constants.HashingAlgorithmBcrypt, + constants.HashingAlgorithmArgon2ID, + } + isValidAlgorithm := false + for _, alg := range validAlgorithms { + if strings.ToLower(c.GatewayController.APIKeyHashing.Algorithm) == alg { + isValidAlgorithm = true + break + } + } + if !isValidAlgorithm { + return fmt.Errorf("api_key_hashing.algorithm must be one of: %s, got: %s", + strings.Join(validAlgorithms, ", "), c.GatewayController.APIKeyHashing.Algorithm) + } + return nil +} + // IsPersistentMode returns true if storage type is not memory func (c *Config) IsPersistentMode() bool { return c.GatewayController.Storage.Type != "memory" diff --git a/gateway/gateway-controller/pkg/constants/constants.go b/gateway/gateway-controller/pkg/constants/constants.go index dc857045c..895e28e10 100644 --- a/gateway/gateway-controller/pkg/constants/constants.go +++ b/gateway/gateway-controller/pkg/constants/constants.go @@ -124,6 +124,11 @@ const ( " - action: SET\n" + " name: '%s'\n" + " value: '%s'\n" + + // HashingAlgorithm constants + HashingAlgorithmSHA256 = "sha256" + HashingAlgorithmBcrypt = "bcrypt" + HashingAlgorithmArgon2ID = "argon2id" ) var WILDCARD_HTTP_METHODS = []string{ diff --git a/gateway/gateway-controller/pkg/storage/apikey_store.go b/gateway/gateway-controller/pkg/storage/apikey_store.go index f6b257b24..ecd66a8c1 100644 --- a/gateway/gateway-controller/pkg/storage/apikey_store.go +++ b/gateway/gateway-controller/pkg/storage/apikey_store.go @@ -19,10 +19,13 @@ package storage import ( + "crypto/sha256" "crypto/subtle" "encoding/base64" + "encoding/hex" "errors" "fmt" + "golang.org/x/crypto/bcrypt" "strings" "sync" "sync/atomic" @@ -118,9 +121,7 @@ func (s *APIKeyStore) Revoke(apiId, plainAPIKeyValue string) bool { // Find the API key by comparing plain text key against stored hashes for _, apiKey := range apiKeys { - // Compare plain text key with stored hashed key using Argon2id - err := compareArgon2id(plainAPIKeyValue, apiKey.APIKey) - if err == nil { + if compareAPIKeys(plainAPIKeyValue, apiKey.APIKey) { // Hash matches - this is our target API key apiKey.Status = models.APIKeyStatusRevoked @@ -219,16 +220,88 @@ func (s *APIKeyStore) removeFromAPIMapping(apiKey *models.APIKey) { } } -// compareArgon2id validates a plain text key against an Argon2id hash -func compareArgon2id(apiKey, hashedAPIKey string) error { - if apiKey == "" { - return fmt.Errorf("plain API key cannot be empty") +// compareAPIKeys compares API keys for external use +// Returns true if the plain API key matches the hash, false otherwise +// If hashing is disabled, performs plain text comparison +func compareAPIKeys(providedAPIKey, storedAPIKey string) bool { + if providedAPIKey == "" || storedAPIKey == "" { + return false + } + + // Check if it's an SHA-256 hash (format: $sha256$$) + if strings.HasPrefix(storedAPIKey, "$sha256$") { + return compareSHA256Hash(providedAPIKey, storedAPIKey) + } + + // Check if it's a bcrypt hash (starts with $2a$, $2b$, or $2y$) + if strings.HasPrefix(storedAPIKey, "$2a$") || + strings.HasPrefix(storedAPIKey, "$2b$") || + strings.HasPrefix(storedAPIKey, "$2y$") { + return compareBcryptHash(providedAPIKey, storedAPIKey) + } + + // Check if it's an Argon2id hash + if strings.HasPrefix(storedAPIKey, "$argon2id$") { + err := compareArgon2id(providedAPIKey, storedAPIKey) + return err == nil + } + + // If no hash format is detected and hashing is enabled, try plain text comparison as fallback + // This handles migration scenarios where some keys might still be stored as plain text + return subtle.ConstantTimeCompare([]byte(providedAPIKey), []byte(storedAPIKey)) == 1 +} + +// compareSHA256Hash validates an encoded SHA-256 hash and compares it to the provided password. +// Expected format: $sha256$$ +// Returns true if the plain API key matches the hash, false otherwise +func compareSHA256Hash(apiKey, encoded string) bool { + if apiKey == "" || encoded == "" { + return false + } + + // Parse the hash format: $sha256$$ + parts := strings.Split(encoded, "$") + if len(parts) != 4 || parts[1] != "sha256" { + return false } - if hashedAPIKey == "" { - return fmt.Errorf("hashed API key cannot be empty") + + // Decode salt and hash from hex + salt, err := hex.DecodeString(parts[2]) + if err != nil { + return false + } + + storedHash, err := hex.DecodeString(parts[3]) + if err != nil { + return false } - parts := strings.Split(hashedAPIKey, "$") + // Compute hash of the provided key with the stored salt + hasher := sha256.New() + hasher.Write([]byte(apiKey)) + hasher.Write(salt) + computedHash := hasher.Sum(nil) + + // Constant-time comparison + return subtle.ConstantTimeCompare(computedHash, storedHash) == 1 +} + +// compareBcryptHash validates an encoded bcrypt hash and compares it to the provided password. +// Returns true if the plain API key matches the hash, false otherwise +func compareBcryptHash(apiKey, encoded string) bool { + if apiKey == "" || encoded == "" { + return false + } + + // Compare the provided key with the stored bcrypt hash + err := bcrypt.CompareHashAndPassword([]byte(encoded), []byte(apiKey)) + return err == nil +} + +// compareArgon2id parses an encoded Argon2id hash and compares it to the provided password. +// Expected format: $argon2id$v=19$m=65536,t=3,p=4$$ +func compareArgon2id(apiKey, encoded string) error { + parts := strings.Split(encoded, "$") if len(parts) != 6 || parts[1] != "argon2id" { return fmt.Errorf("invalid argon2id hash format") } diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index 9b43497d9..372152755 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -20,12 +20,14 @@ package utils import ( "crypto/rand" + "crypto/sha256" "crypto/subtle" "encoding/base64" "encoding/hex" "encoding/json" "errors" "fmt" + "github.com/wso2/api-platform/gateway/gateway-controller/pkg/config" "slices" "strings" "time" @@ -33,10 +35,12 @@ import ( "github.com/google/uuid" commonmodels "github.com/wso2/api-platform/common/models" api "github.com/wso2/api-platform/gateway/gateway-controller/pkg/api/generated" + "github.com/wso2/api-platform/gateway/gateway-controller/pkg/constants" "github.com/wso2/api-platform/gateway/gateway-controller/pkg/models" "github.com/wso2/api-platform/gateway/gateway-controller/pkg/storage" "go.uber.org/zap" "golang.org/x/crypto/argon2" + "golang.org/x/crypto/bcrypt" ) // APIKeyGenerationParams contains parameters for API key generation operations @@ -97,13 +101,6 @@ type ListAPIKeyResult struct { Response api.APIKeyListResponse // Response following the generated schema } -// APIKeyService provides utilities for API configuration deployment -type APIKeyService struct { - store *storage.ConfigStore - db storage.Storage - xdsManager XDSManager -} - // XDSManager interface for API key operations type XDSManager interface { StoreAPIKey(apiId, apiName, apiVersion string, apiKey *models.APIKey, correlationID string) error @@ -111,23 +108,40 @@ type XDSManager interface { RemoveAPIKeysByAPI(apiId, apiName, apiVersion, correlationID string) error } +// APIKeyService provides utilities for API configuration deployment +type APIKeyService struct { + store *storage.ConfigStore + db storage.Storage + xdsManager XDSManager + hashingConfig *config.APIKeyHashingConfig // Configuration for API key hashing +} + // NewAPIKeyService creates a new API key generation service -func NewAPIKeyService(store *storage.ConfigStore, db storage.Storage, xdsManager XDSManager) *APIKeyService { +func NewAPIKeyService(store *storage.ConfigStore, db storage.Storage, xdsManager XDSManager, + hashingConfig *config.APIKeyHashingConfig) *APIKeyService { return &APIKeyService{ - store: store, - db: db, - xdsManager: xdsManager, + store: store, + db: db, + xdsManager: xdsManager, + hashingConfig: hashingConfig, } } const ( APIKeyPrefix = "apip_" + // Argon2id parameters (recommended for production security) argon2Time = 1 // Number of iterations argon2Memory = 64 * 1024 // Memory usage in KiB (64 MiB) argon2Threads = 4 // Number of threads argon2KeyLen = 32 // Length of derived key in bytes argon2SaltLen = 16 // Length of salt in bytes + + // bcrypt parameters (alternative hashing method) + bcryptCost = 12 // Cost parameter for bcrypt (recommended: 10-15) + + // SHA-256 parameters + sha256SaltLen = 32 // Length of salt in bytes for SHA-256 ) // GenerateAPIKey handles the complete API key generation process @@ -301,7 +315,7 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo // Find the API key that matches the provided plain text key if apiKeys != nil { for _, key := range apiKeys { - if s.ValidateAPIKey(providedAPIKeyValue, key.APIKey) { + if s.compareAPIKeys(providedAPIKeyValue, key.APIKey) { matchedKey = key apiKey = key break @@ -1093,13 +1107,85 @@ func (s *APIKeyService) isDeveloper(user *commonmodels.AuthContext) bool { return slices.Contains(user.Roles, "developer") } -// hashAPIKey securely hashes an API key using Argon2id +// hashAPIKey securely hashes an API key using the configured algorithm // Returns the hashed API key that should be stored in database and policy engine +// If hashing is disabled, returns the plain API key func (s *APIKeyService) hashAPIKey(plainAPIKey string) (string, error) { if plainAPIKey == "" { return "", fmt.Errorf("API key cannot be empty") } + // If hashing is disabled, return the plain API key + if !s.hashingConfig.Enabled { + return plainAPIKey, nil + } + + // Hash based on configured algorithm + switch s.hashingConfig.Algorithm { + case constants.HashingAlgorithmSHA256: + return s.hashAPIKeyWithSHA256(plainAPIKey) + case constants.HashingAlgorithmBcrypt: + return s.hashAPIKeyWithBcrypt(plainAPIKey) + case constants.HashingAlgorithmArgon2ID: + return s.hashAPIKeyWithArgon2ID(plainAPIKey) + default: + // Default to SHA256 if algorithm is not recognized + return s.hashAPIKeyWithSHA256(plainAPIKey) + } +} + +// hashAPIKeyWithSHA256 securely hashes an API key using SHA-256 with salt +// Returns the hashed API key that should be st +// Generate random saltored in database and policy engine +func (s *APIKeyService) hashAPIKeyWithSHA256(plainAPIKey string) (string, error) { + if plainAPIKey == "" { + return "", fmt.Errorf("API key cannot be empty") + } + + salt := make([]byte, sha256SaltLen) + if _, err := rand.Read(salt); err != nil { + return "", fmt.Errorf("failed to generate salt: %w", err) + } + + // Generate hash using SHA-256 + hasher := sha256.New() + hasher.Write([]byte(plainAPIKey)) + hasher.Write(salt) + hash := hasher.Sum(nil) + + // Encode salt and hash using hex + saltHex := hex.EncodeToString(salt) + hashHex := hex.EncodeToString(hash) + + // Format: $sha256$$ + hashedKey := fmt.Sprintf("$sha256$%s$%s", saltHex, hashHex) + + return hashedKey, nil +} + +// hashAPIKeyWithBcrypt securely hashes an API key using bcrypt +// Returns the hashed API key that should be stored in database and policy engine +func (s *APIKeyService) hashAPIKeyWithBcrypt(plainAPIKey string) (string, error) { + if plainAPIKey == "" { + return "", fmt.Errorf("API key cannot be empty") + } + + // Generate bcrypt hash with specified cost + hashedKey, err := bcrypt.GenerateFromPassword([]byte(plainAPIKey), bcryptCost) + if err != nil { + return "", fmt.Errorf("failed to hash API key with bcrypt: %w", err) + } + + return string(hashedKey), nil +} + +// hashAPIKeyWithArgon2ID securely hashes an API key using Argon2id +// Returns the hashed API key that should be stored in database and policy engine +func (s *APIKeyService) hashAPIKeyWithArgon2ID(plainAPIKey string) (string, error) { + if plainAPIKey == "" { + return "", fmt.Errorf("API key cannot be empty") + } + // Generate random salt salt := make([]byte, argon2SaltLen) if _, err := rand.Read(salt); err != nil { @@ -1120,20 +1206,82 @@ func (s *APIKeyService) hashAPIKey(plainAPIKey string) (string, error) { return hashedKey, nil } -// ValidateAPIKey is a public method to validate API keys for external use +// compareAPIKeys compares API keys for external use // Returns true if the plain API key matches the hash, false otherwise -func (s *APIKeyService) ValidateAPIKey(providedAPIKey, storedAPIKey string) bool { - if providedAPIKey == "" { +// If hashing is disabled, performs plain text comparison +func (s *APIKeyService) compareAPIKeys(providedAPIKey, storedAPIKey string) bool { + if providedAPIKey == "" || storedAPIKey == "" { return false } - if storedAPIKey == "" { - return false + + // Check if it's an SHA-256 hash (format: $sha256$$) + if strings.HasPrefix(storedAPIKey, "$sha256$") { + return s.compareSHA256Hash(providedAPIKey, storedAPIKey) } + + // Check if it's a bcrypt hash (starts with $2a$, $2b$, or $2y$) + if strings.HasPrefix(storedAPIKey, "$2a$") || + strings.HasPrefix(storedAPIKey, "$2b$") || + strings.HasPrefix(storedAPIKey, "$2y$") { + return s.compareBcryptHash(providedAPIKey, storedAPIKey) + } + + // Check if it's an Argon2id hash if strings.HasPrefix(storedAPIKey, "$argon2id$") { err := s.compareArgon2id(providedAPIKey, storedAPIKey) return err == nil } - return false + + // If no hash format is detected and hashing is enabled, try plain text comparison as fallback + // This handles migration scenarios where some keys might still be stored as plain text + return subtle.ConstantTimeCompare([]byte(providedAPIKey), []byte(storedAPIKey)) == 1 +} + +// compareSHA256Hash validates an encoded SHA-256 hash and compares it to the provided password. +// Expected format: $sha256$$ +// Returns true if the plain API key matches the hash, false otherwise +func (s *APIKeyService) compareSHA256Hash(apiKey, encoded string) bool { + if apiKey == "" || encoded == "" { + return false + } + + // Parse the hash format: $sha256$$ + parts := strings.Split(encoded, "$") + if len(parts) != 4 || parts[1] != "sha256" { + return false + } + + // Decode salt and hash from hex + salt, err := hex.DecodeString(parts[2]) + if err != nil { + return false + } + + storedHash, err := hex.DecodeString(parts[3]) + if err != nil { + return false + } + + // Compute hash of the provided key with the stored salt + hasher := sha256.New() + hasher.Write([]byte(apiKey)) + hasher.Write(salt) + computedHash := hasher.Sum(nil) + + // Constant-time comparison + return subtle.ConstantTimeCompare(computedHash, storedHash) == 1 +} + +// compareBcryptHash validates an encoded bcrypt hash and compares it to the provided password. +// Returns true if the plain API key matches the hash, false otherwise +func (s *APIKeyService) compareBcryptHash(apiKey, encoded string) bool { + if apiKey == "" || encoded == "" { + return false + } + + // Compare the provided key with the stored bcrypt hash + err := bcrypt.CompareHashAndPassword([]byte(encoded), []byte(apiKey)) + return err == nil } // compareArgon2id parses an encoded Argon2id hash and compares it to the provided password. @@ -1191,3 +1339,13 @@ func decodeBase64(s string) ([]byte, error) { // try StdEncoding as a fallback return base64.StdEncoding.DecodeString(s) } + +// SetHashingConfig allows updating the hashing configuration at runtime +func (s *APIKeyService) SetHashingConfig(config *config.APIKeyHashingConfig) { + s.hashingConfig = config +} + +// GetHashingConfig returns the current hashing configuration +func (s *APIKeyService) GetHashingConfig() *config.APIKeyHashingConfig { + return s.hashingConfig +} diff --git a/gateway/gateway-controller/pkg/utils/api_key_hash_test.go b/gateway/gateway-controller/pkg/utils/api_key_hash_test.go index fcbe87293..cebaf0ca3 100644 --- a/gateway/gateway-controller/pkg/utils/api_key_hash_test.go +++ b/gateway/gateway-controller/pkg/utils/api_key_hash_test.go @@ -19,12 +19,20 @@ package utils import ( + "github.com/wso2/api-platform/gateway/gateway-controller/pkg/config" + "github.com/wso2/api-platform/gateway/gateway-controller/pkg/constants" "strings" "testing" ) -func TestAPIKeyHashing(t *testing.T) { - service := &APIKeyService{} +func TestArgon2IDAPIKeyHashing(t *testing.T) { + // Create service with Argon2id hashing configuration + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmArgon2ID, + }, + } // Test API key generation and hashing plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" @@ -46,14 +54,14 @@ func TestAPIKeyHashing(t *testing.T) { } // Test validation with correct key - valid := service.ValidateAPIKey(plainKey, hashedKey) + valid := service.compareAPIKeys(plainKey, hashedKey) if !valid { t.Error("Validation should succeed with correct plain key") } // Test validation with incorrect key wrongKey := "apip_wrong123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - valid = service.ValidateAPIKey(wrongKey, hashedKey) + valid = service.compareAPIKeys(wrongKey, hashedKey) if valid { t.Error("Validation should fail with incorrect plain key") } @@ -64,59 +72,703 @@ func TestAPIKeyHashing(t *testing.T) { t.Error("Hashing empty key should return error") } - valid = service.ValidateAPIKey("", hashedKey) + valid = service.compareAPIKeys("", hashedKey) if valid { t.Error("Validation should fail with empty plain key") } - valid = service.ValidateAPIKey(plainKey, "") + valid = service.compareAPIKeys(plainKey, "") if valid { t.Error("Validation should fail with empty hash") } } -func TestAPIKeyHashParameters(t *testing.T) { - service := &APIKeyService{} +func TestArgon2IDAPIKeyHashDeterminism(t *testing.T) { + // Create service with Argon2id hashing configuration + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmArgon2ID, + }, + } plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + // Generate two hashes of the same key + hash1, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key (1): %v", err) + } + + hash2, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key (2): %v", err) + } + + // Hashes should be different due to random salt + if hash1 == hash2 { + t.Error("Two hashes of the same key should be different (Argon2id uses random salt)") + } + + // But both should validate against the same plain key + if !service.compareAPIKeys(plainKey, hash1) { + t.Error("First hash should validate correctly") + } + + if !service.compareAPIKeys(plainKey, hash2) { + t.Error("Second hash should validate correctly") + } +} + +func TestBcryptAPIKeyHashing(t *testing.T) { + // Create service with bcrypt hashing configuration + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmBcrypt, + }, + } + + // Test API key generation and hashing with bcrypt + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Test bcrypt hashing + hashedKey, err := service.hashAPIKeyWithBcrypt(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key with bcrypt: %v", err) + } + + // Verify the hashed key is different from plain key + if hashedKey == plainKey { + t.Error("Bcrypt hashed key should be different from plain key") + } + + // Verify the hash starts with bcrypt prefix ($2a$, $2b$, or $2y$) + if !strings.HasPrefix(hashedKey, "$2a$") && + !strings.HasPrefix(hashedKey, "$2b$") && + !strings.HasPrefix(hashedKey, "$2y$") { + t.Errorf("Bcrypt hashed key should start with $2a$, $2b$, or $2y$ prefix, got: %s", hashedKey) + } + + // Test validation with correct key + valid := service.compareAPIKeys(plainKey, hashedKey) + if !valid { + t.Error("Bcrypt validation should succeed with correct plain key") + } + + // Test validation with incorrect key + wrongKey := "apip_wrong123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + valid = service.compareAPIKeys(wrongKey, hashedKey) + if valid { + t.Error("Bcrypt validation should fail with incorrect plain key") + } + + // Test empty keys + _, err = service.hashAPIKeyWithBcrypt("") + if err == nil { + t.Error("Bcrypt hashing empty key should return error") + } + + valid = service.compareAPIKeys("", hashedKey) + if valid { + t.Error("Bcrypt validation should fail with empty plain key") + } + + valid = service.compareAPIKeys(plainKey, "") + if valid { + t.Error("Bcrypt validation should fail with empty hash") + } +} + +func TestBcryptAPIKeyHashDeterminism(t *testing.T) { + // Create service with bcrypt hashing configuration + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmBcrypt, + }, + } + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Generate two bcrypt hashes of the same key + hash1, err := service.hashAPIKeyWithBcrypt(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key with bcrypt (1): %v", err) + } + + hash2, err := service.hashAPIKeyWithBcrypt(plainKey) + if err != nil { + t.Fatalf("Failed to hash API key with bcrypt (2): %v", err) + } + + // Hashes should be different due to random salt + if hash1 == hash2 { + t.Error("Two bcrypt hashes of the same key should be different (bcrypt uses random salt)") + } + + // But both should validate against the same plain key + if !service.compareAPIKeys(plainKey, hash1) { + t.Error("First bcrypt hash should validate correctly") + } + + if !service.compareAPIKeys(plainKey, hash2) { + t.Error("Second bcrypt hash should validate correctly") + } +} + +func TestSHA256APIKeyHashing(t *testing.T) { + // Create service with SHA256 hashing configuration + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmSHA256, + }, + } + + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Test SHA256 hashing hashedKey, err := service.hashAPIKey(plainKey) if err != nil { - t.Fatalf("Failed to hash API key: %v", err) + t.Fatalf("Failed to hash API key with SHA256: %v", err) + } + + // Verify the hashed key is different from plain key + if hashedKey == plainKey { + t.Error("SHA256 hashed key should be different from plain key") + } + + // Verify the hash starts with SHA256 prefix + if !strings.HasPrefix(hashedKey, "$sha256$") { + t.Error("SHA256 hashed key should start with $sha256$ prefix") } - // Verify the format includes the expected Argon2id parameters - expectedParams := "m=65536,t=1,p=4" - if !strings.Contains(hashedKey, expectedParams) { - t.Errorf("Hash should contain parameters %s, got: %s", expectedParams, hashedKey) + // Test validation with correct key + valid := service.compareAPIKeys(plainKey, hashedKey) + if !valid { + t.Error("SHA256 validation should succeed with correct plain key") + } + + // Test validation with incorrect key + wrongKey := "apip_wrong123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + valid = service.compareAPIKeys(wrongKey, hashedKey) + if valid { + t.Error("SHA256 validation should fail with incorrect plain key") } } -func TestAPIKeyHashDeterminism(t *testing.T) { - service := &APIKeyService{} +func TestSHA256APIKeyHashDeterminism(t *testing.T) { + // Create service with SHA256 hashing configuration + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmSHA256, + }, + } plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" - // Generate two hashes of the same key + // Generate two SHA256 hashes of the same key hash1, err := service.hashAPIKey(plainKey) if err != nil { - t.Fatalf("Failed to hash API key (1): %v", err) + t.Fatalf("Failed to hash API key with SHA256 (1): %v", err) } hash2, err := service.hashAPIKey(plainKey) if err != nil { - t.Fatalf("Failed to hash API key (2): %v", err) + t.Fatalf("Failed to hash API key with SHA256 (2): %v", err) } // Hashes should be different due to random salt if hash1 == hash2 { - t.Error("Two hashes of the same key should be different (Argon2id uses random salt)") + t.Error("Two SHA256 hashes of the same key should be different (SHA256 uses random salt)") } // But both should validate against the same plain key - if !service.ValidateAPIKey(plainKey, hash1) { - t.Error("First hash should validate correctly") + if !service.compareAPIKeys(plainKey, hash1) { + t.Error("First SHA256 hash should validate correctly") } - if !service.ValidateAPIKey(plainKey, hash2) { - t.Error("Second hash should validate correctly") + if !service.compareAPIKeys(plainKey, hash2) { + t.Error("Second SHA256 hash should validate correctly") + } +} + +func TestAPIKeyHashingDisabled(t *testing.T) { + // Create service with hashing disabled + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: false, + }, + } + + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Test hashing when disabled - should return plain key + result, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to process API key with hashing disabled: %v", err) + } + + // When hashing is disabled, should return the same plain key + if result != plainKey { + t.Error("When hashing is disabled, should return the original plain key") + } + + // Test validation with hashing disabled - should do plain text comparison + valid := service.compareAPIKeys(plainKey, result) + if !valid { + t.Error("Validation should succeed with plain text comparison when hashing is disabled") + } + + // Test validation with wrong key + wrongKey := "apip_wrong123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + valid = service.compareAPIKeys(wrongKey, result) + if valid { + t.Error("Validation should fail with wrong key when hashing is disabled") + } + + // Test empty keys + _, err = service.hashAPIKey("") + if err == nil { + t.Error("Hashing empty key should return error even when hashing is disabled") + } + + valid = service.compareAPIKeys("", result) + if valid { + t.Error("Validation should fail with empty plain key") + } + + valid = service.compareAPIKeys(plainKey, "") + if valid { + t.Error("Validation should fail with empty stored key") + } +} + +func TestAPIKeyHashingDisabledDeterminism(t *testing.T) { + // Create service with hashing disabled + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: false, + }, + } + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Generate "hashes" (should be plain keys) multiple times + result1, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to process API key with hashing disabled (1): %v", err) + } + + result2, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to process API key with hashing disabled (2): %v", err) + } + + // Results should be identical when hashing is disabled + if result1 != result2 { + t.Error("When hashing is disabled, multiple calls should return identical results") + } + + // Both should be equal to the original plain key + if result1 != plainKey || result2 != plainKey { + t.Error("When hashing is disabled, results should equal the original plain key") + } + + // Both should validate correctly + if !service.compareAPIKeys(plainKey, result1) { + t.Error("First result should validate correctly when hashing is disabled") + } + + if !service.compareAPIKeys(plainKey, result2) { + t.Error("Second result should validate correctly when hashing is disabled") + } +} + +func TestHashingConfigurationSwitching(t *testing.T) { + service := &APIKeyService{} + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Test with hashing disabled + service.SetHashingConfig(&config.APIKeyHashingConfig{Enabled: false}) + plainResult, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to process plain API key: %v", err) + } + if plainResult != plainKey { + t.Error("When hashing is disabled, should return plain key") + } + + // Test switching to Argon2id + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmArgon2ID, + }) + argon2Result, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash with Argon2id: %v", err) + } + if !strings.HasPrefix(argon2Result, "$argon2id$") { + t.Error("Argon2id hash should start with $argon2id$ prefix") + } + + // Test switching to bcrypt + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmBcrypt, + }) + bcryptResult, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash with bcrypt: %v", err) + } + if !strings.HasPrefix(bcryptResult, "$2") { + t.Error("bcrypt hash should start with $2 prefix") + } + + // Test switching to SHA256 + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmSHA256, + }) + sha256Result, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Failed to hash with SHA256: %v", err) + } + if !strings.HasPrefix(sha256Result, "$sha256$") { + t.Error("SHA256 hash should start with $sha256$ prefix") + } + + // Validate that all different hashes work with the same plain key + service.SetHashingConfig(&config.APIKeyHashingConfig{Enabled: true}) // Reset for validation + if !service.compareAPIKeys(plainKey, argon2Result) { + t.Error("Argon2id hash should validate correctly") + } + if !service.compareAPIKeys(plainKey, bcryptResult) { + t.Error("bcrypt hash should validate correctly") + } + if !service.compareAPIKeys(plainKey, sha256Result) { + t.Error("SHA256 hash should validate correctly") + } +} + +func TestAPIKeyHashingDisabledMigrationScenario(t *testing.T) { + // Test migration scenario where some keys are hashed and some are plain text + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: false, + }, + } + + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Simulate having a pre-existing hashed key (Argon2id format) + hashedKey := "$argon2id$v=19$m=65536,t=1,p=4$c2FsdA$aGFzaA" // Example format + + // With hashing disabled, validation should still handle hashed keys + // But it should fall back to plain text comparison for the provided case + valid := service.compareAPIKeys(plainKey, hashedKey) + if valid { + t.Error("Plain key should not validate against a different hash when hashing is disabled") + } + + // Plain text key should validate against itself + valid = service.compareAPIKeys(plainKey, plainKey) + if !valid { + t.Error("Plain key should validate against itself when hashing is disabled") + } + + // Test with bcrypt format hash + bcryptHash := "$2a$12$example" // Example format + valid = service.compareAPIKeys(plainKey, bcryptHash) + if valid { + t.Error("Plain key should not validate against bcrypt hash when hashing is disabled") + } + + // Test with SHA256 format hash + sha256Hash := "$sha256$73616c74$68617368" // Example format + valid = service.compareAPIKeys(plainKey, sha256Hash) + if valid { + t.Error("Plain key should not validate against SHA256 hash when hashing is disabled") + } +} + +func TestMixedAPIKeyFormatsValidation(t *testing.T) { + // Test scenario where we have keys in different formats: + // - Plain text keys (legacy) + // - SHA256 hashed keys + // - bcrypt hashed keys + // - Argon2id hashed keys + + plainKey1 := "apip_plain123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + plainKey2 := "apip_test456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01" + plainKey3 := "apip_demo789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123" + plainKey4 := "apip_sample9abcdef0123456789abcdef0123456789abcdef0123456789abcdef012345" + + // Generate hashes using different algorithms + + // 1. Plain text key (simulate legacy storage) + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: false, + }, + } + plainHashed, err := service.hashAPIKey(plainKey1) + if err != nil { + t.Fatalf("Failed to plain text hashing: %v", err) + } + + // 2. SHA256 hashed key + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmSHA256, + }) + sha256Hashed, err := service.hashAPIKey(plainKey2) + if err != nil { + t.Fatalf("Failed to hash key with SHA256: %v", err) + } + + // 3. bcrypt hashed key + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmBcrypt, + }) + bcryptHashed, err := service.hashAPIKey(plainKey3) + if err != nil { + t.Fatalf("Failed to hash key with bcrypt: %v", err) + } + + // 4. Argon2id hashed key + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmArgon2ID, + }) + argon2idHashed, err := service.hashAPIKey(plainKey4) + if err != nil { + t.Fatalf("Failed to hash key with Argon2id: %v", err) + } + + // Reset service to simulate runtime validation (algorithm agnostic) + service.SetHashingConfig(&config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmSHA256, // Current default + }) + + // Test validation of each key format + + // 1. Validate plain text key (should work via fallback) + valid := service.compareAPIKeys(plainKey1, plainHashed) + if !valid { + t.Error("Plain text key should validate against plain text stored value") + } + + // 2. Validate SHA256 hashed key + valid = service.compareAPIKeys(plainKey2, sha256Hashed) + if !valid { + t.Error("Plain key should validate against SHA256 hash") + } + + // Verify SHA256 format + if !strings.HasPrefix(sha256Hashed, "$sha256$") { + t.Error("SHA256 hash should start with $sha256$ prefix") + } + + // 3. Validate bcrypt hashed key + valid = service.compareAPIKeys(plainKey3, bcryptHashed) + if !valid { + t.Error("Plain key should validate against bcrypt hash") + } + + // Verify bcrypt format + if !strings.HasPrefix(bcryptHashed, "$2") { + t.Error("bcrypt hash should start with $2 prefix") + } + + // 4. Validate Argon2id hashed key + valid = service.compareAPIKeys(plainKey4, argon2idHashed) + if !valid { + t.Error("Plain key should validate against Argon2id hash") + } + + // Verify Argon2id format + if !strings.HasPrefix(argon2idHashed, "$argon2id$") { + t.Error("Argon2id hash should start with $argon2id$ prefix") + } + + // Test cross-validation (should fail) + + // Plain key 1 should not validate against other hashes + valid = service.compareAPIKeys(plainKey1, sha256Hashed) + if valid { + t.Error("Wrong plain key should not validate against SHA256 hash") + } + + valid = service.compareAPIKeys(plainKey1, bcryptHashed) + if valid { + t.Error("Wrong plain key should not validate against bcrypt hash") + } + + valid = service.compareAPIKeys(plainKey1, argon2idHashed) + if valid { + t.Error("Wrong plain key should not validate against Argon2id hash") + } + + // Plain key 2 should not validate against other hashes + valid = service.compareAPIKeys(plainKey2, plainHashed) + if valid { + t.Error("Wrong plain key should not validate against plain text stored value") + } + + valid = service.compareAPIKeys(plainKey2, bcryptHashed) + if valid { + t.Error("Wrong plain key should not validate against bcrypt hash") + } + + valid = service.compareAPIKeys(plainKey2, argon2idHashed) + if valid { + t.Error("Wrong plain key should not validate against Argon2id hash") + } + + // Plain key 3 should not validate against other hashes + valid = service.compareAPIKeys(plainKey3, plainHashed) + if valid { + t.Error("Wrong plain key should not validate against plain text stored value") + } + + valid = service.compareAPIKeys(plainKey3, sha256Hashed) + if valid { + t.Error("Wrong plain key should not validate against sha256 hash") + } + + valid = service.compareAPIKeys(plainKey3, argon2idHashed) + if valid { + t.Error("Wrong plain key should not validate against Argon2id hash") + } + + // Plain key 4 should not validate against other hashes + valid = service.compareAPIKeys(plainKey4, plainHashed) + if valid { + t.Error("Wrong plain key should not validate against plain text stored value") + } + + valid = service.compareAPIKeys(plainKey4, sha256Hashed) + if valid { + t.Error("Wrong plain key should not validate against sha256 hash") + } + + valid = service.compareAPIKeys(plainKey4, bcryptHashed) + if valid { + t.Error("Wrong plain key should not validate against bcrypt hash") + } + + // Test with completely wrong keys + wrongKey := "apip_wrong56789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234" + + valid = service.compareAPIKeys(wrongKey, plainHashed) + if valid { + t.Error("Wrong key should not validate against plain text") + } + + valid = service.compareAPIKeys(wrongKey, sha256Hashed) + if valid { + t.Error("Wrong key should not validate against SHA256 hash") + } + + valid = service.compareAPIKeys(wrongKey, bcryptHashed) + if valid { + t.Error("Wrong key should not validate against bcrypt hash") + } + + valid = service.compareAPIKeys(wrongKey, argon2idHashed) + if valid { + t.Error("Wrong key should not validate against Argon2id hash") + } + + // Test empty key scenarios + valid = service.compareAPIKeys("", sha256Hashed) + if valid { + t.Error("Empty key should not validate") + } + + valid = service.compareAPIKeys(plainKey1, "") + if valid { + t.Error("Key should not validate against empty hash") + } +} + +func TestMixedAPIKeyFormatsValidationWithHashingDisabled(t *testing.T) { + // Test mixed formats when hashing is disabled + service := &APIKeyService{ + hashingConfig: &config.APIKeyHashingConfig{ + Enabled: false, + }, + } + + plainKey := "apip_test123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + + // Simulate pre-existing hashed keys from when hashing was enabled + sha256Hash := "$sha256$73616c74$abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890" + bcryptHash := "$2a$12$abcdefghijklmnopqrstuv.abcdefghijklmnopqrstuv.abcdefghijklmnopqr" + argon2idHash := "$argon2id$v=19$m=65536,t=1,p=4$c2FsdA$aGFzaEhhc2hIYXNoSGFzaEhhc2hIYXNoSGFzaEhhc2g" + + // With hashing disabled, only plain text comparison should work + valid := service.compareAPIKeys(plainKey, plainKey) + if !valid { + t.Error("Plain key should validate against itself when hashing is disabled") + } + + // Hashed keys should not validate when hashing is disabled + valid = service.compareAPIKeys(plainKey, sha256Hash) + if valid { + t.Error("Plain key should not validate against SHA256 hash when hashing is disabled") + } + + valid = service.compareAPIKeys(plainKey, bcryptHash) + if valid { + t.Error("Plain key should not validate against bcrypt hash when hashing is disabled") + } + + valid = service.compareAPIKeys(plainKey, argon2idHash) + if valid { + t.Error("Plain key should not validate against Argon2id hash when hashing is disabled") + } + + // Test that we can still process plain text keys + result, err := service.hashAPIKey(plainKey) + if err != nil { + t.Fatalf("Should be able to process plain key when hashing is disabled: %v", err) + } + + if result != plainKey { + t.Error("With hashing disabled, should return the original plain key") + } +} + +func TestHashingConfigurationGetSet(t *testing.T) { + // Initialize service with a default configuration + defaultHashingConfig := &config.APIKeyHashingConfig{ + Enabled: false, + Algorithm: "", + } + service := &APIKeyService{ + hashingConfig: defaultHashingConfig, + } + + // Test default configuration + defaultConfig := service.GetHashingConfig() + if defaultConfig.Enabled != false { + t.Error("Default hashing config should be disabled") + } + + // Test setting configuration + newConfig := config.APIKeyHashingConfig{ + Enabled: true, + Algorithm: constants.HashingAlgorithmArgon2ID, + } + service.SetHashingConfig(&newConfig) + + retrievedConfig := service.GetHashingConfig() + if retrievedConfig.Enabled != newConfig.Enabled { + t.Error("Hashing config enabled flag should match") + } + if retrievedConfig.Algorithm != newConfig.Algorithm { + t.Error("Hashing config algorithm should match") } } diff --git a/sdk/gateway/policy/v1alpha/api_key.go b/sdk/gateway/policy/v1alpha/api_key.go index 59c390bd0..bd7ae692d 100644 --- a/sdk/gateway/policy/v1alpha/api_key.go +++ b/sdk/gateway/policy/v1alpha/api_key.go @@ -1,11 +1,14 @@ package policyv1alpha import ( + "crypto/sha256" "crypto/subtle" "encoding/base64" + "encoding/hex" "encoding/json" "errors" "fmt" + "golang.org/x/crypto/bcrypt" "strings" "sync" "time" @@ -136,14 +139,9 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, pro var targetAPIKey *APIKey for _, ak := range apiKeys { - // Compare provided plain text key with stored hashed key using Argon2id - if strings.HasPrefix(ak.APIKey, "$argon2id$") { - err := compareArgon2id(providedAPIKey, ak.APIKey) - if err == nil { - // Hash matches - this is our target API key - targetAPIKey = ak - break - } + if compareAPIKeys(providedAPIKey, ak.APIKey) { + targetAPIKey = ak + break } } @@ -207,9 +205,8 @@ func (aks *APIkeyStore) RevokeAPIKey(apiId, plainAPIKeyValue string) error { var targetIndex = -1 for i, apiKey := range apiKeys { - // Compare plain text key with stored hashed key using Argon2id - err := compareArgon2id(plainAPIKeyValue, apiKey.APIKey) - if err == nil { + // Compare plain text key with stored hashed key + if compareAPIKeys(plainAPIKeyValue, apiKey.APIKey) { targetAPIKey = apiKey targetIndex = i break @@ -262,16 +259,88 @@ func (aks *APIkeyStore) ClearAll() error { return nil } -// compareArgon2id validates a plain text key against an Argon2id hash -func compareArgon2id(apiKey, hashedAPIKey string) error { - if apiKey == "" { - return fmt.Errorf("plain API key cannot be empty") +// compareAPIKeys compares API keys for external use +// Returns true if the plain API key matches the hash, false otherwise +// If hashing is disabled, performs plain text comparison +func compareAPIKeys(providedAPIKey, storedAPIKey string) bool { + if providedAPIKey == "" || storedAPIKey == "" { + return false + } + + // Check if it's an SHA-256 hash (format: $sha256$$) + if strings.HasPrefix(storedAPIKey, "$sha256$") { + return compareSHA256Hash(providedAPIKey, storedAPIKey) + } + + // Check if it's a bcrypt hash (starts with $2a$, $2b$, or $2y$) + if strings.HasPrefix(storedAPIKey, "$2a$") || + strings.HasPrefix(storedAPIKey, "$2b$") || + strings.HasPrefix(storedAPIKey, "$2y$") { + return compareBcryptHash(providedAPIKey, storedAPIKey) + } + + // Check if it's an Argon2id hash + if strings.HasPrefix(storedAPIKey, "$argon2id$") { + err := compareArgon2id(providedAPIKey, storedAPIKey) + return err == nil + } + + // If no hash format is detected and hashing is enabled, try plain text comparison as fallback + // This handles migration scenarios where some keys might still be stored as plain text + return subtle.ConstantTimeCompare([]byte(providedAPIKey), []byte(storedAPIKey)) == 1 +} + +// compareSHA256Hash validates an encoded SHA-256 hash and compares it to the provided password. +// Expected format: $sha256$$ +// Returns true if the plain API key matches the hash, false otherwise +func compareSHA256Hash(apiKey, encoded string) bool { + if apiKey == "" || encoded == "" { + return false } - if hashedAPIKey == "" { - return fmt.Errorf("hashed API key cannot be empty") + + // Parse the hash format: $sha256$$ + parts := strings.Split(encoded, "$") + if len(parts) != 4 || parts[1] != "sha256" { + return false } - parts := strings.Split(hashedAPIKey, "$") + // Decode salt and hash from hex + salt, err := hex.DecodeString(parts[2]) + if err != nil { + return false + } + + storedHash, err := hex.DecodeString(parts[3]) + if err != nil { + return false + } + + // Compute hash of the provided key with the stored salt + hasher := sha256.New() + hasher.Write([]byte(apiKey)) + hasher.Write(salt) + computedHash := hasher.Sum(nil) + + // Constant-time comparison + return subtle.ConstantTimeCompare(computedHash, storedHash) == 1 +} + +// compareBcryptHash validates an encoded bcrypt hash and compares it to the provided password. +// Returns true if the plain API key matches the hash, false otherwise +func compareBcryptHash(apiKey, encoded string) bool { + if apiKey == "" || encoded == "" { + return false + } + + // Compare the provided key with the stored bcrypt hash + err := bcrypt.CompareHashAndPassword([]byte(encoded), []byte(apiKey)) + return err == nil +} + +// compareArgon2id parses an encoded Argon2id hash and compares it to the provided password. +// Expected format: $argon2id$v=19$m=65536,t=3,p=4$$ +func compareArgon2id(apiKey, encoded string) error { + parts := strings.Split(encoded, "$") if len(parts) != 6 || parts[1] != "argon2id" { return fmt.Errorf("invalid argon2id hash format") } From 646f6975047f934c39a9fc397dd47ffbe46dafa9 Mon Sep 17 00:00:00 2001 From: thivindu Date: Thu, 8 Jan 2026 23:52:42 +0530 Subject: [PATCH 3/9] Add API key ID suffix to api key --- .../pkg/apikeyxds/apikey_manager.go | 4 +- .../pkg/apikeyxds/apikey_snapshot.go | 4 +- .../pkg/constants/constants.go | 5 + .../pkg/storage/apikey_store.go | 19 +-- .../pkg/storage/interface.go | 6 + .../gateway-controller/pkg/storage/memory.go | 68 ++++++++- .../gateway-controller/pkg/storage/sqlite.go | 40 ++++++ .../gateway-controller/pkg/utils/api_key.go | 87 ++++++++---- sdk/gateway/policy/v1alpha/api_key.go | 133 +++++++++++++----- 9 files changed, 284 insertions(+), 82 deletions(-) diff --git a/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go b/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go index e9165fa8f..5e7589afa 100644 --- a/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go +++ b/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go @@ -68,7 +68,7 @@ func (asm *APIKeyStateManager) StoreAPIKey(apiId, apiName, apiVersion string, ap } // RevokeAPIKey revokes an API key and updates the policy engine with the complete state -func (asm *APIKeyStateManager) RevokeAPIKey(apiId, apiName, apiVersion, apiKeyValue, correlationID string) error { +func (asm *APIKeyStateManager) RevokeAPIKey(apiId, apiName, apiVersion, apiKeyID, apiKeyValue, correlationID string) error { asm.logger.Info("Revoking API key with state-of-the-world update", zap.String("api_id", apiId), zap.String("api_name", apiName), @@ -76,7 +76,7 @@ func (asm *APIKeyStateManager) RevokeAPIKey(apiId, apiName, apiVersion, apiKeyVa zap.String("correlation_id", correlationID)) // Revoke the API key and update the snapshot - if err := asm.snapshotManager.RevokeAPIKey(apiId, apiKeyValue); err != nil { + if err := asm.snapshotManager.RevokeAPIKey(apiId, apiKeyID, apiKeyValue); err != nil { asm.logger.Error("Failed to revoke API key and update snapshot", zap.String("api_key_value", asm.MaskAPIKey(apiKeyValue)), zap.Error(err)) diff --git a/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go b/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go index 2aa11ea84..075f9c201 100644 --- a/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go +++ b/gateway/gateway-controller/pkg/apikeyxds/apikey_snapshot.go @@ -133,13 +133,13 @@ func (sm *APIKeySnapshotManager) StoreAPIKey(apiKey *models.APIKey) error { } // RevokeAPIKey revokes an API key and updates the snapshot -func (sm *APIKeySnapshotManager) RevokeAPIKey(apiId, apiKeyValue string) error { +func (sm *APIKeySnapshotManager) RevokeAPIKey(apiId, apiKeyID, apiKeyValue string) error { sm.logger.Info("Revoking API key", zap.String("api_id", apiId), zap.String("api_key_value", MaskAPIKey(apiKeyValue))) // Revoke in the API key store - if !sm.store.Revoke(apiId, apiKeyValue) { + if !sm.store.Revoke(apiId, apiKeyID, apiKeyValue) { sm.logger.Warn("API key not found for revocation", zap.String("api_id", apiId), zap.String("api_key_value", MaskAPIKey(apiKeyValue))) diff --git a/gateway/gateway-controller/pkg/constants/constants.go b/gateway/gateway-controller/pkg/constants/constants.go index 895e28e10..95eb571cd 100644 --- a/gateway/gateway-controller/pkg/constants/constants.go +++ b/gateway/gateway-controller/pkg/constants/constants.go @@ -125,6 +125,11 @@ const ( " name: '%s'\n" + " value: '%s'\n" + // API Key constants + APIKeyPrefix = "apip_" + APIKeyLen = 32 // Length of the random part of the API key in bytes + APIKeySeparator = "." + // HashingAlgorithm constants HashingAlgorithmSHA256 = "sha256" HashingAlgorithmBcrypt = "bcrypt" diff --git a/gateway/gateway-controller/pkg/storage/apikey_store.go b/gateway/gateway-controller/pkg/storage/apikey_store.go index ecd66a8c1..8daa41201 100644 --- a/gateway/gateway-controller/pkg/storage/apikey_store.go +++ b/gateway/gateway-controller/pkg/storage/apikey_store.go @@ -107,24 +107,26 @@ func (s *APIKeyStore) GetAll() []*models.APIKey { } // Revoke marks an API key as revoked by finding it through hash comparison -func (s *APIKeyStore) Revoke(apiId, plainAPIKeyValue string) bool { +func (s *APIKeyStore) Revoke(apiId, apiKeyID, plainAPIKeyValue string) bool { s.mu.Lock() defer s.mu.Unlock() - // Get all API keys for the specified API - apiKeys, exists := s.apiKeysByAPI[apiId] + apiKey, exists := s.apiKeys[apiKeyID] if !exists { - s.logger.Debug("No API keys found for API", - zap.String("api_id", apiId)) + s.logger.Debug("API key ID not found for revocation", + zap.String("api_id", apiId), + zap.String("api_key_id", apiKeyID)) return false } - // Find the API key by comparing plain text key against stored hashes - for _, apiKey := range apiKeys { + if apiKey != nil { if compareAPIKeys(plainAPIKeyValue, apiKey.APIKey) { // Hash matches - this is our target API key apiKey.Status = models.APIKeyStatusRevoked + delete(s.apiKeys, apiKey.ID) + s.removeFromAPIMapping(apiKey) + s.logger.Debug("Revoked API key", zap.String("id", apiKey.ID), zap.String("name", apiKey.Name), @@ -135,7 +137,8 @@ func (s *APIKeyStore) Revoke(apiId, plainAPIKeyValue string) bool { } s.logger.Debug("API key not found for revocation", - zap.String("api_id", apiId)) + zap.String("api_id", apiId), + zap.String("api_key_id", apiKeyID)) return false } diff --git a/gateway/gateway-controller/pkg/storage/interface.go b/gateway/gateway-controller/pkg/storage/interface.go index fcbc80ef1..eaf0ce2ac 100644 --- a/gateway/gateway-controller/pkg/storage/interface.go +++ b/gateway/gateway-controller/pkg/storage/interface.go @@ -151,6 +151,12 @@ type Storage interface { // Implementations should ensure this operation is atomic (all-or-nothing). SaveAPIKey(apiKey *models.APIKey) error + // GetAPIKeyByID retrieves an API key by its ID. + // + // Returns an error if the API key is not found. + // This is used for API key validation during authentication. + GetAPIKeyByID(id string) (*models.APIKey, error) + // GetAPIKeyByKey retrieves an API key by its key value. // // Returns an error if the API key is not found. diff --git a/gateway/gateway-controller/pkg/storage/memory.go b/gateway/gateway-controller/pkg/storage/memory.go index 849f3544b..8d87c165f 100644 --- a/gateway/gateway-controller/pkg/storage/memory.go +++ b/gateway/gateway-controller/pkg/storage/memory.go @@ -41,6 +41,7 @@ type ConfigStore struct { templateIdByHandle map[string]string // API Keys storage + apiKeys map[string]*models.APIKey // key: API key ID apiKeysByAPI map[string][]*models.APIKey // Key: "configID" → Value: slice of APIKeys } @@ -54,6 +55,7 @@ func NewConfigStore() *ConfigStore { TopicManager: NewTopicManager(), templates: make(map[string]*models.StoredLLMProviderTemplate), templateIdByHandle: make(map[string]string), + apiKeys: make(map[string]*models.APIKey), apiKeysByAPI: make(map[string][]*models.APIKey), } } @@ -479,22 +481,37 @@ func (cs *ConfigStore) StoreAPIKey(apiKey *models.APIKey) error { } } - // Note: We no longer check for API key value collisions because: - // 1. API key values are now hashed, making direct comparison impossible - // 2. API keys use 256-bit cryptographic random generation, making collisions highly unlikely - // 3. Any extremely rare hash collisions will be handled at the database level with constraints - if existingKeyIndex >= 0 { // Update the existing entry in apiKeysByAPI + cs.apiKeys[apiKey.ID] = apiKey cs.apiKeysByAPI[apiKey.APIId][existingKeyIndex] = apiKey } else { // Insert new API key + // Check if API key ID already exists + if _, exists := cs.apiKeys[apiKey.ID]; exists { + return ErrConflict + } + // Store by API key value + cs.apiKeys[apiKey.ID] = apiKey cs.apiKeysByAPI[apiKey.APIId] = append(cs.apiKeysByAPI[apiKey.APIId], apiKey) } return nil } +// GetAPIKeyByID retrieves an API key by its ID +func (cs *ConfigStore) GetAPIKeyByID(id string) (*models.APIKey, error) { + cs.mu.RLock() + defer cs.mu.RUnlock() + + apiKey, exists := cs.apiKeys[id] + if !exists { + return nil, ErrNotFound + } + + return apiKey, nil +} + // GetAPIKeysByAPI retrieves all API keys for a specific API func (cs *ConfigStore) GetAPIKeysByAPI(apiId string) ([]*models.APIKey, error) { cs.mu.RLock() @@ -531,16 +548,52 @@ func (cs *ConfigStore) GetAPIKeyByName(apiId, name string) (*models.APIKey, erro return nil, ErrNotFound } +// RemoveAPIKeyByID removes an API key from the in-memory cache by its ID +func (cs *ConfigStore) RemoveAPIKeyByID(id string) error { + cs.mu.Lock() + defer cs.mu.Unlock() + + apiKey, exists := cs.apiKeys[id] + if !exists { + return ErrNotFound + } + + // Remove from apiKeysByAPI slice + apiKeys, apiIdExists := cs.apiKeysByAPI[apiKey.APIId] + if apiIdExists { + for i, key := range apiKeys { + if key.ID == id { + // Remove the API key from the slice + cs.apiKeysByAPI[apiKey.APIId] = append(apiKeys[:i], apiKeys[i+1:]...) + break + } + } + // Clean up empty slices + if len(cs.apiKeysByAPI[apiKey.APIId]) == 0 { + delete(cs.apiKeysByAPI, apiKey.APIId) + } + } + + // Remove from main map + delete(cs.apiKeys, id) + + return nil +} + // RemoveAPIKeysByAPI removes all API keys for a specific API func (cs *ConfigStore) RemoveAPIKeysByAPI(apiId string) error { cs.mu.Lock() defer cs.mu.Unlock() - _, exists := cs.apiKeysByAPI[apiId] + apiKeys, exists := cs.apiKeysByAPI[apiId] if !exists { return nil // No keys to remove } + for _, key := range apiKeys { + delete(cs.apiKeys, key.ID) + } + // Remove from API-specific map delete(cs.apiKeysByAPI, apiId) @@ -582,5 +635,8 @@ func (cs *ConfigStore) RemoveAPIKeyByName(apiId, name string) error { delete(cs.apiKeysByAPI, apiId) } + // Remove from main apiKeys map + delete(cs.apiKeys, targetAPIKey.ID) + return nil } diff --git a/gateway/gateway-controller/pkg/storage/sqlite.go b/gateway/gateway-controller/pkg/storage/sqlite.go index df4a7168e..f773a4f7f 100644 --- a/gateway/gateway-controller/pkg/storage/sqlite.go +++ b/gateway/gateway-controller/pkg/storage/sqlite.go @@ -1161,6 +1161,46 @@ func (s *SQLiteStorage) SaveAPIKey(apiKey *models.APIKey) error { return nil } +// GetAPIKeyByID retrieves an API key by its ID +func (s *SQLiteStorage) GetAPIKeyByID(id string) (*models.APIKey, error) { + query := ` + SELECT id, name, api_key, apiId, operations, status, + created_at, created_by, updated_at, expires_at + FROM api_keys + WHERE id = ? + ` + + var apiKey models.APIKey + var expiresAt sql.NullTime + + err := s.db.QueryRow(query, id).Scan( + &apiKey.ID, + &apiKey.Name, + &apiKey.APIKey, + &apiKey.APIId, + &apiKey.Operations, + &apiKey.Status, + &apiKey.CreatedAt, + &apiKey.CreatedBy, + &apiKey.UpdatedAt, + &expiresAt, + ) + + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("%w: key not found", ErrNotFound) + } + return nil, fmt.Errorf("failed to query API key: %w", err) + } + + // Handle nullable expires_at field + if expiresAt.Valid { + apiKey.ExpiresAt = &expiresAt.Time + } + + return &apiKey, nil +} + // GetAPIKeyByKey retrieves an API key by its key value func (s *SQLiteStorage) GetAPIKeyByKey(key string) (*models.APIKey, error) { query := ` diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index 372152755..6a3cda5d0 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -101,10 +101,16 @@ type ListAPIKeyResult struct { Response api.APIKeyListResponse // Response following the generated schema } +// ParsedAPIKey represents a parsed API key with its components +type ParsedAPIKey struct { + APIKey string + ID string +} + // XDSManager interface for API key operations type XDSManager interface { StoreAPIKey(apiId, apiName, apiVersion string, apiKey *models.APIKey, correlationID string) error - RevokeAPIKey(apiId, apiName, apiVersion, apiKeyValue, correlationID string) error + RevokeAPIKey(apiId, apiName, apiVersion, apiKeyID, apiKeyValue, correlationID string) error RemoveAPIKeysByAPI(apiId, apiName, apiVersion, correlationID string) error } @@ -128,8 +134,6 @@ func NewAPIKeyService(store *storage.ConfigStore, db storage.Storage, xdsManager } const ( - APIKeyPrefix = "apip_" - // Argon2id parameters (recommended for production security) argon2Time = 1 // Number of iterations argon2Memory = 64 * 1024 // Memory usage in KiB (64 MiB) @@ -274,6 +278,12 @@ func (s *APIKeyService) GenerateAPIKey(params APIKeyGenerationParams) (*APIKeyGe func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevocationResult, error) { logger := params.Logger user := params.User + result := &APIKeyRevocationResult{ + Response: api.APIKeyRevocationResponse{ + Status: "success", + Message: "API key revoked successfully", + }, + } // Validate that API exists config, err := s.store.GetByHandle(params.Handle) @@ -292,16 +302,22 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo return nil, fmt.Errorf("API key value is required for revocation") } - // Get all API keys for this API and find the one that matches the plain text key - var apiKey *models.APIKey + parsedAPIkey, ok := s.parseAPIKey(providedAPIKeyValue) + if !ok { + // invalid format + logger.Warn("Invalid API key format", + zap.String("handle", params.Handle), + zap.String("correlation_id", params.CorrelationID)) + return result, nil + } + var matchedKey *models.APIKey - // First, try to get API keys from memory store - apiKeys, err := s.store.GetAPIKeysByAPI(config.ID) + apiKey, err := s.store.GetAPIKeyByID(parsedAPIkey.ID) if err != nil { // If memory store fails, try database if s.db != nil { - apiKeys, err = s.db.GetAPIKeysByAPI(config.ID) + apiKey, err = s.db.GetAPIKeyByID(parsedAPIkey.ID) if err != nil { logger.Debug("Failed to get API keys for revocation", zap.Error(err), @@ -313,23 +329,12 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo } // Find the API key that matches the provided plain text key - if apiKeys != nil { - for _, key := range apiKeys { - if s.compareAPIKeys(providedAPIKeyValue, key.APIKey) { - matchedKey = key - apiKey = key - break - } + if apiKey != nil { + if s.compareAPIKeys(parsedAPIkey.APIKey, apiKey.APIKey) { + matchedKey = apiKey } } - result := &APIKeyRevocationResult{ - Response: api.APIKeyRevocationResponse{ - Status: "success", - Message: "API key revoked successfully", - }, - } - // For security reasons, perform all validations but don't return errors // This prevents information leakage about API key details if apiKey != nil { @@ -431,7 +436,8 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo // Send the plain API key revocation to the policy engine via xDS // The policy engine will find and revoke the matching hashed key if s.xdsManager != nil { - if err := s.xdsManager.RevokeAPIKey(apiId, apiName, apiVersion, providedAPIKeyValue, params.CorrelationID); err != nil { + if err := s.xdsManager.RevokeAPIKey(apiId, apiName, apiVersion, parsedAPIkey.ID, parsedAPIkey.APIKey, + params.CorrelationID); err != nil { logger.Error("Failed to remove API key from policy engine", zap.Error(err), zap.String("correlation_id", params.CorrelationID)) @@ -829,7 +835,10 @@ func (s *APIKeyService) buildAPIKeyResponse(key *models.APIKey, handle string) a // Use PlainAPIKey for response if available, otherwise mask the hashed key var responseAPIKey *string if key.PlainAPIKey != "" { - responseAPIKey = &key.PlainAPIKey + // Format: apip_{key}.{hex_encoded_id} + hexEncodedID := hex.EncodeToString([]byte(key.ID)) + formattedAPIKey := key.PlainAPIKey + constants.APIKeySeparator + hexEncodedID + responseAPIKey = &formattedAPIKey } else { // For existing keys where PlainAPIKey is not available, don't return the hashed key responseAPIKey = nil @@ -1082,11 +1091,11 @@ func (s *APIKeyService) filterAPIKeysByUser(user *commonmodels.AuthContext, apiK // generateAPIKeyValue generates a new API key value with collision handling func (s *APIKeyService) generateAPIKeyValue() (string, error) { - randomBytes := make([]byte, 32) + randomBytes := make([]byte, constants.APIKeyLen) if _, err := rand.Read(randomBytes); err != nil { return "", fmt.Errorf("failed to generate random bytes: %w", err) } - return APIKeyPrefix + hex.EncodeToString(randomBytes), nil + return constants.APIKeyPrefix + hex.EncodeToString(randomBytes), nil } // MaskAPIKey masks an API key for secure logging, showing first 8 and last 4 characters @@ -1340,6 +1349,32 @@ func decodeBase64(s string) ([]byte, error) { return base64.StdEncoding.DecodeString(s) } +// parseAPIKey splits an API key value into its key and ID components +func (s *APIKeyService) parseAPIKey(value string) (ParsedAPIKey, bool) { + idx := strings.LastIndex(value, ".") + if idx <= 0 || idx == len(value)-1 { + return ParsedAPIKey{}, false + } + + apiKey := value[:idx] + hexEncodedID := value[idx+1:] + + // Decode the hex encoded ID back to the raw ID + decodedIDBytes, err := hex.DecodeString(hexEncodedID) + if err != nil { + // If decoding fails, return the hex value as-is for backward compatibility + return ParsedAPIKey{ + APIKey: apiKey, + ID: hexEncodedID, + }, true + } + + return ParsedAPIKey{ + APIKey: apiKey, + ID: string(decodedIDBytes), + }, true +} + // SetHashingConfig allows updating the hashing configuration at runtime func (s *APIKeyService) SetHashingConfig(config *config.APIKeyHashingConfig) { s.hashingConfig = config diff --git a/sdk/gateway/policy/v1alpha/api_key.go b/sdk/gateway/policy/v1alpha/api_key.go index bd7ae692d..d6033f2b0 100644 --- a/sdk/gateway/policy/v1alpha/api_key.go +++ b/sdk/gateway/policy/v1alpha/api_key.go @@ -42,6 +42,12 @@ type APIKey struct { // APIKeyStatus Status of the API key type APIKeyStatus string +// ParsedAPIKey represents a parsed API key with its components +type ParsedAPIKey struct { + APIKey string + ID string +} + // Defines values for APIKeyStatus. const ( Active APIKeyStatus = "active" @@ -68,12 +74,14 @@ var ( type APIkeyStore struct { mu sync.RWMutex // Protects concurrent access // API Keys storage + apiKeys map[string]*APIKey // key: API key ID → value: APIKey apiKeysByAPI map[string][]*APIKey // Key: "API ID" → Value: slice of APIKeys } // NewAPIkeyStore creates a new in-memory API key store func NewAPIkeyStore() *APIkeyStore { return &APIkeyStore{ + apiKeys: make(map[string]*APIKey), apiKeysByAPI: make(map[string][]*APIKey), } } @@ -108,16 +116,18 @@ func (aks *APIkeyStore) StoreAPIKey(apiId string, apiKey *APIKey) error { } } - // Note: We no longer check for API key value collisions because: - // 1. API key values are now hashed, making direct comparison impossible - // 2. API keys use 256-bit cryptographic random generation, making collisions highly unlikely - // 3. Any extremely rare hash collisions will be handled at the database level with constraints - if existingKeyIndex >= 0 { // Update the existing entry in apiKeysByAPI + aks.apiKeys[apiKey.ID] = apiKey aks.apiKeysByAPI[apiId][existingKeyIndex] = apiKey } else { // Insert new API key + // Check if API key ID already exists + if _, exists := aks.apiKeys[apiKey.ID]; exists { + return ErrConflict + } + // Store by API key value + aks.apiKeys[apiKey.ID] = apiKey aks.apiKeysByAPI[apiId] = append(aks.apiKeysByAPI[apiId], apiKey) } @@ -129,19 +139,22 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, pro aks.mu.Lock() defer aks.mu.Unlock() - // Get API keys for the apiId - apiKeys, exists := aks.apiKeysByAPI[apiId] - if !exists { + parsedAPIkey, ok := parseAPIKey(providedAPIKey) + if !ok { return false, ErrNotFound } - // Find the API key that matches the provided plain text key (by comparing against hashed values) var targetAPIKey *APIKey - for _, ak := range apiKeys { - if compareAPIKeys(providedAPIKey, ak.APIKey) { - targetAPIKey = ak - break + apiKey, exists := aks.apiKeys[parsedAPIkey.ID] + if !exists { + return false, ErrNotFound + } + + // Find the API key that matches the provided plain text key (by comparing against hashed values) + if apiKey != nil { + if compareAPIKeys(parsedAPIkey.APIKey, apiKey.APIKey) { + targetAPIKey = apiKey } } @@ -188,46 +201,39 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, pro } // RevokeAPIKey revokes a specific API key by plain text API key value -func (aks *APIkeyStore) RevokeAPIKey(apiId, plainAPIKeyValue string) error { +func (aks *APIkeyStore) RevokeAPIKey(apiId, providedAPIKey string) error { aks.mu.Lock() defer aks.mu.Unlock() - // Get API keys for the apiId - apiKeys, exists := aks.apiKeysByAPI[apiId] - if !exists { - // If the API doesn't exist in our store, we treat revocation as successful - // since the key is effectively "not active" anyway + parsedAPIkey, ok := parseAPIKey(providedAPIKey) + if !ok { return nil } - // Find the API key with the matching hashed key value - var targetAPIKey *APIKey - var targetIndex = -1 + var matchedKey *APIKey - for i, apiKey := range apiKeys { - // Compare plain text key with stored hashed key - if compareAPIKeys(plainAPIKeyValue, apiKey.APIKey) { - targetAPIKey = apiKey - targetIndex = i - break + apiKey, exists := aks.apiKeys[parsedAPIkey.ID] + if !exists { + return nil + } + + // Find the API key that matches the provided plain text key + if apiKey != nil { + if compareAPIKeys(parsedAPIkey.APIKey, apiKey.APIKey) { + matchedKey = apiKey } } // If the API key doesn't exist, treat revocation as successful (idempotent operation) - if targetAPIKey == nil { + if matchedKey == nil { return nil } // Set status to revoked - targetAPIKey.Status = Revoked - - // Remove from apiKeysByAPI slice - aks.apiKeysByAPI[apiId] = append(apiKeys[:targetIndex], apiKeys[targetIndex+1:]...) + matchedKey.Status = Revoked - // Clean up empty slices - if len(aks.apiKeysByAPI[apiId]) == 0 { - delete(aks.apiKeysByAPI, apiId) - } + delete(aks.apiKeys, matchedKey.ID) + aks.removeFromAPIMapping(apiKey) return nil } @@ -237,11 +243,16 @@ func (aks *APIkeyStore) RemoveAPIKeysByAPI(apiId string) error { aks.mu.Lock() defer aks.mu.Unlock() - _, exists := aks.apiKeysByAPI[apiId] + apiKeys, exists := aks.apiKeysByAPI[apiId] if !exists { return nil // No keys to remove } + // Remove from main map + for _, key := range apiKeys { + delete(aks.apiKeys, key.ID) + } + // Remove from API-specific map delete(aks.apiKeysByAPI, apiId) @@ -253,6 +264,9 @@ func (aks *APIkeyStore) ClearAll() error { aks.mu.Lock() defer aks.mu.Unlock() + // Clear the main API keys map + aks.apiKeys = make(map[string]*APIKey) + // Clear the API-specific keys map aks.apiKeysByAPI = make(map[string][]*APIKey) @@ -392,3 +406,46 @@ func decodeBase64(s string) ([]byte, error) { // try StdEncoding as a fallback return base64.StdEncoding.DecodeString(s) } + +// parseAPIKey splits an API key value into its key and ID components +func parseAPIKey(value string) (ParsedAPIKey, bool) { + idx := strings.LastIndex(value, ".") + if idx <= 0 || idx == len(value)-1 { + return ParsedAPIKey{}, false + } + + apiKey := value[:idx] + hexEncodedID := value[idx+1:] + + // Decode the hex encoded ID back to the raw ID + decodedIDBytes, err := hex.DecodeString(hexEncodedID) + if err != nil { + // If decoding fails, return the hex value as-is for backward compatibility + return ParsedAPIKey{ + APIKey: apiKey, + ID: hexEncodedID, + }, true + } + + return ParsedAPIKey{ + APIKey: apiKey, + ID: string(decodedIDBytes), + }, true +} + +// removeFromAPIMapping removes an API key from the API mapping +func (aks *APIkeyStore) removeFromAPIMapping(apiKey *APIKey) { + apiKeys := aks.apiKeysByAPI[apiKey.APIId] + for i, ak := range apiKeys { + if ak.ID == apiKey.ID { + // Remove the element at index i + aks.apiKeysByAPI[apiKey.APIId] = append(apiKeys[:i], apiKeys[i+1:]...) + break + } + } + + // If no API keys left for this API, remove the mapping + if len(aks.apiKeysByAPI[apiKey.APIId]) == 0 { + delete(aks.apiKeysByAPI, apiKey.APIId) + } +} From 824d7892d64e0a6dbc3ae86e4c1a54183a09d2c5 Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 9 Jan 2026 10:56:29 +0530 Subject: [PATCH 4/9] Optimize API key storage --- .../pkg/storage/apikey_store.go | 55 +++++++++++------- .../gateway-controller/pkg/storage/memory.go | 56 ++++++++++--------- .../gateway-controller/pkg/utils/api_key.go | 12 ++-- sdk/gateway/policy/v1alpha/api_key.go | 43 +++++++------- 4 files changed, 94 insertions(+), 72 deletions(-) diff --git a/gateway/gateway-controller/pkg/storage/apikey_store.go b/gateway/gateway-controller/pkg/storage/apikey_store.go index 8daa41201..15c5798c9 100644 --- a/gateway/gateway-controller/pkg/storage/apikey_store.go +++ b/gateway/gateway-controller/pkg/storage/apikey_store.go @@ -38,8 +38,8 @@ import ( // APIKeyStore manages API keys in memory with thread-safe operations type APIKeyStore struct { mu sync.RWMutex - apiKeys map[string]*models.APIKey // key: API key ID - apiKeysByAPI map[string][]*models.APIKey // key: API ID + apiKeys map[string]*models.APIKey // key: API key ID + apiKeysByAPI map[string]map[string]*models.APIKey // Key: configID → Value: map[keyID]*APIKey resourceVersion int64 logger *zap.Logger } @@ -48,7 +48,7 @@ type APIKeyStore struct { func NewAPIKeyStore(logger *zap.Logger) *APIKeyStore { return &APIKeyStore{ apiKeys: make(map[string]*models.APIKey), - apiKeysByAPI: make(map[string][]*models.APIKey), + apiKeysByAPI: make(map[string]map[string]*models.APIKey), logger: logger, } } @@ -88,9 +88,11 @@ func (s *APIKeyStore) GetByAPI(apiId string) []*models.APIKey { defer s.mu.RUnlock() apiKeys := s.apiKeysByAPI[apiId] - // Return a copy to avoid external modification - result := make([]*models.APIKey, len(apiKeys)) - copy(result, apiKeys) + // Convert map values to slice and return a copy to prevent external modification + result := make([]*models.APIKey, 0, len(apiKeys)) + for _, apiKey := range apiKeys { + result = append(result, apiKey) + } return result } @@ -202,25 +204,40 @@ func (s *APIKeyStore) GetResourceVersion() int64 { // addToAPIMapping adds an API key to the API mapping func (s *APIKeyStore) addToAPIMapping(apiKey *models.APIKey) { - apiKeys := s.apiKeysByAPI[apiKey.APIId] - s.apiKeysByAPI[apiKey.APIId] = append(apiKeys, apiKey) + existingKeys, apiIdExists := s.apiKeysByAPI[apiKey.APIId] + var existingKeyID = "" + + if apiIdExists { + for id, existingKey := range existingKeys { + if existingKey.Name == apiKey.Name { + existingKeyID = id + break + } + } + } + + if existingKeyID != "" { + // Update the existing entry in apiKeysByAPI + s.apiKeysByAPI[apiKey.APIId][existingKeyID] = apiKey + } else { + // Initialize the map for this API ID if it doesn't exist + if s.apiKeysByAPI[apiKey.APIId] == nil { + s.apiKeysByAPI[apiKey.APIId] = make(map[string]*models.APIKey) + } + s.apiKeysByAPI[apiKey.APIId][apiKey.ID] = apiKey + } } // removeFromAPIMapping removes an API key from the API mapping func (s *APIKeyStore) removeFromAPIMapping(apiKey *models.APIKey) { - apiKeys := s.apiKeysByAPI[apiKey.APIId] - for i, ak := range apiKeys { - if ak.ID == apiKey.ID { - // Remove the element at index i - s.apiKeysByAPI[apiKey.APIId] = append(apiKeys[:i], apiKeys[i+1:]...) - break + apiKeys, apiIdExists := s.apiKeysByAPI[apiKey.APIId] + if apiIdExists { + delete(apiKeys, apiKey.ID) + // clean up empty maps + if len(s.apiKeysByAPI[apiKey.APIId]) == 0 { + delete(s.apiKeysByAPI, apiKey.APIId) } } - - // If no API keys left for this API, remove the mapping - if len(s.apiKeysByAPI[apiKey.APIId]) == 0 { - delete(s.apiKeysByAPI, apiKey.APIId) - } } // compareAPIKeys compares API keys for external use diff --git a/gateway/gateway-controller/pkg/storage/memory.go b/gateway/gateway-controller/pkg/storage/memory.go index 8d87c165f..ddca7dbd3 100644 --- a/gateway/gateway-controller/pkg/storage/memory.go +++ b/gateway/gateway-controller/pkg/storage/memory.go @@ -41,8 +41,8 @@ type ConfigStore struct { templateIdByHandle map[string]string // API Keys storage - apiKeys map[string]*models.APIKey // key: API key ID - apiKeysByAPI map[string][]*models.APIKey // Key: "configID" → Value: slice of APIKeys + apiKeys map[string]*models.APIKey // key: API key ID + apiKeysByAPI map[string]map[string]*models.APIKey // Key: configID → Value: map[keyID]*APIKey } // NewConfigStore creates a new in-memory config store @@ -56,7 +56,7 @@ func NewConfigStore() *ConfigStore { templates: make(map[string]*models.StoredLLMProviderTemplate), templateIdByHandle: make(map[string]string), apiKeys: make(map[string]*models.APIKey), - apiKeysByAPI: make(map[string][]*models.APIKey), + apiKeysByAPI: make(map[string]map[string]*models.APIKey), } } @@ -470,30 +470,36 @@ func (cs *ConfigStore) StoreAPIKey(apiKey *models.APIKey) error { // Check if an API key with the same apiId and name already exists existingKeys, apiIdExists := cs.apiKeysByAPI[apiKey.APIId] - var existingKeyIndex = -1 + var existingKeyID = "" if apiIdExists { - for i, existingKey := range existingKeys { + for id, existingKey := range existingKeys { if existingKey.Name == apiKey.Name { - existingKeyIndex = i + existingKeyID = id break } } } - if existingKeyIndex >= 0 { + if existingKeyID != "" { // Update the existing entry in apiKeysByAPI cs.apiKeys[apiKey.ID] = apiKey - cs.apiKeysByAPI[apiKey.APIId][existingKeyIndex] = apiKey + cs.apiKeysByAPI[apiKey.APIId][existingKeyID] = apiKey } else { // Insert new API key // Check if API key ID already exists if _, exists := cs.apiKeys[apiKey.ID]; exists { return ErrConflict } + + // Initialize the map for this API ID if it doesn't exist + if cs.apiKeysByAPI[apiKey.APIId] == nil { + cs.apiKeysByAPI[apiKey.APIId] = make(map[string]*models.APIKey) + } + // Store by API key value cs.apiKeys[apiKey.ID] = apiKey - cs.apiKeysByAPI[apiKey.APIId] = append(cs.apiKeysByAPI[apiKey.APIId], apiKey) + cs.apiKeysByAPI[apiKey.APIId][apiKey.ID] = apiKey } return nil @@ -522,9 +528,11 @@ func (cs *ConfigStore) GetAPIKeysByAPI(apiId string) ([]*models.APIKey, error) { return []*models.APIKey{}, nil // Return empty slice instead of nil } - // Return a copy to prevent external modification - result := make([]*models.APIKey, len(apiKeys)) - copy(result, apiKeys) + // Convert map values to slice and return a copy to prevent external modification + result := make([]*models.APIKey, 0, len(apiKeys)) + for _, apiKey := range apiKeys { + result = append(result, apiKey) + } return result, nil } @@ -558,17 +566,11 @@ func (cs *ConfigStore) RemoveAPIKeyByID(id string) error { return ErrNotFound } - // Remove from apiKeysByAPI slice + // Remove from apiKeysByAPI map apiKeys, apiIdExists := cs.apiKeysByAPI[apiKey.APIId] if apiIdExists { - for i, key := range apiKeys { - if key.ID == id { - // Remove the API key from the slice - cs.apiKeysByAPI[apiKey.APIId] = append(apiKeys[:i], apiKeys[i+1:]...) - break - } - } - // Clean up empty slices + delete(apiKeys, id) + // Clean up empty maps if len(cs.apiKeysByAPI[apiKey.APIId]) == 0 { delete(cs.apiKeysByAPI, apiKey.APIId) } @@ -613,12 +615,12 @@ func (cs *ConfigStore) RemoveAPIKeyByName(apiId, name string) error { // Find the API key with the matching name var targetAPIKey *models.APIKey - var targetIndex = -1 + var targetID string - for i, apiKey := range apiKeys { + for id, apiKey := range apiKeys { if apiKey.Name == name { targetAPIKey = apiKey - targetIndex = i + targetID = id break } } @@ -627,10 +629,10 @@ func (cs *ConfigStore) RemoveAPIKeyByName(apiId, name string) error { return ErrNotFound } - // Remove from apiKeysByAPI slice - cs.apiKeysByAPI[apiId] = append(apiKeys[:targetIndex], apiKeys[targetIndex+1:]...) + // Remove from apiKeysByAPI map + delete(cs.apiKeysByAPI[apiId], targetID) - // Clean up empty slices + // Clean up empty maps if len(cs.apiKeysByAPI[apiId]) == 0 { delete(cs.apiKeysByAPI, apiId) } diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index 6a3cda5d0..b470407ed 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -311,13 +311,14 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo return result, nil } + var apiKey *models.APIKey var matchedKey *models.APIKey - apiKey, err := s.store.GetAPIKeyByID(parsedAPIkey.ID) + existingAPIKey, err := s.store.GetAPIKeyByID(parsedAPIkey.ID) if err != nil { // If memory store fails, try database if s.db != nil { - apiKey, err = s.db.GetAPIKeyByID(parsedAPIkey.ID) + existingAPIKey, err = s.db.GetAPIKeyByID(parsedAPIkey.ID) if err != nil { logger.Debug("Failed to get API keys for revocation", zap.Error(err), @@ -329,9 +330,10 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo } // Find the API key that matches the provided plain text key - if apiKey != nil { - if s.compareAPIKeys(parsedAPIkey.APIKey, apiKey.APIKey) { - matchedKey = apiKey + if existingAPIKey != nil { + if s.compareAPIKeys(parsedAPIkey.APIKey, existingAPIKey.APIKey) { + apiKey = existingAPIKey + matchedKey = existingAPIKey } } diff --git a/sdk/gateway/policy/v1alpha/api_key.go b/sdk/gateway/policy/v1alpha/api_key.go index d6033f2b0..32d55e253 100644 --- a/sdk/gateway/policy/v1alpha/api_key.go +++ b/sdk/gateway/policy/v1alpha/api_key.go @@ -74,15 +74,15 @@ var ( type APIkeyStore struct { mu sync.RWMutex // Protects concurrent access // API Keys storage - apiKeys map[string]*APIKey // key: API key ID → value: APIKey - apiKeysByAPI map[string][]*APIKey // Key: "API ID" → Value: slice of APIKeys + apiKeys map[string]*APIKey // key: API key ID → value: APIKey + apiKeysByAPI map[string]map[string]*APIKey // Key: "API ID" → Value: map[API key ID]*APIKey } // NewAPIkeyStore creates a new in-memory API key store func NewAPIkeyStore() *APIkeyStore { return &APIkeyStore{ apiKeys: make(map[string]*APIKey), - apiKeysByAPI: make(map[string][]*APIKey), + apiKeysByAPI: make(map[string]map[string]*APIKey), } } @@ -105,30 +105,36 @@ func (aks *APIkeyStore) StoreAPIKey(apiId string, apiKey *APIKey) error { // Check if an API key with the same apiId and name already exists existingKeys, apiIdExists := aks.apiKeysByAPI[apiId] - var existingKeyIndex = -1 + var existingKeyID = "" if apiIdExists { - for i, existingKey := range existingKeys { + for id, existingKey := range existingKeys { if existingKey.Name == apiKey.Name { - existingKeyIndex = i + existingKeyID = id break } } } - if existingKeyIndex >= 0 { + if existingKeyID != "" { // Update the existing entry in apiKeysByAPI aks.apiKeys[apiKey.ID] = apiKey - aks.apiKeysByAPI[apiId][existingKeyIndex] = apiKey + aks.apiKeysByAPI[apiId][existingKeyID] = apiKey } else { // Insert new API key // Check if API key ID already exists if _, exists := aks.apiKeys[apiKey.ID]; exists { return ErrConflict } + + // Initialize the map for this API ID if it doesn't exist + if aks.apiKeysByAPI[apiId] == nil { + aks.apiKeysByAPI[apiId] = make(map[string]*APIKey) + } + // Store by API key value aks.apiKeys[apiKey.ID] = apiKey - aks.apiKeysByAPI[apiId] = append(aks.apiKeysByAPI[apiId], apiKey) + aks.apiKeysByAPI[apiId][apiKey.ID] = apiKey } return nil @@ -268,7 +274,7 @@ func (aks *APIkeyStore) ClearAll() error { aks.apiKeys = make(map[string]*APIKey) // Clear the API-specific keys map - aks.apiKeysByAPI = make(map[string][]*APIKey) + aks.apiKeysByAPI = make(map[string]map[string]*APIKey) return nil } @@ -435,17 +441,12 @@ func parseAPIKey(value string) (ParsedAPIKey, bool) { // removeFromAPIMapping removes an API key from the API mapping func (aks *APIkeyStore) removeFromAPIMapping(apiKey *APIKey) { - apiKeys := aks.apiKeysByAPI[apiKey.APIId] - for i, ak := range apiKeys { - if ak.ID == apiKey.ID { - // Remove the element at index i - aks.apiKeysByAPI[apiKey.APIId] = append(apiKeys[:i], apiKeys[i+1:]...) - break + apiKeys, apiIdExists := aks.apiKeysByAPI[apiKey.APIId] + if apiIdExists { + delete(apiKeys, apiKey.ID) + // clean up empty maps + if len(aks.apiKeysByAPI[apiKey.APIId]) == 0 { + delete(aks.apiKeysByAPI, apiKey.APIId) } } - - // If no API keys left for this API, remove the mapping - if len(aks.apiKeysByAPI[apiKey.APIId]) == 0 { - delete(aks.apiKeysByAPI, apiKey.APIId) - } } From f70f2ad36736ee0ff98320929d336a9a846cd867 Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 9 Jan 2026 11:15:15 +0530 Subject: [PATCH 5/9] Add missing dependencies --- gateway/gateway-builder/go.mod | 1 + gateway/gateway-builder/go.sum | 3 +++ 2 files changed, 4 insertions(+) diff --git a/gateway/gateway-builder/go.mod b/gateway/gateway-builder/go.mod index 9b9901f04..f7d890920 100644 --- a/gateway/gateway-builder/go.mod +++ b/gateway/gateway-builder/go.mod @@ -10,6 +10,7 @@ require ( require ( golang.org/x/crypto v0.46.0 // indirect + golang.org/x/sys v0.39.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect ) diff --git a/gateway/gateway-builder/go.sum b/gateway/gateway-builder/go.sum index 04655ef8b..4446ba779 100644 --- a/gateway/gateway-builder/go.sum +++ b/gateway/gateway-builder/go.sum @@ -8,8 +8,11 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= +golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0= golang.org/x/mod v0.30.0 h1:fDEXFVZ/fmCKProc/yAXXUijritrDzahmwwefnjoPFk= golang.org/x/mod v0.30.0/go.mod h1:lAsf5O2EvJeSFMiBxXDki7sCgAxEUcZHXoXMKT4GJKc= +golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= +golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= From ca04649ced500088091bbbe894f3891d81b20f51 Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 9 Jan 2026 11:39:03 +0530 Subject: [PATCH 6/9] Address CodeRabbit comments --- gateway/gateway-controller/pkg/config/config.go | 2 +- gateway/gateway-controller/pkg/utils/api_key.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/gateway/gateway-controller/pkg/config/config.go b/gateway/gateway-controller/pkg/config/config.go index 86b2c3910..65b3cf971 100644 --- a/gateway/gateway-controller/pkg/config/config.go +++ b/gateway/gateway-controller/pkg/config/config.go @@ -1102,7 +1102,7 @@ func (c *Config) validateAuthConfig() error { return nil } -// validateControlPlaneConfig validates the control plane configuration +// validateAPIKeyHashingConfig validates the API key hashing configuration func (c *Config) validateAPIKeyHashingConfig() error { // If hashing is disabled, skip validation if !c.GatewayController.APIKeyHashing.Enabled { diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index b470407ed..1c47e9688 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -1132,7 +1132,7 @@ func (s *APIKeyService) hashAPIKey(plainAPIKey string) (string, error) { } // Hash based on configured algorithm - switch s.hashingConfig.Algorithm { + switch strings.ToLower(s.hashingConfig.Algorithm) { case constants.HashingAlgorithmSHA256: return s.hashAPIKeyWithSHA256(plainAPIKey) case constants.HashingAlgorithmBcrypt: @@ -1146,8 +1146,7 @@ func (s *APIKeyService) hashAPIKey(plainAPIKey string) (string, error) { } // hashAPIKeyWithSHA256 securely hashes an API key using SHA-256 with salt -// Returns the hashed API key that should be st -// Generate random saltored in database and policy engine +// Returns the hashed API key that should be stored in database and policy engine func (s *APIKeyService) hashAPIKeyWithSHA256(plainAPIKey string) (string, error) { if plainAPIKey == "" { return "", fmt.Errorf("API key cannot be empty") From 3398eafc68e64b6eece322c85786b172c4d2c6a0 Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 9 Jan 2026 12:20:54 +0530 Subject: [PATCH 7/9] Fix bugs in API key validation --- sdk/gateway/policy/v1alpha/api_key.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/gateway/policy/v1alpha/api_key.go b/sdk/gateway/policy/v1alpha/api_key.go index 32d55e253..01aad55d2 100644 --- a/sdk/gateway/policy/v1alpha/api_key.go +++ b/sdk/gateway/policy/v1alpha/api_key.go @@ -168,6 +168,11 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, pro return false, ErrNotFound } + // Check if the API key belongs to the specified API + if targetAPIKey.APIId != apiId { + return false, nil + } + // Check if the API key is active if targetAPIKey.Status != Active { return false, nil From 717963a2b9c746315c7bde2bcd198bf4096d2bcc Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 9 Jan 2026 14:42:33 +0530 Subject: [PATCH 8/9] Bug fixes in api key revocation --- gateway/gateway-controller/pkg/utils/api_key.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index 1c47e9688..4a5eb222f 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -344,7 +344,7 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo if apiKey.APIId != config.ID { logger.Debug("API key does not belong to the specified API", zap.String("correlation_id", params.CorrelationID)) - return nil, fmt.Errorf("API key revocation failed for API: '%s'", params.Handle) + return result, nil } err := s.canRevokeAPIKey(user, apiKey, logger) From 24ac89f6185fb34b2f2a0aaa0464aadfe6ae0fb6 Mon Sep 17 00:00:00 2001 From: thivindu Date: Sat, 10 Jan 2026 20:52:32 +0530 Subject: [PATCH 9/9] Remove unnecessary in memory API key maps --- .../gateway-controller/cmd/controller/main.go | 12 +-- .../pkg/apikeyxds/apikey_manager.go | 15 ---- .../pkg/storage/apikey_store.go | 43 ----------- .../gateway-controller/pkg/storage/memory.go | 76 +++---------------- .../gateway-controller/pkg/utils/api_key.go | 4 +- sdk/gateway/policy/v1alpha/api_key.go | 21 +---- 6 files changed, 23 insertions(+), 148 deletions(-) diff --git a/gateway/gateway-controller/cmd/controller/main.go b/gateway/gateway-controller/cmd/controller/main.go index 958d92457..53b83e15a 100644 --- a/gateway/gateway-controller/cmd/controller/main.go +++ b/gateway/gateway-controller/cmd/controller/main.go @@ -106,6 +106,8 @@ func main() { // Initialize in-memory API key store for xDS apiKeyStore := storage.NewAPIKeyStore(log) + apiKeySnapshotManager := apikeyxds.NewAPIKeySnapshotManager(apiKeyStore, log) + apiKeyXDSManager := apikeyxds.NewAPIKeyStateManager(apiKeyStore, apiKeySnapshotManager, log) // Load configurations from database on startup (if persistent mode) if cfg.IsPersistentMode() && db != nil { @@ -123,7 +125,7 @@ func main() { if err := storage.LoadAPIKeysFromDatabase(db, configStore, apiKeyStore); err != nil { log.Fatal("Failed to load API keys from database", zap.Error(err)) } - log.Info("Loaded API keys", zap.Int("count", apiKeyStore.Count())) + log.Info("Loaded API keys", zap.Int("count", apiKeyXDSManager.GetAPIKeyCount())) } // Initialize xDS snapshot manager with router config @@ -166,12 +168,10 @@ func main() { } }() - apiKeySnapshotManager := apikeyxds.NewAPIKeySnapshotManager(apiKeyStore, log) - apiKeyXDSManager := apikeyxds.NewAPIKeyStateManager(apiKeyStore, apiKeySnapshotManager, log) - // Generate initial API key snapshot if API keys were loaded from database - if cfg.IsPersistentMode() && apiKeyStore.Count() > 0 { - log.Info("Generating initial API key snapshot for policy engine", zap.Int("api_key_count", apiKeyStore.Count())) + if cfg.IsPersistentMode() && apiKeyXDSManager.GetAPIKeyCount() > 0 { + log.Info("Generating initial API key snapshot for policy engine", + zap.Int("api_key_count", apiKeyXDSManager.GetAPIKeyCount())) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) if err := apiKeySnapshotManager.UpdateSnapshot(ctx); err != nil { log.Warn("Failed to generate initial API key snapshot", zap.Error(err)) diff --git a/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go b/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go index 5e7589afa..6f3e26249 100644 --- a/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go +++ b/gateway/gateway-controller/pkg/apikeyxds/apikey_manager.go @@ -114,21 +114,6 @@ func (asm *APIKeyStateManager) RemoveAPIKeysByAPI(apiId, apiName, apiVersion, co return nil } -// GetAPIKeyByID retrieves an API key by its ID -func (asm *APIKeyStateManager) GetAPIKeyByID(id string) (*models.APIKey, bool) { - return asm.store.GetByID(id) -} - -// GetAPIKeysByAPI retrieves all API keys for a specific API -func (asm *APIKeyStateManager) GetAPIKeysByAPI(apiId string) []*models.APIKey { - return asm.store.GetByAPI(apiId) -} - -// GetAllAPIKeys retrieves all API keys -func (asm *APIKeyStateManager) GetAllAPIKeys() []*models.APIKey { - return asm.store.GetAll() -} - // GetAPIKeyCount returns the total number of API keys func (asm *APIKeyStateManager) GetAPIKeyCount() int { return asm.store.Count() diff --git a/gateway/gateway-controller/pkg/storage/apikey_store.go b/gateway/gateway-controller/pkg/storage/apikey_store.go index 15c5798c9..594961a60 100644 --- a/gateway/gateway-controller/pkg/storage/apikey_store.go +++ b/gateway/gateway-controller/pkg/storage/apikey_store.go @@ -73,29 +73,6 @@ func (s *APIKeyStore) Store(apiKey *models.APIKey) { zap.String("status", string(apiKey.Status))) } -// GetByID retrieves an API key by its ID -func (s *APIKeyStore) GetByID(id string) (*models.APIKey, bool) { - s.mu.RLock() - defer s.mu.RUnlock() - - apiKey, exists := s.apiKeys[id] - return apiKey, exists -} - -// GetByAPI retrieves all API keys for a specific API -func (s *APIKeyStore) GetByAPI(apiId string) []*models.APIKey { - s.mu.RLock() - defer s.mu.RUnlock() - - apiKeys := s.apiKeysByAPI[apiId] - // Convert map values to slice and return a copy to prevent external modification - result := make([]*models.APIKey, 0, len(apiKeys)) - for _, apiKey := range apiKeys { - result = append(result, apiKey) - } - return result -} - // GetAll retrieves all API keys func (s *APIKeyStore) GetAll() []*models.APIKey { s.mu.RLock() @@ -145,26 +122,6 @@ func (s *APIKeyStore) Revoke(apiId, apiKeyID, plainAPIKeyValue string) bool { return false } -// RemoveByID removes an API key by its ID -func (s *APIKeyStore) RemoveByID(id string) bool { - s.mu.Lock() - defer s.mu.Unlock() - - apiKey, exists := s.apiKeys[id] - if !exists { - return false - } - - delete(s.apiKeys, id) - s.removeFromAPIMapping(apiKey) - - s.logger.Debug("Removed API key", - zap.String("id", id), - zap.String("api_id", apiKey.APIId)) - - return true -} - // RemoveByAPI removes all API keys for a specific API func (s *APIKeyStore) RemoveByAPI(apiId string) int { s.mu.Lock() diff --git a/gateway/gateway-controller/pkg/storage/memory.go b/gateway/gateway-controller/pkg/storage/memory.go index ddca7dbd3..13d9bae7d 100644 --- a/gateway/gateway-controller/pkg/storage/memory.go +++ b/gateway/gateway-controller/pkg/storage/memory.go @@ -41,7 +41,6 @@ type ConfigStore struct { templateIdByHandle map[string]string // API Keys storage - apiKeys map[string]*models.APIKey // key: API key ID apiKeysByAPI map[string]map[string]*models.APIKey // Key: configID → Value: map[keyID]*APIKey } @@ -55,7 +54,6 @@ func NewConfigStore() *ConfigStore { TopicManager: NewTopicManager(), templates: make(map[string]*models.StoredLLMProviderTemplate), templateIdByHandle: make(map[string]string), - apiKeys: make(map[string]*models.APIKey), apiKeysByAPI: make(map[string]map[string]*models.APIKey), } } @@ -483,12 +481,11 @@ func (cs *ConfigStore) StoreAPIKey(apiKey *models.APIKey) error { if existingKeyID != "" { // Update the existing entry in apiKeysByAPI - cs.apiKeys[apiKey.ID] = apiKey cs.apiKeysByAPI[apiKey.APIId][existingKeyID] = apiKey } else { // Insert new API key // Check if API key ID already exists - if _, exists := cs.apiKeys[apiKey.ID]; exists { + if _, exists := cs.apiKeysByAPI[apiKey.APIId][apiKey.ID]; exists { return ErrConflict } @@ -498,7 +495,6 @@ func (cs *ConfigStore) StoreAPIKey(apiKey *models.APIKey) error { } // Store by API key value - cs.apiKeys[apiKey.ID] = apiKey cs.apiKeysByAPI[apiKey.APIId][apiKey.ID] = apiKey } @@ -506,11 +502,11 @@ func (cs *ConfigStore) StoreAPIKey(apiKey *models.APIKey) error { } // GetAPIKeyByID retrieves an API key by its ID -func (cs *ConfigStore) GetAPIKeyByID(id string) (*models.APIKey, error) { +func (cs *ConfigStore) GetAPIKeyByID(apiId, id string) (*models.APIKey, error) { cs.mu.RLock() defer cs.mu.RUnlock() - apiKey, exists := cs.apiKeys[id] + apiKey, exists := cs.apiKeysByAPI[apiId][id] if !exists { return nil, ErrNotFound } @@ -557,28 +553,23 @@ func (cs *ConfigStore) GetAPIKeyByName(apiId, name string) (*models.APIKey, erro } // RemoveAPIKeyByID removes an API key from the in-memory cache by its ID -func (cs *ConfigStore) RemoveAPIKeyByID(id string) error { +func (cs *ConfigStore) RemoveAPIKeyByID(apiId, id string) error { cs.mu.Lock() defer cs.mu.Unlock() - apiKey, exists := cs.apiKeys[id] + _, exists := cs.apiKeysByAPI[apiId][id] if !exists { return ErrNotFound } // Remove from apiKeysByAPI map - apiKeys, apiIdExists := cs.apiKeysByAPI[apiKey.APIId] - if apiIdExists { - delete(apiKeys, id) - // Clean up empty maps - if len(cs.apiKeysByAPI[apiKey.APIId]) == 0 { - delete(cs.apiKeysByAPI, apiKey.APIId) - } + apiKeys, _ := cs.apiKeysByAPI[apiId] + delete(apiKeys, id) + // Clean up empty maps + if len(cs.apiKeysByAPI[apiId]) == 0 { + delete(cs.apiKeysByAPI, apiId) } - // Remove from main map - delete(cs.apiKeys, id) - return nil } @@ -587,58 +578,13 @@ func (cs *ConfigStore) RemoveAPIKeysByAPI(apiId string) error { cs.mu.Lock() defer cs.mu.Unlock() - apiKeys, exists := cs.apiKeysByAPI[apiId] + _, exists := cs.apiKeysByAPI[apiId] if !exists { return nil // No keys to remove } - for _, key := range apiKeys { - delete(cs.apiKeys, key.ID) - } - // Remove from API-specific map delete(cs.apiKeysByAPI, apiId) return nil } - -// RemoveAPIKeyByName removes an API key from the in-memory cache by its apiId and name -func (cs *ConfigStore) RemoveAPIKeyByName(apiId, name string) error { - cs.mu.Lock() - defer cs.mu.Unlock() - - // Get API keys for the apiId - apiKeys, exists := cs.apiKeysByAPI[apiId] - if !exists { - return ErrNotFound - } - - // Find the API key with the matching name - var targetAPIKey *models.APIKey - var targetID string - - for id, apiKey := range apiKeys { - if apiKey.Name == name { - targetAPIKey = apiKey - targetID = id - break - } - } - - if targetAPIKey == nil { - return ErrNotFound - } - - // Remove from apiKeysByAPI map - delete(cs.apiKeysByAPI[apiId], targetID) - - // Clean up empty maps - if len(cs.apiKeysByAPI[apiId]) == 0 { - delete(cs.apiKeysByAPI, apiId) - } - - // Remove from main apiKeys map - delete(cs.apiKeys, targetAPIKey.ID) - - return nil -} diff --git a/gateway/gateway-controller/pkg/utils/api_key.go b/gateway/gateway-controller/pkg/utils/api_key.go index 4a5eb222f..07834813a 100644 --- a/gateway/gateway-controller/pkg/utils/api_key.go +++ b/gateway/gateway-controller/pkg/utils/api_key.go @@ -314,7 +314,7 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo var apiKey *models.APIKey var matchedKey *models.APIKey - existingAPIKey, err := s.store.GetAPIKeyByID(parsedAPIkey.ID) + existingAPIKey, err := s.store.GetAPIKeyByID(config.ID, parsedAPIkey.ID) if err != nil { // If memory store fails, try database if s.db != nil { @@ -382,7 +382,7 @@ func (s *APIKeyService) RevokeAPIKey(params APIKeyRevocationParams) (*APIKeyRevo } // Remove the API key from memory store by name (since we have the matched key) - if err := s.store.RemoveAPIKeyByName(config.ID, apiKey.Name); err != nil { + if err := s.store.RemoveAPIKeyByID(config.ID, apiKey.ID); err != nil { logger.Error("Failed to remove API key from memory store", zap.Error(err), zap.String("handle", params.Handle), diff --git a/sdk/gateway/policy/v1alpha/api_key.go b/sdk/gateway/policy/v1alpha/api_key.go index 01aad55d2..0ecfe50bc 100644 --- a/sdk/gateway/policy/v1alpha/api_key.go +++ b/sdk/gateway/policy/v1alpha/api_key.go @@ -74,14 +74,12 @@ var ( type APIkeyStore struct { mu sync.RWMutex // Protects concurrent access // API Keys storage - apiKeys map[string]*APIKey // key: API key ID → value: APIKey apiKeysByAPI map[string]map[string]*APIKey // Key: "API ID" → Value: map[API key ID]*APIKey } // NewAPIkeyStore creates a new in-memory API key store func NewAPIkeyStore() *APIkeyStore { return &APIkeyStore{ - apiKeys: make(map[string]*APIKey), apiKeysByAPI: make(map[string]map[string]*APIKey), } } @@ -118,12 +116,11 @@ func (aks *APIkeyStore) StoreAPIKey(apiId string, apiKey *APIKey) error { if existingKeyID != "" { // Update the existing entry in apiKeysByAPI - aks.apiKeys[apiKey.ID] = apiKey aks.apiKeysByAPI[apiId][existingKeyID] = apiKey } else { // Insert new API key // Check if API key ID already exists - if _, exists := aks.apiKeys[apiKey.ID]; exists { + if _, exists := aks.apiKeysByAPI[apiId][apiKey.ID]; exists { return ErrConflict } @@ -133,7 +130,6 @@ func (aks *APIkeyStore) StoreAPIKey(apiId string, apiKey *APIKey) error { } // Store by API key value - aks.apiKeys[apiKey.ID] = apiKey aks.apiKeysByAPI[apiId][apiKey.ID] = apiKey } @@ -152,7 +148,7 @@ func (aks *APIkeyStore) ValidateAPIKey(apiId, apiOperation, operationMethod, pro var targetAPIKey *APIKey - apiKey, exists := aks.apiKeys[parsedAPIkey.ID] + apiKey, exists := aks.apiKeysByAPI[apiId][parsedAPIkey.ID] if !exists { return false, ErrNotFound } @@ -223,7 +219,7 @@ func (aks *APIkeyStore) RevokeAPIKey(apiId, providedAPIKey string) error { var matchedKey *APIKey - apiKey, exists := aks.apiKeys[parsedAPIkey.ID] + apiKey, exists := aks.apiKeysByAPI[apiId][parsedAPIkey.ID] if !exists { return nil } @@ -243,7 +239,6 @@ func (aks *APIkeyStore) RevokeAPIKey(apiId, providedAPIKey string) error { // Set status to revoked matchedKey.Status = Revoked - delete(aks.apiKeys, matchedKey.ID) aks.removeFromAPIMapping(apiKey) return nil @@ -254,16 +249,11 @@ func (aks *APIkeyStore) RemoveAPIKeysByAPI(apiId string) error { aks.mu.Lock() defer aks.mu.Unlock() - apiKeys, exists := aks.apiKeysByAPI[apiId] + _, exists := aks.apiKeysByAPI[apiId] if !exists { return nil // No keys to remove } - // Remove from main map - for _, key := range apiKeys { - delete(aks.apiKeys, key.ID) - } - // Remove from API-specific map delete(aks.apiKeysByAPI, apiId) @@ -275,9 +265,6 @@ func (aks *APIkeyStore) ClearAll() error { aks.mu.Lock() defer aks.mu.Unlock() - // Clear the main API keys map - aks.apiKeys = make(map[string]*APIKey) - // Clear the API-specific keys map aks.apiKeysByAPI = make(map[string]map[string]*APIKey)