#37; changed to not use unsafe code for RTSecret; tests passed

jymchng · Feb 23, 2024 · 46ca8f0 · 46ca8f0
1 parent 29212ed
commit 46ca8f0
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 35 deletions.
diff --git a/src/runtime/secret.rs b/src/runtime/secret.rs
@@ -79,16 +79,14 @@ impl<
     where
         for<'brand> ClosureType: FnOnce(RTExposedSecret<'brand, &'brand T>) -> ReturnType,
     {
-        // SAFETY: All tuple fields of `RTSecret` are private, there are no setter to them.
-        // `RTSecret` is also not `Sync` so it is not possible to have multithreading race condition.
-        let ec_mut = unsafe { &mut *self.1.as_ptr() };
+        let ec_mut = self.1.get();
         let mec = MEC::cast_unsigned_to_self_type::<MEC>(__private::SealedToken {});
-        if *ec_mut >= mec {
+        if ec_mut >= mec {
             return Err(error::ExposeSecretError::ExposeMoreThanMaximallyAllow(
-                error::ExposeMoreThanMaximallyAllowError { mec, ec: *ec_mut },
+                error::ExposeMoreThanMaximallyAllowError { mec, ec: ec_mut },
             ));
         };
-        *ec_mut += MEC::ONE;
+        self.1.set(ec_mut + MEC::ONE);
         Ok(scope(RTExposedSecret(&self.0, PhantomData)))
     }
 }

