From 6f24308066601798d81ec962f2789ed1209de2f2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 27 Jun 2025 16:50:58 +0000 Subject: [PATCH 1/8] context: rename src/context.rs to src/context/mod.rs Will make it easier to introduce submodules. --- src/{context.rs => context/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{context.rs => context/mod.rs} (100%) diff --git a/src/context.rs b/src/context/mod.rs similarity index 100% rename from src/context.rs rename to src/context/mod.rs From 6fdffac48b43955c5cdf952ff97792e9423ff402 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 20 Jun 2025 20:23:52 +0000 Subject: [PATCH 2/8] context: introduce global rerandomizable context (std only) This introduces the new global context API when std is enabled, using thread locals to allow rerandomizing the context after sensitive operations. As you can see, even the simple case involves some unsafe code and is a bit tricky to implement. --- src/context/internal_std.rs | 75 +++++++++++++++++++++++++++++++++++++ src/context/mod.rs | 7 ++++ src/lib.rs | 4 ++ 3 files changed, 86 insertions(+) create mode 100644 src/context/internal_std.rs diff --git a/src/context/internal_std.rs b/src/context/internal_std.rs new file mode 100644 index 000000000..068bd58dd --- /dev/null +++ b/src/context/internal_std.rs @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: CC0-1.0 + +use std::cell::RefCell; +use std::marker::PhantomData; +use std::mem::ManuallyDrop; +use std::ptr::NonNull; + +use secp256k1_sys as ffi; + +use crate::{All, Context, Secp256k1}; + +thread_local! { + static SECP256K1: RefCell> = RefCell::new(Secp256k1::new()); +} + +/// Borrows the global context and do some operation on it. +/// +/// If provided, after the operation is complete, [`rerandomize_global_context`] +/// is called on the context. If you have some random data available, +pub fn with_global_context) -> T>( + f: F, + rerandomize_seed: Option<&[u8; 32]>, +) -> T { + with_raw_global_context( + |ctx| { + let secp = ManuallyDrop::new(Secp256k1 { ctx, phantom: PhantomData }); + f(&*secp) + }, + rerandomize_seed, + ) +} + +/// Borrows the global context as a raw pointer and do some operation on it. +/// +/// If provided, after the operation is complete, [`rerandomize_global_context`] +/// is called on the context. If you have some random data available, +pub fn with_raw_global_context) -> T>( + f: F, + rerandomize_seed: Option<&[u8; 32]>, +) -> T { + SECP256K1.with(|secp| { + let borrow = secp.borrow(); + let ret = f(borrow.ctx); + drop(borrow); + + if let Some(seed) = rerandomize_seed { + rerandomize_global_context(seed); + } + ret + }) +} + +/// Rerandomize the global context, using the given data as a seed. +/// +/// The provided data will be mixed with the entropy from previous calls in a timing +/// analysis resistant way. It is safe to directly pass secret data to this function. +pub fn rerandomize_global_context(seed: &[u8; 32]) { + SECP256K1.with(|secp| { + let mut borrow = secp.borrow_mut(); + + // If we have access to the thread rng then use it as well. + #[cfg(feature = "rand")] + { + let mut new_seed: [u8; 32] = rand::random(); + for (new, byte) in new_seed.iter_mut().zip(seed.iter()) { + *new ^= *byte; + } + borrow.seeded_randomize(&new_seed); + } + #[cfg(not(feature = "rand"))] + { + borrow.seeded_randomize(seed); + } + }); +} diff --git a/src/context/mod.rs b/src/context/mod.rs index 58348a090..716659087 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -10,6 +10,13 @@ use crate::ffi::types::{c_uint, c_void, AlignedType}; use crate::ffi::{self, CPtr}; use crate::{Error, Secp256k1}; +#[cfg(feature = "std")] +#[cfg_attr(feature = "std", path = "internal_std.rs")] +mod internal; + +#[cfg(feature = "std")] +pub use internal::{rerandomize_global_context, with_global_context, with_raw_global_context}; + #[cfg(all(feature = "global-context", feature = "std"))] /// Module implementing a singleton pattern for a global `Secp256k1` context. pub mod global { diff --git a/src/lib.rs b/src/lib.rs index ffc8038c1..7d8d89f72 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -184,6 +184,10 @@ pub use secp256k1_sys as ffi; #[cfg(feature = "serde")] pub use serde; +#[cfg(feature = "std")] +pub use crate::context::{ + rerandomize_global_context, with_global_context, with_raw_global_context, +}; #[cfg(feature = "alloc")] pub use crate::context::{All, SignOnly, VerifyOnly}; pub use crate::context::{ From cd0fdd168830d86d73f98bd770ce1dea09a12198 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 27 Jun 2025 17:20:27 +0000 Subject: [PATCH 3/8] context: introduce spinlock that gives up after a few iterations Introduces a spinlocking mutex that only offers access to its internals via a "try_unlock" method which spins a small finite number of times before unlocking. We use a spinlock because, in the minimal dependency set we support, there are no synchronization primitives except atomics, so that's the only form of mutex we can create. However, there are a number of problems with spinlocks -- see this article (from Kix in #346) for some of them: https://matklad.github.io/2020/01/02/spinlocks-considered-harmful.html To avoid these problems, we give up after a few spins. The way we will use this in the context object is: 1. When initializing the global context, if we can't get the lock, we just initialize a new stack-local context and use that. (A parallel thread must be initializing the context, which is wasteful but harmless.) 2. Once we unlock the context, we copy it onto the stack and re-lock it in order to minimize the time holding the lock. (The exception is during initialization where we hold the lock for the whole initialization, in the hopes that other threads will block on us instead of doing their own initialization.) If we rerandomize, we do this on the stack-local copy and then only re-lock to copy it back. 3. If we fail to get the lock to copy the rerandomized context back, we just don't copy it. The result is that we wasted some time rerandomizing without any benefit, which is not the end of the world. The spinlock was implemented with help from ChatGPT o3 and the unit tests with help from Claude 4 (though in both cases I did significant refactoring and review by hand). --- src/context/mod.rs | 3 + src/context/spinlock.rs | 226 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 src/context/spinlock.rs diff --git a/src/context/mod.rs b/src/context/mod.rs index 716659087..ef3029b75 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -14,6 +14,9 @@ use crate::{Error, Secp256k1}; #[cfg_attr(feature = "std", path = "internal_std.rs")] mod internal; +#[cfg(test)] // will have a better feature-gate in next commit +mod spinlock; + #[cfg(feature = "std")] pub use internal::{rerandomize_global_context, with_global_context, with_raw_global_context}; diff --git a/src/context/spinlock.rs b/src/context/spinlock.rs new file mode 100644 index 000000000..4bfccc478 --- /dev/null +++ b/src/context/spinlock.rs @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: CC0-1.0 + +use core::cell::UnsafeCell; +use core::hint::spin_loop; +use core::ops::{Deref, DerefMut}; +use core::sync::atomic::{AtomicBool, Ordering}; + +const MAX_SPINLOCK_ATTEMPTS: usize = 128; + +// Best-Effort Spinlock +// +// To obtain exclusive access, call [`Self::try_lock`], which will spinlock +// for some small number of iterations before giving up. By trying again in +// a loop, you can emulate a "true" spinlock that will only yield once it +// has access. However, this would be very dangerous, especially in a nostd +// environment, because if we are pre-empted by an interrupt handler while +// the lock is held, and that interrupt handler attempts to take the lock, +// then we deadlock. +// +// Instead, the strategy we take within this module is to simply create a +// new stack-local context object if we are unable to obtain a lock on the +// global one. This is slow and loses the defense-in-depth "rerandomization" +// anti-sidechannel measure, but it is better than deadlocking.. +pub struct SpinLock { + flag: AtomicBool, + // Invariant: if this is non-None, then the store is valid and can be + // used with `ffi::secp256k1_context_preallocated_create`. + data: UnsafeCell, +} + +// Required by rustc if we have a static of this type. +// Safety: `data` is accessed only while the `flag` is held. +unsafe impl Sync for SpinLock {} + +#[cfg(test)] +impl SpinLock { + pub const fn new() -> Self { Self { flag: AtomicBool::new(false), data: UnsafeCell::new(100) } } +} + +impl SpinLock { + /// Blocks until the lock is acquired, then returns an RAII guard. + pub fn try_lock(&self) -> Option> { + for _ in 0..MAX_SPINLOCK_ATTEMPTS { + if self.flag.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed).is_ok() + { + return Some(SpinLockGuard { lock: self }); + } + spin_loop(); + } + None + } + + /// Unlocks the data held by the spinlock. + /// + /// # Safety + /// + /// Once this method is called, no access to the data within the spinlock + /// should be possible. + /// + /// (This method is private so we can enforce this safety condition here.) + #[inline(always)] + unsafe fn unlock(&self) { self.flag.store(false, Ordering::Release); } +} + +/// Drops the lock when it goes out of scope. +pub struct SpinLockGuard<'a, T> { + lock: &'a SpinLock, +} + +impl Deref for SpinLockGuard<'_, T> { + type Target = T; + fn deref(&self) -> &Self::Target { + // SAFETY: we hold the lock. + unsafe { &*self.lock.data.get() } + } +} + +impl DerefMut for SpinLockGuard<'_, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: mutable access is unique while the guard lives. + unsafe { &mut *self.lock.data.get() } + } +} + +impl Drop for SpinLockGuard<'_, T> { + fn drop(&mut self) { + // SAFETY: access to the data within the spinlock is only possible through + // the `SpinLockGuard` which is being destructed. + unsafe { + self.lock.unlock(); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn basic_lock_unlock() { + let spinlock = SpinLock::::new(); + + let guard = spinlock.try_lock().expect("Should be able to acquire lock"); + assert_eq!(*guard, 100); + drop(guard); + + let guard2 = spinlock.try_lock().expect("Should be able to reacquire lock"); + assert_eq!(*guard2, 100); + } + + #[test] + fn modify_data() { + let spinlock = SpinLock::::new(); + + { + let mut guard = spinlock.try_lock().expect("Should be able to acquire lock"); + *guard = 42; + } + + let guard = spinlock.try_lock().expect("Should be able to reacquire lock"); + assert_eq!(*guard, 42); + } + + #[test] + fn contention_single_thread() { + let spinlock = SpinLock::::new(); + + let _guard1 = spinlock.try_lock().expect("Should be able to acquire lock"); + let result = spinlock.try_lock(); + assert!(result.is_none(), "Should not be able to acquire lock twice"); + } + + #[test] + fn guard_deref() { + let spinlock = SpinLock::::new(); + let guard = spinlock.try_lock().expect("Should be able to acquire lock"); + + // Test Deref + assert_eq!(*guard, 100); + + // Test que nous pouvons utiliser les méthodes de u64 + let value: u64 = *guard; + assert_eq!(value, 100); + } + + #[test] + fn guard_deref_mut() { + let spinlock = SpinLock::::new(); + let mut guard = spinlock.try_lock().expect("Should be able to acquire lock"); + + // Test DerefMut + *guard += 50; + assert_eq!(*guard, 150); + + // Modifier via une méthode qui prend &mut self + *guard = guard.wrapping_add(10); + assert_eq!(*guard, 160); + } +} + +#[cfg(all(test, feature = "std"))] +mod std_tests { + use std::sync::Arc; + use std::thread; + use std::time::Duration; + + use super::*; + + #[test] + fn multiple_threads_no_contention() { + let spinlock = Arc::new(SpinLock::::new()); + let mut handles = vec![]; + + for i in 1..=3 { + let spinlock_clone = Arc::clone(&spinlock); + let handle = thread::spawn(move || { + let mut guard = spinlock_clone.try_lock().expect("Should acquire lock"); + let old_value = *guard; + *guard = old_value + i; + // (drop guard) + }); + handles.push(handle); + + // Sleep between spawning threads. In practice this does not seem to be + // necessary, at least on Andrew's system. + thread::sleep(Duration::from_millis(10)); + } + + for handle in handles { + handle.join().expect("Thread should complete successfully"); + } + + let guard = spinlock.try_lock().expect("Should be able to acquire lock"); + assert_eq!(*guard, 106); + } + + #[test] + fn multiple_threads_contention() { + let spinlock = Arc::new(SpinLock::::new()); + let mut handles = vec![]; + + for i in 1..=3 { + let spinlock_clone = Arc::clone(&spinlock); + let handle = thread::spawn(move || { + loop { + if let Some(mut guard) = spinlock_clone.try_lock() { + let old_value = *guard; + *guard = old_value + i; + // Sleep while holding lock. + thread::sleep(Duration::from_millis(10)); + break; + } + //panic!("uncomment me to check that sometimes contention happens"); + } + }); + handles.push(handle); + } + + for handle in handles { + handle.join().expect("Thread should complete successfully"); + } + + let guard = spinlock.try_lock().expect("Should be able to acquire lock"); + assert_eq!(*guard, 106); + } +} From 68ec3e559db321866ea3e82b12185c4474f2bdd9 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 20 Jun 2025 20:42:21 +0000 Subject: [PATCH 4/8] context: add nostd version of global context See the previous commit description for a high-level overview of the spinlocking logic used in this commit. Next steps are: 1. Update the API to use this logic everywhere; on validation functions we don't need to rerandomize and on signing/keygen functions we should rerandomize using our secret key material. 2. Remove the existing "no context" API, along with the global-context and global-context-less-secure features. 3. Improve our entropy story on nostd by scraping system time or CPU jitter or something and hashing that into our rerandomization. We don't need to do a great job here -- if we can get even a bit or two per signature, that will completely BTFO a timing attacker. --- src/context/internal_nostd.rs | 159 ++++++++++++++++++++++++++++++++++ src/context/mod.rs | 8 +- src/context/spinlock.rs | 11 +++ src/lib.rs | 9 +- 4 files changed, 177 insertions(+), 10 deletions(-) create mode 100644 src/context/internal_nostd.rs diff --git a/src/context/internal_nostd.rs b/src/context/internal_nostd.rs new file mode 100644 index 000000000..956558493 --- /dev/null +++ b/src/context/internal_nostd.rs @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: CC0-1.0 + +use core::marker::PhantomData; +use core::mem::ManuallyDrop; +use core::ptr::NonNull; + +use crate::context::spinlock::SpinLock; +use crate::{ffi, Context, Secp256k1}; + +mod self_contained_context { + use core::mem::MaybeUninit; + use core::ptr::NonNull; + + use crate::ffi::types::{c_void, AlignedType}; + use crate::{ffi, AllPreallocated, Context as _}; + + const MAX_PREALLOC_SIZE: usize = 16; // measured at 208 bytes on Andrew's 64-bit system + + /// A secp256k1 context object which can be allocated on the stack or in static storage. + pub struct SelfContainedContext( + [MaybeUninit; MAX_PREALLOC_SIZE], + Option>, + ); + + // SAFETY: the context object owns all its own data. + unsafe impl Send for SelfContainedContext {} + + impl SelfContainedContext { + /// Creates a new uninitialized self-contained context. + pub const fn new_uninitialized() -> Self { + Self([MaybeUninit::uninit(); MAX_PREALLOC_SIZE], None) + } + + /// Accessor for the underlying raw context pointer + fn buf(&mut self) -> NonNull { + NonNull::new(self.0.as_mut_ptr() as *mut c_void).unwrap() + } + + pub fn clone_into(&mut self, other: &mut SelfContainedContext) { + // SAFETY: just FFI calls + unsafe { + let other = other.raw_ctx().as_ptr(); + assert!( + ffi::secp256k1_context_preallocated_clone_size(other) + <= core::mem::size_of::<[AlignedType; MAX_PREALLOC_SIZE]>(), + "prealloc size exceeds our guessed compile-time upper bound", + ); + ffi::secp256k1_context_preallocated_clone(other, self.buf()); + } + } + + /// Accessor for the context as a raw context pointer. + /// + /// On the first call, this will create the context. + pub fn raw_ctx(&mut self) -> NonNull { + let buf = self.buf(); + *self.1.get_or_insert_with(|| { + // SAFETY: just FFI calls + unsafe { + assert!( + ffi::secp256k1_context_preallocated_size(AllPreallocated::FLAGS) + <= core::mem::size_of::<[AlignedType; MAX_PREALLOC_SIZE]>(), + "prealloc size exceeds our guessed compile-time upper bound", + ); + ffi::secp256k1_context_preallocated_create(buf, AllPreallocated::FLAGS) + } + }) + } + } +} +// Needs to be pub(super) so that we can define a constructor for +// SpinLock in the spinlock module. (We cannot do so generically +// because we need a const constructor.) +pub(super) use self_contained_context::SelfContainedContext; + +static SECP256K1: SpinLock = SpinLock::::new(); + +/// Borrows the global context and do some operation on it. +/// +/// If `randomize_seed` is provided, it is used to rerandomize the context after the +/// operation is complete. If it is not provided, randomization is skipped. +/// +/// Only a bit or two per signing operation is needed; if you have any entropy at all, +/// you should provide it, even if you can't provide 32 random bytes. +pub fn with_global_context) -> T>( + f: F, + rerandomize_seed: Option<&[u8; 32]>, +) -> T { + with_raw_global_context( + |ctx| { + let secp = ManuallyDrop::new(Secp256k1 { ctx, phantom: PhantomData }); + f(&*secp) + }, + rerandomize_seed, + ) +} + +/// Borrows the global context as a raw pointer and do some operation on it. +/// +/// If `randomize_seed` is provided, it is used to rerandomize the context after the +/// operation is complete. If it is not provided, randomization is skipped. +/// +/// Only a bit or two per signing operation is needed; if you have any entropy at all, +/// you should provide it, even if you can't provide 32 random bytes. +pub fn with_raw_global_context) -> T>( + f: F, + rerandomize_seed: Option<&[u8; 32]>, +) -> T { + // Our function may be expensive, so before calling it, we copy the global + // context into this local buffer on the stack. Then we can release it, + // allowing other callers to use it simultaneously. + let mut ctx = SelfContainedContext::new_uninitialized(); + let mut have_global_ctx = false; + if let Some(mut guard) = SECP256K1.try_lock() { + let global_ctx = &mut *guard; + ctx.clone_into(global_ctx); + have_global_ctx = true; + // (the lock is now dropped) + } + + // Obtain a raw pointer to the context, creating one if it has not been already, + // and call the function. + let ctx_ptr = ctx.raw_ctx(); + let ret = f(ctx_ptr); + + // ...then rerandomize the local copy, and try to replace the global one + // with this. Note that even if we got the lock above, we may fail to get + // it now; in that case, we don't rerandomize and leave the contexct in + // the state that we found it in. + // + // We do this, rather than holding the lock continuously through the call + // to `f`, to minimize the likelihood of contention. If we fail to randomize, + // that really isn't a big deal since this is a "defense in depth" measure + // whose value is likely to obtain even if it only succeeds a small fraction + // of the time. + // + // Contention, meanwhile, will lead to users using a stack-local copy of + // the context rather than the global one, which aside from being inefficient, + // means that the context they use won't be rerandomized at all. So there + // isn't even any benefit. + if have_global_ctx { + if let Some(seed) = rerandomize_seed { + // SAFETY: just a FFI call + unsafe { + assert_eq!(ffi::secp256k1_context_randomize(ctx_ptr, seed.as_ptr()), 1); + } + if let Some(ref mut guard) = SECP256K1.try_lock() { + guard.clone_into(&mut ctx); + } + } + } + ret +} + +/// Rerandomize the global context, using the given data as a seed. +/// +/// The provided data will be mixed with the entropy from previous calls in a timing +/// analysis resistant way. It is safe to directly pass secret data to this function. +pub fn rerandomize_global_context(seed: &[u8; 32]) { with_raw_global_context(|_| {}, Some(seed)) } diff --git a/src/context/mod.rs b/src/context/mod.rs index ef3029b75..543ee2d9f 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -10,14 +10,13 @@ use crate::ffi::types::{c_uint, c_void, AlignedType}; use crate::ffi::{self, CPtr}; use crate::{Error, Secp256k1}; -#[cfg(feature = "std")] #[cfg_attr(feature = "std", path = "internal_std.rs")] +#[cfg_attr(not(feature = "std"), path = "internal_nostd.rs")] mod internal; -#[cfg(test)] // will have a better feature-gate in next commit +#[cfg(not(feature = "std"))] mod spinlock; -#[cfg(feature = "std")] pub use internal::{rerandomize_global_context, with_global_context, with_raw_global_context}; #[cfg(all(feature = "global-context", feature = "std"))] @@ -379,7 +378,8 @@ impl<'buf> Secp256k1> { /// * The version of `libsecp256k1` used to create `raw_ctx` must be **exactly the one linked /// into this library**. /// * The lifetime of the `raw_ctx` pointer must outlive `'buf`. - /// * `raw_ctx` must point to writable memory (cannot be `ffi::secp256k1_context_no_precomp`). + /// * `raw_ctx` must point to writable memory (cannot be `ffi::secp256k1_context_no_precomp`), + /// **or** the user must never attempt to rerandomize the context. pub unsafe fn from_raw_all( raw_ctx: NonNull, ) -> ManuallyDrop>> { diff --git a/src/context/spinlock.rs b/src/context/spinlock.rs index 4bfccc478..47d1ab642 100644 --- a/src/context/spinlock.rs +++ b/src/context/spinlock.rs @@ -5,6 +5,8 @@ use core::hint::spin_loop; use core::ops::{Deref, DerefMut}; use core::sync::atomic::{AtomicBool, Ordering}; +use crate::context::internal::SelfContainedContext; + const MAX_SPINLOCK_ATTEMPTS: usize = 128; // Best-Effort Spinlock @@ -32,6 +34,15 @@ pub struct SpinLock { // Safety: `data` is accessed only while the `flag` is held. unsafe impl Sync for SpinLock {} +impl SpinLock { + pub const fn new() -> Self { + Self { + flag: AtomicBool::new(false), + data: UnsafeCell::new(SelfContainedContext::new_uninitialized()), + } + } +} + #[cfg(test)] impl SpinLock { pub const fn new() -> Self { Self { flag: AtomicBool::new(false), data: UnsafeCell::new(100) } } diff --git a/src/lib.rs b/src/lib.rs index 7d8d89f72..61da291e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -184,16 +184,13 @@ pub use secp256k1_sys as ffi; #[cfg(feature = "serde")] pub use serde; -#[cfg(feature = "std")] pub use crate::context::{ - rerandomize_global_context, with_global_context, with_raw_global_context, + rerandomize_global_context, with_global_context, with_raw_global_context, AllPreallocated, + Context, PreallocatedContext, SignOnlyPreallocated, Signing, Verification, + VerifyOnlyPreallocated, }; #[cfg(feature = "alloc")] pub use crate::context::{All, SignOnly, VerifyOnly}; -pub use crate::context::{ - AllPreallocated, Context, PreallocatedContext, SignOnlyPreallocated, Signing, Verification, - VerifyOnlyPreallocated, -}; use crate::ffi::types::AlignedType; use crate::ffi::CPtr; pub use crate::key::{InvalidParityValue, Keypair, Parity, PublicKey, SecretKey, XOnlyPublicKey}; From 19946c6b8ea94c5c13c7bb7a8881590f47df4f5c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 21 Jun 2025 15:32:45 +0000 Subject: [PATCH 5/8] key: remove std/alloc/global-context gates from serde::deserialize and FromStr Since we have a no-feature-gate global context now, we can remove the feature gates from these things. No API change (other than an expansion of the API for users without features enabled). --- src/key.rs | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/key.rs b/src/key.rs index 3aef6302b..6d93524dc 100644 --- a/src/key.rs +++ b/src/key.rs @@ -20,7 +20,8 @@ use crate::ThirtyTwoByteHash; #[cfg(feature = "global-context")] use crate::SECP256K1; use crate::{ - constants, ecdsa, from_hex, schnorr, Message, Scalar, Secp256k1, Signing, Verification, + constants, ecdsa, from_hex, schnorr, AllPreallocated, Message, Scalar, Secp256k1, Signing, + Verification, }; /// Secret key - a 256-bit key used to create ECDSA and Taproot signatures. @@ -1076,20 +1077,14 @@ impl<'a> From<&'a Keypair> for PublicKey { fn from(pair: &'a Keypair) -> Self { PublicKey::from_keypair(pair) } } -#[cfg(any(feature = "global-context", feature = "alloc"))] impl str::FromStr for Keypair { type Err = Error; - #[allow(unused_variables, unreachable_code)] // When built with no default features. fn from_str(s: &str) -> Result { - #[cfg(feature = "global-context")] - let ctx = SECP256K1; - - #[cfg(all(not(feature = "global-context"), feature = "alloc"))] - let ctx = Secp256k1::signing_only(); - - #[allow(clippy::needless_borrow)] - Keypair::from_seckey_str(&ctx, s) + crate::with_global_context( + |secp: &Secp256k1| Self::from_seckey_str(secp, s), + None, + ) } } @@ -1113,8 +1108,6 @@ impl serde::Serialize for Keypair { } #[cfg(feature = "serde")] -#[allow(unused_variables)] // For `data` under some feature combinations (the unconditional panic below). -#[cfg(all(feature = "serde", any(feature = "global-context", feature = "alloc")))] impl<'de> serde::Deserialize<'de> for Keypair { fn deserialize>(d: D) -> Result { if d.is_human_readable() { @@ -1123,14 +1116,10 @@ impl<'de> serde::Deserialize<'de> for Keypair { )) } else { let visitor = super::serde_util::Tuple32Visitor::new("raw 32 bytes Keypair", |data| { - #[cfg(feature = "global-context")] - let ctx = SECP256K1; - - #[cfg(all(not(feature = "global-context"), feature = "alloc"))] - let ctx = Secp256k1::signing_only(); - - #[allow(clippy::needless_borrow)] - Keypair::from_seckey_byte_array(&ctx, data) + crate::with_global_context( + |secp: &Secp256k1| Self::from_seckey_byte_array(secp, data), + None, + ) }); d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor) } From bd0ac6a153ec9dcf8ae220a537807ccf8047cd2b Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 21 Jun 2025 14:52:54 +0000 Subject: [PATCH 6/8] test: remove a ton of rand feature-gating Sometihng like half the tests in this crate are gated on "rand", most of which are for dumb reasons (we are generating random keys from the thread rng). By adding a non-feature=rand "random key generator" we can enable these tests even without the rand feature. We typically also have a gate on "std", which is needed to get the thread rng, but in some cases this is the *only* reason to have a std gate. So by eliminating the rand requirement we can make tests work in nostd. We do this by implementing a parallel LCG which is obviously not cryptographic but is fine for testing. In the LLM-generated tests in musig2.rs we have some rand feature gates for literally no reason at all :/. My bad. In addition to dramatically increasing nostd test coverage, the new "generate random keys" function also gives us an opportunity to use the new global context API including rerandomization. --- src/ecdh.rs | 15 +++---- src/ecdsa/recovery.rs | 74 ++++++++++++++--------------------- src/key.rs | 91 +++++++++++++++++++++++++------------------ src/lib.rs | 41 ++++++++++++++++++- src/musig.rs | 43 +++++++------------- src/scalar.rs | 16 ++++++++ src/schnorr.rs | 36 +++++++++-------- 7 files changed, 180 insertions(+), 136 deletions(-) diff --git a/src/ecdh.rs b/src/ecdh.rs index 4fd90b788..27d03c404 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -194,11 +194,9 @@ mod tests { use crate::Secp256k1; #[test] - #[cfg(all(feature = "rand", feature = "std"))] fn ecdh() { - let s = Secp256k1::signing_only(); - let (sk1, pk1) = s.generate_keypair(&mut rand::rng()); - let (sk2, pk2) = s.generate_keypair(&mut rand::rng()); + let (sk1, pk1) = crate::test_random_keypair(); + let (sk2, pk2) = crate::test_random_keypair(); let sec1 = SharedSecret::new(&pk2, &sk1); let sec2 = SharedSecret::new(&pk1, &sk2); @@ -226,15 +224,14 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] - #[cfg(all(feature = "hashes", feature = "rand", feature = "std"))] + #[cfg(feature = "hashes")] fn hashes_and_sys_generate_same_secret() { use hashes::{sha256, Hash, HashEngine}; use crate::ecdh::shared_secret_point; - let s = Secp256k1::signing_only(); - let (sk1, _) = s.generate_keypair(&mut rand::rng()); - let (_, pk2) = s.generate_keypair(&mut rand::rng()); + let (sk1, _) = crate::test_random_keypair(); + let (_, pk2) = crate::test_random_keypair(); let secret_sys = SharedSecret::new(&pk2, &sk1); @@ -251,7 +248,7 @@ mod tests { } #[test] - #[cfg(all(feature = "serde", feature = "alloc"))] + #[cfg(feature = "serde")] fn serde() { use serde_test::{assert_tokens, Configure, Token}; #[rustfmt::skip] diff --git a/src/ecdsa/recovery.rs b/src/ecdsa/recovery.rs index 7c32efb42..e2d32cde0 100644 --- a/src/ecdsa/recovery.rs +++ b/src/ecdsa/recovery.rs @@ -264,17 +264,17 @@ mod tests { use crate::{Error, Message, Secp256k1, SecretKey}; #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn capabilities() { let sign = Secp256k1::signing_only(); let vrfy = Secp256k1::verification_only(); let full = Secp256k1::new(); - let msg = crate::random_32_bytes(&mut rand::rng()); + let msg = crate::test_random_32_bytes(); let msg = Message::from_digest(msg); // Try key generation - let (sk, pk) = full.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); // Try signing assert_eq!(sign.sign_ecdsa_recoverable(msg, &sk), full.sign_ecdsa_recoverable(msg, &sk)); @@ -296,11 +296,10 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] // fixed sig vectors can't work with fuzz-sigs - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] #[rustfmt::skip] fn sign() { - let mut s = Secp256k1::new(); - s.randomize(&mut rand::rng()); + let s = Secp256k1::new(); let sk = SecretKey::from_byte_array(ONE).unwrap(); let msg = Message::from_digest(ONE); @@ -321,11 +320,10 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] // fixed sig vectors can't work with fuzz-sigs - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] #[rustfmt::skip] fn sign_with_noncedata() { - let mut s = Secp256k1::new(); - s.randomize(&mut rand::rng()); + let s = Secp256k1::new(); let sk = SecretKey::from_byte_array(ONE).unwrap(); let msg = Message::from_digest(ONE); @@ -346,21 +344,17 @@ mod tests { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn sign_and_verify_fail() { - let mut s = Secp256k1::new(); - s.randomize(&mut rand::rng()); - - let msg = crate::random_32_bytes(&mut rand::rng()); - let msg = Message::from_digest(msg); + let s = Secp256k1::new(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let msg = Message::from_digest(crate::test_random_32_bytes()); + let (sk, pk) = crate::test_random_keypair(); let sigr = s.sign_ecdsa_recoverable(msg, &sk); let sig = sigr.to_standard(); - let msg = crate::random_32_bytes(&mut rand::rng()); - let msg = Message::from_digest(msg); + let msg = Message::from_digest(crate::test_random_32_bytes()); assert_eq!(s.verify_ecdsa(&sig, msg, &pk), Err(Error::IncorrectSignature)); let recovered_key = s.recover_ecdsa(msg, &sigr).unwrap(); @@ -368,15 +362,12 @@ mod tests { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn sign_with_recovery() { - let mut s = Secp256k1::new(); - s.randomize(&mut rand::rng()); - - let msg = crate::random_32_bytes(&mut rand::rng()); - let msg = Message::from_digest(msg); + let s = Secp256k1::new(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let msg = Message::from_digest(crate::test_random_32_bytes()); + let (sk, pk) = crate::test_random_keypair(); let sig = s.sign_ecdsa_recoverable(msg, &sk); @@ -384,17 +375,14 @@ mod tests { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn sign_with_recovery_and_noncedata() { - let mut s = Secp256k1::new(); - s.randomize(&mut rand::rng()); - - let msg = crate::random_32_bytes(&mut rand::rng()); - let msg = Message::from_digest(msg); + let s = Secp256k1::new(); - let noncedata = [42u8; 32]; + let msg = Message::from_digest(crate::test_random_32_bytes()); + let noncedata = crate::test_random_32_bytes(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); let sig = s.sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata); @@ -402,12 +390,11 @@ mod tests { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn bad_recovery() { - let mut s = Secp256k1::new(); - s.randomize(&mut rand::rng()); + let s = Secp256k1::new(); - let msg = Message::from_digest([0x55; 32]); + let msg = Message::from_digest(crate::test_random_32_bytes()); // Zero is not a valid sig let sig = RecoverableSignature::from_compact(&[0; 64], RecoveryId::Zero).unwrap(); @@ -468,22 +455,21 @@ mod tests { } #[cfg(bench)] -#[cfg(all(feature = "rand", feature = "std"))] // Currently only a single bench that requires "rand" + "std". +#[cfg(feature = "std")] // Currently only a single bench that requires "rand" + "std". mod benches { use test::{black_box, Bencher}; - use super::{Message, Secp256k1}; + use crate::{Message, Secp256k1, SecretKey}; #[bench] pub fn bench_recover(bh: &mut Bencher) { let s = Secp256k1::new(); - let msg = crate::random_32_bytes(&mut rand::rng()); - let msg = Message::from_digest(msg); - let (sk, _) = s.generate_keypair(&mut rand::thread_rng()); - let sig = s.sign_ecdsa_recoverable(&msg, &sk); + let msg = Message::from_digest(crate::test_random_32_bytes()); + let sk = SecretKey::test_random(); + let sig = s.sign_ecdsa_recoverable(msg, &sk); bh.iter(|| { - let res = s.recover_ecdsa(&msg, &sig).unwrap(); + let res = s.recover_ecdsa(msg, &sig).unwrap(); black_box(res); }); } diff --git a/src/key.rs b/src/key.rs index 6d93524dc..b51efd322 100644 --- a/src/key.rs +++ b/src/key.rs @@ -368,6 +368,22 @@ impl SecretKey { let kp = self.keypair(secp); XOnlyPublicKey::from_keypair(&kp) } + + /// Constructor for unit testing. + #[cfg(test)] + #[cfg(all(feature = "rand", feature = "std"))] + pub fn test_random() -> Self { Self::new(&mut rand::rng()) } + + /// Constructor for unit testing. + #[cfg(test)] + #[cfg(not(all(feature = "rand", feature = "std")))] + pub fn test_random() -> Self { + loop { + if let Ok(ret) = Self::from_byte_array(crate::test_random_32_bytes()) { + return ret; + } + } + } } #[cfg(feature = "hashes")] @@ -1046,6 +1062,16 @@ impl Keypair { /// [`zeroize`](https://docs.rs/zeroize) crate. #[inline] pub fn non_secure_erase(&mut self) { self.0.non_secure_erase(); } + + /// Constructor for unit testing. + #[cfg(test)] + pub fn test_random() -> Self { + let sk = SecretKey::test_random(); + crate::with_global_context( + |secp: &Secp256k1| Self::from_secret_key(secp, &sk), + Some(&sk.secret_bytes()), + ) + } } impl fmt::Debug for Keypair { @@ -1692,11 +1718,8 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] fn keypair_slice_round_trip() { - let s = Secp256k1::new(); - - let (sk1, pk1) = s.generate_keypair(&mut rand::rng()); + let (sk1, pk1) = crate::test_random_keypair(); assert_eq!(SecretKey::from_byte_array(sk1.secret_bytes()), Ok(sk1)); assert_eq!(PublicKey::from_slice(&pk1.serialize()[..]), Ok(pk1)); assert_eq!(PublicKey::from_slice(&pk1.serialize_uncompressed()[..]), Ok(pk1)); @@ -1972,15 +1995,15 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn tweak_add_arbitrary_data() { let s = Secp256k1::new(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); assert_eq!(PublicKey::from_secret_key(&s, &sk), pk); // Sanity check. // TODO: This would be better tested with a _lot_ of different tweaks. - let tweak = Scalar::random(); + let tweak = Scalar::test_random(); let tweaked_sk = sk.add_tweak(&tweak).unwrap(); assert_ne!(sk, tweaked_sk); // Make sure we did something. @@ -1991,11 +2014,11 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn tweak_add_zero() { let s = Secp256k1::new(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); let tweak = Scalar::ZERO; @@ -2006,40 +2029,38 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn tweak_mul_arbitrary_data() { let s = Secp256k1::new(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); assert_eq!(PublicKey::from_secret_key(&s, &sk), pk); // Sanity check. - // TODO: This would be better tested with a _lot_ of different tweaks. - let tweak = Scalar::random(); - - let tweaked_sk = sk.mul_tweak(&tweak).unwrap(); - assert_ne!(sk, tweaked_sk); // Make sure we did something. - let tweaked_pk = pk.mul_tweak(&s, &tweak).unwrap(); - assert_ne!(pk, tweaked_pk); + for _ in 0..10 { + let tweak = Scalar::test_random(); - assert_eq!(PublicKey::from_secret_key(&s, &tweaked_sk), tweaked_pk); + let tweaked_sk = sk.mul_tweak(&tweak).unwrap(); + assert_ne!(sk, tweaked_sk); // Make sure we did something. + let tweaked_pk = pk.mul_tweak(&s, &tweak).unwrap(); + assert_ne!(pk, tweaked_pk); + assert_eq!(PublicKey::from_secret_key(&s, &tweaked_sk), tweaked_pk); + } } #[test] - #[cfg(all(feature = "rand", feature = "std"))] fn tweak_mul_zero() { - let s = Secp256k1::new(); - let (sk, _) = s.generate_keypair(&mut rand::rng()); + let (sk, _) = crate::test_random_keypair(); let tweak = Scalar::ZERO; assert!(sk.mul_tweak(&tweak).is_err()) } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn test_negation() { let s = Secp256k1::new(); - let (sk, pk) = s.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); assert_eq!(PublicKey::from_secret_key(&s, &sk), pk); // Sanity check. @@ -2057,7 +2078,7 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn pubkey_hash() { use std::collections::hash_map::DefaultHasher; use std::collections::HashSet; @@ -2069,11 +2090,10 @@ mod test { s.finish() } - let s = Secp256k1::new(); let mut set = HashSet::new(); const COUNT: usize = 1024; for _ in 0..COUNT { - let (_, pk) = s.generate_keypair(&mut rand::rng()); + let (_, pk) = crate::test_random_keypair(); let hash = hash(&pk); assert!(!set.contains(&hash)); set.insert(hash); @@ -2140,12 +2160,12 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn create_pubkey_combine() { let s = Secp256k1::new(); - let (sk1, pk1) = s.generate_keypair(&mut rand::rng()); - let (sk2, pk2) = s.generate_keypair(&mut rand::rng()); + let (sk1, pk1) = crate::test_random_keypair(); + let (sk2, pk2) = crate::test_random_keypair(); let sum1 = pk1.combine(&pk2); assert!(sum1.is_ok()); @@ -2250,15 +2270,14 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn test_tweak_add_then_tweak_add_check() { let s = Secp256k1::new(); - // TODO: 10 times is arbitrary, we should test this a _lot_ of times. for _ in 0..10 { - let tweak = Scalar::random(); + let tweak = Scalar::test_random(); - let kp = Keypair::new(&s, &mut rand::rng()); + let kp = Keypair::test_random(); let (xonly, _) = XOnlyPublicKey::from_keypair(&kp); let tweaked_kp = kp.add_xonly_tweak(&s, &tweak).expect("keypair tweak add failed"); @@ -2510,10 +2529,8 @@ mod test { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] fn test_keypair_from_str() { - let ctx = crate::Secp256k1::new(); - let keypair = Keypair::new(&ctx, &mut rand::rng()); + let keypair = Keypair::test_random(); let mut buf = [0_u8; constants::SECRET_KEY_SIZE * 2]; // Holds hex digits. let s = to_hex(&keypair.secret_key().secret_bytes(), &mut buf).unwrap(); let parsed_key = Keypair::from_str(s).unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 61da291e0..b79758eb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -455,6 +455,24 @@ pub fn generate_keypair(rng: &mut R) -> (key::SecretKey, SECP256K1.generate_keypair(rng) } +/// Constructor for unit testing. (Calls `generate_keypair` if all +/// the relevant features are on to get coverage of that functoin.) +#[cfg(test)] +#[cfg(all(feature = "global-context", feature = "rand", feature = "std"))] +fn test_random_keypair() -> (key::SecretKey, key::PublicKey) { generate_keypair(&mut rand::rng()) } + +/// Constructor for unit testing. +#[cfg(test)] +#[cfg(not(all(feature = "global-context", feature = "rand", feature = "std")))] +fn test_random_keypair() -> (key::SecretKey, key::PublicKey) { + let sk = SecretKey::test_random(); + let pk = with_global_context( + |secp: &Secp256k1| key::PublicKey::from_secret_key(secp, &sk), + Some(&[0xab; 32]), + ); + (sk, pk) +} + /// Utility function used to parse hex into a target u8 buffer. Returns /// the number of bytes converted or an error if it encounters an invalid /// character or unexpected end of string. @@ -511,6 +529,25 @@ pub(crate) fn random_32_bytes(rng: &mut R) -> [u8; 32] { ret } +/// Generate "random" 32 bytes for unit testing purposes. +#[cfg(test)] +fn test_random_32_bytes() -> [u8; 32] { + // AtomicU64 not available on all platforms we support + use core::sync::atomic::{AtomicU32, Ordering}; + + const PRIME_1: u32 = 1021283; + const PRIME_2: u32 = 3348599; + static RNG: AtomicU32 = AtomicU32::new(0); + + let mut ret = [0; 32]; + for i in 0..8 { + let rng = RNG.load(Ordering::Relaxed).wrapping_mul(PRIME_1).wrapping_add(PRIME_2); + ret[i * 4..(i + 1) * 4].copy_from_slice(&rng.to_be_bytes()); + RNG.store(rng, Ordering::Relaxed); + } + ret +} + #[cfg(test)] mod tests { use std::str::FromStr; @@ -522,7 +559,7 @@ mod tests { use super::*; #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] // In rustc 1.72 this Clippy lint was pulled out of clippy and into rustc, and // was made deny-by-default, breaking compilation of this test. Aside from this // breaking change, which there is no point in bugging, the rename was done so @@ -542,7 +579,7 @@ mod tests { let sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx) }; let mut vrfy = unsafe { Secp256k1::from_raw_verification_only(ctx_vrfy.ctx) }; - let (sk, pk) = full.generate_keypair(&mut rand::rng()); + let (sk, pk) = crate::test_random_keypair(); let msg = Message::from_digest([2u8; 32]); // Try signing assert_eq!(sign.sign_ecdsa(msg, &sk), full.sign_ecdsa(msg, &sk)); diff --git a/src/musig.rs b/src/musig.rs index 06bea1a07..386e65e0b 100644 --- a/src/musig.rs +++ b/src/musig.rs @@ -1378,7 +1378,7 @@ mod tests { use super::*; #[cfg(feature = "std")] #[cfg(feature = "rand")] - use crate::{PublicKey, Secp256k1, SecretKey}; + use crate::{PublicKey, Secp256k1}; #[test] #[cfg(feature = "std")] @@ -1409,14 +1409,11 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] #[cfg(feature = "std")] - #[cfg(feature = "rand")] fn key_agg_cache() { let secp = Secp256k1::new(); - let mut rng = rand::rng(); - let (_seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (_seckey1, pubkey1) = crate::test_random_keypair(); + let (_seckey2, pubkey2) = crate::test_random_keypair(); let pubkeys = [&pubkey1, &pubkey2]; let key_agg_cache = KeyAggCache::new(&secp, &pubkeys); @@ -1430,14 +1427,11 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] #[cfg(feature = "std")] - #[cfg(feature = "rand")] fn key_agg_cache_tweaking() { let secp = Secp256k1::new(); - let mut rng = rand::rng(); - let (_seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (_seckey1, pubkey1) = crate::test_random_keypair(); + let (_seckey2, pubkey2) = crate::test_random_keypair(); let mut key_agg_cache = KeyAggCache::new(&secp, &[&pubkey1, &pubkey2]); let key_agg_cache1 = KeyAggCache::new(&secp, &[&pubkey2, &pubkey1]); @@ -1467,7 +1461,6 @@ mod tests { #[test] #[cfg(feature = "std")] - #[cfg(feature = "rand")] #[should_panic(expected = "Cannot aggregate an empty slice of pubkeys")] fn key_agg_cache_empty_panic() { let secp = Secp256k1::new(); @@ -1481,9 +1474,8 @@ mod tests { let secp = Secp256k1::new(); let mut rng = rand::rng(); - let (_seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (_seckey1, pubkey1) = crate::test_random_keypair(); + let (seckey2, pubkey2) = crate::test_random_keypair(); let key_agg_cache = KeyAggCache::new(&secp, &[&pubkey1, &pubkey2]); @@ -1520,9 +1512,8 @@ mod tests { let secp = Secp256k1::new(); let mut rng = rand::rng(); - let (_seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (_seckey1, pubkey1) = crate::test_random_keypair(); + let (_seckey2, pubkey2) = crate::test_random_keypair(); let key_agg_cache = KeyAggCache::new(&secp, &[&pubkey1, &pubkey2]); @@ -1552,7 +1543,6 @@ mod tests { #[test] #[cfg(feature = "std")] - #[cfg(feature = "rand")] #[should_panic(expected = "Cannot aggregate an empty slice of nonces")] fn aggregated_nonce_empty_panic() { let secp = Secp256k1::new(); @@ -1568,9 +1558,8 @@ mod tests { let secp = Secp256k1::new(); let mut rng = rand::rng(); - let (seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (seckey1, pubkey1) = crate::test_random_keypair(); + let (seckey2, pubkey2) = crate::test_random_keypair(); let pubkeys = [&pubkey1, &pubkey2]; let key_agg_cache = KeyAggCache::new(&secp, &pubkeys); @@ -1651,9 +1640,8 @@ mod tests { let secp = Secp256k1::new(); let mut rng = rand::rng(); - let (seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (seckey1, pubkey1) = crate::test_random_keypair(); + let (seckey2, pubkey2) = crate::test_random_keypair(); let pubkeys = [&pubkey1, &pubkey2]; let key_agg_cache = KeyAggCache::new(&secp, &pubkeys); @@ -1708,9 +1696,8 @@ mod tests { let secp = Secp256k1::new(); let mut rng = rand::rng(); - let (_seckey1, pubkey1) = secp.generate_keypair(&mut rng); - let seckey2 = SecretKey::new(&mut rng); - let pubkey2 = PublicKey::from_secret_key(&secp, &seckey2); + let (_seckey1, pubkey1) = crate::test_random_keypair(); + let (_seckey2, pubkey2) = crate::test_random_keypair(); let pubkeys = [pubkey1, pubkey2]; let mut pubkeys_ref: Vec<&PublicKey> = pubkeys.iter().collect(); diff --git a/src/scalar.rs b/src/scalar.rs index b67ca740e..3d284176c 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -106,6 +106,22 @@ impl Scalar { self.as_be_bytes().as_c_ptr() } + + /// Constructor for unit testing. + #[cfg(test)] + #[cfg(all(feature = "rand", feature = "std"))] + pub fn test_random() -> Self { Self::random() } + + /// Constructor for unit testing. + #[cfg(test)] + #[cfg(not(all(feature = "rand", feature = "std")))] + pub fn test_random() -> Self { + loop { + if let Ok(ret) = Self::from_be_bytes(crate::test_random_32_bytes()) { + return ret; + } + } + } } impl ops::Index for Scalar diff --git a/src/schnorr.rs b/src/schnorr.rs index 45c35d9db..abe0a3666 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -225,10 +225,10 @@ mod tests { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn schnorr_sign_with_aux_rand_verify() { - sign_helper(|secp, msg, seckey, rng| { - let aux_rand = crate::random_32_bytes(rng); + sign_helper((), |secp, msg, seckey, _| { + let aux_rand = crate::test_random_32_bytes(); secp.sign_schnorr_with_aux_rand(msg, seckey, &aux_rand) }) } @@ -236,29 +236,35 @@ mod tests { #[test] #[cfg(all(feature = "rand", feature = "std"))] fn schnor_sign_with_rng_verify() { - sign_helper(|secp, msg, seckey, rng| secp.sign_schnorr_with_rng(msg, seckey, rng)) + sign_helper(&mut rand::rng(), |secp, msg, seckey, rng| { + secp.sign_schnorr_with_rng(msg, seckey, rng) + }) } #[test] - #[cfg(all(feature = "rand", feature = "std"))] - fn schnorr_sign_verify() { sign_helper(|secp, msg, seckey, _| secp.sign_schnorr(msg, seckey)) } + #[cfg(all(feature = "rand", feature = "std"))] // sign_schnorr requires "rand" + fn schnorr_sign_verify() { + sign_helper((), |secp, msg, seckey, _| secp.sign_schnorr(msg, seckey)) + } #[test] - #[cfg(all(feature = "rand", feature = "std"))] + #[cfg(feature = "std")] fn schnorr_sign_no_aux_rand_verify() { - sign_helper(|secp, msg, seckey, _| secp.sign_schnorr_no_aux_rand(msg, seckey)) + sign_helper((), |secp, msg, seckey, _| secp.sign_schnorr_no_aux_rand(msg, seckey)) } - #[cfg(all(feature = "rand", feature = "std"))] - fn sign_helper(sign: fn(&Secp256k1, &[u8], &Keypair, &mut ThreadRng) -> Signature) { + #[cfg(feature = "std")] + fn sign_helper( + mut rng: R, + sign: fn(&Secp256k1, &[u8], &Keypair, &mut R) -> Signature, + ) { let secp = Secp256k1::new(); - let mut rng = rand::rng(); - let kp = Keypair::new(&secp, &mut rng); + let kp = Keypair::test_random(); let (pk, _parity) = kp.x_only_public_key(); for _ in 0..100 { - let msg = crate::random_32_bytes(&mut rand::rng()); + let msg = crate::test_random_32_bytes(); let sig = sign(&secp, &msg, &kp, &mut rng); @@ -358,10 +364,8 @@ mod tests { } #[test] - #[cfg(all(feature = "rand", feature = "std"))] fn test_pubkey_serialize_roundtrip() { - let secp = Secp256k1::new(); - let kp = Keypair::new(&secp, &mut rand::rng()); + let kp = Keypair::test_random(); let (pk, _parity) = kp.x_only_public_key(); let ser = pk.serialize(); From c5adba0ff085a3e7ce76d9fabbf0fe31ffa6abb8 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 20 Jun 2025 21:24:08 +0000 Subject: [PATCH 7/8] key: update a couple arbitrary API functions to no longer take a context This updates a couple functions, and their associated unit tests (which no longer need any std/alloc/global-context feature gates). This runs clean in valgrind, providing some evidence that my new code is sound. --- src/key.rs | 113 +++++++++++++++++++++---------------------------- src/schnorr.rs | 17 +++----- tests/serde.rs | 7 ++- 3 files changed, 59 insertions(+), 78 deletions(-) diff --git a/src/key.rs b/src/key.rs index b51efd322..8449fbd59 100644 --- a/src/key.rs +++ b/src/key.rs @@ -20,8 +20,7 @@ use crate::ThirtyTwoByteHash; #[cfg(feature = "global-context")] use crate::SECP256K1; use crate::{ - constants, ecdsa, from_hex, schnorr, AllPreallocated, Message, Scalar, Secp256k1, Signing, - Verification, + constants, ecdsa, from_hex, schnorr, Message, Scalar, Secp256k1, Signing, Verification, }; /// Secret key - a 256-bit key used to create ECDSA and Taproot signatures. @@ -610,11 +609,12 @@ impl PublicKey { /// Negates the public key. #[inline] #[must_use = "you forgot to use the negated public key"] - pub fn negate(mut self, secp: &Secp256k1) -> PublicKey { - unsafe { - let res = ffi::secp256k1_ec_pubkey_negate(secp.ctx.as_ptr(), &mut self.0); - debug_assert_eq!(res, 1); - } + pub fn negate(mut self) -> PublicKey { + let res = crate::with_raw_global_context( + |ctx| unsafe { ffi::secp256k1_ec_pubkey_negate(ctx.as_ptr(), &mut self.0) }, + None, + ); + debug_assert_eq!(res, 1); self } @@ -624,19 +624,17 @@ impl PublicKey { /// /// Returns an error if the resulting key would be invalid. #[inline] - pub fn add_exp_tweak( - mut self, - secp: &Secp256k1, - tweak: &Scalar, - ) -> Result { - unsafe { - if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx.as_ptr(), &mut self.0, tweak.as_c_ptr()) - == 1 - { - Ok(self) - } else { - Err(Error::InvalidTweak) - } + pub fn add_exp_tweak(mut self, tweak: &Scalar) -> Result { + if crate::with_raw_global_context( + |ctx| unsafe { + ffi::secp256k1_ec_pubkey_tweak_add(ctx.as_ptr(), &mut self.0, tweak.as_c_ptr()) + }, + None, + ) == 1 + { + Ok(self) + } else { + Err(Error::InvalidTweak) } } @@ -880,12 +878,9 @@ impl Keypair { /// or if the encoded number is an invalid scalar. #[deprecated(since = "0.31.0", note = "Use `from_seckey_byte_array` instead.")] #[inline] - pub fn from_seckey_slice( - secp: &Secp256k1, - data: &[u8], - ) -> Result { + pub fn from_seckey_slice(data: &[u8]) -> Result { match <[u8; constants::SECRET_KEY_SIZE]>::try_from(data) { - Ok(data) => Self::from_seckey_byte_array(secp, data), + Ok(data) => Self::from_seckey_byte_array(data), Err(_) => Err(Error::InvalidSecretKey), } } @@ -896,13 +891,16 @@ impl Keypair { /// /// [`Error::InvalidSecretKey`] if the encoded number is an invalid scalar. #[inline] - pub fn from_seckey_byte_array( - secp: &Secp256k1, + pub fn from_seckey_byte_array( data: [u8; constants::SECRET_KEY_SIZE], ) -> Result { unsafe { let mut kp = ffi::Keypair::new(); - if ffi::secp256k1_keypair_create(secp.ctx.as_ptr(), &mut kp, data.as_c_ptr()) == 1 { + if crate::with_raw_global_context( + |ctx| ffi::secp256k1_keypair_create(ctx.as_ptr(), &mut kp, data.as_c_ptr()), + Some(&data), + ) == 1 + { Ok(Keypair(kp)) } else { Err(Error::InvalidSecretKey) @@ -917,13 +915,8 @@ impl Keypair { /// [`Error::InvalidSecretKey`] if the string does not consist of exactly 64 hex characters, /// or if the encoded number is an invalid scalar. #[inline] - pub fn from_seckey_str(secp: &Secp256k1, s: &str) -> Result { - let mut res = [0u8; constants::SECRET_KEY_SIZE]; - match from_hex(s, &mut res) { - Ok(constants::SECRET_KEY_SIZE) => Keypair::from_seckey_byte_array(secp, res), - _ => Err(Error::InvalidSecretKey), - } - } + #[deprecated(note = "use FromStr or parse instead")] + pub fn from_seckey_str(s: &str) -> Result { s.parse() } /// Creates a [`Keypair`] directly from a secret key string and the global [`SECP256K1`] context. /// @@ -932,10 +925,8 @@ impl Keypair { /// [`Error::InvalidSecretKey`] if the string does not consist of exactly 64 hex characters, /// or if the encoded number is an invalid scalar. #[inline] - #[cfg(feature = "global-context")] - pub fn from_seckey_str_global(s: &str) -> Result { - Keypair::from_seckey_str(SECP256K1, s) - } + #[deprecated(note = "use FromStr or parse instead")] + pub fn from_seckey_str_global(s: &str) -> Result { s.parse() } /// Generates a new random key pair. /// # Examples @@ -1107,10 +1098,11 @@ impl str::FromStr for Keypair { type Err = Error; fn from_str(s: &str) -> Result { - crate::with_global_context( - |secp: &Secp256k1| Self::from_seckey_str(secp, s), - None, - ) + let mut res = [0u8; constants::SECRET_KEY_SIZE]; + match from_hex(s, &mut res) { + Ok(constants::SECRET_KEY_SIZE) => Keypair::from_seckey_byte_array(res), + _ => Err(Error::InvalidSecretKey), + } } } @@ -1141,12 +1133,10 @@ impl<'de> serde::Deserialize<'de> for Keypair { "a hex string representing 32 byte Keypair", )) } else { - let visitor = super::serde_util::Tuple32Visitor::new("raw 32 bytes Keypair", |data| { - crate::with_global_context( - |secp: &Secp256k1| Self::from_seckey_byte_array(secp, data), - None, - ) - }); + let visitor = super::serde_util::Tuple32Visitor::new( + "raw 32 bytes Keypair", + Keypair::from_seckey_byte_array, + ); d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor) } } @@ -1726,10 +1716,9 @@ mod test { } #[test] - #[cfg(all(feature = "std", not(secp256k1_fuzz)))] + #[cfg(not(secp256k1_fuzz))] fn erased_keypair_is_valid() { - let s = Secp256k1::new(); - let kp = Keypair::from_seckey_byte_array(&s, [1u8; constants::SECRET_KEY_SIZE]) + let kp = Keypair::from_seckey_byte_array([1u8; constants::SECRET_KEY_SIZE]) .expect("valid secret key"); let mut kp2 = kp; kp2.non_secure_erase(); @@ -2007,24 +1996,21 @@ mod test { let tweaked_sk = sk.add_tweak(&tweak).unwrap(); assert_ne!(sk, tweaked_sk); // Make sure we did something. - let tweaked_pk = pk.add_exp_tweak(&s, &tweak).unwrap(); + let tweaked_pk = pk.add_exp_tweak(&tweak).unwrap(); assert_ne!(pk, tweaked_pk); assert_eq!(PublicKey::from_secret_key(&s, &tweaked_sk), tweaked_pk); } #[test] - #[cfg(feature = "std")] fn tweak_add_zero() { - let s = Secp256k1::new(); - let (sk, pk) = crate::test_random_keypair(); let tweak = Scalar::ZERO; let tweaked_sk = sk.add_tweak(&tweak).unwrap(); assert_eq!(sk, tweaked_sk); // Tweak by zero does nothing. - let tweaked_pk = pk.add_exp_tweak(&s, &tweak).unwrap(); + let tweaked_pk = pk.add_exp_tweak(&tweak).unwrap(); assert_eq!(pk, tweaked_pk); } @@ -2069,9 +2055,9 @@ mod test { let back_sk = neg.negate(); assert_eq!(sk, back_sk); - let neg = pk.negate(&s); + let neg = pk.negate(); assert_ne!(pk, neg); - let back_pk = neg.negate(&s); + let back_pk = neg.negate(); assert_eq!(pk, back_pk); assert_eq!(PublicKey::from_secret_key(&s, &back_sk), pk); @@ -2329,7 +2315,7 @@ mod test { ]; static SK_STR: &str = "01010101010101010001020304050607ffff0000ffff00006363636363636363"; - let sk = Keypair::from_seckey_byte_array(SECP256K1, SK_BYTES).unwrap(); + let sk = Keypair::from_seckey_byte_array(SK_BYTES).unwrap(); #[rustfmt::skip] assert_tokens(&sk.compact(), &[ Token::Tuple{ len: 32 }, @@ -2510,7 +2496,7 @@ mod test { static PK_STR: &str = "18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166"; - let kp = Keypair::from_seckey_byte_array(crate::SECP256K1, SK_BYTES).unwrap(); + let kp = Keypair::from_seckey_byte_array(SK_BYTES).unwrap(); let (pk, _parity) = XOnlyPublicKey::from_keypair(&kp); #[rustfmt::skip] @@ -2538,11 +2524,10 @@ mod test { } #[test] - #[cfg(all(any(feature = "alloc", feature = "global-context"), feature = "serde"))] + #[cfg(feature = "serde")] fn test_keypair_deserialize_serde() { - let ctx = crate::Secp256k1::new(); let sec_key_str = "4242424242424242424242424242424242424242424242424242424242424242"; - let keypair = Keypair::from_seckey_str(&ctx, sec_key_str).unwrap(); + let keypair = Keypair::from_str(sec_key_str).unwrap(); serde_test::assert_tokens(&keypair.readable(), &[Token::String(sec_key_str)]); diff --git a/src/schnorr.rs b/src/schnorr.rs index abe0a3666..8afdb3770 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -279,11 +279,9 @@ mod tests { let secp = Secp256k1::new(); let msg = hex_32!("E48441762FB75010B2AA31A512B62B4148AA3FB08EB0765D76B252559064A614"); - let sk = Keypair::from_seckey_str( - &secp, - "688C77BC2D5AAFF5491CF309D4753B732135470D05B7B2CD21ADD0744FE97BEF", - ) - .unwrap(); + let sk = + Keypair::from_str("688C77BC2D5AAFF5491CF309D4753B732135470D05B7B2CD21ADD0744FE97BEF") + .unwrap(); let aux_rand: [u8; 32] = hex_32!("02CCE08E913F22A36C5648D6405A2C7C50106E7AA2F1649E381C7F09D16B80AB"); let expected_sig = Signature::from_str("6470FD1303DDA4FDA717B9837153C24A6EAB377183FC438F939E0ED2B620E9EE5077C4A8B8DCA28963D772A94F5F0DDF598E1C47C137F91933274C7C3EDADCE8").unwrap(); @@ -378,7 +376,7 @@ mod tests { fn test_xonly_key_extraction() { let secp = Secp256k1::new(); let sk_str = "688C77BC2D5AAFF5491CF309D4753B732135470D05B7B2CD21ADD0744FE97BEF"; - let keypair = Keypair::from_seckey_str(&secp, sk_str).unwrap(); + let keypair = Keypair::from_str(sk_str).unwrap(); let sk = SecretKey::from_keypair(&keypair); assert_eq!(SecretKey::from_str(sk_str).unwrap(), sk); let pk = crate::key::PublicKey::from_keypair(&keypair); @@ -392,14 +390,13 @@ mod tests { fn test_pubkey_display_output() { #[cfg(not(secp256k1_fuzz))] let pk = { - let secp = Secp256k1::new(); static SK_BYTES: [u8; 32] = [ 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0xff, 0xff, 0x00, 0x00, 0xff, 0xff, 0x00, 0x00, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, ]; - let kp = Keypair::from_seckey_byte_array(&secp, SK_BYTES).expect("sk"); + let kp = Keypair::from_seckey_byte_array(SK_BYTES).expect("sk"); // In fuzzing mode secret->public key derivation is different, so // hard-code the expected result. @@ -479,7 +476,7 @@ mod tests { let s = Secp256k1::new(); let msg = [1; 32]; - let keypair = Keypair::from_seckey_byte_array(&s, [2; 32]).unwrap(); + let keypair = Keypair::from_seckey_byte_array([2; 32]).unwrap(); let aux = [3u8; 32]; let sig = s.sign_schnorr_with_aux_rand(&msg, &keypair, &aux); static SIG_BYTES: [u8; constants::SCHNORR_SIGNATURE_SIZE] = [ @@ -712,7 +709,7 @@ mod tests { } in vectors { if let (Some(secret_key), Some(aux_rand)) = (secret_key, aux_rand) { - let keypair = Keypair::from_seckey_byte_array(&secp, secret_key).unwrap(); + let keypair = Keypair::from_seckey_byte_array(secret_key).unwrap(); assert_eq!(keypair.x_only_public_key().0.serialize(), public_key); let sig = secp.sign_schnorr_with_aux_rand(&message, &keypair, &aux_rand); assert_eq!(sig.to_byte_array(), signature); diff --git a/tests/serde.rs b/tests/serde.rs index 4df4db990..7bc70cc24 100644 --- a/tests/serde.rs +++ b/tests/serde.rs @@ -4,9 +4,9 @@ extern crate bincode; extern crate secp256k1; extern crate serde_cbor; -use secp256k1::{musig, PublicKey, SecretKey, XOnlyPublicKey}; #[cfg(feature = "global-context")] -use secp256k1::{Keypair, Secp256k1}; +use secp256k1::Keypair; +use secp256k1::{musig, PublicKey, SecretKey, XOnlyPublicKey}; // Arbitrary key data. @@ -97,8 +97,7 @@ fn bincode_public_key() { #[test] #[cfg(feature = "global-context")] fn bincode_keypair() { - let secp = Secp256k1::new(); - let kp = Keypair::from_seckey_byte_array(&secp, SK_BYTES).expect("failed to create keypair"); + let kp = Keypair::from_seckey_byte_array(SK_BYTES).expect("failed to create keypair"); let ser = bincode::serialize(&kp).unwrap(); assert_eq!(ser, SK_BYTES); From 45c264e1d12b665625dbbab9e05700eca3b8b855 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 21 Jun 2025 14:35:05 +0000 Subject: [PATCH 8/8] recovery: rewrite API to not use context objects This API is basically unused except for some niche or legacy applications, so I feel comfortable breaking it pretty dramatically. Move all the Secp256k1 functions onto RecoverableSignature and use self/Self as appropriate. Leave the stupid ecdsa_recoverable names even though they are even more redundant, because this module is basically in maintenance mode. We only do these changes since we'll be forced to once we drop the Secp256k1 object. --- Cargo.toml | 2 +- examples/sign_verify_recovery.rs | 27 ++---- no_std_test/src/main.rs | 6 +- src/ecdsa/recovery.rs | 155 +++++++++++++------------------ 4 files changed, 75 insertions(+), 115 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 71acbf4a2..87553f076 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,7 @@ unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)', 'cfg(secp256k1_fu [[example]] name = "sign_verify_recovery" -required-features = ["recovery", "hashes", "std"] +required-features = ["recovery", "hashes"] [[example]] name = "sign_verify" diff --git a/examples/sign_verify_recovery.rs b/examples/sign_verify_recovery.rs index 78cbbd54e..f7a49f05e 100644 --- a/examples/sign_verify_recovery.rs +++ b/examples/sign_verify_recovery.rs @@ -2,36 +2,25 @@ extern crate hashes; extern crate secp256k1; use hashes::{sha256, Hash}; -use secp256k1::{ecdsa, Error, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification}; - -fn recover( - secp: &Secp256k1, - msg: &[u8], - sig: [u8; 64], - recovery_id: u8, -) -> Result { +use secp256k1::{ecdsa, Error, Message, PublicKey, SecretKey}; + +fn recover(msg: &[u8], sig: [u8; 64], recovery_id: u8) -> Result { let msg = sha256::Hash::hash(msg); let msg = Message::from_digest(msg.to_byte_array()); let id = ecdsa::RecoveryId::try_from(i32::from(recovery_id))?; let sig = ecdsa::RecoverableSignature::from_compact(&sig, id)?; - secp.recover_ecdsa(msg, &sig) + sig.recover_ecdsa(msg) } -fn sign_recovery( - secp: &Secp256k1, - msg: &[u8], - seckey: [u8; 32], -) -> Result { +fn sign_recovery(msg: &[u8], seckey: [u8; 32]) -> Result { let msg = sha256::Hash::hash(msg); let msg = Message::from_digest(msg.to_byte_array()); let seckey = SecretKey::from_byte_array(seckey)?; - Ok(secp.sign_ecdsa_recoverable(msg, &seckey)) + Ok(ecdsa::RecoverableSignature::sign_ecdsa_recoverable(msg, &seckey)) } fn main() { - let secp = Secp256k1::new(); - let seckey = [ 59, 148, 11, 85, 134, 130, 61, 253, 2, 174, 59, 70, 27, 180, 51, 107, 94, 203, 174, 253, 102, 39, 170, 146, 46, 252, 4, 143, 236, 12, 136, 28, @@ -43,9 +32,9 @@ fn main() { .unwrap(); let msg = b"This is some message"; - let signature = sign_recovery(&secp, msg, seckey).unwrap(); + let signature = sign_recovery(msg, seckey).unwrap(); let (recovery_id, serialize_sig) = signature.serialize_compact(); - assert_eq!(recover(&secp, msg, serialize_sig, recovery_id.to_u8()), Ok(pubkey)); + assert_eq!(recover(msg, serialize_sig, recovery_id.to_u8()), Ok(pubkey)); } diff --git a/no_std_test/src/main.rs b/no_std_test/src/main.rs index ff56e2a74..b82442db3 100644 --- a/no_std_test/src/main.rs +++ b/no_std_test/src/main.rs @@ -51,7 +51,7 @@ use core::panic::PanicInfo; use secp256k1::ecdh::{self, SharedSecret}; use secp256k1::ffi::types::AlignedType; -use secp256k1::rand::{self, RngCore}; +use secp256k1::rand::RngCore; use secp256k1::serde::Serialize; use secp256k1::*; @@ -93,9 +93,9 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize { let sig = secp.sign_ecdsa(message, &secret_key); assert!(secp.verify_ecdsa(&sig, message, &public_key).is_ok()); - let rec_sig = secp.sign_ecdsa_recoverable(message, &secret_key); + let rec_sig = ecdsa::RecoverableSignature::sign_ecdsa_recoverable(message, &secret_key); assert!(secp.verify_ecdsa(&rec_sig.to_standard(), message, &public_key).is_ok()); - assert_eq!(public_key, secp.recover_ecdsa(message, &rec_sig).unwrap()); + assert_eq!(public_key, rec_sig.recover_ecdsa(message).unwrap()); let (rec_id, data) = rec_sig.serialize_compact(); let new_rec_sig = ecdsa::RecoverableSignature::from_compact(&data, rec_id).unwrap(); assert_eq!(rec_sig, new_rec_sig); diff --git a/src/ecdsa/recovery.rs b/src/ecdsa/recovery.rs index e2d32cde0..3136a014f 100644 --- a/src/ecdsa/recovery.rs +++ b/src/ecdsa/recovery.rs @@ -10,7 +10,7 @@ use self::super_ffi::CPtr; use super::ffi as super_ffi; use crate::ecdsa::Signature; use crate::ffi::recovery as ffi; -use crate::{key, Error, Message, Secp256k1, Signing, Verification}; +use crate::{key, Error, Message}; /// A tag used for recovering the public key from a compact signature. #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -157,7 +157,7 @@ impl RecoverableSignature { #[inline] #[cfg(feature = "global-context")] pub fn recover(&self, msg: impl Into) -> Result { - crate::SECP256K1.recover_ecdsa(msg, self) + self.recover_ecdsa(msg) } } @@ -174,30 +174,38 @@ impl From for RecoverableSignature { fn from(sig: ffi::RecoverableSignature) -> RecoverableSignature { RecoverableSignature(sig) } } -impl Secp256k1 { +impl RecoverableSignature { fn sign_ecdsa_recoverable_with_noncedata_pointer( - &self, msg: impl Into, sk: &key::SecretKey, noncedata_ptr: *const super_ffi::types::c_void, - ) -> RecoverableSignature { + ) -> Self { let msg = msg.into(); let mut ret = ffi::RecoverableSignature::new(); - unsafe { - // We can assume the return value because it's not possible to construct - // an invalid signature from a valid `Message` and `SecretKey` - assert_eq!( - ffi::secp256k1_ecdsa_sign_recoverable( - self.ctx.as_ptr(), - &mut ret, - msg.as_c_ptr(), - sk.as_c_ptr(), - super_ffi::secp256k1_nonce_function_rfc6979, - noncedata_ptr - ), - 1 - ); + // xor the secret key and message together to get a rerandomization seed + // for timing analysis defense-in-depth + let mut rerandomize = sk.secret_bytes(); + for (rera, byte) in rerandomize.iter_mut().zip(msg[..].iter()) { + *rera ^= *byte; } + crate::with_raw_global_context( + |ctx| unsafe { + // We can assume the return value because it's not possible to construct + // an invalid signature from a valid `Message` and `SecretKey` + assert_eq!( + ffi::secp256k1_ecdsa_sign_recoverable( + ctx.as_ptr(), + &mut ret, + msg.as_c_ptr(), + sk.as_c_ptr(), + super_ffi::secp256k1_nonce_function_rfc6979, + noncedata_ptr + ), + 1 + ); + }, + Some(&rerandomize), + ); RecoverableSignature::from(ret) } @@ -205,11 +213,10 @@ impl Secp256k1 { /// Constructs a signature for `msg` using the secret key `sk` and RFC6979 nonce /// Requires a signing-capable context. pub fn sign_ecdsa_recoverable( - &self, msg: impl Into, sk: &key::SecretKey, ) -> RecoverableSignature { - self.sign_ecdsa_recoverable_with_noncedata_pointer(msg, sk, ptr::null()) + Self::sign_ecdsa_recoverable_with_noncedata_pointer(msg, sk, ptr::null()) } /// Constructs a signature for `msg` using the secret key `sk` and RFC6979 nonce @@ -218,38 +225,34 @@ impl Secp256k1 { /// signatures are needed for the same Message and SecretKey while still using RFC6979. /// Requires a signing-capable context. pub fn sign_ecdsa_recoverable_with_noncedata( - &self, msg: impl Into, sk: &key::SecretKey, noncedata: &[u8; 32], ) -> RecoverableSignature { let noncedata_ptr = noncedata.as_ptr() as *const super_ffi::types::c_void; - self.sign_ecdsa_recoverable_with_noncedata_pointer(msg, sk, noncedata_ptr) + Self::sign_ecdsa_recoverable_with_noncedata_pointer(msg, sk, noncedata_ptr) } -} -impl Secp256k1 { /// Determines the public key for which `sig` is a valid signature for /// `msg`. Requires a verify-capable context. - pub fn recover_ecdsa( - &self, - msg: impl Into, - sig: &RecoverableSignature, - ) -> Result { + pub fn recover_ecdsa(&self, msg: impl Into) -> Result { let msg = msg.into(); - unsafe { - let mut pk = super_ffi::PublicKey::new(); - if ffi::secp256k1_ecdsa_recover( - self.ctx.as_ptr(), - &mut pk, - sig.as_c_ptr(), - msg.as_c_ptr(), - ) != 1 - { - return Err(Error::InvalidSignature); - } - Ok(key::PublicKey::from(pk)) - } + crate::with_raw_global_context( + |ctx| unsafe { + let mut pk = super_ffi::PublicKey::new(); + if ffi::secp256k1_ecdsa_recover( + ctx.as_ptr(), + &mut pk, + self.as_c_ptr(), + msg.as_c_ptr(), + ) != 1 + { + return Err(Error::InvalidSignature); + } + Ok(key::PublicKey::from(pk)) + }, + None, + ) } } @@ -264,28 +267,13 @@ mod tests { use crate::{Error, Message, Secp256k1, SecretKey}; #[test] - #[cfg(feature = "std")] fn capabilities() { - let sign = Secp256k1::signing_only(); - let vrfy = Secp256k1::verification_only(); - let full = Secp256k1::new(); - let msg = crate::test_random_32_bytes(); let msg = Message::from_digest(msg); - // Try key generation let (sk, pk) = crate::test_random_keypair(); - - // Try signing - assert_eq!(sign.sign_ecdsa_recoverable(msg, &sk), full.sign_ecdsa_recoverable(msg, &sk)); - let sigr = full.sign_ecdsa_recoverable(msg, &sk); - - // Try pk recovery - assert!(vrfy.recover_ecdsa(msg, &sigr).is_ok()); - assert!(full.recover_ecdsa(msg, &sigr).is_ok()); - - assert_eq!(vrfy.recover_ecdsa(msg, &sigr), full.recover_ecdsa(msg, &sigr)); - assert_eq!(full.recover_ecdsa(msg, &sigr), Ok(pk)); + let sigr = RecoverableSignature::sign_ecdsa_recoverable(msg, &sk); + assert_eq!(sigr.recover_ecdsa(msg), Ok(pk)); } #[test] @@ -296,15 +284,11 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] // fixed sig vectors can't work with fuzz-sigs - #[cfg(feature = "std")] #[rustfmt::skip] fn sign() { - let s = Secp256k1::new(); - let sk = SecretKey::from_byte_array(ONE).unwrap(); let msg = Message::from_digest(ONE); - - let sig = s.sign_ecdsa_recoverable(msg, &sk); + let sig = RecoverableSignature::sign_ecdsa_recoverable(msg, &sk); assert_eq!(Ok(sig), RecoverableSignature::from_compact(&[ 0x66, 0x73, 0xff, 0xad, 0x21, 0x47, 0x74, 0x1f, @@ -320,16 +304,13 @@ mod tests { #[test] #[cfg(not(secp256k1_fuzz))] // fixed sig vectors can't work with fuzz-sigs - #[cfg(feature = "std")] #[rustfmt::skip] fn sign_with_noncedata() { - let s = Secp256k1::new(); - let sk = SecretKey::from_byte_array(ONE).unwrap(); - let msg = Message::from_digest(ONE); let noncedata = [42u8; 32]; + let msg = Message::from_digest(ONE); - let sig = s.sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata); + let sig = RecoverableSignature::sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata); assert_eq!(Ok(sig), RecoverableSignature::from_compact(&[ 0xb5, 0x0b, 0xb6, 0x79, 0x5f, 0x31, 0x74, 0x8a, @@ -351,57 +332,48 @@ mod tests { let msg = Message::from_digest(crate::test_random_32_bytes()); let (sk, pk) = crate::test_random_keypair(); - let sigr = s.sign_ecdsa_recoverable(msg, &sk); + let sigr = RecoverableSignature::sign_ecdsa_recoverable(msg, &sk); let sig = sigr.to_standard(); let msg = Message::from_digest(crate::test_random_32_bytes()); assert_eq!(s.verify_ecdsa(&sig, msg, &pk), Err(Error::IncorrectSignature)); - let recovered_key = s.recover_ecdsa(msg, &sigr).unwrap(); + let recovered_key = sigr.recover_ecdsa(msg).unwrap(); assert!(recovered_key != pk); } #[test] - #[cfg(feature = "std")] fn sign_with_recovery() { - let s = Secp256k1::new(); - let msg = Message::from_digest(crate::test_random_32_bytes()); let (sk, pk) = crate::test_random_keypair(); - let sig = s.sign_ecdsa_recoverable(msg, &sk); + let sig = RecoverableSignature::sign_ecdsa_recoverable(msg, &sk); - assert_eq!(s.recover_ecdsa(msg, &sig), Ok(pk)); + assert_eq!(sig.recover_ecdsa(msg), Ok(pk)); } #[test] - #[cfg(feature = "std")] fn sign_with_recovery_and_noncedata() { - let s = Secp256k1::new(); - let msg = Message::from_digest(crate::test_random_32_bytes()); let noncedata = crate::test_random_32_bytes(); let (sk, pk) = crate::test_random_keypair(); - let sig = s.sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata); + let sig = RecoverableSignature::sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata); - assert_eq!(s.recover_ecdsa(msg, &sig), Ok(pk)); + assert_eq!(sig.recover_ecdsa(msg), Ok(pk)); } #[test] - #[cfg(feature = "std")] fn bad_recovery() { - let s = Secp256k1::new(); - let msg = Message::from_digest(crate::test_random_32_bytes()); // Zero is not a valid sig let sig = RecoverableSignature::from_compact(&[0; 64], RecoveryId::Zero).unwrap(); - assert_eq!(s.recover_ecdsa(msg, &sig), Err(Error::InvalidSignature)); + assert_eq!(sig.recover_ecdsa(msg), Err(Error::InvalidSignature)); // ...but 111..111 is let sig = RecoverableSignature::from_compact(&[1; 64], RecoveryId::Zero).unwrap(); - assert!(s.recover_ecdsa(msg, &sig).is_ok()); + assert!(sig.recover_ecdsa(msg).is_ok()); } #[test] @@ -455,21 +427,20 @@ mod tests { } #[cfg(bench)] -#[cfg(feature = "std")] // Currently only a single bench that requires "rand" + "std". mod benches { use test::{black_box, Bencher}; - use crate::{Message, Secp256k1, SecretKey}; + use super::RecoverableSignature; + use crate::{Message, SecretKey}; #[bench] pub fn bench_recover(bh: &mut Bencher) { - let s = Secp256k1::new(); let msg = Message::from_digest(crate::test_random_32_bytes()); let sk = SecretKey::test_random(); - let sig = s.sign_ecdsa_recoverable(msg, &sk); + let sig = RecoverableSignature::sign_ecdsa_recoverable(msg, &sk); bh.iter(|| { - let res = s.recover_ecdsa(msg, &sig).unwrap(); + let res = sig.recover_ecdsa(msg).unwrap(); black_box(res); }); }