Use SCRAM-SHA-256 to authenticate PgBouncer

We originally used MD5 to authenticate with PostgreSQL to avoid storing a plaintext password. Requiring SCRAM during the lifetime of the Pod is better. Issue: [ch11210]

Author: cbandy

internal/pgbouncer/config.go

@@ -38,7 +38,8 @@ const (
 iniFileProjectionPath = "~postgres-operator.ini"
 
 authFileSecretKey = "pgbouncer-users.txt" // #nosec G101 this is a name, not a credential
-credentialSecretKey = "pgbouncer-verifier" // #nosec G101 this is a name, not a credential
+passwordSecretKey = "pgbouncer-password" // #nosec G101 this is a name, not a credential
+verifierSecretKey = "pgbouncer-verifier" // #nosec G101 this is a name, not a credential
 emptyConfigMapKey = "pgbouncer-empty"
 iniFileConfigMapKey = "pgbouncer.ini"
 )
@@ -71,15 +72,15 @@ func (vs iniValueSet) String() string {
 }
 
 // authFileContents returns a PgBouncer user database.
-func authFileContents(password []byte) []byte {
+func authFileContents(password string) []byte {
 // > There should be at least 2 fields, surrounded by double quotes.
 // > Double quotes in a field value can be escaped by writing two double quotes.
 // - https://www.pgbouncer.org/config.html#authentication-file-format
 quote := func(s string) string {
 return `"` + strings.ReplaceAll(s, `"`, `""`) + `"`
 }
 
-user1 := quote(postgresqlUser) + " " + quote(string(password)) + "\n"
+user1 := quote(postgresqlUser) + " " + quote(password) + "\n"
 
 return []byte(user1)
 }

internal/pgbouncer/config_test.go

@@ -44,7 +44,7 @@ func TestAuthFileContents(t *testing.T) {
 t.Parallel()
 
 password := `very"random`
-data := authFileContents([]byte(password))
+data := authFileContents(password)
 assert.Equal(t, string(data), `"_crunchypgbouncer" "very""random"`+"\n")
 }
 

internal/pgbouncer/postgres.go

@@ -207,7 +207,7 @@ REVOKE ALL PRIVILEGES
 map[string]string{
 "username": postgresqlUser,
 "namespace": postgresqlSchema,
-"verifier": string(clusterSecret.Data[credentialSecretKey]),
+"verifier": string(clusterSecret.Data[verifierSecretKey]),
 
 "ON_ERROR_STOP": "on", // Abort when any one statement fails.
 "QUIET": "on", // Do not print successful statements to stdout.
@@ -218,21 +218,29 @@ REVOKE ALL PRIVILEGES
 return err
 }
 
-func generateVerifier() ([]byte, error) {
-v, err := util.GeneratePassword(32)
+func generatePassword() (plaintext, verifier string, err error) {
+// PgBouncer can login to PostgreSQL using either MD5 or SCRAM-SHA-256.
+// When using MD5, the (hashed) verifier can be stored in PgBouncer's
+// authentication file. When using SCRAM, the plaintext password must be
+// stored.
+// - https://www.pgbouncer.org/config.html#authentication-file-format
+// - https://github.com/pgbouncer/pgbouncer/issues/508#issuecomment-713339834
+
+plaintext, err = util.GeneratePassword(32)
 if err == nil {
-// NOTE(cbandy): It is not possible to use a SCRAM password for the
-// "auth_user" account.
-// - https://github.com/pgbouncer/pgbouncer/issues/508#issuecomment-713339834
-v, err = password.NewMD5Password(postgresqlUser, v).Build()
+verifier, err = password.NewSCRAMPassword(plaintext).Build()
 }
-return []byte(v), err
+return
 }
 
-func postgresqlHBA() postgres.HostBasedAuthentication {
-// PgBouncer connects over TLS using an MD5 password.
-// NOTE(cbandy): It is not possible to use a SCRAM password for the
-// "auth_user" account.
-// - https://github.com/pgbouncer/pgbouncer/issues/508#issuecomment-713339834
-return *postgres.NewHBA().User(postgresqlUser).TLS().Method("md5")
+func postgresqlHBAs() []postgres.HostBasedAuthentication {
+// PgBouncer must connect over TLS using a SCRAM password. Other network
+// connections are forbidden.
+// - https://www.postgresql.org/docs/current/auth-pg-hba-conf.html
+// - https://www.postgresql.org/docs/current/auth-password.html
+
+return []postgres.HostBasedAuthentication{
+*postgres.NewHBA().User(postgresqlUser).TLS().Method("scram-sha-256"),
+*postgres.NewHBA().User(postgresqlUser).TCP().Method("reject"),
+}
 }

internal/pgbouncer/postgres_test.go

@@ -193,6 +193,9 @@ COMMIT;`))
 assert.Equal(t, expected, EnableInPostgreSQL(ctx, exec, secret))
 }
 
-func TestPostgreSQLHBA(t *testing.T) {
-assert.Equal(t, postgresqlHBA().String(), `hostssl all "_crunchypgbouncer" all md5`)
+func TestPostgreSQLHBAs(t *testing.T) {
+rules := postgresqlHBAs()
+assert.Equal(t, len(rules), 2)
+assert.Equal(t, rules[0].String(), `hostssl all "_crunchypgbouncer" all scram-sha-256`)
+assert.Equal(t, rules[1].String(), `host all "_crunchypgbouncer" all reject`)
 }

internal/pgbouncer/reconcile.go

@@ -62,14 +62,23 @@ func Secret(ctx context.Context,
 var err error
 initialize.ByteMap(&outSecret.Data)
 
-verifier := inSecret.Data[credentialSecretKey]
-
-if err == nil && len(verifier) == 0 {
-verifier, err = generateVerifier()
+// Use the existing password and verifier. Generate both when either is missing.
+// NOTE(cbandy): We don't have a function to compare a plaintext password
+// to a SCRAM verifier.
+password := string(inSecret.Data[passwordSecretKey])
+verifier := string(inSecret.Data[verifierSecretKey])
+
+if err == nil && (len(password) == 0 || len(verifier) == 0) {
+password, verifier, err = generatePassword()
+err = errors.WithStack(err)
 }
+
 if err == nil {
-outSecret.Data[authFileSecretKey] = authFileContents(verifier)
-outSecret.Data[credentialSecretKey] = verifier
+// Store the SCRAM verifier alongside the plaintext password so that
+// later reconciles don't generate it repeatedly.
+outSecret.Data[authFileSecretKey] = authFileContents(password)
+outSecret.Data[passwordSecretKey] = []byte(password)
+outSecret.Data[verifierSecretKey] = []byte(verifier)
 }
 
 if inCluster.Spec.Proxy.PGBouncer.CustomTLSSecret == nil {
@@ -216,5 +225,5 @@ func PostgreSQL(
 return
 }
 
-outHBAs.Mandatory = append(outHBAs.Mandatory, postgresqlHBA())
+outHBAs.Mandatory = append(outHBAs.Mandatory, postgresqlHBAs()...)
 }

internal/pgbouncer/reconcile_test.go

@@ -87,6 +87,7 @@ func TestSecret(t *testing.T) {
 assert.DeepEqual(t, constant, existing)
 
 // A password should be generated.
+assert.Assert(t, len(intent.Data["pgbouncer-password"]) != 0)
 assert.Assert(t, len(intent.Data["pgbouncer-verifier"]) != 0)
 
 // The output of authFileContents should go into intent.
@@ -350,7 +351,7 @@ func TestPostgreSQL(t *testing.T) {
 
 assert.DeepEqual(t, hbas,
 &postgres.HBAs{
-Mandatory: []postgres.HostBasedAuthentication{postgresqlHBA()},
+Mandatory: postgresqlHBAs(),
 },
 // postgres.HostBasedAuthentication has unexported fields. Call String() to compare.
 cmp.Transformer("", postgres.HostBasedAuthentication.String))