[WIP] Use zerocopy byteorder module to replace endianness stuff

[joshlf]

@briansmith Here's a prototype of replacing endianness support with zerocopy. It's mergeable as-is, but there are two gaps to mention which are worth calling out:

• There's still a dependency on the byteorder crate
• The byteorder types have no alignment, which may pessimize codegen

Both of these are on our roadmap to fix.

[codecov]

Codecov Report

Merging #1693 (0b237cb) into main (3a77fe1) will increase coverage by 3.89%.
Report is 280 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1693      +/-   ##
==========================================
+ Coverage   92.07%   95.97%   +3.89%     
==========================================
  Files         132      133       +1     
  Lines       18921    20012    +1091     
  Branches      199      199              
==========================================
+ Hits        17422    19206    +1784     
+ Misses       1463      767     -696     
- Partials       36       39       +3     

Files	Coverage Δ
src/aead/aes.rs	98.65% <100.00%> (+17.88%)	⬆️
src/aead/block.rs	97.22% <100.00%> (ø)
src/aead/chacha20_poly1305.rs	93.16% <100.00%> (ø)
src/aead/chacha20_poly1305_openssh.rs	87.50% <100.00%> (-0.60%)	⬇️
src/digest.rs	97.32% <100.00%> (+0.29%)	⬆️
src/lib.rs	70.83% <ø> (+36.21%)	⬆️

... and 23 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[briansmith]

In general this is looking really good. It would be good for us to talk through the timing side channel stuff to see what we can do about that.

[joshlf]

In general this is looking really good. It would be good for us to talk through the timing side channel stuff to see what we can do about that.

Sounds good. Can you say more about which operations you're thinking about wrt timing side channels? I see that there's an impl of BitXorAssign in endian.rs and there are byte order swapping operations, but there are also methods to expose those types as e.g. byte arrays; I presume this means that some operations happen on the byte array representation, which the code in endian.rs (and similarly in zerocopy) doesn't have any control over.

[briansmith]

Sounds good. Can you say more about which operations you're thinking about wrt timing side channels? I see that there's an impl of BitXorAssign in endian.rs and there are byte order swapping operations,

Byte order swapping, XOR, and eventually we'll need to write a constant-time comparison of arrays. I think right the constant-time comparison is done using the byte array representation but we're hoping we'll be able to make use of SIMD for the comparison in the future, probably by using inline assembly.

[briansmith]

By "constant-time comparison," we mean "a == b" in constant time for an array of u8/BEU32/etc./values.

[joshlf]

Makes sense. My gut is that it wouldn't make sense for zerocopy make any promises about constant time-ness because the compiler can always undermine those, and it'd make more sense to do any operations that need to be guaranteed constant time on the byte representation. (Obviously all of the standard caveats about constant time execution technically being impossible without formal language support, the compiler can do whatever it wants, etc etc).

[joshlf]

What would it take to gain confidence in performance and/or constant time-ness of this PR? I still need to write the changes to get rid of the byteorder crate dependency, but it'd be good if I can just do that after we've confirmed that this PR even makes sense from a performance and constant time-ness perspective.

[briansmith]

Once we import in the benchmarks for ring::{digest,hmac,pbkdf2} then we can evaluate the performance of PR #1731. I expect that even if PR #1731 as written has performance issues, we will be able to tweak it so it doesn't. This will almost definitely eliminate this code completely.

[briansmith]

PR #1731 will (I hope) address this as well, by making the code 100% safe as long as we are happy with the safety guarantees for the code that writes to the self.state union.

Which functions within digest need to be marked unsafe and which can be safe with unsafe blocks is something that could use discussion as it seems we may disagree, which means I might be misunderstanding something.