hynek / argon2-cffi

Secure Password Hashes for Python
https://argon2-cffi.readthedocs.io/
MIT License
549 stars 47 forks source link

Associated data cannot be specified in the low_level functions #143

Closed hbs closed 11 months ago

hbs commented 1 year ago

Given the test vectors for Argon2 contain additional data the fact that this field cannot be specified when calling argon2.low_level.hash_secret is bothering.

Would it be possible to expose this parameter in the low_level API?

hbs commented 1 year ago

Actually the secret cannot be set either, only the password. So as is the library is only providing partial compatibility with the Argon2 described in RFC 9106.

hynek commented 1 year ago

Hmmm, are you somewhat familiar with the Argon2 C library we’re wrapping? I believe that both these things aren’t exposed using their public higher-level APIs argon2_hash and argon2_verify which I’m trying to limit my usage to.

I’ve had people ask for keyed hashing in #25 and I hoped Argon2 would expand their public APIs, but sadly they’re not very active anymore and the only way currently is using half-public low-level APIs that are exposed by argon2-cffi-bindings, but that make me uncomfortable…

hynek commented 1 year ago

Going thru the docs for other reasons, I've just realized this is even documented: https://argon2-cffi.readthedocs.io/en/stable/api.html#argon2.low_level.core

hbs commented 11 months ago

I tried using the low level API to compute the hash for the 2id test vectors from RFC 9106 but the result does not match the one in the test vectors. Is there something I missed in my example?

The test vectors I used can be found here:

https://www.rfc-editor.org/rfc/rfc9106.html#name-argon2id-test-vectors

from argon2.low_level import (
    ARGON2_VERSION,
    Type,
    core,
    ffi,
    lib,
)

password = bytes.fromhex('0101010101010101010101010101010101010101010101010101010101010101')
salt = bytes.fromhex('02020202020202020202020202020202')
secret = bytes.fromhex('0303030303030303')
additional = bytes.fromhex('040404040404040404040404')

##
## Create Argon2_Context
##

hash_len = 32

cout = ffi.new("uint8_t[]", hash_len)
cpwd = ffi.new("uint8_t[]", password)
cad = ffi.new("uint8_t[]", additional)
csalt = ffi.new("uint8_t[]", salt)
csecret = ffi.new("uint8_t[]", secret)

ctx = ffi.new(
  "argon2_context *",
  {
    "out": cout,
    "outlen": hash_len,
    "version": ARGON2_VERSION,
    "pwd": cpwd,
    "pwdlen": len(cpwd),
    "salt": csalt,
    "saltlen": len(csalt),
    "secret": csecret,
    "secretlen": len(csecret),
    "ad": cad,
    "adlen": len(cad),
    "t_cost": 3,
    "m_cost": 32,
    "lanes": 4,
    "threads": 4,
    "allocate_cbk": ffi.NULL,
    "free_cbk": ffi.NULL,
    "flags": lib.ARGON2_DEFAULT_FLAGS,
  },
)

rv = core(ctx, Type.ID.value)

hash = bytes(ffi.buffer(ctx.out, ctx.outlen))

assert '0d640df58d78766c08c037a34a8b53c9d01ef0452d75b65eb52520e96b01e659' == hash.hex()
hbs commented 11 months ago

Ok, solved the issue, I needed to pass the lengths as len(array) - 1 to get rid of the final 0x00 byte.

hynek commented 11 months ago

Can this be close then or is there something I could add to argon2-cffi (/ its docs) to make any of this easier?

hbs commented 11 months ago

The issue can be closed, maybe add some documentation with the above example for people who want to access all the parameters of Argon2.

Thanks for providing your Python module.

hynek commented 11 months ago

I have replaced the existing examples with yours since it's demonstrating more options: https://github.com/hynek/argon2-cffi/commit/3d5eb74a38c01714a72da1a7181b13103a892dae

LMK if you feel like I've gotten something wrong (rendered version at https://argon2-cffi.readthedocs.io/en/latest/api.html#argon2.low_level.core)

hbs commented 11 months ago

You coud modify the low_level_hash signature to include t_cost, m_cost, lanes and threads as parameters

hynek commented 11 months ago

OK, done in 1380a12.

I'm afraid I won't come around implementing this stuff myself in the end. :( Originally I was hoping they'll add these parameters to their high-level functions but it doesn't look like they'll do anything past the absolute minimum.