Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Salt Implementación #8

Open
oscarmcm opened this issue Aug 1, 2014 · 21 comments
Open

Salt Implementación #8

oscarmcm opened this issue Aug 1, 2014 · 21 comments
Labels

Comments

@oscarmcm
Copy link
Member

oscarmcm commented Aug 1, 2014

exports.passwordHash = function( password ){ var passwordHash = crypto.createHash('md5').update( password + '_' + config.salt ).digest('hex'); return passwordHash;}

Estan usando una mala implementacion de salt, ya que se esta ejecuntando el mismo salt para multiples hash. Por ejemplo si se usa la mismas salt sobre cada hash y dos usuarios poseen las mismas passwords entonces ellos tienen las mismas hash, tomando esto en cuenta un ataque de diccionario sobre las hash sera fácil de romper y obtener

siguiendo el principio de Kerckhoffs's un atacante no atacara a la hash pero si usualmente tiene acceso al código fuente, entonces puede obtener el password-hash y no seria difícil aplicar algo de ingeniería inversa a el algoritmo

la forma correcta de como hacer la hash es que cada password sea generada mediante CSPRNG ( Cryptographically Secure Pseudo-Random Number Generator ) básicamente el salt debería ser UNICO para password de usuario, cada ves que un usuario cambie la contraseña deberá de ser hashed usando una función random.

NUNCA PERO NUNCA REHÚSES UNA SALT

@paulomcnally
Copy link
Member

si la salt es un md5 y el password es una cadena de texto, cómo el atacante puede suar un diccionario?

Si mi salta es 12345678901234567890123456789012 y al combinar el salt com mi password sería algo como md5( '12345678901234567890123456789012_soyunpassword' ), que asu ves esa conversión daría: ad4a194b30fb9046cd26395ed01a01c4 (que es la conversión a md5 de la cadena anterior).

Por ejemplo el sat que utilizamos es el contenido de una página web completa convertido a 32 caracteres de un md5. Cómo un diccionario podría tener mi salt?

@paulomcnally
Copy link
Member

Acabo de terminar comprendiendo lo de la ingeniería inversa.

Cómo harías vos el código para la implementación de un hash seguro?

Saludos

@amosrivera
Copy link
Member

Bien ahí Oscar, que fiera que estás leyendo el código.

