fix(pgbouncer,vault) — ce qu'un client laisse sur une connexion, le suivant n'en hérite plus #25

Merged

arcodange merged 2 commits from arcodange/fuite-set-role into main 2026-07-29 20:22:14 +02:00

Conversation 1 Commits 2 Files Changed 2

+84 -1

2 Commits

Author	SHA1	Message	Date
arcodangeandClaude Opus 5	7d13a8d764	doc(pgbouncer) — la version mesurée n'était pas la déployée, et DISCARD ALL a un prix ... 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 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdUCA5Uz8QyMwa2P4Pg2hK	2026-07-28 23:23:18 +02:00
arcodangeandClaude Opus 5	1dd905dc5f	fix(pgbouncer,vault) — ce qu'un client laisse sur une connexion, le suivant n'en hérite plus ... 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 <app>_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 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdUCA5Uz8QyMwa2P4Pg2hK	2026-07-28 22:48:49 +02:00