Merge pull request #1661 from briansmith/revert-1657-unsafe-cleanup

Revert "Document or remove some uses of `unsafe`"

Author: briansmith

Cargo.toml

@@ -168,7 +168,6 @@ name = "ring"
 [dependencies]
 getrandom = { version = "0.2.8" }
 untrusted = { version = "0.9" }
-zerocopy = { version = "0.7.5", features = ["derive"] }
 
 [target.'cfg(any(target_arch = "x86",target_arch = "x86_64", all(any(target_arch = "aarch64", target_arch = "arm"), any(target_os = "android", target_os = "fuchsia", target_os = "linux", target_os = "windows"))))'.dependencies]
 spin = { version = "0.9.2", default-features = false, features = ["once"] }

src/aead/gcm.rs

@@ -126,12 +126,6 @@ impl Context {
         debug_assert!(input_bytes > 0);
 
         let input = input.as_ptr() as *const [u8; BLOCK_LEN];
-        // SAFETY:
-        // - `[[u8; BLOCK_LEN]]` has the same bit validity as `[u8]`.
-        // - `[[u8; BLOCK_LEN]]` has the same alignment requirement as `[u8]`.
-        // - `input_bytes / BLOCK_LEN` ensures that the total length in bytes of
-        //   the new `[[u8; BLOCK_LEN]]` will not be longer than the original
-        //   `[u8]`.
         let input = unsafe { core::slice::from_raw_parts(input, input_bytes / BLOCK_LEN) };
 
         // Although these functions take `Xi` and `h_table` as separate

src/digest.rs

@@ -24,9 +24,12 @@
 // The goal for this implementation is to drive the overhead as close to zero
 // as possible.
 
-use crate::{c, cpu, debug, endian::BigEndian, polyfill};
+use crate::{
+    c, cpu, debug,
+    endian::{ArrayEncoding, BigEndian},
+    polyfill,
+};
 use core::num::Wrapping;
-use zerocopy::{AsBytes, FromBytes};
 
 mod sha1;
 mod sha2;
@@ -111,9 +114,7 @@ impl BlockContext {
 
         Digest {
             algorithm: self.algorithm,
-            // SAFETY: Invariant on this field promises that this is the correct
-            // format function for this algorithm's block size.
-            value: unsafe { (self.algorithm.format_output)(self.state) },
+            value: (self.algorithm.format_output)(self.state),
         }
     }
 }
