From 393dd3e2d94e7d999b423d0265b21014a98b6ca0 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 23 Sep 2026 16:12:18 +0000 Subject: [PATCH] db: store all credentials in one SHA-256-hashed table API keys, pre-auth keys and OAuth clients/tokens share one table and verify path. Secrets carry 256 bits of crypto/rand entropy, so a SHA-256 digest needs no stretching; bcrypt/argon2id rows rehash on use until 0.32. --- AGENTS.md | 8 +- CHANGELOG.md | 28 ++ hscontrol/api/v1/apikeys.go | 1 + hscontrol/api/v1/nodes.go | 8 +- hscontrol/api/v1/preauthkeys.go | 15 +- hscontrol/api/v2/README.md | 8 +- hscontrol/auth_test.go | 10 +- hscontrol/db/api_key.go | 140 +++--- hscontrol/db/api_key_test.go | 98 +++- hscontrol/db/credential.go | 72 +++ hscontrol/db/db.go | 64 ++- hscontrol/db/db_test.go | 474 +++++++++++++++++- hscontrol/db/migrate_credentials.go | 369 ++++++++++++++ hscontrol/db/node.go | 10 +- hscontrol/db/oauth.go | 241 +++------ hscontrol/db/oauth_test.go | 176 +++++-- hscontrol/db/preauth_keys.go | 182 ++++--- hscontrol/db/preauth_keys_test.go | 195 +++---- hscontrol/db/schema.sql | 71 +-- hscontrol/db/secret.go | 216 ++++++++ .../testdata/postgres/headscale_0.29.3.pssql | Bin 0 -> 19319 bytes hscontrol/db/users_test.go | 6 +- hscontrol/mapper/tail_test.go | 2 +- hscontrol/policy/v2/types_test.go | 2 +- hscontrol/state/auth_tagged_expiry_test.go | 2 +- hscontrol/state/ephemeral_test.go | 9 +- hscontrol/state/persist_test.go | 45 ++ hscontrol/state/state.go | 19 +- hscontrol/types/api_key.go | 11 +- hscontrol/types/common.go | 4 +- hscontrol/types/config.go | 3 +- hscontrol/types/credential.go | 90 ++++ hscontrol/types/node.go | 10 +- hscontrol/types/node_tags_test.go | 14 +- hscontrol/types/oauth.go | 20 +- hscontrol/types/preauth_key.go | 13 +- hscontrol/types/types_clone.go | 56 ++- hscontrol/types/types_view.go | 147 +++++- integration/cli_apikeys_test.go | 17 +- 39 files changed, 2176 insertions(+), 680 deletions(-) create mode 100644 hscontrol/db/credential.go create mode 100644 hscontrol/db/migrate_credentials.go create mode 100644 hscontrol/db/secret.go create mode 100644 hscontrol/db/testdata/postgres/headscale_0.29.3.pssql create mode 100644 hscontrol/types/credential.go diff --git a/AGENTS.md b/AGENTS.md index c8f48115..ecbd799f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -145,7 +145,9 @@ headscale/ `NodeStore` (`node_store.go`). All cross-subsystem operations go through `State`. - `db/` — GORM layer, migrations, schema. `node.go`, `users.go`, - `api_key.go`, `preauth_keys.go`, `ip.go`, `policy.go`. + `ip.go`, `policy.go`; credentials (API keys, pre-auth keys, OAuth) share + one table: `secret.go` (generate/verify), `api_key.go`, + `preauth_keys.go`, `oauth.go`, `migrate_credentials.go`. - `mapper/` — streaming batcher that distributes MapResponses to clients: `batcher.go`, `node_conn.go`, `builder.go`, `mapper.go`. Performance-critical. @@ -196,6 +198,8 @@ migrations must: 4. **Use the migration ID format** `YYYYMMDDHHMM-short-description` (timestamp + descriptive suffix). Example: `202607241200-clear-tagged-node-expiry`. 5. **Never use `AutoMigrate`** in a migration; write explicit DDL. +6. **Run multi-statement migrations in `tx.Transaction`** so a failure + leaves the database retryable. ## Tags-as-Identity @@ -309,7 +313,7 @@ a broken one. - **Tests**: prefer `hscontrol/servertest/` for server-level tests that don't need Docker — faster than full integration tests. - **View types in read paths**: response serializers must read through - `NodeView`/`UserView`/`PreAuthKeyView` accessors. `AsStruct()` clones the + `NodeView`/`UserView`/`PreAuthKeyView`/`CredentialView` accessors. `AsStruct()` clones the whole record on every read — it is only for DB-write/merge clones and mutable working copies, never to build an API response. `grep AsStruct hscontrol/api` must come back empty. diff --git a/CHANGELOG.md b/CHANGELOG.md index 52d3623f..644ffba7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,11 +35,39 @@ tags; any other tag is rejected, for new and re-registering nodes alike. See [#3351](https://github.com/juanfont/headscale/pull/3351) +### Credentials stored in one table, hashed with SHA-256 + +API keys, pre-auth keys, OAuth clients and OAuth access tokens now live in a +single `credentials` table and are verified by one shared code path. Their +secrets are 256 bits of server-generated randomness, never user-chosen, so they +are stored as a SHA-256 digest: recovering one means searching the whole secret +space, and password stretching would only add latency to every request. + +Keys created by older releases keep working on 0.30. Support for them is +removed on this schedule: + +- **0.31** drops the upgrade from the 0.29 tables: upgrade to 0.30 first, as + the one-minor-version-at-a-time rule already requires. +- **0.32** drops bcrypt verification. A bcrypt-hashed key is rehashed to SHA-256 + the first time it authenticates, so use every API key and pre-auth key you + still need at least once on 0.30 or 0.31. Keys not used by then stop working + and must be reissued. +- **0.32** drops the legacy key formats: API keys of the form `prefix.secret` + (shown with a 7-character prefix in `headscale apikeys list`) and pre-auth + keys from before 0.28.0 (shown as `hskey-auth-legacy-…` in + `headscale preauthkeys list`). Using them does not help; reissue them before + upgrading to 0.32. + +[#3352](https://github.com/juanfont/headscale/pull/3352) + ### BREAKING #### Database - Only upgrades from 0.29.x are supported; migrations for older releases are removed and headscale refuses to start on an older database. Upgrade to the latest 0.29.x first [#3352](https://github.com/juanfont/headscale/pull/3352) +- The `pre_auth_keys`, `api_keys`, `oauth_clients` and `oauth_access_tokens` tables are merged into `credentials` and dropped. The migration cannot be reversed; take a backup before upgrading, as downgrading means restoring it [#3352](https://github.com/juanfont/headscale/pull/3352) +- API key IDs are renumbered, as all credential kinds now share one ID sequence; scripts using `headscale apikeys expire|delete --id` should look IDs up again or use `--prefix` [#3352](https://github.com/juanfont/headscale/pull/3352) +- Deleting a user now clears the owner of its API keys and OAuth clients instead of leaving a dangling reference [#3352](https://github.com/juanfont/headscale/pull/3352) #### API diff --git a/hscontrol/api/v1/apikeys.go b/hscontrol/api/v1/apikeys.go index 2411d59d..9fe50392 100644 --- a/hscontrol/api/v1/apikeys.go +++ b/hscontrol/api/v1/apikeys.go @@ -241,5 +241,6 @@ func apiKeyMaskedPrefix(prefix string) string { return "hskey-api-" + prefix + "-***" } + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). return prefix + "***" } diff --git a/hscontrol/api/v1/nodes.go b/hscontrol/api/v1/nodes.go index 5659f25d..73ea5f9a 100644 --- a/hscontrol/api/v1/nodes.go +++ b/hscontrol/api/v1/nodes.go @@ -628,12 +628,12 @@ func nodeFromView(view types.NodeView) Node { return n } -// nodePreAuthKeyFromView builds the embedded NodePreAuthKey, masking the key to -// its prefix (legacy plaintext keys are shown in full). -func nodePreAuthKeyFromView(key types.PreAuthKeyView) *NodePreAuthKey { +// nodePreAuthKeyFromView builds the embedded NodePreAuthKey from a node's +// AuthKey credential, masking the secret to its identifier prefix. +func nodePreAuthKeyFromView(key types.CredentialView) *NodePreAuthKey { pak := &NodePreAuthKey{ ID: formatID(key.ID()), - Key: maskedPreAuthKey(key), + Key: maskedPreAuthKey(key.Identifier()), Reusable: key.Reusable(), Ephemeral: key.Ephemeral(), Used: key.Used(), diff --git a/hscontrol/api/v1/preauthkeys.go b/hscontrol/api/v1/preauthkeys.go index 9afb0aa2..b28692f4 100644 --- a/hscontrol/api/v1/preauthkeys.go +++ b/hscontrol/api/v1/preauthkeys.go @@ -243,7 +243,7 @@ func preAuthKeyNewToResponse(key *types.PreAuthKeyNew) PreAuthKey { func preAuthKeyToResponse(key *types.PreAuthKey) PreAuthKey { out := PreAuthKey{ ID: formatID(key.ID), - Key: maskedPreAuthKey(key.View()), + Key: maskedPreAuthKey(key.Prefix), Reusable: key.Reusable, Ephemeral: key.Ephemeral, Used: key.Used, @@ -266,15 +266,10 @@ func preAuthKeyToResponse(key *types.PreAuthKey) PreAuthKey { return out } -// maskedPreAuthKey masks new keys (those with a stored prefix) so the secret is -// never returned; legacy plaintext keys are returned in full for backwards -// compatibility. -func maskedPreAuthKey(key types.PreAuthKeyView) string { - if key.Prefix() != "" { - return "hskey-auth-" + key.Prefix() + "-***" - } - - return key.Key() +// maskedPreAuthKey renders a pre-auth key by its public identifier so the +// secret is never returned. +func maskedPreAuthKey(identifier string) string { + return "hskey-auth-" + identifier + "-***" } // nonNilTags ensures aclTags serializes as [] rather than null, matching diff --git a/hscontrol/api/v2/README.md b/hscontrol/api/v2/README.md index c6fc68e8..84c3caf7 100644 --- a/hscontrol/api/v2/README.md +++ b/hscontrol/api/v2/README.md @@ -71,8 +71,9 @@ operator is OAuth-only. Supporting OAuth lets all of them drive Headscale. owned-by them via the policy `tagOwners` (`State.TagOwnedByTags` → `policy/v2`), so e.g. an operator token tagged `tag:k8s-operator` may mint `tag:k8s` keys. -- Credentials/tokens are stored like API keys: a public id/prefix plus an - **Argon2id** hash of the secret (no JWT, no signing keys). `OAuthClient` and +- Credentials/tokens are stored like API keys, as rows of the unified + `credentials` table: a public id/prefix plus a **SHA-256** hash of the + 256-bit secret (no JWT, no signing keys). `OAuthClient` and `OAuthAccessToken` live in `types/oauth.go` and `db/oauth.go`. ## OAuth with the tailscale client and GitHub Action @@ -130,7 +131,8 @@ go test ./hscontrol/servertest/ -run TestAPIv2` is green._ 7. **Update the CLI** only if the v2 operation fully replaces a v1 one. Tailscale has no separate key-expire verb (its `DELETE` _is_ the revoke), so v2 maps `DELETE` to a soft revoke: the key stays retrievable with `invalid: true` - until the collector reaps it (`preauth_keys.revoked_retention`), the + until the collector reaps it (`preauth_keys.revoked_retention`; keys still + backing a node are kept), the equivalent of v1 `preauthkeys expire`. `headscale preauthkeys` still stays on v1 for now (it is the cross-user admin surface), but the verb gap that previously blocked migration is closed. diff --git a/hscontrol/auth_test.go b/hscontrol/auth_test.go index 3ecf399f..5ff08980 100644 --- a/hscontrol/auth_test.go +++ b/hscontrol/auth_test.go @@ -3624,8 +3624,8 @@ func TestIssue2830_ExistingNodeReregistersWithExpiredKey(t *testing.T) { // Now expire the key by updating it in the database to have an expiry in the past. // This simulates the real-world scenario where a key expires after initial registration. pastExpiry := time.Now().Add(-1 * time.Hour) - err = app.state.DB().DB.Model(&types.PreAuthKey{}). - Where("id = ?", pak.ID). + err = app.state.DB().DB.Model(&types.Credential{}). + Where("kind = ? AND id = ?", types.CredentialPreAuthKey, pak.ID). Update("expiration", pastExpiry).Error require.NoError(t, err, "should be able to update key expiration") @@ -4066,7 +4066,7 @@ func TestDeletedPreAuthKeyNotRecreatedOnNodeUpdate(t *testing.T) { // Verify the PreAuthKey exists in the database var pakCount int64 - err = app.state.DB().DB.Model(&types.PreAuthKey{}).Where("id = ?", pakID).Count(&pakCount).Error + err = app.state.DB().DB.Model(&types.Credential{}).Where("kind = ? AND id = ?", types.CredentialPreAuthKey, pakID).Count(&pakCount).Error require.NoError(t, err) require.Equal(t, int64(1), pakCount, "PreAuthKey should exist in database") @@ -4077,7 +4077,7 @@ func TestDeletedPreAuthKeyNotRecreatedOnNodeUpdate(t *testing.T) { require.NoError(t, err, "deleting PreAuthKey should succeed") // Verify the PreAuthKey is gone from the database - err = app.state.DB().DB.Model(&types.PreAuthKey{}).Where("id = ?", pakID).Count(&pakCount).Error + err = app.state.DB().DB.Model(&types.Credential{}).Where("kind = ? AND id = ?", types.CredentialPreAuthKey, pakID).Count(&pakCount).Error require.NoError(t, err) require.Equal(t, int64(0), pakCount, "PreAuthKey should be deleted from database") t.Log("PreAuthKey deleted from database") @@ -4112,7 +4112,7 @@ func TestDeletedPreAuthKeyNotRecreatedOnNodeUpdate(t *testing.T) { t.Log("Simulated MapRequest update completed") // THE CRITICAL CHECK: Verify the PreAuthKey was NOT recreated - err = app.state.DB().DB.Model(&types.PreAuthKey{}).Where("id = ?", pakID).Count(&pakCount).Error + err = app.state.DB().DB.Model(&types.Credential{}).Where("kind = ? AND id = ?", types.CredentialPreAuthKey, pakID).Count(&pakCount).Error require.NoError(t, err) require.Equal(t, int64(0), pakCount, "BUG: PreAuthKey was recreated! The deleted PreAuthKey should NOT reappear after node update") diff --git a/hscontrol/db/api_key.go b/hscontrol/db/api_key.go index b56f1c18..519b5a2f 100644 --- a/hscontrol/db/api_key.go +++ b/hscontrol/db/api_key.go @@ -7,109 +7,110 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "golang.org/x/crypto/bcrypt" "gorm.io/gorm" - "tailscale.com/util/rands" ) const ( - apiKeyPrefix = "hskey-api-" //nolint:gosec // This is a prefix, not a credential - apiKeyPrefixLength = 12 - apiKeyHashLength = 64 + apiKeyPrefix = "hskey-api-" //nolint:gosec // This is a prefix, not a credential - // Legacy format constants. + // Legacy format constant: the prefix length of pre-hskey "prefix.secret" keys. + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). legacyAPIPrefixLength = 7 - legacyAPIKeyLength = 32 ) var ( ErrAPIKeyFailedToParse = errors.New("failed to parse ApiKey") ErrAPIKeyGenerationFailed = errors.New("failed to generate API key") ErrAPIKeyExpired = errors.New("API key expired") + ErrAPIKeyNotFound = fmt.Errorf("API key not found: %w", gorm.ErrRecordNotFound) ) // CreateAPIKey creates a new [types.APIKey] in a user, and returns it. func (hsdb *HSDatabase) CreateAPIKey( expiration *time.Time, ) (string, *types.APIKey, error) { - // Generate public prefix (12 chars) - prefix := rands.HexString(apiKeyPrefixLength) + keyStr, identifier, hash := generateSecret(apiKeyPrefix) - // Generate secret (64 chars) - secret := rands.HexString(apiKeyHashLength) - - // Full key string (shown ONCE to user) - keyStr := apiKeyPrefix + prefix + "-" + secret - - // bcrypt hash of secret - hash, err := bcrypt.GenerateFromPassword([]byte(secret), bcrypt.DefaultCost) - if err != nil { - return "", nil, err - } - - key := types.APIKey{ - Prefix: prefix, + cred := types.Credential{ + Kind: types.CredentialAPIKey, + Identifier: identifier, Hash: hash, Expiration: expiration, } - if err := hsdb.DB.Save(&key).Error; err != nil { //nolint:noinlineerr + if err := hsdb.DB.Save(&cred).Error; err != nil { //nolint:noinlineerr return "", nil, fmt.Errorf("saving API key to database: %w", err) } - return keyStr, &key, nil + return keyStr, credentialToAPIKey(&cred), nil } // ListAPIKeys returns the list of [types.APIKey] values for a user. func (hsdb *HSDatabase) ListAPIKeys() ([]types.APIKey, error) { - keys := []types.APIKey{} + var creds []types.Credential - err := hsdb.DB.Find(&keys).Error + err := hsdb.DB.Where("kind = ?", types.CredentialAPIKey).Order("id").Find(&creds).Error if err != nil { return nil, err } + keys := make([]types.APIKey, 0, len(creds)) + for i := range creds { + keys = append(keys, *credentialToAPIKey(&creds[i])) + } + return keys, nil } // GetAPIKey returns a [types.APIKey] for a given key. func (hsdb *HSDatabase) GetAPIKey(prefix string) (*types.APIKey, error) { - key := types.APIKey{} - if result := hsdb.DB.First(&key, "prefix = ?", prefix); result.Error != nil { + var cred types.Credential + if result := hsdb.DB.First(&cred, "kind = ? AND identifier = ?", types.CredentialAPIKey, prefix); result.Error != nil { return nil, result.Error } - return &key, nil + return credentialToAPIKey(&cred), nil } // GetAPIKeyByID returns a [types.APIKey] for a given id. func (hsdb *HSDatabase) GetAPIKeyByID(id uint64) (*types.APIKey, error) { - key := types.APIKey{} + var cred types.Credential // Query on an explicit primary-key clause: a struct condition would drop a // zero-valued ID, making the lookup unconditional and returning the first // row instead of not-found. - if result := hsdb.DB.First(&key, "id = ?", id); result.Error != nil { + if result := hsdb.DB.First(&cred, "kind = ? AND id = ?", types.CredentialAPIKey, id); result.Error != nil { return nil, result.Error } - return &key, nil + return credentialToAPIKey(&cred), nil } -// DestroyAPIKey destroys a [types.APIKey]. Returns error if the [types.APIKey] -// does not exist. +// DestroyAPIKey destroys a [types.APIKey]. Returns [ErrAPIKeyNotFound] if the +// [types.APIKey] does not exist. func (hsdb *HSDatabase) DestroyAPIKey(key types.APIKey) error { - if result := hsdb.DB.Unscoped().Delete(key); result.Error != nil { - return result.Error - } + res := hsdb.DB.Unscoped(). + Delete(&types.Credential{}, "kind = ? AND id = ?", types.CredentialAPIKey, key.ID) - return nil + return apiKeyAffected(res) } // ExpireAPIKey marks a [types.APIKey] as expired. func (hsdb *HSDatabase) ExpireAPIKey(key *types.APIKey) error { - err := hsdb.DB.Model(&key).Update("Expiration", time.Now()).Error - if err != nil { - return err + res := hsdb.DB.Model(&types.Credential{}). + Where("kind = ? AND id = ?", types.CredentialAPIKey, key.ID). + Update("expiration", time.Now()) + + return apiKeyAffected(res) +} + +// apiKeyAffected maps a write that matched no API key row to [ErrAPIKeyNotFound]. +func apiKeyAffected(res *gorm.DB) error { + if res.Error != nil { + return res.Error + } + + if res.RowsAffected == 0 { + return ErrAPIKeyNotFound } return nil @@ -148,9 +149,11 @@ func (hsdb *HSDatabase) AuthenticateAPIKey(keyStr string) (*types.APIKey, error) // SetAPIKeyUser sets the owning user of an API key. Used when an admin mints a // key on behalf of a user (headscale apikeys create --user). func (hsdb *HSDatabase) SetAPIKeyUser(keyID uint64, userID types.UserID) error { - return hsdb.DB.Model(&types.APIKey{}). - Where("id = ?", keyID). - Update("user_id", uint(userID)).Error + res := hsdb.DB.Model(&types.Credential{}). + Where("kind = ? AND id = ?", types.CredentialAPIKey, keyID). + Update("user_id", uint(userID)) + + return apiKeyAffected(res) } // ParseAPIKeyPrefix extracts the database prefix from a display prefix. @@ -158,7 +161,7 @@ func (hsdb *HSDatabase) SetAPIKeyUser(keyID uint64, userID types.UserID) error { // Returns the 12-character prefix suitable for database lookup. func ParseAPIKeyPrefix(displayPrefix string) (string, error) { // If it's already just the 12-character prefix, return it - if len(displayPrefix) == apiKeyPrefixLength && isValidBase64URLSafe(displayPrefix) { + if len(displayPrefix) == keyIdentifierLength && isValidBase64URLSafe(displayPrefix) { return displayPrefix, nil } @@ -171,11 +174,11 @@ func ParseAPIKeyPrefix(displayPrefix string) (string, error) { } // Extract just the first 12 characters (the actual prefix) - if len(remainder) < apiKeyPrefixLength { + if len(remainder) < keyIdentifierLength { return "", fmt.Errorf("%w: prefix too short", ErrAPIKeyFailedToParse) } - prefix := remainder[:apiKeyPrefixLength] + prefix := remainder[:keyIdentifierLength] // Validate it's base64 URL-safe if !isValidBase64URLSafe(prefix) { @@ -186,6 +189,7 @@ func ParseAPIKeyPrefix(displayPrefix string) (string, error) { } // For legacy 7-character prefixes or other formats, return as-is + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). return displayPrefix, nil } @@ -202,38 +206,26 @@ func validateAPIKey(db *gorm.DB, keyStr string) (*types.APIKey, error) { if !found { // Legacy format: prefix.secret + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). return validateLegacyAPIKey(db, keyStr) } - // New format: parse and verify - prefix, secret, err := parsePrefixedKey( - prefixAndSecret, - apiKeyPrefixLength, - apiKeyHashLength, - ErrAPIKeyFailedToParse, - ) + prefix, secret, err := parsePrefixedKey(prefixAndSecret, ErrAPIKeyFailedToParse) if err != nil { return nil, err } - // Look up by prefix (indexed) - var key types.APIKey - - err = db.First(&key, "prefix = ?", prefix).Error + cred, err := authenticateCredential(db, types.CredentialAPIKey, prefix, secret, ErrAPIKeyNotFound) if err != nil { - return nil, fmt.Errorf("API key not found: %w", err) + return nil, err } - // Verify bcrypt hash - err = bcrypt.CompareHashAndPassword(key.Hash, []byte(secret)) - if err != nil { - return nil, fmt.Errorf("invalid API key: %w", err) - } - - return &key, nil + return credentialToAPIKey(cred), nil } // validateLegacyAPIKey validates a legacy format API key (prefix.secret). +// +// TODO(kradalby): remove in 0.32 with legacy key formats (announced). func validateLegacyAPIKey(db *gorm.DB, keyStr string) (*types.APIKey, error) { // Legacy format uses "." as separator prefix, secret, found := strings.Cut(keyStr, ".") @@ -246,18 +238,10 @@ func validateLegacyAPIKey(db *gorm.DB, keyStr string) (*types.APIKey, error) { return nil, fmt.Errorf("%w: legacy prefix length mismatch", ErrAPIKeyFailedToParse) } - var key types.APIKey - - err := db.First(&key, "prefix = ?", prefix).Error + cred, err := authenticateCredential(db, types.CredentialAPIKey, prefix, secret, ErrAPIKeyNotFound) if err != nil { - return nil, fmt.Errorf("API key not found: %w", err) + return nil, err } - // Verify bcrypt (key.Hash stores bcrypt of full secret) - err = bcrypt.CompareHashAndPassword(key.Hash, []byte(secret)) - if err != nil { - return nil, fmt.Errorf("invalid API key: %w", err) - } - - return &key, nil + return credentialToAPIKey(cred), nil } diff --git a/hscontrol/db/api_key_test.go b/hscontrol/db/api_key_test.go index a34dd94b..8094a812 100644 --- a/hscontrol/db/api_key_test.go +++ b/hscontrol/db/api_key_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/crypto/bcrypt" + "gorm.io/gorm" ) func TestCreateAPIKey(t *testing.T) { @@ -177,6 +178,7 @@ func TestAPIKeyWithPrefix(t *testing.T) { }, }, { + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). name: "legacy_key_still_works", test: func(t *testing.T, db *HSDatabase) { t.Helper() @@ -185,17 +187,27 @@ func TestAPIKeyWithPrefix(t *testing.T) { legacyPrefix := "abcdefg" legacySecret := strings.Repeat("x", 32) legacyKey := legacyPrefix + "." + legacySecret - hash, err := bcrypt.GenerateFromPassword([]byte(legacySecret), bcrypt.DefaultCost) + hash, err := bcrypt.GenerateFromPassword([]byte(legacySecret), bcrypt.MinCost) require.NoError(t, err) now := time.Now() err = db.DB.Exec(` - INSERT INTO api_keys (prefix, hash, created_at) - VALUES (?, ?, ?) - `, legacyPrefix, hash, now).Error + INSERT INTO credentials (kind, identifier, hash, created_at) + VALUES (?, ?, ?, ?) + `, types.CredentialAPIKey, legacyPrefix, hash, now).Error require.NoError(t, err) - // Validate legacy key + // Validate legacy key through the v2 path, which must also + // upgrade the stored hash. + key, err := db.AuthenticateAPIKey(legacyKey) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(string(key.Hash), hashPrefixSHA256)) + + stored, err := db.GetAPIKey(legacyPrefix) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(string(stored.Hash), hashPrefixSHA256), + "legacy key must be rehashed on first auth") + valid, err := db.ValidateAPIKey(legacyKey) require.NoError(t, err) assert.True(t, valid) @@ -273,3 +285,79 @@ func TestGetAPIKeyByIDNotFound(t *testing.T) { require.Error(t, err) assert.Nil(t, key) } + +// TestAPIKeyLazyRehashesBcrypt seeds a new-format key whose secret is stored as +// a legacy bcrypt hash and asserts that authenticating it upgrades the stored +// hash to SHA-256, while continuing to authenticate. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. +func TestAPIKeyLazyRehashesBcrypt(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + prefix := "abcdefghijkl" + secret := strings.Repeat("a", 64) + keyStr := "hskey-api-" + prefix + "-" + secret + + hash, err := bcrypt.GenerateFromPassword([]byte(secret), bcrypt.MinCost) + require.NoError(t, err) + + err = db.DB.Exec( + `INSERT INTO credentials (kind, identifier, hash, created_at) VALUES (?, ?, ?, ?)`, + types.CredentialAPIKey, prefix, hash, time.Now(), + ).Error + require.NoError(t, err) + + valid, err := db.ValidateAPIKey(keyStr) + require.NoError(t, err) + assert.True(t, valid) + + stored, err := db.GetAPIKey(prefix) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(string(stored.Hash), hashPrefixSHA256), + "a bcrypt-stored key must be rehashed to SHA-256 on first auth") + + valid, err = db.ValidateAPIKey(keyStr) + require.NoError(t, err) + assert.True(t, valid, "key must still authenticate against the upgraded hash") +} + +// TestCredentialKindsAreIsolated asserts that ids and identifiers, which now +// share one table, never let an operation or secret of one kind reach another. +func TestCredentialKindsAreIsolated(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + apiKeyStr, apiKey, err := db.CreateAPIKey(nil) + require.NoError(t, err) + + pakNew, err := db.CreatePreAuthKey(nil, true, false, nil, []string{"tag:x"}) + require.NoError(t, err) + + // A pre-auth key id is not an API key and vice versa. + _, err = db.GetAPIKeyByID(pakNew.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + require.ErrorIs(t, db.DestroyAPIKey(types.APIKey{ID: pakNew.ID}), ErrAPIKeyNotFound) + require.ErrorIs(t, db.ExpireAPIKey(&types.APIKey{ID: pakNew.ID}), ErrAPIKeyNotFound) + require.ErrorIs(t, db.SetAPIKeyUser(pakNew.ID, 1), ErrAPIKeyNotFound) + + _, err = db.GetPreAuthKeyByID(apiKey.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + require.ErrorIs(t, db.SetPreAuthKeyDescription(apiKey.ID, "x"), ErrPreAuthKeyNotFound) + + // Both rows survived the cross-kind attempts. + _, err = db.GetPreAuthKeyByID(pakNew.ID) + require.NoError(t, err) + + _, err = db.GetAPIKeyByID(apiKey.ID) + require.NoError(t, err) + + // A secret presented under another kind's prefix is rejected. + _, apiRest, _ := strings.Cut(apiKeyStr, apiKeyPrefix) + _, err = db.GetPreAuthKey(authKeyPrefix + apiRest) + require.ErrorIs(t, err, ErrPreAuthKeyNotFound) + + _, pakRest, _ := strings.Cut(pakNew.Key, authKeyPrefix) + _, err = db.AuthenticateAPIKey(apiKeyPrefix + pakRest) + require.ErrorIs(t, err, ErrAPIKeyNotFound) +} diff --git a/hscontrol/db/credential.go b/hscontrol/db/credential.go new file mode 100644 index 00000000..95a31712 --- /dev/null +++ b/hscontrol/db/credential.go @@ -0,0 +1,72 @@ +package db + +import ( + "github.com/juanfont/headscale/hscontrol/types" +) + +// credentialToAPIKey projects a unified credentials row onto the [types.APIKey] +// shape the API, state, and CLI layers consume. +func credentialToAPIKey(c *types.Credential) *types.APIKey { + return &types.APIKey{ + ID: c.ID, + Prefix: c.Identifier, + Hash: c.Hash, + UserID: c.UserID, + CreatedAt: c.CreatedAt, + Expiration: c.Expiration, + LastSeen: c.LastSeen, + } +} + +// credentialToOAuthClient projects a unified credentials row onto the +// [types.OAuthClient] shape. The client id is stored as the row's identifier. +func credentialToOAuthClient(c *types.Credential) *types.OAuthClient { + return &types.OAuthClient{ + ID: c.ID, + ClientID: c.Identifier, + SecretHash: c.Hash, + Scopes: c.Scopes, + Tags: c.Tags, + Description: c.Description, + UserID: c.UserID, + CreatedAt: c.CreatedAt, + Revoked: c.Revoked, + } +} + +// credentialToPreAuthKey projects a unified credentials row onto the +// [types.PreAuthKey] shape. The lookup prefix is stored as the row's identifier; +// the User association is carried through when preloaded. +func credentialToPreAuthKey(c *types.Credential) *types.PreAuthKey { + return &types.PreAuthKey{ + ID: c.ID, + Prefix: c.Identifier, + Hash: c.Hash, + UserID: c.UserID, + User: c.User, + Description: c.Description, + Reusable: c.Reusable, + Ephemeral: c.Ephemeral, + Used: c.Used, + Tags: c.Tags, + CreatedAt: c.CreatedAt, + Expiration: c.Expiration, + Revoked: c.Revoked, + } +} + +// credentialToOAuthAccessToken projects a unified credentials row onto the +// [types.OAuthAccessToken] shape. The token's lookup prefix is stored as the +// row's identifier; ClientID links back to the issuing client's identifier. +func credentialToOAuthAccessToken(c *types.Credential) *types.OAuthAccessToken { + return &types.OAuthAccessToken{ + ID: c.ID, + Prefix: c.Identifier, + Hash: c.Hash, + ClientID: c.ClientID, + Scopes: c.Scopes, + Tags: c.Tags, + Expiration: c.Expiration, + CreatedAt: c.CreatedAt, + } +} diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index 96da55b3..1bd8dd01 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -76,6 +76,8 @@ func NewHeadscaleDatabase(cfg *types.Config) (*HSDatabase, error) { // - AutoMigrate depends on the struct staying exactly the same, which it won't over time. // - Never write migrations that requires foreign keys to be disabled. // - ALL errors in migrations must be handled properly. + // Shipped in 0.29.1. + // TODO(kradalby): remove in 0.31, which upgrades only from 0.30. { // Recover user_id on untagged nodes detached by the earlier // version of 202602201200-clear-tagged-node-user-id, which @@ -106,6 +108,8 @@ WHERE user_id IS NULL }, Rollback: func(db *gorm.DB) error { return nil }, }, + // 0.30 development: columns and tables that 202609231300 reads. + // TODO(kradalby): remove in 0.31 with the credentials migration. { // Add an optional owning user to API keys so the v2 API can // create user-owned (untagged) auth keys, mirroring Tailscale's @@ -225,6 +229,8 @@ WHERE user_id IS NULL }, Rollback: func(db *gorm.DB) error { return nil }, }, + // Shipped in 0.29.3. + // TODO(kradalby): remove in 0.31, which upgrades only from 0.30. { // Clear stale key expiry on tagged nodes. A tagged node is // owned by its tags and never expires (KB 1068), but a buggy @@ -252,20 +258,48 @@ WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null' }, Rollback: func(db *gorm.DB) error { return nil }, }, + // 0.30: unified credentials table (InitSchema keeps ensureCredentialsTable). + // TODO(kradalby): remove in 0.31 with the credentials migration. + { + // Create the unified credentials table; the next migration + // backfills it. Explicit DDL for both dialects (no AutoMigrate). + ID: "202609231200-create-credentials", + Migrate: ensureCredentialsTable, + Rollback: func(db *gorm.DB) error { return nil }, + }, + { + // Move every credential into the unified table and drop the + // per-kind tables (see migrateToCredentials). + ID: "202609231300-migrate-to-credentials", + Migrate: func(tx *gorm.DB) error { + // Already migrated (e.g. fresh DB via InitSchema): nothing to do. + if !tx.Migrator().HasTable("pre_auth_keys") && + !tx.Migrator().HasTable("api_keys") { + return nil + } + + return tx.Transaction(migrateToCredentials) + }, + Rollback: func(db *gorm.DB) error { return nil }, + }, }, ) migrations.InitSchema(func(tx *gorm.DB) error { - // Create all tables using AutoMigrate - err := tx.AutoMigrate( - &types.User{}, - &types.PreAuthKey{}, - &types.APIKey{}, - &types.Node{}, - &types.Policy{}, - &types.OAuthClient{}, - &types.OAuthAccessToken{}, - ) + // Credentials use the migration's explicit DDL (AutoMigrate cannot + // express its CHECK constraints), created before Node so the + // nodes.auth_key_id foreign key to credentials(id) can be created. + err := tx.AutoMigrate(&types.User{}) + if err != nil { + return err + } + + err = ensureCredentialsTable(tx) + if err != nil { + return err + } + + err = tx.AutoMigrate(&types.Node{}, &types.Policy{}) if err != nil { return err } @@ -274,14 +308,11 @@ WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null' // to ensure we can recreate them in the correct format dropIndexes := []string{ `DROP INDEX IF EXISTS "idx_users_deleted_at"`, - `DROP INDEX IF EXISTS "idx_api_keys_prefix"`, `DROP INDEX IF EXISTS "idx_policies_deleted_at"`, `DROP INDEX IF EXISTS "idx_provider_identifier"`, `DROP INDEX IF EXISTS "idx_name_provider_identifier"`, `DROP INDEX IF EXISTS "idx_name_no_provider_identifier"`, - `DROP INDEX IF EXISTS "idx_pre_auth_keys_prefix"`, - `DROP INDEX IF EXISTS "idx_oauth_clients_client_id"`, - `DROP INDEX IF EXISTS "idx_oauth_access_tokens_prefix"`, + `DROP INDEX IF EXISTS "idx_nodes_auth_key_id"`, } for _, dropSQL := range dropIndexes { @@ -294,14 +325,11 @@ WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null' // Recreate indexes without backticks to match schema.sql format indexes := []string{ `CREATE INDEX idx_users_deleted_at ON users(deleted_at)`, - `CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix)`, `CREATE INDEX idx_policies_deleted_at ON policies(deleted_at)`, `CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE provider_identifier IS NOT NULL`, `CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier)`, `CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL`, - `CREATE UNIQUE INDEX idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != ''`, - `CREATE UNIQUE INDEX idx_oauth_clients_client_id ON oauth_clients(client_id)`, - `CREATE UNIQUE INDEX idx_oauth_access_tokens_prefix ON oauth_access_tokens(prefix)`, + `CREATE INDEX idx_nodes_auth_key_id ON nodes(auth_key_id)`, } for _, indexSQL := range indexes { diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go index 89d6be06..fb4e7b97 100644 --- a/hscontrol/db/db_test.go +++ b/hscontrol/db/db_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/juanfont/headscale/hscontrol/types" "github.com/stretchr/testify/assert" @@ -15,6 +16,12 @@ import ( "gorm.io/gorm" ) +// legacyPlaintextKey is the pre-0.28 plaintext pre-auth key (id 5) seeded in +// both 0.29.3 fixtures. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +const legacyPlaintextKey = "plaintextlegacykey0000000000000000000000000000" + // TestSQLiteMigrationAndDataValidation tests specific SQLite migration scenarios // and validates data integrity after migration. All migrations that require data validation // should be added here. @@ -23,6 +30,91 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) { dbPath string wantFunc func(*testing.T, *HSDatabase) }{ + // TODO(kradalby): remove in 0.31 with the credentials migration. + // Real v0.29.3 database: the supported upgrade path into the unified + // credentials table. Key strings are listed in the fixture header. + { + dbPath: "testdata/sqlite/headscale_0.29.3_dump.sql", + wantFunc: func(t *testing.T, hsdb *HSDatabase) { + t.Helper() + + for _, table := range []string{"pre_auth_keys", "api_keys", "oauth_clients", "oauth_access_tokens"} { + assert.False(t, hsdb.DB.Migrator().HasTable(table), "%s must be dropped", table) + } + + nodes, err := Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) { + return ListNodes(rx) + }) + require.NoError(t, err) + require.Len(t, nodes, 5) + + // Pre-auth key ids are preserved, so every node keeps its key. + for _, n := range nodes { + require.NotNil(t, n.AuthKeyID, "node %d", n.ID) + assert.Equal(t, uint64(n.ID), *n.AuthKeyID, "node %d", n.ID) + require.NotNil(t, n.AuthKey, "node %d", n.ID) + assert.Equal(t, types.CredentialPreAuthKey, n.AuthKey.Kind) + } + + used, err := hsdb.GetPreAuthKeyByID(2) + require.NoError(t, err) + assert.True(t, used.Used) + assert.False(t, used.Reusable) + + tagged, err := hsdb.GetPreAuthKeyByID(4) + require.NoError(t, err) + assert.Equal(t, []string{"tag:server"}, tagged.Tags) + assert.Nil(t, tagged.UserID) + require.NotNil(t, tagged.Expiration) + + // The legacy plaintext key is hashed, keeps authenticating + // and still backs its ephemeral node. + legacy, err := hsdb.GetPreAuthKey(legacyPlaintextKey) + require.NoError(t, err) + assert.Equal(t, uint64(5), legacy.ID) + assert.Equal(t, legacyAuthKeyIdentifier(legacyPlaintextKey), legacy.Prefix) + assert.Equal(t, hashSecret(legacyPlaintextKey), legacy.Hash) + assert.Nil(t, legacy.Revoked) + require.NoError(t, legacy.Validate()) + + ephemeral, err := hsdb.ListEphemeralNodes() + require.NoError(t, err) + + ephemeralIDs := make([]types.NodeID, 0, len(ephemeral)) + for _, n := range ephemeral { + ephemeralIDs = append(ephemeralIDs, n.ID) + } + + assert.ElementsMatch(t, []types.NodeID{3, 5}, ephemeralIDs) + + // The SQLite nodes rebuild keeps the id counter past the + // deleted node 6. + var seq int64 + require.NoError(t, hsdb.DB.Raw(`SELECT seq FROM sqlite_sequence WHERE name = 'nodes'`).Scan(&seq).Error) + assert.Equal(t, int64(6), seq) + + // bcrypt keys authenticate and are upgraded to SHA-256. + apiKey, err := hsdb.AuthenticateAPIKey( + "hskey-api-ZRVzG0vKkUb4-dqem7jxt7Aun0JqfZpbsvrBDYdQV-RK8S9qbiAAniiuTxIj73LeDUDukVYJBqmDh") + require.NoError(t, err) + assert.Greater(t, apiKey.ID, uint64(5), "API keys are renumbered after pre-auth keys") + + pak, err := hsdb.GetPreAuthKey( + "hskey-auth-H3XVw1W-6s4J-KTmfCUFG_4gJ8CuI5j3W67DX8eMnQJi6W8ToVK0esXMrPK5YTm_p8THq6VnH22-K") + require.NoError(t, err) + assert.Equal(t, uint64(1), pak.ID) + + stored, err := hsdb.GetPreAuthKeyByID(1) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(string(stored.Hash), hashPrefixSHA256)) + + // New credentials never collide with migrated ids. + _, newKey, err := hsdb.CreateAPIKey(nil) + require.NoError(t, err) + assert.Greater(t, newKey.ID, apiKey.ID) + }, + }, + // TODO(kradalby): remove in 0.31 with the 0.29.x migrations. // Test for the null-tags user_id recovery migration. Databases that // already upgraded to 0.29.0 had user_id wrongly cleared on untagged // nodes with tags='null'. The recovery migration re-derives user_id @@ -69,6 +161,7 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) { assert.Equal(t, uint(1), *node4.UserID, "node4 should still belong to user1") }, }, + // TODO(kradalby): remove in 0.31 with the 0.29.x migrations. // Test for the clear-tagged-node-expiry migration // (202607241200-clear-tagged-node-expiry). A buggy handleLogout stamped // a key expiry on tagged nodes, which never expire (KB 1068), leaving @@ -287,8 +380,64 @@ func TestPostgresMigrationAndDataValidation(t *testing.T) { tests := []struct { name string dbPath string + preSQL []string // run after restore, before migrating wantFunc func(*testing.T, *HSDatabase) - }{} + }{ + // TODO(kradalby): remove in 0.31 with the credentials migration. + // Real v0.29.3 Postgres database (pg_dump -Fc), same shape as the + // SQLite 0.29.3 fixture: exercises the explicit-id backfill, the + // sequence reset and the in-place foreign key swap. + { + name: "0.29.3", + dbPath: "testdata/postgres/headscale_0.29.3.pssql", + // Keys 6-9 were created and deleted before the upgrade. + preSQL: []string{`SELECT setval('pre_auth_keys_id_seq', 9)`}, + wantFunc: func(t *testing.T, hsdb *HSDatabase) { + t.Helper() + + nodes, err := Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) { + return ListNodes(rx) + }) + require.NoError(t, err) + require.Len(t, nodes, 5) + + for _, n := range nodes { + require.NotNil(t, n.AuthKey, "node %d", n.ID) + assert.Equal(t, uint64(n.ID), n.AuthKey.ID, "node %d", n.ID) + } + + legacy, err := hsdb.GetPreAuthKey(legacyPlaintextKey) + require.NoError(t, err) + assert.Equal(t, uint64(5), legacy.ID) + require.NoError(t, legacy.Validate()) + + ephemeral, err := hsdb.ListEphemeralNodes() + require.NoError(t, err) + assert.Len(t, ephemeral, 2) + + apiKey, err := hsdb.AuthenticateAPIKey( + "hskey-api-SdBE2-ozHMyK-HjrXcK0p7TYzcbylruVbFWlyt6HjcHwo7x_GLPLhAalGRfqet4IrZU-q91oeYeEN") + require.NoError(t, err) + assert.Greater(t, apiKey.ID, uint64(5)) + + _, err = hsdb.GetPreAuthKey( + "hskey-auth-RsYnjtPRQ36o-diMamZSeVhqfNiuKCp8sogtHIbfYfAkZ9iQm7e4naK-Wm2uUK-kHd56_IKGQQNTo") + require.NoError(t, err) + + // The sequence was advanced past the explicit pre-auth ids. + _, newKey, err := hsdb.CreateAPIKey(nil) + require.NoError(t, err) + assert.Greater(t, newKey.ID, apiKey.ID) + + requireCredentialConstraints(t, hsdb.DB) + + // Ids of keys deleted before the upgrade are not reused. + pak, err := hsdb.CreatePreAuthKey(nil, false, false, nil, []string{"tag:x"}) + require.NoError(t, err) + assert.Greater(t, pak.ID, uint64(9)) + }, + }, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -312,6 +461,18 @@ func TestPostgresMigrationAndDataValidation(t *testing.T) { t.Fatalf("failed to restore postgres database: %s", err) } + if len(tt.preSQL) > 0 { + raw, err := sql.Open("pgx", u.String()) + require.NoError(t, err) + + for _, stmt := range tt.preSQL { + _, err := raw.ExecContext(context.Background(), stmt) + require.NoError(t, err, stmt) + } + + require.NoError(t, raw.Close()) + } + db := newHeadscaleDBFromPostgresURL(t, u) if tt.wantFunc != nil { @@ -370,6 +531,106 @@ func dbForTestWithPath(t *testing.T, sqlFilePath string) *HSDatabase { return db } +// TestSQLiteMigrationDanglingCredentialOwner covers API keys and OAuth clients +// whose user was deleted: their user_id never had a foreign key, so the +// backfill into credentials must null it instead of failing the upgrade. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func TestSQLiteMigrationDanglingCredentialOwner(t *testing.T) { + dbPath := t.TempDir() + "/headscale_test.db" + + require.NoError(t, createSQLiteFromSQLFile("testdata/sqlite/headscale_0.29.3_dump.sql", dbPath)) + + // Bring the dump to the pre-credentials development schema by hand, with + // rows owned by a user id that does not exist. + raw, err := sql.Open("sqlite", dbPath) + require.NoError(t, err) + + for _, stmt := range []string{ + `ALTER TABLE api_keys ADD COLUMN user_id integer`, + `UPDATE api_keys SET user_id = 99`, + `ALTER TABLE pre_auth_keys ADD COLUMN description text`, + `ALTER TABLE pre_auth_keys ADD COLUMN revoked datetime`, + `CREATE TABLE oauth_clients(id integer PRIMARY KEY AUTOINCREMENT, client_id text, secret_hash blob, scopes text, tags text, description text, user_id integer, created_at datetime, revoked datetime)`, + `CREATE UNIQUE INDEX idx_oauth_clients_client_id ON oauth_clients(client_id)`, + `INSERT INTO oauth_clients(client_id, secret_hash, scopes, tags, user_id, created_at) VALUES('client000001', '$sha256$00', '[]', '["tag:ci"]', 99, '2026-01-01 00:00:00')`, + `CREATE TABLE oauth_access_tokens(id integer PRIMARY KEY AUTOINCREMENT, prefix text, hash blob, client_id text, scopes text, tags text, expiration datetime, created_at datetime)`, + `CREATE UNIQUE INDEX idx_oauth_access_tokens_prefix ON oauth_access_tokens(prefix)`, + `INSERT OR IGNORE INTO migrations VALUES('202606181200-recover-null-tags-node-user-id'), ('202606191500-api-key-user-id'), ('202606191501-pre-auth-key-description'), ('202606201200-pre-auth-key-revoked'), ('202606211200-oauth-clients-and-tokens'), ('202607241200-clear-tagged-node-expiry')`, + } { + _, err := raw.ExecContext(context.Background(), stmt) + require.NoError(t, err, stmt) + } + + require.NoError(t, raw.Close()) + + hsdb, err := NewHeadscaleDatabase(&types.Config{ + Database: types.DatabaseConfig{ + Type: "sqlite3", + Sqlite: types.SqliteConfig{Path: dbPath}, + }, + Policy: types.PolicyConfig{Mode: types.PolicyModeDB}, + }) + require.NoError(t, err) + + keys, err := hsdb.ListAPIKeys() + require.NoError(t, err) + require.Len(t, keys, 1) + assert.Nil(t, keys[0].UserID) + + client, err := hsdb.GetOAuthClientByClientID("client000001") + require.NoError(t, err) + assert.Nil(t, client.UserID) +} + +// TestSQLiteMigrationToCredentialsIsAtomic fails the credentials migration at +// its last DDL step and asserts nothing was committed, so a retry succeeds. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func TestSQLiteMigrationToCredentialsIsAtomic(t *testing.T) { + dbPath := t.TempDir() + "/headscale_test.db" + + require.NoError(t, createSQLiteFromSQLFile("testdata/sqlite/headscale_0.29.3_dump.sql", dbPath)) + + raw, err := sql.Open("sqlite", dbPath) + require.NoError(t, err) + + defer raw.Close() + + // An index of the same name makes the migration's final CREATE INDEX fail + // after the backfill and the nodes rebuild have run. + _, err = raw.ExecContext(context.Background(), `CREATE INDEX idx_nodes_auth_key_id ON users(id)`) + require.NoError(t, err) + + cfg := &types.Config{ + Database: types.DatabaseConfig{ + Type: "sqlite3", + Sqlite: types.SqliteConfig{Path: dbPath}, + }, + Policy: types.PolicyConfig{Mode: types.PolicyModeDB}, + } + + _, err = NewHeadscaleDatabase(cfg) + require.Error(t, err) + + var paks, nodes int + + require.NoError(t, raw.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM pre_auth_keys`).Scan(&paks)) + require.NoError(t, raw.QueryRowContext(context.Background(), `SELECT COUNT(*) FROM nodes`).Scan(&nodes)) + assert.Equal(t, 5, paks, "pre_auth_keys must be untouched") + assert.Equal(t, 5, nodes, "nodes must be untouched") + + _, err = raw.ExecContext(context.Background(), `DROP INDEX idx_nodes_auth_key_id`) + require.NoError(t, err) + + hsdb, err := NewHeadscaleDatabase(cfg) + require.NoError(t, err) + + pak, err := hsdb.GetPreAuthKeyByID(1) + require.NoError(t, err) + assert.True(t, pak.Reusable) +} + // TestSQLiteRejectsPre029Database ensures a real pre-0.29 database is refused // rather than silently skipping the migrations that were removed. func TestSQLiteRejectsPre029Database(t *testing.T) { @@ -438,3 +699,214 @@ func TestSQLiteAllTestdataMigrations(t *testing.T) { }) } } + +// TestCredentialTableRoundTrip confirms the unified credentials table is created +// by migration (newSQLiteTestDB validates the schema with squibble) and stores +// and reads back a credential of each kind. +func TestCredentialTableRoundTrip(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + now := time.Now().UTC() + creds := []types.Credential{ + {Kind: types.CredentialAPIKey, Identifier: "apikey000001", Hash: []byte("$h1"), CreatedAt: &now}, + {Kind: types.CredentialPreAuthKey, Identifier: "authkey00001", Hash: []byte("$h2"), Reusable: true, Tags: []string{"tag:a"}, CreatedAt: &now}, + {Kind: types.CredentialOAuthClient, Identifier: "client000001", Hash: []byte("$h3"), Scopes: []string{"devices:read"}, CreatedAt: &now}, + {Kind: types.CredentialOAuthToken, Identifier: "oauthtok0001", Hash: []byte("$h4"), ClientID: "client000001", CreatedAt: &now}, + } + + for i := range creds { + require.NoError(t, db.DB.Save(&creds[i]).Error) + } + + var got []types.Credential + require.NoError(t, db.DB.Order("id").Find(&got).Error) + require.Len(t, got, 4) + assert.Equal(t, types.CredentialPreAuthKey, got[1].Kind) + assert.Equal(t, []string{"tag:a"}, got[1].Tags) + assert.Equal(t, "client000001", got[3].ClientID) + + // The composite (kind, identifier) index permits the same identifier under a + // different kind but rejects a duplicate within a kind. + require.NoError(t, db.DB.Save(&types.Credential{ + Kind: types.CredentialAPIKey, Identifier: "client000001", Hash: []byte("$h5"), CreatedAt: &now, + }).Error) + + err = db.DB.Save(&types.Credential{ + Kind: types.CredentialAPIKey, Identifier: "apikey000001", Hash: []byte("$dup"), CreatedAt: &now, + }).Error + require.Error(t, err, "duplicate (kind, identifier) must be rejected") +} + +// requireCredentialConstraints asserts the credentials table rejects rows the +// application must never write: an unknown kind, a hash in no known format, +// and a missing hash on a usable (unrevoked) row. +func requireCredentialConstraints(t *testing.T, db *gorm.DB) { + t.Helper() + + now := time.Now().UTC() + + bad := map[string]types.Credential{ + "unknown kind": {Kind: "bogus", Identifier: "chk000000001", Hash: []byte("$sha256$00")}, + "hash format": {Kind: types.CredentialAPIKey, Identifier: "chk000000002", Hash: []byte("plain")}, + "missing hash usable": {Kind: types.CredentialAPIKey, Identifier: "chk000000003"}, + } + for name, cred := range bad { + require.Error(t, db.Create(&cred).Error, name) + } + + // Only the migration writes hashless rows, as NULL. + require.NoError(t, db.Exec( + `INSERT INTO credentials (kind, identifier, hash, revoked) VALUES (?, ?, NULL, ?)`, + types.CredentialPreAuthKey, "chk000000004", now, + ).Error, "a revoked row may lack a hash") +} + +func TestCredentialConstraints(t *testing.T) { + t.Run("fresh sqlite", func(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + requireCredentialConstraints(t, db.DB) + }) + + // TODO(kradalby): remove in 0.31 with the credentials migration. + t.Run("migrated sqlite", func(t *testing.T) { + hsdb := dbForTestWithPath(t, "testdata/sqlite/headscale_0.29.3_dump.sql") + requireCredentialConstraints(t, hsdb.DB) + }) + + t.Run("fresh postgres", func(t *testing.T) { + requireCredentialConstraints(t, newPostgresTestDB(t).DB) + }) +} + +// TestNodeAuthKeyOnlyResolvesPreAuthKeys points a node at a credential of +// another kind, which the shared table's foreign key permits, and asserts it +// is not loaded as the node's pre-auth key. +func TestNodeAuthKeyOnlyResolvesPreAuthKeys(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + user := db.CreateUserForTest("kind-filter") + node := db.CreateNodeForTest(user, "kind-filter") + + _, apiKey, err := db.CreateAPIKey(nil) + require.NoError(t, err) + + require.NoError(t, db.DB.Model(&types.Node{}).Where("id = ?", node.ID). + Update("auth_key_id", apiKey.ID).Error) + + got, err := db.GetNodeByID(node.ID) + require.NoError(t, err) + assert.Nil(t, got.AuthKey, "an API key must not load as a node's pre-auth key") +} + +func sqliteTestConfig(path string) *types.Config { + return &types.Config{ + Database: types.DatabaseConfig{ + Type: "sqlite3", + Sqlite: types.SqliteConfig{Path: path}, + }, + Policy: types.PolicyConfig{Mode: types.PolicyModeDB}, + } +} + +// TestSQLiteMigrationKeepsPreAuthKeySequence deletes keys before the upgrade +// (the old table's AUTOINCREMENT remembers them) and asserts their ids are +// not handed out again, so a stale request by id cannot hit a new key. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func TestSQLiteMigrationKeepsPreAuthKeySequence(t *testing.T) { + dbPath := t.TempDir() + "/headscale_test.db" + + require.NoError(t, createSQLiteFromSQLFile("testdata/sqlite/headscale_0.29.3_dump.sql", dbPath)) + + raw, err := sql.Open("sqlite", dbPath) + require.NoError(t, err) + + _, err = raw.ExecContext(context.Background(), `UPDATE sqlite_sequence SET seq = 9 WHERE name = 'pre_auth_keys'`) + require.NoError(t, err) + require.NoError(t, raw.Close()) + + hsdb, err := NewHeadscaleDatabase(sqliteTestConfig(dbPath)) + require.NoError(t, err) + + pak, err := hsdb.CreatePreAuthKey(nil, false, false, nil, []string{"tag:x"}) + require.NoError(t, err) + assert.Greater(t, pak.ID, uint64(9)) +} + +// TestRevokedKeysBackingNodesSurviveCollection revokes the keys behind the +// migrated ephemeral nodes, runs the collector past the retention window and +// reloads: keys still backing a node must survive, so the nodes stay +// ephemeral, while unreferenced revoked keys are reaped. +// +// TODO(kradalby): seed without the 0.29.3 fixture in 0.31, when the +// credentials migration is dropped. +func TestRevokedKeysBackingNodesSurviveCollection(t *testing.T) { + dbPath := t.TempDir() + "/headscale_test.db" + + require.NoError(t, createSQLiteFromSQLFile("testdata/sqlite/headscale_0.29.3_dump.sql", dbPath)) + + hsdb, err := NewHeadscaleDatabase(sqliteTestConfig(dbPath)) + require.NoError(t, err) + + unused, err := hsdb.CreatePreAuthKey(nil, false, false, nil, []string{"tag:x"}) + require.NoError(t, err) + + for _, id := range []uint64{3, 5, unused.ID} { + require.NoError(t, hsdb.RevokePreAuthKey(id)) + } + + reaped, err := hsdb.DestroyRevokedPreAuthKeysBefore(time.Now().Add(24 * time.Hour)) + require.NoError(t, err) + assert.Equal(t, 1, reaped, "only the unreferenced revoked key is reaped") + + require.NoError(t, hsdb.Close()) + + hsdb, err = NewHeadscaleDatabase(sqliteTestConfig(dbPath)) + require.NoError(t, err) + + ephemeral, err := hsdb.ListEphemeralNodes() + require.NoError(t, err) + + ids := make([]types.NodeID, 0, len(ephemeral)) + for _, n := range ephemeral { + ids = append(ids, n.ID) + } + + assert.ElementsMatch(t, []types.NodeID{3, 5}, ids) +} + +// TestInterruptedInitSchemaRecovers simulates a first start that created the +// schema but died before gormigrate recorded the migrations, and asserts the +// next start completes instead of failing on the existing credentials table. +func TestInterruptedInitSchemaRecovers(t *testing.T) { + t.Run("sqlite", func(t *testing.T) { + dbPath := t.TempDir() + "/headscale_test.db" + + hsdb, err := NewHeadscaleDatabase(sqliteTestConfig(dbPath)) + require.NoError(t, err) + require.NoError(t, hsdb.DB.Exec(`DELETE FROM migrations`).Error) + require.NoError(t, hsdb.Close()) + + hsdb, err = NewHeadscaleDatabase(sqliteTestConfig(dbPath)) + require.NoError(t, err) + + _, _, err = hsdb.CreateAPIKey(nil) + require.NoError(t, err) + }) + + t.Run("postgres", func(t *testing.T) { + u := newPostgresDBForTest(t) + + hsdb := newHeadscaleDBFromPostgresURL(t, u) + require.NoError(t, hsdb.DB.Exec(`DELETE FROM migrations`).Error) + require.NoError(t, hsdb.Close()) + + hsdb = newHeadscaleDBFromPostgresURL(t, u) + + _, _, err := hsdb.CreateAPIKey(nil) + require.NoError(t, err) + }) +} diff --git a/hscontrol/db/migrate_credentials.go b/hscontrol/db/migrate_credentials.go new file mode 100644 index 00000000..3290f1fb --- /dev/null +++ b/hscontrol/db/migrate_credentials.go @@ -0,0 +1,369 @@ +package db + +import ( + "fmt" + + "github.com/juanfont/headscale/hscontrol/types" + "gorm.io/gorm" +) + +// credentialsDDLSQLite matches schema.sql byte-for-byte (the squibble digest is +// the SQLite source of truth). Only revoked legacy rows may lack a hash, and +// every stored hash format ($sha256$, $argon2id$, bcrypt $2a$) starts with '$'. +// TODO(kradalby): in 0.31, with the credentials migration, tighten +// chk_credentials_hash to hash NOT NULL (needs a migration clearing the +// hashless revoked rows). +// +//nolint:gosec // DDL, not a credential +const credentialsDDLSQLite = `CREATE TABLE credentials( + id integer PRIMARY KEY AUTOINCREMENT, + kind text NOT NULL CHECK(kind IN ('api','authkey','oauth_client','oauth_token')), + identifier text, + hash blob, + user_id integer, + description text, + scopes text, + tags text, + reusable numeric, + ephemeral numeric DEFAULT false, + used numeric DEFAULT false, + last_seen datetime, + client_id text, + created_at datetime, + expiration datetime, + revoked datetime, + + CONSTRAINT fk_credentials_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE SET NULL, + CONSTRAINT chk_credentials_hash CHECK(hash IS NOT NULL OR revoked IS NOT NULL), + CONSTRAINT chk_credentials_hash_format CHECK(hash IS NULL OR substr(CAST(hash AS TEXT), 1, 1) = '$') +)` + +// credentialsDDLPostgres is the Postgres form of [credentialsDDLSQLite]; both +// InitSchema and the migration use it, so fresh and upgraded databases match. +// +//nolint:gosec // DDL, not a credential +const credentialsDDLPostgres = `CREATE TABLE credentials( + id bigserial PRIMARY KEY, + kind text NOT NULL CHECK(kind IN ('api','authkey','oauth_client','oauth_token')), + identifier text, + hash bytea, + user_id bigint, + description text, + scopes text, + tags text, + reusable boolean, + ephemeral boolean DEFAULT false, + used boolean DEFAULT false, + last_seen timestamptz, + client_id text, + created_at timestamptz, + expiration timestamptz, + revoked timestamptz, + + CONSTRAINT fk_credentials_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE SET NULL, + CONSTRAINT chk_credentials_hash CHECK(hash IS NOT NULL OR revoked IS NOT NULL), + CONSTRAINT chk_credentials_hash_format CHECK(hash IS NULL OR substring(hash from 1 for 1) = '\x24'::bytea) +)` + +// credentialIndexes are created with the table. +var credentialIndexes = []string{ + `CREATE UNIQUE INDEX idx_credentials_identifier ON credentials(kind, identifier)`, + `CREATE INDEX idx_credentials_user_id ON credentials(user_id)`, +} + +// ensureCredentialsTable creates the credentials table and its indexes in one +// transaction, and is a no-op once the table exists. InitSchema's records are +// written after it runs, so an interrupted first start re-runs it. +func ensureCredentialsTable(tx *gorm.DB) error { + if tx.Migrator().HasTable(&types.Credential{}) { + return nil + } + + return tx.Transaction(createCredentialsTable) +} + +// createCredentialsTable creates the unified credentials table and its indexes. +func createCredentialsTable(tx *gorm.DB) error { + ddl := credentialsDDLSQLite + if tx.Name() != "sqlite" { + ddl = credentialsDDLPostgres + } + + err := tx.Exec(ddl).Error + if err != nil { + return fmt.Errorf("creating credentials table: %w", err) + } + + for _, stmt := range credentialIndexes { + err := tx.Exec(stmt).Error + if err != nil { + return fmt.Errorf("creating credentials index: %w", err) + } + } + + return nil +} + +// migrateToCredentials backfills the unified credentials table from the four +// per-kind tables and drops them. Pre-auth keys keep their ids so +// nodes.auth_key_id stays valid; the nodes FK is then retargeted to +// credentials(id). The caller runs it in one transaction, so a failure leaves +// the database untouched and the migration can be retried. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func migrateToCredentials(tx *gorm.DB) error { + // user_id on api_keys and oauth_clients never had a foreign key and + // DestroyUser left it behind, so it can reference a deleted user. Null it + // rather than fail the new foreign key. + const ownerOrNull = `CASE WHEN user_id IN (SELECT id FROM users) THEN user_id END` + + // A row without a hash can never authenticate; keep it revoked rather + // than drop it, so references to it stay valid. + const revokedIfNoHash = `CASE WHEN hash IS NULL THEN CURRENT_TIMESTAMP END` + + // Legacy plaintext pre-auth keys (no prefix) land as hashless revoked + // placeholders here; hashLegacyPreAuthKeys then hashes the ones that still + // carry their key so they keep working. + err := tx.Exec(`INSERT INTO credentials + (id, kind, identifier, hash, user_id, description, reusable, ephemeral, used, tags, expiration, revoked, created_at) +SELECT id, ?, + CASE WHEN prefix IS NULL OR prefix = '' THEN 'legacy-id-' || id ELSE prefix END, + CASE WHEN prefix IS NULL OR prefix = '' THEN NULL ELSE hash END, + `+ownerOrNull+`, description, reusable, ephemeral, used, tags, expiration, + CASE WHEN prefix IS NULL OR prefix = '' OR hash IS NULL + THEN COALESCE(revoked, CURRENT_TIMESTAMP) ELSE revoked END, + created_at +FROM pre_auth_keys`, types.CredentialPreAuthKey).Error + if err != nil { + return fmt.Errorf("backfilling pre-auth keys: %w", err) + } + + err = hashLegacyPreAuthKeys(tx) + if err != nil { + return err + } + + // Continue the pre-auth key id sequence, not just max(id): ids of deleted + // keys must not be handed out again, or a stale request by id would hit a + // new key. This also moves Postgres past the explicit-id inserts above. + err = continuePreAuthKeySequence(tx) + if err != nil { + return err + } + + err = tx.Exec(`INSERT INTO credentials (kind, identifier, hash, user_id, last_seen, expiration, revoked, created_at) +SELECT ?, prefix, hash, `+ownerOrNull+`, last_seen, expiration, `+revokedIfNoHash+`, created_at +FROM api_keys ORDER BY id`, types.CredentialAPIKey).Error + if err != nil { + return fmt.Errorf("backfilling api keys: %w", err) + } + + err = tx.Exec(`INSERT INTO credentials (kind, identifier, hash, scopes, tags, description, user_id, revoked, created_at) +SELECT ?, client_id, secret_hash, scopes, tags, description, `+ownerOrNull+`, + CASE WHEN secret_hash IS NULL THEN COALESCE(revoked, CURRENT_TIMESTAMP) ELSE revoked END, created_at +FROM oauth_clients ORDER BY id`, types.CredentialOAuthClient).Error + if err != nil { + return fmt.Errorf("backfilling oauth clients: %w", err) + } + + err = tx.Exec(`INSERT INTO credentials (kind, identifier, hash, client_id, scopes, tags, expiration, revoked, created_at) +SELECT ?, prefix, hash, client_id, scopes, tags, expiration, `+revokedIfNoHash+`, created_at +FROM oauth_access_tokens ORDER BY id`, types.CredentialOAuthToken).Error + if err != nil { + return fmt.Errorf("backfilling oauth access tokens: %w", err) + } + + err = retargetNodesAuthKeyFK(tx) + if err != nil { + return err + } + + err = tx.Exec(`CREATE INDEX idx_nodes_auth_key_id ON nodes(auth_key_id)`).Error + if err != nil { + return fmt.Errorf("creating nodes auth_key_id index: %w", err) + } + + for _, table := range []string{"pre_auth_keys", "api_keys", "oauth_clients", "oauth_access_tokens"} { + err := tx.Migrator().DropTable(table) + if err != nil { + return fmt.Errorf("dropping %s: %w", table, err) + } + } + + return nil +} + +// hashLegacyPreAuthKeys stores each pre-0.28 plaintext pre-auth key as a SHA-256 +// hash under its derived identifier (see [legacyAuthKeyIdentifier]) and lifts +// the placeholder revocation, so the key keeps authenticating after the +// upgrade. They hold 192 bits of crypto/rand entropy, so the reasoning at +// [hashPrefixSHA256] applies. SQLite has no SHA-256 function, so this runs in Go. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func hashLegacyPreAuthKeys(tx *gorm.DB) error { + type legacyKey struct { + ID uint64 + Key string + } + + var keys []legacyKey + + err := tx.Raw(`SELECT id, key FROM pre_auth_keys +WHERE (prefix IS NULL OR prefix = '') AND key IS NOT NULL AND key != ''`). + Scan(&keys).Error + if err != nil { + return fmt.Errorf("reading legacy pre-auth keys: %w", err) + } + + for _, k := range keys { + err := tx.Exec(`UPDATE credentials +SET identifier = ?, hash = ?, revoked = (SELECT revoked FROM pre_auth_keys WHERE id = ?) +WHERE id = ?`, legacyAuthKeyIdentifier(k.Key), hashSecret(k.Key), k.ID, k.ID).Error + if err != nil { + return fmt.Errorf("hashing legacy pre-auth key %d: %w", k.ID, err) + } + } + + return nil +} + +// retargetNodesAuthKeyFK repoints the nodes.auth_key_id foreign key from +// pre_auth_keys(id) to credentials(id). Postgres alters the constraint in place; +// SQLite, which cannot alter a foreign key, rebuilds the table. The rebuild runs +// with foreign keys enabled: no table references nodes, and every auth_key_id +// now points at a credentials row, so no FK toggling is required. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func retargetNodesAuthKeyFK(tx *gorm.DB) error { + if tx.Name() != "sqlite" { + err := tx.Exec(`ALTER TABLE nodes DROP CONSTRAINT IF EXISTS fk_nodes_auth_key`).Error + if err != nil { + return fmt.Errorf("dropping nodes auth_key constraint: %w", err) + } + + err = tx.Exec(`ALTER TABLE nodes ADD CONSTRAINT fk_nodes_auth_key +FOREIGN KEY (auth_key_id) REFERENCES credentials(id)`).Error + if err != nil { + return fmt.Errorf("adding nodes auth_key constraint: %w", err) + } + + return nil + } + + // Dropping the table resets its AUTOINCREMENT counter to max(id); carry the + // old counter over so ids of deleted nodes are never handed out again. + seq, err := sqliteSequence(tx, "nodes") + if err != nil { + return err + } + + stmts := []string{ + `CREATE TABLE nodes_new( + id integer PRIMARY KEY AUTOINCREMENT, + machine_key text, + node_key text, + disco_key text, + endpoints text, + host_info text, + ipv4 text, + ipv6 text, + hostname text, + given_name varchar(63), + user_id integer, + register_method text, + tags text, + auth_key_id integer, + last_seen datetime, + expiry datetime, + approved_routes text, + + created_at datetime, + updated_at datetime, + deleted_at datetime, + + CONSTRAINT fk_nodes_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE, + CONSTRAINT fk_nodes_auth_key FOREIGN KEY(auth_key_id) REFERENCES credentials(id) +)`, + `INSERT INTO nodes_new + (id, machine_key, node_key, disco_key, endpoints, host_info, ipv4, ipv6, hostname, given_name, user_id, register_method, tags, auth_key_id, last_seen, expiry, approved_routes, created_at, updated_at, deleted_at) +SELECT id, machine_key, node_key, disco_key, endpoints, host_info, ipv4, ipv6, hostname, given_name, user_id, register_method, tags, auth_key_id, last_seen, expiry, approved_routes, created_at, updated_at, deleted_at +FROM nodes`, + `DROP TABLE nodes`, + `ALTER TABLE nodes_new RENAME TO nodes`, + } + + for _, stmt := range stmts { + err := tx.Exec(stmt).Error + if err != nil { + return fmt.Errorf("rebuilding nodes table: %w", err) + } + } + + return raiseSQLiteSequence(tx, "nodes", seq) +} + +// continuePreAuthKeySequence advances the credentials id sequence to at least +// the last id pre_auth_keys ever allocated. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func continuePreAuthKeySequence(tx *gorm.DB) error { + if tx.Name() == "sqlite" { + seq, err := sqliteSequence(tx, "pre_auth_keys") + if err != nil { + return err + } + + return raiseSQLiteSequence(tx, "credentials", seq) + } + + var last int64 + + // pg_sequences.last_value is NULL for a sequence that was never used. + err := tx.Raw(`SELECT COALESCE(MAX(last_value), 0) FROM pg_sequences +WHERE schemaname = current_schema() + AND sequencename = (SELECT relname FROM pg_class WHERE oid = pg_get_serial_sequence('pre_auth_keys', 'id')::regclass)`). + Scan(&last).Error + if err != nil { + return fmt.Errorf("reading pre_auth_keys id sequence: %w", err) + } + + err = tx.Exec(`SELECT setval(pg_get_serial_sequence('credentials','id'), +GREATEST((SELECT COALESCE(MAX(id), 0) FROM credentials), ?, 1))`, last).Error + if err != nil { + return fmt.Errorf("advancing credentials id sequence: %w", err) + } + + return nil +} + +// sqliteSequence returns the AUTOINCREMENT counter of table, 0 if unset. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func sqliteSequence(tx *gorm.DB, table string) (int64, error) { + var seq int64 + + err := tx.Raw(`SELECT COALESCE(MAX(seq), 0) FROM sqlite_sequence WHERE name = ?`, table). + Scan(&seq).Error + if err != nil { + return 0, fmt.Errorf("reading %s sequence: %w", table, err) + } + + return seq, nil +} + +// raiseSQLiteSequence sets the AUTOINCREMENT counter of table to at least seq. +// +// TODO(kradalby): remove in 0.31 with the credentials migration. +func raiseSQLiteSequence(tx *gorm.DB, table string, seq int64) error { + res := tx.Exec(`UPDATE sqlite_sequence SET seq = MAX(seq, ?) WHERE name = ?`, seq, table) + if res.Error == nil && res.RowsAffected == 0 && seq > 0 { + // No row yet: nothing has been inserted into table. + res = tx.Exec(`INSERT INTO sqlite_sequence(name, seq) VALUES(?, ?)`, table, seq) + } + + if res.Error != nil { + return fmt.Errorf("restoring %s sequence: %w", table, res.Error) + } + + return nil +} diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go index c983c841..31c1fa7e 100644 --- a/hscontrol/db/node.go +++ b/hscontrol/db/node.go @@ -32,10 +32,12 @@ const ( var ErrNodeNameNotUnique = errors.New("node name is not unique") // preloadNode returns a session that eager-loads a node's AuthKey, the -// AuthKey's User, and the node's User. +// AuthKey's User, and the node's User. The AuthKey is filtered to pre-auth +// keys: auth_key_id references the shared credentials table, and only that +// kind may back a node. func preloadNode(tx *gorm.DB) *gorm.DB { return tx. - Preload("AuthKey"). + Preload("AuthKey", "kind = ?", types.CredentialPreAuthKey). Preload("AuthKey.User"). Preload("User") } @@ -98,7 +100,9 @@ func (hsdb *HSDatabase) ListEphemeralNodes() (types.Nodes, error) { return Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) { nodes := types.Nodes{} - err := rx.Joins("AuthKey").Where(`"AuthKey"."ephemeral" = true`).Find(&nodes).Error + err := rx.Joins("AuthKey"). + Where(`"AuthKey"."ephemeral" = true AND "AuthKey"."kind" = ?`, types.CredentialPreAuthKey). + Find(&nodes).Error if err != nil { return nil, err } diff --git a/hscontrol/db/oauth.go b/hscontrol/db/oauth.go index dbbda311..7e6e4d39 100644 --- a/hscontrol/db/oauth.go +++ b/hscontrol/db/oauth.go @@ -1,40 +1,19 @@ package db import ( - "crypto/rand" - "crypto/subtle" - "encoding/base64" "errors" "fmt" - "runtime" "slices" "strings" "time" "github.com/juanfont/headscale/hscontrol/scope" "github.com/juanfont/headscale/hscontrol/types" - "golang.org/x/crypto/argon2" "gorm.io/gorm" "tailscale.com/util/multierr" - "tailscale.com/util/rands" "tailscale.com/util/set" ) -const ( - // OAuth client secret: hskey-client--. The clientID - // is the public, indexed lookup key (the analogue of an API key's prefix) and - // is embedded in the secret so the token endpoint can derive it. The prefix - // itself lives in the types package ([types.OAuthClientPrefix]). - oauthClientIDLength = 12 - oauthClientSecretLength = 64 - - // OAuth access token: hskey-oauthtok--. The distinct - // prefix (vs hskey-api- admin keys, [types.AccessTokenPrefix]) lets the auth - // middleware dispatch a scoped token from an all-access admin key alone. - accessTokenPrefixLength = 12 - accessTokenSecretLength = 64 -) - var ( ErrOAuthClientNotFound = fmt.Errorf("oauth client not found: %w", gorm.ErrRecordNotFound) ErrOAuthClientFailedToParse = errors.New("failed to parse oauth client secret") @@ -45,96 +24,8 @@ var ( ErrAccessTokenFailedToParse = errors.New("failed to parse oauth access token") ErrAccessTokenExpired = errors.New("oauth access token expired") ErrAccessTokenClientRevoked = errors.New("oauth access token issuing client revoked or deleted") - - errSecretHashMalformed = errors.New("malformed secret hash") - errSecretMismatch = errors.New("secret does not match hash") ) -// Argon2id parameters, OWASP's minimum recommendation (19 MiB, 2 iterations, 1 -// lane). They are encoded into every stored hash, so raising them later still -// verifies credentials stored under the old cost. -const ( - argon2Time = 2 - argon2Memory = 19 * 1024 - argon2Threads = 1 - argon2KeyLen = 32 - argon2SaltLen = 16 -) - -// argon2Limiter bounds concurrent Argon2id computations. Each costs ~19 MiB and -// the unauthenticated OAuth token endpoint runs one per attempt, so an unbounded -// flood could exhaust memory. ponytail: a global semaphore sized to GOMAXPROCS; -// revisit only if credential hashing ever becomes a throughput bottleneck. -var argon2Limiter = make(chan struct{}, max(2, runtime.GOMAXPROCS(0))) - -// hashSecret hashes a credential secret with Argon2id, encoded in PHC string -// form so the parameters travel with the hash. Argon2id is the current OWASP -// recommendation, replacing bcrypt for new credential storage. -func hashSecret(secret string) ([]byte, error) { - salt := make([]byte, argon2SaltLen) - - _, err := rand.Read(salt) - if err != nil { - return nil, fmt.Errorf("generating salt: %w", err) - } - - hash := argon2.IDKey([]byte(secret), salt, argon2Time, argon2Memory, argon2Threads, argon2KeyLen) - - encoded := fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", - argon2.Version, argon2Memory, argon2Time, argon2Threads, - base64.RawStdEncoding.EncodeToString(salt), - base64.RawStdEncoding.EncodeToString(hash), - ) - - return []byte(encoded), nil -} - -// verifySecret reports whether secret matches a hashSecret-encoded hash. It -// reads the cost parameters from the stored hash and compares in constant time -// so a mismatch leaks no timing signal. -func verifySecret(encoded []byte, secret string) error { - parts := strings.Split(string(encoded), "$") - if len(parts) != 6 || parts[1] != "argon2id" { - return errSecretHashMalformed - } - - var version int - if _, err := fmt.Sscanf(parts[2], "v=%d", &version); err != nil || version != argon2.Version { //nolint:noinlineerr - return errSecretHashMalformed - } - - var ( - memory, time uint32 - threads uint8 - ) - - if _, err := fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &memory, &time, &threads); err != nil { //nolint:noinlineerr - return errSecretHashMalformed - } - - salt, err := base64.RawStdEncoding.DecodeString(parts[4]) - if err != nil { - return errSecretHashMalformed - } - - want, err := base64.RawStdEncoding.DecodeString(parts[5]) - if err != nil { - return errSecretHashMalformed - } - - argon2Limiter <- struct{}{} - //nolint:gosec // want is a 32-byte hash read back from storage, no overflow - got := argon2.IDKey([]byte(secret), salt, time, memory, threads, uint32(len(want))) - - <-argon2Limiter - - if subtle.ConstantTimeCompare(got, want) != 1 { - return errSecretMismatch - } - - return nil -} - // validateScopes rejects any value outside the vocabulary in [scope.Known]. An // unknown scope satisfies no requirement, so storing one would silently mint a // client with fewer permissions than asked for. Every offending scope is @@ -172,19 +63,13 @@ func (hsdb *HSDatabase) CreateOAuthClient( scopes = set.SetOf(scopes).Slice() slices.Sort(scopes) - clientID := rands.HexString(oauthClientIDLength) - secret := rands.HexString(oauthClientSecretLength) - secretStr := types.OAuthClientPrefix + clientID + "-" + secret - - hash, err := hashSecret(secret) - if err != nil { - return "", nil, err - } + secretStr, clientID, hash := generateSecret(types.OAuthClientPrefix) now := time.Now().UTC() - client := types.OAuthClient{ - ClientID: clientID, - SecretHash: hash, + cred := types.Credential{ + Kind: types.CredentialOAuthClient, + Identifier: clientID, + Hash: hash, Scopes: scopes, Tags: tags, Description: description, @@ -193,13 +78,13 @@ func (hsdb *HSDatabase) CreateOAuthClient( } err = hsdb.Write(func(tx *gorm.DB) error { - return tx.Save(&client).Error + return tx.Save(&cred).Error }) if err != nil { return "", nil, fmt.Errorf("saving oauth client: %w", err) } - return secretStr, &client, nil + return secretStr, credentialToOAuthClient(&cred), nil } // AuthenticateOAuthClient validates a presented client secret and returns the @@ -226,51 +111,49 @@ func (hsdb *HSDatabase) AuthenticateOAuthClient(secretStr string) (*types.OAuthC return nil, ErrOAuthClientFailedToParse } - clientID, secret, err := parsePrefixedKey( - rest, - oauthClientIDLength, - oauthClientSecretLength, - ErrOAuthClientFailedToParse, + clientID, secret, err := parsePrefixedKey(rest, ErrOAuthClientFailedToParse) + if err != nil { + return nil, err + } + + cred, err := authenticateCredential( + hsdb.DB, types.CredentialOAuthClient, clientID, secret, ErrOAuthClientNotFound, ) if err != nil { return nil, err } - var client types.OAuthClient - if err := hsdb.DB.First(&client, "client_id = ?", clientID).Error; err != nil { //nolint:noinlineerr - return nil, ErrOAuthClientNotFound - } - - if err := verifySecret(client.SecretHash, secret); err != nil { //nolint:noinlineerr - return nil, fmt.Errorf("invalid oauth client secret: %w", err) - } - - if client.Revoked != nil { + if cred.Revoked != nil { return nil, ErrOAuthClientRevoked } - return &client, nil + return credentialToOAuthClient(cred), nil } // GetOAuthClientByClientID returns a [types.OAuthClient] by its public client id. func (hsdb *HSDatabase) GetOAuthClientByClientID(clientID string) (*types.OAuthClient, error) { - var client types.OAuthClient - if result := hsdb.DB.First(&client, "client_id = ?", clientID); result.Error != nil { + var cred types.Credential + if result := hsdb.DB.First(&cred, "kind = ? AND identifier = ?", types.CredentialOAuthClient, clientID); result.Error != nil { return nil, result.Error } - return &client, nil + return credentialToOAuthClient(&cred), nil } // ListOAuthClients returns every [types.OAuthClient]. func (hsdb *HSDatabase) ListOAuthClients() ([]types.OAuthClient, error) { - clients := []types.OAuthClient{} + var creds []types.Credential - err := hsdb.DB.Find(&clients).Error + err := hsdb.DB.Where("kind = ?", types.CredentialOAuthClient).Find(&creds).Error if err != nil { return nil, err } + clients := make([]types.OAuthClient, 0, len(creds)) + for i := range creds { + clients = append(clients, *credentialToOAuthClient(&creds[i])) + } + return clients, nil } @@ -280,13 +163,14 @@ func (hsdb *HSDatabase) ListOAuthClients() ([]types.OAuthClient, error) { // OAuth client has no such history and is removed outright, matching Tailscale. func (hsdb *HSDatabase) RevokeOAuthClient(clientID string) error { return hsdb.Write(func(tx *gorm.DB) error { - err := tx.Where("client_id = ?", clientID). - Delete(&types.OAuthAccessToken{}).Error + err := tx.Where("kind = ? AND client_id = ?", types.CredentialOAuthToken, clientID). + Delete(&types.Credential{}).Error if err != nil { return fmt.Errorf("deleting oauth access tokens: %w", err) } - res := tx.Where("client_id = ?", clientID).Delete(&types.OAuthClient{}) + res := tx.Where("kind = ? AND identifier = ?", types.CredentialOAuthClient, clientID). + Delete(&types.Credential{}) if res.Error != nil { return res.Error } @@ -307,18 +191,12 @@ func (hsdb *HSDatabase) MintAccessToken( scopes, tags []string, expiration *time.Time, ) (string, *types.OAuthAccessToken, error) { - prefix := rands.HexString(accessTokenPrefixLength) - secret := rands.HexString(accessTokenSecretLength) - tokenStr := types.AccessTokenPrefix + prefix + "-" + secret - - hash, err := hashSecret(secret) - if err != nil { - return "", nil, err - } + tokenStr, prefix, hash := generateSecret(types.AccessTokenPrefix) now := time.Now().UTC() - token := types.OAuthAccessToken{ - Prefix: prefix, + cred := types.Credential{ + Kind: types.CredentialOAuthToken, + Identifier: prefix, Hash: hash, ClientID: clientID, Scopes: scopes, @@ -329,25 +207,29 @@ func (hsdb *HSDatabase) MintAccessToken( // Mint inside a transaction that re-checks the client still exists and is // not revoked, so a mint cannot complete against a client being deleted. - err = hsdb.Write(func(tx *gorm.DB) error { - var client types.OAuthClient + err := hsdb.Write(func(tx *gorm.DB) error { + var client types.Credential - err := tx.First(&client, "client_id = ?", clientID).Error - if err != nil { + err := tx.First(&client, "kind = ? AND identifier = ?", types.CredentialOAuthClient, clientID).Error + if errors.Is(err, gorm.ErrRecordNotFound) { return ErrOAuthClientNotFound } + if err != nil { + return err + } + if client.Revoked != nil { return ErrOAuthClientRevoked } - return tx.Save(&token).Error + return tx.Save(&cred).Error }) if err != nil { return "", nil, fmt.Errorf("saving oauth access token: %w", err) } - return tokenStr, &token, nil + return tokenStr, credentialToOAuthAccessToken(&cred), nil } // AuthenticateAccessToken validates a presented bearer token and returns the @@ -363,23 +245,16 @@ func (hsdb *HSDatabase) AuthenticateAccessToken(tokenStr string) (*types.OAuthAc return nil, ErrAccessTokenFailedToParse } - prefix, secret, err := parsePrefixedKey( - rest, - accessTokenPrefixLength, - accessTokenSecretLength, - ErrAccessTokenFailedToParse, - ) + prefix, secret, err := parsePrefixedKey(rest, ErrAccessTokenFailedToParse) if err != nil { return nil, err } - var token types.OAuthAccessToken - if err := hsdb.DB.First(&token, "prefix = ?", prefix).Error; err != nil { //nolint:noinlineerr - return nil, ErrAccessTokenNotFound - } - - if err := verifySecret(token.Hash, secret); err != nil { //nolint:noinlineerr - return nil, fmt.Errorf("invalid oauth access token: %w", err) + token, err := authenticateCredential( + hsdb.DB, types.CredentialOAuthToken, prefix, secret, ErrAccessTokenNotFound, + ) + if err != nil { + return nil, err } if token.Expiration != nil && token.Expiration.Before(time.Now()) { @@ -390,16 +265,18 @@ func (hsdb *HSDatabase) AuthenticateAccessToken(tokenStr string) (*types.OAuthAc // revoked or deleted is rejected. This closes a mint/revoke race (where a // token could be inserted after the client's tokens were purged) and any // orphan left by manual deletion or a future soft-revoke path. - var client types.OAuthClient - if err := hsdb.DB.First(&client, "client_id = ?", token.ClientID).Error; err != nil { //nolint:noinlineerr + var client types.Credential + + err = hsdb.DB.First(&client, "kind = ? AND identifier = ?", types.CredentialOAuthClient, token.ClientID).Error + if errors.Is(err, gorm.ErrRecordNotFound) || (err == nil && client.Revoked != nil) { return nil, ErrAccessTokenClientRevoked } - if client.Revoked != nil { - return nil, ErrAccessTokenClientRevoked + if err != nil { + return nil, fmt.Errorf("looking up issuing oauth client: %w", err) } - return &token, nil + return credentialToOAuthAccessToken(token), nil } // DeleteExpiredAccessTokens hard-deletes every access token that expired before @@ -407,8 +284,8 @@ func (hsdb *HSDatabase) AuthenticateAccessToken(tokenStr string) (*types.OAuthAc // expired tokens; the hourly reaper (see app.go) calls this only to keep the // table from growing unbounded. func (hsdb *HSDatabase) DeleteExpiredAccessTokens(cutoff time.Time) (int64, error) { - res := hsdb.DB.Where("expiration IS NOT NULL AND expiration < ?", cutoff). - Delete(&types.OAuthAccessToken{}) + res := hsdb.DB.Where("kind = ? AND expiration IS NOT NULL AND expiration < ?", types.CredentialOAuthToken, cutoff). + Delete(&types.Credential{}) return res.RowsAffected, res.Error } diff --git a/hscontrol/db/oauth_test.go b/hscontrol/db/oauth_test.go index 1d983929..9cd059a2 100644 --- a/hscontrol/db/oauth_test.go +++ b/hscontrol/db/oauth_test.go @@ -1,6 +1,8 @@ package db import ( + "encoding/base64" + "fmt" "strings" "sync" "testing" @@ -9,13 +11,94 @@ import ( "github.com/juanfont/headscale/hscontrol/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/crypto/argon2" + "golang.org/x/crypto/bcrypt" ) -// TestVerifySecretConcurrent runs more concurrent verifications than the Argon2 -// concurrency semaphore admits, asserting the limiter releases correctly (no -// deadlock) and stays correct under contention. Run with -race. +// legacyArgon2idHash builds a PHC-encoded Argon2id hash in the form development +// builds stored secrets before the switch to SHA-256. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. +func legacyArgon2idHash(secret string, memory, time uint32, threads uint8) []byte { + salt := []byte("0123456789abcdef") + sum := argon2.IDKey([]byte(secret), salt, time, memory, threads, argon2KeyLen) + + return fmt.Appendf(nil, "$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", + argon2.Version, memory, time, threads, + base64.RawStdEncoding.EncodeToString(salt), + base64.RawStdEncoding.EncodeToString(sum), + ) +} + +func TestVerifySecret(t *testing.T) { + const secret = "s3cr3t" + + bcryptHash, err := bcrypt.GenerateFromPassword([]byte(secret), bcrypt.MinCost) + require.NoError(t, err) + + tests := []struct { + name string + hash []byte + wantRehash bool + wantErr error + wrongSecret bool + }{ + {name: "sha256", hash: hashSecret(secret)}, + {name: "sha256 wrong secret", hash: hashSecret(secret), wrongSecret: true, wantErr: errSecretMismatch}, + // TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. + {name: "argon2id legacy", hash: legacyArgon2idHash(secret, 19*1024, 2, 1), wantRehash: true}, + {name: "argon2id wrong secret", hash: legacyArgon2idHash(secret, 19*1024, 2, 1), wrongSecret: true, wantErr: errSecretMismatch}, + {name: "bcrypt legacy", hash: bcryptHash, wantRehash: true}, + {name: "bcrypt wrong secret", hash: bcryptHash, wrongSecret: true, wantErr: errSecretMismatch}, + {name: "nil hash", hash: nil, wantErr: errSecretHashMalformed}, + {name: "garbage", hash: []byte("not-a-hash"), wantErr: errSecretHashMalformed}, + {name: "truncated bcrypt", hash: bcryptHash[:20], wantErr: errSecretHashMalformed}, + {name: "sha256 bad hex", hash: []byte("$sha256$zz"), wantErr: errSecretHashMalformed}, + // Parameters argon2 would panic on, or that would allocate unbounded + // memory, must be rejected before hashing. + {name: "argon2id t=0", hash: []byte("$argon2id$v=19$m=19456,t=0,p=1$MDEyMzQ1Njc4OWFiY2RlZg$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), wantErr: errSecretHashMalformed}, + {name: "argon2id p=0", hash: []byte("$argon2id$v=19$m=19456,t=2,p=0$MDEyMzQ1Njc4OWFiY2RlZg$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), wantErr: errSecretHashMalformed}, + {name: "argon2id huge memory", hash: []byte("$argon2id$v=19$m=4294967295,t=2,p=1$MDEyMzQ1Njc4OWFiY2RlZg$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), wantErr: errSecretHashMalformed}, + {name: "argon2id empty key", hash: []byte("$argon2id$v=19$m=19456,t=2,p=1$MDEyMzQ1Njc4OWFiY2RlZg$"), wantErr: errSecretHashMalformed}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + presented := secret + if tt.wrongSecret { + presented = "wrong" + } + + needsRehash, err := verifySecret(tt.hash, presented) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + assert.False(t, needsRehash) + + return + } + + require.NoError(t, err) + assert.Equal(t, tt.wantRehash, needsRehash) + }) + } + + // A malformed hash must not leak a limiter slot: after more failures than + // the limiter holds, a legacy verify still completes. + for range cap(legacyHashLimiter) + 1 { + _, _ = verifySecret([]byte("$argon2id$v=19$m=19456,t=0,p=1$MDEyMzQ1Njc4OWFiY2RlZg$AAAA"), secret) + } + + _, err = verifySecret(bcryptHash, secret) + require.NoError(t, err) +} + +// TestVerifySecretConcurrent runs more concurrent legacy verifications than the +// limiter admits, asserting it releases correctly (no deadlock) and stays +// correct under contention. Run with -race. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. func TestVerifySecretConcurrent(t *testing.T) { - hash, err := hashSecret("s3cr3t") + hash, err := bcrypt.GenerateFromPassword([]byte("s3cr3t"), bcrypt.MinCost) require.NoError(t, err) const n = 64 @@ -25,17 +108,13 @@ func TestVerifySecretConcurrent(t *testing.T) { errs := make([]error, n) for i := range n { - wg.Add(1) - - go func(i int) { - defer wg.Done() - + wg.Go(func() { if i%2 == 0 { - errs[i] = verifySecret(hash, "s3cr3t") + _, errs[i] = verifySecret(hash, "s3cr3t") } else { - errs[i] = verifySecret(hash, "wrong") + _, errs[i] = verifySecret(hash, "wrong") } - }(i) + }) } wg.Wait() @@ -44,11 +123,30 @@ func TestVerifySecretConcurrent(t *testing.T) { if i%2 == 0 { assert.NoError(t, e, "correct secret must verify") } else { - assert.Error(t, e, "wrong secret must fail") + assert.ErrorIs(t, e, errSecretMismatch, "wrong secret must fail") } } } +// TestGenerateSecret verifies the unified key shape +// - and that the stored hash verifies the +// secret without needing a rehash. +func TestGenerateSecret(t *testing.T) { + full, identifier, hash := generateSecret("hskey-test-") + + require.Len(t, identifier, keyIdentifierLength) + require.True(t, strings.HasPrefix(full, "hskey-test-"+identifier+"-")) + + secret := strings.TrimPrefix(full, "hskey-test-"+identifier+"-") + require.Len(t, secret, keySecretLength) + assert.True(t, strings.HasPrefix(string(hash), hashPrefixSHA256)) + assert.NotContains(t, string(hash), secret, "secret must not be stored") + + needsRehash, err := verifySecret(hash, secret) + require.NoError(t, err) + require.False(t, needsRehash) +} + func TestOAuthClientCreateAndAuthenticate(t *testing.T) { db, err := newSQLiteTestDB() require.NoError(t, err) @@ -68,9 +166,8 @@ func TestOAuthClientCreateAndAuthenticate(t *testing.T) { // Scopes/tags are deduplicated and sorted for stable storage. assert.Equal(t, []string{"auth_keys", "devices:core"}, client.Scopes) assert.Equal(t, []string{"tag:ci"}, client.Tags) - // Only the Argon2id hash is stored, never the plaintext. - assert.NotEmpty(t, client.SecretHash) - assert.True(t, strings.HasPrefix(string(client.SecretHash), "$argon2id$")) + // Only the hash is stored, never the plaintext. + assert.True(t, strings.HasPrefix(string(client.SecretHash), hashPrefixSHA256)) // The secret authenticates, deriving the client id from the secret itself. got, err := db.AuthenticateOAuthClient(secret) @@ -79,11 +176,12 @@ func TestOAuthClientCreateAndAuthenticate(t *testing.T) { // A truncated/garbage secret does not. _, err = db.AuthenticateOAuthClient("hskey-client-deadbeef-nope") - require.Error(t, err) + require.ErrorIs(t, err, ErrOAuthClientFailedToParse) - // Wrong secret for a real client id is rejected by the constant-time compare. + // A wrong secret for a real client id reads as an unknown client. _, err = db.AuthenticateOAuthClient("hskey-client-" + client.ClientID + "-" + strings.Repeat("0", 64)) - require.Error(t, err) + require.ErrorIs(t, err, ErrOAuthClientNotFound) + require.ErrorIs(t, err, errSecretMismatch) } // TestOAuthClientAuthenticateTailscalePrefix asserts the same stored client @@ -134,24 +232,6 @@ func TestOAuthClientAuthenticateTailscalePrefix(t *testing.T) { } } -func TestHashSecretRoundTrip(t *testing.T) { - const secret = "a-high-entropy-credential-secret" - - encoded, err := hashSecret(secret) - require.NoError(t, err) - assert.True(t, strings.HasPrefix(string(encoded), "$argon2id$v=")) - - // The same secret hashes to a different value each time (random salt) yet - // still verifies. - encoded2, err := hashSecret(secret) - require.NoError(t, err) - assert.NotEqual(t, encoded, encoded2) - - require.NoError(t, verifySecret(encoded, secret)) - require.ErrorIs(t, verifySecret(encoded, "wrong-secret"), errSecretMismatch) - require.ErrorIs(t, verifySecret([]byte("not-a-phc-string"), secret), errSecretHashMalformed) -} - func TestOAuthClientRevoke(t *testing.T) { db, err := newSQLiteTestDB() require.NoError(t, err) @@ -201,13 +281,27 @@ func TestOAuthAccessTokenMintAuthenticateExpire(t *testing.T) { _, err = db.AuthenticateAccessToken(expiredStr) require.ErrorIs(t, err, ErrAccessTokenExpired) - // The reaper deletes the expired row; the live token is untouched. + // Expired credentials of other kinds share the table; the reaper must + // leave them alone. + apiKeyStr, _, err := db.CreateAPIKey(&past) + require.NoError(t, err) + + pak, err := db.CreatePreAuthKey(nil, false, false, &past, []string{"tag:ci"}) + require.NoError(t, err) + + // The reaper deletes the expired token row; the live token is untouched. n, err := db.DeleteExpiredAccessTokens(time.Now()) require.NoError(t, err) assert.Equal(t, int64(1), n) _ = token + _, err = db.AuthenticateAPIKey(apiKeyStr) + require.ErrorIs(t, err, ErrAPIKeyExpired, "expired API key must survive the token reaper") + + _, err = db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err, "expired pre-auth key must survive the token reaper") + _, err = db.AuthenticateAccessToken(tokenStr) require.NoError(t, err) } @@ -231,7 +325,7 @@ func TestAccessTokenRejectedWhenClientGone(t *testing.T) { // Delete only the client row, leaving the token orphaned (the state a // mint/revoke race or manual deletion would produce). - require.NoError(t, db.DB.Where("client_id = ?", client.ClientID).Delete(&types.OAuthClient{}).Error) + require.NoError(t, db.DB.Where("kind = ? AND identifier = ?", types.CredentialOAuthClient, client.ClientID).Delete(&types.Credential{}).Error) _, err = db.AuthenticateAccessToken(tokenStr) require.ErrorIs(t, err, ErrAccessTokenClientRevoked) @@ -244,8 +338,8 @@ func TestAccessTokenRejectedWhenClientGone(t *testing.T) { require.NoError(t, err) now := time.Now() - require.NoError(t, db.DB.Model(&types.OAuthClient{}). - Where("client_id = ?", client2.ClientID).Update("revoked", now).Error) + require.NoError(t, db.DB.Model(&types.Credential{}). + Where("kind = ? AND identifier = ?", types.CredentialOAuthClient, client2.ClientID).Update("revoked", now).Error) _, err = db.AuthenticateAccessToken(tokenStr2) require.ErrorIs(t, err, ErrAccessTokenClientRevoked) diff --git a/hscontrol/db/preauth_keys.go b/hscontrol/db/preauth_keys.go index f913314a..1389f774 100644 --- a/hscontrol/db/preauth_keys.go +++ b/hscontrol/db/preauth_keys.go @@ -1,6 +1,8 @@ package db import ( + "crypto/sha256" + "encoding/hex" "errors" "fmt" "slices" @@ -8,9 +10,7 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "golang.org/x/crypto/bcrypt" "gorm.io/gorm" - "tailscale.com/util/rands" "tailscale.com/util/set" ) @@ -57,11 +57,7 @@ func (hsdb *HSDatabase) CreatePreAuthKey( }) } -const ( - authKeyPrefix = "hskey-auth-" - authKeyPrefixLength = 12 - authKeyLength = 64 -) +const authKeyPrefix = "hskey-auth-" // CreatePreAuthKey creates a new [types.PreAuthKey] in a user, and returns it. // The uid parameter can be nil for system-created tagged keys. @@ -103,42 +99,35 @@ func CreatePreAuthKey( now := time.Now().UTC() - prefix := rands.HexString(authKeyPrefixLength) + keyStr, identifier, hash := generateSecret(authKeyPrefix) - toBeHashed := rands.HexString(authKeyLength) - - keyStr := authKeyPrefix + prefix + "-" + toBeHashed - - hash, err := bcrypt.GenerateFromPassword([]byte(toBeHashed), bcrypt.DefaultCost) - if err != nil { - return nil, err - } - - key := types.PreAuthKey{ + // Set only UserID for the write so GORM does not upsert the User row; the + // User is attached afterwards for the returned projection. + cred := types.Credential{ + Kind: types.CredentialPreAuthKey, + Identifier: identifier, + Hash: hash, UserID: userID, // nil for system-created keys, or "created by" for tagged keys - User: user, // nil for system-created keys Reusable: reusable, Ephemeral: ephemeral, CreatedAt: &now, Expiration: expiration, Tags: aclTags, // empty for user-owned keys - Prefix: prefix, // Store prefix - Hash: hash, // Store hash } - if err := tx.Save(&key).Error; err != nil { //nolint:noinlineerr + if err := tx.Save(&cred).Error; err != nil { //nolint:noinlineerr return nil, fmt.Errorf("creating key in database: %w", err) } return &types.PreAuthKeyNew{ - ID: key.ID, + ID: cred.ID, Key: keyStr, - Reusable: key.Reusable, - Ephemeral: key.Ephemeral, - Tags: key.Tags, - Expiration: key.Expiration, - CreatedAt: key.CreatedAt, - User: key.User, + Reusable: cred.Reusable, + Ephemeral: cred.Ephemeral, + Tags: cred.Tags, + Expiration: cred.Expiration, + CreatedAt: cred.CreatedAt, + User: user, // nil for system-created keys }, nil } @@ -146,9 +135,18 @@ func CreatePreAuthKey( // The v2 keys API sets it after creation rather than threading it through the // many-armed CreatePreAuthKey signature shared by every other caller. func (hsdb *HSDatabase) SetPreAuthKeyDescription(id uint64, description string) error { - return hsdb.DB.Model(&types.PreAuthKey{}). - Where("id = ?", id). - Update("description", description).Error + res := hsdb.DB.Model(&types.Credential{}). + Where("kind = ? AND id = ?", types.CredentialPreAuthKey, id). + Update("description", description) + if res.Error != nil { + return res.Error + } + + if res.RowsAffected == 0 { + return ErrPreAuthKeyNotFound + } + + return nil } func (hsdb *HSDatabase) ListPreAuthKeys() ([]types.PreAuthKey, error) { @@ -157,25 +155,39 @@ func (hsdb *HSDatabase) ListPreAuthKeys() ([]types.PreAuthKey, error) { // ListPreAuthKeys returns all [types.PreAuthKey] values in the database. func ListPreAuthKeys(tx *gorm.DB) ([]types.PreAuthKey, error) { - var keys []types.PreAuthKey + var creds []types.Credential - err := tx.Preload("User").Find(&keys).Error + err := tx.Preload("User"). + Where("kind = ?", types.CredentialPreAuthKey). + Find(&creds).Error if err != nil { return nil, err } + keys := make([]types.PreAuthKey, 0, len(creds)) + for i := range creds { + keys = append(keys, *credentialToPreAuthKey(&creds[i])) + } + return keys, nil } // ListPreAuthKeysByUser returns all [types.PreAuthKey] values belonging to a specific user. func ListPreAuthKeysByUser(tx *gorm.DB, uid types.UserID) ([]types.PreAuthKey, error) { - var keys []types.PreAuthKey + var creds []types.Credential - err := tx.Preload("User").Where("user_id = ?", uint(uid)).Find(&keys).Error + err := tx.Preload("User"). + Where("kind = ? AND user_id = ?", types.CredentialPreAuthKey, uint(uid)). + Find(&creds).Error if err != nil { return nil, err } + keys := make([]types.PreAuthKey, 0, len(creds)) + for i := range creds { + keys = append(keys, *credentialToPreAuthKey(&creds[i])) + } + return keys, nil } @@ -185,49 +197,45 @@ var ( ) func findAuthKey(tx *gorm.DB, keyStr string) (*types.PreAuthKey, error) { - var pak types.PreAuthKey - // Validate input is not empty if keyStr == "" { return nil, ErrPreAuthKeyFailedToParse } - _, prefixAndHash, found := strings.Cut(keyStr, authKeyPrefix) + // Unprefixed: a migrated pre-0.28 plaintext key. + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). + identifier, secret := legacyAuthKeyIdentifier(keyStr), keyStr - if !found { - // Legacy format (plaintext) - backwards compatibility - err := tx.Preload("User").First(&pak, "key = ?", keyStr).Error + _, prefixAndSecret, found := strings.Cut(keyStr, authKeyPrefix) + if found { + var err error + + identifier, secret, err = parsePrefixedKey(prefixAndSecret, ErrPreAuthKeyFailedToParse) if err != nil { - return nil, ErrPreAuthKeyNotFound + return nil, err } - - return &pak, nil } - // New format: hskey-auth-{12-char-prefix}-{64-char-hash} - prefix, hash, err := parsePrefixedKey( - prefixAndHash, - authKeyPrefixLength, - authKeyLength, - ErrPreAuthKeyFailedToParse, + cred, err := authenticateCredential( + tx, types.CredentialPreAuthKey, identifier, secret, ErrPreAuthKeyNotFound, "User", ) if err != nil { return nil, err } - // Look up key by prefix - err = tx.Preload("User").First(&pak, "prefix = ?", prefix).Error - if err != nil { - return nil, ErrPreAuthKeyNotFound - } + return credentialToPreAuthKey(cred), nil +} - // Verify hash matches - err = bcrypt.CompareHashAndPassword(pak.Hash, []byte(hash)) - if err != nil { - return nil, fmt.Errorf("invalid auth key: %w", err) - } +// legacyAuthKeyIdentifier derives the lookup identifier of a pre-0.28 plaintext +// pre-auth key, which has no embedded identifier. The migration stores these +// keys under it with the whole key hashed as the secret; the "legacy-" prefix +// keeps it disjoint from the hex identifiers of current keys. +// +// TODO(kradalby): remove in 0.32 with legacy key formats (announced). +func legacyAuthKeyIdentifier(key string) string { + sum := sha256.Sum256([]byte(key)) - return &pak, nil + return "legacy-" + hex.EncodeToString(sum[:])[:keyIdentifierLength] } // parsePrefixedKey splits the prefix-and-secret portion of a new-format key @@ -235,12 +243,12 @@ func findAuthKey(tx *gorm.DB, keyStr string) (*types.PreAuthKey, error) { // secret components, validating the length, separator position, and that both // components are base64 URL-safe. Fixed-length parsing is used instead of // separator-based to handle dashes in base64 URL-safe characters. -func parsePrefixedKey( - prefixAndSecret string, - //nolint:unparam // kept explicit though every credential kind uses a 12-char prefix and 64-char secret today - prefixLen, secretLen int, - parseErr error, -) (string, string, error) { +func parsePrefixedKey(prefixAndSecret string, parseErr error) (string, string, error) { + const ( + prefixLen = keyIdentifierLength + secretLen = keySecretLength + ) + expectedMinLength := prefixLen + 1 + secretLen if len(prefixAndSecret) < expectedMinLength { return "", "", fmt.Errorf( @@ -295,9 +303,10 @@ func parsePrefixedKey( } // isValidBase64URLSafe reports whether s contains only base64 URL-safe -// characters (A-Za-z0-9-_). Key material is now generated as hex, a subset of -// this alphabet, so this accepts both current hex keys and any legacy keys -// still stored in the database. +// characters (A-Za-z0-9-_): current keys are hex, older ones used the wider +// base64url alphabet. +// +// TODO(kradalby): accept hex only in 0.32, with legacy key formats. func isValidBase64URLSafe(s string) bool { return !strings.ContainsFunc(s, func(c rune) bool { return (c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9') && c != '-' && c != '_' @@ -317,15 +326,16 @@ func GetPreAuthKey(tx *gorm.DB, key string) (*types.PreAuthKey, error) { // GetPreAuthKeyByID returns a [types.PreAuthKey] by its primary key, with the // owning user preloaded. func (hsdb *HSDatabase) GetPreAuthKeyByID(id uint64) (*types.PreAuthKey, error) { - pak := types.PreAuthKey{} + var cred types.Credential // Explicit primary-key clause: a struct condition would drop a zero-valued // ID, making the lookup unconditional and returning the first row instead // of not-found. - if result := hsdb.DB.Preload("User").First(&pak, "id = ?", id); result.Error != nil { + if result := hsdb.DB.Preload("User"). + First(&cred, "kind = ? AND id = ?", types.CredentialPreAuthKey, id); result.Error != nil { return nil, result.Error } - return &pak, nil + return credentialToPreAuthKey(&cred), nil } // DestroyPreAuthKey destroys a preauthkey. Returns error if the [types.PreAuthKey] @@ -343,7 +353,8 @@ func DestroyPreAuthKey(tx *gorm.DB, id uint64) error { // Then delete the pre-auth key, on the same savepoint as the // node update so both roll back together. - res := db.Unscoped().Delete(&types.PreAuthKey{}, id) + res := db.Unscoped(). + Delete(&types.Credential{}, "kind = ? AND id = ?", types.CredentialPreAuthKey, id) if res.Error != nil { return res.Error } @@ -380,8 +391,8 @@ func (hsdb *HSDatabase) RevokePreAuthKey(id uint64) error { // window. An already-revoked or unknown id returns [ErrPreAuthKeyNotFound], so a // repeated DELETE is a clean 404. func RevokePreAuthKey(tx *gorm.DB, id uint64) error { - res := tx.Model(&types.PreAuthKey{}). - Where("id = ? AND revoked IS NULL", id). + res := tx.Model(&types.Credential{}). + Where("kind = ? AND id = ? AND revoked IS NULL", types.CredentialPreAuthKey, id). Update("revoked", time.Now()) if res.Error != nil { return res.Error @@ -396,15 +407,18 @@ func RevokePreAuthKey(tx *gorm.DB, id uint64) error { // DestroyRevokedPreAuthKeysBefore hard-deletes every key revoked before cutoff, // returning how many were removed. The background collector calls this to reap -// soft-revoked keys after the retention window. +// soft-revoked keys after the retention window. Keys still referenced by a node +// are kept: the node's ephemerality lives on its key, and revoking a key must +// not change nodes already registered with it. func (hsdb *HSDatabase) DestroyRevokedPreAuthKeysBefore(cutoff time.Time) (int, error) { var count int err := hsdb.Write(func(tx *gorm.DB) error { var ids []uint64 - err := tx.Model(&types.PreAuthKey{}). - Where("revoked IS NOT NULL AND revoked < ?", cutoff). + err := tx.Model(&types.Credential{}). + Where("kind = ? AND revoked IS NOT NULL AND revoked < ?", types.CredentialPreAuthKey, cutoff). + Where("id NOT IN (SELECT auth_key_id FROM nodes WHERE auth_key_id IS NOT NULL)"). Pluck("id", &ids).Error if err != nil { return err @@ -432,8 +446,8 @@ func (hsdb *HSDatabase) DestroyRevokedPreAuthKeysBefore(cutoff time.Time) (int, // guard the previous code (Update("used", true) with no WHERE) would // silently let both transactions claim the key. func UsePreAuthKey(tx *gorm.DB, k *types.PreAuthKey) error { - res := tx.Model(&types.PreAuthKey{}). - Where("id = ? AND used = ?", k.ID, false). + res := tx.Model(&types.Credential{}). + Where("kind = ? AND id = ? AND used = ?", types.CredentialPreAuthKey, k.ID, false). Update("used", true) if res.Error != nil { return fmt.Errorf("updating key used status in database: %w", res.Error) @@ -453,7 +467,9 @@ func UsePreAuthKey(tx *gorm.DB, k *types.PreAuthKey) error { func ExpirePreAuthKey(tx *gorm.DB, id uint64) error { now := time.Now() - res := tx.Model(&types.PreAuthKey{}).Where("id = ?", id).Update("expiration", now) + res := tx.Model(&types.Credential{}). + Where("kind = ? AND id = ?", types.CredentialPreAuthKey, id). + Update("expiration", now) if res.Error != nil { return res.Error } diff --git a/hscontrol/db/preauth_keys_test.go b/hscontrol/db/preauth_keys_test.go index 5835758f..0068c9f4 100644 --- a/hscontrol/db/preauth_keys_test.go +++ b/hscontrol/db/preauth_keys_test.go @@ -1,7 +1,6 @@ package db import ( - "fmt" "slices" "strings" "testing" @@ -11,6 +10,7 @@ import ( "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" "gorm.io/gorm" ) @@ -131,7 +131,8 @@ func TestCannotDeleteAssignedPreAuthKey(t *testing.T) { } db.DB.Save(&node) - err = db.DB.Delete(&types.PreAuthKey{ID: key.ID}).Error + err = db.DB.Where("kind = ? AND id = ?", types.CredentialPreAuthKey, key.ID). + Delete(&types.Credential{}).Error require.ErrorContains(t, err, "constraint failed: FOREIGN KEY constraint failed") } @@ -145,40 +146,24 @@ func TestPreAuthKeyAuthentication(t *testing.T) { name string setupKey func() string // Returns key string to test wantFindErr bool // Error when finding the key + findErrIs error // Specific error expected when finding, if any wantValidateErr bool // Error when validating the key validateResult func(*testing.T, *types.PreAuthKey) }{ { - name: "legacy_key_plaintext", + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). + name: "unknown_unprefixed_key_rejected", setupKey: func() string { - // Insert legacy key directly using GORM (simulate existing production key) - // Note: We use raw SQL to bypass GORM's handling and set prefix to empty string - // which simulates how legacy keys exist in production databases - legacyKey := "abc123def456ghi789jkl012mno345pqr678stu901vwx234yz" - now := time.Now() - - // Use raw SQL to insert with empty prefix to avoid UNIQUE constraint - err := db.DB.Exec(` - INSERT INTO pre_auth_keys (key, user_id, reusable, ephemeral, used, created_at) - VALUES (?, ?, ?, ?, ?, ?) - `, legacyKey, user.ID, true, false, false, now).Error - require.NoError(t, err) - - return legacyKey + // An unprefixed string is looked up as a migrated legacy + // plaintext key; one that was never stored is unknown. + return "abc123def456ghi789jkl012mno345pqr678stu901vwx234yz" }, - wantFindErr: false, + wantFindErr: true, + findErrIs: ErrPreAuthKeyNotFound, wantValidateErr: false, - validateResult: func(t *testing.T, pak *types.PreAuthKey) { - t.Helper() - - assert.Equal(t, user.ID, *pak.UserID) - assert.NotEmpty(t, pak.Key) // Legacy keys have Key populated - assert.Empty(t, pak.Prefix) // Legacy keys have empty Prefix - assert.Nil(t, pak.Hash) // Legacy keys have nil Hash - }, }, { - name: "new_key_bcrypt", + name: "new_key_sha256", setupKey: func() string { // Create new key via API keyStr, err := db.CreatePreAuthKey( @@ -195,10 +180,8 @@ func TestPreAuthKeyAuthentication(t *testing.T) { t.Helper() assert.Equal(t, user.ID, *pak.UserID) - assert.Empty(t, pak.Key) // New keys have empty Key - assert.NotEmpty(t, pak.Prefix) // New keys have Prefix - assert.NotNil(t, pak.Hash) // New keys have Hash - assert.Len(t, pak.Prefix, 12) // Prefix is 12 chars + assert.Len(t, pak.Prefix, 12) + assert.True(t, strings.HasPrefix(string(pak.Hash), hashPrefixSHA256)) }, }, { @@ -231,7 +214,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) { wantValidateErr: false, }, { - name: "invalid_bcrypt_hash", + name: "wrong_secret", setupKey: func() string { // Create valid key key, err := db.CreatePreAuthKey( @@ -242,13 +225,15 @@ func TestPreAuthKeyAuthentication(t *testing.T) { keyStr := key.Key - // Return key with tampered hash using fixed-length parsing + // Well-formed key with the right identifier but a wrong secret: + // must read as an unknown key (401), not a server error. _, prefixAndHash, _ := strings.Cut(keyStr, "hskey-auth-") prefix := prefixAndHash[:12] - return "hskey-auth-" + prefix + "-" + "wrong_hash_here_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + return "hskey-auth-" + prefix + "-" + strings.Repeat("0", 64) }, wantFindErr: true, + findErrIs: errSecretMismatch, wantValidateErr: false, }, { @@ -308,43 +293,6 @@ func TestPreAuthKeyAuthentication(t *testing.T) { wantFindErr: true, wantValidateErr: false, }, - { - name: "expired_legacy_key", - setupKey: func() string { - legacyKey := "expired_legacy_key_123456789012345678901234" - now := time.Now() - expiration := time.Now().Add(-1 * time.Hour) // Expired 1 hour ago - - // Use raw SQL to avoid UNIQUE constraint on empty prefix - err := db.DB.Exec(` - INSERT INTO pre_auth_keys (key, user_id, reusable, ephemeral, used, created_at, expiration) - VALUES (?, ?, ?, ?, ?, ?, ?) - `, legacyKey, user.ID, true, false, false, now, expiration).Error - require.NoError(t, err) - - return legacyKey - }, - wantFindErr: false, - wantValidateErr: true, - }, - { - name: "used_single_use_legacy_key", - setupKey: func() string { - legacyKey := "used_legacy_key_123456789012345678901234567" - now := time.Now() - - // Use raw SQL to avoid UNIQUE constraint on empty prefix - err := db.DB.Exec(` - INSERT INTO pre_auth_keys (key, user_id, reusable, ephemeral, used, created_at) - VALUES (?, ?, ?, ?, ?, ?) - `, legacyKey, user.ID, false, false, true, now).Error - require.NoError(t, err) - - return legacyKey - }, - wantFindErr: false, - wantValidateErr: true, - }, } for _, tt := range tests { @@ -354,7 +302,13 @@ func TestPreAuthKeyAuthentication(t *testing.T) { pak, err := db.GetPreAuthKey(keyStr) if tt.wantFindErr { - assert.Error(t, err) + require.Error(t, err) + + if tt.findErrIs != nil { + require.ErrorIs(t, err, tt.findErrIs) + require.ErrorIs(t, err, gorm.ErrRecordNotFound, "must map to 401") + } + return } @@ -376,74 +330,30 @@ func TestPreAuthKeyAuthentication(t *testing.T) { } } -func TestMultipleLegacyKeysAllowed(t *testing.T) { +// TestPreAuthKeysHaveUniqueIdentifiers verifies that freshly created pre-auth +// keys get distinct identifiers in the unified credentials table. +func TestPreAuthKeysHaveUniqueIdentifiers(t *testing.T) { db, err := newSQLiteTestDB() require.NoError(t, err) - user, err := db.CreateUser(types.User{Name: "test-legacy"}) + user, err := db.CreateUser(types.User{Name: "test-unique"}) require.NoError(t, err) - // Create multiple legacy keys by directly inserting with empty prefix - // This simulates the migration scenario where existing databases have multiple - // plaintext keys without prefix/hash fields - now := time.Now() - - for i := range 5 { - legacyKey := fmt.Sprintf("legacy_key_%d_%s", i, strings.Repeat("x", 40)) - - err := db.DB.Exec(` - INSERT INTO pre_auth_keys (key, prefix, hash, user_id, reusable, ephemeral, used, created_at) - VALUES (?, '', NULL, ?, ?, ?, ?, ?) - `, legacyKey, user.ID, true, false, false, now).Error - require.NoError(t, err, "should allow multiple legacy keys with empty prefix") - } - - // Verify all legacy keys can be retrieved - var legacyKeys []types.PreAuthKey - - err = db.DB.Where("prefix = '' OR prefix IS NULL").Find(&legacyKeys).Error - require.NoError(t, err) - assert.Len(t, legacyKeys, 5, "should have created 5 legacy keys") - - // Now create new bcrypt-based keys - these should have unique prefixes key1, err := db.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) require.NoError(t, err) - assert.NotEmpty(t, key1.Key) key2, err := db.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) require.NoError(t, err) - assert.NotEmpty(t, key2.Key) - // Verify the new keys have different prefixes pak1, err := db.GetPreAuthKey(key1.Key) require.NoError(t, err) - assert.NotEmpty(t, pak1.Prefix) pak2, err := db.GetPreAuthKey(key2.Key) require.NoError(t, err) + + assert.NotEmpty(t, pak1.Prefix) assert.NotEmpty(t, pak2.Prefix) - - assert.NotEqual(t, pak1.Prefix, pak2.Prefix, "new keys should have unique prefixes") - - // Verify we cannot manually insert duplicate non-empty prefixes - duplicatePrefix := "test_prefix1" - hash1 := []byte("hash1") - hash2 := []byte("hash2") - - // First insert should succeed - err = db.DB.Exec(` - INSERT INTO pre_auth_keys (key, prefix, hash, user_id, reusable, ephemeral, used, created_at) - VALUES ('', ?, ?, ?, ?, ?, ?, ?) - `, duplicatePrefix, hash1, user.ID, true, false, false, now).Error - require.NoError(t, err, "first key with prefix should succeed") - - // Second insert with same prefix should fail - err = db.DB.Exec(` - INSERT INTO pre_auth_keys (key, prefix, hash, user_id, reusable, ephemeral, used, created_at) - VALUES ('', ?, ?, ?, ?, ?, ?, ?) - `, duplicatePrefix, hash2, user.ID, true, false, false, now).Error - require.Error(t, err, "duplicate non-empty prefix should be rejected") - assert.Contains(t, err.Error(), "UNIQUE constraint failed", "should fail with UNIQUE constraint error") + assert.NotEqual(t, pak1.Prefix, pak2.Prefix, "new keys should have unique identifiers") } // TestUsePreAuthKeyAtomicCAS verifies that UsePreAuthKey is an atomic @@ -500,3 +410,42 @@ func TestGetPreAuthKeyUnknownMapsToRecordNotFound(t *testing.T) { require.ErrorIs(t, err, gorm.ErrRecordNotFound, "unknown pre-auth key must map to record-not-found (handled as 401)") } + +// TestPreAuthKeyLazyRehashesBcrypt seeds a new-format key whose secret is stored +// as a legacy bcrypt hash and asserts that authenticating it upgrades the stored +// hash to SHA-256, while continuing to authenticate. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. +func TestPreAuthKeyLazyRehashesBcrypt(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + user := db.CreateUserForTest("rehash-user") + + prefix := "abcdefghijkl" + secret := strings.Repeat("b", 64) + keyStr := "hskey-auth-" + prefix + "-" + secret + + hash, err := bcrypt.GenerateFromPassword([]byte(secret), bcrypt.MinCost) + require.NoError(t, err) + + err = db.DB.Exec( + `INSERT INTO credentials (kind, identifier, hash, user_id, reusable, ephemeral, used, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + types.CredentialPreAuthKey, prefix, hash, user.ID, true, false, false, time.Now(), + ).Error + require.NoError(t, err) + + pak, err := db.GetPreAuthKey(keyStr) + require.NoError(t, err) + require.NotNil(t, pak) + + reloaded, err := db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(string(reloaded.Hash), hashPrefixSHA256), + "a bcrypt-stored key must be rehashed to SHA-256 on first auth") + + // Still authenticates against the upgraded hash. + _, err = db.GetPreAuthKey(keyStr) + require.NoError(t, err) +} diff --git a/hscontrol/db/schema.sql b/hscontrol/db/schema.sql index 59b60160..f6fbd45a 100644 --- a/hscontrol/db/schema.sql +++ b/hscontrol/db/schema.sql @@ -38,67 +38,35 @@ CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier); CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL; -CREATE TABLE pre_auth_keys( +-- Unified store for every authenticatable secret (API keys, pre-auth keys, +-- OAuth clients and access tokens), discriminated by kind. Only a hash of the +-- secret is stored (SHA-256; legacy bcrypt/Argon2id until next use); identifier +-- is the public lookup value, unique within a kind. Per-kind columns are sparse +-- by design. +CREATE TABLE credentials( id integer PRIMARY KEY AUTOINCREMENT, - key text, - prefix text, + kind text NOT NULL CHECK(kind IN ('api','authkey','oauth_client','oauth_token')), + identifier text, hash blob, user_id integer, description text, + scopes text, + tags text, reusable numeric, ephemeral numeric DEFAULT false, used numeric DEFAULT false, - tags text, + last_seen datetime, + client_id text, + created_at datetime, expiration datetime, revoked datetime, - created_at datetime, - - CONSTRAINT fk_pre_auth_keys_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE SET NULL + CONSTRAINT fk_credentials_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE SET NULL, + CONSTRAINT chk_credentials_hash CHECK(hash IS NOT NULL OR revoked IS NOT NULL), + CONSTRAINT chk_credentials_hash_format CHECK(hash IS NULL OR substr(CAST(hash AS TEXT), 1, 1) = '$') ); -CREATE UNIQUE INDEX idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != ''; - -CREATE TABLE api_keys( - id integer PRIMARY KEY AUTOINCREMENT, - prefix text, - hash blob, - user_id integer, - expiration datetime, - last_seen datetime, - - created_at datetime -); -CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix); - --- OAuth 2.0 client-credentials clients for the v2 API. client_id is public and --- embedded in the secret (hskey-client--); only the bcrypt --- hash of the secret is stored. Mirrors the api_keys security model. -CREATE TABLE oauth_clients( - id integer PRIMARY KEY AUTOINCREMENT, - client_id text, - secret_hash blob, - scopes text, - tags text, - description text, - user_id integer, - created_at datetime, - revoked datetime -); -CREATE UNIQUE INDEX idx_oauth_clients_client_id ON oauth_clients(client_id); - --- Short-lived bearer access tokens minted by an oauth_client. Stored as a bcrypt --- hash of the secret, looked up by prefix. -CREATE TABLE oauth_access_tokens( - id integer PRIMARY KEY AUTOINCREMENT, - prefix text, - hash blob, - client_id text, - scopes text, - tags text, - expiration datetime, - created_at datetime -); -CREATE UNIQUE INDEX idx_oauth_access_tokens_prefix ON oauth_access_tokens(prefix); +CREATE UNIQUE INDEX idx_credentials_identifier ON credentials(kind, identifier); +CREATE INDEX idx_credentials_user_id ON credentials(user_id); CREATE TABLE nodes( id integer PRIMARY KEY AUTOINCREMENT, @@ -127,8 +95,9 @@ CREATE TABLE nodes( deleted_at datetime, CONSTRAINT fk_nodes_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE, - CONSTRAINT fk_nodes_auth_key FOREIGN KEY(auth_key_id) REFERENCES pre_auth_keys(id) + CONSTRAINT fk_nodes_auth_key FOREIGN KEY(auth_key_id) REFERENCES credentials(id) ); +CREATE INDEX idx_nodes_auth_key_id ON nodes(auth_key_id); CREATE TABLE policies( id integer PRIMARY KEY AUTOINCREMENT, diff --git a/hscontrol/db/secret.go b/hscontrol/db/secret.go new file mode 100644 index 00000000..9b183461 --- /dev/null +++ b/hscontrol/db/secret.go @@ -0,0 +1,216 @@ +package db + +import ( + "bytes" + "crypto/sha256" + "crypto/subtle" + "encoding/base64" + "encoding/hex" + "errors" + "fmt" + "runtime" + "strings" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/rs/zerolog/log" + "golang.org/x/crypto/argon2" + "golang.org/x/crypto/bcrypt" + "gorm.io/gorm" + "tailscale.com/util/rands" +) + +// Every credential is a "-" string, where +// the prefix is one of hskey-api-, hskey-auth-, hskey-client- or +// hskey-oauthtok-. The identifier is the public, indexed lookup key and only +// the secret is hashed. +const ( + keyIdentifierLength = 12 + keySecretLength = 64 +) + +// hashPrefixSHA256 marks the current hash format. Secrets hold 256 bits of +// crypto/rand entropy and are never user-chosen, so recovering one from its +// SHA-256 digest means searching that whole space; security comes from the +// entropy, not hash cost. Password stretching (bcrypt, Argon2id) defends +// guessable secrets and would only add cost to every authentication. The same +// entropy reasoning underlies NIST SP 800-63B-4 §3.1.2.2 (look-up secrets need +// a salted password hash only below 112 bits) and RFC 6819 §5.1.4.1.3 (salt +// hardens low-entropy credentials such as passwords). +const hashPrefixSHA256 = "$sha256$" + +// Bounds for legacy Argon2id hashes read back from storage, so a corrupt row +// cannot panic argon2 or allocate unbounded memory. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. +const ( + argon2KeyLen = 32 + argon2MaxMemory = 64 * 1024 +) + +var ( + errSecretHashMalformed = errors.New("malformed secret hash") + errSecretMismatch = errors.New("secret does not match hash") +) + +// legacyHashLimiter bounds concurrent bcrypt/Argon2id verifications. Both are +// deliberately expensive and reachable from unauthenticated endpoints; they +// only run until every stored hash has been upgraded to SHA-256. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. +var legacyHashLimiter = make(chan struct{}, max(2, runtime.GOMAXPROCS(0))) + +// generateSecret builds a new credential string prefix+identifier+"-"+secret, +// returning the full string (shown ONCE to the user), the public identifier +// used for lookup, and the hash of the secret to store. +func generateSecret(prefix string) (string, string, []byte) { + identifier := rands.HexString(keyIdentifierLength) + secret := rands.HexString(keySecretLength) + + return prefix + identifier + "-" + secret, identifier, hashSecret(secret) +} + +// hashSecret returns the storage form of a credential secret. +func hashSecret(secret string) []byte { + sum := sha256.Sum256([]byte(secret)) + + return []byte(hashPrefixSHA256 + hex.EncodeToString(sum[:])) +} + +// authenticateCredential looks up the credential of kind by its public +// identifier and verifies secret against it, upgrading a legacy hash on +// success. An unknown identifier and a wrong secret both return notFound, so a +// caller cannot tell which half of the key was wrong. preloads names the +// associations the caller needs, so the common path stays one query. +func authenticateCredential( + tx *gorm.DB, + kind types.CredentialKind, + identifier, secret string, + notFound error, + preloads ...string, +) (*types.Credential, error) { + var cred types.Credential + + query := tx + for _, p := range preloads { + query = query.Preload(p) + } + + err := query.First(&cred, "kind = ? AND identifier = ?", kind, identifier).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return nil, notFound + } + + if err != nil { + return nil, fmt.Errorf("looking up %s credential: %w", kind, err) + } + + needsRehash, err := verifySecret(cred.Hash, secret) + if errors.Is(err, errSecretMismatch) { + return nil, fmt.Errorf("%w: %w", notFound, err) + } + + if err != nil { + return nil, fmt.Errorf("verifying %s credential %d: %w", kind, cred.ID, err) + } + + // TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. + if needsRehash { + // Best effort: on failure the legacy hash stays and the next + // authentication retries. + cred.Hash = hashSecret(secret) + + err := tx.Model(&types.Credential{}).Where("id = ?", cred.ID). + Update("hash", cred.Hash).Error + if err != nil { + log.Warn().Err(err).Uint64("credential", cred.ID). + Msg("upgrading legacy credential hash") + } + } + + return &cred, nil +} + +// verifySecret reports whether secret matches a stored hash. Legacy bcrypt and +// Argon2id hashes still verify and return needsRehash=true so the caller can +// upgrade them to SHA-256. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. Any +// credential not rehashed by then stops authenticating (pre-announced). +func verifySecret(encoded []byte, secret string) (bool, error) { + if hexSum, ok := bytes.CutPrefix(encoded, []byte(hashPrefixSHA256)); ok { + want, err := hex.DecodeString(string(hexSum)) + if err != nil || len(want) != sha256.Size { + return false, errSecretHashMalformed + } + + got := sha256.Sum256([]byte(secret)) + if subtle.ConstantTimeCompare(got[:], want) != 1 { + return false, errSecretMismatch + } + + return false, nil + } + + legacyHashLimiter <- struct{}{} + defer func() { <-legacyHashLimiter }() + + if bytes.HasPrefix(encoded, []byte("$argon2id$")) { + err := verifyArgon2id(encoded, secret) + + return err == nil, err + } + + switch err := bcrypt.CompareHashAndPassword(encoded, []byte(secret)); { + case err == nil: + return true, nil + case errors.Is(err, bcrypt.ErrMismatchedHashAndPassword): + return false, errSecretMismatch + default: + return false, errSecretHashMalformed + } +} + +// verifyArgon2id checks secret against a PHC-encoded Argon2id hash, rejecting +// parameters argon2 would panic on or that would allocate unbounded memory. +// +// TODO(kradalby): remove in 0.32 with bcrypt/Argon2id support. +func verifyArgon2id(encoded []byte, secret string) error { + parts := strings.Split(string(encoded), "$") + if len(parts) != 6 || parts[1] != "argon2id" { + return errSecretHashMalformed + } + + var version int + + _, err := fmt.Sscanf(parts[2], "v=%d", &version) + if err != nil || version != argon2.Version { + return errSecretHashMalformed + } + + var ( + memory, time uint32 + threads uint8 + ) + + _, err = fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &memory, &time, &threads) + if err != nil || time < 1 || threads < 1 || memory > argon2MaxMemory { + return errSecretHashMalformed + } + + salt, err := base64.RawStdEncoding.DecodeString(parts[4]) + if err != nil || len(salt) == 0 { + return errSecretHashMalformed + } + + want, err := base64.RawStdEncoding.DecodeString(parts[5]) + if err != nil || len(want) != argon2KeyLen { + return errSecretHashMalformed + } + + got := argon2.IDKey([]byte(secret), salt, time, memory, threads, argon2KeyLen) + if subtle.ConstantTimeCompare(got, want) != 1 { + return errSecretMismatch + } + + return nil +} diff --git a/hscontrol/db/testdata/postgres/headscale_0.29.3.pssql b/hscontrol/db/testdata/postgres/headscale_0.29.3.pssql new file mode 100644 index 0000000000000000000000000000000000000000..4b81f6b9837591cda9515033dc7004aadc17a3e8 GIT binary patch literal 19319 zcmc&+3tSY{{vXhMaxain+ASSJ1#T;{Pr&lA!lJ9NAS{)p>|J&TS%uwocTw=FnfXY~ z%4eaVqBndLlA7kTt7(~ODWzE#EFXo}%5K{2s+ILW=giE`?3ptQ2!8(a`8e#%dH%lV z_xqjS?{|J<*6`GftdIz*eW#ER>Rt+5vG5iSZx6s*CwQ9)Z|Ld_?uCri?zLHq7^*#Z zQYOXggddN=S~ZdI4zCdKFM+$mo(!gx%v7UkIO^8_5DKgI1pjgjX4+QdV4OY+_Ss-nJ5_)CH8}m{W7i*u0x^7^b!yVYa2*j&y6m!}v^&EoZWz#lK1qRZLJ3D(L_(4fyw8Ih$ok6b90P?WT>{>uWE=Em1I-uEezu~YG@3`yWNVLfuBpS66^T)B9wyW+#>J=_&RWn5$HK)eYTlpRV8|1G=bI`29uc{Hil+j znKJ1Nqv;8KdaeOgfzSGKeer*b8F{b&`&P1lfVM%(r?O%M`}r1xzmy%z2;^NiZc1n~+ehe-mdKOfd5iYr2?t+vJ0J9)UncN(t zXtszU6EKF{P87GGPzI5^TtE$3?&Qdk2%w~5+eo4^86*50m@0eWtdy)nR!y;E0=w@> zL~s;nM-oAyg&=#jLaWDW^D!QJiq%sFY_nLERvYIRI3!2V&XmfL(3^D0g^9ONfWBc{ zf;&dFM#CCQPM4iCCq%|T9I%i`1f$9Vtwc+p*lH_uI2mYUh|B@l5_@HLcx^819pkjS zUBJ5VikcPzo5$h+7O&`?!#zb4_^1;-!WK?zF(Z04!7&9`Vd$lFg>>pT*7hjzG9I8n zI1`6`=V2x|ykI>o#f-1eWf%47vjW3guz0K`zCuV2-T^Xb02V>5o~Df%b~aYG+vA!7 zbhXD-;$y_aH`~B!hPG$NvNJ_YbG8M)JsV(QAzo=iPX1b+la*8$C`1FwtACNbkJ||1 zmUltrM}x@`Rlay!DN-#`^SHL#K~(b+n1ZPGL15No)h?d4M4h9;wo&1#Bus^0mSq9p z`LRYPa9JEueRdc$B=>;O8b8iElvi3d5CmO5MPF;LdrrcUQqM5aNB51H-g#a05{0n>nOMk>ebfo5xsW<)%Y^!El3OU=dr z(53v$uGt-^fb(oiaK|l2rJ9NYuLA~_*8?*^Rc_t=4A1%Iq#o{qGI$isfVlsX#+M@4 z5{;nJVj5v5Q6tF07i2%q0926oV-gus-5?gTwowo&4Myx&V2bR8vrK{-g(0CpN)&@i zMlqnuO(=wB%UD|)y{tCC91P&ifBtYB_0aI*s6p5S!XuUh^$jbmn42}W^a1@2w zd}-jT0!I;JaXW055)a!I(gtL!8-XU}VPRETXb61Cuc$-|B&d+pjRusL{vcMjxaD1t zdRePmJgyX}7D>Ar6M$VrX@^#~c+g;*zr58g?m;5msIZNsT7?nyV+79wP^wkB1dabl zK|^%~ulP7EI2@olf@f~)WUZNr;03F;Snn{?ls*QJz0z!jl+5(p3==$JfTvyj2J8e! z3RA2_u`%R;Zp^@ez(cnI|Je(p{R6S#utt)R(7wP8)kA@?GOR{Io8zK?YVDinjY6+I&UAcMkO&+5SDZu9DQ)6Jhic%-Z4}h5{tb&K>5vVWJ5xP-Ng3^@CEa_Gr>#!?mz?&c&KtaPK z00k`==?Yr#brrNf<{d|;WoKr9{1%XH1NQI^pl_DpL<53%a_8_Io$SGKsMCO|Qq)=^ zo$}|d!3j+9I8**z02fav&1LBsd(NJc^X51RlB@@GtdKl!621r1M> z&|SyAhVGq$7LJX;in5183K}~RQqVB)qoDc77tp92pM!DzJPyhOk8T3xLSPp#SO2+7 z+u+Y`@S#(NbF8z4j6WVj555Fh*b?}6T(k;ektdS|$ONJ7HX)GPc|$29N8mrm1QMf^ z!Ks{uQz_m$p@b-C#9o04PsAx~uo3VBA7Dg$1e&otBgRQkjctMo7m5mfm`7zj8I^$0 zOmH-oVrZ~iS+f_M6)AC`1mFVWBXGlzghhmx=$AMM(l+?8&Us1=*16}g@ z`{-wjQds|sd!syR>SQhYm_ec$k?|9d_9>5J0ho{+j*^Z^fJ!RKBAeZkp(GHK$7vokqE(6Pina6pB#Ktwaz{QfMzL zl;OfxlW^jGcv@IT7^NRYlJ z^&<~?aGsSC(n?(AbmK4bxT;W+3*=qa5D?@C2Q^&)Rgh`Ckw~BkPzs$xmV>8UeEt;v)kZtq!pn>(>-a0{bxd4$GfCjnb*pMv1~Atd%kw5F+PLq8>C-9ByKRDx!0%I6B*q2DW4HL2?dn;sgjKi1kA}^-WDfVkb-HAO*UahrKW` zrBQ3StP8(@;~rcWdE}MICd813j39^loTkVPiaP8$5OMeh?8_vy5kEqdA$zBS(**Fp z52WaGQuL_?fV+?#C=IEx^x!tPA9vVsD>2vjGlpa`3p&YS4web#V+QaN4dxc`0_g?0 zHCPd34FYu7DGZHT zgB>u0P$>W7RRCvcMaIY5y1bhecF(Cj|9Q0GwQu_FNxb5XYOnWAzM8*z{P;7qi5DN4 zbMPZ0{mJ-OZtH(N`fKUT%Ds+he@!31eNpb*MFS$E6c^IM-}usfbkM&i9!!5EanSnL zlFszGKD+$o1^-P;S+qKO)`|^V(_To9^eVbnzC1L-yD|4bZOxPQou+=ay@P9MucApA zJJ)_TBsb*NnJd}VRm-o3R4?xjbsWr1LHQfd?k}xSf2~lTQg1)4Zq%p}RG(_!Jgrqd zk)YYB+SUG%LG4CGM@M%nZr5&W_d)UT@zl|Eo!Cr+5DFzp{0SWAl~$Cze8_Y-A#C1% zW;_sGpRw}l^QZbunbq~7C0ULG{nt+#x$52bKfSbd^PD+J2Fq`kcT{fN_v_WeSHJsV z@{Mr`+YR~E+I?Yt>pm>_n{T&rb=X*s{eJgb)#JA9?wu1qbl%91l^gYG`$Ma?)V$W? z`K&`jyQPFz6(3BzJWgM^&Ub3~=J$QmU+KF4%(YK8e$(6Q2un?#oB6Eiov~kazPWXY zVcl~N+N!!PJ~;Tu*at_PvCfLxdU^R+N4Ik!k*41xUhm@$OSyh-@TQx#<-Kn&R9$-O zp3^nv$$N)vx;C*_#{~m#J+$MOm#W`N{$Kym^|zLtFecv^oY#1Md#kwd9Ehq zW@wKATY8P0Z4BGhtyAx*FLhWv?D6QUtLF8p8g^z#uXiuh=lrT2So&xE=~wA{V@A6U zzVMUxQfO{nKbLVs<6rG@pB<~~diHOMg!(s7lp$UDMyRv@vC8)#;VdlfrT; zH;&$|`!Fv;^Yk`~-nQ=9Z})GSIivr{%NZX=XuR7luGq2UWXydo=c`X2Ul6n7p;4!n z+P35!S^oJr?;6dVvQIZ2y>Me~NXU!*h9_N2ICDOF{mQy+hmzuJ&K^1R%>EhWuEEbd zfB1@(ruWVFW&RdYV(%U@B(&n_W6LVfZk*q)#9Ezq;`HM3t>stdl{pSHK34Frg6i?` z)>S>#+L(8uVR3nF`7K!1@AkmG1>Z`{6DCk^mkE!cXtahmO&xAJ~ES6fqmV(;-c>Q79w z4t_X%{$WeHy$=3=3Xs!LLI(VRs&(zqOA6MM#msg8 ze6!^3%J|yB+ir)wex_UJWqm(C5~5RX>-J) z%12jk(qx!EHlIu1KVq5o;`oe(>hIIil)cBykE%ZP<~uXOqmG$u$$e)o^BrP(KYp^j z@_K#u3*S88`t9gCE7USWd1c+IcMCfI#Z&kDnh~R~#jYD3)iLyAUzo9R<;78T9bZ1b z_=AQ8edes5{?56|@uxj+_sZ*LG<>w_>22*YFZGGu@Y)k|_r~j1m8oXueYDDVO!b33 z{@o6fDbR Y;=|0@<@aqk`rdo@P*sb1mZ9GN4+Qmbs{jB1 literal 0 HcmV?d00001 diff --git a/hscontrol/db/users_test.go b/hscontrol/db/users_test.go index bec969ab..49dc55d5 100644 --- a/hscontrol/db/users_test.go +++ b/hscontrol/db/users_test.go @@ -56,10 +56,10 @@ func TestDestroyUserErrors(t *testing.T) { err = db.DestroyUser(types.UserID(user.ID)) require.NoError(t, err) - // Verify preauth key was deleted (need to search by prefix for new keys) - var foundPak types.PreAuthKey + // Verify preauth key credential was deleted. + var foundPak types.Credential - result := db.DB.First(&foundPak, "id = ?", pak.ID) + result := db.DB.First(&foundPak, "kind = ? AND id = ?", types.CredentialPreAuthKey, pak.ID) assert.ErrorIs(t, result.Error, gorm.ErrRecordNotFound) }, }, diff --git a/hscontrol/mapper/tail_test.go b/hscontrol/mapper/tail_test.go index 991ae503..ddee26ee 100644 --- a/hscontrol/mapper/tail_test.go +++ b/hscontrol/mapper/tail_test.go @@ -104,7 +104,7 @@ func TestTailNode(t *testing.T) { Name: "mini", }, Tags: []string{}, - AuthKey: &types.PreAuthKey{}, + AuthKey: &types.Credential{}, LastSeen: &lastSeen, Expiry: &expire, Hostinfo: &tailcfg.Hostinfo{ diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index bd8461d9..25c5e635 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -2476,7 +2476,7 @@ func TestResolvePolicy(t *testing.T) { }, // not matching pak tag { - AuthKey: &types.PreAuthKey{ + AuthKey: &types.Credential{ Tags: []string{"tag:alsotagged"}, }, IPv4: ap("100.100.101.11"), diff --git a/hscontrol/state/auth_tagged_expiry_test.go b/hscontrol/state/auth_tagged_expiry_test.go index e4011107..60288f6d 100644 --- a/hscontrol/state/auth_tagged_expiry_test.go +++ b/hscontrol/state/auth_tagged_expiry_test.go @@ -722,7 +722,7 @@ func TestTaggedReauthUntagClearsEphemeralAuthKey(t *testing.T) { require.NoError(t, err) seeded, ok := n.s.nodeStore.UpdateNode(n.id, func(nd *types.Node) { - nd.AuthKey = pak + nd.AuthKey = pak.AsCredential() nd.AuthKeyID = &pak.ID }) require.True(t, ok) diff --git a/hscontrol/state/ephemeral_test.go b/hscontrol/state/ephemeral_test.go index b7a69193..8e28d3b9 100644 --- a/hscontrol/state/ephemeral_test.go +++ b/hscontrol/state/ephemeral_test.go @@ -204,9 +204,8 @@ func TestPersistNodeToDBPreventsRaceCondition(t *testing.T) { // the node could be re-inserted into the database even though it was deleted func TestEphemeralNodeLogoutRaceCondition(t *testing.T) { ephemeralNode := createTestNode(4, 1, "test-user", "ephemeral-node") - ephemeralNode.AuthKey = &types.PreAuthKey{ + ephemeralNode.AuthKey = &types.Credential{ ID: 1, - Key: "test-key", Ephemeral: true, } @@ -284,9 +283,8 @@ func TestEphemeralNodeLogoutRaceCondition(t *testing.T) { // 8. Node gets re-inserted into database instead of staying deleted. func TestUpdateNodeFromMapRequestEphemeralLogoutSequence(t *testing.T) { ephemeralNode := createTestNode(5, 1, "test-user", "ephemeral-node-5") - ephemeralNode.AuthKey = &types.PreAuthKey{ + ephemeralNode.AuthKey = &types.Credential{ ID: 2, - Key: "test-key-2", Ephemeral: true, } @@ -418,9 +416,8 @@ func TestUpdateNodeDeletedInSameBatchReturnsInvalid(t *testing.T) { // 6. persistNodeAndRefreshPolicy must detect the node is deleted and refuse to persist. func TestPersistNodeToDBChecksNodeStoreBeforePersist(t *testing.T) { ephemeralNode := createTestNode(7, 1, "test-user", "ephemeral-node-7") - ephemeralNode.AuthKey = &types.PreAuthKey{ + ephemeralNode.AuthKey = &types.Credential{ ID: 3, - Key: "test-key-3", Ephemeral: true, } diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go index abad20cf..92161857 100644 --- a/hscontrol/state/persist_test.go +++ b/hscontrol/state/persist_test.go @@ -668,3 +668,48 @@ func TestUpdatePolicyManagerUsersUnchangedKeepsSnapshot(t *testing.T) { require.NotSame(t, before, s.nodeStore.data.Load(), "a user change must rebuild the peer map") } + +// TestSingleUsePreAuthKeyUsedInNodeStore asserts that consuming a single-use +// key marks it used in both the database and the node's cached AuthKey, on +// first registration and when re-registering with a fresh single-use key. +func TestSingleUsePreAuthKeyUsedInNodeStore(t *testing.T) { + dbPath := t.TempDir() + "/headscale.db" + + s, err := NewState(persistTestConfig(dbPath)) + require.NoError(t, err) + t.Cleanup(func() { _ = s.Close() }) + + user := s.CreateUserForTest("single-use") + machine := key.NewMachine() + + register := func(t *testing.T) types.NodeView { + t.Helper() + + pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) + require.NoError(t, err) + + node, _, err := s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "single-use"}, + Expiry: time.Now().Add(24 * time.Hour), + }, machine.Public()) + require.NoError(t, err) + + cached, ok := s.GetNodeByID(node.ID()) + require.True(t, ok) + require.True(t, cached.AuthKey().Valid()) + assert.Equal(t, pak.ID, cached.AuthKey().ID()) + assert.True(t, cached.AuthKey().Used(), "NodeStore copy must be marked used") + + stored, err := s.db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + assert.True(t, stored.Used, "database row must be marked used") + + return cached + } + + first := register(t) + second := register(t) + assert.Equal(t, first.ID(), second.ID(), "second key re-registers the same node") +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index aef53a96..726ce26d 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -2015,7 +2015,7 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro nodeToRegister.Tags = nil } - nodeToRegister.AuthKey = params.PreAuthKey + nodeToRegister.AuthKey = params.PreAuthKey.AsCredential() nodeToRegister.AuthKeyID = ¶ms.PreAuthKey.ID } else { // Non-PreAuthKey registration (OIDC, CLI) - always user-owned @@ -2090,7 +2090,10 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro // New node - database first to get ID, then [NodeStore] savedNode, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) { - err := tx.Save(&nodeToRegister).Error + // Omit the AuthKey association: only auth_key_id is persisted here, the + // credential row is owned by the credential CRUD and must not be + // upserted from this node's (possibly stale) in-memory copy (#2862). + err := tx.Omit("AuthKey").Save(&nodeToRegister).Error if err != nil { return nil, fmt.Errorf("saving node: %w", err) } @@ -2100,6 +2103,10 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro if err != nil { return nil, fmt.Errorf("using pre auth key: %w", err) } + + // UsePreAuthKey marked the key used; refresh the node's in-memory + // AuthKey so the NodeStore copy matches the database. + nodeToRegister.AuthKey = params.PreAuthKey.AsCredential() } return &nodeToRegister, nil @@ -2742,8 +2749,14 @@ func (s *State) HandleNodeFromPreAuthKey( } } - node.AuthKey = pak + node.AuthKey = pak.AsCredential() node.AuthKeyID = &pak.ID + // If this registration will consume a single-use key (the tx below + // calls UsePreAuthKey under the same condition), reflect that in the + // cached AuthKey so the NodeStore copy matches the database. + if !pak.Reusable && !pak.Used { + node.AuthKey.Used = true + } // Preserve online state during re-registration so a live node does // not appear offline before the client restarts its map stream. node.LastSeen = new(time.Now()) diff --git a/hscontrol/types/api_key.go b/hscontrol/types/api_key.go index 845f88ca..7e453d4f 100644 --- a/hscontrol/types/api_key.go +++ b/hscontrol/types/api_key.go @@ -10,8 +10,11 @@ import ( // NewAPIKeyPrefixLength is the length of the prefix for new API keys. const NewAPIKeyPrefixLength = 12 -// APIKey describes the datamodel for API keys used to remotely authenticate with -// headscale. +// APIKey is the API/CLI projection of a [Credential] of kind api. Its gorm tags +// remain only for the post-0.29 migrations that still alter the legacy +// api_keys table. +// +// TODO(kradalby): drop the gorm tags in 0.31 with the credentials migration. type APIKey struct { ID uint64 `gorm:"primary_key"` Prefix string `gorm:"uniqueIndex"` @@ -20,8 +23,7 @@ type APIKey struct { // Optional owning user id. When set, an auth key created through the v2 API // with no tags is owned by this user — mirroring Tailscale, where a key is // owned by the identity that created it. Nil for legacy/admin keys, which - // can only create tagged keys. Kept as a plain column (no foreign key) so an - // upgraded database matches a freshly-migrated one. + // can only create tagged keys, and cleared when the user is deleted. UserID *uint CreatedAt *time.Time @@ -36,6 +38,7 @@ func (k *APIKey) maskedPrefix() string { return "hskey-api-" + k.Prefix + "-***" } + // TODO(kradalby): remove in 0.32 with legacy key formats (announced). return k.Prefix + "***" } diff --git a/hscontrol/types/common.go b/hscontrol/types/common.go index e96aef9b..69cd1279 100644 --- a/hscontrol/types/common.go +++ b/hscontrol/types/common.go @@ -1,7 +1,7 @@ -//go:generate go tool viewer --type=User,Node,PreAuthKey +//go:generate go tool viewer --type=User,Node,PreAuthKey,Credential package types -//go:generate go run tailscale.com/cmd/viewer --type=User,Node,PreAuthKey +//go:generate go run tailscale.com/cmd/viewer --type=User,Node,PreAuthKey,Credential import ( "errors" diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index 52fa0ce4..f6f59ac4 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -85,7 +85,8 @@ type RouteConfig struct { type PreAuthKeysConfig struct { // RevokedRetention is how long a soft-revoked pre-auth key (revoked via the // v2 API's DELETE) is kept retrievable before the background collector - // hard-deletes it. A zero or negative duration disables the collector. + // hard-deletes it. Keys still backing a node are kept until the node is + // deleted. A zero or negative duration disables the collector. RevokedRetention time.Duration } diff --git a/hscontrol/types/credential.go b/hscontrol/types/credential.go new file mode 100644 index 00000000..beee8eed --- /dev/null +++ b/hscontrol/types/credential.go @@ -0,0 +1,90 @@ +package types + +import ( + "time" +) + +// CredentialKind discriminates rows of the credentials table. +type CredentialKind string + +const ( + CredentialAPIKey CredentialKind = "api" + CredentialPreAuthKey CredentialKind = "authkey" + CredentialOAuthClient CredentialKind = "oauth_client" //nolint:gosec // discriminator value, not a credential + CredentialOAuthToken CredentialKind = "oauth_token" //nolint:gosec // discriminator value, not a credential +) + +// Credential is the unified storage model for every authenticatable secret: +// API keys, pre-auth keys, OAuth clients, and OAuth access tokens. Each row is +// discriminated by [Credential.Kind]. Only a hash of the secret is stored: +// SHA-256, or a legacy bcrypt/Argon2id hash until the next authentication. +// Identifier is the public, indexed lookup value — the 12-char prefix for API +// keys, pre-auth keys and access tokens, and the client id for OAuth clients — +// and is unique within a kind. +// +// Per-kind fields are sparse by design: Reusable/Ephemeral/Used apply to +// pre-auth keys, LastSeen to API keys, Scopes to OAuth credentials, ClientID +// links an OAuth token to its issuing client, and Tags to pre-auth keys and +// OAuth credentials. +type Credential struct { + ID uint64 `gorm:"primary_key"` + Kind CredentialKind `gorm:"not null;index:idx_credentials_identifier,unique,priority:1"` + Identifier string `gorm:"index:idx_credentials_identifier,unique,priority:2"` + Hash []byte + + // UserID records the owning (user-owned pre-auth key, API key) or creating + // (tagged pre-auth key, OAuth client) user. Deleting the user nulls it. + UserID *uint + User *User `gorm:"constraint:OnDelete:SET NULL;"` + + Description string + + Scopes []string `gorm:"serializer:json"` + Tags []string `gorm:"serializer:json"` + + Reusable bool + Ephemeral bool `gorm:"default:false"` + Used bool `gorm:"default:false"` + + LastSeen *time.Time + + // ClientID links an OAuth access token (Kind == CredentialOAuthToken) back to + // the Identifier of its issuing OAuth client. + ClientID string + + CreatedAt *time.Time + Expiration *time.Time + Revoked *time.Time +} + +// IsTagged reports whether this credential carries tags. For a pre-auth key +// credential, a node registered with it becomes a tagged node. +func (c *Credential) IsTagged() bool { + return len(c.Tags) > 0 +} + +// AsCredential projects a pre-auth key back onto a [Credential] (kind +// authkey), used to set a node's AuthKey association during registration from a +// pre-auth key projection. +func (pak *PreAuthKey) AsCredential() *Credential { + if pak == nil { + return nil + } + + return &Credential{ + ID: pak.ID, + Kind: CredentialPreAuthKey, + Identifier: pak.Prefix, + Hash: pak.Hash, + UserID: pak.UserID, + User: pak.User, + Description: pak.Description, + Reusable: pak.Reusable, + Ephemeral: pak.Ephemeral, + Used: pak.Used, + Tags: pak.Tags, + CreatedAt: pak.CreatedAt, + Expiration: pak.Expiration, + Revoked: pak.Revoked, + } +} diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 1c1f2206..3059b8a5 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -158,12 +158,12 @@ type Node struct { // Tags cannot be removed once set (one-way transition). Tags Strings `gorm:"column:tags;serializer:json"` - // When a node has been created with a [PreAuthKey], we need to - // prevent the preauthkey from being deleted before the node. - // The preauthkey can define "tags" of the node so we need it - // around. + // When a node has been created with a pre-auth key, we keep the key + // credential around: it can define the node's tags and must not be deleted + // before the node. The association is to the unified [Credential] (kind + // authkey), which is what auth_key_id references. AuthKeyID *uint64 `sql:"DEFAULT:NULL"` - AuthKey *PreAuthKey + AuthKey *Credential Expiry *time.Time diff --git a/hscontrol/types/node_tags_test.go b/hscontrol/types/node_tags_test.go index e468604c..7d6ea555 100644 --- a/hscontrol/types/node_tags_test.go +++ b/hscontrol/types/node_tags_test.go @@ -48,7 +48,7 @@ func TestNodeIsTagged(t *testing.T) { // [Node.IsTagged] only checks [Node.Tags], not [PreAuthKey.Tags]. name: "node registered with tagged authkey only - not tagged (tags should be copied)", node: Node{ - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:database"}, }, }, @@ -58,7 +58,7 @@ func TestNodeIsTagged(t *testing.T) { name: "node with both tags and authkey tags - is tagged", node: Node{ Tags: []string{"tag:server"}, - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:database"}, }, }, @@ -101,7 +101,7 @@ func TestNodeViewIsTagged(t *testing.T) { // with only [PreAuthKey.Tags] and no [Node.Tags] would be invalid in practice. name: "node with only AuthKey tags - not tagged (tags should be copied)", node: Node{ - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:web"}, }, }, @@ -154,7 +154,7 @@ func TestNodeHasTag(t *testing.T) { // [Node.HasTag] only checks [Node.Tags], not [PreAuthKey.Tags] name: "node has tag only in authkey - returns false", node: Node{ - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:database"}, }, }, @@ -166,7 +166,7 @@ func TestNodeHasTag(t *testing.T) { name: "node has tag in Tags but not in AuthKey", node: Node{ Tags: []string{"tag:server"}, - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:database"}, }, }, @@ -205,7 +205,7 @@ func TestNodeTagsImmutableAfterRegistration(t *testing.T) { taggedNode := Node{ ID: 1, Tags: []string{"tag:server"}, - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:server"}, }, RegisterMethod: util.RegisterMethodAuthKey, @@ -264,7 +264,7 @@ func TestNodeOwnershipModel(t *testing.T) { node: Node{ ID: 3, UserID: new(uint(5)), // "created by" user 5 - AuthKey: &PreAuthKey{ + AuthKey: &Credential{ Tags: []string{"tag:database"}, }, }, diff --git a/hscontrol/types/oauth.go b/hscontrol/types/oauth.go index 8db2f466..a537af05 100644 --- a/hscontrol/types/oauth.go +++ b/hscontrol/types/oauth.go @@ -27,8 +27,8 @@ const ( ) // OAuthClient is a long-lived OAuth 2.0 client-credentials principal. It mints -// short-lived [OAuthAccessToken]s limited to its Scopes and Tags. The secret is -// stored only as an Argon2id hash. ClientID is public and embedded in the secret +// short-lived [OAuthAccessToken]s limited to its Scopes and Tags. It is the +// API projection of a [Credential] of kind oauth_client. ClientID is public and embedded in the secret // string (hskey-client--) so the token endpoint can derive it // from the secret alone, matching Tailscale, where the client id is a substring // of the client secret. @@ -49,23 +49,23 @@ type OAuthClient struct { Description string - // UserID records who created the client. Kept as a plain column with no - // foreign key so an upgraded database matches a freshly-migrated one. + // UserID records who created the client; cleared when the user is deleted. UserID *uint CreatedAt *time.Time Revoked *time.Time } -// TableName pins the table name. GORM's naming strategy would otherwise render -// OAuthClient as "o_auth_clients" (it breaks the OAuth initialism), diverging -// from the hand-written migration DDL and schema.sql. +// TableName pins the legacy table name for the post-0.29 migration that creates +// it; GORM would otherwise render OAuthClient as "o_auth_clients". +// +// TODO(kradalby): remove in 0.31 with the credentials migration. func (*OAuthClient) TableName() string { return "oauth_clients" } // OAuthAccessToken is a short-lived bearer token minted by an [OAuthClient] via // the client-credentials grant. It carries the scope/tag set granted at mint -// time (a subset of the issuing client's), is stored as an Argon2id hash of its -// secret, and authenticates v2 API requests as Authorization: Bearer. +// time (a subset of the issuing client's), is the API projection of a +// [Credential] of kind oauth_token, and authenticates v2 API requests as Authorization: Bearer. type OAuthAccessToken struct { ID uint64 `gorm:"primary_key"` Prefix string `gorm:"uniqueIndex"` @@ -82,6 +82,8 @@ type OAuthAccessToken struct { } // TableName pins the table name (see [OAuthClient.TableName]). +// +// TODO(kradalby): remove in 0.31 with the credentials migration. func (*OAuthAccessToken) TableName() string { return "oauth_access_tokens" } // maskedClientID returns the client id in masked form for safe logging. diff --git a/hscontrol/types/preauth_key.go b/hscontrol/types/preauth_key.go index 27502f3d..0f76efea 100644 --- a/hscontrol/types/preauth_key.go +++ b/hscontrol/types/preauth_key.go @@ -34,16 +34,17 @@ func (pak *PreAuthKeyNew) StringID() string { return strconv.FormatUint(pak.ID, util.Base10) } -// PreAuthKey describes a pre-authorization key usable in a particular user. +// PreAuthKey is the API/state projection of a [Credential] of kind authkey. +// Its gorm tags remain only for the post-0.29 migrations that still alter the +// legacy pre_auth_keys table. +// +// TODO(kradalby): drop the gorm tags in 0.31 with the credentials migration. type PreAuthKey struct { ID uint64 `gorm:"primary_key"` - // Legacy plaintext key (for backwards compatibility) - Key string - - // New bcrypt-based authentication + // Prefix is the credential identifier; Hash is the stored secret hash. Prefix string - Hash []byte // bcrypt + Hash []byte // For tagged keys: [PreAuthKey.UserID] tracks who created the key (informational) // For user-owned keys: [PreAuthKey.UserID] tracks the node owner diff --git a/hscontrol/types/types_clone.go b/hscontrol/types/types_clone.go index 1040e214..bfbe416e 100644 --- a/hscontrol/types/types_clone.go +++ b/hscontrol/types/types_clone.go @@ -97,7 +97,7 @@ var _NodeCloneNeedsRegeneration = Node(struct { RegisterMethod string Tags Strings AuthKeyID *uint64 - AuthKey *PreAuthKey + AuthKey *Credential Expiry *time.Time LastSeen *time.Time ApprovedRoutes Prefixes @@ -141,7 +141,6 @@ func (src *PreAuthKey) Clone() *PreAuthKey { // A compilation failure here means this code must be regenerated, with the command at the top of this file. var _PreAuthKeyCloneNeedsRegeneration = PreAuthKey(struct { ID uint64 - Key string Prefix string Hash []byte UserID *uint @@ -155,3 +154,56 @@ var _PreAuthKeyCloneNeedsRegeneration = PreAuthKey(struct { Expiration *time.Time Revoked *time.Time }{}) + +// Clone makes a deep copy of Credential. +// The result aliases no memory with the original. +func (src *Credential) Clone() *Credential { + if src == nil { + return nil + } + dst := new(Credential) + *dst = *src + dst.Hash = append(src.Hash[:0:0], src.Hash...) + if dst.UserID != nil { + dst.UserID = new(*src.UserID) + } + if dst.User != nil { + dst.User = new(*src.User) + } + dst.Scopes = append(src.Scopes[:0:0], src.Scopes...) + dst.Tags = append(src.Tags[:0:0], src.Tags...) + if dst.LastSeen != nil { + dst.LastSeen = new(*src.LastSeen) + } + if dst.CreatedAt != nil { + dst.CreatedAt = new(*src.CreatedAt) + } + if dst.Expiration != nil { + dst.Expiration = new(*src.Expiration) + } + if dst.Revoked != nil { + dst.Revoked = new(*src.Revoked) + } + return dst +} + +// A compilation failure here means this code must be regenerated, with the command at the top of this file. +var _CredentialCloneNeedsRegeneration = Credential(struct { + ID uint64 + Kind CredentialKind + Identifier string + Hash []byte + UserID *uint + User *User + Description string + Scopes []string + Tags []string + Reusable bool + Ephemeral bool + Used bool + LastSeen *time.Time + ClientID string + CreatedAt *time.Time + Expiration *time.Time + Revoked *time.Time +}{}) diff --git a/hscontrol/types/types_view.go b/hscontrol/types/types_view.go index 0a084e06..bc43cd04 100644 --- a/hscontrol/types/types_view.go +++ b/hscontrol/types/types_view.go @@ -20,7 +20,7 @@ import ( "tailscale.com/types/views" ) -//go:generate go run tailscale.com/cmd/cloner -clonefunc=false -type=User,Node,PreAuthKey +//go:generate go run tailscale.com/cmd/cloner -clonefunc=false -type=User,Node,PreAuthKey,Credential // View returns a read-only view of User. func (p *User) View() UserView { @@ -230,13 +230,13 @@ func (v NodeView) RegisterMethod() string { return v.ж.RegisterMethod } // Tags cannot be removed once set (one-way transition). func (v NodeView) Tags() views.Slice[string] { return views.SliceOf(v.ж.Tags) } -// When a node has been created with a [PreAuthKey], we need to -// prevent the preauthkey from being deleted before the node. -// The preauthkey can define "tags" of the node so we need it -// around. +// When a node has been created with a pre-auth key, we keep the key +// credential around: it can define the node's tags and must not be deleted +// before the node. The association is to the unified [Credential] (kind +// authkey), which is what auth_key_id references. func (v NodeView) AuthKeyID() views.ValuePointer[uint64] { return views.ValuePointerOf(v.ж.AuthKeyID) } -func (v NodeView) AuthKey() PreAuthKeyView { return v.ж.AuthKey.View() } +func (v NodeView) AuthKey() CredentialView { return v.ж.AuthKey.View() } func (v NodeView) Expiry() views.ValuePointer[time.Time] { return views.ValuePointerOf(v.ж.Expiry) } // LastSeen is when the node was last in contact with @@ -300,7 +300,7 @@ var _NodeViewNeedsRegeneration = Node(struct { RegisterMethod string Tags Strings AuthKeyID *uint64 - AuthKey *PreAuthKey + AuthKey *Credential Expiry *time.Time LastSeen *time.Time ApprovedRoutes Prefixes @@ -382,13 +382,8 @@ func (v *PreAuthKeyView) UnmarshalJSONFrom(dec *jsontext.Decoder) error { func (v PreAuthKeyView) ID() uint64 { return v.ж.ID } -// Legacy plaintext key (for backwards compatibility) -func (v PreAuthKeyView) Key() string { return v.ж.Key } - -// New bcrypt-based authentication -func (v PreAuthKeyView) Prefix() string { return v.ж.Prefix } - -// bcrypt +// Prefix is the credential identifier; Hash is the stored secret hash. +func (v PreAuthKeyView) Prefix() string { return v.ж.Prefix } func (v PreAuthKeyView) Hash() views.ByteSlice[[]byte] { return views.ByteSliceOf(v.ж.Hash) } // For tagged keys: [PreAuthKey.UserID] tracks who created the key (informational) @@ -427,7 +422,6 @@ func (v PreAuthKeyView) Revoked() views.ValuePointer[time.Time] { // A compilation failure here means this code must be regenerated, with the command at the top of this file. var _PreAuthKeyViewNeedsRegeneration = PreAuthKey(struct { ID uint64 - Key string Prefix string Hash []byte UserID *uint @@ -441,3 +435,126 @@ var _PreAuthKeyViewNeedsRegeneration = PreAuthKey(struct { Expiration *time.Time Revoked *time.Time }{}) + +// View returns a read-only view of Credential. +func (p *Credential) View() CredentialView { + return CredentialView{ж: p} +} + +// CredentialView provides a read-only view over Credential. +// +// Its methods should only be called if `Valid()` returns true. +type CredentialView struct { + // ж is the underlying mutable value, named with a hard-to-type + // character that looks pointy like a pointer. + // It is named distinctively to make you think of how dangerous it is to escape + // to callers. You must not let callers be able to mutate it. + ж *Credential +} + +// Valid reports whether v's underlying value is non-nil. +func (v CredentialView) Valid() bool { return v.ж != nil } + +// AsStruct returns a clone of the underlying value which aliases no memory with +// the original. +func (v CredentialView) AsStruct() *Credential { + if v.ж == nil { + return nil + } + return v.ж.Clone() +} + +// MarshalJSON implements [jsonv1.Marshaler]. +func (v CredentialView) MarshalJSON() ([]byte, error) { + return jsonv1.Marshal(v.ж) +} + +// MarshalJSONTo implements [jsonv2.MarshalerTo]. +func (v CredentialView) MarshalJSONTo(enc *jsontext.Encoder) error { + return jsonv2.MarshalEncode(enc, v.ж) +} + +// UnmarshalJSON implements [jsonv1.Unmarshaler]. +func (v *CredentialView) UnmarshalJSON(b []byte) error { + if v.ж != nil { + return errors.New("already initialized") + } + if len(b) == 0 { + return nil + } + var x Credential + if err := jsonv1.Unmarshal(b, &x); err != nil { + return err + } + v.ж = &x + return nil +} + +// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom]. +func (v *CredentialView) UnmarshalJSONFrom(dec *jsontext.Decoder) error { + if v.ж != nil { + return errors.New("already initialized") + } + var x Credential + if err := jsonv2.UnmarshalDecode(dec, &x); err != nil { + return err + } + v.ж = &x + return nil +} + +func (v CredentialView) ID() uint64 { return v.ж.ID } +func (v CredentialView) Kind() CredentialKind { return v.ж.Kind } +func (v CredentialView) Identifier() string { return v.ж.Identifier } +func (v CredentialView) Hash() views.ByteSlice[[]byte] { return views.ByteSliceOf(v.ж.Hash) } + +// UserID records the owning (user-owned pre-auth key, API key) or creating +// (tagged pre-auth key, OAuth client) user. Deleting the user nulls it. +func (v CredentialView) UserID() views.ValuePointer[uint] { return views.ValuePointerOf(v.ж.UserID) } + +func (v CredentialView) User() UserView { return v.ж.User.View() } +func (v CredentialView) Description() string { return v.ж.Description } +func (v CredentialView) Scopes() views.Slice[string] { return views.SliceOf(v.ж.Scopes) } +func (v CredentialView) Tags() views.Slice[string] { return views.SliceOf(v.ж.Tags) } +func (v CredentialView) Reusable() bool { return v.ж.Reusable } +func (v CredentialView) Ephemeral() bool { return v.ж.Ephemeral } +func (v CredentialView) Used() bool { return v.ж.Used } +func (v CredentialView) LastSeen() views.ValuePointer[time.Time] { + return views.ValuePointerOf(v.ж.LastSeen) +} + +// ClientID links an OAuth access token (Kind == CredentialOAuthToken) back to +// the Identifier of its issuing OAuth client. +func (v CredentialView) ClientID() string { return v.ж.ClientID } +func (v CredentialView) CreatedAt() views.ValuePointer[time.Time] { + return views.ValuePointerOf(v.ж.CreatedAt) +} + +func (v CredentialView) Expiration() views.ValuePointer[time.Time] { + return views.ValuePointerOf(v.ж.Expiration) +} + +func (v CredentialView) Revoked() views.ValuePointer[time.Time] { + return views.ValuePointerOf(v.ж.Revoked) +} + +// A compilation failure here means this code must be regenerated, with the command at the top of this file. +var _CredentialViewNeedsRegeneration = Credential(struct { + ID uint64 + Kind CredentialKind + Identifier string + Hash []byte + UserID *uint + User *User + Description string + Scopes []string + Tags []string + Reusable bool + Ephemeral bool + Used bool + LastSeen *time.Time + ClientID string + CreatedAt *time.Time + Expiration *time.Time + Revoked *time.Time +}{}) diff --git a/integration/cli_apikeys_test.go b/integration/cli_apikeys_test.go index fab3a1c7..385304ef 100644 --- a/integration/cli_apikeys_test.go +++ b/integration/cli_apikeys_test.go @@ -1,6 +1,7 @@ package integration import ( + "strconv" "testing" "time" @@ -72,11 +73,17 @@ func TestApiKeyCommand(t *testing.T) { assert.Len(t, listedAPIKeys, 5) - assert.Equal(t, "1", listedAPIKeys[0].Id) - assert.Equal(t, "2", listedAPIKeys[1].Id) - assert.Equal(t, "3", listedAPIKeys[2].Id) - assert.Equal(t, "4", listedAPIKeys[3].Id) - assert.Equal(t, "5", listedAPIKeys[4].Id) + // IDs are drawn from the shared credentials table, so they are not + // guaranteed to start at 1 (pre-auth keys created during env setup take the + // first ids). They are still listed in strictly increasing creation order. + var prevID uint64 + + for _, key := range listedAPIKeys { + id, err := strconv.ParseUint(key.Id, 10, 64) + require.NoError(t, err) + assert.Greater(t, id, prevID, "API key ids must be strictly increasing") + prevID = id + } assert.NotEmpty(t, listedAPIKeys[0].Prefix) assert.NotEmpty(t, listedAPIKeys[1].Prefix)