Mirrors/juanfont.headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2025-11-10 01:20:58 +01:00
Code Issues Projects Releases Wiki Activity

db: use partial unique index for pre_auth_keys.prefix

Browse Source 
This commit fixes the UNIQUE constraint on the pre_auth_keys.prefix
column to allow multiple legacy keys (with NULL or empty prefix) while
still enforcing uniqueness for new bcrypt-based keys.

Changes:
- Modified migration to create partial unique index with WHERE clause
- Updated schema.sql to match migration
- Added comprehensive test (TestMultipleLegacyKeysAllowed) to verify:
  * Multiple legacy keys with empty prefix can coexist
  * New bcrypt keys have unique prefixes
  * Duplicate non-empty prefixes are rejected
- Fixed lint issues (errcheck, intrange)
- Fixed hardening: position-based parsing instead of separator-based
- Added validation for empty string, length, separator, and character set

The partial index uses:
  WHERE prefix IS NOT NULL AND prefix != ''

This allows unlimited legacy keys (backward compatibility) while
preventing duplicate prefixes for new keys (security).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Kristoffer Dalby 2025-11-02 11:25:24 +01:00
parent c31217b62b
commit d49ae71a80
No known key found for this signature in database
5 changed files with 125 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
hscontrol/db/db.go
View File

@ -961,10 +961,16 @@ AND auth_key_id NOT IN (
// them with the same security model as api keys.
ID: "202511011637-preauthkey-bcrypt",
Migrate: func(tx *gorm.DB) error {
tx.Migrator().AddColumn(&types.PreAuthKey{}, "prefix")
tx.Migrator().AddColumn(&types.PreAuthKey{}, "hash")
if err := tx.Migrator().AddColumn(&types.PreAuthKey{}, "prefix"); err != nil {
return fmt.Errorf("adding prefix column: %w", err)
}
if err := tx.Migrator().AddColumn(&types.PreAuthKey{}, "hash"); err != nil {
return fmt.Errorf("adding hash column: %w", err)
}
if err := tx.Exec("CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix)").Error; err != nil {
// Create partial unique index to allow multiple legacy keys (NULL/empty prefix)
// while enforcing uniqueness for new bcrypt-based keys
if err := tx.Exec("CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != ''").Error; err != nil {
return fmt.Errorf("creating prefix index: %w", err)
}

13
hscontrol/db/preauth_keys.go
View File

@ -79,10 +79,10 @@ func CreatePreAuthKey(
// Validate generated prefix (should always be valid, but be defensive)
if len(prefix) != authKeyPrefixLength {
return nil, fmt.Errorf("generated prefix has invalid length: expected %d, got %d", authKeyPrefixLength, len(prefix))
return nil, fmt.Errorf("%w: generated prefix has invalid length: expected %d, got %d", ErrPreAuthKeyFailedToParse, authKeyPrefixLength, len(prefix))
}
if !isValidBase64URLSafe(prefix) {
return nil, fmt.Errorf("generated prefix contains invalid characters")
return nil, fmt.Errorf("%w: generated prefix contains invalid characters", ErrPreAuthKeyFailedToParse)
}
toBeHashed, err := util.GenerateRandomStringURLSafe(authKeyLength)
@ -92,10 +92,10 @@ func CreatePreAuthKey(
// Validate generated hash (should always be valid, but be defensive)
if len(toBeHashed) != authKeyLength {
return nil, fmt.Errorf("generated hash has invalid length: expected %d, got %d", authKeyLength, len(toBeHashed))
return nil, fmt.Errorf("%w: generated hash has invalid length: expected %d, got %d", ErrPreAuthKeyFailedToParse, authKeyLength, len(toBeHashed))
}
if !isValidBase64URLSafe(toBeHashed) {
return nil, fmt.Errorf("generated hash contains invalid characters")
return nil, fmt.Errorf("%w: generated hash contains invalid characters", ErrPreAuthKeyFailedToParse)
}
keyStr := authKeyPrefix + prefix + "-" + toBeHashed
@ -234,14 +234,15 @@ func findAuthKey(tx *gorm.DB, keyStr string) (*types.PreAuthKey, error) {
return &pak, nil
}
// isValidBase64URLSafe checks if a string contains only base64 URL-safe characters
// isValidBase64URLSafe checks if a string contains only base64 URL-safe characters.
func isValidBase64URLSafe(s string) bool {
for _, c := range s {
if !((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') ||
(c >= '0' && c <= '9') || c == '-' || c == '_') {
(c >= '0' && c <= '9') || c == '-' || c == '_') {
return false
}
}
return true
}

127
hscontrol/db/preauth_keys_test.go
View File

@ -1,6 +1,7 @@
package db
import (
"fmt"
"slices"
"strings"
"testing"
@ -15,8 +16,8 @@ import (
func TestCreatePreAuthKey(t *testing.T) {
tests := []struct {
name string
test func(*testing.T, *HSDatabase)
name string
test func(*testing.T, *HSDatabase)
}{
{
name: "error_invalid_user_id",
@ -65,8 +66,8 @@ func TestCreatePreAuthKey(t *testing.T) {
func TestPreAuthKeyACLTags(t *testing.T) {
tests := []struct {
name string
test func(*testing.T, *HSDatabase)
name string
test func(*testing.T, *HSDatabase)
}{
{
name: "reject_malformed_tags",
@ -115,10 +116,10 @@ func TestCannotDeleteAssignedPreAuthKey(t *testing.T) {
db, err := newSQLiteTestDB()
require.NoError(t, err)
user, err := db.CreateUser(types.User{Name: "test8"})
assert.NoError(t, err)
require.NoError(t, err)
key, err := db.CreatePreAuthKey(types.UserID(user.ID), false, false, nil, []string{"tag:good"})
assert.NoError(t, err)
require.NoError(t, err)
node := types.Node{
ID: 0,
@ -139,11 +140,11 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
user := db.CreateUserForTest("test-user")
tests := []struct {
name string
setupKey func() string // Returns key string to test
wantFindErr bool // Error when finding the key
wantValidateErr bool // Error when validating the key
validateResult func(*testing.T, *types.PreAuthKey)
name string
setupKey func() string // Returns key string to test
wantFindErr bool // Error when finding the key
wantValidateErr bool // Error when validating the key
validateResult func(*testing.T, *types.PreAuthKey)
}{
{
name: "legacy_key_plaintext",
@ -160,9 +161,10 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
VALUES (?, ?, ?, ?, ?, ?)
`, legacyKey, user.ID, true, false, false, now).Error
require.NoError(t, err)
return legacyKey
},
wantFindErr: false,
wantFindErr: false,
wantValidateErr: false,
validateResult: func(t *testing.T, pak *types.PreAuthKey) {
assert.Equal(t, user.ID, pak.UserID)
@ -180,9 +182,10 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
true, false, nil, []string{"tag:test"},
)
require.NoError(t, err)
return keyStr.Key
},
wantFindErr: false,
wantFindErr: false,
wantValidateErr: false,
validateResult: func(t *testing.T, pak *types.PreAuthKey) {
assert.Equal(t, user.ID, pak.UserID)
@ -208,17 +211,17 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
// Extract prefix and hash using fixed-length parsing like the real code does
_, prefixAndHash, found := strings.Cut(keyStr.Key, "hskey-auth-")
assert.True(t, found)
assert.True(t, len(prefixAndHash) >= 12+1+64) // prefix + '-' + hash minimum
assert.GreaterOrEqual(t, len(prefixAndHash), 12+1+64) // prefix + '-' + hash minimum
prefix := prefixAndHash[:12]
assert.Len(t, prefix, 12) // Prefix is 12 chars
assert.Len(t, prefix, 12) // Prefix is 12 chars
assert.Equal(t, byte('-'), prefixAndHash[12]) // Separator
hash := prefixAndHash[13:]
assert.Len(t, hash, 64) // Hash is 64 chars
return keyStr.Key
},
wantFindErr: false,
wantFindErr: false,
wantValidateErr: false,
},
{
@ -235,9 +238,10 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
// Return key with tampered hash using fixed-length parsing
_, prefixAndHash, _ := strings.Cut(keyStr, "hskey-auth-")
prefix := prefixAndHash[:12]
return "hskey-auth-" + prefix + "-" + "wrong_hash_here_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -245,7 +249,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
setupKey: func() string {
return ""
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -253,7 +257,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
setupKey: func() string {
return "hskey-auth-short"
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -261,7 +265,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
setupKey: func() string {
return "hskey-auth-ABCDEFGHIJKLabcdefghijklmnopqrstuvwxyz1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZ"
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -269,7 +273,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
setupKey: func() string {
return "hskey-auth-ABCDEFGHIJKL-short"
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -277,7 +281,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
setupKey: func() string {
return "hskey-auth-ABC$EF@HIJKL-" + strings.Repeat("a", 64)
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -285,7 +289,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
setupKey: func() string {
return "hskey-auth-ABCDEFGHIJKL-" + "invalid$chars" + strings.Repeat("a", 54)
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -294,7 +298,7 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
// Create a validly formatted key but with a prefix that doesn't exist
return "hskey-auth-NotInDB12345-" + strings.Repeat("a", 64)
},
wantFindErr: true,
wantFindErr: true,
wantValidateErr: false,
},
{
@ -310,9 +314,10 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
VALUES (?, ?, ?, ?, ?, ?, ?)
`, legacyKey, user.ID, true, false, false, now, expiration).Error
require.NoError(t, err)
return legacyKey
},
wantFindErr: false,
wantFindErr: false,
wantValidateErr: true,
},
{
@ -327,9 +332,10 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
VALUES (?, ?, ?, ?, ?, ?)
`, legacyKey, user.ID, false, false, true, now).Error
require.NoError(t, err)
return legacyKey
},
wantFindErr: false,
wantFindErr: false,
wantValidateErr: true,
},
}
@ -361,3 +367,72 @@ func TestPreAuthKeyAuthentication(t *testing.T) {
})
}
}
func TestMultipleLegacyKeysAllowed(t *testing.T) {
db, err := newSQLiteTestDB()
require.NoError(t, err)
user, err := db.CreateUser(types.User{Name: "test-legacy"})
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(types.UserID(user.ID), true, false, nil, nil)
require.NoError(t, err)
assert.NotEmpty(t, key1.Key)
key2, err := db.CreatePreAuthKey(types.UserID(user.ID), 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, 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
assert.Error(t, err, "duplicate non-empty prefix should be rejected")
assert.Contains(t, err.Error(), "UNIQUE constraint failed", "should fail with UNIQUE constraint error")
}

2
hscontrol/db/schema.sql
View File

@ -61,7 +61,7 @@ CREATE TABLE pre_auth_keys(
CONSTRAINT fk_pre_auth_keys_user FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE SET NULL
);
CREATE UNIQUE INDEX idx_pre_auth_keys_prefix ON pre_auth_keys(prefix);
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,

14
hscontrol/db/users_test.go
View File

@ -31,8 +31,8 @@ func TestCreateAndDestroyUser(t *testing.T) {
func TestDestroyUserErrors(t *testing.T) {
tests := []struct {
name string
test func(*testing.T, *HSDatabase)
name string
test func(*testing.T, *HSDatabase)
}{
{
name: "error_user_not_found",
@ -95,8 +95,8 @@ func TestDestroyUserErrors(t *testing.T) {
func TestRenameUser(t *testing.T) {
tests := []struct {
name string
test func(*testing.T, *HSDatabase)
name string
test func(*testing.T, *HSDatabase)
}{
{
name: "success_rename",
@ -113,7 +113,7 @@ func TestRenameUser(t *testing.T) {
users, err = db.ListUsers(&types.User{Name: "test"})
require.NoError(t, err)
assert.Len(t, users, 0)
assert.Empty(t, users)
users, err = db.ListUsers(&types.User{Name: "test-renamed"})
require.NoError(t, err)
@ -155,8 +155,8 @@ func TestRenameUser(t *testing.T) {
func TestAssignNodeToUser(t *testing.T) {
tests := []struct {
name string
test func(*testing.T, *HSDatabase)
name string
test func(*testing.T, *HSDatabase)
}{
{
name: "success_reassign_node",