RustCrypto / hashes

Collection of cryptographic hash functions written in pure Rust
1.82k stars 247 forks source link

Inline ASM sha1 #450

Closed laudiacay closed 1 year ago

laudiacay commented 1 year ago

draft.....

laudiacay commented 1 year ago

partially will address https://github.com/RustCrypto/asm-hashes/issues/45 per https://github.com/RustCrypto/hashes/pull/449#issuecomment-1407518944

laudiacay commented 1 year ago

@newpavlov having a little trouble getting the q6 register back into state[0..4]. I have gotten everything else done for the aarch64 implementation (well, we'll find out for sure when I can run it).

Do you know the right syntax for this? Everything I've tried (state.as_mut_ptr(), state[0].as_mut_ptr(), state[0..4].as_mut_ptr().... several other iterations) says "cannot assign to this". I tried using the "v6.1s" syntax to individually assign u32s to indices and apparently those aren't valid registers for in/out.

problem line in question: https://github.com/RustCrypto/hashes/blob/8b8e56c9aa15f0f4a5d724d9a2ec8caea125d11e/sha1/src/asm/aarch64.rs#L265

newpavlov commented 1 year ago

I am not very familiar with ARM assembly, so I can not comment on that. But lateout("q6") state as *mut u32, places pointer to the register, while I would expect you to place value from the state directly into it. The compress function should look roughly like this:

pub fn compress(state: &mut [u32; 5], blocks: &[Block<Sha1Core>]) {
    let [mut s0, mut s1, mut s2, mut s3, mut s4] = *state;
    unsafe {
        // you can use different register names
        asm!(
            // assembly impl
            inout("q0") s0,
            inout("q1") s1,
            inout("q2") s2,
            inout("q3") s3,
            inout("q4") s4,
            in("q5") blocks.as_ptr(),
            in("q6") blocks.len(),
            // clobbers
            options(pure, nostack),
        );
    }
    *state = [s0, s1, s2, s3, s4];
}

Also note that the ARM assembly relies on instructions from the crypto extension. You should add runtime detection using the cpufeatures crate and use the software backend as a fallback.

i don't know why the "do not enable by library crates" warning is in the asm feature

The idea is that the choice should be made by binary crates. Otherwise it will be hard for binary crates to change it if a library unconditionally enables such feature.

newpavlov commented 1 year ago

Closing as stale. Feel free to reopen it or create a new PR, if you would like to resume to work on this problem.