maj: sémantique, révision vérification password #3
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "maj"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ajout d'un customHasher pour le password qui implémente un salt éventuellement récupéré si configuré et si présent
ainsi qu'un pepper si non null dans les variables d'environnement.
Révision sémantique namespace et noms de classe
PDO pour SQLLogin
Symfony Security Check Report
1 package has known vulnerabilities.
twig/twig (v3.3.10)
Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.
Test report
PHP CS Fixer
Overview
Total duration: 0s
See details
Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLPasswordException
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginException
Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect
Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator
Errors
`Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService`
Output
applied fixers:
`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginException`
Output
applied fixers:
`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`
Output
Rapport PHPStan
@ -0,0 +24,4 @@
if ($this->isPasswordTooLong($plainPassword)) {
throw new InvalidPasswordException();
}
$hash = hash($this->hashAlgo, $plainPassword.$salt.$this->pepper);
Je pense que ce code ne sera pas compatible avec les applications ayant choisies d'utiliser les algorithmes
argon2id
,scrypt
etbcrypt
(qui sont d'ailleurs les recommandations OWASP 1 aujourd'hui).Normalement, en PHP on utilise les méthodes
password_hash()
2 etpassword_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éthodestrstr()
5 par exemple) pour permettre à l'utilisateur de spécifiquer le format de concaténation.https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html ↩︎
https://www.php.net/manual/en/function.password-hash.php ↩︎
https://www.php.net/manual/en/function.password-verify.php ↩︎
https://stackoverflow.com/questions/40993645/understanding-bcrypt-salt-as-used-by-php-password-hash ↩︎
https://www.php.net/manual/en/function.strtr.php ↩︎
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
@ -0,0 +35,4 @@
return false;
}
if ($this->hash($plainPassword, $salt) === $hashedPassword) {
Il ne faut pas utiliser l'opérateur
===
pour faire des comparaisons de mots de passe (ça ouvre à des failles du type "Timing attack" 1).Il serait mieux d'utiliser https://www.php.net/manual/en/function.hash-equals.php je pense dans ce cas.
https://www.chosenplaintext.ca/articles/beginners-guide-constant-time-cryptography.html ↩︎
maj: sémantique, révision vérification ppasswordto maj: sémantique, révision vérification password@ -25,2 +25,4 @@
locales: '%env(APP_LOCALES)%'
app.supported_locales: ~
env(PEPPER): "257d62c24cd352c21b51c26dba678c8ff05011a89022aec106185bf67c69aa8b"
Hmm, je pense que par défaut le pepper devrait être vide.
@ -0,0 +28,4 @@
public const ERROR_PDO = 'error_pdo';
protected string $baseUrl;
private SQLLoginService $pdoService;
Je pense que la variable n'a pas été renommée
$pdoService
en accord avec le changement de nom du service.@ -0,0 +32,4 @@
private UrlGeneratorInterface $router;
private PasswordEncoder $passwordHasher;
public function __construct(string $baseUrl, SQLLoginService $pdoService, UrlGeneratorInterface $router, PasswordEncoder $passwordHasher)
Idem, cf. commentaire précédent
@ -0,0 +35,4 @@
public function __construct(string $baseUrl, SQLLoginService $pdoService, UrlGeneratorInterface $router, PasswordEncoder $passwordHasher)
{
$this->baseUrl = $baseUrl;
$this->pdoService = $pdoService;
Idem, cf. commentaire précédent
@ -0,0 +72,4 @@
public function getConnection()
{
// Appel du singleton
$pdo = SQLLoginConnect::getInstance();
Idem, cf. commentaire précédent
@ -1 +1 @@
123
134
Ce fichier doit il vraiment être commit dans le dépôt ? Attention avec les
git add -A
:PSymfony Security Check Report
1 package has known vulnerabilities.
twig/twig (v3.3.10)
Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.
Test report
PHP CS Fixer
Overview
Total duration: 0s
See details
Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginAlgoException
Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginConfigurationException
Cadoles_hydra-sql_PR-3/src/SQLLogin/SQLLoginConnect
Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator
Errors
`Cadoles_hydra-sql_PR-3/src/Service/SQLLoginService`
Output
applied fixers:
`Cadoles_hydra-sql_PR-3/src/SQLLogin/Exception/InvalidSQLLoginConfigurationException`
Output
applied fixers:
`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`
Output
Rapport PHPStan
73eae4176e
tob8fb01da5c
Symfony Security Check Report
No packages have known vulnerabilities.
Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.
Test report
PHP CS Fixer
Overview
Total duration: 0s
See details
Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator
Errors
`Cadoles_hydra-sql_PR-3/src/Security/SQLLoginUserAuthenticator`
Output
Rapport PHPStan
Symfony Security Check Report
No packages have known vulnerabilities.
Note that this checker can only detect vulnerabilities that are referenced in the security advisories database.
Execute this command regularly to check the newly discovered vulnerabilities.
Test report
PHP CS Fixer
Overview
Total duration: 0s
See details
All OK
Rapport PHPStan