diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go index b3b60df04..57fce14eb 100644 --- a/pkg/util/users/users.go +++ b/pkg/util/users/users.go @@ -60,11 +60,13 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM } } else { r := spec.PgSyncUserRequest{} - newMD5Password := util.NewEncryptor(strategy.PasswordEncryption).PGUserPassword(newUser) - // do not compare for roles coming from docker image - if dbUser.Password != newMD5Password { - r.User.Password = newMD5Password + // A plain string comparison with a freshly generated hash would + // re-issue ALTER ROLE on every sync for SCRAM-SHA-256, because + // each generated verifier embeds a new random salt. Verify the + // stored hash against the desired password instead. + if !util.PGUserPasswordUpToDate(newUser, dbUser.Password, strategy.PasswordEncryption) { + r.User.Password = util.NewEncryptor(strategy.PasswordEncryption).PGUserPassword(newUser) r.Kind = spec.PGsyncUserAlter } if addNewRoles, equal := util.SubstractStringSlices(newUser.MemberOf, dbUser.MemberOf); !equal { diff --git a/pkg/util/util.go b/pkg/util/util.go index 79ff52282..689d8441c 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -13,6 +13,7 @@ import ( "reflect" "regexp" "sort" + "strconv" "strings" "time" @@ -94,14 +95,21 @@ func NewEncryptor(encryption string) *Encryptor { } func (e *Encryptor) PGUserPassword(user spec.PgUser) string { - if (len(user.Password) == md5.Size*2+len(md5prefix) && user.Password[:3] == md5prefix) || - (len(user.Password) > len(scramsha256prefix) && user.Password[:len(scramsha256prefix)] == scramsha256prefix) || user.Password == "" { + if isMD5Hash(user.Password) || isScramHash(user.Password) || user.Password == "" { // Avoid processing already encrypted or empty passwords return user.Password } return e.encrypt(user) } +func isMD5Hash(password string) bool { + return len(password) == md5.Size*2+len(md5prefix) && password[:3] == md5prefix +} + +func isScramHash(password string) bool { + return len(password) > len(scramsha256prefix) && password[:len(scramsha256prefix)] == scramsha256prefix +} + func (e *Encryptor) PGUserPasswordMD5(user spec.PgUser) string { s := md5.Sum([]byte(user.Password + user.Name)) // #nosec, using md5 since PostgreSQL uses it for hashing passwords. return md5prefix + hex.EncodeToString(s[:]) @@ -127,6 +135,91 @@ func (e *Encryptor) PGUserPasswordScramSHA256(user spec.PgUser) string { return pass } +// PGUserPasswordUpToDate reports whether the password hash stored in the +// database already corresponds to the user's desired password and the +// configured password encryption, i.e. whether ALTER ROLE ... PASSWORD can +// be skipped during role sync. +// +// A SCRAM-SHA-256 verifier embeds a random salt, so regenerating one from +// the plaintext and comparing strings never matches. Instead, the salt and +// iteration count are taken from the stored verifier and the derived keys +// are compared. A stored hash whose type differs from the configured +// encryption is reported as outdated so that changing password_encryption +// still re-hashes the roles. +func PGUserPasswordUpToDate(user spec.PgUser, storedPassword, encryption string) bool { + // Empty and pre-hashed desired passwords can only be compared verbatim, + // mirroring the early return in PGUserPassword. + if user.Password == "" || isMD5Hash(user.Password) || isScramHash(user.Password) { + return user.Password == storedPassword + } + + switch { + case isMD5Hash(storedPassword): + if encryption != "md5" { + return false + } + return NewEncryptor(encryption).PGUserPassword(user) == storedPassword + case isScramHash(storedPassword): + if encryption == "md5" { + return false + } + return scramVerifierMatches(user.Password, storedPassword) + } + + return false +} + +// scramVerifierMatches verifies a plaintext password against a stored +// SCRAM-SHA-256 verifier of the form +// SCRAM-SHA-256$:$: +// by re-deriving the keys with the stored salt and iteration count. +func scramVerifierMatches(password, verifier string) bool { + rest := strings.TrimPrefix(verifier, scramsha256prefix+"$") + if rest == verifier { + return false + } + + saltedParams, keys, found := strings.Cut(rest, "$") + if !found { + return false + } + iterationsPart, saltPart, found := strings.Cut(saltedParams, ":") + if !found { + return false + } + storedKeyPart, serverKeyPart, found := strings.Cut(keys, ":") + if !found { + return false + } + + iterationCount, err := strconv.Atoi(iterationsPart) + if err != nil || iterationCount < 1 { + return false + } + salt, err := base64.StdEncoding.DecodeString(saltPart) + if err != nil { + return false + } + storedKey, err := base64.StdEncoding.DecodeString(storedKeyPart) + if err != nil { + return false + } + serverKey, err := base64.StdEncoding.DecodeString(serverKeyPart) + if err != nil { + return false + } + + key := pbkdf2.Key([]byte(password), salt, iterationCount, 32, sha256.New) + mac := hmac.New(sha256.New, key) + mac.Write([]byte("Server Key")) + derivedServerKey := mac.Sum(nil) + mac = hmac.New(sha256.New, key) + mac.Write([]byte("Client Key")) + derivedStoredKey := sha256.Sum256(mac.Sum(nil)) + + return hmac.Equal(derivedServerKey, serverKey) && hmac.Equal(derivedStoredKey[:], storedKey) +} + // Diff returns diffs between 2 objects func Diff(a, b interface{}) []string { return pretty.Diff(a, b) diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 37e41f1cf..755aa4318 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -162,6 +162,41 @@ func TestPGUserPassword(t *testing.T) { } } +func TestPGUserPasswordUpToDate(t *testing.T) { + user := spec.PgUser{Name: "someuser", Password: "password"} + md5Hash := NewEncryptor("md5").PGUserPassword(user) + // real generation path: random salt in the verifier + scramHash := NewEncryptor("scram-sha-256").PGUserPassword(user) + + tests := []struct { + name string + user spec.PgUser + stored string + encryption string + want bool + }{ + {"scram verifier matches its plaintext", user, scramHash, "scram-sha-256", true}, + {"a differently salted verifier of the same password matches", user, NewEncryptor("scram-sha-256").PGUserPassword(user), "scram-sha-256", true}, + {"scram verifier of another password does not match", spec.PgUser{Name: "someuser", Password: "different"}, scramHash, "scram-sha-256", false}, + {"md5 hash matches its plaintext", user, md5Hash, "md5", true}, + {"md5 hash of another password does not match", spec.PgUser{Name: "someuser", Password: "different"}, md5Hash, "md5", false}, + {"stored md5 is outdated when scram is configured", user, md5Hash, "scram-sha-256", false}, + {"stored scram is outdated when md5 is configured", user, scramHash, "md5", false}, + {"pre-hashed desired password compares verbatim", spec.PgUser{Name: "someuser", Password: md5Hash}, md5Hash, "md5", true}, + {"pre-hashed desired password differs from stored", spec.PgUser{Name: "someuser", Password: md5Hash}, scramHash, "md5", false}, + {"empty desired password matches empty stored", spec.PgUser{Name: "someuser"}, "", "scram-sha-256", true}, + {"empty desired password differs from stored hash", spec.PgUser{Name: "someuser"}, scramHash, "scram-sha-256", false}, + {"malformed stored hash is outdated", user, "not-a-hash", "scram-sha-256", false}, + {"truncated scram verifier is outdated", user, "SCRAM-SHA-256$4096:c2FsdA==", "scram-sha-256", false}, + {"scram verifier with bad base64 is outdated", user, "SCRAM-SHA-256$4096:!!$aaaa:bbbb", "scram-sha-256", false}, + } + for _, tt := range tests { + if got := PGUserPasswordUpToDate(tt.user, tt.stored, tt.encryption); got != tt.want { + t.Errorf("%s: PGUserPasswordUpToDate expected %v, got %v", tt.name, tt.want, got) + } + } +} + func TestPrettyDiff(t *testing.T) { for _, tt := range prettyDiffTest { if actual := PrettyDiff(tt.inA, tt.inB); actual != tt.out {