diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c4028cbd..71a1b083d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,16 @@ keys remain all-access. [#3334](https://github.com/juanfont/headscale/pull/3334) +### Credential hashing unified on Argon2id + +API keys and pre-auth keys now hash their secrets with Argon2id, the scheme +already used for OAuth client secrets and access tokens, through one shared +generator and verifier. Existing bcrypt-hashed keys keep working and are +transparently upgraded to Argon2id the next time they authenticate. Bcrypt +verification will be removed in 0.32, so any key left unused between now and +then must be regenerated. Legacy plaintext pre-auth keys, the format that +predates the 2025-11 bcrypt keys, are no longer accepted and must be recreated. + ### BREAKING #### API diff --git a/hscontrol/db/api_key.go b/hscontrol/db/api_key.go index b56f1c188..d7b163f80 100644 --- a/hscontrol/db/api_key.go +++ b/hscontrol/db/api_key.go @@ -7,9 +7,7 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "golang.org/x/crypto/bcrypt" "gorm.io/gorm" - "tailscale.com/util/rands" ) const ( @@ -17,9 +15,8 @@ const ( apiKeyPrefixLength = 12 apiKeyHashLength = 64 - // Legacy format constants. + // Legacy format constant: the prefix length of pre-hskey "prefix.secret" keys. legacyAPIPrefixLength = 7 - legacyAPIKeyLength = 32 ) var ( @@ -32,17 +29,7 @@ var ( func (hsdb *HSDatabase) CreateAPIKey( expiration *time.Time, ) (string, *types.APIKey, error) { - // Generate public prefix (12 chars) - prefix := rands.HexString(apiKeyPrefixLength) - - // 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) + keyStr, prefix, hash, err := generateSecret(apiKeyPrefix) if err != nil { return "", nil, err } @@ -224,12 +211,15 @@ func validateAPIKey(db *gorm.DB, keyStr string) (*types.APIKey, error) { return nil, fmt.Errorf("API key not found: %w", err) } - // Verify bcrypt hash - err = bcrypt.CompareHashAndPassword(key.Hash, []byte(secret)) + needsRehash, err := verifySecret(key.Hash, secret) if err != nil { return nil, fmt.Errorf("invalid API key: %w", err) } + if needsRehash { + rehashToArgon2id(db, &key, secret) + } + return &key, nil } @@ -253,11 +243,14 @@ func validateLegacyAPIKey(db *gorm.DB, keyStr string) (*types.APIKey, error) { return nil, fmt.Errorf("API key not found: %w", err) } - // Verify bcrypt (key.Hash stores bcrypt of full secret) - err = bcrypt.CompareHashAndPassword(key.Hash, []byte(secret)) + needsRehash, err := verifySecret(key.Hash, secret) if err != nil { return nil, fmt.Errorf("invalid API key: %w", err) } + if needsRehash { + rehashToArgon2id(db, &key, secret) + } + return &key, nil } diff --git a/hscontrol/db/api_key_test.go b/hscontrol/db/api_key_test.go index a34dd94bb..9b4cb07fa 100644 --- a/hscontrol/db/api_key_test.go +++ b/hscontrol/db/api_key_test.go @@ -273,3 +273,37 @@ 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 argon2id, while continuing to authenticate. +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.DefaultCost) + require.NoError(t, err) + + err = db.DB.Exec( + `INSERT INTO api_keys (prefix, hash, created_at) VALUES (?, ?, ?)`, + 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), "$argon2id$"), + "a bcrypt-stored key must be rehashed to argon2id on first auth") + + valid, err = db.ValidateAPIKey(keyStr) + require.NoError(t, err) + assert.True(t, valid, "key must still authenticate against the upgraded hash") +} diff --git a/hscontrol/db/oauth.go b/hscontrol/db/oauth.go index 6ce757bc7..ec205aa24 100644 --- a/hscontrol/db/oauth.go +++ b/hscontrol/db/oauth.go @@ -1,20 +1,14 @@ package db import ( - "crypto/rand" - "crypto/subtle" - "encoding/base64" "errors" "fmt" - "runtime" "slices" "strings" "time" "github.com/juanfont/headscale/hscontrol/types" - "golang.org/x/crypto/argon2" "gorm.io/gorm" - "tailscale.com/util/rands" "tailscale.com/util/set" ) @@ -42,96 +36,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 -} - // CreateOAuthClient creates a new [types.OAuthClient] and returns the plaintext // secret (shown ONCE) alongside the stored client. creatorUserID is the user who // created it (informational), or nil. @@ -148,11 +54,7 @@ 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) + secretStr, clientID, hash, err := generateSecret(types.OAuthClientPrefix) if err != nil { return "", nil, err } @@ -212,7 +114,7 @@ func (hsdb *HSDatabase) AuthenticateOAuthClient(secretStr string) (*types.OAuthC return nil, ErrOAuthClientNotFound } - if err := verifySecret(client.SecretHash, secret); err != nil { //nolint:noinlineerr + if _, err := verifySecret(client.SecretHash, secret); err != nil { //nolint:noinlineerr return nil, fmt.Errorf("invalid oauth client secret: %w", err) } @@ -278,11 +180,7 @@ 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) + tokenStr, prefix, hash, err := generateSecret(types.AccessTokenPrefix) if err != nil { return "", nil, err } @@ -349,7 +247,7 @@ func (hsdb *HSDatabase) AuthenticateAccessToken(tokenStr string) (*types.OAuthAc return nil, ErrAccessTokenNotFound } - if err := verifySecret(token.Hash, secret); err != nil { //nolint:noinlineerr + if _, err := verifySecret(token.Hash, secret); err != nil { //nolint:noinlineerr return nil, fmt.Errorf("invalid oauth access token: %w", err) } diff --git a/hscontrol/db/oauth_test.go b/hscontrol/db/oauth_test.go index 1bf8c72d4..93be4b7e5 100644 --- a/hscontrol/db/oauth_test.go +++ b/hscontrol/db/oauth_test.go @@ -31,9 +31,9 @@ func TestVerifySecretConcurrent(t *testing.T) { defer wg.Done() 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) } @@ -99,9 +99,14 @@ func TestHashSecretRoundTrip(t *testing.T) { 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) + _, err = verifySecret(encoded, secret) + require.NoError(t, err) + + _, err = verifySecret(encoded, "wrong-secret") + require.ErrorIs(t, err, errSecretMismatch) + + _, err = verifySecret([]byte("not-a-phc-string"), secret) + require.ErrorIs(t, err, errSecretHashMalformed) } func TestOAuthClientRevoke(t *testing.T) { diff --git a/hscontrol/db/preauth_keys.go b/hscontrol/db/preauth_keys.go index e839ff507..5bb802cc5 100644 --- a/hscontrol/db/preauth_keys.go +++ b/hscontrol/db/preauth_keys.go @@ -8,9 +8,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" ) @@ -103,13 +101,7 @@ func CreatePreAuthKey( now := time.Now().UTC() - prefix := rands.HexString(authKeyPrefixLength) - - toBeHashed := rands.HexString(authKeyLength) - - keyStr := authKeyPrefix + prefix + "-" + toBeHashed - - hash, err := bcrypt.GenerateFromPassword([]byte(toBeHashed), bcrypt.DefaultCost) + keyStr, prefix, hash, err := generateSecret(authKeyPrefix) if err != nil { return nil, err } @@ -192,21 +184,17 @@ func findAuthKey(tx *gorm.DB, keyStr string) (*types.PreAuthKey, error) { return nil, ErrPreAuthKeyFailedToParse } - _, prefixAndHash, found := strings.Cut(keyStr, authKeyPrefix) - + _, prefixAndSecret, found := strings.Cut(keyStr, authKeyPrefix) if !found { - // Legacy format (plaintext) - backwards compatibility - err := tx.Preload("User").First(&pak, "key = ?", keyStr).Error - if err != nil { - return nil, ErrPreAuthKeyNotFound - } - - return &pak, nil + // Legacy plaintext keys (pre-0.30) are no longer supported. An + // unprefixed string is treated as an unknown key so the registration + // handler maps it to a 401. + return nil, ErrPreAuthKeyNotFound } - // New format: hskey-auth-{12-char-prefix}-{64-char-hash} - prefix, hash, err := parsePrefixedKey( - prefixAndHash, + // New format: hskey-auth-{12-char-prefix}-{64-char-secret} + prefix, secret, err := parsePrefixedKey( + prefixAndSecret, authKeyPrefixLength, authKeyLength, ErrPreAuthKeyFailedToParse, @@ -221,12 +209,15 @@ func findAuthKey(tx *gorm.DB, keyStr string) (*types.PreAuthKey, error) { return nil, ErrPreAuthKeyNotFound } - // Verify hash matches - err = bcrypt.CompareHashAndPassword(pak.Hash, []byte(hash)) + needsRehash, err := verifySecret(pak.Hash, secret) if err != nil { return nil, fmt.Errorf("invalid auth key: %w", err) } + if needsRehash { + rehashToArgon2id(tx, &pak, secret) + } + return &pak, nil } diff --git a/hscontrol/db/preauth_keys_test.go b/hscontrol/db/preauth_keys_test.go index 5835758f3..bddfde607 100644 --- a/hscontrol/db/preauth_keys_test.go +++ b/hscontrol/db/preauth_keys_test.go @@ -11,6 +11,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" ) @@ -149,15 +150,14 @@ func TestPreAuthKeyAuthentication(t *testing.T) { validateResult func(*testing.T, *types.PreAuthKey) }{ { - name: "legacy_key_plaintext", + name: "legacy_plaintext_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 + // Plaintext pre-auth keys (pre-0.30) are no longer supported. Seed + // one directly, the way it would exist in an upgraded production + // database, and confirm it can no longer be found. 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 (?, ?, ?, ?, ?, ?) @@ -166,16 +166,8 @@ func TestPreAuthKeyAuthentication(t *testing.T) { return legacyKey }, - wantFindErr: false, + wantFindErr: true, 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", @@ -308,43 +300,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 { @@ -500,3 +455,40 @@ 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 argon2id, while continuing to authenticate. +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.DefaultCost) + require.NoError(t, err) + + err = db.DB.Exec( + `INSERT INTO pre_auth_keys (prefix, hash, user_id, reusable, ephemeral, used, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + 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), "$argon2id$"), + "a bcrypt-stored key must be rehashed to argon2id on first auth") + + // Still authenticates against the upgraded hash. + _, err = db.GetPreAuthKey(keyStr) + require.NoError(t, err) +} diff --git a/hscontrol/db/secret.go b/hscontrol/db/secret.go new file mode 100644 index 000000000..4bd9e14f8 --- /dev/null +++ b/hscontrol/db/secret.go @@ -0,0 +1,169 @@ +package db + +import ( + "bytes" + "crypto/rand" + "crypto/subtle" + "encoding/base64" + "errors" + "fmt" + "runtime" + "strings" + + "golang.org/x/crypto/argon2" + "golang.org/x/crypto/bcrypt" + "gorm.io/gorm" + "tailscale.com/util/rands" +) + +// Every credential is a "hskey---" string: the +// identifier is the public, indexed lookup key and only the secret is hashed. +const ( + keyIdentifierLength = 12 + keySecretLength = 64 +) + +// 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 +) + +var ( + errSecretHashMalformed = errors.New("malformed secret hash") + errSecretMismatch = errors.New("secret does not match hash") +) + +// 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. A global semaphore sized to GOMAXPROCS; +// TODO(kradalby): revisit only if credential hashing becomes a throughput bottleneck. +var argon2Limiter = make(chan struct{}, max(2, runtime.GOMAXPROCS(0))) + +// generateSecret builds a new credential string of the form +// prefix+identifier+"-"+secret, returning the full string (shown ONCE to the +// user), the public identifier used for lookup, and the Argon2id hash of the +// secret to store. It is the single generator for every credential kind. +func generateSecret(prefix string) (string, string, []byte, error) { + identifier := rands.HexString(keyIdentifierLength) + secret := rands.HexString(keySecretLength) + full := prefix + identifier + "-" + secret + + hash, err := hashSecret(secret) + if err != nil { + return "", "", nil, err + } + + return full, identifier, hash, nil +} + +// rehashToArgon2id upgrades a credential's stored hash to Argon2id after a +// successful verify reported the secret was still under legacy bcrypt. It is +// best effort: a failed upgrade leaves the bcrypt hash in place, so the next +// authentication simply tries again. model must be a pointer to a credential +// row with its primary key set and a "hash" column. +func rehashToArgon2id(db *gorm.DB, model any, secret string) { + hash, err := hashSecret(secret) + if err != nil { + return + } + + _ = db.Model(model).Update("hash", hash).Error +} + +// 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 stored hash, transparently +// accepting both the current Argon2id PHC form and legacy bcrypt hashes. A +// matched bcrypt hash returns needsRehash=true so the caller can upgrade the +// stored hash to Argon2id on the next successful authentication. +// +// TODO(kradalby): remove the bcrypt branch in 0.32; any credential not rehashed +// by then stops authenticating (pre-announced breaking change). +func verifySecret(encoded []byte, secret string) (bool, error) { + if bytes.HasPrefix(encoded, []byte("$argon2id$")) { + return false, verifyArgon2id(encoded, secret) + } + + // Legacy bcrypt hash: a clean compare means the secret is valid but stored + // under the old algorithm, so request a rehash. A mismatch is a wrong + // secret; any other bcrypt error means the stored hash is not a usable hash. + 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 reads the cost parameters from the stored hash and compares in +// constant time so a mismatch leaks no timing signal. +func verifyArgon2id(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 +} diff --git a/hscontrol/db/secret_test.go b/hscontrol/db/secret_test.go new file mode 100644 index 000000000..80f0a6fe5 --- /dev/null +++ b/hscontrol/db/secret_test.go @@ -0,0 +1,58 @@ +package db + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" +) + +// TestVerifySecretArgon2idNoRehash verifies that an Argon2id-stored secret +// matches and reports no rehash needed. +func TestVerifySecretArgon2idNoRehash(t *testing.T) { + hash, err := hashSecret("s3cr3t") + require.NoError(t, err) + + needsRehash, err := verifySecret(hash, "s3cr3t") + require.NoError(t, err) + require.False(t, needsRehash, "argon2id hash should never need a rehash") +} + +// TestVerifySecretBcryptNeedsRehash verifies that a legacy bcrypt-stored secret +// still authenticates and is flagged for lazy rehash to Argon2id. +func TestVerifySecretBcryptNeedsRehash(t *testing.T) { + hash, err := bcrypt.GenerateFromPassword([]byte("s3cr3t"), bcrypt.DefaultCost) + require.NoError(t, err) + + needsRehash, err := verifySecret(hash, "s3cr3t") + require.NoError(t, err) + require.True(t, needsRehash, "a matched bcrypt hash must request a rehash to argon2id") + + _, err = verifySecret(hash, "wrong") + require.ErrorIs(t, err, errSecretMismatch) +} + +// TestVerifySecretMalformed verifies that a hash in no recognised format is +// rejected rather than silently accepted. +func TestVerifySecretMalformed(t *testing.T) { + _, err := verifySecret([]byte("not-a-hash"), "s3cr3t") + require.Error(t, err) +} + +// TestGenerateSecret verifies the unified key generation: hskey--, +// a 12-char identifier, a 64-char secret, and an argon2id hash of the secret. +func TestGenerateSecret(t *testing.T) { + full, identifier, hash, err := generateSecret("hskey-test-") + require.NoError(t, err) + + require.Len(t, identifier, 12) + require.True(t, strings.HasPrefix(full, "hskey-test-"+identifier+"-")) + + secret := strings.TrimPrefix(full, "hskey-test-"+identifier+"-") + require.Len(t, secret, 64) + + needsRehash, err := verifySecret(hash, secret) + require.NoError(t, err) + require.False(t, needsRehash) +}