diff --git a/internal/pgbouncer/postgres.go b/internal/pgbouncer/postgres.go index d7d2bae5cf..ce8ef74171 100644 --- a/internal/pgbouncer/postgres.go +++ b/internal/pgbouncer/postgres.go @@ -12,8 +12,6 @@ import ( "github.com/crunchydata/postgres-operator/internal/logging" "github.com/crunchydata/postgres-operator/internal/postgres" - "github.com/crunchydata/postgres-operator/internal/postgres/password" - "github.com/crunchydata/postgres-operator/internal/util" ) const ( @@ -203,21 +201,6 @@ REVOKE ALL PRIVILEGES return err } -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.GenerateASCIIPassword(32) - if err == nil { - verifier, err = password.NewSCRAMPassword(plaintext).Build() - } - return -} - func postgresqlHBAs() []*postgres.HostBasedAuthentication { // PgBouncer must connect over TLS using a SCRAM password. Other network // connections are forbidden. diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index 4181cea478..66cf1c8df5 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -18,6 +18,8 @@ import ( "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/pki" "github.com/crunchydata/postgres-operator/internal/postgres" + passwd "github.com/crunchydata/postgres-operator/internal/postgres/password" + "github.com/crunchydata/postgres-operator/internal/util" "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) @@ -54,14 +56,29 @@ func Secret(ctx context.Context, var err error initialize.Map(&outSecret.Data) - // Use the existing password and verifier. Generate both when either is missing. + // Use the existing password and verifier. Generate when one is missing. + // 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 // 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() + if len(password) == 0 { + // If the password is empty, generate new password and verifier. + password, err = util.GenerateASCIIPassword(32) + err = errors.WithStack(err) + if err == nil { + verifier, err = passwd.NewSCRAMPassword(password).Build() + err = errors.WithStack(err) + } + } else if len(password) != 0 && len(verifier) == 0 { + // If the password is non-empty and the verifier is empty, generate a new verifier. + verifier, err = passwd.NewSCRAMPassword(password).Build() err = errors.WithStack(err) } diff --git a/internal/pgbouncer/reconcile_test.go b/internal/pgbouncer/reconcile_test.go index 927f8a25fb..b8c2a2a9fe 100644 --- a/internal/pgbouncer/reconcile_test.go +++ b/internal/pgbouncer/reconcile_test.go @@ -91,6 +91,53 @@ func TestSecret(t *testing.T) { assert.DeepEqual(t, before, intent) } +func TestSCRAMVerifier(t *testing.T) { + t.Parallel() + + ctx := context.Background() + cluster := new(v1beta1.PostgresCluster) + service := new(corev1.Service) + existing := new(corev1.Secret) + intent := new(corev1.Secret) + + root, err := pki.NewRootCertificateAuthority() + assert.NilError(t, err) + + cluster.Spec.Proxy = new(v1beta1.PostgresProxySpec) + cluster.Spec.Proxy.PGBouncer = new(v1beta1.PGBouncerPodSpec) + cluster.Default() + + // Simulate the setting of a password only + existing.Data = map[string][]byte{ + "pgbouncer-password": []byte("password"), + } + + // Verify that a SCRAM verifier is set + assert.NilError(t, Secret(ctx, cluster, root, existing, service, intent)) + assert.Assert(t, len(intent.Data["pgbouncer-verifier"]) != 0) + + // Simulate the setting of a password and a verifier + intent = new(corev1.Secret) + existing.Data = map[string][]byte{ + "pgbouncer-verifier": []byte("SCRAM-SHA-256$4096:randomsalt:storedkey:serverkey"), + "pgbouncer-password": []byte("password"), + } + assert.NilError(t, Secret(ctx, cluster, root, existing, service, intent)) + assert.Equal(t, string(intent.Data["pgbouncer-verifier"]), "SCRAM-SHA-256$4096:randomsalt:storedkey:serverkey") + assert.Equal(t, string(intent.Data["pgbouncer-password"]), "password") + + // Simulate the setting of a verifier only + intent = new(corev1.Secret) + existing.Data = map[string][]byte{ + "pgbouncer-verifier": []byte("SCRAM-SHA-256$4096:randomsalt:storedkey:serverkey"), + } + assert.NilError(t, Secret(ctx, cluster, root, existing, service, intent)) + assert.Assert(t, string(intent.Data["pgbouncer-verifier"]) != "SCRAM-SHA-256$4096:randomsalt:storedkey:serverkey") + assert.Assert(t, len(intent.Data["pgbouncer-password"]) != 0) + assert.Assert(t, len(intent.Data["pgbouncer-verifier"]) != 0) + +} + func TestPod(t *testing.T) { t.Parallel()