@@ -247,11 +248,8 @@ impl Digest {
 impl AsRef<[u8]> for Digest {
     #[inline(always)]
     fn as_ref(&self) -> &[u8] {
-        let data = (&self.value as *const Output).cast::<u8>();
-        // SAFETY: So long as `self.algorithm` is the correct algorithm, all
-        // code initializes all bytes of `self.value` in the range `[0,
-        // self.algorithm.output_len)`.
-        unsafe { core::slice::from_raw_parts(data, self.algorithm.output_len) }
+        let as64 = unsafe { &self.value.as64 };
+        &as64.as_byte_array()[..self.algorithm.output_len]
     }
 }
 
@@ -272,9 +270,7 @@ pub struct Algorithm {
     len_len: usize,
 
     block_data_order: unsafe extern "C" fn(state: &mut State, data: *const u8, num: c::size_t),
-    // INVARIANT: This is always set to the correct output for the algorithm's
-    // block size.
-    format_output: unsafe fn(input: State) -> Output,
+    format_output: fn(input: State) -> Output,
 
     initial_state: State,
 
@@ -478,38 +474,14 @@ pub const MAX_OUTPUT_LEN: usize = 512 / 8;
 /// algorithms in this module.
 pub const MAX_CHAINING_LEN: usize = MAX_OUTPUT_LEN;
 
-fn sha256_format_output(input: State) -> Output
-where
-    [BigEndian<u32>; 256 / 8 / core::mem::size_of::<BigEndian<u32>>()]: FromBytes,
-    [BigEndian<u64>; 512 / 8 / core::mem::size_of::<BigEndian<u64>>()]: AsBytes,
-{
-    // SAFETY: There are two cases:
-    // - The union is initialized as `as32`, in which case this is trivially
-    //   sound.
-    // - The union is initialized as `as64`. In this case, the `as64` variant is
-    //   longer than the `as32` variant, so all bytes of `as32` are initialized
-    //   as they are in the prefix of `as64`. Since `as64`'s type is `AsBytes`
-    //   (see the where bound on this function), all of its bytes are
-    //   initialized (ie, none are padding). Since `as32`'s type is `FromBytes`,
-    //   any initialized sequence of bytes constitutes a valid instance of the
-    //   type, so this is sound.
+fn sha256_format_output(input: State) -> Output {
     let input = unsafe { &input.as32 };
     Output {
         as32: input.map(BigEndian::from),
     }
 }
 
-/// # Safety
-///
-/// The caller must ensure that all bytes of `State` have been initialized.
-unsafe fn sha512_format_output(input: State) -> Output
-where
-    [BigEndian<u64>; 512 / 8 / core::mem::size_of::<BigEndian<u64>>()]: FromBytes,
-{
-    // SAFETY: Caller has promised that all bytes are initialized. Since
-    // `input.as64` is `FromBytes`, we know that this is sufficient to guarantee
-    // that the input has been initialized to a valid instance of the type of
-    // `input.as64`.
+fn sha512_format_output(input: State) -> Output {
     let input = unsafe { &input.as64 };
     Output {
         as64: input.map(BigEndian::from),

src/endian.rs

@@ -1,12 +1,10 @@
-use core::{mem, num::Wrapping};
-
-use zerocopy::{AsBytes, FromBytes, FromZeroes};
+use core::num::Wrapping;
 
 /// An `Encoding` of a type `T` can be converted to/from its byte
 /// representation without any byte swapping or other computation.
 ///
 /// The `Self: Copy` constraint addresses `clippy::declare_interior_mutable_const`.
-pub trait Encoding<T>: From<T> + Into<T> + FromBytes + AsBytes
+pub trait Encoding<T>: From<T> + Into<T>
 where
     Self: Copy,
 {
@@ -27,7 +25,7 @@ pub trait FromByteArray<T> {
 
 macro_rules! define_endian {
     ($endian:ident) => {
-        #[derive(Clone, Copy, FromZeroes, FromBytes, AsBytes)]
+        #[derive(Clone, Copy)]
         #[repr(transparent)]
         pub struct $endian<T>(T);
 
@@ -50,7 +48,7 @@ macro_rules! impl_from_byte_array {
         {
             #[inline]
             fn from_byte_array(a: &[u8; $elems * core::mem::size_of::<$base>()]) -> Self {
-                zerocopy::transmute!(*a)
+                unsafe { core::mem::transmute_copy(a) }
             }
         }
     };
@@ -60,24 +58,11 @@ macro_rules! impl_array_encoding {
     ($endian:ident, $base:ident, $elems:expr) => {
         impl ArrayEncoding<[u8; $elems * core::mem::size_of::<$base>()]>
             for [$endian<$base>; $elems]
-        where
-            Self: AsBytes,
         {
             #[inline]
             fn as_byte_array(&self) -> &[u8; $elems * core::mem::size_of::<$base>()] {
-                const _: () = assert!(
-                    mem::size_of::<[$endian<$base>; $elems]>()
-                        == $elems * core::mem::size_of::<$base>()
-                );
                 let as_bytes_ptr =
                     self.as_ptr() as *const [u8; $elems * core::mem::size_of::<$base>()];
-                // SAFETY:
-                // - `Self: AsBytes`, so it's sound to observe the bytes of
-                //   `self` and to have a reference to those bytes alive at the
-                //   same time as `&self`.
-                // - As confirmed by the preceding assertion, the sizes of
-                //   `Self` and the return type are equal.
-                // - `[u8; N]` has no alignment requirement.
                 unsafe { &*as_bytes_ptr }
             }
         }

src/rsa/public_exponent.rs

@@ -12,14 +12,8 @@ impl PublicExponent {
 
     // TODO: Use `NonZeroU64::new(...).unwrap()` when `feature(const_panic)` is
     // stable.
-    pub(super) const _3: Self = Self(match NonZeroU64::new(3) {
-        Some(nz) => nz,
-        None => unreachable!(),
-    });
-    pub(super) const _65537: Self = Self(match NonZeroU64::new(65537) {
-        Some(nz) => nz,
-        None => unreachable!(),
-    });
+    pub(super) const _3: Self = Self(unsafe { NonZeroU64::new_unchecked(3) });
+    pub(super) const _65537: Self = Self(unsafe { NonZeroU64::new_unchecked(65537) });
 
     // This limit was chosen to bound the performance of the simple
     // exponentiation-by-squaring implementation in `elem_exp_vartime`. In
@@ -35,10 +29,7 @@ impl PublicExponent {
     //
     // TODO: Use `NonZeroU64::new(...).unwrap()` when `feature(const_panic)` is
     // stable.
-    const MAX: Self = Self(match NonZeroU64::new((1u64 << 33) - 1) {
-        Some(nz) => nz,
-        None => unreachable!(),
-    });
+    const MAX: Self = Self(unsafe { NonZeroU64::new_unchecked((1u64 << 33) - 1) });
 
     pub(super) fn from_be_bytes(
         input: untrusted::Input,