From 1dd905dc5f3c0e4bc0329d7635ec284e410e10ab Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Tue, 28 Jul 2026 22:48:49 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(pgbouncer,vault)=20=E2=80=94=20ce=20qu'?= =?UTF-8?q?un=20client=20laisse=20sur=20une=20connexion,=20le=20suivant=20?= =?UTF-8?q?n'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 From 7d13a8d76415506f3c69e7ece2a06a28906d16e2 Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Tue, 28 Jul 2026 23:23:18 +0200 Subject: [PATCH 2/2] =?UTF-8?q?doc(pgbouncer)=20=E2=80=94=20la=20version?= =?UTF-8?q?=20mesur=C3=A9e=20n'=C3=A9tait=20pas=20la=20d=C3=A9ploy=C3=A9e,?= =?UTF-8?q?=20et=20DISCARD=20ALL=20a=20un=20prix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trois corrections au raisonnement, toutes issues d'une contre-vérification qui a refait les mesures au lieu de les relire. 1. La mesure invoquait pgbouncer 1.25.2 « la version en prod ». Le pod tourne 1.23.1 (ghcr.io/icoretech/pgbouncer-docker:1.23.1-fixed, chart 2.3.1, up 133 j). Tout a été rejoué sur 1.23.1 avec la ConfigMap extraite du cluster : les conclusions tiennent, mais la preuve invoquée portait sur autre chose. 2. La portée de la fuite était laissée en suspens, donc lue au pire. Les pools sont partitionnés par (base, utilisateur) — 210 pools, aucun mélange. Le seul héritier possible du SET ROLE de kadans-api est kadans-api. Défaut d'hygiène, pas brèche inter-applications. Le dire baisse la gravité, et c'est plus utile qu'une alarme vague. 3. ⚠ Le point manquant, et c'est l'inverse de ce qu'on croyait : DISCARD ALL rend la protection de db.go STRICTEMENT PLUS FRAGILE. Aujourd'hui (DEALLOCATE ALL) elle survivrait à un passage en transaction pooling ; après, non — le SET ROLE d'AfterConnect est effacé entre deux transactions, et poserRoleProprietaire ne peut pas le voir puisqu'il relit current_role à l'ouverture. Sans danger tant que pool_mode reste `session` (le défaut, non déclaré) — mais c'est précisément le genre de trappe qui se referme des mois plus tard sur quelqu'un qui optimise. Les quatre combinaisons sont mesurées et écrites au-dessus de la ligne. La ceinture Vault couvre ce cas (un défaut de rôle survit à DISCARD ALL), mais seulement au renouvellement des baux. D'où l'avertissement explicite. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GdUCA5Uz8QyMwa2P4Pg2hK --- pgbouncer/values.yaml | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/pgbouncer/values.yaml b/pgbouncer/values.yaml index 48295b6..314018f 100644 --- a/pgbouncer/values.yaml +++ b/pgbouncer/values.yaml @@ -22,10 +22,20 @@ pgbouncer: &pgbouncer_config # `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. + # pgbouncer **1.23.1** — la version RÉELLEMENT déployée (image + # ghcr.io/icoretech/pgbouncer-docker:1.23.1-fixed, chart pgbouncer-2.3.1), + # montée avec CETTE configuration extraite du cluster (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. + # + # Portée de la fuite, mesurée et non supposée : les pools sont partitionnés + # par (base, utilisateur) — 210 pools, aucun ne mélange deux bases ni deux + # comptes. Le seul héritier possible d'un `SET ROLE` posé par kadans-api + # est un client de la base `kadans` avec le MÊME identifiant éphémère, + # c'est-à-dire kadans-api elle-même. Défaut d'hygiène, pas brèche + # inter-applications. # # `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` @@ -37,6 +47,29 @@ pgbouncer: &pgbouncer_config # 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. + # + # ⚠⚠ CE QUE CETTE LIGNE REND FRAGILE — et c'est l'inverse de ce qu'on croit. + # + # `pool_mode` n'est PAS déclaré ici, donc il vaut `session`, le défaut. + # C'est ce qui rend `DISCARD ALL` sans danger. **Le jour où quelqu'un + # écrira `pool_mode: transaction`, il cassera kadans-api en silence** : + # son `SET ROLE` est posé UNE FOIS à l'ouverture (`AfterConnect`, db.go), + # et en transaction pooling `DISCARD ALL` court ENTRE deux transactions — + # donc le rôle est effacé avant les migrations suivantes. MESURÉ, les + # quatre combinaisons, propriétaire de la table créée : + # + # session + DEALLOCATE ALL → rôle stable ✅ (mais la fuite reste) + # session + DISCARD ALL → rôle stable ✅ ← ce qu'on déploie + # transaction + DEALLOCATE ALL → rôle stable ✅ (par ACCIDENT : la fuite + # qu'on referme est ce qui le sauvait) + # transaction + DISCARD ALL → rôle ÉPHÉMÈRE ❌ le défaut du 28/07, + # qui a mis l'API à terre une demi-journée + # + # Et `poserRoleProprietaire` ne peut pas le voir : sa relecture de + # `current_role` a lieu à l'ouverture, où le rôle est encore correct. + # Ce qui couvre ce cas, c'est la ceinture Vault (`ALTER ROLE … SET ROLE`, + # module app_roles) : un défaut de rôle survit à `DISCARD ALL`. **Avant de + # passer en transaction pooling, vérifier que les baux Vault ont tourné.** server_reset_query: DISCARD ALL # défaut pgbouncer — ⚠ ne pas réduire : voir ci-dessus server_idle_timeout: 7200 pgbouncerExporter: