¿Está este inicio de sesión de usuario seguro? -- php campo con mysql campo con security camp codereview Relacionados El problema

Is this user login secure?


6
vote

problema

Español

Cuando llegue a la seguridad, trato de ser lo más importante posible, pero no tengo el conocimiento.

De acuerdo con lo que leí en línea, mi siguiente código debería ser bueno, pero podría usar algunos de sus comentarios / críticos / correcciones

Aquí hay una clase simple solo por ejemplo, cómo haría un inicio de sesión. ¿Se parece lo suficientemente seguro?

  class UserClass { private $dbCon = null; public $Error = ''; public function __construct(PDO $dbCon) {     $this->dbCon = $dbCon; } public function login($Email,$Password,$RegisterCustomerSession = FALSE) {     $GetSalt = $this->dbCon->prepare('SELECT id,salt,hashPass FROM `customer` WHERE `email` = :Email');     $GetSalt -> bindValue(':Email',$Email);     $GetSalt -> execute();     if($GetSalt -> rowCount() == 0)     {         $this->Error = "No customer is registered with that email";         return false;     }     elseif($GetSalt->rowCount()>0)     {         $CustomerInfo = $GetSalt->fetch(PDO::FETCH_ASSOC);         if(sha1($Password.$CustomerInfo['salt'])==$CustomerInfo['hashPass'])         {             if($RegisterCustomerSession)                 self::RegisterAllCustomerSession($CustomerInfo['id']);             return true;         }         else         {             $this->Error = "Invalid Password";             return false;         }     } } public function SetPassword($CustomerId,$Password) {     $Salt = self::CreateSalt(16);     $HashPass = sha1($Password.$Salt);      $SetPasswordAndSalt = $this->dbCon->prepare('UPDATE `customer` SET `hashPass` = :HashPass,`salt` = :Salt WHERE `id` = :CustomerId;');     $SetPasswordAndSalt -> bindValue(':CustomerId',$CustomerId);     $SetPasswordAndSalt -> bindValue(':Salt',$Salt);     $SetPasswordAndSalt -> bindValue(':HashPass',$HashPass);     try{         $SetPasswordAndSalt ->execute();         return true;     }catch(PDOException $e){echo $e->getMessage(); return false; }  } private function CreateSalt($HowLong = 16) {     $CharStr = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-=+_<>';     $ReturnStr = '';     for($i = 0;$i<$HowLong;$i++)     {         $ReturnStr .= $CharStr{mt_rand(0,77)};     }     return $ReturnStr; } private function RegisterAllCustomerSession($CustomerId) {     // some code. } }   
Original en ingles

When it come to security I try to be to better as possible but I don't have the knowledge.

According to what I read on-line my following code should be good but I could use some of your comment/critic/fixes

Here is a simple class just to example how I would do a login. Does it look secure enough?

class UserClass { private $dbCon = null; public $Error = ''; public function __construct(PDO $dbCon) {     $this->dbCon = $dbCon; } public function login($Email,$Password,$RegisterCustomerSession = FALSE) {     $GetSalt = $this->dbCon->prepare('SELECT id,salt,hashPass FROM `customer` WHERE `email` = :Email');     $GetSalt -> bindValue(':Email',$Email);     $GetSalt -> execute();     if($GetSalt -> rowCount() == 0)     {         $this->Error = "No customer is registered with that email";         return false;     }     elseif($GetSalt->rowCount()>0)     {         $CustomerInfo = $GetSalt->fetch(PDO::FETCH_ASSOC);         if(sha1($Password.$CustomerInfo['salt'])==$CustomerInfo['hashPass'])         {             if($RegisterCustomerSession)                 self::RegisterAllCustomerSession($CustomerInfo['id']);             return true;         }         else         {             $this->Error = "Invalid Password";             return false;         }     } } public function SetPassword($CustomerId,$Password) {     $Salt = self::CreateSalt(16);     $HashPass = sha1($Password.$Salt);      $SetPasswordAndSalt = $this->dbCon->prepare('UPDATE `customer` SET `hashPass` = :HashPass,`salt` = :Salt WHERE `id` = :CustomerId;');     $SetPasswordAndSalt -> bindValue(':CustomerId',$CustomerId);     $SetPasswordAndSalt -> bindValue(':Salt',$Salt);     $SetPasswordAndSalt -> bindValue(':HashPass',$HashPass);     try{         $SetPasswordAndSalt ->execute();         return true;     }catch(PDOException $e){echo $e->getMessage(); return false; }  } private function CreateSalt($HowLong = 16) {     $CharStr = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-=+_<>';     $ReturnStr = '';     for($i = 0;$i<$HowLong;$i++)     {         $ReturnStr .= $CharStr{mt_rand(0,77)};     }     return $ReturnStr; } private function RegisterAllCustomerSession($CustomerId) {     // some code. } } 
        

Lista de respuestas

5
 
vote
vote
La mejor respuesta
 

No estoy lo suficientemente familiarizado con el PDO para decir más en su acceso de base de datos que que no parece tener ninguna vulnerabilidad de inyección. Sin embargo, hay algunas cosas que lo haría de manera diferente.

Entopía de la sal

Hay dos cosas que me parecen tan extrañas sobre tu generación de sal. En primer lugar, utilizando una codificación base-77. Si configura su base de datos correctamente, puede usar los 8 bits completos de cada byte de sal. Eso es más eficiente, evita posibles errores con la generación de un número en el rango incorrecto (¿qué tan seguro es que 9988776655544336 ?), Y está más cerca de las suposiciones realizadas al analizar el hashing salado.

En segundo lugar, mientras que mt_rand es mejor que rand No es un prng criptográfico. El mejor PRNG seguro portátil en PHP es openssl_random_pseudo_bytes ; Si no está planeando implementar en Windows, también podría obtener una buena entropía de ++a[p]0 .

Hash

SHA no se recomienda generalmente como un hash para contraseñas. La SABIDURÍA CONVENCIONAL actual es que desea que la contraseña sea lenta, y debe usar BCIPT o Scrypt. Si insiste en SHA, debe usarlo como un componente de PBKDF2.

Existencia de la cuenta Oracle

Hay dos escuelas de pensamiento al decirle a la gente "que el nombre de usuario no existe". El argumento de la usabilidad es que es preferible decirle a alguien que obtuvieron su nombre de usuario equivocado. El argumento de seguridad es que siempre debe decir "O bien, su nombre de usuario no existe o tiene su contraseña incorrecta" para evitar que las personas que identifique cuentas que existen y luego intentan forzar la fuerza bruta sus contraseñas. (Las técnicas de forzamiento anti-bruto son un problema separado que no voy a abordar en detalle).

Si prefiere el argumento de seguridad sobre el argumento de la usabilidad, debe evitar decirle a las personas indirectamente que el nombre de usuario no existe. Eso significa que si ++a[p]1 aún debe hacer una operación de hashing para evitar una carga de página más rápida que fugan la información.

Puntos de estilo menores

  ++a[p]2  

Veo tres cosas mal aquí:

  1. ¿Qué otras posibilidades hay? ¿No debería ese ++a[p]3 sólo sea un ++a[p]4 ?
  2. ¿Por qué ++a[p]5 en lugar de ++a[p]6 ?
  3. Elija un estilo para el uso de espacios en blanco y quédate con él.
 

I'm not sufficiently familiar with PDO to say more on your database access than that you don't seem to have any injection vulnerabilities. However, there are a few things which I would do differently.

Salt entropy

There are two things which strike me as odd about your salt generation. Firstly, using a base-77 encoding. If you configure your database correctly then you can use the full 8 bits of each byte of salt. That's more efficient, avoids possible bugs with generating a number in the wrong range (how sure are you that strlen($CharStr) === 78?), and is closer to the assumptions made when analysing salted hashing.

Secondly, while mt_rand is better than rand it's not a cryptographic PRNG. The best portable secure PRNG in PHP is openssl_random_pseudo_bytes; if you're not planning to deploy to Windows then you could also get good entropy from /dev/random.

Hash

SHA is not generally recommended as a hash for passwords. The current conventional wisdom is that you want password hashing to be slow, and should use either bcrypt or scrypt. If you insist on SHA then you should use it as a component of PBKDF2.

Account existence oracle

There are two schools of thought on telling people "That username doesn't exist". The usability argument is that it's preferable to tell someone that they got their username wrong. The security argument is that you should always say "Either your username doesn't exist or you got your password wrong" to prevent people identifying accounts which do exist and then trying to brute-force their passwords. (Anti-brute-forcing techniques is a separate issue which I'm not going to address in detail).

If you favour the security argument over the usability argument then you need to avoid telling people indirectly that the username doesn't exist. That means that if $GetSalt -> rowCount() === 0 you should still do a hashing operation to avoid a quicker page load which leaks information.

Minor style points

if($GetSalt -> rowCount() == 0)     ... elseif($GetSalt->rowCount()>0) 

I see three things wrong here:

  1. What other possibilities are there? Shouldn't that elseif just be an else?
  2. Why == instead of ===?
  3. Pick a style for use of whitespace and stick with it.
 
 
     
     

Relacionados problema

1  ¿Cómo puedo determinar y probar Vunerabilidades de inyección de SQL en este código de inicio de sesión?  ( How can i ascertain and test sql injection vunerabilities on this login code ) 
Sé que el escenario "Little Bobby Tablas" y se estaba preguntando si este código es vulnerable a tales inyecciones de SQL. Soy bastante nuevo en PHP y teng...

11  ¿Es esta una forma suficiente de prevenir las inyecciones de guiones y otras cosas malas en las cuerdas?  ( Is this a sufficient way to prevent script injections and other bad stuff in str ) 
¿Esta función será suficiente para eliminar todo el código malicioso y los caracteres extraños de una cadena? import java.security.SecureRandom; import jav...

10  Generando tokens de sesión difícil de adivinar  ( Generating hard to guess session tokens ) 
¿Este código crea tokens de sesión suficientemente difíciles de adivinar, asumiendo que el servidor y el cliente se comunican a través de HTTPS? Tomar 2 ...

1  Simple Node.js WebServer  ( Simple node js webserver ) 
Estoy en el proceso de escribir un servidor web simple.js. Estoy a mitad de camino en términos de funcionalidad. Consulte el código completo en este pastebin...

11  Cifrado simétrico en C #  ( Symmetrical encryption in c ) 
Estoy tratando de cifrar simétricamente algunos datos utilizando C #, y parece que hay mucha información engañosa o incorrecta en el sujeto. He creado un pr...

8  Convertir un segurreque en una matriz de bytes  ( Converting a securestring to a byte array ) 
¿Asigna algo que no ha liberado? ¿Algo que permanece en la memoria? Excepto la matriz de bytes de resultado, OFC. checkOnline.sh1 ...

3  Formulario de contacto de WordPress PHP - Fallas de seguridad  ( Wordpress php contact form security flaws ) 
Tengo el siguiente formulario de contacto incluido con mi tema de WordPress como plantilla. Ahora, un usuario en StackOverFlow señaló que este formulario ti...

51  Cifrado seguro simplificado de una cadena  ( Simplified secure encryption of a string ) 
Tengo dos ejemplos de código que escribí para las mejores prácticas cifrando una cadena que busco comentarios tanto en términos de mejores prácticas y segurid...

6  Programa de recuperación de contraseña  ( Password recovery program ) 
Este es un programa de recuperación de contraseña que hice, y solo quiero que se revise. Estos no son todos los archivos para el sistema de inicio de sesión y...

2  Formulario básico de comentarios de PHP  ( Basic php comment form ) 
Soy un novato de programación. He escrito este simple script PHP para ejecutar un formulario de comentarios muy básico y apreciaría cualquier comentario, espe...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos