Issues #5059 #4927 #3824 et #2109 : on reforme la logique du login #5064

Merged
marcimat merged 17 commits from refactor_auth_spip into master 6 months ago
Owner
  • plus de hashage+sel cote client, car les algos js sont penibles a gerer, voire buggués, et https est maintenant un standard de securite
  • cote serveur on utilise les fonction modernes de PHP pour la gestion des mots de passe (sel, poivre, hashage et verification)

Directement inspiré du plugin chiffrer de g0uz https://git.spip.net/spip-contrib-extensions/chiffrer avec en plus:

- plus de hashage+sel cote client, car les algos js sont penibles a gerer, voire buggués, et https est maintenant un standard de securite - cote serveur on utilise les fonction modernes de PHP pour la gestion des mots de passe (sel, poivre, hashage et verification) Directement inspiré du plugin chiffrer de g0uz https://git.spip.net/spip-contrib-extensions/chiffrer avec en plus: * utilisation de sodium qui est natif avec PHP depuis 7.2 https://php.watch/articles/modern-php-encryption-decryption-sodium * la gestion de plusieurs cles, dont aussi le secret_du_site qui n'est plus en meta * une passe de modernisation sur les cles d'action egalement
marcimat added 13 commits 6 months ago
c1f063446e Issues #5059 #4927 #3824 et #2109 : on reforme la logique du login
3c5fa1b88e Refactoring de Chiffrer en découppant en plus d’éléments
35c65d7ac2 Etre tres prudent sur la generation d'un nouveau secret_des_auth : cela ne peut avoir lieu que si on a plus aucun backup d'aucun webmestre
997f176a6f Champ `backup_cles` sur spip_auteurs, et `secret_du_site` hors de spip_meta
cf5f87ecc3 Gestion de l'installation :
1cacf8ae25 Chiffrement plus poussé : pad / unpad du message + hash si password (g0uZ)
marcimat added 1 commit 6 months ago
48f76808bf Suite de 23994e0b1 : Ne pas mettre de jeton d’url directement en bdd (cas de l’inscription d’auteur ou du changement de mot de passe)

Excellent, merci marcimat pour le dernier commit, j'ai relu et je pense que ça couvre bien les besoins en notifs !

Pour la suite (dans des versions ultérieures) :

Notons que ces histoires de jeton mériteraient quelques améliorations

  • permettre l’expiration des jetons
  • permettre plusieurs jetons pour un même auteur directement

Il me semble que pour ça, il faudrait une "table des clés", ou "table des jetons" (je sais pas si cette table ne serait que pour les auteurs, mais au moins pour ça), qui contiendrait à minima : id_auteur, cle, date_creation
Ce qui permettrait bien :

  • plusieurs clés par compte (et donc PLUS besoin de chercher l'ancienne clé ! on peut alors en générer une nouvelle différente à chaque notification, ce qui est encore mieux)
  • avec la date, de ne PAS dire qu'un jeton est valide, si on a dépassé X temps (défini dans un define par ex ou autre), cf le ticket : #4869 (ça pourrait être fait déjà même avec un seul jeton en ajoutant un champ dans spip_auteurs, mais peut-être que tant qu'à faire autant attendre une table multi ?)
Excellent, merci marcimat pour le dernier commit, j'ai relu et je pense que ça couvre bien les besoins en notifs ! Pour la suite (dans des versions ultérieures) : > Notons que ces histoires de jeton mériteraient quelques améliorations > - permettre l’expiration des jetons > - permettre plusieurs jetons pour un même auteur directement Il me semble que pour ça, il faudrait une "table des clés", ou "table des jetons" (je sais pas si cette table ne serait que pour les auteurs, mais au moins pour ça), qui contiendrait à minima : id_auteur, cle, date_creation Ce qui permettrait bien : - plusieurs clés par compte (et donc PLUS besoin de chercher l'ancienne clé ! on peut alors en générer une nouvelle différente à chaque notification, ce qui est encore mieux) - avec la date, de ne PAS dire qu'un jeton est valide, si on a dépassé X temps (défini dans un define par ex ou autre), cf le ticket : https://git.spip.net/spip/spip/issues/4869 (ça pourrait être fait déjà même avec un seul jeton en ajoutant un champ dans spip_auteurs, mais peut-être que tant qu'à faire autant attendre une table multi ?)
marcimat added 1 commit 6 months ago
3ba4787877 Sécurité des authentifications avec le secret du site (g0uZ) :
Poster
Owner

J’ai tenté de suivre les recommandations de g0oZ qui disait que c’était dommage de mettre la clé secret_du_site sur le disque car si qqn·e arrive à l’afficher il peut alors générer certaines clés d’authentifications.

De la même manière si qqn·e peut lire la Bdd il peut aussi finalement…

Du coup, on s’est dit avec @cerdic que découper la clé en 2 était une solution convenable : il faut les 2 parties pour reconstituer la clé valide.

C’est ce que j’ai fait dans 3ba4787877

Cependant…
Il y a un hic.
La table spip_meta se sauvegarde (en clair) dans tmp/meta_cache.php

Et donc qqn·e avec un accès disque peut obtenir les 2 parties quand même pour le momment.

Que faire alors ?

J’ai tenté de suivre les recommandations de g0oZ qui disait que c’était dommage de mettre la clé secret_du_site sur le disque car si qqn·e arrive à l’afficher il peut alors générer certaines clés d’authentifications. De la même manière si qqn·e peut lire la Bdd il peut aussi finalement… Du coup, on s’est dit avec @cerdic que découper la clé en 2 était une solution convenable : il faut les 2 parties pour reconstituer la clé valide. C’est ce que j’ai fait dans 3ba4787877 Cependant… Il y a un hic. La table spip_meta se sauvegarde (en clair) dans tmp/meta_cache.php Et donc qqn·e avec un accès disque peut obtenir les 2 parties quand même pour le momment. Que faire alors ?

Pour avoir rapidement regardé, la solution la plus propre (pour avancer avec l'existant sans tout revoir) me semble être de ne pas écrire cette valeure dans meta/cache.php.

Ce cas a déjà été prévu mais commenté dans la fonction touch_meta() :

https://github.com/spip/SPIP/blob/master/ecrire/inc/meta.php#L137


			// le secret du site est utilise pour encoder les contextes ajax que l'on considere fiables
			// mais le sortir deu cache meta implique une requete sql des qu'on a un form dynamique
			// meme si son squelette est en cache
			//unset($r['secret_du_site']);
            
            

J'ai également cherché au sein du core et des plugins les éventuelles utilisations direct de $GLOBALS['meta']['secret_du_site']. Seul le plugin mutualisation devrait être adapté : https://git.spip.net/spip-contrib-extensions/mutualisation/search?q=secret_du_site

Nous avons du mal a comprendre le commentaire et l'impact de la sortie du cache de cette valeur, Cédric tu saurais nous aider ?

Pour avoir rapidement regardé, la solution la plus propre (pour avancer avec l'existant sans tout revoir) me semble être de ne pas écrire cette valeure dans meta/cache.php. Ce cas a déjà été prévu mais commenté dans la fonction touch_meta() : https://github.com/spip/SPIP/blob/master/ecrire/inc/meta.php#L137 ``` // le secret du site est utilise pour encoder les contextes ajax que l'on considere fiables // mais le sortir deu cache meta implique une requete sql des qu'on a un form dynamique // meme si son squelette est en cache //unset($r['secret_du_site']); ``` J'ai également cherché au sein du core et des plugins les éventuelles utilisations direct de $GLOBALS['meta']['secret_du_site']. Seul le plugin mutualisation devrait être adapté : https://git.spip.net/spip-contrib-extensions/mutualisation/search?q=secret_du_site Nous avons du mal a comprendre le commentaire et l'impact de la sortie du cache de cette valeur, Cédric tu saurais nous aider ?
Poster
Owner

Bon.

J’ai vaguement l’impression que ça nécessite des réflexions complémentaires et des tests plus poussés et va être sujet à des corrections encore, et que ça repousserait d’autant la sortie de SPIP 4.1, ce qui me parait là assez dommage compte tenu qu’elle était prête.

Alors proposition : Est-ce que ça peut attendre une 4.2, qui sortirait disons fin avril après avoir validé et testé cette PR et éventuellement celle sur l’introduction de composer #5056 qui a déjà une base intéressante ?

Une version intermédiaire donc avant une éventuelle 4.3 ensuite début de l’été ?

Des avis / envies ou pas ?
Est-il préférable de reporter plutôt la 4.1 pour intégrer (et tester cette partie) ?

Bon. J’ai vaguement l’impression que ça nécessite des réflexions complémentaires et des tests plus poussés et va être sujet à des corrections encore, et que ça repousserait d’autant la sortie de SPIP 4.1, ce qui me parait là assez dommage compte tenu qu’elle était prête. Alors proposition : Est-ce que ça peut attendre une 4.2, qui sortirait disons fin avril après avoir validé et testé cette PR et éventuellement celle sur l’introduction de composer #5056 qui a déjà une base intéressante ? Une version intermédiaire donc avant une éventuelle 4.3 ensuite début de l’été ? Des avis / envies ou pas ? Est-il préférable de reporter plutôt la 4.1 pour intégrer (et tester cette partie) ?
marcimat added 15 commits 6 months ago
fdec8d5177 Issues #5059 #4927 #3824 et #2109 : on reforme la logique du login
2d8cbe8861 Refactoring de Chiffrer en découppant en plus d’éléments
1d5223d18a Etre tres prudent sur la generation d'un nouveau secret_des_auth : cela ne peut avoir lieu que si on a plus aucun backup d'aucun webmestre
29e5fe09b9 Champ `backup_cles` sur spip_auteurs, et `secret_du_site` hors de spip_meta
6e86e5469a Gestion de l'installation :
d5960cf956 Chiffrement plus poussé : pad / unpad du message + hash si password (g0uZ)
437adec529 Suite de 23994e0b1 : Ne pas mettre de jeton d’url directement en bdd (cas de l’inscription d’auteur ou du changement de mot de passe)
f9424719aa Sécurité des authentifications avec le secret du site (g0uZ) :
Owner

Alors proposition : Est-ce que ça peut attendre une 4.2, qui sortirait disons fin avril après avoir validé et testé cette PR et éventuellement celle sur l’introduction de composer #5056 qui a déjà une base intéressante ?

+1 ça serait dommage de continuer à retarder la 4.1 à cause de cette "grosse évolution" qui arrive un peu sur le tard (sans remettre en cause son intérêt).

> Alors proposition : Est-ce que ça peut attendre une 4.2, qui sortirait disons fin avril après avoir validé et testé cette PR et éventuellement celle sur l’introduction de composer #5056 qui a déjà une base intéressante ? > +1 ça serait dommage de continuer à retarder la 4.1 à cause de cette "grosse évolution" qui arrive un peu sur le tard (sans remettre en cause son intérêt).
Owner

Je ne vois pas d'inconvénient à ce que cette évolution intègre la 4.2 en juillet prochain. Ce n'est peut-être pas utile de rajouter une 4.3 pour ça

Je ne vois pas d'inconvénient à ce que cette évolution intègre la 4.2 en juillet prochain. Ce n'est peut-être pas utile de rajouter une 4.3 pour ça

Dommage, a 2 chars du push ^^

Plus sérieusement je vois pas de soucis à repousser.

A coté de ca c'est dommage de différer a cause de l'existence d'un chemin (avec un prérequis important) permettant de retrouver la clé complète. Il y en a certainement d'autres a trouver/fixer ; et amha ce n'est pas l'objectif initial.

Ca pourrait/devrait faire l'objet d'un ticket différent de la refonte du login non (indépendamment du quand) ?

Trouver tous les chemins permettant d'arriver a éxecuter du code avec un pré-requis type "je peux lire les fichiers sur le serveur" risque d'être long :)

Dommage, a 2 chars du push ^^ Plus sérieusement je vois pas de soucis à repousser. A coté de ca c'est dommage de différer a cause de l'existence d'un chemin (avec un prérequis important) permettant de retrouver la clé complète. Il y en a certainement d'autres a trouver/fixer ; et amha ce n'est pas l'objectif initial. Ca pourrait/devrait faire l'objet d'un ticket différent de la refonte du login non (indépendamment du quand) ? Trouver tous les chemins permettant d'arriver a éxecuter du code avec un pré-requis type "je peux lire les fichiers sur le serveur" risque d'être long :)
Owner

Je rappelle que ce n'est pas juste une "évolution" mais un fix de #5059 : si vous voulez repousser cette PR, ça veut dire qu'il faut proposer un autre fix car on va pas décemment releaser une version avec un hash 256 en JS qu'on sait buggué.

Cette course à la release à tout prix me semble un peu ridicule...

Concernant cette PR, après relecture des derniers commentaires, il me semble qu'on est bon jusqu'à f9424719aa

Si l'on exclue ce commit on a : un fix de #5059, et une remise au propre de l'authentification pour être conforme à l'état de l'art, ce qui traite #4927 #3824 et #2109.

Sur le point particulier du secret_du_site() on ne dégrade rien par rapport à l'existant puisqu'il était dejà stocké sur disque dans tmp/meta_cache.php et donc un accès au disque suffisait à obtenir le secret pour forger des contextes ajax et de formulaires.

  • dans le cas des contextes ajax cela permettrait potentiellement de faire des inclusions arbitraires, mais si tu as accès au disque tu peux déjà probablement faire ça
  • dans le cas des formulaires, la gravité est fortement réduite depuis que les formulaires sont signés pour chaque session auteur. Tu ne peux donc forger que des formulaires anonymes ou pour ta propre session

@g0uZ la raison pour laquelle on a remis le secret_du_site dans le cache des meta, est qu'il est utilisé pour calculer les env des formulaires, lesquels sont la plupart du temps dynamique. Ça veut dire que si tu as un formulaire dans la page (par exemple de notation, de forum), le simple affichage du formulaire va requerir une requete en base, et donc on casse la feature de pouvoir afficher une page en cache sans requete en base, ce qui a un gros impact performance également.

Je rappelle que ce n'est pas juste une "évolution" mais un fix de #5059 : si vous voulez repousser cette PR, ça veut dire qu'il faut proposer un autre fix car on va pas décemment releaser une version avec un hash 256 en JS qu'on sait buggué. Cette course à la release à tout prix me semble un peu ridicule... Concernant cette PR, après relecture des derniers commentaires, il me semble qu'on est bon jusqu'à f9424719aa5e1b758bcd588c0c4a96e5c8bc2887 Si l'on exclue ce commit on a : un fix de #5059, et une remise au propre de l'authentification pour être conforme à l'état de l'art, ce qui traite #4927 #3824 et #2109. Sur le point particulier du secret_du_site() on ne dégrade rien par rapport à l'existant puisqu'il était *dejà* stocké sur disque dans `tmp/meta_cache.php` et donc un accès au disque suffisait à obtenir le secret pour forger des contextes ajax et de formulaires. * dans le cas des contextes ajax cela permettrait potentiellement de faire des inclusions arbitraires, mais si tu as accès au disque tu peux déjà probablement faire ça * dans le cas des formulaires, la gravité est fortement réduite depuis que les formulaires sont signés pour chaque session auteur. Tu ne peux donc forger que des formulaires anonymes ou pour ta propre session @g0uZ la raison pour laquelle on a remis le secret_du_site dans le cache des meta, est qu'il est utilisé pour calculer les env des formulaires, lesquels sont la plupart du temps dynamique. Ça veut dire que si tu as un formulaire dans la page (par exemple de notation, de forum), le simple affichage du formulaire va requerir une requete en base, et donc on casse la feature de pouvoir afficher une page en cache sans requete en base, ce qui a un gros impact performance également.