diff --git a/src/traits.rs b/src/traits.rs
@@ -162,6 +162,7 @@ mod debug_secret {
 
 pub trait ChooseMinimallyRepresentableUInt: __private::SealedTrait {
     type Output: AddAssign
+        + Add<Self::Output, Output = Self::Output>
         + PartialOrd
         + Debug
         + Display

diff --git a/tests/extern_bin_rt.rs b/tests/extern_bin_rt.rs
@@ -95,32 +95,41 @@ fn test_min_uint_outcomes() {
     let _: <U365 as ChooseMinimallyRepresentableUInt>::Output = 365 * 99;
 }
 
-// #[test]
-// fn test_expose_secret_runtime_cannot_return_secret() {
-
-//     #[cfg(feature = "zeroize")]
-//     use zeroize::Zeroize;
-
-//     struct A {
-//         inner: i32,
-//     }
-
-//     #[cfg(feature = "zeroize")]
-//     impl Zeroize for A {
-//         fn zeroize(&mut self) {
-//             self.inner.zeroize()
-//         }
-//     }
-
-//     let secret_one = RTSecret::<A, 2>::new(A {
-//         inner: 69,
-//     });
-
-//     let _ = secret_one.expose_secret(|exposed_secret| {
-//         exposed_secret
-//     });
-
-//     let _ = secret_one.expose_secret(|exposed_secret| {
-//         *exposed_secret
-//     });
-// }
+#[test]
+fn test_can_cross_unwind_boundaries_if_copy() {
+    use core::panic::AssertUnwindSafe;
+
+    extern crate std;
+    use sosecrets_rs::{
+        prelude::typenum::U2,
+        runtime::{secret::RTSecret, traits::RTExposeSecret},
+    };
+    use std::panic::catch_unwind;
+
+    #[cfg(feature = "zeroize")]
+    use zeroize::Zeroize;
+
+    #[derive(Copy, Clone)]
+    struct A {
+        inner: i32,
+    }
+
+    #[cfg(feature = "zeroize")]
+    impl Zeroize for A {
+        fn zeroize(&mut self) {
+            self.inner.zeroize()
+        }
+    }
+
+    let mut opt_a: Option<A> = Option::<A>::None;
+
+    let secret_one = RTSecret::<A, U2>::new(A { inner: 69 });
+
+    let _ = catch_unwind(AssertUnwindSafe(|| {
+        secret_one.expose_secret(|exposed_secret| {
+            opt_a.replace(*exposed_secret);
+            panic!();
+        });
+    }));
+    assert_eq!(opt_a.unwrap().inner, 69);
+}
diff --git a/tests/trybuild_tests_rt.rs b/tests/trybuild_tests_rt.rs
@@ -2,6 +2,8 @@
 fn test_compile_fails() {
     let t = trybuild::TestCases::new();
     t.compile_fail("trybuild_tests/runtime/cannot_impl_minimally_representable_uints.rs");
+    t.compile_fail("trybuild_tests/runtime/cannot_cross_unwind_if_not_copy.rs");
+    t.compile_fail("trybuild_tests/runtime/cannot_return_exposed_secret.rs");
     // t.compile_fail("trybuild_tests/test_compile_fail_two.rs");
     // t.compile_fail("trybuild_tests/test_compile_fail_three.rs");
 

diff --git a/trybuild_tests/runtime/cannot_cross_unwind_if_not_copy.rs b/trybuild_tests/runtime/cannot_cross_unwind_if_not_copy.rs
@@ -0,0 +1,36 @@
+fn main() {
+    use core::panic::AssertUnwindSafe;
+
+    extern crate std;
+    use sosecrets_rs::{
+        prelude::typenum::U2,
+        runtime::{secret::RTSecret, traits::RTExposeSecret},
+    };
+    use std::panic::catch_unwind;
+
+    #[cfg(feature = "zeroize")]
+    use zeroize::Zeroize;
+
+    struct A {
+        inner: i32,
+    }
+
+    #[cfg(feature = "zeroize")]
+    impl Zeroize for A {
+        fn zeroize(&mut self) {
+            self.inner.zeroize()
+        }
+    }
+
+    let mut opt_a: Option<A> = Option::<A>::None;
+
+    let secret_one = RTSecret::<A, U2>::new(A { inner: 69 });
+
+    let _ = catch_unwind(AssertUnwindSafe(|| {
+        secret_one.expose_secret(|exposed_secret| {
+            opt_a.replace(*exposed_secret);
+            panic!();
+        });
+    }));
+    assert_eq!(opt_a.unwrap().inner, 69);
+}
diff --git a/trybuild_tests/runtime/cannot_cross_unwind_if_not_copy.stderr b/trybuild_tests/runtime/cannot_cross_unwind_if_not_copy.stderr
@@ -0,0 +1,5 @@
+error[E0507]: cannot move out of dereference of `RTExposedSecret<'_, &A>`
+  --> trybuild_tests/runtime/cannot_cross_unwind_if_not_copy.rs:31:27
+   |
+31 |             opt_a.replace(*exposed_secret);
+   |                           ^^^^^^^^^^^^^^^ move occurs because value has type `A`, which does not implement the `Copy` trait
diff --git a/trybuild_tests/runtime/cannot_return_exposed_secret.rs b/trybuild_tests/runtime/cannot_return_exposed_secret.rs
@@ -0,0 +1,26 @@
+fn main() {
+    use sosecrets_rs::{
+        prelude::typenum::U2,
+        runtime::{secret::RTSecret, traits::RTExposeSecret},
+    };
+
+    #[cfg(feature = "zeroize")]
+    use zeroize::Zeroize;
+
+    struct A {
+        inner: i32,
+    }
+
+    #[cfg(feature = "zeroize")]
+    impl Zeroize for A {
+        fn zeroize(&mut self) {
+            self.inner.zeroize()
+        }
+    }
+
+    let secret_one = RTSecret::<A, U2>::new(A { inner: 69 });
+
+    let _ = secret_one.expose_secret(|exposed_secret| exposed_secret);
+
+    let _ = secret_one.expose_secret(|exposed_secret| *exposed_secret);
+}
diff --git a/trybuild_tests/runtime/cannot_return_exposed_secret.stderr b/trybuild_tests/runtime/cannot_return_exposed_secret.stderr
@@ -0,0 +1,19 @@
+error: lifetime may not live long enough
+  --> trybuild_tests/runtime/cannot_return_exposed_secret.rs:23:9
+   |
+22 |     let _ = secret_one.expose_secret(|exposed_secret| {
+   |                                       --------------- return type of closure is RTExposedSecret<'2, &A>
+   |                                       |
+   |                                       has type `RTExposedSecret<'1, &'1 A>`
+23 |         exposed_secret
+   |         ^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
+   |
+   = note: requirement occurs because of the type `RTExposedSecret<'_, &A>`, which makes the generic argument `'_` invariant
+   = note: the struct `RTExposedSecret<'brand, T>` is invariant over the parameter `'brand`
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
+
+error[E0507]: cannot move out of dereference of `RTExposedSecret<'_, &A>`
+  --> trybuild_tests/runtime/cannot_return_exposed_secret.rs:27:9
+   |
+27 |         *exposed_secret
+   |         ^^^^^^^^^^^^^^^ move occurs because value has type `A`, which does not implement the `Copy` trait