jedisct1 / rust-aegis

AEGIS high performance ciphers for Rust.
MIT License
24 stars 4 forks source link

Fix Aegis128LMac alignment #4

Closed tomaszstrejczek closed 4 months ago

tomaszstrejczek commented 4 months ago

Libaegis aegis128l_mac* C functions expect the aegis128l_state data to be aligned to 32-bytes boundary.

jedisct1 commented 4 months ago

That shouldn't be the case. Can you clarify why you think aegis128l_state requires a specific alignment?

tomaszstrejczek commented 4 months ago

Please look into state_init() in aegis128l_common.h. At the beginning of the function there is:

    _aegis128l_state *const st =
        (_aegis128l_state *) ((((uintptr_t) &st_->opaque) + (RATE - 1)) & ~(uintptr_t) (RATE - 1));

RATE is set to 32. Without alignment, the test I provided fails on Windows with MSVC, rustc 1.78.0.

jedisct1 commented 4 months ago
_aegis128l_state *const st =
        (_aegis128l_state *) ((((uintptr_t) &st_->opaque) + (RATE - 1)) & ~(uintptr_t) (RATE - 1));

What that line does is to align the structure to 32 bytes, if it's not already aligned. So it's not expected to be aligned.

Maybe aegis128l_state is not large enough? What happens if you make it larger?

the test I provided fails

How does it fail exactly?

jedisct1 commented 4 months ago

Maybe the issue is in clone() (that calls aegis128l_mac_state_clone()) ?

Are you using the current version (0.6.5) ?

jedisct1 commented 4 months ago

I think the issue is that Rust silently makes copies of aegis128l_state.

For example in the new function, st has different addresses when it is initialized and when it's part of the Aegis128LMac structure.

We should probably add Pin, Box and other horrors.

jedisct1 commented 4 months ago

Can you check if f60b5aa5f2ef5ad6fac24c387c8ac04bc8d3e7ff fixes your issue?

jedisct1 commented 4 months ago

Using Box works. But people are going to complain that it doesn't work with no_std.

I guess the only thing we can do is change the upstream library to add an alignment hint to the type and make the promise that a state can be copied without calling the state_clone function if it is guaranteed to be properly aligned.

jedisct1 commented 4 months ago

Version 0.6.6 should fix this.

Thanks!

tomaszstrejczek commented 4 months ago

It fixed. Thank you.