Skip to content

replace bcrypt with scrypt #1015

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AdityaKirad
Copy link
Contributor

refactor(auth): replace bcrypt with much better password hashing algorithm built into node crypto module for password hashing and validation
refactor(validation): update password schema to reflect scrypt limits
refactor(tests): use crypto for password hashing in test utilities

…rithm built into node crypto module for password hashing and validation

refactor(validation): update password schema to reflect scrypt limits
refactor(tests): use crypto for password hashing in test utilities
@kentcdodds
Copy link
Member

I love built-in solutions. Thanks!

  1. Could you write a decision doc? Include within it, a migration guide
  2. Could you check why the tests are failing?

@AdityaKirad
Copy link
Contributor Author

I love built-in solutions. Thanks!

  1. Could you write a decision doc? Include within it, a migration guide
  2. Could you check why the tests are failing?

sure, I will write a decision doc also looks like in scryptSync is consuming memory then what Github actions runner is providing as per my calculation with the current config it should password hashing will consume 32mb memory should I try reducing the cost

@kentcdodds
Copy link
Member

How does that compare with bcrypt? That sounds like a lot of memory.

@AdityaKirad
Copy link
Contributor Author

AdityaKirad commented Jun 9, 2025

bcrypt was specifically designed to CPU hard by increasing the cost we can increase the CPU time it is resistant to the GPU brute force attack but it's not memory hard also we already know that in bcrypt there is limitation over how many bytes password can have while scrypt is designed to memory hard with N,r,p parameters we can increase the CPU time and memory hardness also due to its memory hardness it very hard for attackers using GPUs, ASICs also unlike the bcrypt it doesn't have any limit over the bytes in password basically scrypt makes very hard for attackers for cracking the password

@AdityaKirad
Copy link
Contributor Author

I also plan to add the zxcvbn library by dropbox we already have checkIsCommonPassword function which checks if the password is leaked or not but sometimes in some case it can fale because password may contain some patterns which are not leaked yet and this library is designed to find patterns in passwords etc https://github.com/dropbox/zxcvbn

@AdityaKirad
Copy link
Contributor Author

I forgot to mention how much memory bcrypt consumes since it's memory light it typically consumes around 4kb of memory per hash operation sometimes even less

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this! I'm a little bit concerned that this is going to cause issues for people running the Epic Stack within the free allowances. Can you show me what the Firecracker graphs look like for memory when passwords are being checked?

@AdityaKirad
Copy link
Contributor Author

Sure, but I think there will no problem for people running Epic Stack within free allowances as the scrypt parameters carefully choosed so that password safety don't get compromised and still stay within free limits for eg vercel in their free plan gives 1GB of memory for Nodejs Functions

@kentcdodds
Copy link
Member

I'm thinking about Fly.io (which is where the Epic Stack is deployed). They have unofficial "forgiveness" of charges less than $5, https://fly.io/calculator

That gives you 256GB of memory. It's a tight budget for Node.js-based apps like ours. Most people have to bump that up pretty quick for real stuff, but I'd like simple projects/demos to be able to stay under that threshold.

@AdityaKirad
Copy link
Contributor Author

Hey Kent, I don't know much about the Firecracker Graph you are talking about but this something I recorded in the performance tab memory checkbox enabled during the login attempt with email-password and current scrypt config
image

@andrecasal
Copy link
Contributor

andrecasal commented Jun 14, 2025

You can see the Firecracker graph here: https://fly.io/apps/your-app-name/metrics

There’s also a button on the top right of each card that takes you to Graphana.
IMG_0051

@AdityaKirad
Copy link
Contributor Author

But this is specifically for fly right I was saying something during the development so that we can know how much memory we are consuming

@kentcdodds
Copy link
Member

I'm not concerned about development. People have plenty of memory on their machines. I'm concerned about production.

@AdityaKirad
Copy link
Contributor Author

well in that case then you only can test the app in production as I don't have fly account, and I think the unofficial "forgiveness" of charges less than $5 only applies to the account which were created before the introduced pay as you go pricing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants