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

Params limits too low - Invalid memoryCost, must be between 2048 and 4294967295. #303

Closed
rarg opened this issue Jun 7, 2021 · 7 comments

Comments

@rarg
Copy link

rarg commented Jun 7, 2021

Steps to reproduce

  1. Call the hash function with the following parameters
const argon2 = require('argon2');

const options= {
    parallelism: 1,
    timeCost: 1,
    memoryCost: 128
  };

const hash = await argon2.hash("password", options);

Expected behaviour

Argon computation is done succesfully.
It worked on version 0.27.2 and lower.

Actual behaviour

On version 0.27.3 and higher, the following errors happens :
ERR_ASSERTION - Invalid memoryCost, must be between 2048 and 4294967295.
ERR_ASSERTION - Invalid timeCost, must be between 2 and xxxx.

Environment

Operating system:
Unix (ubi8)
Node version:
14.17.0
Compiler version:
?
node-argon2 version:
0.27.3 and superior

Comments

According to this commit minimum parameters have been increased for security reason.

This is understandable, as increasing security is a good practice for all systems.

But this behavior is out of the specs of the Argon2 algorithm (top of page 5).
According to the specs, timeCost minimum value can be 1,
and memoryCost minimum is 8 x timeCost (thus 8 is the lowest value).

I see several solution to this issue.

  • Solution 1 : set the limits on the same values as described in the specs
  • Solution 2 : set the limits down + create a warning when values are too low (according to the new thresholds [2 & 2048])
  • Solution 3 : add a new option (~assertLimits = false) allowing to disable the checks on the parameters

All these solutions have pros and cons, being more or less standard, adding security etc.
If you see other solution, please add a comment.

@ranisalt please give me your opinion on this issue.

If necessary, after chosing what's the best way, I can provide a PR (I can't dev in C++).

Thanks

@rarg rarg changed the title Params limits - Invalid memoryCost, must be between 2048 and 4294967295. Params limits too low - Invalid memoryCost, must be between 2048 and 4294967295. Jun 7, 2021
@ranisalt
Copy link
Owner

ranisalt commented Jun 7, 2021

We have strengthened the minimum parameters due to a potential attack on the Argon2 algorithm, described here: https://github.com/ALRBP/Attack-Argon2i

The author of the attack suggests that the minimum parameters to avoid generating weak hashes are 2 passes for time cost, and 2048 KB of memory per call.

My solution would be 4) use needsRehash and rehash your passwords if they were previously hashed with now-considered-unsafe parameters. Therefore, the library won't be able to produce new hashes using lower safety, but still is able to verify them - and rehash.

If you do need to hash using unsafe parameters, I would pick 3 and name the option much more explicitly, like React's dangerouslySetInnerHTML or Sway's --my-next-gpu-wont-be-nvidia so it screams unsafety in the code.

@romansavchenko
Copy link
Contributor

Is this a problem only for argon2i, or also for argon2id?

@ranisalt
Copy link
Owner

Is this a problem only for argon2i, or also for argon2id?

Both have the same minimum requirements. What is the use case for weak parameters?

@romansavchenko
Copy link
Contributor

romansavchenko commented Jun 17, 2021

The use case is for incredibly high throughput systems that need to keep response time down.
The report in that attack repo even suggests that We suggest using >1MiB. Using greater amounts of memory increases security ... using too much memory space can make authentication problematic in some use cases, taking resources from effective services or inducing long waiting time when an authentication server is under heavy load. It can also induce sensitivity to some kinds of DDos Attacks. page 19 (should the min in the repo at least go down to 1024kb?)

Im curious why the memory is limited in particular.
I was under the impression that you can get the same security benefits by using lower memory and increasing the number or iterations?

I haven't seen similar memory restrictions in other argon2 projects (such as https://github.com/phxql/argon2-jvm).
Is there any more information beyond https://github.com/ALRBP/Attack-Argon2i ?
Has a similar attack been show to be effective on argon2id?

@ranisalt
Copy link
Owner

The wording in the paper is not crystal clear, and >1MiB to me means exactly 1MiB is still not enough. Since the memory is in powers of two, 2 MiB is the next step, thus it was selected as the minimum. I would accept a merge request reducing the minimum to 1MiB (you have to change this line), and if you would like to use even lower memory cost, I would prefer to have an option as described in my first reply.

@romansavchenko
Copy link
Contributor

Opened #304

@ranisalt
Copy link
Owner

#304 solves this issue

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

No branches or pull requests

3 participants