RustCrypto / hashes

Collection of cryptographic hash functions written in pure Rust
1.88k stars 253 forks source link

Kupyna: implemented hashing function #621

Open AnarchistHoneybun opened 1 month ago

AnarchistHoneybun commented 1 month ago

function spec

Scrapped previous PR since it ran into a lot of problems. I'm aware that #601 exists, creating this one to check against the repo's test suite as I iterate on my implementation, and potentially make it the main pr for kupyna if the other one becomes inactive.

Currently the implementation does everything the paper describes (testing leaves a bit to be desired) so step one is to clean up any linter complaints, and then proceed to making it no-std compatible.

AnarchistHoneybun commented 1 month ago

@newpavlov could you give me some idea on why the tests are failing right now? I don't think I've touched the Cargo.toml let alone the version of digest; checked and it's the same version as that present in some other toml's. Any idea why this could be happening?

AnarchistHoneybun commented 1 month ago

Might be a bit unrelated but when I try cargo run on my local it gives me this error:

error: failed to select a version for `digest`.
    ... required by package `kupyna v0.1.0 (/home/vrin/kupyna_hashes_contrib/kupyna)`
versions that meet the requirements `=0.11.0-pre.9` are: 0.11.0-pre.9

all possible versions conflict with previously selected packages.

  previously selected package `digest v0.11.0-pre.8`
    ... which satisfies dependency `digest = "=0.11.0-pre.8"` of package `ascon-hash v0.3.0-pre (/home/vrin/kupyna_hashes_contrib/ascon-hash)`

failed to select a version for `digest` which could resolve this conflict

now this worked fine before updating the toml. Post toml update the CI is fine, but the main function is failing. Is this expected behaviour?

newpavlov commented 1 month ago

Try to rebase your branch to master.

AnarchistHoneybun commented 1 month ago

I don't have a lot (read any) experience with rebasing, sadly. Could you explain a bit more?

newpavlov commented 1 month ago

You should be able to rebase to this repos master using something like this (it accounts for the fact that you've forked from a fork):

git remote add upstream git@github.com:RustCrypto/hashes.git
git pull upstream
git checkout main_repo_contrib
git rebase upstream/master
git push --force

Note that I do not guarantee correctness of these commands since I wrote them from memory.

AnarchistHoneybun commented 1 month ago

rebased and made the hex formatting changes. I was wondering if I should also format the mds and s-box matrices the same way, but i think it'd make me put hex-literal as a proper dependency instead of a dev dependency. Any particular way you want me to approach this?

AnarchistHoneybun commented 1 month ago

In the previous PR I was told to remove all heap allocations from the code. This is the code snippet that was flagged:

fn matrix_to_block(matrix: Matrix) -> Vec<u8> {
    let mut block = vec![0u8; ROWS * COLS];

This was when ROWS and COLS were constants, and the hash code length was fixed on 512. This has been made dynamic since, and looks something like this now:

fn matrix_to_block(matrix: Matrix) -> Vec<u8> {
    let rows = matrix.len();
    let cols = matrix[0].len();

    let mut block = vec![0u8; rows * cols];

where the matrix type doesn't have a predefined size. Is there a way to go around the heap allocations in this?

newpavlov commented 1 month ago

It's probably worth to make the output size a type parameter (i.e. Kupyna<OutputSize: ArraySize>) and calculate ROWS and COLUMNS from it. Alternatively, you could allocate the biggest block size on stack and then slice it to a smaller block, it certainly will be more efficient than using heap allocations.

AnarchistHoneybun commented 1 month ago

Could you explain the first approach a bit?

newpavlov commented 1 month ago

I took a look at the spec and I think that the best option will be to follow the groestl crate here, i.e. to define two structs KupynaShortVarCore and KupynaLongVarCore implementing the VariableOutputCore trait. You then can define type aliases for Kupyna256, Kupyna384, and Kupyna512 by using the appropriate wrappers. The compress function then can be implemented separately for l equal to 512 and 1024.