From a0465ea279a3f9a8f17bf1931d5ae0d720b0ffca Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 1 Feb 2022 15:43:46 +1100 Subject: [PATCH 1/4] Remove feature global-context-less-secure Instead of providing a mechanism for users to opt out of randomization we can just feature gate the call site i.e., opportunistically randomize the global context on creation if `rand-std` feature is enabled. --- Cargo.toml | 3 +-- src/context.rs | 23 +++++++++++++++-------- src/key.rs | 6 +++--- src/lib.rs | 10 ++++------ 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bf9a231..586c8fb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,8 +26,7 @@ alloc = [] rand-std = ["rand/std"] recovery = ["secp256k1-sys/recovery"] lowmemory = ["secp256k1-sys/lowmemory"] -global-context = ["std", "rand-std", "global-context-less-secure"] -global-context-less-secure = [] +global-context = ["std"] [dependencies] secp256k1-sys = { version = "0.4.2", default-features = false, path = "./secp256k1-sys" } diff --git a/src/context.rs b/src/context.rs index f14f2cb..89fd580 100644 --- a/src/context.rs +++ b/src/context.rs @@ -9,11 +9,11 @@ use Secp256k1; #[cfg_attr(docsrs, doc(cfg(any(feature = "std", feature = "alloc"))))] pub use self::alloc_only::*; -#[cfg(all(feature = "global-context-less-secure", feature = "std"))] -#[cfg_attr(docsrs, doc(cfg(any(feature = "global-context", feature = "global-context-less-secure"))))] +#[cfg(all(feature = "global-context", feature = "std"))] +#[cfg_attr(docsrs, doc(cfg(all(feature = "global-context", feature = "std"))))] /// Module implementing a singleton pattern for a global `Secp256k1` context pub mod global { - #[cfg(feature = "global-context")] + #[cfg(feature = "rand-std")] use rand; use std::ops::Deref; @@ -26,22 +26,29 @@ pub mod global { __private: (), } - /// A global, static context to avoid repeatedly creating contexts where one can't be passed + /// A global static context to avoid repeatedly creating contexts. /// - /// If the global-context feature is enabled (and not just the global-context-less-secure), - /// this will have been randomized. + /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. + /// + /// ``` + /// # #[cfg(all(feature = "global-context", feature = "rand-std"))] { + /// use secp256k1::{PublicKey, SECP256K1}; + /// use secp256k1::rand::thread_rng; + /// let _ = SECP256K1.generate_keypair(&mut thread_rng()); + /// # } + /// ``` pub static SECP256K1: &GlobalContext = &GlobalContext { __private: () }; impl Deref for GlobalContext { type Target = Secp256k1; - #[allow(unused_mut)] // Unused when "global-context" is not enabled. + #[allow(unused_mut)] // Unused when `rand-std` is not enabled. fn deref(&self) -> &Self::Target { static ONCE: Once = Once::new(); static mut CONTEXT: Option> = None; ONCE.call_once(|| unsafe { let mut ctx = Secp256k1::new(); - #[cfg(feature = "global-context")] + #[cfg(feature = "rand-std")] { ctx.randomize(&mut rand::thread_rng()); } diff --git a/src/key.rs b/src/key.rs index bfebe63..562f02e 100644 --- a/src/key.rs +++ b/src/key.rs @@ -641,7 +641,7 @@ impl Ord for PublicKey { /// feature active. This is due to security considerations, see the [`serde_keypair`] documentation /// for details. /// -/// If the `serde` and `global-context[-less-secure]` features are active `KeyPair`s can be serialized and +/// If the `serde` and `global-context` features are active `KeyPair`s can be serialized and /// deserialized by annotating them with `#[serde(with = "secp256k1::serde_keypair")]` /// inside structs or enums for which [`Serialize`] and [`Deserialize`] are being derived. /// @@ -1320,7 +1320,7 @@ impl<'de> ::serde::Deserialize<'de> for XOnlyPublicKey { /// /// [`SecretKey`]: crate::SecretKey /// [global context]: crate::SECP256K1 -#[cfg(all(feature = "global-context-less-secure", feature = "serde"))] +#[cfg(all(feature = "global-context", feature = "serde"))] pub mod serde_keypair { use serde::{Deserialize, Deserializer, Serialize, Serializer}; use key::KeyPair; @@ -1924,7 +1924,7 @@ mod test { } #[test] - #[cfg(all(feature = "global-context-less-secure", feature = "serde"))] + #[cfg(all(feature = "global-context", feature = "serde"))] fn test_serde_keypair() { use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_test::{Configure, Token, assert_tokens}; diff --git a/src/lib.rs b/src/lib.rs index 7c73f4c..f5a474d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,9 +125,7 @@ //! * `rand-std` - use `rand` library with its `std` feature enabled. (Implies `rand`.) //! * `recovery` - enable functions that can compute the public key from signature. //! * `lowmemory` - optimize the library for low-memory environments. -//! * `global-context` - enable use of global secp256k1 context. (Implies `std`, `rand-std` and -//! `global-context-less-secure`.) -//! * `global-context-less-secure` - enables global context without extra sidechannel protection. +//! * `global-context` - enable use of global secp256k1 context (implies `std`). //! * `serde` - implements serialization and deserialization for types in this crate using `serde`. //! **Important**: `serde` encoding is **not** the same as consensus encoding! //! * `bitcoin_hashes` - enables interaction with the `bitcoin-hashes` crate (e.g. conversions). @@ -195,8 +193,8 @@ use core::marker::PhantomData; use core::{mem, fmt, str}; use ffi::{CPtr, types::AlignedType}; -#[cfg(feature = "global-context-less-secure")] -#[cfg_attr(docsrs, doc(cfg(any(feature = "global-context", feature = "global-context-less-secure"))))] +#[cfg(feature = "global-context")] +#[cfg_attr(docsrs, doc(cfg(any(feature = "global-context", feature = "global-context"))))] pub use context::global::SECP256K1; #[cfg(feature = "bitcoin_hashes")] @@ -955,7 +953,7 @@ mod tests { } - #[cfg(feature = "global-context-less-secure")] + #[cfg(feature = "global-context")] #[test] fn test_global_context() { use super::SECP256K1; From 1693d51ce7b7acd96183ad4bc585489087ef3f6d Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 1 Feb 2022 15:51:08 +1100 Subject: [PATCH 2/4] Randomize context on creation Randomize context on creation if `rand-std` feature is enabled. --- src/context.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/context.rs b/src/context.rs index 89fd580..b5edc12 100644 --- a/src/context.rs +++ b/src/context.rs @@ -115,6 +115,9 @@ mod alloc_only { #[cfg(not(feature = "std"))] use alloc::alloc; + #[cfg(feature = "rand-std")] + use rand; + impl private::Sealed for SignOnly {} impl private::Sealed for All {} impl private::Sealed for VerifyOnly {} @@ -174,7 +177,10 @@ mod alloc_only { } impl Secp256k1 { - /// Lets you create a context in a generic manner(sign/verify/all) + /// Lets you create a context in a generic manner (sign/verify/all). + /// + /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. + #[allow(unused_mut)] // Unused when `rand-std` is not enabled. pub fn gen_new() -> Secp256k1 { #[cfg(target_arch = "wasm32")] ffi::types::sanity_checks_for_wasm(); @@ -182,30 +188,43 @@ mod alloc_only { let size = unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) }; let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap(); let ptr = unsafe {alloc::alloc(layout)}; - Secp256k1 { + let mut ctx = Secp256k1 { ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) }, phantom: PhantomData, size, + }; + + #[cfg(feature = "rand-std")] + { + ctx.randomize(&mut rand::thread_rng()); } + + ctx } } impl Secp256k1 { - /// Creates a new Secp256k1 context with all capabilities + /// Creates a new Secp256k1 context with all capabilities. + /// + /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. pub fn new() -> Secp256k1 { Secp256k1::gen_new() } } impl Secp256k1 { - /// Creates a new Secp256k1 context that can only be used for signing + /// Creates a new Secp256k1 context that can only be used for signing. + /// + /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. pub fn signing_only() -> Secp256k1 { Secp256k1::gen_new() } } impl Secp256k1 { - /// Creates a new Secp256k1 context that can only be used for verification + /// Creates a new Secp256k1 context that can only be used for verification. + /// + /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. pub fn verification_only() -> Secp256k1 { Secp256k1::gen_new() } From cf1496b64e09fce34e98fa28ea6382da3cd8f373 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 1 Feb 2022 16:29:33 +1100 Subject: [PATCH 3/4] Add documentation about rand-std feature We recently implemented opportunistic randomization of the context object if the the `rand-std` feature is enabled. Both for the global context and also for signing context constructors. Add documentation about `rand-std` feature in relation to the context object. --- src/lib.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f5a474d..c007c98 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,9 +20,13 @@ //! and its derivatives. //! //! To minimize dependencies, some functions are feature-gated. To generate -//! random keys or to re-randomize a context object, compile with the "rand" -//! feature. To de/serialize objects with serde, compile with "serde". -//! **Important**: `serde` encoding is **not** the same as consensus encoding! +//! random keys or to re-randomize a context object, compile with the +//! `rand-std` feature. If you are willing to use the `rand-std` feature, we +//! have enabled an additional defense-in-depth sidechannel protection for +//! our context objects, which re-blinds certain operations on secret key +//! data. To de/serialize objects with serde, compile with "serde". +//! **Important**: `serde` encoding is **not** the same as consensus +//! encoding! //! //! Where possible, the bindings use the Rust type system to ensure that //! API usage errors are impossible. For example, the library uses context From 8339ca57061d6ae664a50263a9564acc363aabae Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Wed, 2 Feb 2022 09:06:36 +1100 Subject: [PATCH 4/4] Add documentation guiding users towards randomization Now that we opportunistically randomize the context on creation if `rand-std` is enabled it would be nice to encourage users who do not wish to use `rand-std` to randomize the context. We already have an API to do this but it requires a separate call to do so. Instead of adding a bunch of additional constructors elect to add documentation to the current constructors guiding users towards randomization. --- src/context.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/context.rs b/src/context.rs index b5edc12..310da6d 100644 --- a/src/context.rs +++ b/src/context.rs @@ -180,6 +180,19 @@ mod alloc_only { /// Lets you create a context in a generic manner (sign/verify/all). /// /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. + /// If `rand-std` feature is not enabled please consider randomizing the context as follows: + /// ``` + /// # #[cfg(all(feature = "rand-std", any(feature = "alloc", feature = "std")))] { + /// # use secp256k1::Secp256k1; + /// # use secp256k1::rand::{thread_rng, RngCore}; + /// let mut ctx = Secp256k1::new(); + /// # let mut rng = thread_rng(); + /// # let mut seed = [0u8; 32]; + /// # rng.fill_bytes(&mut seed); + /// // let seed = <32 bytes of random data> + /// ctx.seeded_randomize(&seed); + /// # } + /// ``` #[allow(unused_mut)] // Unused when `rand-std` is not enabled. pub fn gen_new() -> Secp256k1 { #[cfg(target_arch = "wasm32")] @@ -207,6 +220,8 @@ mod alloc_only { /// Creates a new Secp256k1 context with all capabilities. /// /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. + /// If `rand-std` feature is not enabled please consider randomizing the context (see docs + /// for `Secp256k1::gen_new()`). pub fn new() -> Secp256k1 { Secp256k1::gen_new() } @@ -216,6 +231,8 @@ mod alloc_only { /// Creates a new Secp256k1 context that can only be used for signing. /// /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. + /// If `rand-std` feature is not enabled please consider randomizing the context (see docs + /// for `Secp256k1::gen_new()`). pub fn signing_only() -> Secp256k1 { Secp256k1::gen_new() } @@ -225,6 +242,8 @@ mod alloc_only { /// Creates a new Secp256k1 context that can only be used for verification. /// /// If `rand-std` feature is enabled, context will have been randomized using `thread_rng`. + /// If `rand-std` feature is not enabled please consider randomizing the context (see docs + /// for `Secp256k1::gen_new()`). pub fn verification_only() -> Secp256k1 { Secp256k1::gen_new() }