Merge 36455ed581 into 6fbf962b1c

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 {

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$<iterations>:<salt>$<storedKey>:<serverKey>
+// 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)

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 {