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

Closed
cerdic wants to merge 27 commits from refacor_auth_spip_alter into 4.1
cerdic commented 11 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:

C'est a priori bon a tester (et on fera un merge plutot qu'un rebase amha)

- 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 C'est a priori bon a tester (et on fera un merge plutot qu'un rebase amha)
cerdic added 13 commits 11 months ago
3e0f841632 Issues #5059 #4927 #3824 et #2109 : on reforme la logique du login
e61bf84b2b Refactoring de la proposition de chiffrer en découppant en plus d’éléments
290b65c900 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
cerdic added 1 commit 11 months ago
cd0b21d893 Gestion de l'installation :
Poster
Owner

J'ai ajouté la prise en charge des cles lors de l'installation pour notamment gérer le cas où on a perdu le fichier config/cle.php et permettre de le restaurer en demandant à un webmestre existant de faire la procedure d'install (avec son mot de passe).

A noter que j'ai également modifié spip-cli pour la commande de modif des mots de passe pour qu'elle puisse marcher avec toutes les versions de SPIP
2f86ad5c76

Enfin, je pense qu'il faut ajouter une commande spip-cli pour forcer un raz des authentifications, ce qui revient à

  • supprimer le fichier config/cles.php
  • vider le champ backup_cles sur tous les auteurs de spip_auteurs

C'est un cas extrême qui invalide tous les mots de passe en base, mais qui peut être utile si on a perdu les clé et que tous les webmestres ont perdu leur mot de passe...

J'ai ajouté la prise en charge des cles lors de l'installation pour notamment gérer le cas où on a perdu le fichier config/cle.php et permettre de le restaurer en demandant à un webmestre existant de faire la procedure d'install (avec son mot de passe). A noter que j'ai également modifié spip-cli pour la commande de modif des mots de passe pour qu'elle puisse marcher avec toutes les versions de SPIP https://git.spip.net/spip-contrib-outils/spip-cli/commit/2f86ad5c76bf016bc905d94f6d1c3b859f5e976b Enfin, je pense qu'il faut ajouter une commande spip-cli pour forcer un raz des authentifications, ce qui revient à - supprimer le fichier `config/cles.php` - vider le champ `backup_cles` sur tous les auteurs de `spip_auteurs` C'est un cas extrême qui invalide tous les mots de passe en base, mais qui peut être utile si on a perdu les clé et que tous les webmestres ont perdu leur mot de passe...
marcimat added 3 commits 11 months ago
cerdic added 1 commit 11 months ago
marcimat added 1 commit 11 months ago
marcimat added 1 commit 11 months ago
Poster
Owner

Sur une install neuve, on a la creation des cles a l'etape 3B, lorsqu'on créé le premier webmestre, et il a automatiquement un backup des cles qui lui est assigné. On a donc immédiatement un webmestre avec une copie chiffrée des clés.

Sur une install neuve, on a la creation des cles a l'etape 3B, lorsqu'on créé le premier webmestre, et il a automatiquement un backup des cles qui lui est assigné. On a donc immédiatement un webmestre avec une copie chiffrée des clés.
marcimat added 1 commit 11 months ago
cerdic added 1 commit 11 months ago
Owner

En prévision de #5056 je vais peut être déplacer ecrire/chiffrer/* dans ecrire/src/Chiffrer/ sous le namespace Spip\Chiffrer directement (au lieu de Spip\Core\Chiffrer)

En prévision de #5056 je vais peut être déplacer ecrire/chiffrer/* dans ecrire/src/Chiffrer/ sous le namespace Spip\Chiffrer directement (au lieu de Spip\Core\Chiffrer)
marcimat added 2 commits 11 months ago
marcimat added 1 commit 11 months ago
marcimat added 1 commit 11 months ago
Owner

J’pense qu’on est bon là dessus.

J’pense qu’on est bon là dessus.
marcimat approved these changes 11 months ago

Afin de prévenir la compromission du SPIP (RCE) lors d'un accès non autorisé à la DB il est nécessaire de protéger certaines données, notamment le champ cooki_oubli (utilisé pour les "token utilisateurs") :

Proposition :

158feda047/ecrire/action/inscrire_auteur.php (L354)

sql_updateq('spip_auteurs', ['cookie_oubli' => chiffrer($jeton)], 'id_auteur=' . intval($id_auteur));

158feda047/ecrire/action/inscrire_auteur.php (L373)

$desc = sql_fetsel('*', 'spip_auteurs', 'cookie_oubli=' . sql_quote(chiffrer($jeton), '', 'string'));

L'attaque suivante n'est plus réalisable :

  1. identifier une injection SQL en lecture
  2. générer le token pour un webmestre via le formulaire de perte de mot de passe
  3. lire le token en DB avec l'injection SQL
  4. s'authentifier en tant que webmetre et installer un plugin pour exectuion de code PHP
Afin de prévenir la compromission du SPIP (RCE) lors d'un accès non autorisé à la DB il est nécessaire de protéger certaines données, notamment le champ cooki_oubli (utilisé pour les "token utilisateurs") : Proposition : https://github.com/spip/SPIP/blob/158feda0472615afa9daf205b536502cd285bf08/ecrire/action/inscrire_auteur.php#L354 ``` sql_updateq('spip_auteurs', ['cookie_oubli' => chiffrer($jeton)], 'id_auteur=' . intval($id_auteur)); ``` https://github.com/spip/SPIP/blob/158feda0472615afa9daf205b536502cd285bf08/ecrire/action/inscrire_auteur.php#L373 ``` $desc = sql_fetsel('*', 'spip_auteurs', 'cookie_oubli=' . sql_quote(chiffrer($jeton), '', 'string')); ``` L'attaque suivante n'est plus réalisable : 1. identifier une injection SQL en lecture 2. générer le token pour un webmestre via le formulaire de perte de mot de passe 3. lire le token en DB avec l'injection SQL 4. s'authentifier en tant que webmetre et installer un plugin pour exectuion de code PHP
marcimat added 1 commit 11 months ago
Owner

J’ai réarrangé les commits et squashé / fixup certains pour en diminuer pas mal le nombre. La nouvelle PR est là #5064

J’ai réarrangé les commits et squashé / fixup certains pour en diminuer pas mal le nombre. La nouvelle PR est là #5064
marcimat closed this pull request 11 months ago

Reviewers

marcimat approved these changes 11 months ago
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Loading…
There is no content yet.