Évolution du système d'authentification #4927

Closed
opened 8 months ago by g0uZ · 15 comments
g0uZ commented 8 months ago

Une proposition fonctionnelle pour SPIP 4.0 sous forme de plugin permettant d'améliorer la sécurité de l'authentification par mot de passe/hash de SPIP :

https://git.spip.net/spip-contrib-extensions/chiffrer

Peut on planifier son intégration dans le core ou dans -dist ?

Une proposition fonctionnelle pour SPIP 4.0 sous forme de plugin permettant d'améliorer la sécurité de l'authentification par mot de passe/hash de SPIP : https://git.spip.net/spip-contrib-extensions/chiffrer Peut on planifier son intégration dans le core ou dans -dist ?
Owner

Amha ça concerne plus la 4.1, donc je basculerais bien sur ce jalon, mais attendons le retour des autres.

Amha ça concerne plus la 4.1, donc je basculerais bien sur ce jalon, mais attendons le retour des autres.
b_b added the
authentification
amélioration
labels 8 months ago

@g0uZ Si je comprend bien tu proposes que ça soit intégré dans le core. C'est super si ça améliore la sécurité, mais c'est inquiétant aussi car ça impacte les comptes et mot de passe de tous les utilisateurs, et que c'est un chemin sans retour...

Alors pourrais tu préciser :

  • Basiquement, suite à la mise en place, les utilisateurs se rendent ils compte du changement ? Est-ce qu'on leur demande de faire quelque chose pour pouvoir continuer à s'identifier, genre changer leur mdp ou visiter une url ?

  • Y a t il des conséquences en l'état pour d'autres fonctions de SPIP, API ou pour des parties de code de plugins ?

  • Tu écris que « La fonctionnalité de perte de mot de passe n'a pas encore subit un traitement similaire, de manière générale tous les aléas secrets générés devraient y passer » : est-ce que ces fonctionnalités (oubli mdp et autres usages des aléas secrets générés) fonctionnent quand même (sans bénéficier du supplément de sécurité) avec ce patch en l'état ?

@g0uZ Si je comprend bien tu proposes que ça soit intégré dans le core. C'est super si ça améliore la sécurité, mais c'est inquiétant aussi car ça impacte les comptes et mot de passe de tous les utilisateurs, et que c'est un chemin sans retour... Alors pourrais tu préciser : - Basiquement, suite à la mise en place, les utilisateurs se rendent ils compte du changement ? Est-ce qu'on leur demande de faire quelque chose pour pouvoir continuer à s'identifier, genre changer leur mdp ou visiter une url ? - Y a t il des conséquences en l'état pour d'autres fonctions de SPIP, API ou pour des parties de code de plugins ? - Tu écris que « La fonctionnalité de perte de mot de passe n'a pas encore subit un traitement similaire, de manière générale tous les aléas secrets générés devraient y passer » : est-ce que ces fonctionnalités (oubli mdp et autres usages des aléas secrets générés) fonctionnent quand même (sans bénéficier du supplément de sécurité) avec ce patch en l'état ?

Si je comprends bien, ce que tu proposes là est une réponse très concrête à cette objection trouvée sur Twitter : https://twitter.com/breard_r/status/1414993786518790151?t=pFy1eL-Rqfb74t8gliQL2w&s=09

Si je comprends bien, ce que tu proposes là est une réponse très concrête à cette objection trouvée sur Twitter : https://twitter.com/breard_r/status/1414993786518790151?t=pFy1eL-Rqfb74t8gliQL2w&s=09

Je crois pas realEt. Breard_r sur twitter est soit de mauvaise foi ou soit dans l'ignorance, car les mots de passe dans SPIP sont cryptés salés. C'est pour ça que g0uz par le de poivre et pas de sel, car c'est une couche en plus : le poivre empêche de nuire même si on a chopé le mdp crypté avec le sel.

Je crois pas realEt. Breard_r sur twitter est soit de mauvaise foi ou soit dans l'ignorance, car les mots de passe dans SPIP sont cryptés salés. C'est pour ça que g0uz par le de `poivre` et pas de `sel`, car c'est une couche _en plus_ : le poivre empêche de nuire même si on a chopé le mdp crypté avec le sel.
Poster
  • Basiquement, suite à la mise en place, les utilisateurs se rendent ils compte du changement ? Est-ce qu'on leur demande de faire quelque chose pour pouvoir continuer à s'identifier, genre changer leur mdp ou visiter une url ?

Non c'est transparent pour l'utilisateur. On attend seulement d'avoir son mot de passe en clair pour pouvoir le hasher avec un nouvel algorithme et MAJ le champ pass de la table spip_auteurs dans la base de données.

  • Y a t il des conséquences en l'état pour d'autres fonctions de SPIP, API ou pour des parties de code de plugins ?

Non, mais il doit rester des endroits ou on utilise sha256 (création/modification de l'auteur mais ce n'a pas de conséquence : l'authentification supportant tous les algorithmes de hash possibles (MD5/SHA256/bcrypt/argon2i)

Le seul fonctionnement qui est supprimé est celui permettant de s'authentifier directement avec le hash du pass : vulnérable a des attaques de type Pass The Hash.

  • Tu écris que « La fonctionnalité de perte de mot de passe n'a pas encore subit un traitement similaire, de manière générale tous les aléas secrets générés devraient y passer » : est-ce que ces fonctionnalités (oubli mdp et autres usages des aléas secrets générés) fonctionnent quand même (sans bénéficier du supplément de sécurité) avec ce patch en l'état ?

Tout fonctionne sans problème. Ce que je veux dire par là c'est que le bénéfice en sécurité est moindre/nul tant que tous les cas permettant de s'authentifier avec une donnée contenu dans la DB ne sauront pas correctement gérés (avec chiffrer()/dechiffrer() par exemple).

> - Basiquement, suite à la mise en place, les utilisateurs se rendent ils compte du changement ? Est-ce qu'on leur demande de faire quelque chose pour pouvoir continuer à s'identifier, genre changer leur mdp ou visiter une url ? Non c'est transparent pour l'utilisateur. On attend seulement d'avoir son mot de passe en clair pour pouvoir le hasher avec un nouvel algorithme et MAJ le champ pass de la table spip_auteurs dans la base de données. > - Y a t il des conséquences en l'état pour d'autres fonctions de SPIP, API ou pour des parties de code de plugins ? Non, mais il doit rester des endroits ou on utilise sha256 (création/modification de l'auteur mais ce n'a pas de conséquence : l'authentification supportant tous les algorithmes de hash possibles (MD5/SHA256/bcrypt/argon2i) Le seul fonctionnement qui est supprimé est celui permettant de s'authentifier directement avec le hash du pass : vulnérable a des attaques de type Pass The Hash. > - Tu écris que « La fonctionnalité de perte de mot de passe n'a pas encore subit un traitement similaire, de manière générale tous les aléas secrets générés devraient y passer » : est-ce que ces fonctionnalités (oubli mdp et autres usages des aléas secrets générés) fonctionnent quand même (sans bénéficier du supplément de sécurité) avec ce patch en l'état ? Tout fonctionne sans problème. Ce que je veux dire par là c'est que le bénéfice en sécurité est moindre/nul tant que tous les cas permettant de s'authentifier avec une donnée contenu dans la DB ne sauront pas correctement gérés (avec chiffrer()/dechiffrer() par exemple).
Owner

Suite à #5059 il me semble essentiel d'intégrer cette évolution dans la 4.1 que l'on s'apprête à releaser même si il faut prendre un peu plus de temps pour tester.

J'ai fait une revue du plugin chiffrer de @g0uZ qui me parait assez simple à intégrer. Essentiellement il supprime tout ce qui est hashage côté client, on se repose maintenant sur https comme tout le monde, et côté serveur il a une mécanique beaucoup plus safe et robuste qui repose sur les fonctions natives de PHP.

Je peux préparer une PR sur la 4.1 pour tests et avis si ça va à tout le monde

Suite à #5059 il me semble essentiel d'intégrer cette évolution dans la 4.1 que l'on s'apprête à releaser même si il faut prendre un peu plus de temps pour tester. J'ai fait une revue du plugin chiffrer de @g0uZ qui me parait assez simple à intégrer. Essentiellement il supprime tout ce qui est hashage côté client, on se repose maintenant sur https comme tout le monde, et côté serveur il a une mécanique beaucoup plus safe et robuste qui repose sur les fonctions natives de PHP. Je peux préparer une PR sur la 4.1 pour tests et avis si ça va à tout le monde

totalement ok pour https + son plugin à intégrer en core

totalement ok pour https + son plugin à intégrer en core
Owner

Je ne sais qu’en penser…

Ok on est en beta, c’est possible, mais c’est un changement vraiment pas anodin quand même. Tu envisagerais pour quand de préparer une PR ?

Ensuite sur l’implémentation de g0oZ à base de GLOBALS, non. Si on peut éviter ça please…

Ça voudrait dire a minima introduire dans sa proposition une fonction lire_cle('cle_secrete') qui ferait sa tambouille (statique?)

Probablement renommer les fonctions

  • initialiser_cle en initialiser_cles
  • restaurer_cle en restaurer_cles

Que le fichier cles.php returne un array (ne peuple pas de globale)

Je ne sais qu’en penser… Ok on est en beta, c’est possible, mais c’est un changement vraiment pas anodin quand même. Tu envisagerais pour quand de préparer une PR ? Ensuite sur l’implémentation de g0oZ à base de `GLOBALS`, non. Si on peut éviter ça please… Ça voudrait dire a minima introduire dans sa proposition une fonction `lire_cle('cle_secrete')` qui ferait sa tambouille (statique?) Probablement renommer les fonctions - `initialiser_cle` en `initialiser_cles` - `restaurer_cle` en `restaurer_cles` Que le fichier `cles.php` returne un array (ne peuple pas de globale)
Owner

Ok on est en beta, c’est possible, mais c’est un changement vraiment pas anodin quand même. Tu envisagerais pour quand de préparer une PR ?

amha le respect du calendrier de "release accéléré" pour SPIP ne semble pas un argument suffisant au regard du gain (important !) de sécurité que ce changement amènera...
=> idem Rasta donc : totalement OK pour l'intégration de ce changement maintenant

> Ok on est en beta, c’est possible, mais c’est un changement vraiment pas anodin quand même. Tu envisagerais pour quand de préparer une PR ? amha le respect du calendrier de "release accéléré" pour SPIP ne semble pas un argument suffisant au regard du gain (important !) de sécurité que ce changement amènera... => idem Rasta donc : totalement OK pour l'intégration de ce changement maintenant
Owner

Essentiellement il supprime tout ce qui est hashage côté client, on se repose maintenant sur https comme tout le monde,

quid du fonctionnement pour les sites qui ne sont pas en HTTPS ? (malgré la généralisation actuelle on en croise encore quelques uns...)

> Essentiellement il supprime tout ce qui est hashage côté client, on se repose maintenant sur https comme tout le monde, quid du fonctionnement pour les sites qui ne sont pas en HTTPS ? (malgré la généralisation actuelle on en croise encore quelques uns...)

quid du fonctionnement pour les sites qui ne sont pas en HTTPS ? (malgré la généralisation actuelle on en croise encore quelques uns...)

Je me posais la même question. Est-ce qu'il y aurait moyen d'imposer :

  • pas de maj via spip_loader sans https
  • un message de warning dans l'espace privé si pas de https + un mail au / à la webmestre
> quid du fonctionnement pour les sites qui ne sont pas en HTTPS ? (malgré la généralisation actuelle on en croise encore quelques uns...) Je me posais la même question. Est-ce qu'il y aurait moyen d'imposer : - pas de maj via spip_loader sans https - un message de warning dans l'espace privé si pas de https + un mail au / à la webmestre
Owner

On revient sur une situation identique à tous @cy.altern : securité = https et si tu es en http le mot de passe navigue en clair (comme te l'indique ton navigateur)

On revient sur une situation identique à tous @cy.altern : securité = https et si tu es en http le mot de passe navigue en clair (comme te l'indique ton navigateur)
  • pas de maj via spip_loader sans https

Pas du tout une bonne idée pour au moins 2 cas :

  • poste de dev avec site pas (ou pas encore) en https
  • intranet pas en https
> - pas de maj via spip_loader sans https Pas du tout une bonne idée pour au moins 2 cas : * poste de dev avec site pas (ou pas encore) en https * intranet pas en https

Pas du tout une bonne idée pour au moins 2 cas :

effectivement, oublions

> Pas du tout une bonne idée pour au moins 2 cas : effectivement, oublions
Owner

intégré par #5064

intégré par #5064
cerdic closed this issue 4 months ago
cerdic added this to the 4.1 milestone 4 months ago
Sign in to join this conversation.
No Milestone
No project
No Assignees
9 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.