search

serverless / serverless-graphql

Serverless GraphQL Examples for AWS AppSync and Apollo
https://www.serverless.com
MIT License
2.72k stars 364 forks source link

Passwords not salted #16

Closed mwawrusch closed 8 years ago

mwawrusch commented 8 years ago

Hi guys,

I just glanced over the code, and it looks like the passwords are not salted - basically each user needs an individual random salt string that is added to the password and used within the hash function.

eahefnawy commented 8 years ago

@mwawrusch thanks for the heads up! You're absolutely right! I thought the secret should be enough, but salting each user is definitely a better practice.

kennu commented 8 years ago

I would recommend against hashing passwords, even with salt, as they are fairly easy to crack.

Instead, a key derivation function like https://www.npmjs.com/package/bcrypt-nodejs can provide much better security. (I've used that package with Serverless; it doesn't require native libraries.)

marksteele commented 8 years ago

+1

Salted passwords are much much harder to crack than unsalted ones (salted password protect against rainbow table lookups and brute force). That being said, key derivation functions (like bcrypt) will make it all but impossible for an attacker who might gain access to the password database to be able to reverse the passwords that are stored. It will however increase the runtime of the functions that check the password proportionally to the number of derivation iterations. The point of key derivation functions is to make them computationally expensive and slow.

brettstack commented 8 years ago

+1

This should be the default, but maybe we should mention the hashing optionin the README since Lambda charges based on computation-time. Users can do their own cost-benefit analysis.

On Mon, 28 Mar 2016 at 13:17 Mark Steele notifications@github.com wrote:

+1

Salted passwords are much much harder to crack than unsalted ones (salted password protect against rainbow table lookups and brute force). That being said, key derivation functions (like bcrypt) will make it all but impossible for an attacker who might gain access to the password database to be able to reverse the passwords that are stored. It will however increase the runtime of the functions that check the password proportionally to the number of derivation iterations. The point of key derivation functions is to make them computationally expensive and slow.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/serverless/serverless-boilerplate/issues/16#issuecomment-202564669

mwawrusch commented 8 years ago

+1 for bcrypt - should have mentioned that ;-)

eahefnawy commented 8 years ago

bcrypt and salting added! Thank you all for your feedback! :blush: