From 6f638632fc8ee09773b64a274f03da9e6df7c2a6 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 28 Feb 2023 19:35:51 +0000 Subject: [PATCH 01/16] Add github worker for CI and remove Travis. Fixes #105 --- .github/workflows/test.yml | 54 ++++++++++++++++++++++++++++++++++++++ .travis.yml | 41 ----------------------------- 2 files changed, 54 insertions(+), 41 deletions(-) create mode 100644 .github/workflows/test.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..05bb690 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,54 @@ +on: [push, pull_request] + +name: Test + +jobs: + test: + name: cargo test + runs-on: ubuntu-latest + strategy: + matrix: + rust: + - stable + - beta + - nightly + - 1.41.0 + steps: + - name: checkout + uses: actions/checkout@v2 + - name: toolchain + uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: ${{ matrix.rust }} + target: thumbv7em-none-eabi + override: true + - name: test + uses: actions-rs/cargo@v1 + with: + command: test + - name: nightly + uses: actions-rs/cargo@v1 + with: + command: test + args: --features nightly + - name: no-default-features + uses: actions-rs/cargo@v1 + with: + command: test + args: --no-default-features + - name: std + uses: actions-rs/cargo@v1 + with: + command: test + args: --no-default-features --features std + - name: std i128 + uses: actions-rs/cargo@v1 + with: + command: test + args: --no-default-features --features "std i128" + - name: no std build + uses: actions-rs/cargo@v1 + with: + command: build + args: --no-default-features --target thumbv7em-none-eabi diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index ef3b7f8..0000000 --- a/.travis.yml +++ /dev/null @@ -1,41 +0,0 @@ -language: rust -cache: cargo - -rust: - - stable - - beta - - nightly - -matrix: - include: - # Test nightly feature - - rust: nightly - env: NAME="nightly feature" - script: cargo test --features nightly - # Test MSRV - - rust: 1.41.0 - env: NAME="MSRV test" - script: cargo test --no-default-features --features std - # Test if crate can be truly built without std - - env: TARGET=thumbv7em-none-eabi - script: cargo build --no-default-features --target $TARGET - install: - - rustup target add $TARGET - -script: - - cargo test && cargo test --no-default-features && - cargo test --no-default-features --features std && - cargo test --no-default-features --features "std i128" && - cargo test --no-default-features --features "std core_hint_black_box" && - cargo test --no-default-features --features "std const-generics" && - cargo test --no-default-features --features "std i128 core_hint_black_box" && - cargo test --no-default-features --features "std i128 core_hint_black_box const-generics" - -notifications: - slack: - rooms: - - dalek-cryptography:Xxv9WotKYWdSoKlgKNqXiHoD#dalek-bots - -cache: - directories: - - /home/travis/.cargo From 98525cb5d1017ecf2f815b3497fbd004658c53e4 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 28 Feb 2023 19:49:48 +0000 Subject: [PATCH 02/16] Quiet the unused_attribute warnings. --- src/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 795eade..41e186d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -270,6 +270,9 @@ impl From for Choice { /// assert_eq!(x.ct_eq(&y).unwrap_u8(), 0); /// assert_eq!(x.ct_eq(&x).unwrap_u8(), 1); /// ``` +// +// #[inline] is specified on these function prototypes to signify that they +#[allow(unused_attributes)] // should be in the actual implementation pub trait ConstantTimeEq { /// Determine if two items are equal. /// @@ -397,6 +400,9 @@ impl ConstantTimeEq for cmp::Ordering { /// /// This trait also provides generic implementations of conditional /// assignment and conditional swaps. +// +// #[inline] is specified on these function prototypes to signify that they +#[allow(unused_attributes)] // should be in the actual implementation pub trait ConditionallySelectable: Copy { /// Select `a` or `b` according to `choice`. /// @@ -604,6 +610,9 @@ where /// A generic implementation of `ConditionallyNegatable` is provided /// for types `T` which are `ConditionallySelectable` and have `Neg` /// implemented on `&T`. +// +// #[inline] is specified on these function prototypes to signify that they +#[allow(unused_attributes)] // should be in the actual implementation pub trait ConditionallyNegatable { /// Negate `self` if `choice == Choice(1)`; otherwise, leave it /// unchanged. From 415cb23f039e19e215abac3fa6d717b48b4bb25a Mon Sep 17 00:00:00 2001 From: Amber Sprenkels Date: Thu, 9 Mar 2023 20:39:00 +0800 Subject: [PATCH 03/16] Revert "Change new black_box impl to also be #[inline(never)]." This reverts commit 6410953c200285f704556e3e3748e3e95a363532. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 795eade..ff58b6e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -243,7 +243,7 @@ fn black_box(input: u8) -> u8 { } #[cfg(feature = "core_hint_black_box")] -#[inline(never)] +#[inline] fn black_box(input: u8) -> u8 { debug_assert!((input == 0u8) | (input == 1u8)); core::hint::black_box(input) From d49e4638d121f165fe83140139adecdc7f0d559d Mon Sep 17 00:00:00 2001 From: Amber Sprenkels Date: Thu, 9 Mar 2023 20:43:23 +0800 Subject: [PATCH 04/16] Revert "Add core_hint_black_box feature that uses core::hint::black_box" This reverts commit 0dfc57262ad545bada6d255faa4a7655b1cb8245. --- .travis.yml | 4 +--- Cargo.toml | 1 + README.md | 12 ++---------- src/lib.rs | 48 +++--------------------------------------------- 4 files changed, 7 insertions(+), 58 deletions(-) diff --git a/.travis.yml b/.travis.yml index ef3b7f8..c5d887c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,10 +26,8 @@ script: - cargo test && cargo test --no-default-features && cargo test --no-default-features --features std && cargo test --no-default-features --features "std i128" && - cargo test --no-default-features --features "std core_hint_black_box" && cargo test --no-default-features --features "std const-generics" && - cargo test --no-default-features --features "std i128 core_hint_black_box" && - cargo test --no-default-features --features "std i128 core_hint_black_box const-generics" + cargo test --no-default-features --features "std i128 const-generics" notifications: slack: diff --git a/Cargo.toml b/Cargo.toml index 64532ed..3bbc305 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ rand = { version = "0.8" } [features] const-generics = [] +# DEPRECATED: As of 2.5.1, this feature does nothing. core_hint_black_box = [] default = ["std", "i128"] std = [] diff --git a/README.md b/README.md index de77575..928bcb7 100644 --- a/README.md +++ b/README.md @@ -26,10 +26,6 @@ prevent this refinement, the crate tries to hide the value of a `Choice`'s inner `u8` by passing it through a volatile read. For more information, see the _About_ section below. -Rust versions from 1.66 or higher support a new best-effort optimization -barrier ([`core::hint::black_box`]). To use the new optimization barrier, -enable the `core_hint_black_box` feature. - Rust versions from 1.51 or higher have const generics support. You may enable `const-generics` feautre to have `subtle` traits implemented for arrays `[T; N]`. @@ -47,7 +43,7 @@ Documentation is available [here][docs]. ## Minimum Supported Rust Version -Rust **1.41** or higher. +Rust **1.66** or higher. Minimum supported Rust version can be changed in the future, but it will be done with a minor version bump. @@ -59,11 +55,8 @@ Old versions of the optimization barrier in `impl From for Choice` were based on Tim Maclean's [work on `rust-timing-shield`][rust-timing-shield], which attempts to provide a more comprehensive approach for preventing software side-channels in Rust code. - From version `2.2`, it was based on Diane Hosfelt and Amber Sprenkels' work on -"Secret Types in Rust". Version `2.5` adds the `core_hint_black_box` feature, -which uses the original method through the [`core::hint::black_box`] function -from the Rust standard library. +"Secret Types in Rust". `subtle` is authored by isis agora lovecruft and Henry de Valence. @@ -78,5 +71,4 @@ effort is fundamentally limited. **USE AT YOUR OWN RISK** [docs]: https://docs.rs/subtle -[`core::hint::black_box`]: https://doc.rust-lang.org/core/hint/fn.black_box.html [rust-timing-shield]: https://www.chosenplaintext.ca/open-source/rust-timing-shield/security diff --git a/src/lib.rs b/src/lib.rs index ff58b6e..207fe4e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,10 +41,6 @@ //! inner `u8` by passing it through a volatile read. For more information, see //! the _About_ section below. //! -//! Rust versions from 1.66 or higher support a new best-effort optimization -//! barrier ([`core::hint::black_box`]). To use the new optimization barrier, -//! enable the `core_hint_black_box` feature. -//! //! Rust versions from 1.51 or higher have const generics support. You may enable //! `const-generics` feautre to have `subtle` traits implemented for arrays `[T; N]`. //! @@ -74,11 +70,8 @@ //! based on Tim Maclean's [work on `rust-timing-shield`][rust-timing-shield], //! which attempts to provide a more comprehensive approach for preventing //! software side-channels in Rust code. -//! //! From version `2.2`, it was based on Diane Hosfelt and Amber Sprenkels' work on -//! "Secret Types in Rust". Version `2.5` adds the `core_hint_black_box` feature, -//! which uses the original method through the [`core::hint::black_box`] function -//! from the Rust standard library. +//! "Secret Types in Rust". //! //! `subtle` is authored by isis agora lovecruft and Henry de Valence. //! @@ -93,7 +86,6 @@ //! **USE AT YOUR OWN RISK** //! //! [docs]: https://docs.rs/subtle -//! [`core::hint::black_box`]: https://doc.rust-lang.org/core/hint/fn.black_box.html //! [rust-timing-shield]: https://www.chosenplaintext.ca/open-source/rust-timing-shield/security #[cfg(feature = "std")] @@ -214,47 +206,13 @@ impl Not for Choice { } } -/// This function is a best-effort attempt to prevent the compiler from knowing -/// anything about the value of the returned `u8`, other than its type. -/// -/// Because we want to support stable Rust, we don't have access to inline -/// assembly or test::black_box, so we use the fact that volatile values will -/// never be elided to register values. -/// -/// Note: Rust's notion of "volatile" is subject to change over time. While this -/// code may break in a non-destructive way in the future, “constant-time” code -/// is a continually moving target, and this is better than doing nothing. -#[cfg(not(feature = "core_hint_black_box"))] -#[inline(never)] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - - unsafe { - // Optimization barrier - // - // Unsafe is ok, because: - // - &input is not NULL; - // - size of input is not zero; - // - u8 is neither Sync, nor Send; - // - u8 is Copy, so input is always live; - // - u8 type is always properly aligned. - core::ptr::read_volatile(&input as *const u8) - } -} - -#[cfg(feature = "core_hint_black_box")] -#[inline] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - core::hint::black_box(input) -} - impl From for Choice { #[inline] fn from(input: u8) -> Choice { + debug_assert!((input == 0u8) | (input == 1u8)); // Our goal is to prevent the compiler from inferring that the value held inside the // resulting `Choice` struct is really an `i1` instead of an `i8`. - Choice(black_box(input)) + Choice(core::hint::black_box(input)) } } From c1d3561e943c459ab9d70fda5d2d4527fa0be75c Mon Sep 17 00:00:00 2001 From: Amber Sprenkels Date: Thu, 9 Mar 2023 20:44:05 +0800 Subject: [PATCH 05/16] Revert "Add assertion checking that a u8 bool is valid" This reverts commit df58872775b48bda1d0f4fdb782762cd7c70c557. --- src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 207fe4e..1aa0acf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,7 +209,6 @@ impl Not for Choice { impl From for Choice { #[inline] fn from(input: u8) -> Choice { - debug_assert!((input == 0u8) | (input == 1u8)); // Our goal is to prevent the compiler from inferring that the value held inside the // resulting `Choice` struct is really an `i1` instead of an `i8`. Choice(core::hint::black_box(input)) From 52549346d2b9132485b6ebe8b0004f0c67683bdc Mon Sep 17 00:00:00 2001 From: Amber Sprenkels Date: Thu, 9 Mar 2023 20:44:13 +0800 Subject: [PATCH 06/16] Revert "Replace black_box with std::hint::black_box" This reverts commit 4978f4235b37cae2eb9f125cc5095772b3adc561. --- README.md | 2 +- src/lib.rs | 29 ++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 928bcb7..0753be3 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Documentation is available [here][docs]. ## Minimum Supported Rust Version -Rust **1.66** or higher. +Rust **1.41** or higher. Minimum supported Rust version can be changed in the future, but it will be done with a minor version bump. diff --git a/src/lib.rs b/src/lib.rs index 1aa0acf..63cf19f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -206,12 +206,39 @@ impl Not for Choice { } } +/// This function is a best-effort attempt to prevent the compiler from knowing +/// anything about the value of the returned `u8`, other than its type. +/// +/// Because we want to support stable Rust, we don't have access to inline +/// assembly or test::black_box, so we use the fact that volatile values will +/// never be elided to register values. +/// +/// Note: Rust's notion of "volatile" is subject to change over time. While this +/// code may break in a non-destructive way in the future, “constant-time” code +/// is a continually moving target, and this is better than doing nothing. +#[inline(never)] +fn black_box(input: u8) -> u8 { + debug_assert!((input == 0u8) | (input == 1u8)); + + unsafe { + // Optimization barrier + // + // Unsafe is ok, because: + // - &input is not NULL; + // - size of input is not zero; + // - u8 is neither Sync, nor Send; + // - u8 is Copy, so input is always live; + // - u8 type is always properly aligned. + core::ptr::read_volatile(&input as *const u8) + } +} + impl From for Choice { #[inline] fn from(input: u8) -> Choice { // Our goal is to prevent the compiler from inferring that the value held inside the // resulting `Choice` struct is really an `i1` instead of an `i8`. - Choice(core::hint::black_box(input)) + Choice(black_box(input)) } } From 705c2072cf31408d491d47fcf04c3f63dd88f50c Mon Sep 17 00:00:00 2001 From: isis lovecruft Date: Mon, 3 Apr 2023 16:54:20 -0700 Subject: [PATCH 07/16] Add github action testing for const-generics feature. --- .github/workflows/test.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 05bb690..c5fe9e4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,11 +42,21 @@ jobs: with: command: test args: --no-default-features --features std + - name: std const-generics + uses: actions-rs/cargo@v1 + with: + command: test + args: --no-default-features --features "std const-generics" - name: std i128 uses: actions-rs/cargo@v1 with: command: test args: --no-default-features --features "std i128" + - name: std i128 const-generics + uses: actions-rs/cargo@v1 + with: + command: test + args: --no-default-features --features "std i128 const-generics" - name: no std build uses: actions-rs/cargo@v1 with: From 11c89b14aad0c931eb83e64b1f2b5f0fea596fbf Mon Sep 17 00:00:00 2001 From: isis lovecruft Date: Mon, 14 Aug 2023 16:32:11 -0700 Subject: [PATCH 08/16] Fix typo in unit test assertion. Closes #108. --- tests/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mod.rs b/tests/mod.rs index f6b3982..51f9fd8 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -58,7 +58,7 @@ macro_rules! generate_integer_conditional_select_tests { let x: $t = 0; // all 0 bits let y: $t = !0; // all 1 bits - assert_eq!(<$t>::conditional_select(&x, &y, 0.into()), 0); + assert_eq!(<$t>::conditional_select(&x, &y, 0.into()), x); assert_eq!(<$t>::conditional_select(&x, &y, 1.into()), y); let mut z = x; From 855cdd4db8ee32bc10a4a2da32345127d712b2d8 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Thu, 1 Feb 2024 17:48:01 -0700 Subject: [PATCH 09/16] Add `CtOption::into_option` A small wrapper for `impl CtOption for Option` which is friendlier for type inference purposes. Using `Option::from(ct_option)` often doesn't work because the compiler is unable to infer `T` for `Option`, so you have to manually annotate `Option::::from`. In practice this often winds up looking like: Option::::from(T::constructor(x))` which feels quite redundant. Using an inherent method instead fixes type inference so the above can be: T::constructor(x).into_option() --- src/lib.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 795eade..6b0042d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -801,6 +801,22 @@ impl CtOption { Self::conditional_select(&self, &f, is_none) } + + /// Convert the `CtOption` wrapper into an `Option`, depending on whether + /// the underlying `is_some` `Choice` was a `0` or a `1` once unwrapped. + /// + /// # Note + /// + /// This function exists to avoid ending up with ugly, verbose and/or bad handled + /// conversions from the `CtOption` wraps to an `Option` or `Result`. + /// This implementation doesn't intend to be constant-time nor try to protect the + /// leakage of the `T` since the `Option` will do it anyways. + /// + /// It's equivalent to the corresponding `From` impl, however this version is is + /// friendlier for type inference. + pub fn into_option(self) -> Option { + self.into() + } } impl ConditionallySelectable for CtOption { From 4ffd51d4e56a698bfa16158ad2340c79cfbf348b Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Thu, 8 Feb 2024 09:28:15 -0700 Subject: [PATCH 10/16] Update src/lib.rs Co-authored-by: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 6b0042d..1aba339 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -812,7 +812,7 @@ impl CtOption { /// This implementation doesn't intend to be constant-time nor try to protect the /// leakage of the `T` since the `Option` will do it anyways. /// - /// It's equivalent to the corresponding `From` impl, however this version is is + /// It's equivalent to the corresponding `From` impl, however this version is /// friendlier for type inference. pub fn into_option(self) -> Option { self.into() From 54441e7c1f020d687c58d0a3b1385e617d2dfa49 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 12 Feb 2024 18:27:40 -0600 Subject: [PATCH 11/16] Silence warnings --- src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index b7584c0..e8b6524 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -267,6 +267,7 @@ pub trait ConstantTimeEq { /// * `Choice(1u8)` if `self == other`; /// * `Choice(0u8)` if `self != other`. #[inline] + #[allow(unused_attributes)] fn ct_eq(&self, other: &Self) -> Choice; /// Determine if two items are NOT equal. @@ -413,6 +414,7 @@ pub trait ConditionallySelectable: Copy { /// # } /// ``` #[inline] + #[allow(unused_attributes)] fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self; /// Conditionally assign `other` to `self`, according to `choice`. @@ -603,6 +605,7 @@ pub trait ConditionallyNegatable { /// /// This function should execute in constant time. #[inline] + #[allow(unused_attributes)] fn conditional_negate(&mut self, choice: Choice); } From 2e974645e889be2dc2920f0331ac323beb694cae Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 18 Jun 2024 20:06:32 -0600 Subject: [PATCH 12/16] Add `BlackBox` Adds a `VolatileCell`-like struct which introduces an optimization barrier on all accesses. The `*Cell`-like `get()` method is the only accessor for the inner value and uses a more generalized (i.e. for all `Copy` types) `black_box` to preclude optimizations. This is useful for things like bitwise mask values to ensure LLVM doesn't try to optimize around special cases like `0`, especially in the context of loops. For an example use case, see: https://rustsec.org/advisories/RUSTSEC-2024-0344.html --- src/lib.rs | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 795eade..09b5c30 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -104,6 +104,9 @@ use core::cmp; use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Neg, Not}; use core::option::Option; +#[cfg(feature = "core_hint_black_box")] +use core::hint::black_box; + /// The `Choice` struct represents a choice for use in conditional assignment. /// /// It is a wrapper around a `u8`, which should have the value either `1` (true) @@ -226,34 +229,25 @@ impl Not for Choice { /// is a continually moving target, and this is better than doing nothing. #[cfg(not(feature = "core_hint_black_box"))] #[inline(never)] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - +fn black_box(input: T) -> T { unsafe { // Optimization barrier // - // Unsafe is ok, because: - // - &input is not NULL; - // - size of input is not zero; - // - u8 is neither Sync, nor Send; - // - u8 is Copy, so input is always live; - // - u8 type is always properly aligned. - core::ptr::read_volatile(&input as *const u8) + // SAFETY: + // - &input is not NULL because we own input; + // - input is Copy and always live; + // - input is always properly aligned. + core::ptr::read_volatile(&input) } } -#[cfg(feature = "core_hint_black_box")] -#[inline(never)] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - core::hint::black_box(input) -} - impl From for Choice { #[inline] fn from(input: u8) -> Choice { + debug_assert!((input == 0u8) | (input == 1u8)); + // Our goal is to prevent the compiler from inferring that the value held inside the - // resulting `Choice` struct is really an `i1` instead of an `i8`. + // resulting `Choice` struct is really a `bool` instead of a `u8`. Choice(black_box(input)) } } @@ -974,3 +968,14 @@ impl ConstantTimeLess for cmp::Ordering { (a as u8).ct_lt(&(b as u8)) } } + +/// Wrapper type which implements an optimization barrier for all accesses. +#[derive(Clone, Copy, Debug)] +pub struct BlackBox(T); + +impl BlackBox { + /// Read the inner value, applying an optimization barrier on access. + pub fn get(self) -> T { + black_box(self.0) + } +} From c0774e94e50c955607b0bdce9a40258939a81c84 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 18 Jun 2024 20:06:32 -0600 Subject: [PATCH 13/16] Add `BlackBox` Adds a `VolatileCell`-like struct which introduces an optimization barrier on all accesses. The `*Cell`-like `get()` method is the only accessor for the inner value and uses a more generalized (i.e. for all `Copy` types) `black_box` to preclude optimizations. This is useful for things like bitwise mask values to ensure LLVM doesn't try to optimize around special cases like `0`, especially in the context of loops. For an example use case, see: https://rustsec.org/advisories/RUSTSEC-2024-0344.html --- src/lib.rs | 48 ++++++++++++++++++++++++++++++------------------ tests/mod.rs | 7 +++++++ 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 795eade..9bc7d2c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -104,6 +104,9 @@ use core::cmp; use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Neg, Not}; use core::option::Option; +#[cfg(feature = "core_hint_black_box")] +use core::hint::black_box; + /// The `Choice` struct represents a choice for use in conditional assignment. /// /// It is a wrapper around a `u8`, which should have the value either `1` (true) @@ -226,34 +229,25 @@ impl Not for Choice { /// is a continually moving target, and this is better than doing nothing. #[cfg(not(feature = "core_hint_black_box"))] #[inline(never)] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - +fn black_box(input: T) -> T { unsafe { // Optimization barrier // - // Unsafe is ok, because: - // - &input is not NULL; - // - size of input is not zero; - // - u8 is neither Sync, nor Send; - // - u8 is Copy, so input is always live; - // - u8 type is always properly aligned. - core::ptr::read_volatile(&input as *const u8) + // SAFETY: + // - &input is not NULL because we own input; + // - input is Copy and always live; + // - input is always properly aligned. + core::ptr::read_volatile(&input) } } -#[cfg(feature = "core_hint_black_box")] -#[inline(never)] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - core::hint::black_box(input) -} - impl From for Choice { #[inline] fn from(input: u8) -> Choice { + debug_assert!((input == 0u8) | (input == 1u8)); + // Our goal is to prevent the compiler from inferring that the value held inside the - // resulting `Choice` struct is really an `i1` instead of an `i8`. + // resulting `Choice` struct is really a `bool` instead of a `u8`. Choice(black_box(input)) } } @@ -974,3 +968,21 @@ impl ConstantTimeLess for cmp::Ordering { (a as u8).ct_lt(&(b as u8)) } } + +/// Wrapper type which implements an optimization barrier for all accesses. +#[derive(Clone, Copy, Debug)] +pub struct BlackBox(T); + +impl BlackBox { + /// Constructs a new instance of `BlackBox` which will wrap the specified value. + /// + /// All access to the inner value will be mediated by a `black_box` optimization barrier. + pub const fn new(value: T) -> Self { + Self(value) + } + + /// Read the inner value, applying an optimization barrier on access. + pub fn get(self) -> T { + black_box(self.0) + } +} diff --git a/tests/mod.rs b/tests/mod.rs index f6b3982..7460311 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -423,3 +423,10 @@ fn less_than_ordering() { assert_eq!(cmp::Ordering::Greater.ct_lt(&cmp::Ordering::Less).unwrap_u8(), 0); assert_eq!(cmp::Ordering::Less.ct_lt(&cmp::Ordering::Greater).unwrap_u8(), 1); } + +#[test] +fn black_box_round_trip() { + let n = 42u64; + let black_box = BlackBox::new(n); + assert_eq!(n, black_box.get()); +} From bf87dee3ca4c813d5de9998b0729e462f981e58c Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Tue, 18 Jun 2024 20:06:32 -0600 Subject: [PATCH 14/16] Add `BlackBox` Adds a `VolatileCell`-like struct which introduces an optimization barrier on all accesses. The `*Cell`-like `get()` method is the only accessor for the inner value and uses a more generalized (i.e. for all `Copy` types) `black_box` to preclude optimizations. This is useful for things like bitwise mask values to ensure LLVM doesn't try to optimize around special cases like `0`, especially in the context of loops. For an example use case, see: https://rustsec.org/advisories/RUSTSEC-2024-0344.html --- src/lib.rs | 39 +++++++++++++++++++++++++++++---------- tests/mod.rs | 7 +++++++ 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e8b6524..f0bbee4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -96,6 +96,9 @@ use core::cmp; use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Neg, Not}; use core::option::Option; +#[cfg(feature = "core_hint_black_box")] +use core::hint::black_box; + /// The `Choice` struct represents a choice for use in conditional assignment. /// /// It is a wrapper around a `u8`, which should have the value either `1` (true) @@ -217,25 +220,23 @@ impl Not for Choice { /// code may break in a non-destructive way in the future, “constant-time” code /// is a continually moving target, and this is better than doing nothing. #[inline(never)] -fn black_box(input: u8) -> u8 { - debug_assert!((input == 0u8) | (input == 1u8)); - +fn black_box(input: T) -> T { unsafe { // Optimization barrier // - // Unsafe is ok, because: - // - &input is not NULL; - // - size of input is not zero; - // - u8 is neither Sync, nor Send; - // - u8 is Copy, so input is always live; - // - u8 type is always properly aligned. - core::ptr::read_volatile(&input as *const u8) + // SAFETY: + // - &input is not NULL because we own input; + // - input is Copy and always live; + // - input is always properly aligned. + core::ptr::read_volatile(&input) } } impl From for Choice { #[inline] fn from(input: u8) -> Choice { + debug_assert!((input == 0u8) | (input == 1u8)); + // Our goal is to prevent the compiler from inferring that the value held inside the // resulting `Choice` struct is really an `i1` instead of an `i8`. Choice(black_box(input)) @@ -986,3 +987,21 @@ impl ConstantTimeLess for cmp::Ordering { (a as u8).ct_lt(&(b as u8)) } } + +/// Wrapper type which implements an optimization barrier for all accesses. +#[derive(Clone, Copy, Debug)] +pub struct BlackBox(T); + +impl BlackBox { + /// Constructs a new instance of `BlackBox` which will wrap the specified value. + /// + /// All access to the inner value will be mediated by a `black_box` optimization barrier. + pub fn new(value: T) -> Self { + Self(value) + } + + /// Read the inner value, applying an optimization barrier on access. + pub fn get(self) -> T { + black_box(self.0) + } +} diff --git a/tests/mod.rs b/tests/mod.rs index 51f9fd8..888b9d0 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -423,3 +423,10 @@ fn less_than_ordering() { assert_eq!(cmp::Ordering::Greater.ct_lt(&cmp::Ordering::Less).unwrap_u8(), 0); assert_eq!(cmp::Ordering::Less.ct_lt(&cmp::Ordering::Greater).unwrap_u8(), 1); } + +#[test] +fn black_box_round_trip() { + let n = 42u64; + let black_box = BlackBox::new(n); + assert_eq!(n, black_box.get()); +} From 1da204b36716402a9237f28c7c4cdf23c2c2d98d Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 19 Jun 2024 22:11:14 +0000 Subject: [PATCH 15/16] Update copyright year. --- LICENSE | 1 + 1 file changed, 1 insertion(+) diff --git a/LICENSE b/LICENSE index 927c02c..9e2751f 100644 --- a/LICENSE +++ b/LICENSE @@ -1,4 +1,5 @@ Copyright (c) 2016-2017 Isis Agora Lovecruft, Henry de Valence. All rights reserved. +Copyright (c) 2016-2024 Isis Agora Lovecruft. All rights reserved. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are From 1da93bf97e374b9dc2d99f9d299c3c035cfd4ff6 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 19 Jun 2024 22:11:36 +0000 Subject: [PATCH 16/16] Bump version to 2.6. --- Cargo.toml | 2 +- README.md | 2 +- src/lib.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3bbc305..ae7361c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ name = "subtle" # - update html_root_url # - update README if necessary by semver # - if any updates were made to the README, also update the module documentation in src/lib.rs -version = "2.5.0" +version = "2.6.0" edition = "2018" authors = ["Isis Lovecruft ", "Henry de Valence "] diff --git a/README.md b/README.md index 0753be3..a9eb261 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ instead of `bool` which are intended to execute in constant-time. The `Choice` type is a wrapper around a `u8` that holds a `0` or `1`. ```toml -subtle = "2.5" +subtle = "2.6" ``` This crate represents a “best-effort” attempt, since side-channels diff --git a/src/lib.rs b/src/lib.rs index 09ec1bf..b6e42c4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,7 @@ #![no_std] #![deny(missing_docs)] #![doc(html_logo_url = "https://doc.dalek.rs/assets/dalek-logo-clear.png")] -#![doc(html_root_url = "https://docs.rs/subtle/2.5.0")] +#![doc(html_root_url = "https://docs.rs/subtle/2.6.0")] //! # subtle [![](https://img.shields.io/crates/v/subtle.svg)](https://crates.io/crates/subtle) [![](https://img.shields.io/badge/dynamic/json.svg?label=docs&uri=https%3A%2F%2Fcrates.io%2Fapi%2Fv1%2Fcrates%2Fsubtle%2Fversions&query=%24.versions%5B0%5D.num&colorB=4F74A6)](https://doc.dalek.rs/subtle) [![](https://travis-ci.org/dalek-cryptography/subtle.svg?branch=master)](https://travis-ci.org/dalek-cryptography/subtle) //! @@ -22,7 +22,7 @@ //! type is a wrapper around a `u8` that holds a `0` or `1`. //! //! ```toml -//! subtle = "2.5" +//! subtle = "2.6" //! ``` //! //! This crate represents a “best-effort” attempt, since side-channels