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

changes memlimit to interactive #6

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

willemolding
Copy link
Contributor

Closes #4

I was previously getting around this with a git patch but I see the need to make this module npm install-able soon.

The errors returned by libsodium are very unhelpful so I have no idea why it is crashing with the large memory use setting.

Would it be ok to have this setting for closed alpha or at least until we figure out why it is not working with the suggested settings?

Copy link

@neonphog neonphog left a comment

Choose a reason for hiding this comment

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

@willemolding - can you expose the opt to the caller and have it still default to sensitive?

also, does moderate work without errors?

@willemolding
Copy link
Contributor Author

So it looks like MODERATE works fine. It takes around 2.5 seconds to hash a password.

I guess the purpose of this PR is to establish if we definitely need the SENSITIVE variant and in that case how to figure out the root of the problem, or if a less memory intensive variant is ok for the closed alpha. I'm not sure if it makes sense to let the calling code decide this if it leads to insecure password hashing.

What are your thoughts @neonphog ?

@neonphog
Copy link

neonphog commented Mar 5, 2019

@willemolding I think MODERATE should be ok for the MEM for the Holo use-case (especially for closed alpha) so long as we keep the cpu limit at SENSITIVE. We're a little constrained by the wasm heap as far as memory goes, and I don't think it's worth mucking with that.

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.

In pwHash, sodium.crypto_pwhash_MEMLIMIT causes error when set to SENSITIVE
2 participants