From 1dd905dc5f3c0e4bc0329d7635ec284e410e10ab Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Tue, 28 Jul 2026 22:48:49 +0200 Subject: [PATCH] =?UTF-8?q?fix(pgbouncer,vault)=20=E2=80=94=20ce=20qu'un?= =?UTF-8?q?=20client=20laisse=20sur=20une=20connexion,=20le=20suivant=20n'?= =?UTF-8?q?en=20h=C3=A9rite=20plus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Un pgbouncer partagé n'a qu'UNE barrière entre deux clients d'un même pool : le `server_reset_query`. La nôtre ne nettoyait que les requêtes préparées. Tout le reste de l'état de session — variables, LISTEN, tables TEMP, et le rôle courant — traversait la déconnexion et tombait dans les mains du client suivant. CE QUI A ÉTÉ MESURÉ, PAS SUPPOSÉ pgbouncer 1.25.2 (la version qui tourne), monté avec NOTRE configuration : `pool_mode = session`, `server_reset_query = DEALLOCATE ALL`, `server_reset_query_always = 1`, `default_pool_size = 1`. Le client 1 pose son état puis se déconnecte ; le client 2, qui n'a rien demandé, arrive. DEALLOCATE ALL (l'existant) client 2 hérite : work_mem=17MB, 1 LISTEN, la table TEMP du client 1, et son SET ROLE — au point de créer des tables appartenant à un rôle qui n'est pas le sien. DISCARD ALL (ce commit) client 2 obtient work_mem=4MB, 0 LISTEN, pas de table TEMP, et son PROPRE rôle. POURQUOI `DISCARD ALL` NE PEUT RIEN CASSER C'est le défaut de pgbouncer, et un sur-ensemble STRICT des deux valeurs qui l'ont précédé ici : il contient `DEALLOCATE ALL` — donc le correctif crowdsec de 07e2c6d (« prepared statement already exists ») est conservé, et c'est vérifié : deux clients successifs préparent le même nom sans erreur — et il contient `SELECT pg_advisory_unlock_all()`, la valeur que pose le sous-chart. Et il ne touche jamais un client vivant : en `pool_mode = session` la connexion serveur n'est rendue qu'à la déconnexion, donc le reset ne court pas entre deux requêtes d'une même session. Vérifié : table TEMP, `SET work_mem` et `LISTEN` d'un client VIVANT survivent intacts. PORTÉE DE LA FUITE — ce qu'elle est, et ce qu'elle n'est pas Les pools de pgbouncer sont partitionnés par (base, utilisateur) : 210 pools mesurés sur l'instance, aucun ne mélange deux bases ni deux comptes. Un rôle posé par kadans ne peut donc PAS atterrir chez crowdsec ou plausible : le seul héritier possible est un client de la même base avec le même identifiant, donc l'application elle-même. Ce n'est pas une élévation de privilège entre applications — c'est un défaut d'hygiène, et il est déjà là aujourd'hui pour n'importe quel `SET` de n'importe quelle application. LA CEINTURE, CÔTÉ VAULT `ALTER ROLE "{{name}}" SET ROLE _role` dans les creation_statements fait de l'endossement un défaut de CONNEXION, immune au `pool_mode` comme au `server_reset_query`, et valable pour les clients qui ne sont pas l'application (le psql d'un job). Elle n'apporte aucun privilège : le `GRANT` de la ligne précédente rend déjà le rôle éphémère membre du rôle stable — d'où le fait qu'elle ne peut pas échouer là où le GRANT réussit. Elle règle à la racine ce que le CronJob `pg-fix-table-ownership` rattrape tous les jours à 03:00 : les objets naissent chez le rôle stable au lieu d'être réattribués après coup. ⚠ Elle ne vaut que pour les identifiants créés APRÈS l'apply — elle ne protège donc PAS ce soir ; c'est `DISCARD ALL` qui protège ce soir. Mesuré (PostgreSQL 16, compte CREATEROLE non-superutilisateur comme celui de Vault) : l'instruction passe, le login donne bien `current_role` = rôle stable avec `session_user` = rôle éphémère, un `ALTER ROLE … RESET role` la retire, et elle SURVIT à `RESET ALL` / `DISCARD ALL` — les deux mesures composent au lieu de s'annuler. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GdUCA5Uz8QyMwa2P4Pg2hK --- hashicorp-vault/iac/modules/app_roles/main.tf | 27 +++++++++++++++++++ pgbouncer/values.yaml | 25 ++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/hashicorp-vault/iac/modules/app_roles/main.tf b/hashicorp-vault/iac/modules/app_roles/main.tf index ee8b766..0d30327 100644 --- a/hashicorp-vault/iac/modules/app_roles/main.tf +++ b/hashicorp-vault/iac/modules/app_roles/main.tf @@ -30,9 +30,36 @@ resource "vault_database_secret_backend_role" "role" { backend = local.vault_mount_postgres.path name = local.instance db_name = "postgres" + # ── Le rôle ÉPHÉMÈRE endosse le rôle STABLE, dès le login ─────────────────── + # + # En PostgreSQL, un objet appartient au rôle qui l'a CRÉÉ. Comme chaque + # démarrage de pod obtient un rôle `v-kubernet-…` neuf, toute migration crée + # des objets que le pod SUIVANT ne peut plus lire. C'est ce qui a mis l'API + # kadans à terre une demi-journée le 2026-07-28 (« permission denied for table + # qualification_video »), et c'est ce que le CronJob `pg-fix-table-ownership` + # rattrape tous les jours à 03:00 — a posteriori, et pour les seules TABLES. + # + # `ALTER ROLE … SET ROLE` fait de l'endossement un DÉFAUT DE CONNEXION : plus + # rien à poser côté application, et ça vaut aussi pour les clients qui ne sont + # pas l'application (le `psql` d'un job, une console d'exploitation). + # + # AUCUN privilège nouveau : le `GRANT` de la ligne précédente rend déjà le + # rôle éphémère MEMBRE du rôle stable. Endosser une casquette qu'on porte + # déjà, ce n'est pas une élévation — et c'est pourquoi cette instruction ne + # peut pas échouer là où le `GRANT` réussit. + # + # MESURÉ (PostgreSQL 16, compte CREATEROLE non-superutilisateur, comme celui + # de Vault) : l'instruction passe, le login donne `session_user=v-test-1` / + # `current_role=proprio_v`, et un `ALTER ROLE … RESET role` la retire. + # Vérifié aussi qu'elle SURVIT à `RESET ALL` / `DISCARD ALL` — donc elle + # compose avec le `server_reset_query` de pgbouncer au lieu de s'y opposer. + # + # ⚠ Ne vaut que pour les identifiants créés APRÈS l'apply : les baux en cours + # gardent leur ancien comportement jusqu'à leur renouvellement. creation_statements = [ "CREATE ROLE \"{{name}}\" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';", "GRANT ${local.owner_role} TO \"{{name}}\";", + "ALTER ROLE \"{{name}}\" SET ROLE ${local.owner_role};", ] revocation_statements = [ "REASSIGN OWNED BY \"{{name}}\" TO ${local.owner_role};", # reassign must be executed in the database where the reassgined objects are - TODO (one connection per database/app) diff --git a/pgbouncer/values.yaml b/pgbouncer/values.yaml index 7e2f431..48295b6 100644 --- a/pgbouncer/values.yaml +++ b/pgbouncer/values.yaml @@ -14,7 +14,30 @@ pgbouncer: &pgbouncer_config auth_type: scram-sha-256 auth_query: SELECT uname, phash FROM user_lookup($1) ignore_startup_parameters: extra_float_digits # unsupported jdbc extra_float_digits=2 argument - server_reset_query: DEALLOCATE ALL # fix prepared statement already exist (crowdsec) + # Ce pgbouncer est PARTAGÉ (crowdsec, plausible, kadans, + le compte que + # Vault utilise sur la base `postgres`). Ce qu'un client laisse derrière + # lui sur une connexion serveur, le client SUIVANT en hérite : le reset + # est la SEULE barrière entre deux clients d'un même pool. + # + # `DEALLOCATE ALL` ne nettoie QUE les requêtes préparées — d'où sa mise en + # place (07e2c6d, « prepared statement already exists » de crowdsec). + # Tout le reste de l'état de session passait au suivant. MESURÉ contre un + # pgbouncer 1.25.2 monté avec CETTE configuration (session, pool_size=1) : + # le client 2, qui n'avait rien demandé, héritait de `work_mem=17MB`, du + # `LISTEN canal_test` du client 1, de sa table TEMP — et de son `SET ROLE`, + # au point de créer des tables appartenant à un autre rôle que le sien. + # + # `DISCARD ALL` est le défaut de pgbouncer, et c'est un SUR-ENSEMBLE strict + # des deux valeurs qui l'ont précédé ici : il contient `DEALLOCATE ALL` + # (donc le correctif crowdsec est conservé — vérifié : deux clients + # successifs préparent le même nom sans erreur) ET + # `SELECT pg_advisory_unlock_all()` (la valeur que pose le sous-chart). + # + # Il ne peut RIEN casser pour un client vivant : en `pool_mode = session` + # la connexion serveur n'est rendue qu'à la déconnexion du client, donc le + # reset ne court jamais entre deux requêtes d'une même session. Vérifié : + # table TEMP, `SET work_mem` et `LISTEN` d'un client VIVANT survivent. + server_reset_query: DISCARD ALL # défaut pgbouncer — ⚠ ne pas réduire : voir ci-dessus server_idle_timeout: 7200 pgbouncerExporter: enabled: false