Une autre proposition pour protéger globalement tmp/meta_cache.php (ce fichier contient beaucoup d'information qui mériterait d'être protégé) d'un attaquant qui peut lire des fichiers serait de le stocker avec un nom non prédictible (et qui n'implique pas une requête en DB pour le reconstituer).

Est ce qu'on peut trouver une valeure sécrète et constante (qui change rarement) uniquement récupérable via PHP, très rapidement, qu'on pourrait utiliser pour stocker tmp/meta_cache_[md5(mavaleure)].php ?

gouz@T530 /home/gouz $ time php -r 'for ($i=0; $i< 1000;$i++) { $r=md5(serialize(ini_get_all())); }'

real    0m0,119s

C'est peut être une très mauvaise idée, je ne sais pas bien :)

Si bonne idée, l'implémentation peut être assez rapide si on conserve tmp/meta_cache.php qui contiendrait uniquement le code suivant :


<?php

$token=md5(serialize(ini_get_all()));
include("meta_cache_".$token.".php");
Une autre proposition pour protéger globalement tmp/meta_cache.php (ce fichier contient beaucoup d'information qui mériterait d'être protégé) d'un attaquant qui peut lire des fichiers serait de le stocker avec un nom non prédictible (et qui n'implique pas une requête en DB pour le reconstituer). Est ce qu'on peut trouver une valeure sécrète et constante (qui change rarement) uniquement récupérable via PHP, très rapidement, qu'on pourrait utiliser pour stocker tmp/meta_cache_[md5(mavaleure)].php ? ``` gouz@T530 /home/gouz $ time php -r 'for ($i=0; $i< 1000;$i++) { $r=md5(serialize(ini_get_all())); }' real 0m0,119s ``` C'est peut être une très mauvaise idée, je ne sais pas bien :) Si bonne idée, l'implémentation peut être assez rapide si on conserve tmp/meta_cache.php qui contiendrait uniquement le code suivant : ``` <?php $token=md5(serialize(ini_get_all())); include("meta_cache_".$token.".php"); ```
Owner

@g0uZ j'ai répondu dans #5066 pour ne pas mélanger les sujets

@g0uZ j'ai répondu dans #5066 pour ne pas mélanger les sujets
Poster
Owner

Je rappelle que ce n'est pas juste une "évolution" mais un fix de #5059

À ce que je sache la 4.0 et la 3.2 auront toujours le bug (cette PR ne sera pas reportée). Et des bugs / évolutions à traiter c’est pas ce qui manque à côté non plus.

Je ne cherche pas à minimiser le problème que ça corrige ; juste que les corrections mêmes apportées par ce ticket nécessitent manifestement quelques tests (en passant d’ailleurs on n’a a pas encore fait de TU dessus, ce qui serait pourtant une bonne idée).

gOuZ signale au fur et à mesure des améliorations à apporter, à juste titre, et en soit par conséquent, on voit que ça ne semble pas parfaitement mature. Je ne pense pas avoir manqué de mauvaise volonté sur ce ticket.

Je vois juste que ça va reporter encore, parce que "profitons de cette sortie qui arrive pour faire xxx" (c’est vite fait) :)

Cette course à la release à tout prix me semble un peu ridicule...

Soit.

Je vais essayer de réexprimer mes pensées :

  1. on avait essayé de se mettre d’accord sur un calendrier. J’ai essayé de le tenir, (et d’adapter constamment mes scripts de release entre autres), mais peut être que ça peut pas marcher finalement ? Ou peut être que ça convient pas ?

  2. définir un calendrier évite (normallement) justement ce genre de situation : ne pas intégrer au dernier moment une évolution qui débarque sans prévenir et repousse la sortie : parce que ça peut durer longtemps et être sans fin (c’est pas comme si on savait pas… )

  3. on n’est pas forcé d’avoir toujours des grosses releases avec plein de features dedans. Des petites releases X.Y plus fréquentes avec moins de delta, ça peut aussi fonctionner a priori aussi. C’est pour ça que j’ai tenté de proposer de décaler ce ticket.

Maintenant osef, j’ai pas envie de batailler sur ça, on peut attendre pour de l’intégrer et de tester pour la 4.1

Et sortir la 4.1 «quand elle sera prête». C’est bien cette date là, au moins ça ennuira personne.

Bah vous n’aurez qu’à me dire quand ce sera le cas, quand faut releaser et quoi…
Sinon vous savez aussi où sont mes scripts de release.


L’idée de hacher le chemin tmp/cache/meta/meta_{hash}.php me semble intéressant. Reste à trouver avec quoi calculer le hash par exemple

(mais ça veut dire aussi supprimer les vieux fichiers à un moment dans ce cas non ?)

> Je rappelle que ce n'est pas juste une "évolution" mais un fix de #5059 À ce que je sache la 4.0 et la 3.2 auront toujours le bug (cette PR ne sera pas reportée). Et des bugs / évolutions à traiter c’est pas ce qui manque à côté non plus. Je ne cherche pas à minimiser le problème que ça corrige ; juste que les corrections mêmes apportées par ce ticket nécessitent manifestement quelques tests (en passant d’ailleurs on n’a a pas encore fait de TU dessus, ce qui serait pourtant une bonne idée). gOuZ signale au fur et à mesure des améliorations à apporter, à juste titre, et en soit par conséquent, on voit que ça ne semble pas parfaitement mature. Je ne pense pas avoir manqué de mauvaise volonté sur ce ticket. Je vois juste que ça va reporter encore, parce que "profitons de cette sortie qui arrive pour faire xxx" (c’est vite fait) :) > Cette course à la release à tout prix me semble un peu ridicule... Soit. Je vais essayer de réexprimer mes pensées : 1) on avait essayé de se mettre d’accord sur un calendrier. J’ai essayé de le tenir, (et d’adapter constamment mes scripts de release entre autres), mais peut être que ça peut pas marcher finalement ? Ou peut être que ça convient pas ? 2) définir un calendrier évite (normallement) justement ce genre de situation : ne pas intégrer au dernier moment une évolution qui débarque sans prévenir et repousse la sortie : parce que ça peut durer longtemps et être sans fin (c’est pas comme si on savait pas… ) 3) on n’est pas forcé d’avoir toujours des grosses releases avec plein de features dedans. Des petites releases X.Y plus fréquentes avec moins de delta, ça peut aussi fonctionner a priori aussi. C’est pour ça que j’ai tenté de proposer de décaler ce ticket. Maintenant osef, j’ai pas envie de batailler sur ça, on peut attendre pour de l’intégrer et de tester pour la 4.1 Et sortir la 4.1 «quand elle sera prête». C’est bien cette date là, au moins ça ennuira personne. Bah vous n’aurez qu’à me dire quand ce sera le cas, quand faut releaser et quoi… Sinon vous savez aussi où sont mes scripts de release. ---- L’idée de hacher le chemin tmp/cache/meta/meta_{hash}.php me semble intéressant. Reste à trouver avec quoi calculer le hash par exemple (mais ça veut dire aussi supprimer les vieux fichiers à un moment dans ce cas non ?)
Owner

Je pense qu'il y a une marge entre dire "OK ce bug ne sera pas fixé sur les versions anciennes." et "OK on release une nouvelle version qui contient toujours ce bug"...

Il s'agit pas de reporter sans cesse. Encore une fois, hormis le dernier commit qui traite un problème qu'on peut traiter plus tard, tout semble bon.

Je propose donc d'intégrer sans ce dernier commit et pour ce faire, je peux refaire une passe de tests au préalable et passer en live sur contrib pour vérifier qu'on est bon avant de merger et release...

(et je suis tout à fait d'accord qu'il faudrait aussi faire des tests unitaires, mais considérons que ce n'est pas bloquant et qu'on peut finir ça ensuite)

Je pense qu'il y a une marge entre dire "OK ce bug ne sera pas fixé sur les versions anciennes." et "OK on release une nouvelle version qui contient toujours ce bug"... Il s'agit pas de reporter sans cesse. Encore une fois, hormis le dernier commit qui traite un problème qu'on peut traiter plus tard, tout semble bon. Je propose donc d'intégrer sans ce dernier commit et pour ce faire, je peux refaire une passe de tests au préalable et passer en live sur contrib pour vérifier qu'on est bon avant de merger et release... (et je suis tout à fait d'accord qu'il faudrait aussi faire des tests unitaires, mais considérons que ce n'est pas bloquant et qu'on peut finir ça ensuite)
Owner

(pour les anciennes versions 3.2, on peut proposer le palliatif d'utiliser la constante _AUTORISER_AUTH_FAIBLE qui permet de ne plus hasher les mots de passe côté client, pour les sites qui seraient embêtés par le bug et qui sont en https)

(pour les anciennes versions 3.2, on peut proposer le palliatif d'utiliser la constante `_AUTORISER_AUTH_FAIBLE` qui permet de ne plus hasher les mots de passe côté client, pour les sites qui seraient embêtés par le bug et qui sont en https)
Poster
Owner

on peut proposer le palliatif d'utiliser la constante _AUTORISER_AUTH_FAIBLE

Comme tu le notes judicieusement, ça aurait aussi été une solution en 4.1 sans ce correctif :p

(don’t feed the troll)

> on peut proposer le palliatif d'utiliser la constante _AUTORISER_AUTH_FAIBLE Comme tu le notes judicieusement, ça aurait aussi été une solution en 4.1 sans ce correctif :p (don’t feed the troll)
Owner

Après relecture, on peut même garder le dernier commit qui concerne le secret_du_site: même si il ne corrige pas complètement la remarque initiale de @g0uZ il améliore la sécurité globale puisqu'il faut accèder à la fois à config/cle.php et tmp/meta_cache.php pour avoir le secret.

Je viens de faire une série de test :

  • connexion/deconnexion OK
  • connexion webmestre apres suppression du fichier cle.php OK
  • tentative de connexion redacteur apres suppression du fichier cle.php OK : pas de connexion possible, pas de generation des cles auth, log qui indique qu'un webmestre avec un backup doit de connecter pour restaurer les cles
  • re-installation OK
  • re-installation par un webemstre existant apres suppression du fichier cle.php OK
  • re-installation en essayant de creer un nouveau webmestre apres suppression du fichier cle.php => OK message erreur indiquant que un webmestre existant avec un backup doit faire la procedure d'install (chaines de langues ajoutées)

Je test un upgrade sur dev-contrib et si ok je place la branche en prod sur contrib (je fais un backup de spip_auteurs avant pour pouvoir revert les alea/pass si besoin)

Après relecture, on peut même garder le dernier commit qui concerne le secret_du_site: même si il ne corrige pas complètement la remarque initiale de @g0uZ il améliore la sécurité globale puisqu'il faut accèder à la fois à `config/cle.php` et `tmp/meta_cache.php` pour avoir le secret. Je viens de faire une série de test : - connexion/deconnexion OK - connexion webmestre apres suppression du fichier cle.php OK - tentative de connexion redacteur apres suppression du fichier cle.php OK : pas de connexion possible, pas de generation des cles auth, log qui indique qu'un webmestre avec un backup doit de connecter pour restaurer les cles - re-installation OK - re-installation par un webemstre existant apres suppression du fichier cle.php OK - re-installation en essayant de creer un nouveau webmestre apres suppression du fichier cle.php => OK message erreur indiquant que un webmestre existant avec un backup doit faire la procedure d'install (chaines de langues ajoutées) Je test un upgrade sur dev-contrib et si ok je place la branche en prod sur contrib (je fais un backup de spip_auteurs avant pour pouvoir revert les alea/pass si besoin)
cerdic added 2 commits 6 months ago
7be4a254ea Robustesse des migrations : on ne genere pas le secret_des_auth tant qu'on n'est pas en mesure de faire un backup des cles dans la foulee :
Owner

Lors de l'installation, on créé le fichier cles.php ET on fait un backup des cles immediatement sur le compte webmestre créé, donc pas de risque de perdre les cles.
Mais lors de la mise à jour on avait un risque car on créait le fichier de cles au premier login, et on commençait à hasher les password avec même si aucun webmestre ne s'était connecté.

J'ai envoyé 7be4a254e pour résoudre ça : tant qu'un webmestre ne se loge pas après upgrade, on ne génère pas la nouvelle clé.

J'ai testé une mise à jour sur dev-contrib sans soucis, et j'ai mis la branche https://git.spip.net/spip/spip/src/branch/refactor_auth_spip41 (rebase de cette PR en 4.1) en prod, donc on est en test live

Lors de l'installation, on créé le fichier cles.php ET on fait un backup des cles immediatement sur le compte webmestre créé, donc pas de risque de perdre les cles. Mais lors de la mise à jour on avait un risque car on créait le fichier de cles au premier login, et on commençait à hasher les password avec même si aucun webmestre ne s'était connecté. J'ai envoyé 7be4a254e pour résoudre ça : tant qu'un webmestre ne se loge pas après upgrade, on ne génère pas la nouvelle clé. J'ai testé une mise à jour sur dev-contrib sans soucis, et j'ai mis la branche https://git.spip.net/spip/spip/src/branch/refactor_auth_spip41 (rebase de cette PR en 4.1) en prod, donc on est en test live

Ce soir j'ai voulu me connecter sur contrib->il ne reconnaissait pas mon mot de passe. Et pourtant il n'a pas changé depuis hier, et je l'avais en plus en memoire dans firefox.

Seule solution : reinitialiser mon mot de passe.

je tente de voir en local.

Ce soir j'ai voulu me connecter sur contrib->il ne reconnaissait pas mon mot de passe. Et pourtant il n'a pas changé depuis hier, et je l'avais en plus en memoire dans firefox. Seule solution : reinitialiser mon mot de passe. je tente de voir en local.

Refus de connexion aussi mais Firefox n'a pas perdu la mémoire.

Refus de connexion aussi mais Firefox n'a pas perdu la mémoire.

Bon, en local je ne reproduis pas...

Bon, en local je ne reproduis pas...
Poster
Owner

Ah attend c’est peut être ma faute…
Je me demande s’il ne s’est pas remis sur un commit hors de la branche de test

Ah attend c’est peut être ma faute… Je me demande s’il ne s’est pas remis sur un commit hors de la branche de test
Poster
Owner

Et donc on était bien sortis de la branche de test sur Contrib tout à l’heure… désolé

Et donc on était bien sortis de la branche de test sur Contrib tout à l’heure… désolé
marcimat added 17 commits 6 months ago
07642edded Issues #5059 #4927 #3824 et #2109 : on reforme la logique du login
c31437f00a Refactoring de Chiffrer en découppant en plus d’éléments
388c45cdf1 Etre tres prudent sur la generation d'un nouveau secret_des_auth : cela ne peut avoir lieu que si on a plus aucun backup d'aucun webmestre
b50a3ed169 Champ `backup_cles` sur spip_auteurs, et `secret_du_site` hors de spip_meta
88fc960c3c Gestion de l'installation :
5e27fb3c85 Chiffrement plus poussé : pad / unpad du message + hash si password (g0uZ)
628514421d Suite de 23994e0b1 : Ne pas mettre de jeton d’url directement en bdd (cas de l’inscription d’auteur ou du changement de mot de passe)
20908f7888 Sécurité des authentifications avec le secret du site (g0uZ) :
b469db9569 Robustesse des migrations : on ne genere pas le secret_des_auth tant qu'on n'est pas en mesure de faire un backup des cles dans la foulee :
marcimat merged commit b469db9569 into master 6 months ago
marcimat deleted branch refactor_auth_spip 6 months ago
The pull request has been merged as b469db9569.
Sign in to join this conversation.
Loading…
There is no content yet.