diff --git a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml index abd625f827..8b8bfee823 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml @@ -39,6 +39,86 @@ spec: spec: description: PostgresClusterSpec defines the desired state of PostgresCluster properties: + authentication: + properties: + rules: + description: 'More info: https://www.postgresql.org/docs/current/auth-pg-hba-conf.html' + items: + properties: + connection: + description: |- + The connection transport this rule matches. Typical values are: + 1. "host" for network connections that may or may not be encrypted. + 2. "hostssl" for network connections encrypted using TLS. + 3. "hostgssenc" for network connections encrypted using GSSAPI. + maxLength: 20 + minLength: 1 + pattern: ^[-a-z0-9]+$ + type: string + databases: + description: Which databases this rule matches. When omitted + or empty, this rule matches all databases. + items: + maxLength: 63 + minLength: 1 + type: string + maxItems: 20 + type: array + x-kubernetes-list-type: atomic + hba: + description: One line of the "pg_hba.conf" file. Changes + to this value will be automatically reloaded without validation. + maxLength: 100 + minLength: 1 + pattern: ^[[:print:]]+$ + type: string + x-kubernetes-validations: + - message: cannot include other files + rule: '!self.trim().startsWith("include")' + method: + description: |- + The authentication method to use when a connection matches this rule. + The special value "reject" refuses connections that match this rule. + More info: https://www.postgresql.org/docs/current/auth-methods.html + maxLength: 20 + minLength: 1 + pattern: ^[-a-z0-9]+$ + type: string + x-kubernetes-validations: + - message: the "trust" method is unsafe + rule: self != "trust" + options: + additionalProperties: + anyOf: + - type: integer + - type: string + x-kubernetes-int-or-string: true + maxProperties: 20 + type: object + x-kubernetes-map-type: atomic + users: + description: Which user names this rule matches. When omitted + or empty, this rule matches all users. + items: + maxLength: 63 + minLength: 1 + type: string + maxItems: 20 + type: array + x-kubernetes-list-type: atomic + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: '"hba" cannot be combined with other fields' + rule: 'has(self.hba) ? !has(self.connection) && !has(self.databases) + && !has(self.method) && !has(self.options) && !has(self.users) + : true' + - message: '"connection" and "method" are required' + rule: 'has(self.hba) ? true : has(self.connection) && has(self.method)' + maxItems: 10 + type: array + x-kubernetes-list-type: atomic + type: object backups: description: PostgreSQL backup configuration properties: diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index a2f6fb8611..8a3a73c8bd 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -42,12 +42,49 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +// generatePostgresHBA converts one API rule into a structured HBA rule that +// safely formats its values. +func (*Reconciler) generatePostgresHBA(spec *v1beta1.PostgresHBARule) *postgres.HostBasedAuthentication { + if spec == nil { + return nil + } + + result := postgres.NewHBA() + result.Origin(spec.Connection) + + // The "password" method is not recommended. More likely, the user wants to + // use passwords generally. The most compatible method for that is "md5" + // which accepts a password in the format in which it is hashed in the database. + // - https://www.postgresql.org/docs/current/auth-password.html + if spec.Method == "password" { + result.Method("md5") + } else { + result.Method(spec.Method) + } + + if len(spec.Databases) > 0 { + result.Databases(spec.Databases[0], spec.Databases[1:]...) + } + if len(spec.Users) > 0 { + result.Users(spec.Users[0], spec.Users[1:]...) + } + if len(spec.Options) > 0 { + opts := make(map[string]string, len(spec.Options)) + for k, v := range spec.Options { + opts[k] = v.String() + } + result.Options(opts) + } + + return result +} + // generatePostgresHBAs produces the HBA rules for cluster that incorporates, // from highest to lowest precedence: // 1. mandatory rules determined by controllers // 2. rules in cluster.spec.patroni.dynamicConfiguration // 3. default rules, when none were in cluster.spec -func (*Reconciler) generatePostgresHBAs( +func (r *Reconciler) generatePostgresHBAs( ctx context.Context, cluster *v1beta1.PostgresCluster, ) *postgres.OrderedHBAs { builtin := postgres.NewHBAs() @@ -58,13 +95,25 @@ func (*Reconciler) generatePostgresHBAs( // so connections are matched against them first. result := new(postgres.OrderedHBAs) result.Append(builtin.Mandatory...) + mandatory := result.Length() + + // Append any rules specified in the Authentication section. + // These take precedence over any in the Patroni section. + if authn := cluster.Spec.Authentication; authn != nil { + for _, in := range authn.Rules { + if len(in.HBA) > 0 { + result.AppendUnstructured(in.HBA) + } else { + result.Append(r.generatePostgresHBA(&in.PostgresHBARule)) + } + } + } // Append any rules specified in the Patroni section. - before := result.Length() result.AppendUnstructured(patroni.PostgresHBAs(cluster.Spec.Patroni)...) // When there are no specified rules, include the recommended defaults. - if result.Length() == before { + if result.Length() == mandatory { result.Append(builtin.Default...) } diff --git a/internal/controller/postgrescluster/postgres_test.go b/internal/controller/postgrescluster/postgres_test.go index 4f967d1088..86310c717b 100644 --- a/internal/controller/postgrescluster/postgres_test.go +++ b/internal/controller/postgrescluster/postgres_test.go @@ -34,6 +34,45 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestGeneratePostgresHBA(t *testing.T) { + reconciler := &Reconciler{} + + assert.Assert(t, reconciler.generatePostgresHBA(nil) == nil, + "expected nil to return nil") + + for _, tt := range []struct { + rule, expected string + }{ + { + rule: `{ connection: host, method: scram }`, + expected: `"host" all all all "scram"`, + }, + { + rule: `{ connection: local, method: peer, databases: [one, two] }`, + expected: `"local" "one","two" all all "peer"`, + }, + { + rule: `{ connection: local, method: peer, users: [alice, bob] }`, + expected: `"local" all "alice","bob" all "peer"`, + }, + { + rule: `{ connection: hostssl, method: md5, options: { clientcert: verify-ca } }`, + expected: `"hostssl" all all all "md5" "clientcert"="verify-ca"`, + }, + // "password" input should be "md5" output + { + rule: `{ connection: hostssl, method: password }`, + expected: `"hostssl" all all all "md5"`, + }, + } { + var rule *v1beta1.PostgresHBARule + require.UnmarshalInto(t, &rule, tt.rule) + + hba := reconciler.generatePostgresHBA(rule) + assert.Equal(t, hba.String(), tt.expected, "\n%#v", rule) + } +} + func TestGeneratePostgresHBAs(t *testing.T) { ctx := context.Background() reconciler := &Reconciler{} @@ -50,12 +89,35 @@ func TestGeneratePostgresHBAs(t *testing.T) { assert.Assert(t, len(required) > 0, "expected at least one mandatory rule") + t.Run("Authentication", func(t *testing.T) { + cluster := v1beta1.NewPostgresCluster() + require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ + rules: [ + { connection: host, method: scram }, + { connection: local, method: peer, users: [alice, bob] }, + ], + }`) + + result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings() + assert.Assert(t, cmp.Len(result, len(required)+2), + "expected two rules from the Authentication section and no defaults") + + // mandatory rules should be first + assert.DeepEqual(t, result[:len(required)], required) + + // specified rules should be last and in their original order + assert.DeepEqual(t, result[len(required):], []string{ + `"host" all all all "scram"`, + `"local" all "alice","bob" all "peer"`, + }) + }) + t.Run("Patroni", func(t *testing.T) { cluster := v1beta1.NewPostgresCluster() require.UnmarshalInto(t, &cluster.Spec.Patroni, `{ - dynamicConfiguration: { - postgresql: { pg_hba: [ "first custom", "another" ] }, - }, + dynamicConfiguration: { + postgresql: { pg_hba: [ "first custom", "another" ] }, + }, }`) result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings() @@ -68,6 +130,36 @@ func TestGeneratePostgresHBAs(t *testing.T) { // specified rules should be last and in their original order assert.DeepEqual(t, result[len(required):], []string{`first custom`, `another`}) }) + + t.Run("Precedence", func(t *testing.T) { + cluster := v1beta1.NewPostgresCluster() + require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ + rules: [ + { connection: host, method: scram }, + { connection: local, method: peer, users: [alice, bob] }, + ], + }`) + require.UnmarshalInto(t, &cluster.Spec.Patroni, `{ + dynamicConfiguration: { + postgresql: { pg_hba: [ "another" ] }, + }, + }`) + + result := reconciler.generatePostgresHBAs(ctx, cluster).AsStrings() + assert.Assert(t, cmp.Len(result, len(required)+2+1), + "expected two rules from the Authentication section"+ + " plus one from the Patroni section") + + // mandatory rules should be first + assert.DeepEqual(t, result[:len(required)], required) + + // specified rules are next, no defaults + assert.DeepEqual(t, result[len(required):], []string{ + `"host" all all all "scram"`, // Authentication + `"local" all "alice","bob" all "peer"`, // Authentication + `another`, // Patroni + }) + }) } func TestGeneratePostgresParameters(t *testing.T) { diff --git a/internal/pgbouncer/postgres.go b/internal/pgbouncer/postgres.go index d7d2bae5cf..87b915caac 100644 --- a/internal/pgbouncer/postgres.go +++ b/internal/pgbouncer/postgres.go @@ -225,7 +225,7 @@ func postgresqlHBAs() []*postgres.HostBasedAuthentication { // - 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"), + postgres.NewHBA().Users(PostgresqlUser).TLS().Method("scram-sha-256"), + postgres.NewHBA().Users(PostgresqlUser).TCP().Method("reject"), } } diff --git a/internal/pgbouncer/postgres_test.go b/internal/pgbouncer/postgres_test.go index eb3bb65818..3a4c1c3ef8 100644 --- a/internal/pgbouncer/postgres_test.go +++ b/internal/pgbouncer/postgres_test.go @@ -186,6 +186,6 @@ COMMIT;`)) 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`) + assert.Equal(t, rules[0].String(), `hostssl all "_crunchypgbouncer" all "scram-sha-256"`) + assert.Equal(t, rules[1].String(), `host all "_crunchypgbouncer" all "reject"`) } diff --git a/internal/pgmonitor/postgres.go b/internal/pgmonitor/postgres.go index 08a428d465..1d7817c9a3 100644 --- a/internal/pgmonitor/postgres.go +++ b/internal/pgmonitor/postgres.go @@ -27,9 +27,9 @@ func PostgreSQLHBAs(ctx context.Context, inCluster *v1beta1.PostgresCluster, out if ExporterEnabled(ctx, inCluster) || feature.Enabled(ctx, feature.OpenTelemetryMetrics) { // Limit the monitoring user to local connections using SCRAM. outHBAs.Mandatory = append(outHBAs.Mandatory, - postgres.NewHBA().TCP().User(MonitoringUser).Method("scram-sha-256").Network("127.0.0.0/8"), - postgres.NewHBA().TCP().User(MonitoringUser).Method("scram-sha-256").Network("::1/128"), - postgres.NewHBA().TCP().User(MonitoringUser).Method("reject")) + postgres.NewHBA().TCP().Users(MonitoringUser).Method("scram-sha-256").Network("127.0.0.0/8"), + postgres.NewHBA().TCP().Users(MonitoringUser).Method("scram-sha-256").Network("::1/128"), + postgres.NewHBA().TCP().Users(MonitoringUser).Method("reject")) } } diff --git a/internal/pgmonitor/postgres_test.go b/internal/pgmonitor/postgres_test.go index 3b6bff58de..4c1acc1dcf 100644 --- a/internal/pgmonitor/postgres_test.go +++ b/internal/pgmonitor/postgres_test.go @@ -39,9 +39,9 @@ func TestPostgreSQLHBA(t *testing.T) { PostgreSQLHBAs(ctx, inCluster, &outHBAs) assert.Equal(t, len(outHBAs.Mandatory), 3) - assert.Equal(t, outHBAs.Mandatory[0].String(), `host all "ccp_monitoring" "127.0.0.0/8" scram-sha-256`) - assert.Equal(t, outHBAs.Mandatory[1].String(), `host all "ccp_monitoring" "::1/128" scram-sha-256`) - assert.Equal(t, outHBAs.Mandatory[2].String(), `host all "ccp_monitoring" all reject`) + assert.Equal(t, outHBAs.Mandatory[0].String(), `host all "ccp_monitoring" "127.0.0.0/8" "scram-sha-256"`) + assert.Equal(t, outHBAs.Mandatory[1].String(), `host all "ccp_monitoring" "::1/128" "scram-sha-256"`) + assert.Equal(t, outHBAs.Mandatory[2].String(), `host all "ccp_monitoring" all "reject"`) }) } diff --git a/internal/postgres/hba.go b/internal/postgres/hba.go index 875a476ee2..f4fe83d114 100644 --- a/internal/postgres/hba.go +++ b/internal/postgres/hba.go @@ -6,6 +6,8 @@ package postgres import ( "fmt" + "maps" + "regexp" "slices" "strings" ) @@ -15,15 +17,15 @@ func NewHBAs() HBAs { return HBAs{ Mandatory: []*HostBasedAuthentication{ // The "postgres" superuser must always be able to connect locally. - NewHBA().Local().User("postgres").Method("peer"), + NewHBA().Local().Users("postgres").Method("peer"), // The replication user must always connect over TLS using certificate // authentication. Patroni also connects to the "postgres" database // when calling `pg_rewind`. // - https://www.postgresql.org/docs/current/warm-standby.html#STREAMING-REPLICATION-AUTHENTICATION - NewHBA().TLS().User(ReplicationUser).Method("cert").Replication(), - NewHBA().TLS().User(ReplicationUser).Method("cert").Database("postgres"), - NewHBA().TCP().User(ReplicationUser).Method("reject"), + NewHBA().TLS().Users(ReplicationUser).Method("cert").Replication(), + NewHBA().TLS().Users(ReplicationUser).Method("cert").Databases("postgres"), + NewHBA().TCP().Users(ReplicationUser).Method("reject"), }, Default: []*HostBasedAuthentication{ @@ -50,10 +52,51 @@ func NewHBA() *HostBasedAuthentication { return new(HostBasedAuthentication).AllDatabases().AllNetworks().AllUsers() } +// hbaRegexSpecialCharacters matches a superset of the special characters in +// PostgreSQL [regular expressions] for: +// +// - [HostBasedAuthentication.quoteDatabase] +// - [HostBasedAuthentication.quoteUser] +// +// [regular expressions]: https://www.postgresql.org/docs/current/functions-matching.html#POSIX-SYNTAX-DETAILS +var hbaRegexSpecialCharacters = regexp.MustCompile(`[^\pL\pN_]`) + func (*HostBasedAuthentication) quote(value string) string { return `"` + strings.ReplaceAll(value, `"`, `""`) + `"` } +func (hba *HostBasedAuthentication) quoteDatabase(name string) string { + // Since PostgreSQL 16, a quoted string beginning with slash U+002F is + // interpreted as a regular expression. Express these names as a Postgres + // regex that exactly matches the entire name. + if len(name) > 0 && name[0] == '/' { + name = "/^" + + hbaRegexSpecialCharacters.ReplaceAllStringFunc(name, + func(match string) string { return "[" + match + "]" }) + + "$" + } + + // Quotes indicate the value is NOT a keyword (all, sameuser, etc.) + // and NOT to be expanded as a filename (at sign U+0040). + return hba.quote(name) +} + +func (hba *HostBasedAuthentication) quoteUser(name string) string { + // Since PostgreSQL 16, a quoted string beginning with slash U+002F is + // interpreted as a regular expression. Express these names as a Postgres + // regex that exactly matches the entire name. + if len(name) > 0 && name[0] == '/' { + name = "/^" + + hbaRegexSpecialCharacters.ReplaceAllStringFunc(name, + func(match string) string { return "[" + match + "]" }) + + "$" + } + + // Quotes indicate the value is NOT a keyword (all), NOT a group (plus U+002B), + // and NOT to be expanded as a filename (at sign U+0040). + return hba.quote(name) +} + // AllDatabases makes hba match connections made to any database. func (hba *HostBasedAuthentication) AllDatabases() *HostBasedAuthentication { hba.database = "all" @@ -72,9 +115,12 @@ func (hba *HostBasedAuthentication) AllUsers() *HostBasedAuthentication { return hba } -// Database makes hba match connections made to a specific database. -func (hba *HostBasedAuthentication) Database(name string) *HostBasedAuthentication { - hba.database = hba.quote(name) +// Databases makes hba match connections made to specific databases. +func (hba *HostBasedAuthentication) Databases(name string, names ...string) *HostBasedAuthentication { + hba.database = hba.quoteDatabase(name) + for _, n := range names { + hba.database += "," + hba.quoteDatabase(n) + } return hba } @@ -86,7 +132,12 @@ func (hba *HostBasedAuthentication) Local() *HostBasedAuthentication { // Method specifies the authentication method to use when a connection matches hba. func (hba *HostBasedAuthentication) Method(name string) *HostBasedAuthentication { - hba.method = name + hba.method = hba.quote(name) + return hba +} + +func (hba *HostBasedAuthentication) Origin(name string) *HostBasedAuthentication { + hba.origin = hba.quote(name) return hba } @@ -105,8 +156,8 @@ func (hba *HostBasedAuthentication) NoSSL() *HostBasedAuthentication { // Options specifies any options for the authentication method. func (hba *HostBasedAuthentication) Options(opts map[string]string) *HostBasedAuthentication { hba.options = "" - for k, v := range opts { - hba.options = fmt.Sprintf("%s %s=%s", hba.options, k, hba.quote(v)) + for _, k := range slices.Sorted(maps.Keys(opts)) { + hba.options = fmt.Sprintf("%s %s=%s", hba.options, hba.quote(k), hba.quote(opts[k])) } return hba } @@ -136,9 +187,12 @@ func (hba *HostBasedAuthentication) TCP() *HostBasedAuthentication { return hba } -// User makes hba match connections by a specific user. -func (hba *HostBasedAuthentication) User(name string) *HostBasedAuthentication { - hba.user = hba.quote(name) +// Users makes hba match connections by specific users. +func (hba *HostBasedAuthentication) Users(name string, names ...string) *HostBasedAuthentication { + hba.user = hba.quoteUser(name) + for _, n := range names { + hba.user += "," + hba.quoteUser(n) + } return hba } @@ -175,7 +229,9 @@ func (o *OrderedHBAs) AppendUnstructured(hbas ...string) { // control characters, space, and backslash return r > '~' || r < '!' || r == '\\' }) - if len(hba) > 0 { + + // NOTE: Skipping "include" directives here is a security measure. + if len(hba) > 0 && !strings.HasPrefix(hba, "include") { o.records = append(o.records, hba) } } diff --git a/internal/postgres/hba_test.go b/internal/postgres/hba_test.go index c15cf5cc77..737d530024 100644 --- a/internal/postgres/hba_test.go +++ b/internal/postgres/hba_test.go @@ -30,35 +30,53 @@ func TestNewHBAs(t *testing.T) { hba := NewHBAs() assert.Assert(t, matches(hba.Mandatory, ` -local all "postgres" peer -hostssl replication "_crunchyrepl" all cert -hostssl "postgres" "_crunchyrepl" all cert -host all "_crunchyrepl" all reject +local all "postgres" "peer" +hostssl replication "_crunchyrepl" all "cert" +hostssl "postgres" "_crunchyrepl" all "cert" +host all "_crunchyrepl" all "reject" `)) assert.Assert(t, matches(hba.Default, ` -hostssl all all all md5 +hostssl all all all "md5" `)) } func TestHostBasedAuthentication(t *testing.T) { - assert.Equal(t, `local all "postgres" peer`, - NewHBA().Local().User("postgres").Method("peer").String()) + assert.Equal(t, `local all "postgres","pgo" "peer"`, + NewHBA().Local().Users("postgres", "pgo").Method("peer").String()) - assert.Equal(t, `host all all "::1/128" trust`, + assert.Equal(t, `host all all "::1/128" "trust"`, NewHBA().TCP().Network("::1/128").Method("trust").String()) - assert.Equal(t, `host replication "KD6-3.7" samenet scram-sha-256`, + assert.Equal(t, `host replication "KD6-3.7" samenet "scram-sha-256"`, NewHBA().TCP().SameNetwork().Replication(). - User("KD6-3.7").Method("scram-sha-256"). + Users("KD6-3.7").Method("scram-sha-256"). String()) - assert.Equal(t, `hostssl "data" all all md5 clientcert="verify-ca"`, - NewHBA().TLS().Database("data"). + assert.Equal(t, `hostssl "data","bits" all all "md5" "clientcert"="verify-ca"`, + NewHBA().TLS().Databases("data", "bits"). Method("md5").Options(map[string]string{"clientcert": "verify-ca"}). String()) - assert.Equal(t, `hostnossl all all all reject`, + assert.Equal(t, `hostnossl all all all "reject"`, NewHBA().NoSSL().Method("reject").String()) + + t.Run("OptionsSorted", func(t *testing.T) { + assert.Equal(t, `hostssl all all all "ldap" "ldapbasedn"="dc=example,dc=org" "ldapserver"="example.org"`, + NewHBA().TLS().Method("ldap").Options(map[string]string{ + "ldapserver": "example.org", + "ldapbasedn": "dc=example,dc=org", + }).String()) + }) + + t.Run("SpecialCharactersEscaped", func(t *testing.T) { + // Databases; slash U+002F triggers regex escaping; regex characters themselves do not + assert.Equal(t, `local "/^[/]asdf_[+][?]1234$","/^[/][*][$]$","+*$" all`, + NewHBA().Local().Databases(`/asdf_+?1234`, `/*$`, `+*$`).String()) + + // Users; slash U+002F triggers regex escaping; regex characters themselves do not + assert.Equal(t, `local all "/^[/]asdf_[+][?]1234$","/^[/][*][$]$","+*$"`, + NewHBA().Local().Users(`/asdf_+?1234`, `/*$`, `+*$`).String()) + }) } func TestOrderedHBAs(t *testing.T) { @@ -89,7 +107,7 @@ func TestOrderedHBAs(t *testing.T) { }) // Append and AppendUnstructured do not have a separate order. - ordered.Append(NewHBA().User("zoro")) + ordered.Append(NewHBA().Users("zoro")) assert.Equal(t, ordered.Length(), 3) assert.DeepEqual(t, ordered.AsStrings(), []string{ `all all all`, @@ -109,6 +127,22 @@ func TestOrderedHBAs(t *testing.T) { }) }) + // See [internal/testing/validation.TestPostgresAuthenticationRules] + t.Run("NoInclude", func(t *testing.T) { + rules := new(OrderedHBAs) + rules.AppendUnstructured( + `one`, + `include "/etc/passwd"`, + ` include_dir /tmp`, + `include_if_exists postgresql.auto.conf`, + `two`, + ) + assert.DeepEqual(t, rules.AsStrings(), []string{ + `one`, + `two`, + }) + }) + t.Run("SpecialCharactersStripped", func(t *testing.T) { rules := new(OrderedHBAs) rules.AppendUnstructured( diff --git a/internal/testing/validation/postgrescluster_test.go b/internal/testing/validation/postgrescluster_test.go index 5c8bd9f0e3..18a17de069 100644 --- a/internal/testing/validation/postgrescluster_test.go +++ b/internal/testing/validation/postgrescluster_test.go @@ -21,6 +21,105 @@ import ( "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1" ) +func TestPostgresAuthenticationRules(t *testing.T) { + ctx := context.Background() + cc := require.Kubernetes(t) + t.Parallel() + + namespace := require.Namespace(t, cc) + base := v1beta1.NewPostgresCluster() + + // Start with a bunch of required fields. + require.UnmarshalInto(t, &base.Spec, `{ + postgresVersion: 16, + backups: { + pgbackrest: { + repos: [{ name: repo1 }], + }, + }, + instances: [{ + dataVolumeClaimSpec: { + accessModes: [ReadWriteOnce], + resources: { requests: { storage: 1Mi } }, + }, + }], + }`) + + base.Namespace = namespace.Name + base.Name = "postgres-authentication-rules" + + assert.NilError(t, cc.Create(ctx, base.DeepCopy(), client.DryRunAll), + "expected this base cluster to be valid") + + t.Run("OneTopLevel", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ + rules: [ + { connection: host, hba: anything }, + { users: [alice, bob], hba: anything }, + ], + }`) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 2)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i)) + assert.Assert(t, cmp.Contains(cause.Message, "cannot be combined")) + } + }) + + t.Run("NoInclude", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ + rules: [ + { hba: 'include "/etc/passwd"' }, + { hba: ' include_dir /tmp' }, + { hba: 'include_if_exists postgresql.auto.conf' }, + ], + }`) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 3)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].hba", i)) + assert.Assert(t, cmp.Contains(cause.Message, "cannot include")) + } + }) + + t.Run("NoStructuredTrust", func(t *testing.T) { + cluster := base.DeepCopy() + require.UnmarshalInto(t, &cluster.Spec.Authentication, `{ + rules: [ + { connection: local, method: trust }, + { connection: hostssl, method: trust }, + { connection: hostgssenc, method: trust }, + ], + }`) + + err := cc.Create(ctx, cluster, client.DryRunAll) + assert.Assert(t, apierrors.IsInvalid(err)) + + status := require.StatusError(t, err) + assert.Assert(t, status.Details != nil) + assert.Assert(t, cmp.Len(status.Details.Causes, 3)) + + for i, cause := range status.Details.Causes { + assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].method", i)) + assert.Assert(t, cmp.Contains(cause.Message, "unsafe")) + } + }) +} + func TestPostgresConfigParameters(t *testing.T) { ctx := context.Background() cc := require.Kubernetes(t) diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go index c2f5cc8d0b..8f950dbfa9 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgres_types.go @@ -9,6 +9,15 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" ) +type PostgresAuthenticationSpec struct { + // More info: https://www.postgresql.org/docs/current/auth-pg-hba-conf.html + // --- + // +kubebuilder:validation:MaxItems=10 + // +listType=atomic + // +optional + Rules []PostgresHBARuleSpec `json:"rules,omitempty"` +} + type PostgresConfig struct { // Files to mount under "/etc/postgres". // --- @@ -68,6 +77,70 @@ type PostgresConfig struct { Parameters map[string]intstr.IntOrString `json:"parameters,omitempty"` } +// --- +type PostgresHBARule struct { + // The connection transport this rule matches. Typical values are: + // 1. "host" for network connections that may or may not be encrypted. + // 2. "hostssl" for network connections encrypted using TLS. + // 3. "hostgssenc" for network connections encrypted using GSSAPI. + // --- + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=20 + // +kubebuilder:validation:Pattern=`^[-a-z0-9]+$` + // +optional + Connection string `json:"connection,omitempty"` + + // Which databases this rule matches. When omitted or empty, this rule matches all databases. + // --- + // +kubebuilder:validation:MaxItems=20 + // +listType=atomic + // +optional + Databases []PostgresIdentifier `json:"databases,omitempty"` + + // The authentication method to use when a connection matches this rule. + // The special value "reject" refuses connections that match this rule. + // More info: https://www.postgresql.org/docs/current/auth-methods.html + // --- + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=20 + // +kubebuilder:validation:Pattern=`^[-a-z0-9]+$` + // +kubebuilder:validation:XValidation:rule=`self != "trust"`,message=`the "trust" method is unsafe` + // +optional + Method string `json:"method,omitempty"` + + // --- + // +kubebuilder:validation:MaxProperties=20 + // +mapType=atomic + // +optional + Options map[string]intstr.IntOrString `json:"options,omitempty"` + + // Which user names this rule matches. When omitted or empty, this rule matches all users. + // --- + // +kubebuilder:validation:MaxItems=20 + // +listType=atomic + // +optional + Users []PostgresIdentifier `json:"users,omitempty"` +} + +// --- +// Emulate OpenAPI "anyOf" aka Kubernetes union. +// +kubebuilder:validation:XValidation:rule=`has(self.hba) ? !has(self.connection) && !has(self.databases) && !has(self.method) && !has(self.options) && !has(self.users) : true`,message=`"hba" cannot be combined with other fields` +// +kubebuilder:validation:XValidation:rule=`has(self.hba) ? true : has(self.connection) && has(self.method)`,message=`"connection" and "method" are required` +// +// +structType=atomic +type PostgresHBARuleSpec struct { + // One line of the "pg_hba.conf" file. Changes to this value will be automatically reloaded without validation. + // --- + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=100 + // +kubebuilder:validation:Pattern=`^[[:print:]]+$` + // +kubebuilder:validation:XValidation:rule=`!self.trim().startsWith("include")`,message=`cannot include other files` + // +optional + HBA string `json:"hba,omitempty"` + + PostgresHBARule `json:",inline"` +} + // --- // PostgreSQL identifiers are limited in length but may contain any character. // - https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go index 9f661b0640..2a9f982caf 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go @@ -21,6 +21,9 @@ type PostgresClusterSpec struct { // +optional DataSource *DataSource `json:"dataSource,omitempty"` + // +optional + Authentication *PostgresAuthenticationSpec `json:"authentication,omitempty"` + // PostgreSQL backup configuration // +optional Backups Backups `json:"backups,omitempty"` diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go index 875d1ce000..677a1a1fe9 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go @@ -1754,6 +1754,28 @@ func (in *PatroniSwitchover) DeepCopy() *PatroniSwitchover { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresAuthenticationSpec) DeepCopyInto(out *PostgresAuthenticationSpec) { + *out = *in + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]PostgresHBARuleSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresAuthenticationSpec. +func (in *PostgresAuthenticationSpec) DeepCopy() *PostgresAuthenticationSpec { + if in == nil { + return nil + } + out := new(PostgresAuthenticationSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresCluster) DeepCopyInto(out *PostgresCluster) { *out = *in @@ -1864,6 +1886,11 @@ func (in *PostgresClusterSpec) DeepCopyInto(out *PostgresClusterSpec) { *out = new(DataSource) (*in).DeepCopyInto(*out) } + if in.Authentication != nil { + in, out := &in.Authentication, &out.Authentication + *out = new(PostgresAuthenticationSpec) + (*in).DeepCopyInto(*out) + } in.Backups.DeepCopyInto(&out.Backups) if in.Config != nil { in, out := &in.Config, &out.Config @@ -2067,6 +2094,54 @@ func (in *PostgresConfig) DeepCopy() *PostgresConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresHBARule) DeepCopyInto(out *PostgresHBARule) { + *out = *in + if in.Databases != nil { + in, out := &in.Databases, &out.Databases + *out = make([]PostgresIdentifier, len(*in)) + copy(*out, *in) + } + if in.Options != nil { + in, out := &in.Options, &out.Options + *out = make(map[string]intstr.IntOrString, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Users != nil { + in, out := &in.Users, &out.Users + *out = make([]PostgresIdentifier, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresHBARule. +func (in *PostgresHBARule) DeepCopy() *PostgresHBARule { + if in == nil { + return nil + } + out := new(PostgresHBARule) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresHBARuleSpec) DeepCopyInto(out *PostgresHBARuleSpec) { + *out = *in + in.PostgresHBARule.DeepCopyInto(&out.PostgresHBARule) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresHBARuleSpec. +func (in *PostgresHBARuleSpec) DeepCopy() *PostgresHBARuleSpec { + if in == nil { + return nil + } + out := new(PostgresHBARuleSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresInstanceSetSpec) DeepCopyInto(out *PostgresInstanceSetSpec) { *out = *in