safety-dance icon indicating copy to clipboard operation
safety-dance copied to clipboard

Audit curve25519-dalek

Open dbrgn opened this issue 4 years ago • 2 comments

The curve25519-dalek crate has almost 5 million downloads (1.3 million of those in the last 90 days). It is a building block for many crypto libraries.

The library only uses unsafe for the two SIMD-backends (avx2 / ifma). From the README:

//! The implementation is memory-safe, and contains no significant
//! `unsafe` code.  The SIMD backend uses `unsafe` internally to call SIMD
//! intrinsics.  These are marked `unsafe` only because invoking them on an
//! inappropriate CPU would cause `SIGILL`, but the entire backend is only
//! compiled with appropriate `target_feature`s, so this cannot occur.

However, it would still be good if someone would audit those usages and maybe upload a cargo-crev review.

dbrgn avatar Oct 20 '21 14:10 dbrgn

I see two things that could be done here:

  • Use the safe_arch crate to delegate the unsafe code to it (it provides safe wrappers for SIMD intrinsics)
  • Use the (nightly-only) portable SIMD types from the standard library, which get lowered into target-appropriate instructions.

I am not sufficiently familiar with SIMD and safe_arch crate to tell whether migrating to it is a good idea in this case.

Shnatsel avatar Mar 07 '22 12:03 Shnatsel

It already uses packed_simd(_2), which is the predecessor of std::simd, so it would probably make sense to eventually migrate to the implementation that's in nightly.

tarcieri avatar Mar 07 '22 16:03 tarcieri