Revisión de seguridad - implementación de password_hash para PHP

Question

Revisión de seguridad - implementación de password_hash para PHP

#1 de D.W. (4 votos)
#2 de Jacco (1 votos)
#3 de Cyril N. (0 votos)

45

Actualmente estoy trabajando en una "función de ayuda" para el núcleo de PHP para que la creación de contraseñas sea más segura y más fácil para la mayoría de los desarrolladores. Básicamente, el objetivo es hacerlo tan fácil, que sea más difícil inventar su propia implementación que usar la principal segura. También está diseñado para actualizarse y ampliarse en el futuro a medida que mejoren los métodos de hash.

He escrito un RFC para la adición , y tengo un implementación de las funciones también.

La premisa básica es que crypt es demasiado difícil de usar directamente para la mayoría de los desarrolladores. Los errores extraños devuelven, las sales de base64-like (pero usando un alfabeto diferente), etc. Así que estas funciones están diseñadas para eliminar esas conjeturas y proporcionar una API muy simple.

string password_hash(string $password, string $algo = PASSWORD_DEFAULT, array $options = array())
bool password_verify(string $password, string $hash)
string password_make_salt(int $length, bool $raw_output = false)

Password_Hash toma una contraseña, un especificador de algoritmo opcional (en este momento, solo se admite la implementación mejorada de CRYPT_BCRYPT , pero me gustaría agregar scrypt como una opción más adelante) y una matriz de opciones. La matriz de opciones puede especificar el parámetro cost para bcrypt, así como un valor de sal predefinido.

Password_Verify acepta una contraseña y un hash existente. Luego vuelve a escribir la contraseña (idéntica a $tmp = crypt($password, $hash) ). Luego, utiliza una función de comparación de tiempo constante para determinar si los dos hashes son iguales.

Password_Make_Salt existe para generar una cadena aleatoria de la longitud dada. Si raw_output se establece en falso (predeterminado), el "salt" generado será codificado en base64 de una manera que sea directamente compatible con crypt() . Si raw_output es verdadero, devolverá la misma cadena de longitud utilizando bytes completos aleatorios ( 0-255 ).

Ejemplo:

$hash = password_hash("foo");
if (password_verify("foo", $hash)) {
    // always should be true
} else {
}

Una nota sobre la lectura del código fuente de PHP: las funciones de PHP (aquellas expuestas al código php) están rodeadas por la macro PHP_FUNCTION() . Además, las variables php (zval's) se utilizan en algunos lugares. Las macros que acceden a partes de ellas son

Z_TYPE_P() (encontrar el tipo de un puntero a un zval)
Z_STRVAL_P() (obtener el puntero al valor de cadena)
Z_STRLEN_P() (obtenga la longitud int del tipo de cadena)
Z_LVAL_P() (obtenga el valor long del tipo entero)

Además, zval_ptr_dtor() es un mecanismo de refcount para disminuir el refcount en un zval , y limpiarlo si llega a 0 .

Estoy buscando una revisión de la implementación por parte de al menos algunos expertos en seguridad antes de proponer oficialmente el cambio. Es razonablemente corto (solo unas 300 líneas de código) ...

Actualizar

La API ha sido aprobada, por lo que he configurado una solicitud de extracción para la funcionalidad. Por favor revísalo si tienes tiempo. ¡Gracias !: enlace

passwords hash php c code-review

pregunta ircmaxell 27.06.2012 - 05:00

fuente

3 respuestas

Lea otras preguntas en las etiquetas passwords hash php c code-review

Cuenta de usuario sin contraseña ¿Es teóricamente posible el modo de "suspensión segura" de una computadora portátil?

score 4 · Answer 1

En C, generalmente recomiendo que uses constantemente size_t para almacenar todos los valores de longitud, no int o long . El uso de int y long lo expone a errores enteros con signo / sin signo. Referencia: INT01-C de el estándar de codificación segura CERT C .

Sin embargo, existe una complicación en este caso, debido a la forma en que funciona la interfaz de código nativo de PHP:

Cuando lees un número entero usando zend_parse_parameters() , PHP espera que lo guardes en un long . Por lo tanto, debe pasar el puntero zend_parse_parameters() a un long , luego convierta el long en un size_t con cuidado: valide que el número no sea negativo y se encuentre dentro del rango para size_t ( l >= 0 && l < SIZE_MAX ), luego conviértalo a size_t y use un size_t a partir de entonces.
Cuando lees una cadena usando zend_parse_parameters() , PHP espera que pases un búfer para almacenar la cadena y un puntero a un int para almacenar la longitud de la cadena. Por lo tanto, debe pasar a zend_parse_parameters() lo que está esperando. Luego, convierta cuidadosamente int a size_t : valide que la longitud no sea negativa y se encuentre dentro del rango para size_t ( len >= 0 && len < SIZE_MAX ), luego conviértalo a size_t y use size_t a partir de entonces.

De forma similar, Z_STRLEN_PP() devuelve un int (creo) por lo que probablemente querrá hacer lo mismo por él.

(Tenga cuidado de pasar un long como una longitud a memcpy() ; no lo sé con seguridad, pero ese código me parece bastante dudoso y de alto riesgo para un error de conversión / truncamiento de enteros).

buffer = php_base64_encode((unsigned char*) str, str_len, NULL);
for (pos = 0; pos < out_len; pos++) {
    if (buffer[pos] == '+') {
         ...
    } else {
         ret[pos] = buffer[pos];

Parece que podría leer más allá del final de buffer para mí. Creo que necesitas una verificación adicional para asegurarte de que pos sea más pequeño que la longitud de buffer .

score 1 · Answer 2

¡Muy buena iniciativa!

Comentarios rápidos
(Leeré el RFC con más detalle más adelante.)

'las sales solo necesitan ser únicas en un sistema' es una suposición errónea; por favor lea también: esta respuesta sobre el salado

No veo cuál es la necesidad de proporcionar manualmente un salt para el algoritmo BCrypt. Por lo que puedo ver, solo abre una opción para que las personas cometan errores innecesarios.

password_make_salt parece estar más relacionado con la familia de funciones crypt_; quizás cambiarle el nombre a crypt_make_salt en lugar de evitar confusiones.

el primer ejemplo de uso básico promueve el uso de un valor constante 'usesomesillystringfor', muchos desarrolladores cometen el error de usar algún tipo de valor constante en lugar de una sal adecuada (aleatoria).

El password_make_salt debe usar la mejor fuente aleatoria posible, criptográficamente segura si es posible (no estoy seguro acerca de la calidad php_win32_get_random_bytes (), el / dev / urandom es bueno)

el ejemplo user_needs_rehash.php no es muy legible; podría usar una limpieza.

el ejemplo de especificar_salt.php sugiere que puede proporcionar bytes aleatorios como un valor de sal válido para el algoritmo BCrypt. Esto contradice el requisito del alfabeto base64 personalizado para el BCrypt.

score 0 · Answer 3

Desde una perspectiva hash, el uso de Bcrypt es una de las técnicas más recomendadas en estos días, por lo que es bueno.

Permitir que el usuario elija usar otro algoritmo (como Scrypt, ¿y quizás su propia implementación más adelante?) también sería una gran opción: por defecto (y para desarrolladores de PHP no expertos), solo tendrán que usar El BCrypt predeterminado, y si lo desean, podrán adaptarse a sus necesidades.