PasswordHashFamily::from_params documents Argon2 parameters out-of-order

[ghost]

According to the docs,

Argon2{i,d,id} would use iterations, memory, parallelism for i{1-3}

However, it seems like iterations and memory are switched here. Take this snippet:

std::unique_ptr<Botan::PasswordHashFamily> family = Botan::PasswordHashFamily::create("Argon2id");
std::unique_ptr<Botan::PasswordHash> hash = family->from_params(16, 1000000, 1);

std::cout << "Iterations: " << hash->iterations() << std::endl;
std::cout << "Memory: " << hash->memory_param() << std::endl;

The docs say that this should output:

Iterations: 16
Memory: 1000000

but it actually outputs:

Iterations: 1000000
Memory: 16

This error is present both in the handbook and the internal function docs. Seems like the memory and iterations need to be swapped, whether it be in the docs or the function itself.

It's also worth noting that the docs for this function could be a lot better, honestly. Something like this:

Create a password hash using some scheme specific format. Parameters are as follows:
- For PBKDF2, PGP-S2K, and Bcrypt-PBKDF, i1 is iterations
- Scrypt uses N, r, p for i{1-3} (potentially include a link to what these parameters do, such as here: https://blog.filippo.io/the-scrypt-parameters/)
- Argon2 family uses memory (in KB), iterations, and parallelism for i{1-3}
All unneeded parameters should be set to 0 or left blank.

[AtomicNibble]

I also just ran into this, in addition Argon2_Family::from_iterations is also passing iterations as memory with a iteration count of 1 resulting in:

Memory: iter
Iterations: 1
parallelism: 1

Which I think is too low.

Personally I think the order should be changed in the code instead of the docs. So: Argon2_Family::from_params(size_t M, size_t t, size_t p) const Becomes: Argon2_Family::from_params(size_t t, size_t M, size_t p) const

But that would be a breaking change :(

[droidmonkey]

A code change would be a disaster without changing the major version. The docs just need to be updated... simple, cheap, effective.

[ghost]

That's what I was thinking. Absolutely no reason to change code when changing the documentation is an option as well

[AtomicNibble]

Well I'm not sugesting a code change should actually be made, just in a ideal world.

Since currently from_params for Argon2 is the only one that is not using the first param as iterations so is inconsistent.