fix(apiserver): bcrypt passwords and default-credential warnings in config-file auth (bug-1947)
Cause: ConfigFileProvider.Authenticate compared the stored plaintext password with != (timing side channel, passwords readable from the file), and config/auth.yaml shipped admin/admin, user1/user1 etc. as plaintext with nothing reporting that a default credential was active. Fix: auth.yaml passwords may be bcrypt hashes (golang.org/x/crypto/bcrypt, v0.23.0 as already pinned in go.mod, now vendored); plaintext entries are still accepted but compared in constant time (SHA-256 + subtle), and an unknown user costs the same bcrypt work. The provider reports startup warnings (SecurityWarner, logged by main) for every plaintext entry and for every shipped default credential still active. The shipped auth.yaml and auth.yaml.example now store bcrypt hashes of the same dev passwords, so tests/regression/run.sh (admin/admin) keeps working. tools/encrypt gains -action=hash to generate entries. Tests: TestConfigFileProvider_BcryptPassword, TestConfigFileProvider_SecurityWarnings, TestShippedAuthConfig_HashedDefaults. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d3ce298232
commit
f91642dbc0
14 changed files with 1037 additions and 16 deletions
|
|
@ -1,8 +1,13 @@
|
|||
package auth
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"crypto/subtle"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
|
|
@ -18,15 +23,32 @@ type ConfigFileAuth struct {
|
|||
}
|
||||
|
||||
// UserEntry represents a single user in auth.yaml.
|
||||
// Password is a bcrypt hash ("$2a$"/"$2b$"/"$2y$" prefix, recommended) or,
|
||||
// for backward compatibility, a plaintext password (warned at startup).
|
||||
type UserEntry struct {
|
||||
Username string `yaml:"username"`
|
||||
Password string `yaml:"password"`
|
||||
Roles []string `yaml:"roles"`
|
||||
}
|
||||
|
||||
// knownDefaultPasswords lists the dev credentials shipped in config/auth.yaml
|
||||
// and auth.yaml.example; a deployment still using one is warned at startup.
|
||||
var knownDefaultPasswords = map[string]string{
|
||||
"admin": "admin",
|
||||
"user1": "user1",
|
||||
"user2": "user2",
|
||||
"op7x2k9": "CHANGE_ME_dev_only",
|
||||
}
|
||||
|
||||
// dummyHash is compared against for unknown users so that an unknown
|
||||
// username costs the same bcrypt work as a known one (no user enumeration
|
||||
// through response timing).
|
||||
var dummyHash = []byte("$2a$10$lvIFyyQJsUB5cZz68N0Yl.9sMCtqXARtlLnRmPc//gjDJfiOzplPq")
|
||||
|
||||
// ConfigFileProvider authenticates users against a YAML config file.
|
||||
type ConfigFileProvider struct {
|
||||
users map[string]UserEntry
|
||||
users map[string]UserEntry
|
||||
warnings []string
|
||||
}
|
||||
|
||||
// NewConfigFileProvider creates a provider from raw YAML bytes.
|
||||
|
|
@ -51,21 +73,74 @@ func NewConfigFileProvider(data []byte) (*ConfigFileProvider, error) {
|
|||
users[u.Username] = u
|
||||
}
|
||||
|
||||
return &ConfigFileProvider{users: users}, nil
|
||||
p := &ConfigFileProvider{users: users}
|
||||
p.warnings = p.collectWarnings()
|
||||
return p, nil
|
||||
}
|
||||
|
||||
// Authenticate checks username/password against the config file.
|
||||
func (p *ConfigFileProvider) Authenticate(username, password string) error {
|
||||
entry, ok := p.users[username]
|
||||
if !ok {
|
||||
_ = bcrypt.CompareHashAndPassword(dummyHash, []byte(password))
|
||||
return fmt.Errorf("invalid username or password")
|
||||
}
|
||||
if entry.Password != password {
|
||||
if !passwordMatches(entry.Password, password) {
|
||||
return fmt.Errorf("invalid username or password")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// passwordMatches compares a candidate password with a stored bcrypt hash,
|
||||
// or in constant time with a stored plaintext password.
|
||||
func passwordMatches(stored, candidate string) bool {
|
||||
if isBcryptHash(stored) {
|
||||
return bcrypt.CompareHashAndPassword([]byte(stored), []byte(candidate)) == nil
|
||||
}
|
||||
// Hash both sides first so the comparison does not leak the length.
|
||||
s := sha256.Sum256([]byte(stored))
|
||||
c := sha256.Sum256([]byte(candidate))
|
||||
return subtle.ConstantTimeCompare(s[:], c[:]) == 1
|
||||
}
|
||||
|
||||
// isBcryptHash reports whether s looks like a bcrypt hash.
|
||||
func isBcryptHash(s string) bool {
|
||||
if len(s) != 60 {
|
||||
return false
|
||||
}
|
||||
return strings.HasPrefix(s, "$2a$") || strings.HasPrefix(s, "$2b$") || strings.HasPrefix(s, "$2y$")
|
||||
}
|
||||
|
||||
// collectWarnings returns one message per user with a plaintext password or
|
||||
// a known default credential. Messages name the user, never the password.
|
||||
func (p *ConfigFileProvider) collectWarnings() []string {
|
||||
names := make([]string, 0, len(p.users))
|
||||
for name := range p.users {
|
||||
names = append(names, name)
|
||||
}
|
||||
sort.Strings(names)
|
||||
|
||||
var warnings []string
|
||||
for _, name := range names {
|
||||
entry := p.users[name]
|
||||
if !isBcryptHash(entry.Password) {
|
||||
warnings = append(warnings, fmt.Sprintf(
|
||||
"auth.yaml user %q has a plaintext password; store a bcrypt hash instead", name))
|
||||
}
|
||||
if def, ok := knownDefaultPasswords[name]; ok && passwordMatches(entry.Password, def) {
|
||||
warnings = append(warnings, fmt.Sprintf(
|
||||
"auth.yaml user %q uses the shipped default password; change it before exposing the API server", name))
|
||||
}
|
||||
}
|
||||
return warnings
|
||||
}
|
||||
|
||||
// SecurityWarnings returns the startup warnings about stored credentials
|
||||
// (implements SecurityWarner).
|
||||
func (p *ConfigFileProvider) SecurityWarnings() []string {
|
||||
return p.warnings
|
||||
}
|
||||
|
||||
// GetUserRoles returns roles defined in the config for the given user.
|
||||
func (p *ConfigFileProvider) GetUserRoles(username string) []string {
|
||||
if entry, ok := p.users[username]; ok {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package auth
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
|
@ -144,3 +145,112 @@ func TestOAuth2Provider_Name(t *testing.T) {
|
|||
t.Error("expected not-implemented error")
|
||||
}
|
||||
}
|
||||
|
||||
// bcrypt hashes (cost 10) used by the bug-1947 tests.
|
||||
const (
|
||||
testS3cretHash = "$2a$10$lvIFyyQJsUB5cZz68N0Yl.9sMCtqXARtlLnRmPc//gjDJfiOzplPq" // "s3cret"
|
||||
testAdminHash = "$2a$10$WVwHd5Nd22UP1DfIpkekjum0yYVGctD2N.hheBP.5ZxGL92W/u5Ja" // "admin"
|
||||
)
|
||||
|
||||
// bug-1947: auth.yaml passwords may be stored as bcrypt hashes.
|
||||
func TestConfigFileProvider_BcryptPassword(t *testing.T) {
|
||||
yaml := `
|
||||
provider: config_file
|
||||
config_file:
|
||||
users:
|
||||
- username: alice
|
||||
password: "` + testS3cretHash + `"
|
||||
roles: [basic_user]
|
||||
`
|
||||
provider, err := NewConfigFileProvider([]byte(yaml))
|
||||
if err != nil {
|
||||
t.Fatalf("NewConfigFileProvider: %v", err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
pass string
|
||||
wantErr bool
|
||||
}{
|
||||
{"correct password", "s3cret", false},
|
||||
{"wrong password", "S3cret", true},
|
||||
{"empty password", "", true},
|
||||
{"the stored hash itself", testS3cretHash, true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := provider.Authenticate("alice", tt.pass)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Errorf("Authenticate() error = %v, wantErr %v", err, tt.wantErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1947: startup warnings for plaintext entries and known default
|
||||
// credentials; none for a bcrypt entry with a non-default password.
|
||||
func TestConfigFileProvider_SecurityWarnings(t *testing.T) {
|
||||
yaml := `
|
||||
provider: config_file
|
||||
config_file:
|
||||
users:
|
||||
- username: alice
|
||||
password: "` + testS3cretHash + `"
|
||||
roles: [basic_user]
|
||||
- username: bob
|
||||
password: plain-not-default
|
||||
roles: [basic_user]
|
||||
- username: admin
|
||||
password: "` + testAdminHash + `"
|
||||
roles: [cluster_admin]
|
||||
`
|
||||
provider, err := NewConfigFileProvider([]byte(yaml))
|
||||
if err != nil {
|
||||
t.Fatalf("NewConfigFileProvider: %v", err)
|
||||
}
|
||||
warnings := provider.SecurityWarnings()
|
||||
joined := strings.Join(warnings, "\n")
|
||||
if !strings.Contains(joined, `"bob"`) || !strings.Contains(joined, "plaintext") {
|
||||
t.Errorf("expected a plaintext warning for bob, got %q", warnings)
|
||||
}
|
||||
if !strings.Contains(joined, `"admin"`) || !strings.Contains(joined, "default") {
|
||||
t.Errorf("expected a default-credential warning for admin, got %q", warnings)
|
||||
}
|
||||
if strings.Contains(joined, `"alice"`) {
|
||||
t.Errorf("alice has a bcrypt non-default password, got warning %q", warnings)
|
||||
}
|
||||
for _, w := range warnings {
|
||||
if strings.Contains(w, "plain-not-default") || strings.Contains(w, "$2a$") {
|
||||
t.Errorf("warning must not contain a password or hash: %q", w)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// bug-1947: the shipped auth.yaml (and its example) store only bcrypt
|
||||
// hashes; the dev default admin/admin still logs in (tests/regression/run.sh
|
||||
// relies on it) but is reported as a default credential at startup.
|
||||
func TestShippedAuthConfig_HashedDefaults(t *testing.T) {
|
||||
for _, path := range []string{"../../config/auth.yaml", "../../config/auth.yaml.example"} {
|
||||
t.Run(path, func(t *testing.T) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", path, err)
|
||||
}
|
||||
provider, err := NewConfigFileProvider(data)
|
||||
if err != nil {
|
||||
t.Fatalf("NewConfigFileProvider: %v", err)
|
||||
}
|
||||
for name, u := range provider.users {
|
||||
if !isBcryptHash(u.Password) {
|
||||
t.Errorf("user %q: shipped password is not a bcrypt hash", name)
|
||||
}
|
||||
}
|
||||
if err := provider.Authenticate("admin", "admin"); err != nil {
|
||||
t.Errorf("admin/admin dev login must keep working: %v", err)
|
||||
}
|
||||
if !strings.Contains(strings.Join(provider.SecurityWarnings(), "\n"), `"admin"`) {
|
||||
t.Errorf("admin/admin must be reported as a default credential")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,13 @@ type UserRoleSeeder interface {
|
|||
AllUsers() []string
|
||||
}
|
||||
|
||||
// SecurityWarner is an optional interface for providers that detect insecure
|
||||
// configuration (plaintext or default credentials); main logs each warning at
|
||||
// startup.
|
||||
type SecurityWarner interface {
|
||||
SecurityWarnings() []string
|
||||
}
|
||||
|
||||
// AuthProvider is the interface for authentication strategies.
|
||||
type AuthProvider interface {
|
||||
Authenticate(username, password string) error
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue