Argon2 confusing param mapping between pwdhash and pwdhash_timed

[dewyatt]

I could be missing something, but I found this to be confusing:

#include <botan/ffi.h>
#include <string>
#include <vector>
#include <cassert>

int main(int argc, char *argv[]) {
  size_t param1, param2, param3;
  const std::string password("password");
  std::vector<uint8_t> buf(16);
  std::vector<uint8_t> salt(8); // pretend this is ok
  assert(0 == botan_pwdhash_timed("Argon2id", 100, &param1, &param2, &param3,
                                  &buf[0], buf.size(), password.c_str(),
                                  password.size(), salt.data(), salt.size()));

  printf("param1: %zd\nparam2: %zd\nparam3: %zd\n", param1, param2, param3);

  assert(0 == botan_pwdhash("Argon2id", param1, param2, param3, &buf[0],
                            buf.size(), password.c_str(), password.size(),
                            salt.data(), salt.size()));
}

This will trigger an assertion on the second call because the parameters (as in param*) do not map in the same order between these functions.

botan_pwdhash uses from_params which constructs Argon2_Family with M, t, p, but botan_pwdhash_timed returns iterations(), parallelism(), memory_param() (t, p, M).

[randombit]

Oh that is terrible. Sorry. I think Scrypt is affected in the same way.

I think strictly SemVer speaking we can't change this. But we can add a new variant of botan_pwdhash_timed that does it correctly (and with a test to verify we get it right this time), then deprecate the current one.

[dewyatt]

Yeah I saw scrypt has the same issue after I posted this :(. I should note that I'm using these APIs via botan-rs (derive_key_from_password*). How do you plan to handle it on that end?

[randombit]

Re botan-rs I'm not positive, but first inclination would be basically same approach as in FFI, add an alternate API, deprecate current. In botan-rs case we are not 1.0 so I would probably remove the current versions after a release or three.