Cadoles
/
hydra-sql
21
0
Fork 0
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

maj: sémantique, révision vérification password #3

Merged
cmsassot merged 6 commits from maj into develop 2023-01-05 14:09:41 +01:00
Conversation 12 Commits 6 Files Changed 38 +2 -7
3 changed files with 2 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit b8fb01da5c - Show all commits

2
Jenkinsfile vendored
View File

@ -4,4 +4,4 @@
// Utilisation du pipeline partagé pour les applications Symfony de Cadoles // Utilisation du pipeline partagé pour les applications Symfony de Cadoles
// Le nom de l'image Docker passée en paramètre vous permet de préciser l'environnement de test // Le nom de l'image Docker passée en paramètre vous permet de préciser l'environnement de test
// de votre application Symfony // de votre application Symfony
symfonyAppPipeline("ubuntu:20.04") symfonyAppPipeline("ubuntu:22.04")

2
src/Security/Hasher/PasswordEncoder.php
View File

@ -27,7 +27,7 @@ class PasswordEncoder implements LegacyPasswordHasherInterface
} }
rmasson marked this conversation as resolved Outdated
wpetit commented 2022-12-15 21:30:54 +01:00
Outdated
Review
Copy Link

Je pense que ce code ne sera pas compatible avec les applications ayant choisies d'utiliser les algorithmes argon2id, scrypt et bcrypt (qui sont d'ailleurs les recommandations OWASP 1 aujourd'hui).

Normalement, en PHP on utilise les méthodes password_hash() 2 et password_verify() 3 pour utiliser ces algorithmes (et d'ailleurs le salt est directement stocké dans le hash final 4).

Autre chose: la méthode assume un certain format pour la concaténation des différents éléments à hacher ($plainPassword.$salt.$this->pepper). Il est fort peu probable que cette séquence soit toujours respectée dans toutes les applications. À mon avis il serait certainement préférable d'utiliser un patron (avec la méthode strstr() 5 par exemple) pour permettre à l'utilisateur de spécifiquer le format de concaténation.

Je pense que ce code ne sera pas compatible avec les applications ayant choisies d'utiliser les algorithmes `argon2id`, `scrypt` et `bcrypt` (qui sont d'ailleurs les recommandations OWASP [^1] aujourd'hui). Normalement, en PHP on utilise les méthodes `password_hash()` [^2] et `password_verify()` [^3] pour utiliser ces algorithmes (et d'ailleurs le salt est directement stocké dans le hash final [^4]). Autre chose: la méthode assume un certain format pour la concaténation des différents éléments à hacher (`$plainPassword.$salt.$this->pepper`). Il est fort peu probable que cette séquence soit toujours respectée dans toutes les applications. À mon avis il serait certainement préférable d'utiliser un patron (avec la méthode `strstr()` [^5] par exemple) pour permettre à l'utilisateur de spécifiquer le format de concaténation. [^1]: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html [^2]: https://www.php.net/manual/en/function.password-hash.php [^3]: https://www.php.net/manual/en/function.password-verify.php [^4]: https://stackoverflow.com/questions/40993645/understanding-bcrypt-salt-as-used-by-php-password-hash [^5]: https://www.php.net/manual/en/function.strtr.php
rmasson commented 2022-12-16 09:22:08 +01:00
Outdated
Review
Copy Link

j'ai utilisé les fonction password_algos() et hash_algos() pour faire la différence entre les algos, pour utiliser hash() ou password_hash()

Mais effectivement le pattern est pas pris en compte

j'ai utilisé les fonction password_algos() et hash_algos() pour faire la différence entre les algos, pour utiliser hash() ou password_hash() Mais effectivement le pattern est pas pris en compte
/** /**
* Pas utilisé * Pas utilisé, mais on doit le garder pour le implements
*/ */
public function hash(string $plainPassword, string $salt = null): string public function hash(string $plainPassword, string $salt = null): string
{ {

5
src/Security/SQLLoginUserAuthenticator.php
View File

@ -80,12 +80,7 @@ class SQLLoginUserAuthenticator extends AbstractAuthenticator
if ($remoteHashedPassword) { if ($remoteHashedPassword) {
try { try {
// Comparaison remote hash et hash du input password + salt // Comparaison remote hash et hash du input password + salt
// dump($remoteHashedPassword, $plaintextPassword, $remoteSalt, password_verify($plaintextPassword, $remoteHashedPassword));
$this->passwordHasher->verify($remoteHashedPassword, $plaintextPassword, $remoteSalt); $this->passwordHasher->verify($remoteHashedPassword, $plaintextPassword, $remoteSalt);
// if ($this->passwordHasher->needsRehash($remoteHashedPassword)) {
// $hash = $this->passwordHasher->hash($plaintextPassword);
// $this->sqlLoginService->updatePassword($login, $hash, null);
// }
$attributes = $this->sqlLoginService->fetchDatas($login); $attributes = $this->sqlLoginService->fetchDatas($login);
$user = new User($login, $remoteHashedPassword, $attributes, $rememberMe); $user = new User($login, $remoteHashedPassword, $attributes, $rememberMe);