Bug fix and additional tests for Gimli hash function

[tanmay2004]

Recently, I discovered that the gimli hash function implemented here in Rust was giving different outputs than the expected outputs of the original python implementation for some inputs.

So, I randomly generated many test cases of variable input length. With some help from @franziskuskiefer, I discovered that the output was wrong only when the length of the input was a multiple of 16, as is evident from the test cases in gimli_hash.rs! We can use these tests to verify the accuracy of future attempts to fix the bug.

Due to the large size of the new gimli hash tests, I have split the gimli tests into 2 files: one for testing the gimli cipher and the other for the gimli hash exclusively. I have shifted the hash KATs to gimli_hash.rs.

[karthikbhargavan]

great!

…

On 17 May 2021, at 14:21, Tanmay Garg ***@***.***> wrote: I have found and fixed the bug! It was in the gimli_hash_state function. https://github.com/hacspec/hacspec/blob/e146dc81f7ae284a73603ec9fde9fea7dbc95926/examples/gimli/src/gimli.rs#L78 <https://github.com/hacspec/hacspec/blob/e146dc81f7ae284a73603ec9fde9fea7dbc95926/examples/gimli/src/gimli.rs#L78> <https://user-images.githubusercontent.com/40921153/118485570-3c235e80-b736-11eb-81de-3e4db6e3bf59.png> The issue was that when the input length was a multiple of rate, 16 in this case, input.num_chunks(rate) returned length / rate when it should instead have been 1 more than that. As the picture below (taken from gimli's official documentation <https://csrc.nist.gov/CSRC/media/Projects/Lightweight-Cryptography/documents/round-1/spec-doc/gimli-spec.pdf> states, the input ends with one non-full (empty or partial) block. Thus, even a 0 block of input left must be processed as mentioned in the documentation. However, the gimli hash function currently did not consider that 0 block input and didn't process it, which led to errors in the final hash output when the input length was a multiple of 16. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#110 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFUVS5MCKED3A2ID4KI5OLTOEC5VANCNFSM447COJ3Q>.

[tanmay2004]

I have found and fixed the bug! It was in the gimli_hash_state function.

hacspec/examples/gimli/src/gimli.rs

Line 78 in e146dc8

for i in 0..input.num_chunks(rate) {

The issue was that when the input length was a multiple of rate, 16 in this case, input.num_chunks(rate) returned length / rate when it should instead have been 1 more than that. As the picture below (taken from gimli's official documentation) states, the input ends with one non-full (empty or partial) block.

Thus, even a 0 block of input left must be processed as mentioned in the documentation. However, the current gimli hash function currently did not consider that 0 block leftover and didn't process it, which led to errors in the final hash output when the input length was a multiple of 16.

I have made the fix in gimli.rs and it should mean that all tests added will pass!

[franziskuskiefer]

Thanks for finding and fixing this!
See comments inline.

Could you run rustfmt on the test files to make them look a little nicer?

[franziskuskiefer]

Thanks!