diff --git a/README.md b/README.md index 1dbda91..51a996d 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,10 @@ AboutBits PostgreSQL Operator is a Kubernetes operator that helps you manage Pos > **Note:** Kubernetes 1.29+ is required due to the use of CRD CEL validations (GA in 1.29, Beta in 1.25). +The admin role used by the operator does not need to be a superuser. +Managed services such as AWS RDS, Amazon Aurora, Google Cloud SQL, and Azure Database for PostgreSQL are supported. +See [Admin privileges](docs/cluster-connection.md#admin-privileges). + ## Architecture ``` diff --git a/docs/cluster-connection.md b/docs/cluster-connection.md index cbfc74a..2193072 100644 --- a/docs/cluster-connection.md +++ b/docs/cluster-connection.md @@ -35,7 +35,6 @@ Use this option when the credentials should be mounted as a file inside the oper |--------|----------|-----------------------------------------------------------------------------------------|----------| | `path` | `string` | The absolute path inside the operator Pod to the file containing the admin credentials. | Yes | - #### File format The file must contain JSON with the following fields: @@ -66,6 +65,21 @@ The value of `adminSecretFileRef.path` is the `mountPath` plus the name of the f See [Using a file reference](#using-a-file-reference-adminsecretfileref) in the examples for a complete setup with each volume source. +## Admin privileges + +The admin role does not need to be a superuser. This makes the operator usable with managed services such as AWS RDS, Amazon Aurora, Google Cloud SQL, or Azure Database for PostgreSQL, where no superuser is available. + +| Custom Resource | Required privilege of the admin role | +|---------------------------------------|-----------------------------------------------------------------| +| `ClusterConnection` | `LOGIN` | +| `Role` | `CREATEROLE` | +| `Database` | `CREATEDB` | +| `Schema`, `Grant`, `DefaultPrivilege` | Ownership of, or the matching privileges on, the target objects | + +The master user of the managed services above has `LOGIN`, `CREATEDB`, and `CREATEROLE`. See [Role](role.md#non-superuser-admins) for the limits that apply to a non-superuser admin. + +The operator reads role state from the public view `pg_roles`. It does not read `pg_authid` or `pg_shadow`, which these services deny. + ## Examples ### Using a Kubernetes Secret (`adminSecretRef`) diff --git a/docs/role.md b/docs/role.md index cdea41b..3e8d89d 100644 --- a/docs/role.md +++ b/docs/role.md @@ -4,13 +4,14 @@ The `Role` Custom Resource Definition (CRD) manages PostgreSQL roles (users). ## Spec -| Field | Type | Description | Required | Mutable | -|---------------------|---------------|-------------------------------------------------------------------------------------|----------|---------| -| `clusterRef` | `ResourceRef` | Reference to the `ClusterConnection` to use. | Yes | Yes | -| `name` | `string` | The name of the role to create in the database. | Yes | No | -| `comment` | `string` | A comment to add to the role. | No | Yes | -| `passwordSecretRef` | `ResourceRef` | Reference to a secret containing the password for the role to make it a LOGIN role. | No | Yes | -| `flags` | `RoleFlags` | Flags and attributes for the role. | No | Yes | +| Field | Type | Description | Required | Mutable | +|----------------------|---------------|-------------------------------------------------------------------------------------|----------|---------| +| `clusterRef` | `ResourceRef` | Reference to the `ClusterConnection` to use. | Yes | Yes | +| `name` | `string` | The name of the role to create in the database. | Yes | No | +| `comment` | `string` | A comment to add to the role. | No | Yes | +| `passwordSecretRef` | `ResourceRef` | Reference to a secret containing the password for the role to make it a LOGIN role. | No | Yes | +| `passwordEncryption` | `string` | How the password is sent to PostgreSQL: `scram-sha-256` (default) or `server`. | No | Yes | +| `flags` | `RoleFlags` | Flags and attributes for the role. | No | Yes | ### ResourceRef (`clusterRef` and `passwordSecretRef`) @@ -45,6 +46,55 @@ The operator uses the presence of the `passwordSecretRef` field to determine if - **Login Role (User)**: If `passwordSecretRef` is specified, the role is created with the `LOGIN` attribute. It uses the password from the referenced secret. - **No-Login Role (Group)**: If `passwordSecretRef` is omitted, the role is created with the `NOLOGIN` attribute. This is useful for creating roles that serve as groups for permissions. +### Password handling + +The operator does not read the password hash from `pg_authid`. +That catalog is readable by superusers only, and managed PostgreSQL services of cloud providers, such as AWS RDS, Google Cloud SQL, or Azure Database for PostgreSQL, revoke it from every role, including the master user. + +Instead, the operator stores a keyed fingerprint of the password it applied last in `status.passwordFingerprint`. +On each reconcile it compares the referenced Secret against that fingerprint. When they differ, the operator runs `ALTER ROLE ... PASSWORD`. + +The fingerprint is an `HMAC-SHA256`. Its key is random and private to the operator. +The operator generates the key once and stores it in a Secret named `postgresql-operator-password-fingerprint-key` in its own namespace. A reader of the `Role` status learns nothing about the password without that key. +The Secret name is set by the configuration property `postgresql-operator.password-fingerprint.secret-name`, for example through the environment variable `POSTGRESQL_OPERATOR_PASSWORD_FINGERPRINT_SECRET_NAME`. +The Helm chart grants `create` on Secrets through a `Role` and `RoleBinding` in the operator namespace only. The `ClusterRole` of the operator keeps read access to Secrets. + +**Consequences:** + +- The Secret is the source of truth. A password change made directly in PostgreSQL is not detected. +- If the key Secret is lost, the operator generates a new key and re-applies every `Role` password once. +- After the upgrade to the version that introduced the fingerprint, every existing `Role` gets one password update, because its status has no fingerprint yet. + +**Threat model:** + +The fingerprint is a keyed hash. It is not a password hash with a work factor, such as `bcrypt` or `PBKDF2`. + +- Without the key, the fingerprint reveals nothing about the password. A reader of the `Role` status alone cannot attack it. This includes a backup of etcd and a user with `get` on `Role` resources. +- With the key, an attacker can test password guesses offline at `HMAC-SHA256` speed. Treat the key Secret as a credential. Keep the number of principals with `get` on Secrets in the operator namespace small. +- An attacker who reads the key Secret can usually also read the password Secrets that the `Role` resources reference. In that case the fingerprint adds no exposure that the attacker does not already have. +- The operator never writes the password, its `SCRAM-SHA-256` verifier, or the key into the `Role` status. +- To retire a key, delete the key Secret. The operator generates a new key and re-applies every `Role` password once. Every old fingerprint then becomes meaningless. + +#### `passwordEncryption` + +| Value | Behavior | +|-----------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `scram-sha-256` | **Default**. The operator computes the `SCRAM-SHA-256` verifier itself and sends only the verifier. The cleartext password never reaches the server, its statement log, or extensions such as `pgaudit`. | +| `server` | The operator sends the cleartext password. The server hashes it according to its `password_encryption` setting. Use this for clients that only support MD5 authentication. | + +If the Secret already contains an `MD5` or `SCRAM-SHA-256` verifier, the operator forwards it unchanged in both modes. + +**Note:** +A pre-hashed password bypasses server-side password policies. The `credcheck` extension rejects it unless `credcheck.encrypted_password_allowed` is on. For example a Cloud SQL password policy does not apply to hashed passwords. Set `passwordEncryption: server` when such a policy must apply. + +### Non-superuser admins + +The admin role of the `ClusterConnection` does not need to be a superuser. `CREATEROLE` is sufficient for `Role` resources. +See [ClusterConnection](cluster-connection.md#admin-privileges) for the full list of privileges. The following limits apply when the admin is not a superuser: + +- The flags `superuser`, `replication`, and `bypassrls` cannot be set. PostgreSQL rejects them, and the `Role` status shows the error. +- On PostgreSQL 16 and later, the admin can only alter roles on which it holds `ADMIN OPTION`. Roles created by the operator qualify. Roles created by another user do not, unless that user grants the admin `ADMIN OPTION`. + ### Example ```yaml diff --git a/generated/build.gradle.kts b/generated/build.gradle.kts index 9642969..3af6ca2 100644 --- a/generated/build.gradle.kts +++ b/generated/build.gradle.kts @@ -39,6 +39,7 @@ jooq { | pg_default_acl | pg_get_userbyid | pg_namespace + | pg_roles | shobj_description """.trimIndent() excludes = """ diff --git a/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/PgCatalog.java b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/PgCatalog.java index c19c97a..f2fce16 100644 --- a/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/PgCatalog.java +++ b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/PgCatalog.java @@ -12,6 +12,7 @@ import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgDbRoleSetting; import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgDefaultAcl; import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgNamespace; +import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgRoles; import it.aboutbits.postgresql.core.infrastructure.persistence.tables.records.AclexplodeRecord; import java.util.Arrays; @@ -122,6 +123,11 @@ public static Aclexplode ACLEXPLODE( */ public final PgNamespace PG_NAMESPACE = PgNamespace.PG_NAMESPACE; + /** + * The table pg_catalog.pg_roles. + */ + public final PgRoles PG_ROLES = PgRoles.PG_ROLES; + /** * No further instances allowed */ @@ -145,7 +151,8 @@ public final List> getTables() { PgDatabase.PG_DATABASE, PgDbRoleSetting.PG_DB_ROLE_SETTING, PgDefaultAcl.PG_DEFAULT_ACL, - PgNamespace.PG_NAMESPACE + PgNamespace.PG_NAMESPACE, + PgRoles.PG_ROLES ); } } diff --git a/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/Tables.java b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/Tables.java index abf6067..cef6128 100644 --- a/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/Tables.java +++ b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/Tables.java @@ -12,6 +12,7 @@ import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgDbRoleSetting; import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgDefaultAcl; import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgNamespace; +import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgRoles; import it.aboutbits.postgresql.core.infrastructure.persistence.tables.records.AclexplodeRecord; import javax.annotation.processing.Generated; @@ -107,4 +108,9 @@ public static Aclexplode ACLEXPLODE( * The table pg_catalog.pg_namespace. */ public static final PgNamespace PG_NAMESPACE = PgNamespace.PG_NAMESPACE; + + /** + * The table pg_catalog.pg_roles. + */ + public static final PgRoles PG_ROLES = PgRoles.PG_ROLES; } diff --git a/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/tables/PgRoles.java b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/tables/PgRoles.java new file mode 100644 index 0000000..954b235 --- /dev/null +++ b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/tables/PgRoles.java @@ -0,0 +1,298 @@ +/* + * This file is generated by jOOQ. + */ +package it.aboutbits.postgresql.core.infrastructure.persistence.tables; + + +import it.aboutbits.postgresql.core.infrastructure.persistence.PgCatalog; +import it.aboutbits.postgresql.core.infrastructure.persistence.tables.records.PgRolesRecord; + +import java.time.OffsetDateTime; +import java.util.Collection; + +import javax.annotation.processing.Generated; + +import org.jooq.Condition; +import org.jooq.Field; +import org.jooq.Name; +import org.jooq.PlainSQL; +import org.jooq.QueryPart; +import org.jooq.SQL; +import org.jooq.Schema; +import org.jooq.Stringly; +import org.jooq.Table; +import org.jooq.TableField; +import org.jooq.TableLike; +import org.jooq.TableOptions; +import org.jooq.impl.DSL; +import org.jooq.impl.Internal; +import org.jooq.impl.SQLDataType; +import org.jooq.impl.TableImpl; + + +/** + * This class is generated by jOOQ. + */ +@Generated( + value = { + "https://www.jooq.org", + "jOOQ version:3.21.4" + }, + comments = "This class is generated by jOOQ" +) +@SuppressWarnings({ "all", "unchecked", "rawtypes", "this-escape" }) +public class PgRoles extends TableImpl { + + private static final long serialVersionUID = 1L; + + /** + * The reference instance of pg_catalog.pg_roles + */ + public static final PgRoles PG_ROLES = new PgRoles(); + + /** + * The class holding records for this type + */ + @Override + public Class getRecordType() { + return PgRolesRecord.class; + } + + /** + * The column pg_catalog.pg_roles.rolname. + */ + public final TableField ROLNAME = createField(DSL.name("rolname"), SQLDataType.VARCHAR, this, ""); + + /** + * The column pg_catalog.pg_roles.rolsuper. + */ + public final TableField ROLSUPER = createField(DSL.name("rolsuper"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolinherit. + */ + public final TableField ROLINHERIT = createField(DSL.name("rolinherit"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolcreaterole. + */ + public final TableField ROLCREATEROLE = createField(DSL.name("rolcreaterole"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolcreatedb. + */ + public final TableField ROLCREATEDB = createField(DSL.name("rolcreatedb"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolcanlogin. + */ + public final TableField ROLCANLOGIN = createField(DSL.name("rolcanlogin"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolreplication. + */ + public final TableField ROLREPLICATION = createField(DSL.name("rolreplication"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolconnlimit. + */ + public final TableField ROLCONNLIMIT = createField(DSL.name("rolconnlimit"), SQLDataType.INTEGER, this, ""); + + /** + * The column pg_catalog.pg_roles.rolpassword. + */ + public final TableField ROLPASSWORD = createField(DSL.name("rolpassword"), SQLDataType.CLOB, this, ""); + + /** + * The column pg_catalog.pg_roles.rolvaliduntil. + */ + public final TableField ROLVALIDUNTIL = createField(DSL.name("rolvaliduntil"), SQLDataType.TIMESTAMPWITHTIMEZONE(6), this, ""); + + /** + * The column pg_catalog.pg_roles.rolbypassrls. + */ + public final TableField ROLBYPASSRLS = createField(DSL.name("rolbypassrls"), SQLDataType.BOOLEAN, this, ""); + + /** + * The column pg_catalog.pg_roles.rolconfig. + */ + public final TableField ROLCONFIG = createField(DSL.name("rolconfig"), SQLDataType.CLOB.array(), this, ""); + + /** + * The column pg_catalog.pg_roles.oid. + */ + public final TableField OID = createField(DSL.name("oid"), SQLDataType.BIGINT, this, ""); + + private PgRoles(Name alias, Table aliased) { + this(alias, aliased, (Field[]) null, null); + } + + private PgRoles(Name alias, Table aliased, Field[] parameters, Condition where) { + super(alias, null, aliased, parameters, DSL.comment(""), TableOptions.view(""" + CREATE VIEW "pg_roles" AS SELECT pg_authid.rolname, + pg_authid.rolsuper, + pg_authid.rolinherit, + pg_authid.rolcreaterole, + pg_authid.rolcreatedb, + pg_authid.rolcanlogin, + pg_authid.rolreplication, + pg_authid.rolconnlimit, + '********'::text AS rolpassword, + pg_authid.rolvaliduntil, + pg_authid.rolbypassrls, + s.setconfig AS rolconfig, + pg_authid.oid + FROM (pg_authid + LEFT JOIN pg_db_role_setting s ON (((pg_authid.oid = s.setrole) AND (s.setdatabase = (0)::oid)))); + """), where); + } + + /** + * Create an aliased pg_catalog.pg_roles table reference + */ + public PgRoles(String alias) { + this(DSL.name(alias), PG_ROLES); + } + + /** + * Create an aliased pg_catalog.pg_roles table reference + */ + public PgRoles(Name alias) { + this(alias, PG_ROLES); + } + + /** + * Create a pg_catalog.pg_roles table reference + */ + public PgRoles() { + this(DSL.name("pg_roles"), null); + } + + @Override + public Schema getSchema() { + return aliased() ? null : PgCatalog.PG_CATALOG; + } + + @Override + public PgRoles as(String alias) { + return new PgRoles(DSL.name(alias), this); + } + + @Override + public PgRoles as(Name alias) { + return new PgRoles(alias, this); + } + + @Override + public PgRoles as(Table alias) { + return new PgRoles(alias.getQualifiedName(), this); + } + + /** + * Rename this table + */ + @Override + public PgRoles rename(String name) { + return new PgRoles(DSL.name(name), null); + } + + /** + * Rename this table + */ + @Override + public PgRoles rename(Name name) { + return new PgRoles(name, null); + } + + /** + * Rename this table + */ + @Override + public PgRoles rename(Table name) { + return new PgRoles(name.getQualifiedName(), null); + } + + /** + * Create an inline derived table from this table + */ + @Override + public PgRoles where(Condition condition) { + return new PgRoles(getQualifiedName(), aliased() ? this : null, null, Internal.condition(this, condition)); + } + + /** + * Create an inline derived table from this table + */ + @Override + public PgRoles where(Collection conditions) { + return where(DSL.and(conditions)); + } + + /** + * Create an inline derived table from this table + */ + @Override + public PgRoles where(Condition... conditions) { + return where(DSL.and(conditions)); + } + + /** + * Create an inline derived table from this table + */ + @Override + public PgRoles where(Field condition) { + return where(DSL.condition(condition)); + } + + /** + * Create an inline derived table from this table + */ + @Override + @PlainSQL + public PgRoles where(SQL condition) { + return where(DSL.condition(condition)); + } + + /** + * Create an inline derived table from this table + */ + @Override + @PlainSQL + public PgRoles where(@Stringly.SQL String condition) { + return where(DSL.condition(condition)); + } + + /** + * Create an inline derived table from this table + */ + @Override + @PlainSQL + public PgRoles where(@Stringly.SQL String condition, Object... binds) { + return where(DSL.condition(condition, binds)); + } + + /** + * Create an inline derived table from this table + */ + @Override + @PlainSQL + public PgRoles where(@Stringly.SQL String condition, QueryPart... parts) { + return where(DSL.condition(condition, parts)); + } + + /** + * Create an inline derived table from this table + */ + @Override + public PgRoles whereExists(TableLike select) { + return where(DSL.exists(select)); + } + + /** + * Create an inline derived table from this table + */ + @Override + public PgRoles whereNotExists(TableLike select) { + return where(DSL.notExists(select)); + } +} diff --git a/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/tables/records/PgRolesRecord.java b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/tables/records/PgRolesRecord.java new file mode 100644 index 0000000..9c7443a --- /dev/null +++ b/generated/src/main/java/it/aboutbits/postgresql/core/infrastructure/persistence/tables/records/PgRolesRecord.java @@ -0,0 +1,258 @@ +/* + * This file is generated by jOOQ. + */ +package it.aboutbits.postgresql.core.infrastructure.persistence.tables.records; + + +import it.aboutbits.postgresql.core.infrastructure.persistence.tables.PgRoles; + +import java.time.OffsetDateTime; + +import javax.annotation.processing.Generated; + +import org.jooq.impl.TableRecordImpl; + + +/** + * This class is generated by jOOQ. + */ +@Generated( + value = { + "https://www.jooq.org", + "jOOQ version:3.21.4" + }, + comments = "This class is generated by jOOQ" +) +@SuppressWarnings({ "all", "unchecked", "rawtypes", "this-escape" }) +public class PgRolesRecord extends TableRecordImpl { + + private static final long serialVersionUID = 1L; + + /** + * Setter for pg_catalog.pg_roles.rolname. + */ + public PgRolesRecord setRolname(String value) { + set(0, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolname. + */ + public String getRolname() { + return (String) get(0); + } + + /** + * Setter for pg_catalog.pg_roles.rolsuper. + */ + public PgRolesRecord setRolsuper(Boolean value) { + set(1, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolsuper. + */ + public Boolean getRolsuper() { + return (Boolean) get(1); + } + + /** + * Setter for pg_catalog.pg_roles.rolinherit. + */ + public PgRolesRecord setRolinherit(Boolean value) { + set(2, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolinherit. + */ + public Boolean getRolinherit() { + return (Boolean) get(2); + } + + /** + * Setter for pg_catalog.pg_roles.rolcreaterole. + */ + public PgRolesRecord setRolcreaterole(Boolean value) { + set(3, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolcreaterole. + */ + public Boolean getRolcreaterole() { + return (Boolean) get(3); + } + + /** + * Setter for pg_catalog.pg_roles.rolcreatedb. + */ + public PgRolesRecord setRolcreatedb(Boolean value) { + set(4, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolcreatedb. + */ + public Boolean getRolcreatedb() { + return (Boolean) get(4); + } + + /** + * Setter for pg_catalog.pg_roles.rolcanlogin. + */ + public PgRolesRecord setRolcanlogin(Boolean value) { + set(5, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolcanlogin. + */ + public Boolean getRolcanlogin() { + return (Boolean) get(5); + } + + /** + * Setter for pg_catalog.pg_roles.rolreplication. + */ + public PgRolesRecord setRolreplication(Boolean value) { + set(6, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolreplication. + */ + public Boolean getRolreplication() { + return (Boolean) get(6); + } + + /** + * Setter for pg_catalog.pg_roles.rolconnlimit. + */ + public PgRolesRecord setRolconnlimit(Integer value) { + set(7, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolconnlimit. + */ + public Integer getRolconnlimit() { + return (Integer) get(7); + } + + /** + * Setter for pg_catalog.pg_roles.rolpassword. + */ + public PgRolesRecord setRolpassword(String value) { + set(8, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolpassword. + */ + public String getRolpassword() { + return (String) get(8); + } + + /** + * Setter for pg_catalog.pg_roles.rolvaliduntil. + */ + public PgRolesRecord setRolvaliduntil(OffsetDateTime value) { + set(9, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolvaliduntil. + */ + public OffsetDateTime getRolvaliduntil() { + return (OffsetDateTime) get(9); + } + + /** + * Setter for pg_catalog.pg_roles.rolbypassrls. + */ + public PgRolesRecord setRolbypassrls(Boolean value) { + set(10, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolbypassrls. + */ + public Boolean getRolbypassrls() { + return (Boolean) get(10); + } + + /** + * Setter for pg_catalog.pg_roles.rolconfig. + */ + public PgRolesRecord setRolconfig(String[] value) { + set(11, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.rolconfig. + */ + public String[] getRolconfig() { + return (String[]) get(11); + } + + /** + * Setter for pg_catalog.pg_roles.oid. + */ + public PgRolesRecord setOid(Long value) { + set(12, value); + return this; + } + + /** + * Getter for pg_catalog.pg_roles.oid. + */ + public Long getOid() { + return (Long) get(12); + } + + // ------------------------------------------------------------------------- + // Constructors + // ------------------------------------------------------------------------- + + /** + * Create a detached PgRolesRecord + */ + public PgRolesRecord() { + super(PgRoles.PG_ROLES); + } + + /** + * Create a detached, initialised PgRolesRecord + */ + public PgRolesRecord(String rolname, Boolean rolsuper, Boolean rolinherit, Boolean rolcreaterole, Boolean rolcreatedb, Boolean rolcanlogin, Boolean rolreplication, Integer rolconnlimit, String rolpassword, OffsetDateTime rolvaliduntil, Boolean rolbypassrls, String[] rolconfig, Long oid) { + super(PgRoles.PG_ROLES); + + setRolname(rolname); + setRolsuper(rolsuper); + setRolinherit(rolinherit); + setRolcreaterole(rolcreaterole); + setRolcreatedb(rolcreatedb); + setRolcanlogin(rolcanlogin); + setRolreplication(rolreplication); + setRolconnlimit(rolconnlimit); + setRolpassword(rolpassword); + setRolvaliduntil(rolvaliduntil); + setRolbypassrls(rolbypassrls); + setRolconfig(rolconfig); + setOid(oid); + resetTouchedOnNotNull(); + } +} diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/PasswordFingerprintService.java b/operator/src/main/java/it/aboutbits/postgresql/core/PasswordFingerprintService.java new file mode 100644 index 0000000..dc72b3b --- /dev/null +++ b/operator/src/main/java/it/aboutbits/postgresql/core/PasswordFingerprintService.java @@ -0,0 +1,152 @@ +package it.aboutbits.postgresql.core; + +import io.fabric8.kubernetes.api.model.SecretBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; +import it.aboutbits.postgresql.crd.role.PasswordEncryption; +import jakarta.inject.Singleton; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import java.net.HttpURLConnection; +import java.nio.charset.StandardCharsets; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.util.Base64; + +/// Computes keyed fingerprints (HMAC-SHA256) of `Role` passwords. +/// +/// The operator stores the fingerprint of the password it applied last in the `Role` status. +/// On the next reconcile it compares the referenced Secret against that fingerprint. +/// This replaces a read of the password hash from `pg_authid`, which needs superuser rights. +/// Managed PostgreSQL services of cloud providers, such as AWS RDS, Google Cloud SQL, or Azure Database for PostgreSQL, +/// do not grant these rights. +/// +/// The HMAC key is random, generated once, and kept in a Secret in the operator namespace. +/// Without the key, the fingerprint in the status is useless for an attack on the password. +/// If the key Secret is lost, the operator generates a new key and re-applies every `Role` password once. +@Slf4j +@Singleton +@RequiredArgsConstructor +@NullMarked +public class PasswordFingerprintService { + public static final String SECRET_DATA_KEY = "key"; + + private static final String HMAC_SHA_256 = "HmacSHA256"; + private static final int KEY_LENGTH_BYTES = 32; + private static final byte SEPARATOR = 0; + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + private final KubernetesClient kubernetesClient; + + @SuppressWarnings("NullAway.Init") + @ConfigProperty(name = "postgresql-operator.password-fingerprint.secret-name") + String secretName; + + private byte @Nullable [] key; + + /// Fingerprint of the password together with the requested encryption, + /// so that a change of either re-applies the password. + public String fingerprint( + String password, + PasswordEncryption passwordEncryption + ) { + var encryption = passwordEncryption.toValue().getBytes(StandardCharsets.UTF_8); + var secret = password.getBytes(StandardCharsets.UTF_8); + + // Load the key outside the try block, so that a Kubernetes error keeps its own message + var key = getKey(); + + try { + var mac = Mac.getInstance(HMAC_SHA_256); + mac.init(new SecretKeySpec(key, HMAC_SHA_256)); + mac.update(encryption); + mac.update(SEPARATOR); + mac.update(secret); + + return Base64.getEncoder().encodeToString(mac.doFinal()); + } catch (NoSuchAlgorithmException | InvalidKeyException e) { + throw new IllegalStateException("%s not available".formatted(HMAC_SHA_256), e); + } + } + + private synchronized byte[] getKey() { + var current = key; + if (current == null) { + current = loadOrCreateKey(); + key = current; + } + + return current; + } + + private byte[] loadOrCreateKey() { + var namespace = kubernetesClient.getNamespace(); + if (namespace == null) { + throw new IllegalStateException( + "Cannot determine the operator namespace to store the password fingerprint key Secret [secret.name=%s]".formatted(secretName) + ); + } + + var secrets = kubernetesClient.secrets() + .inNamespace(namespace) + .withName(secretName); + + var secret = secrets.get(); + if (secret == null) { + var generatedKey = new byte[KEY_LENGTH_BYTES]; + SECURE_RANDOM.nextBytes(generatedKey); + + var newSecret = new SecretBuilder() + .withNewMetadata() + .withNamespace(namespace) + .withName(secretName) + .endMetadata() + .withType("Opaque") + .addToData(SECRET_DATA_KEY, Base64.getEncoder().encodeToString(generatedKey)) + .build(); + + try { + secret = kubernetesClient.secrets() + .inNamespace(namespace) + .resource(newSecret) + .create(); + + log.info( + "Created password fingerprint key Secret [secret.namespace={}, secret.name={}]", + namespace, + secretName + ); + } catch (KubernetesClientException e) { + if (e.getCode() != HttpURLConnection.HTTP_CONFLICT) { + throw e; + } + + // Another operator replica created the Secret in the meantime + secret = secrets.require(); + } + } + + var data = secret.getData(); + var keyBase64 = data == null + ? null + : data.get(SECRET_DATA_KEY); + if (keyBase64 == null || keyBase64.isBlank()) { + throw new IllegalStateException( + "The password fingerprint key Secret is missing required data '%s' [secret.namespace=%s, secret.name=%s]".formatted( + SECRET_DATA_KEY, + namespace, + secretName + ) + ); + } + + return Base64.getDecoder().decode(keyBase64); + } +} diff --git a/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationService.java b/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationService.java index 967ff2d..77ab18a 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationService.java +++ b/operator/src/main/java/it/aboutbits/postgresql/core/PostgreSQLAuthenticationService.java @@ -1,10 +1,8 @@ package it.aboutbits.postgresql.core; import com.ongres.scram.common.StringPreparation; -import it.aboutbits.postgresql.crd.role.RoleSpec; +import it.aboutbits.postgresql.crd.role.PasswordEncryption; import jakarta.inject.Singleton; -import lombok.extern.slf4j.Slf4j; -import org.jooq.DSLContext; import org.jspecify.annotations.NullMarked; import javax.crypto.Mac; @@ -13,175 +11,126 @@ import javax.crypto.spec.SecretKeySpec; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; import java.util.Arrays; import java.util.Base64; import java.util.HexFormat; import java.util.Locale; -import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_AUTHID; - -@Slf4j +/// Builds the password literal that the operator sends to PostgreSQL in `CREATE ROLE` and `ALTER ROLE`. +/// +/// By default, the operator computes the SCRAM-SHA-256 verifier itself, +/// exactly like `psql \password` or libpq's `PQencryptPasswordConn`. +/// PostgreSQL stores a verifier as is, regardless of its `password_encryption` setting. +/// This keeps the cleartext password out of the server's statement log and out of extensions such as `pg_stat_statements` or `pgaudit`. @Singleton @NullMarked public final class PostgreSQLAuthenticationService { + public static final String SCRAM_SHA_256_PREFIX = "SCRAM-SHA-256$"; + + /// PostgreSQL's default for `scram_iterations`. + public static final int SCRAM_SHA_256_ITERATIONS = 4096; + private static final String MD5 = "MD5"; + private static final int MD5_VERIFIER_LENGTH = 3 + 32; private static final String SHA_256 = "SHA-256"; private static final String HMAC_SHA_256 = "HmacSHA256"; private static final String PBKDF2_WITH_HMAC_SHA256 = "PBKDF2WithHmacSHA256"; - - public boolean passwordMatches( - DSLContext dsl, - RoleSpec spec, - String expectedPassword + private static final int SCRAM_SALT_LENGTH_BYTES = 16; + private static final int SCRAM_KEY_LENGTH_BYTES = 32; + + private final SecureRandom secureRandom = new SecureRandom(); + + /// Convert the password from the Secret into the literal to send to PostgreSQL. + /// + /// A value that already is an MD5 or SCRAM-SHA-256 verifier is forwarded unchanged, so users keep + /// full control over the stored hash. + public String toServerPassword( + String password, + PasswordEncryption passwordEncryption ) { - var currentPasswordVerifier = dsl - .select(PG_AUTHID.ROLPASSWORD) - .from(PG_AUTHID) - .where(PG_AUTHID.ROLNAME.eq(spec.getName())) - .fetchSingle(PG_AUTHID.ROLPASSWORD); - - if (currentPasswordVerifier == null || currentPasswordVerifier.isBlank()) { - return false; + if (passwordEncryption == PasswordEncryption.SERVER || isEncrypted(password)) { + return password; } - // PostgreSQL stores either: - // - SCRAM verifier: SCRAM-SHA-256$:$: - // - or legacy md5: md5 - if (currentPasswordVerifier.startsWith("SCRAM-SHA-256$")) { - return verifyPostgresScramSha256( - currentPasswordVerifier, - expectedPassword - ); - } - - if (currentPasswordVerifier.startsWith(MD5.toLowerCase(Locale.ROOT))) { - return verifyPostgresMd5( - currentPasswordVerifier, - expectedPassword, - spec.getName() - ); - } + return scramSha256Verifier(password); + } - // Unknown format (or plain text, which PG should not store in rolpassword) - return false; + /// Whether the given password is already an MD5 or SCRAM-SHA-256 verifier. + public static boolean isEncrypted(String password) { + return password.startsWith(SCRAM_SHA_256_PREFIX) || isMd5Verifier(password); } - private static boolean verifyPostgresScramSha256(String postgresVerifier, String cleartextPassword) { - // Prepare the cleartext password with SASLprep + /// Compute a PostgreSQL SCRAM-SHA-256 verifier for the given cleartext password. + /// + /// Format: `SCRAM-SHA-256$:$:` + /// as described in RFC 5802 and RFC 7677 and used by PostgreSQL since version 10. + public String scramSha256Verifier(String cleartextPassword) { + // Prepare the cleartext password with SASLprep, as PostgreSQL does var preparedPassword = StringPreparation.POSTGRESQL_PREPARATION.normalize( cleartextPassword.toCharArray() ); - // Format: SCRAM-SHA-256$:$: - var afterPrefix = postgresVerifier.substring("SCRAM-SHA-256$".length()); - var dollar = afterPrefix.indexOf('$'); - if (dollar < 0) { - return false; - } - - // : - var iterationsAndSalt = afterPrefix.substring(0, dollar); - // : - var keys = afterPrefix.substring(dollar + 1); - - var colonIterationsAndSalt = iterationsAndSalt.indexOf(':'); - if (colonIterationsAndSalt < 0) { - return false; - } - - int iterations; - try { - iterations = Integer.parseInt(iterationsAndSalt.substring(0, colonIterationsAndSalt)); - } catch (NumberFormatException e) { - log.error("Invalid iterations format in PostgreSQL verifier: %s".formatted(postgresVerifier), e); - return false; - } - if (iterations <= 0) { - return false; - } - - var saltB64 = iterationsAndSalt.substring(colonIterationsAndSalt + 1); - - var colonKeys = keys.indexOf(':'); - if (colonKeys < 0) { - return false; - } - - var storedKeyB64 = keys.substring(0, colonKeys); - - byte[] salt; - byte[] currentStoredKey; - try { - salt = Base64.getDecoder().decode(saltB64); - currentStoredKey = Base64.getDecoder().decode(storedKeyB64); - } catch (IllegalArgumentException e) { - log.error("Invalid salt or stored key format in PostgreSQL verifier: %s".formatted(postgresVerifier), e); - return false; - } + var salt = new byte[SCRAM_SALT_LENGTH_BYTES]; + secureRandom.nextBytes(salt); byte[] saltedPassword = null; byte[] clientKey = null; - byte[] expectedStoredKey = null; + byte[] storedKey = null; + byte[] serverKey = null; try { // RFC 5802/7677: // saltedPassword := Hi(password, salt, iterations) (PBKDF2-HMAC-SHA-256, 32 bytes) // clientKey := HMAC(saltedPassword, "Client Key") // storedKey := H(clientKey) (SHA-256) - saltedPassword = pbkdf2HmacSha256(preparedPassword, salt, iterations, 32); + // serverKey := HMAC(saltedPassword, "Server Key") + saltedPassword = pbkdf2HmacSha256(preparedPassword, salt, SCRAM_SHA_256_ITERATIONS, SCRAM_KEY_LENGTH_BYTES); clientKey = hmacSha256(saltedPassword, "Client Key".getBytes(StandardCharsets.UTF_8)); - expectedStoredKey = sha256(clientKey); + storedKey = sha256(clientKey); + serverKey = hmacSha256(saltedPassword, "Server Key".getBytes(StandardCharsets.UTF_8)); + + var encoder = Base64.getEncoder(); - return MessageDigest.isEqual( - currentStoredKey, - expectedStoredKey + return "%s%d:%s$%s:%s".formatted( + SCRAM_SHA_256_PREFIX, + SCRAM_SHA_256_ITERATIONS, + encoder.encodeToString(salt), + encoder.encodeToString(storedKey), + encoder.encodeToString(serverKey) ); } finally { + Arrays.fill(preparedPassword, '\0'); if (saltedPassword != null) { Arrays.fill(saltedPassword, (byte) 0); } if (clientKey != null) { Arrays.fill(clientKey, (byte) 0); } - if (expectedStoredKey != null) { - Arrays.fill(expectedStoredKey, (byte) 0); + if (storedKey != null) { + Arrays.fill(storedKey, (byte) 0); + } + if (serverKey != null) { + Arrays.fill(serverKey, (byte) 0); } } } - private static boolean verifyPostgresMd5( - String postgresMd5, - String expectedPassword, - String username - ) { - // PostgreSQL md5 is: "md5" + md5(password + username) - if (postgresMd5.length() != 3 + 32 || !postgresMd5.regionMatches(true, 0, MD5, 0, 3)) { + private static boolean isMd5Verifier(String password) { + // PostgreSQL md5 is: "md5" + md5(password + username) as 32 hex characters + if (password.length() != MD5_VERIFIER_LENGTH || !password.regionMatches(true, 0, MD5, 0, 3)) { return false; } - byte[] currentDigest; try { - currentDigest = HexFormat.of().parseHex( - postgresMd5, + HexFormat.of().parseHex( + password.toLowerCase(Locale.ROOT), 3, - postgresMd5.length() + password.length() ); - } catch (IllegalArgumentException e) { - log.error("Invalid MD5 format in PostgreSQL verifier: %s".formatted(postgresMd5), e); - return false; // not valid hex - } - - MessageDigest md5; - try { - md5 = MessageDigest.getInstance(MD5); - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("%s not available".formatted(MD5), e); + return true; + } catch (IllegalArgumentException _) { + return false; } - - md5.update((expectedPassword + username).getBytes(StandardCharsets.UTF_8)); - var expectedDigest = md5.digest(); - - return MessageDigest.isEqual(currentDigest, expectedDigest); } private static byte[] pbkdf2HmacSha256( diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/role/PasswordEncryption.java b/operator/src/main/java/it/aboutbits/postgresql/crd/role/PasswordEncryption.java new file mode 100644 index 0000000..99cf0e1 --- /dev/null +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/role/PasswordEncryption.java @@ -0,0 +1,26 @@ +package it.aboutbits.postgresql.crd.role; + +import com.fasterxml.jackson.annotation.JsonValue; +import lombok.RequiredArgsConstructor; +import org.jspecify.annotations.NullMarked; + +/// Controls how the operator sends the password of a `Role` to PostgreSQL. +@RequiredArgsConstructor +@NullMarked +public enum PasswordEncryption { + /// The operator computes a SCRAM-SHA-256 verifier and sends only the verifier. + /// The cleartext password never reaches the server or its statement log. + SCRAM_SHA_256("scram-sha-256"), + + /// The operator sends the cleartext password. + /// The server hashes it according to its `password_encryption` setting. + /// Use this for clients that only support MD5 authentication. + SERVER("server"); + + private final String value; + + @JsonValue + public String toValue() { + return value; + } +} diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/role/Role.java b/operator/src/main/java/it/aboutbits/postgresql/crd/role/Role.java index 121d06d..6b048a1 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/role/Role.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/role/Role.java @@ -6,7 +6,6 @@ import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.Version; -import it.aboutbits.postgresql.core.CRStatus; import it.aboutbits.postgresql.core.Named; import org.jspecify.annotations.NullMarked; @@ -39,7 +38,7 @@ ) @NullMarked public class Role - extends CustomResource + extends CustomResource implements Namespaced, Named { @Override @JsonIgnore diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleReconciler.java b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleReconciler.java index 8e98b77..0845f84 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleReconciler.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleReconciler.java @@ -17,8 +17,8 @@ import io.quarkiverse.operatorsdk.annotations.RBACRule; import it.aboutbits.postgresql.core.BaseReconciler; import it.aboutbits.postgresql.core.CRPhase; -import it.aboutbits.postgresql.core.CRStatus; import it.aboutbits.postgresql.core.KubernetesService; +import it.aboutbits.postgresql.core.PasswordFingerprintService; import it.aboutbits.postgresql.core.PostgreSQLAuthenticationService; import it.aboutbits.postgresql.core.PostgreSQLContextFactory; import lombok.RequiredArgsConstructor; @@ -37,17 +37,20 @@ @RBACRule( apiGroups = {""}, resources = {"secrets"}, + // `create` for the password fingerprint key Secret is granted by a namespaced Role in the operator namespace, + // see `quarkus.kubernetes.rbac` in application.yml. It does not belong in this ClusterRole. verbs = {"get", "list", "watch"} ) }) @RequiredArgsConstructor @NullMarked public class RoleReconciler - extends BaseReconciler + extends BaseReconciler implements Reconciler, Cleaner { private final RoleService roleService; private final KubernetesService kubernetesService; private final PostgreSQLAuthenticationService postgreSQLAuthenticationService; + private final PasswordFingerprintService passwordFingerprintService; private final KubernetesClient kubernetesClient; private final PostgreSQLContextFactory contextFactory; @@ -112,15 +115,32 @@ public UpdateControl reconcile( UpdateControl updateControl; try (var dsl = contextFactory.getDSLContext(clusterConnection)) { + var passwordEncryption = spec.getPasswordEncryption(); + + // The password hash in pg_authid is readable by superusers only. + // Managed PostgreSQL services of cloud providers do not grant that, + // so the operator tracks a keyed fingerprint of the applied password in the status. + var expectedFingerprint = password != null + ? passwordFingerprintService.fingerprint(password, passwordEncryption) + : null; + var serverPassword = password != null + ? postgreSQLAuthenticationService.toServerPassword(password, passwordEncryption) + : null; + // Run everything in a single transaction updateControl = dsl.transactionResult( cfg -> reconcileInTransaction( cfg.dsl(), resource, status, - password + expectedFingerprint, + serverPassword ) ); + + // Record the fingerprint only after the commit. A failed commit must not leave + // the fingerprint of a password that PostgreSQL never stored. + status.setPasswordFingerprint(expectedFingerprint); } catch (Exception e) { return handleError( resource, @@ -239,15 +259,19 @@ public List> prepareEventSources(EventSourceContext c } @Override - protected CRStatus newStatus() { - return new CRStatus(); + protected RoleStatus newStatus() { + return new RoleStatus(); } + /// @param expectedFingerprint the fingerprint of the Secret password, or `null` for a `NOLOGIN` role; + /// compared against the fingerprint in the status, and never written here + /// @param serverPassword the password literal to send to PostgreSQL, or `null` for a `NOLOGIN` role private UpdateControl reconcileInTransaction( DSLContext tx, Role resource, - CRStatus status, - @Nullable String password + RoleStatus status, + @Nullable String expectedFingerprint, + @Nullable String serverPassword ) { var namespace = resource.getMetadata().getNamespace(); var name = resource.getMetadata().getName(); @@ -255,6 +279,8 @@ private UpdateControl reconcileInTransaction( var spec = resource.getSpec(); var expectedFlags = spec.getFlags(); + var loginExpected = serverPassword != null; + // Create and return the role if it doesn't exist yet if (!roleService.roleExists(tx, spec)) { log.info( @@ -266,7 +292,7 @@ private UpdateControl reconcileInTransaction( roleService.createRole( tx, spec, - password + serverPassword ); status.setPhase(CRPhase.READY) @@ -275,23 +301,12 @@ private UpdateControl reconcileInTransaction( return UpdateControl.patchStatus(resource); } - // When there is NOLOGIN, we set no password - var passwordMatches = true; - var roleLoginMatches = roleService.roleLoginMatches(tx, spec); + var currentCanLogin = roleService.roleCanLogin(tx, spec); + var roleLoginMatches = loginExpected == currentCanLogin; var currentFlags = roleService.fetchCurrentFlags(tx, spec); var flagsMatch = expectedFlags.equals(currentFlags); var commentMatches = roleService.roleCommentMatches(tx, spec); - - var passwordSecretRef = spec.getPasswordSecretRef(); - var loginExpected = passwordSecretRef != null; - - if (loginExpected && password != null) { - passwordMatches = postgreSQLAuthenticationService.passwordMatches( - tx, - spec, - password - ); - } + var passwordMatches = Objects.equals(expectedFingerprint, status.getPasswordFingerprint()); if (roleLoginMatches && passwordMatches && flagsMatch && commentMatches) { log.info( @@ -311,12 +326,14 @@ private UpdateControl reconcileInTransaction( name ); - if (!roleLoginMatches || !passwordMatches || !flagsMatch) { + if (!roleLoginMatches || changePassword || !flagsMatch) { roleService.alterRole( tx, spec, + currentFlags, + currentCanLogin, changePassword, - password + serverPassword ); } diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleService.java b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleService.java index daa3c2f..286e5e2 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleService.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleService.java @@ -13,10 +13,12 @@ import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Objects; +import java.util.Optional; -import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_AUTHID; import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_AUTH_MEMBERS; +import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_ROLES; import static org.jooq.impl.DSL.field; import static org.jooq.impl.DSL.keyword; import static org.jooq.impl.DSL.multiset; @@ -27,16 +29,25 @@ import static org.jooq.impl.DSL.sql; import static org.jooq.impl.DSL.val; +/// Reads and writes PostgreSQL roles. +/// +/// All reads use the public view `pg_roles` and the public catalog `pg_auth_members` instead of +/// `pg_authid`. `pg_authid` is readable by superusers only, and managed PostgreSQL services of cloud +/// providers revoke it from every role, including the master user. @Singleton @NullMarked public final class RoleService { + /// Shared object comments of roles are keyed by the `pg_authid` catalog in `pg_shdescription`. + /// This is only a name for `shobj_description`, no `SELECT` on `pg_authid` is issued. + private static final String ROLE_COMMENT_CATALOG = "pg_authid"; + public boolean roleExists( DSLContext tx, RoleSpec spec ) { return tx.fetchExists(selectOne() - .from(PG_AUTHID) - .where(PG_AUTHID.ROLNAME.eq(spec.getName())) + .from(PG_ROLES) + .where(PG_ROLES.ROLNAME.eq(spec.getName())) ); } @@ -65,23 +76,32 @@ public void createRole( } } + /// Alter the role so that it matches the spec. + /// + /// The statement names only the options that differ from the current state. + /// PostgreSQL rejects `SUPERUSER`, `REPLICATION`, and `BYPASSRLS` (and their `NO` variants) from a non-superuser even + /// when the value does not change, so a full statement would fail on managed services. + /// + /// @param currentFlags the flags as read with [#fetchCurrentFlags] + /// @param currentCanLogin whether the role currently has `LOGIN` + /// @param changePassword whether to set the given password + /// @param password the password literal to set, or `null` for a `NOLOGIN` role public void alterRole( DSLContext tx, RoleSpec spec, + RoleSpec.Flags currentFlags, + boolean currentCanLogin, boolean changePassword, @Nullable String password ) { - var roleName = spec.getName(); - var flags = spec.getFlags(); - - tx.execute( - buildAlterRole( - roleName, - flags, - changePassword, - password - ) - ); + buildAlterRole( + spec.getName(), + spec.getFlags(), + currentFlags, + currentCanLogin, + changePassword, + password + ).ifPresent(tx::execute); } public void updateComment( @@ -119,55 +139,59 @@ public boolean roleCommentMatches( DSLContext tx, String roleName ) { - return tx .select(Routines.shobjDescription( - PG_AUTHID.OID, - val(PG_AUTHID.getUnqualifiedName().last()) + PG_ROLES.OID, + val(ROLE_COMMENT_CATALOG) )) - .from(PG_AUTHID) - .where(PG_AUTHID.ROLNAME.eq(roleName)) + .from(PG_ROLES) + .where(PG_ROLES.ROLNAME.eq(roleName)) .fetchOneInto(String.class); } + public boolean roleCanLogin( + DSLContext tx, + RoleSpec spec + ) { + return tx.fetchExists(selectOne() + .from(PG_ROLES) + .where(PG_ROLES.ROLNAME.eq(spec.getName())) + .and(PG_ROLES.ROLCANLOGIN.isTrue()) + ); + } + public boolean roleLoginMatches( DSLContext tx, RoleSpec spec ) { var loginExpected = spec.getPasswordSecretRef() != null; - var canLogin = tx.fetchExists(selectOne() - .from(PG_AUTHID) - .where(PG_AUTHID.ROLNAME.eq(spec.getName())) - .and(PG_AUTHID.ROLCANLOGIN.isTrue()) - ); - - return loginExpected == canLogin; + return loginExpected == roleCanLogin(tx, spec); } public RoleSpec.Flags fetchCurrentFlags( DSLContext tx, RoleSpec spec ) { - var member = PG_AUTHID.as("member"); - var parent = PG_AUTHID.as("parent"); + var member = PG_ROLES.as("member"); + var parent = PG_ROLES.as("parent"); return tx .select( - PG_AUTHID.ROLSUPER.as("superuser"), - PG_AUTHID.ROLCREATEDB.as("createdb"), - PG_AUTHID.ROLCREATEROLE.as("createrole"), - PG_AUTHID.ROLINHERIT.as("inherit"), - PG_AUTHID.ROLREPLICATION.as("replication"), - PG_AUTHID.ROLBYPASSRLS.as("bypassrls"), - PG_AUTHID.ROLCONNLIMIT.as("connectionLimit"), - field("nullif({0}, 'infinity')", PG_AUTHID.ROLVALIDUNTIL.getDataType(), PG_AUTHID.ROLVALIDUNTIL).as("validUntil"), + PG_ROLES.ROLSUPER.as("superuser"), + PG_ROLES.ROLCREATEDB.as("createdb"), + PG_ROLES.ROLCREATEROLE.as("createrole"), + PG_ROLES.ROLINHERIT.as("inherit"), + PG_ROLES.ROLREPLICATION.as("replication"), + PG_ROLES.ROLBYPASSRLS.as("bypassrls"), + PG_ROLES.ROLCONNLIMIT.as("connectionLimit"), + field("nullif({0}, 'infinity')", PG_ROLES.ROLVALIDUNTIL.getDataType(), PG_ROLES.ROLVALIDUNTIL).as("validUntil"), multiset( select(parent.ROLNAME) .from(PG_AUTH_MEMBERS) .join(member).on(member.OID.eq(PG_AUTH_MEMBERS.MEMBER)) .join(parent).on(parent.OID.eq(PG_AUTH_MEMBERS.ROLEID)) - .where(member.OID.eq(PG_AUTHID.OID)) + .where(member.OID.eq(PG_ROLES.OID)) .orderBy(parent.ROLNAME) ).as("inRole").convertFrom(result -> result.map(Record1::value1)), multiset( @@ -175,12 +199,12 @@ public RoleSpec.Flags fetchCurrentFlags( .from(PG_AUTH_MEMBERS) .join(parent).on(parent.OID.eq(PG_AUTH_MEMBERS.ROLEID)) .join(member).on(member.OID.eq(PG_AUTH_MEMBERS.MEMBER)) - .where(parent.OID.eq(PG_AUTHID.OID)) + .where(parent.OID.eq(PG_ROLES.OID)) .orderBy(member.ROLNAME) ).as("role").convertFrom(result -> result.map(Record1::value1)) ) - .from(PG_AUTHID) - .where(PG_AUTHID.ROLNAME.eq(spec.getName())) + .from(PG_ROLES) + .where(PG_ROLES.ROLNAME.eq(spec.getName())) .fetchSingleInto(RoleSpec.Flags.class); } @@ -325,9 +349,16 @@ private static Query buildCreateRole( ); } - private static Query buildAlterRole( + /// Build: `ALTER ROLE [ [ WITH ] option [ ... ] ]` + /// + /// See [PostgreSQL: Documentation: ALTER ROLE](https://www.postgresql.org/docs/current/sql-alterrole.html) + /// + /// Only options that differ from the current state are added. Returns empty when nothing differs. + private static Optional buildAlterRole( String roleName, RoleSpec.Flags flags, + RoleSpec.Flags currentFlags, + boolean currentCanLogin, boolean changePassword, @Nullable String password ) { @@ -335,14 +366,21 @@ private static Query buildAlterRole( var loginExpected = password != null; // LOGIN / NOLOGIN - options.add(keyword(loginExpected - ? RoleFlag.LOGIN.flag() - : RoleFlag.NO_LOGIN.flag() - )); + if (loginExpected != currentCanLogin) { + options.add(keyword(loginExpected + ? RoleFlag.LOGIN.flag() + : RoleFlag.NO_LOGIN.flag() + )); + } // Password handling - // - if NOLOGIN, remove the password - // - if LOGIN and passwordChanged, set the new password + // - if no login is expected, remove the password + // - if LOGIN and the password changed, set the new password + // + // `PASSWORD NULL` is not conditional on the current state. `pg_roles` masks `rolpassword` + // with a constant, so the operator cannot tell whether a `NOLOGIN` role still holds a + // password. The reconciler calls this method only when the role differs from the spec, + // so the statement does not run on every reconcile. if (!loginExpected) { options.add(keyword(RoleFlag.PASSWORD.flag())); options.add(keyword("NULL")); @@ -351,48 +389,53 @@ private static Query buildAlterRole( options.add(val(password)); } - // Explicitly set the expected state to make the statement idempotent - options.add(keyword(flags.isSuperuser() - ? RoleFlag.SUPERUSER.flag() - : RoleFlag.NO_SUPERUSER.flag() - )); - options.add(keyword(flags.isCreatedb() - ? RoleFlag.CREATEDB.flag() - : RoleFlag.NO_CREATEDB.flag() - )); - options.add(keyword(flags.isCreaterole() - ? RoleFlag.CREATEROLE.flag() - : RoleFlag.NO_CREATEROLE.flag() - )); - options.add(keyword(flags.isInherit() - ? RoleFlag.INHERIT.flag() - : RoleFlag.NO_INHERIT.flag() - )); - options.add(keyword(flags.isReplication() - ? RoleFlag.REPLICATION.flag() - : RoleFlag.NO_REPLICATION.flag() - )); - options.add(keyword(flags.isBypassrls() - ? RoleFlag.BYPASSRLS.flag() - : RoleFlag.NO_BYPASSRLS.flag() - )); + addFlagIfChanged(options, flags.isSuperuser(), currentFlags.isSuperuser(), RoleFlag.SUPERUSER, RoleFlag.NO_SUPERUSER); + addFlagIfChanged(options, flags.isCreatedb(), currentFlags.isCreatedb(), RoleFlag.CREATEDB, RoleFlag.NO_CREATEDB); + addFlagIfChanged(options, flags.isCreaterole(), currentFlags.isCreaterole(), RoleFlag.CREATEROLE, RoleFlag.NO_CREATEROLE); + addFlagIfChanged(options, flags.isInherit(), currentFlags.isInherit(), RoleFlag.INHERIT, RoleFlag.NO_INHERIT); + addFlagIfChanged(options, flags.isReplication(), currentFlags.isReplication(), RoleFlag.REPLICATION, RoleFlag.NO_REPLICATION); + addFlagIfChanged(options, flags.isBypassrls(), currentFlags.isBypassrls(), RoleFlag.BYPASSRLS, RoleFlag.NO_BYPASSRLS); - options.add(keyword(RoleFlag.CONNECTION_LIMIT.flag())); - options.add(val(flags.getConnectionLimit())); + if (flags.getConnectionLimit() != currentFlags.getConnectionLimit()) { + options.add(keyword(RoleFlag.CONNECTION_LIMIT.flag())); + options.add(val(flags.getConnectionLimit())); + } var validUntil = flags.getValidUntil(); - options.add(keyword(RoleFlag.VALID_UNTIL.flag())); - if (validUntil != null) { - options.add(val(validUntil.toString())); - } else { - options.add(val("infinity")); + if (!Objects.equals(validUntil, currentFlags.getValidUntil())) { + options.add(keyword(RoleFlag.VALID_UNTIL.flag())); + options.add(val(validUntil != null + ? validUntil.toString() + : "infinity" + )); } - return query( + if (options.isEmpty()) { + return Optional.empty(); + } + + return Optional.of(query( "alter role {0} with {1}", role(roleName), SQLUtil.concatenateQueryPartsWithSpaces(options) - ); + )); + } + + private static void addFlagIfChanged( + List options, + boolean expected, + boolean current, + RoleFlag enabled, + RoleFlag disabled + ) { + if (expected == current) { + return; + } + + options.add(keyword(expected + ? enabled.flag() + : disabled.flag() + )); } private static Query buildGrantRoleToMember( diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleSpec.java b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleSpec.java index e794a09..a69a5bb 100644 --- a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleSpec.java +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleSpec.java @@ -37,6 +37,10 @@ public class RoleSpec { @io.fabric8.generator.annotation.Nullable private @Nullable ResourceRef passwordSecretRef; + /// How the operator sends the password to PostgreSQL. Defaults to an operator-side SCRAM-SHA-256 verifier. + @io.fabric8.generator.annotation.Nullable + private PasswordEncryption passwordEncryption = PasswordEncryption.SCRAM_SHA_256; + @io.fabric8.generator.annotation.Nullable private Flags flags = new Flags(); diff --git a/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleStatus.java b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleStatus.java new file mode 100644 index 0000000..c02383a --- /dev/null +++ b/operator/src/main/java/it/aboutbits/postgresql/crd/role/RoleStatus.java @@ -0,0 +1,23 @@ +package it.aboutbits.postgresql.crd.role; + +import it.aboutbits.postgresql.core.CRStatus; +import lombok.Getter; +import lombok.Setter; +import lombok.experimental.Accessors; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/// Status Object for the `Role` Custom Resource. +@Getter +@Setter +@Accessors(chain = true) +@NullMarked +public class RoleStatus extends CRStatus { + /// Keyed fingerprint (HMAC-SHA256) of the password the operator applied last. + /// + /// The operator cannot read the password hash from `pg_authid` without superuser rights, + /// which managed PostgreSQL services of cloud providers do not grant. It therefore compares the referenced + /// Secret against this fingerprint to detect a password change. The HMAC key is private to the + /// operator, so a reader of this status learns nothing about the password. + private @Nullable String passwordFingerprint = null; +} diff --git a/operator/src/main/resources/application.yml b/operator/src/main/resources/application.yml index b16a3a2..a4d7b19 100644 --- a/operator/src/main/resources/application.yml +++ b/operator/src/main/resources/application.yml @@ -1,3 +1,9 @@ +postgresql-operator: + password-fingerprint: + # Name of the Secret in the operator namespace that holds the HMAC key for the Role password fingerprints. + # The operator creates the Secret on first use. + secret-name: postgresql-operator-password-fingerprint-key + quarkus: console: color: true @@ -214,3 +220,22 @@ quarkus: QUARKUS_CONSOLE_COLOR: ${quarkus.console.color} QUARKUS_LOG_CONSOLE_JSON_ENABLED: ${quarkus.log.console.json.enabled} QUARKUS_LOG_CONSOLE_JSON_LOG_FORMAT: ${quarkus.log.console.json.log-format} + rbac: + # The operator creates the password fingerprint key Secret in its own namespace. + # `create` on Secrets is granted through this namespaced Role only, not through the ClusterRole of the Role controller, + # which would allow the operator to create Secrets in every namespace. + # `api-groups` is omitted, so the rule targets the core API group. + roles: + postgresql-operator-password-fingerprint-key: + policy-rules: + create-secrets: + resources: secrets + verbs: create + role-bindings: + postgresql-operator-password-fingerprint-key: + role-name: postgresql-operator-password-fingerprint-key + # The kubernetes-client extension stops generating its default binding to the `view` ClusterRole as soon as + # `role-bindings` are configured. This entry keeps that binding, so that the chart does not lose it. + postgresql-operator-view: + role-name: view + cluster-wide: true diff --git a/operator/src/test/java/it/aboutbits/postgresql/_support/PostgreSQLPasswordVerifier.java b/operator/src/test/java/it/aboutbits/postgresql/_support/PostgreSQLPasswordVerifier.java new file mode 100644 index 0000000..3f0be40 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/postgresql/_support/PostgreSQLPasswordVerifier.java @@ -0,0 +1,122 @@ +package it.aboutbits.postgresql._support; + +import com.ongres.scram.common.StringPreparation; +import org.jooq.DSLContext; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import javax.crypto.Mac; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.crypto.spec.SecretKeySpec; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.Base64; +import java.util.HexFormat; +import java.util.Locale; + +import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_AUTHID; + +/// Test helper that verifies a cleartext password against the verifier stored in `pg_authid`. +/// +/// Reading `pg_authid` needs superuser rights, so the given `DSLContext` must connect as a superuser. +/// The operator itself does not read `pg_authid` anymore. +@NullMarked +public final class PostgreSQLPasswordVerifier { + private static final String SCRAM_SHA_256_PREFIX = "SCRAM-SHA-256$"; + private static final String MD5 = "MD5"; + + /// The verifier as stored in `pg_authid.rolpassword`. + public static @Nullable String storedVerifier( + DSLContext superuserDsl, + String roleName + ) { + return superuserDsl + .select(PG_AUTHID.ROLPASSWORD) + .from(PG_AUTHID) + .where(PG_AUTHID.ROLNAME.eq(roleName)) + .fetchSingle(PG_AUTHID.ROLPASSWORD); + } + + public static boolean passwordMatches( + DSLContext superuserDsl, + String roleName, + String expectedPassword + ) { + var currentPasswordVerifier = storedVerifier(superuserDsl, roleName); + + if (currentPasswordVerifier == null || currentPasswordVerifier.isBlank()) { + return false; + } + + // PostgreSQL stores either: + // - SCRAM verifier: SCRAM-SHA-256$:$: + // - or legacy md5: md5 + if (currentPasswordVerifier.startsWith(SCRAM_SHA_256_PREFIX)) { + return verifyScramSha256(currentPasswordVerifier, expectedPassword); + } + + if (currentPasswordVerifier.startsWith(MD5.toLowerCase(Locale.ROOT))) { + return verifyMd5(currentPasswordVerifier, expectedPassword, roleName); + } + + return false; + } + + private static boolean verifyScramSha256( + String postgresVerifier, + String cleartextPassword + ) { + var preparedPassword = StringPreparation.POSTGRESQL_PREPARATION.normalize( + cleartextPassword.toCharArray() + ); + + var afterPrefix = postgresVerifier.substring(SCRAM_SHA_256_PREFIX.length()); + var dollar = afterPrefix.indexOf('$'); + var iterationsAndSalt = afterPrefix.substring(0, dollar); + var keys = afterPrefix.substring(dollar + 1); + + var colonIterationsAndSalt = iterationsAndSalt.indexOf(':'); + var iterations = Integer.parseInt(iterationsAndSalt.substring(0, colonIterationsAndSalt)); + var salt = Base64.getDecoder().decode(iterationsAndSalt.substring(colonIterationsAndSalt + 1)); + + var storedKey = Base64.getDecoder().decode(keys.substring(0, keys.indexOf(':'))); + + try { + var secretKeyFactory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); + var saltedPassword = secretKeyFactory.generateSecret( + new PBEKeySpec(preparedPassword, salt, iterations, 32 * 8) + ).getEncoded(); + + var mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(saltedPassword, "HmacSHA256")); + var clientKey = mac.doFinal("Client Key".getBytes(StandardCharsets.UTF_8)); + + var expectedStoredKey = MessageDigest.getInstance("SHA-256").digest(clientKey); + + return MessageDigest.isEqual(storedKey, expectedStoredKey); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + + private static boolean verifyMd5( + String postgresMd5, + String expectedPassword, + String username + ) { + try { + var currentDigest = HexFormat.of().parseHex(postgresMd5, 3, postgresMd5.length()); + + var md5 = MessageDigest.getInstance(MD5); + md5.update((expectedPassword + username).getBytes(StandardCharsets.UTF_8)); + + return MessageDigest.isEqual(currentDigest, md5.digest()); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + + private PostgreSQLPasswordVerifier() { + } +} diff --git a/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/RoleCreate.java b/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/RoleCreate.java index 064ede8..a673afc 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/RoleCreate.java +++ b/operator/src/test/java/it/aboutbits/postgresql/_support/testdata/persisted/creator/RoleCreate.java @@ -5,6 +5,7 @@ import it.aboutbits.postgresql._support.testdata.base.TestDataCreator; import it.aboutbits.postgresql._support.testdata.persisted.Given; import it.aboutbits.postgresql.core.ResourceRef; +import it.aboutbits.postgresql.crd.role.PasswordEncryption; import it.aboutbits.postgresql.crd.role.Role; import it.aboutbits.postgresql.crd.role.RoleSpec; import lombok.AccessLevel; @@ -40,6 +41,8 @@ public class RoleCreate extends TestDataCreator { private RoleSpec.@Nullable Flags withFlags; + private @Nullable PasswordEncryption withPasswordEncryption; + public RoleCreate( int numberOfItems, Given given, @@ -102,6 +105,10 @@ protected Role create(int index) { spec.setFlags(withFlags); } + if (withPasswordEncryption != null) { + spec.setPasswordEncryption(withPasswordEncryption); + } + item.setSpec(spec); kubernetesClient.resources(Role.class) diff --git a/operator/src/test/java/it/aboutbits/postgresql/core/PasswordFingerprintServiceTest.java b/operator/src/test/java/it/aboutbits/postgresql/core/PasswordFingerprintServiceTest.java new file mode 100644 index 0000000..96e5631 --- /dev/null +++ b/operator/src/test/java/it/aboutbits/postgresql/core/PasswordFingerprintServiceTest.java @@ -0,0 +1,198 @@ +package it.aboutbits.postgresql.core; + +import io.fabric8.kubernetes.api.model.SecretBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; +import it.aboutbits.postgresql.crd.role.PasswordEncryption; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.util.Base64; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +@NullMarked +@EnableKubernetesMockClient(crud = true) +class PasswordFingerprintServiceTest { + private static final String SECRET_NAME = "test-password-fingerprint-key"; + + @SuppressWarnings("NullAway.Init") + static KubernetesClient client; + + @BeforeEach + void clearSecrets() { + client.secrets().inAnyNamespace().delete(); + } + + /// Each instance has its own key cache, like a fresh operator process. + private static PasswordFingerprintService newService() { + var service = new PasswordFingerprintService(client); + service.secretName = SECRET_NAME; + + return service; + } + + @Nested + class KeySecret { + @Test + @DisplayName("When the key Secret does not exist, should create it with a random 32 byte key") + void whenSecretMissing_shouldCreateIt() { + // given + var service = newService(); + + // when + service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + // then + var secret = client.secrets() + .inNamespace(client.getNamespace()) + .withName(SECRET_NAME) + .get(); + + assertThat(secret).isNotNull(); + assertThat(secret.getType()).isEqualTo("Opaque"); + assertThat(secret.getData()).containsOnlyKeys(PasswordFingerprintService.SECRET_DATA_KEY); + + var key = Base64.getDecoder().decode(secret.getData().get(PasswordFingerprintService.SECRET_DATA_KEY)); + assertThat(key).hasSize(32); + } + + @Test + @DisplayName("When the key Secret exists, should reuse its key") + void whenSecretExists_shouldReuseKey() { + // given + var fingerprint = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + // when: a second operator process starts + var fingerprintOfSecondProcess = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + // then + assertThat(fingerprintOfSecondProcess).isEqualTo(fingerprint); + } + + @Test + @DisplayName("When the key Secret is lost, should create a new key and the fingerprints change") + void whenSecretLost_shouldCreateNewKey() { + // given + var fingerprint = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + client.secrets() + .inNamespace(client.getNamespace()) + .withName(SECRET_NAME) + .delete(); + + // when + var fingerprintWithNewKey = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + // then + assertThat(fingerprintWithNewKey).isNotEqualTo(fingerprint); + } + + @Test + @DisplayName("When the key Secret has no 'key' entry, should fail with a message that names the Secret") + void whenSecretHasNoKeyEntry_shouldFail() { + // given + client.secrets() + .inNamespace(client.getNamespace()) + .resource(new SecretBuilder() + .withNewMetadata() + .withName(SECRET_NAME) + .endMetadata() + .addToData("other", Base64.getEncoder().encodeToString("value".getBytes(StandardCharsets.UTF_8))) + .build() + ) + .create(); + + var service = newService(); + + // when / then + assertThatThrownBy(() -> service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("missing required data 'key'") + .hasMessageContaining(SECRET_NAME); + } + } + + @Nested + class Fingerprint { + @Test + @DisplayName("When the same password and encryption are given, should return the same fingerprint") + void whenSameInput_shouldReturnSameFingerprint() { + // given + var service = newService(); + + // when + var first = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + var second = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + // then + assertThat(second).isEqualTo(first); + } + + @Test + @DisplayName("When the password changes, should return a different fingerprint") + void whenPasswordChanges_shouldReturnDifferentFingerprint() { + // given + var service = newService(); + + // when + var first = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + var second = service.fingerprint("other-password", PasswordEncryption.SCRAM_SHA_256); + + // then + assertThat(second).isNotEqualTo(first); + } + + @Test + @DisplayName("When the encryption changes, should return a different fingerprint") + void whenEncryptionChanges_shouldReturnDifferentFingerprint() { + // given + var service = newService(); + + // when + var first = service.fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + var second = service.fingerprint("password", PasswordEncryption.SERVER); + + // then + assertThat(second).isNotEqualTo(first); + } + + @Test + @DisplayName("Should be the Base64 HMAC-SHA256 of ' 0x00 ' with the key from the Secret") + void shouldMatchDocumentedConstruction() throws GeneralSecurityException { + // Fingerprints in existing Role statuses must stay valid after an upgrade, so the construction is fixed. + + // given + var fingerprint = newService().fingerprint("password", PasswordEncryption.SCRAM_SHA_256); + + var key = Base64.getDecoder().decode( + client.secrets() + .inNamespace(client.getNamespace()) + .withName(SECRET_NAME) + .require() + .getData() + .get(PasswordFingerprintService.SECRET_DATA_KEY) + ); + + // when + var mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(key, "HmacSHA256")); + mac.update("scram-sha-256".getBytes(StandardCharsets.UTF_8)); + mac.update((byte) 0); + mac.update("password".getBytes(StandardCharsets.UTF_8)); + + var expected = Base64.getEncoder().encodeToString(mac.doFinal()); + + // then + assertThat(fingerprint).isEqualTo(expected); + } + } +} diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerNonSuperuserTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerNonSuperuserTest.java new file mode 100644 index 0000000..154bf6f --- /dev/null +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerNonSuperuserTest.java @@ -0,0 +1,401 @@ +package it.aboutbits.postgresql.crd.role; + +import io.fabric8.kubernetes.api.model.SecretBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.quarkus.test.junit.QuarkusTest; +import it.aboutbits.postgresql._support.PostgreSQLPasswordVerifier; +import it.aboutbits.postgresql._support.testdata.base.TestUtil; +import it.aboutbits.postgresql._support.testdata.persisted.Given; +import it.aboutbits.postgresql.core.CRPhase; +import it.aboutbits.postgresql.core.PostgreSQLContextFactory; +import it.aboutbits.postgresql.crd.clusterconnection.ClusterConnection; +import lombok.RequiredArgsConstructor; +import org.jooq.DSLContext; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; +import java.util.stream.Stream; + +import static it.aboutbits.postgresql.core.KubernetesService.SECRET_DATA_BASIC_AUTH_PASSWORD_KEY; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; +import static org.jooq.impl.DSL.role; + +/// Runs the Role reconciler with an admin role that is not a superuser. +/// +/// The admin role has only `LOGIN`, `CREATEDB`, and `CREATEROLE`, like the master user of managed PostgreSQL services +/// of cloud providers, such as AWS RDS, Google Cloud SQL, or Azure Database for PostgreSQL. +/// Such a role cannot read `pg_authid` or `pg_shadow` and cannot name `SUPERUSER`, `REPLICATION`, or `BYPASSRLS` in `ALTER ROLE`. +@QuarkusTest +@RequiredArgsConstructor +@NullMarked +class RoleReconcilerNonSuperuserTest { + private static final String ADMIN_ROLE = "test_non_superuser_admin"; + private static final String ADMIN_PASSWORD = "test-non-superuser-admin-password"; + + private final Given given; + + private final RoleService roleService; + private final PostgreSQLContextFactory postgreSQLContextFactory; + + private final KubernetesClient kubernetesClient; + + @BeforeEach + void resetEnvironment() { + TestUtil.resetEnvironment(kubernetesClient); + } + + @Test + @DisplayName("When the admin is not a superuser, a Role (LOGIN) should still be created, updated, and dropped") + void nonSuperuserAdmin_createsUpdatesAndDropsLoginRole() { + // given + var rootConnection = givenRootClusterConnection("test-connection-root-login"); + var rootDsl = postgreSQLContextFactory.getDSLContext(rootConnection); + var adminConnection = givenNonSuperuserClusterConnection(rootDsl, "test-connection-non-superuser-login"); + var adminDsl = postgreSQLContextFactory.getDSLContext(adminConnection); + + var roleName = "test-non-superuser-role-login"; + var initialPassword = "initial-password"; + var newPassword = "new-password"; + + var secretRef = given.one() + .secretRef() + .withPassword(initialPassword) + .returnFirst(); + + // when: create + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(adminConnection.getMetadata().getName()) + .withPasswordSecretRef(secretRef) + .withComment("created by a non-superuser admin") + .returnFirst(); + + // then: the reads through pg_roles work as non-superuser + assertThat(role.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(role.getStatus().getPasswordFingerprint()).isNotBlank(); + assertThat(roleService.roleExists(adminDsl, role.getSpec())).isTrue(); + assertThat(roleService.roleLoginMatches(adminDsl, role.getSpec())).isTrue(); + assertThat(roleService.fetchCurrentRoleComment(adminDsl, roleName)).isEqualTo("created by a non-superuser admin"); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + rootDsl, + roleName, + initialPassword + )).isTrue(); + + // when: update flags, comment, and connection limit + var spec = role.getSpec(); + spec.setComment("updated comment"); + spec.getFlags().setCreatedb(true); + spec.getFlags().setConnectionLimit(7); + + var updated = applyRole(role); + + // then + assertThat(updated.getStatus().getPhase()).isEqualTo(CRPhase.READY); + + var currentFlags = roleService.fetchCurrentFlags(adminDsl, spec); + assertThat(currentFlags.isCreatedb()).isTrue(); + assertThat(currentFlags.getConnectionLimit()).isEqualTo(7); + assertThat(roleService.fetchCurrentRoleComment(adminDsl, roleName)).isEqualTo("updated comment"); + + // when: rotate the password in the Secret + var secret = kubernetesClient.secrets() + .inNamespace(kubernetesClient.getNamespace()) + .withName(secretRef.getName()) + .require(); + secret.getMetadata().setManagedFields(null); + secret = new SecretBuilder(secret) + .addToStringData(SECRET_DATA_BASIC_AUTH_PASSWORD_KEY, newPassword) + .build(); + + kubernetesClient.secrets() + .inNamespace(kubernetesClient.getNamespace()) + .resource(secret) + .serverSideApply(); + + // then + await().atMost(5, TimeUnit.SECONDS) + .pollInterval(100, TimeUnit.MILLISECONDS) + .until(() -> PostgreSQLPasswordVerifier.passwordMatches( + rootDsl, + roleName, + newPassword + )); + + // when: drop + kubernetesClient.resources(Role.class) + .inNamespace(role.getMetadata().getNamespace()) + .withName(role.getMetadata().getName()) + .withTimeout(5, TimeUnit.SECONDS) + .delete(); + + // then + await().atMost(5, TimeUnit.SECONDS) + .pollInterval(100, TimeUnit.MILLISECONDS) + .until(() -> !roleService.roleExists(rootDsl, role.getSpec())); + } + + @Test + @DisplayName("When the admin is not a superuser, login and membership of an existing Role should be updated") + void nonSuperuserAdmin_updatesLoginAndMembership() { + // given + var rootConnection = givenRootClusterConnection("test-connection-root-membership"); + var rootDsl = postgreSQLContextFactory.getDSLContext(rootConnection); + var adminConnection = givenNonSuperuserClusterConnection(rootDsl, "test-connection-non-superuser-membership"); + var adminDsl = postgreSQLContextFactory.getDSLContext(adminConnection); + + // The admin creates the parent role, so it holds ADMIN OPTION on it (required since PostgreSQL 16) + var parentRole = "test_non_superuser_parent"; + adminDsl.execute("drop role if exists {0}", role(parentRole)); + adminDsl.execute("create role {0}", role(parentRole)); + + var roleName = "test-non-superuser-role-membership"; + + // when: create a NOLOGIN role + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(adminConnection.getMetadata().getName()) + .returnFirst(); + + var spec = role.getSpec(); + + // then + assertThat(role.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(roleService.roleCanLogin(adminDsl, spec)).isFalse(); + + // when: turn it into a LOGIN role and add it to the parent role + var password = "member-password"; + spec.setPasswordSecretRef(given.one() + .secretRef() + .withPassword(password) + .returnFirst() + ); + spec.getFlags().setInRole(List.of(parentRole)); + + var updated = applyRole(role); + + // then + assertThat(updated.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(roleService.roleCanLogin(adminDsl, spec)).isTrue(); + assertThat(roleService.fetchCurrentFlags(adminDsl, spec).getInRole()).containsExactly(parentRole); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + rootDsl, + roleName, + password + )).isTrue(); + + // when: back to NOLOGIN without membership + spec.setPasswordSecretRef(null); + spec.getFlags().setInRole(List.of()); + + updated = applyRole(role); + + // then + assertThat(updated.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(updated.getStatus().getPasswordFingerprint()).isNull(); + assertThat(roleService.roleCanLogin(adminDsl, spec)).isFalse(); + assertThat(roleService.fetchCurrentFlags(adminDsl, spec).getInRole()).isEmpty(); + + // cleanup + adminDsl.execute("drop role if exists {0}", role(parentRole)); + } + + @ParameterizedTest(name = "flag {0}") + @MethodSource("provideSuperuserOnlyFlags") + @DisplayName("When the admin is not a superuser and the Role asks for a superuser-only flag, the status should be ERROR") + void nonSuperuserAdmin_superuserOnlyFlag_setsError( + String flagName, + BiConsumer flagSetter + ) { + // given + var rootConnection = givenRootClusterConnection("test-connection-root-%s-flag".formatted(flagName)); + var rootDsl = postgreSQLContextFactory.getDSLContext(rootConnection); + var adminConnection = givenNonSuperuserClusterConnection(rootDsl, "test-connection-non-superuser-%s-flag".formatted(flagName)); + + var roleName = "test-non-superuser-role-%s-flag".formatted(flagName); + + var flags = new RoleSpec.Flags(); + flagSetter.accept(flags, true); + + // when + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(adminConnection.getMetadata().getName()) + .withFlags(flags) + .returnFirst(); + + // then + var failed = kubernetesClient.resources(Role.class) + .inNamespace(role.getMetadata().getNamespace()) + .withName(role.getMetadata().getName()) + .waitUntilCondition( + r -> r.getStatus() != null && r.getStatus().getPhase() == CRPhase.ERROR, + 5, + TimeUnit.SECONDS + ); + + assertThat(failed.getStatus().getMessage()) + .containsAnyOf("permission denied", "must be superuser"); + assertThat(roleService.roleExists(rootDsl, role.getSpec())).isFalse(); + } + + @Test + @DisplayName("When the admin is not a superuser and passwordEncryption is 'server', the server should hash the password") + void nonSuperuserAdmin_serverPasswordEncryption_letsTheServerHash() { + // given + var rootConnection = givenRootClusterConnection("test-connection-root-server-encryption"); + var rootDsl = postgreSQLContextFactory.getDSLContext(rootConnection); + var adminConnection = givenNonSuperuserClusterConnection(rootDsl, "test-connection-non-superuser-server-encryption"); + + var roleName = "test-non-superuser-role-server-encryption"; + var password = "server-side-password"; + + var secretRef = given.one() + .secretRef() + .withPassword(password) + .returnFirst(); + + // when + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(adminConnection.getMetadata().getName()) + .withPasswordSecretRef(secretRef) + .withPasswordEncryption(PasswordEncryption.SERVER) + .returnFirst(); + + // then + assertThat(role.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + rootDsl, + roleName, + password + )).isTrue(); + } + + @Test + @DisplayName("When the admin is not a superuser and the Secret contains a SCRAM-SHA-256 verifier, it should be stored verbatim") + void nonSuperuserAdmin_preHashedPassword_isStoredVerbatim() { + // given + var rootConnection = givenRootClusterConnection("test-connection-root-prehashed"); + var rootDsl = postgreSQLContextFactory.getDSLContext(rootConnection); + var adminConnection = givenNonSuperuserClusterConnection(rootDsl, "test-connection-non-superuser-prehashed"); + + var roleName = "test-non-superuser-role-prehashed"; + + // Verifier for the password "abc", generated by PostgreSQL + var verifier = "SCRAM-SHA-256$4096:gxUQWxfrRYegSTNiHXFT+g==$lxMC2yO9Lx9gm2dgNPo/1Qar+pjAvxCP2VN4yPWYnzE=:0QzcS9VJHJszBq4vSce3n4M6NZmyWa1GWdkJDi8hRNc="; + + var secretRef = given.one() + .secretRef() + .withPassword(verifier) + .returnFirst(); + + // when + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(adminConnection.getMetadata().getName()) + .withPasswordSecretRef(secretRef) + .returnFirst(); + + // then + assertThat(role.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(PostgreSQLPasswordVerifier.storedVerifier(rootDsl, roleName)).isEqualTo(verifier); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + rootDsl, + roleName, + "abc" + )).isTrue(); + } + + private static Stream provideSuperuserOnlyFlags() { + return Stream.of( + Arguments.of("superuser", (BiConsumer) RoleSpec.Flags::setSuperuser), + Arguments.of("replication", (BiConsumer) RoleSpec.Flags::setReplication), + Arguments.of("bypassrls", (BiConsumer) RoleSpec.Flags::setBypassrls) + ); + } + + private ClusterConnection givenRootClusterConnection(String name) { + return given.one() + .clusterConnection() + .withName(name) + .returnFirst(); + } + + /// Creates the non-superuser admin role in PostgreSQL (if missing) and a ClusterConnection that uses it. + private ClusterConnection givenNonSuperuserClusterConnection( + DSLContext rootDsl, + String name + ) { + rootDsl.execute( + """ + do $$ + begin + if not exists (select from pg_catalog.pg_roles where rolname = '%s') then + create role %s with login nosuperuser createdb createrole noreplication nobypassrls password '%s'; + end if; + end + $$ + """.formatted(ADMIN_ROLE, ADMIN_ROLE, ADMIN_PASSWORD) + ); + + var adminSecretRef = given.one() + .secretRef() + .withUsername(ADMIN_ROLE) + .withPassword(ADMIN_PASSWORD) + .returnFirst(); + + var clusterConnection = given.one() + .clusterConnection() + .withName(name) + .withAdminSecretRef(adminSecretRef) + .returnFirst(); + + return kubernetesClient.resources(ClusterConnection.class) + .inNamespace(clusterConnection.getMetadata().getNamespace()) + .withName(clusterConnection.getMetadata().getName()) + .waitUntilCondition( + c -> c.getStatus() != null && c.getStatus().getPhase() == CRPhase.READY, + 5, + TimeUnit.SECONDS + ); + } + + private Role applyRole(Role role) { + var namespace = kubernetesClient.getNamespace(); + + role.getMetadata().setManagedFields(null); + role.getMetadata().setResourceVersion(null); + + var applied = kubernetesClient.resources(Role.class) + .inNamespace(namespace) + .resource(role) + .serverSideApply(); + + var generation = applied.getMetadata().getGeneration(); + + //noinspection ConstantConditions + return kubernetesClient.resources(Role.class) + .inNamespace(namespace) + .withName(applied.getMetadata().getName()) + .waitUntilCondition( + r -> r.getStatus() != null && r.getStatus().getObservedGeneration() >= generation, + 5, + TimeUnit.SECONDS + ); + } +} diff --git a/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerTest.java b/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerTest.java index 89551fc..f78636a 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/crd/role/RoleReconcilerTest.java @@ -3,11 +3,11 @@ import io.fabric8.kubernetes.api.model.SecretBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.quarkus.test.junit.QuarkusTest; +import it.aboutbits.postgresql._support.PostgreSQLPasswordVerifier; import it.aboutbits.postgresql._support.testdata.base.TestUtil; import it.aboutbits.postgresql._support.testdata.persisted.Given; import it.aboutbits.postgresql.core.CRPhase; import it.aboutbits.postgresql.core.CRStatus; -import it.aboutbits.postgresql.core.PostgreSQLAuthenticationService; import it.aboutbits.postgresql.core.PostgreSQLContextFactory; import it.aboutbits.postgresql.core.ResourceRef; import lombok.RequiredArgsConstructor; @@ -32,10 +32,13 @@ import java.util.stream.Stream; import static it.aboutbits.postgresql.core.KubernetesService.SECRET_DATA_BASIC_AUTH_PASSWORD_KEY; -import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_AUTHID; +import static it.aboutbits.postgresql.core.infrastructure.persistence.Tables.PG_ROLES; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; +import static org.jooq.impl.DSL.inline; +import static org.jooq.impl.DSL.query; import static org.jooq.impl.DSL.role; +import static org.jooq.impl.DSL.val; @QuarkusTest @RequiredArgsConstructor @@ -45,7 +48,6 @@ class RoleReconcilerTest { private final RoleService roleService; private final PostgreSQLContextFactory postgreSQLContextFactory; - private final PostgreSQLAuthenticationService postgreSQLAuthenticationService; private final KubernetesClient kubernetesClient; @@ -55,7 +57,7 @@ void resetEnvironment() { } @Test - @DisplayName("When a Role (LOGIN) is created, it should be reconciled to READY and present in pg_authid") + @DisplayName("When a Role (LOGIN) is created, it should be reconciled to READY and present in pg_roles") void createRole_withLogin_andStatusReady() { // given var clusterConnection = given.one() @@ -75,7 +77,7 @@ void createRole_withLogin_andStatusReady() { .returnFirst(); // then: assert READY - var expectedStatus = new CRStatus() + var expectedStatus = new RoleStatus() .setName(roleName) .setPhase(CRPhase.READY) .setObservedGeneration(1L); @@ -90,6 +92,7 @@ void createRole_withLogin_andStatusReady() { assertThat(roleService.roleExists(dsl, role.getSpec())).isTrue(); assertThat(roleService.roleLoginMatches(dsl, role.getSpec())).isTrue(); + assertThat(role.getStatus().getPasswordFingerprint()).isNotBlank(); } @Test @@ -111,7 +114,7 @@ void createRole_withoutLogin_andStatusReady() { .withClusterConnectionName(clusterConnection.getMetadata().getName()) .returnFirst(); - var expectedStatus = new CRStatus() + var expectedStatus = new RoleStatus() .setName(roleName) .setPhase(CRPhase.READY) .setObservedGeneration(1L); @@ -126,10 +129,11 @@ void createRole_withoutLogin_andStatusReady() { assertThat(roleService.roleExists(dsl, role.getSpec())).isTrue(); assertThat(roleService.roleLoginMatches(dsl, role.getSpec())).isTrue(); + assertThat(role.getStatus().getPasswordFingerprint()).isNull(); } @Test - @DisplayName("When a Role login state is changed, it should be updated correctly in pg_authid") + @DisplayName("When a Role login state is changed, it should be updated correctly in pg_roles") void toggleRoleLogin_updatesCorrectly() { // given var clusterConnection = given.one() @@ -152,7 +156,7 @@ void toggleRoleLogin_updatesCorrectly() { // then assertThatRoleHasExpectedStatus( role, - new CRStatus() + new RoleStatus() .setName(roleName) .setPhase(CRPhase.READY) .setObservedGeneration(1L), @@ -166,7 +170,7 @@ void toggleRoleLogin_updatesCorrectly() { ).isTrue(); assertThat( - getRoleFlagValue(dsl, roleName, PG_AUTHID.ROLCANLOGIN) + getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCANLOGIN) ).isFalse(); // 2. Add a passwordSecretRef to make it a login role @@ -179,7 +183,7 @@ void toggleRoleLogin_updatesCorrectly() { ); // then - assertThat(getRoleFlagValue(dsl, roleName, PG_AUTHID.ROLCANLOGIN)).isTrue(); + assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCANLOGIN)).isTrue(); // 3. Remove passwordSecretRef again spec.setPasswordSecretRef(null); @@ -191,7 +195,49 @@ void toggleRoleLogin_updatesCorrectly() { ); // then - assertThat(getRoleFlagValue(dsl, roleName, PG_AUTHID.ROLCANLOGIN)).isFalse(); + assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCANLOGIN)).isFalse(); + } + + @Test + @DisplayName("When a NOLOGIN Role still holds a password, the next update should clear it") + void noLoginRole_withLeftoverPassword_clearsPassword() { + // given: a reconciled NOLOGIN role + var clusterConnection = given.one() + .clusterConnection() + .withName("test-connection-role-leftover-password") + .returnFirst(); + + var roleName = "test-role-leftover-password"; + + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(clusterConnection.getMetadata().getName()) + .returnFirst(); + + var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection); + + assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCANLOGIN)).isFalse(); + + // and: somebody sets a password directly in PostgreSQL. + // The role keeps NOLOGIN, so the login state still matches the spec. + // `pg_roles` masks `rolpassword`, so the operator cannot see that password. + dsl.execute(query( + "alter role {0} with password {1}", + role(roleName), + val("leftover-password") + )); + + assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isNotNull(); + + // when: an unrelated flag changes, so the operator alters the role + role.getSpec().getFlags().setCreatedb(true); + + applyRole(role); + + // then: the operator cleared the leftover password + assertThat(getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCREATEDB)).isTrue(); + assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isNull(); } @Test @@ -275,9 +321,9 @@ void secretChange_triggersReconciliation() { // Wait for password to match because reconciliation might take a bit await().atMost(5, TimeUnit.SECONDS) .pollInterval(100, TimeUnit.MILLISECONDS) - .until(() -> postgreSQLAuthenticationService.passwordMatches( + .until(() -> PostgreSQLPasswordVerifier.passwordMatches( dsl, - role.getSpec(), + role.getSpec().getName(), initialPassword )); @@ -295,9 +341,9 @@ void secretChange_triggersReconciliation() { // then: password should eventually match the new one await().atMost(5, TimeUnit.SECONDS) .pollInterval(100, TimeUnit.MILLISECONDS) - .until(() -> postgreSQLAuthenticationService.passwordMatches( + .until(() -> PostgreSQLPasswordVerifier.passwordMatches( dsl, - role.getSpec(), + role.getSpec().getName(), newPassword )); } @@ -343,9 +389,9 @@ void secretRefChange_triggersReconciliation() { // then: password should match the initial one await().atMost(5, TimeUnit.SECONDS) .pollInterval(100, TimeUnit.MILLISECONDS) - .until(() -> postgreSQLAuthenticationService.passwordMatches( + .until(() -> PostgreSQLPasswordVerifier.passwordMatches( dsl, - role.getSpec(), + role.getSpec().getName(), initialPassword )); @@ -357,13 +403,185 @@ void secretRefChange_triggersReconciliation() { // then: password should eventually match the new one await().atMost(5, TimeUnit.SECONDS) .pollInterval(100, TimeUnit.MILLISECONDS) - .until(() -> postgreSQLAuthenticationService.passwordMatches( + .until(() -> PostgreSQLPasswordVerifier.passwordMatches( dsl, - updatedRole.getSpec(), + updatedRole.getSpec().getName(), newPassword )); } + @Test + @DisplayName("When an existing Role has no password fingerprint in its status, the Secret password should be applied once") + void missingPasswordFingerprint_appliesSecretPasswordOnce() { + // given: a Role created by the operator + var clusterConnection = given.one() + .clusterConnection() + .withName("test-connection-role-fingerprint-upgrade") + .returnFirst(); + + var roleName = "test-role-fingerprint-upgrade"; + var password = "secret-password"; + + var secretRef = given.one() + .secretRef() + .withPassword(password) + .returnFirst(); + + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(clusterConnection.getMetadata().getName()) + .withPasswordSecretRef(secretRef) + .returnFirst(); + + var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection); + + var initialFingerprint = role.getStatus().getPasswordFingerprint(); + assertThat(initialFingerprint).isNotBlank(); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + dsl, + roleName, + password + )).isTrue(); + + // given: the password in PostgreSQL differs from the Secret, and the status has no fingerprint, + // like a Role that was reconciled by a version of the operator without fingerprints + dsl.execute("alter role {0} with password {1}", role(roleName), inline("out-of-band-password")); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + dsl, + roleName, + password + )).isFalse(); + + kubernetesClient.resources(Role.class) + .inNamespace(role.getMetadata().getNamespace()) + .withName(role.getMetadata().getName()) + .editStatus(current -> { + current.getStatus().setPasswordFingerprint(null); + return current; + }); + + // when: a spec change triggers the next reconcile + role.getSpec().setComment("triggers a reconcile"); + + var updatedRole = applyRole(role); + + // then: the Secret password is applied once and the fingerprint is set again + assertThat(updatedRole.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(updatedRole.getStatus().getPasswordFingerprint()).isEqualTo(initialFingerprint); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + dsl, + roleName, + password + )).isTrue(); + + // when: the next reconcile finds a matching fingerprint and leaves the password alone + var verifierAfterUpgrade = PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName); + + updatedRole.getSpec().setComment("triggers another reconcile"); + + updatedRole = applyRole(updatedRole); + + // then + assertThat(updatedRole.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(updatedRole.getStatus().getPasswordFingerprint()).isEqualTo(initialFingerprint); + assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isEqualTo(verifierAfterUpgrade); + } + + @Test + @DisplayName("When the Secret already contains a SCRAM-SHA-256 verifier, it should be stored verbatim") + void preHashedPassword_isStoredVerbatim() { + // given + var clusterConnection = given.one() + .clusterConnection() + .withName("test-connection-role-prehashed") + .returnFirst(); + + var roleName = "test-role-prehashed"; + + // Verifier for the password "abc", generated by PostgreSQL + var verifier = "SCRAM-SHA-256$4096:gxUQWxfrRYegSTNiHXFT+g==$lxMC2yO9Lx9gm2dgNPo/1Qar+pjAvxCP2VN4yPWYnzE=:0QzcS9VJHJszBq4vSce3n4M6NZmyWa1GWdkJDi8hRNc="; + + var secretRef = given.one() + .secretRef() + .withPassword(verifier) + .returnFirst(); + + // when + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(clusterConnection.getMetadata().getName()) + .withPasswordSecretRef(secretRef) + .returnFirst(); + + var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection); + + // then + assertThat(role.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isEqualTo(verifier); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + dsl, + roleName, + "abc" + )).isTrue(); + } + + @Test + @DisplayName("When passwordEncryption is 'server', the server should hash the password") + void serverPasswordEncryption_letsTheServerHash() { + // given + var clusterConnection = given.one() + .clusterConnection() + .withName("test-connection-role-server-encryption") + .returnFirst(); + + var roleName = "test-role-server-encryption"; + var password = "server-side-password"; + + var secretRef = given.one() + .secretRef() + .withPassword(password) + .returnFirst(); + + // when + var role = given.one() + .role() + .withName(roleName) + .withClusterConnectionName(clusterConnection.getMetadata().getName()) + .withPasswordSecretRef(secretRef) + .withPasswordEncryption(PasswordEncryption.SERVER) + .returnFirst(); + + var dsl = postgreSQLContextFactory.getDSLContext(clusterConnection); + + // then + assertThat(role.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + dsl, + roleName, + password + )).isTrue(); + + // when: switch to operator-side SCRAM, the password is re-applied with a fresh verifier + var previousVerifier = PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName); + var previousFingerprint = role.getStatus().getPasswordFingerprint(); + + role.getSpec().setPasswordEncryption(PasswordEncryption.SCRAM_SHA_256); + + var updatedRole = applyRole(role); + + // then + assertThat(updatedRole.getStatus().getPhase()).isEqualTo(CRPhase.READY); + assertThat(updatedRole.getStatus().getPasswordFingerprint()).isNotEqualTo(previousFingerprint); + assertThat(PostgreSQLPasswordVerifier.storedVerifier(dsl, roleName)).isNotEqualTo(previousVerifier); + assertThat(PostgreSQLPasswordVerifier.passwordMatches( + dsl, + roleName, + password + )).isTrue(); + } + @Test @DisplayName("When the comment is changed, it should be updated in the database") void comment_updatesCorrectly() { @@ -524,7 +742,7 @@ void connectionLimit_updatesCorrectly() { // then assertThat( - getRoleFlagValue(dsl, roleName, PG_AUTHID.ROLCONNLIMIT) + getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCONNLIMIT) ).isEqualTo(10); // 2. Change connection limit @@ -538,7 +756,7 @@ void connectionLimit_updatesCorrectly() { // then assertThat( - getRoleFlagValue(dsl, roleName, PG_AUTHID.ROLCONNLIMIT) + getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCONNLIMIT) ).isEqualTo(20); // 3. Reset connection limit to -1 @@ -552,7 +770,7 @@ void connectionLimit_updatesCorrectly() { // then assertThat( - getRoleFlagValue(dsl, roleName, PG_AUTHID.ROLCONNLIMIT) + getRoleFlagValue(dsl, roleName, PG_ROLES.ROLCONNLIMIT) ).isEqualTo(-1); } @@ -874,19 +1092,19 @@ void deleteRole_removesFromDatabase() { Field field ) { return dsl.select(field) - .from(PG_AUTHID) - .where(PG_AUTHID.ROLNAME.eq(roleName)) + .from(PG_ROLES) + .where(PG_ROLES.ROLNAME.eq(roleName)) .fetchSingle(field); } private static Stream provideBooleanFlags() { return Stream.of( - Arguments.of(PG_AUTHID.ROLSUPER, (BiConsumer) RoleSpec.Flags::setSuperuser), - Arguments.of(PG_AUTHID.ROLCREATEDB, (BiConsumer) RoleSpec.Flags::setCreatedb), - Arguments.of(PG_AUTHID.ROLCREATEROLE, (BiConsumer) RoleSpec.Flags::setCreaterole), - Arguments.of(PG_AUTHID.ROLINHERIT, (BiConsumer) RoleSpec.Flags::setInherit), - Arguments.of(PG_AUTHID.ROLREPLICATION, (BiConsumer) RoleSpec.Flags::setReplication), - Arguments.of(PG_AUTHID.ROLBYPASSRLS, (BiConsumer) RoleSpec.Flags::setBypassrls) + Arguments.of(PG_ROLES.ROLSUPER, (BiConsumer) RoleSpec.Flags::setSuperuser), + Arguments.of(PG_ROLES.ROLCREATEDB, (BiConsumer) RoleSpec.Flags::setCreatedb), + Arguments.of(PG_ROLES.ROLCREATEROLE, (BiConsumer) RoleSpec.Flags::setCreaterole), + Arguments.of(PG_ROLES.ROLINHERIT, (BiConsumer) RoleSpec.Flags::setInherit), + Arguments.of(PG_ROLES.ROLREPLICATION, (BiConsumer) RoleSpec.Flags::setReplication), + Arguments.of(PG_ROLES.ROLBYPASSRLS, (BiConsumer) RoleSpec.Flags::setBypassrls) ); } @@ -955,7 +1173,7 @@ private static void assertThatRoleHasExpectedStatus( ); }) .usingRecursiveComparison() - .ignoringFields("lastProbeTime", "lastPhaseTransitionTime") + .ignoringFields("lastProbeTime", "lastPhaseTransitionTime", "passwordFingerprint") .isEqualTo(expectedStatus); } } diff --git a/operator/src/test/java/it/aboutbits/postgresql/helm/HelmTest.java b/operator/src/test/java/it/aboutbits/postgresql/helm/HelmTest.java index 6e284a2..79298a2 100644 --- a/operator/src/test/java/it/aboutbits/postgresql/helm/HelmTest.java +++ b/operator/src/test/java/it/aboutbits/postgresql/helm/HelmTest.java @@ -156,6 +156,8 @@ void helmInstall_createsDeployment() throws IOException { assertThat(chartPath.resolve("templates/clusterrole.yaml")).exists(); assertThat(chartPath.resolve("templates/clusterrolebinding.yaml")).exists(); assertThat(chartPath.resolve("templates/deployment.yaml")).exists(); + // The namespaced Role that grants `create` on the password fingerprint key Secret + assertThat(chartPath.resolve("templates/role.yaml")).exists(); assertThat(chartPath.resolve("templates/rolebinding.yaml")).exists(); assertThat(chartPath.resolve("templates/service.yaml")).exists(); assertThat(chartPath.resolve("templates/serviceaccount.yaml")).exists();