Incorrect output when generating more than 64 bytes

[camerondm9]

When comparing outputs and performance between this library and Konscious.Security.Cryptography.Argon2, I noticed that your output is different when requesting more than 64 bytes. I verified that your output is also different from that produced by antelle.net/argon2-browser, which means there is probably a bug in this library. This difference is present for all addressing modes (Argon2i, Argon2id, Argon2d).

There also isn't any kind of error when setting the memory cost to less than 8 (the minimum in the spec), but your output is different from Konscious.Security.Cryptography.Argon2 again. Not sure whose bug it is, but if you want to match the spec you should probably throw an ArgumentException when memory cost < 8.

[mheyman]

Thanks! Will look into it...

[mheyman]

I've looked into it...

• Memory cost translates into a target count of memory blocks to use for hashing. The actual memory block count minimum is 2*SyncPointCount*Lanes (SyncPointCount is defined as 4 in the spec which is where the 8 comes from when Lanes=1). The memory blocks actually used gets clipped to a valid range no matter the target memory cost. Oh, and a memory block is 1024 bytes and the memory size mentioned in the spec is only a target memory size too, it will get floored to 4*Lanes (and memory size is in blocks, not bytes).
  • I originally wrote this code a long time ago based on an early implementation written in C that, I think, became the first reference implementation. I remember getting confused by the "memory cost" verbiage back then so maybe I should have picked a different name but I fear the ship has sailed on that... I've checked the spec and it only mentions "memory cost" as a security consideration (and that is only in more recent versions of the spec).
  • <rant>I feel like if a coder would have been involved in Argon2 from the beginning, the configuration parameters wouldn't be so messy</rant>.
• I pulled in the 512-byte tag test vectors from Konscious.Security.Cryptography.Argon2 and the Isopoh implementation passes that.
  • I'm still looking into whether calling in a different way to get more than 64-bytes of output causes a failure (the unit tests only tests test vectors one way - I will fix that oversight 😁).
  • The code obviously needed to test greater than 64-byte results. I only had the spec test vectors to go by and the spec still only has 32-byte tag test vectors.

[mheyman]

Found it! The issue was the length passed into in the final blake2 block. That was a fun bug.

It is a breaking change unfortunately. Since I'm breaking things, I removed the $data field from the argon hash string for the additional data input. This must have been a pre-releasism I included back when I first wrote this code that got elided before the real reference code was released.

The code now builds the reference command line utility (slightly modified with some additional fields the original didn't allow setting). It then creates 5-600 "reference" vectors to check against (looking at corner cases) in a C# code generation build step. I'm pretty confident this code now behaves correctly at all hash lengths (the code does handle >2GB hashes on C#). I'm disappointed I didn't do this back at the start because I'm usually more aware of testing crypto code properly.

Currently I have a certificate issue pushing a nuget package - working on that...

[mheyman]

Fix pushed to nuget

[camerondm9]

Finally got back to testing this.

Looks like it's fixed. Thank you!