Ya aprendí algo nuevo con lo del principio de Kerckhoffs* (el 's es por el
posesivo Kerckhoffs's principle). Ojo que que el 'enemigo' conozca el
sistema no quiere decir que conoce el hash pero lo que decís es cierto.

La idea creo que sería guardar un salt random por cada usuario en la bd que
se genera por cada contraseña. Una googleada rápida me manda al modulo de
Crypto:
http://nodejs.org/api/crypto.html#crypto_crypto_randombytes_size_callback
para la generación.

Y siempre usando un salt global sería algo como encriptar(salt_global,
salt_por_usuario, contraseña). La idea de mantener el salt global es por si
el atacante tiene acceso a la bd, todavia necesitaria el salt global para
un ataque.

Te apuntas a mandar un PR Oscar?

@oscarmcm
Copy link
Member Author

oscarmcm commented Aug 1, 2014

Tambien hay un paquete llamado bcrypt

https://www.npmjs.org/package/bcrypt

para hacerla usando bcrypt seria algo asi

bcrypt.genSalt(10, function(err, salt) {
    bcrypt.hash("B4c0/\/", salt, function(err, hash) {
        // Store hash in your password DB.
    });
});

@paulomcnally
Copy link
Member

Amós, pero es que si vamos a tener otros hash más en la db nos preocupa que si el atacante ya tiene acceso a la base de datos, esto no lo haría más seguro.

@paulomcnally
Copy link
Member

Awebo que sería bcrypt la jugada.

Esto implica que cada usuario deberá generar una nueva contraseña, entonces no es solo implementar, también generarles un forgot url a los usuarios y notificarles que procedan a hacer el cambio.

@oscarmcm si haces pull request tendrás que esperar a que preparemos el correo para los usuarios.

Amós, si lo haces vos por favor no lo mandes a master, lo podes dejar en una rama.

Gracias por el consejo Oscar, esto cae demasiado bien a todos los cambios que se vienen en el source de NodeNica.

Saludos.

@amosrivera
Copy link
Member

@paulomcnally creo que la idea en el escenario inicial no es que el atacante tenga acceso a la bd. Es que sepa cual es el algoritmo para generar los passwords. De que otra manera tendríamos salts generados por un crypto generator por cada password sin guardarlos en la bd?.

Y si sí los guardamos en la bd, necesitamos tener un salt global por si el atacante tiene acceso a la bd. De la misma manera si el maje tiene acceso al salt global y no tiene la bd tampoco puede atacar. Ver por ejemplo: http://stackoverflow.com/questions/2999197/do-i-need-a-random-salt-once-per-password-or-only-once-per-database

@paulomcnally
Copy link
Member

Comprendo @amosrivera sin embargo por experiencia, si tengo acceso a "A" es muy probable que se me haga fácil tener acceso a "B" y deseamos que aún teniendo acceso a "A", "B" o "C" los password estén seguros (en la medida de lo posible).

@amosrivera
Copy link
Member

@paulomcnally En la medida de lo posible es lo que se puede hacer. Si alguien obtiene la base de datos y el código fuente con los archivos de configuración y que todavía no pueda hacer un ataque con un diccionario? Qué propones?

Lo que hay que hacer es tratar de que eso no pase creo. No hay mucho más.

@amosrivera
Copy link
Member

Ah y sí, bcrypt es la jugada!

@paulomcnally
Copy link
Member

@amosrivera lo que capté de lo que decía @oscarmcm es que si usamos algo como:

var salt = 'patito';
var amos_password = '12345';
var atacante_password = '12345';

var amos_hash = '6f6c586ac60d55e229a5fa724f6c2a19'; // patito_12345 convertido a md5
var atacante_password = '6f6c586ac60d55e229a5fa724f6c2a19';

Entonces yo como atacante inserto en la base de datos "N" registros usando como password el texto del diccionario fácilmente puedo identificar de que usuario autentico mis registros tienen el mismo hash de password.

Ahora bien, si tengo un campo unique_salt en la base de datos y es distinto en cada usuario al final comienzo con mi ataque de fuerza bruta a insertar "N" registros usando el salt único de cada usuario, si tengo 10 usuarios y 20 palabras en mi diccionario ahora no serían 20 usuarios no autenticos del atacante, serían 200 por que voy a intentar averiguar con que palabra pego el password del user.

Ahora sería patito_[unique_salt]_12345

@oscarmcm
Copy link
Member Author

oscarmcm commented Aug 1, 2014

usr1. usa "node" de pass y usr2 usa "node" de pass, entonces esto produce la misma hash haciendo la salt inutil, perdiendo el punto total de hacer salting.

una googleada y esta fiera como lo hace bcrypt

// Load the bcrypt module
var bcrypt = require('bcrypt');
// Generate a salt
var salt = bcrypt.genSaltSync(10);
// Hash the password with the salt
var hash = bcrypt.hashSync("pass", salt);

@paulomcnally
Copy link
Member

@oscarmcm te comento que @amosrivera ya está trabajando en la integración de bcrypt 👍

@oscarmcm
Copy link
Member Author

oscarmcm commented Aug 1, 2014

Fiera!! cuando lo solucionen me avisan 😃

@paulomcnally
Copy link
Member

Claro, por correo te llegará una notificación diciendo que cambies tu password hahaha.

@paulomcnally
Copy link
Member

Es el que está utilizando loopback :) https://github.com/strongloop/loopback/blob/master/lib/models/user.js#L453

@amosrivera amosrivera added the bug label Aug 8, 2014
@paulomcnally
Copy link
Member

Resolveremos esto usando #loopbackJS

https://github.com/nodenica/nodenica-api

Hoy empiezo con el API.

@paulomcnally
Copy link
Member

Resuelto en el API.

@oscarmcm
Copy link
Member Author

oscarmcm commented Dec 8, 2014

Serias tan amable estimado @paulomcnally de mostrar como lo resolvistes, gracias 😄

@paulomcnally
Copy link
Member

Estamos repensando nodenica y posiblemente no se implemente el branch develop, así que si seguimos usando el codigo que esta en master debemos solucionar este bug en esta rama.

@paulomcnally paulomcnally reopened this Dec 29, 2015
@oscarmcm oscarmcm mentioned this issue Dec 31, 2015
7 tasks
@paulomcnally paulomcnally changed the title Salt Implementacion Salt Implementación Dec 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants