Merge rust-bitcoin/rust-secp256k1#806: context: introduce new global context API with rerandomization

19cfe160d85d5ed385e5aed4779c27890e749801 recovery: rewrite API to not use context objects (Andrew Poelstra)
4f600dbcce04fe8b0285449d464afd3fc0059db8 key: update a couple arbitrary API functions to no longer take a context (Andrew Poelstra)
362495bf3efbef7746b63d527e4cc9fe1a753f1a test: remove a ton of rand feature-gating (Andrew Poelstra)
5fa32b33e2ed3c72ed1c53a04f998ce6dcb06b01 key: remove std/alloc/global-context gates from serde::deserialize and FromStr (Andrew Poelstra)
0e429502b2e99eeb187347c4b88c5a89244f6347 context: add nostd version of global context (Andrew Poelstra)
979aa1a0a76822cc82a97cd0cfe1976c73f2a993 context: introduce spinlock that gives up after a few iterations (Andrew Poelstra)
9d548728e84687a8a650843e8cdc2b9f46ea3dd7 context: introduce global rerandomizable context (std only) (Andrew Poelstra)
1e45d4cd93ae848c614db76595526cd601a424ee context: rename src/context.rs to src/context/mod.rs (Andrew Poelstra)

Pull request description:

  As discussed in #388 and its parent issues, when `std` is enabled we have a fairly straightforward way to enable global contexts. We use thread-local variables and on every access we rerandomize them. When the `rand` crate is also available the situation is even better, because we don't need to think too hard about where to get entropy from.
  
  In the nostd case things are harder. We have no thread locals and basically no synchronization primitives except atomics, which can be used to implement spinlocks but nothing else.  [Kix has argued strongly against spinlocks](rust-bitcoin/rust-secp256k1#346 (comment)) but in the [following several messages](rust-bitcoin/rust-secp256k1#346 (comment)) we came to a solution in which do a "soft spinlock" where after a couple iterations we just give up and don't rerandomize.
  
  Kix suggested adding some logging and debugging facilities, which I did not include in my solution here. We can add those in a followup.
  
  Kix also suggested setting the maximum spin count to 0, on the theory that in most cases there will never be any contention except in cases of reentrancy, and in that case spinning is pointless. I think it should be higher than zero to help in situations where there really are multiple threads. I set it to 128 which shouldn't be a noticable (or even measurable) burden even in the case where the spinning is pointless.
  
  This mostly resolves #388. To completely resolve that issue, we need to:
  
  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.
  
  Once we've done that, we will be much better-equipped to address #346. To do *that*, we should attempt to scrape together some entropy even on nostd without the rand crate. I believe we can do this by reading the system time and CPU jitter. We don't need to do a very good job for this to work; even a bit or two of entropy on each signature will BTFO an attacker attempting to learn timing information from multiple signatures.


ACKs for top commit:
  tcharding:
    ACK 19cfe160d85d5ed385e5aed4779c27890e749801


Tree-SHA512: 5b0be1472ef7a52221a01c141ac58f080c85f954515c567e2ecba6549f2d970996a0f7ce3c5349c2391b1eee3b504b695efdddf86a5cc70ab411dd5f3a40704b

Author: chain-forgexcr45

Cargo.toml

@@ -57,7 +57,7 @@ unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)', 'cfg(secp256k1_fu
 
 [[example]]
 name = "sign_verify_recovery"
-required-features = ["recovery", "std"]
+required-features = ["recovery"]
 
 [[example]]
 name = "sign_verify"

examples/sign_verify_recovery.rs

@@ -1,33 +1,25 @@
 extern crate secp256k1;
 
-use secp256k1::{ecdsa, Error, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification};
+use secp256k1::{ecdsa, Error, Message, PublicKey, SecretKey};
 
-fn recover<C: Verification>(
-    secp: &Secp256k1<C>,
-    msg_digest: [u8; 32],
-    sig: [u8; 64],
-    recovery_id: u8,
-) -> Result<PublicKey, Error> {
-    let msg = Message::from_digest(msg_digest);
+fn recover(msg_digest: [u8; 32], sig: [u8; 64], recovery_id: u8) -> Result<PublicKey, Error> {
     let id = ecdsa::RecoveryId::try_from(i32::from(recovery_id))?;
     let sig = ecdsa::RecoverableSignature::from_compact(&sig, id)?;
+    let msg = Message::from_digest(msg_digest);
 
-    secp.recover_ecdsa(msg, &sig)
+    sig.recover_ecdsa(msg)
 }
 
-fn sign_recovery<C: Signing>(
-    secp: &Secp256k1<C>,
+fn sign_recovery(
     msg_digest: [u8; 32],
     seckey: [u8; 32],
 ) -> Result<ecdsa::RecoverableSignature, Error> {
     let msg = Message::from_digest(msg_digest);
     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,
@@ -39,9 +31,9 @@ fn main() {
     .unwrap();
     let msg_digest = *b"this must be secure hash output.";
 
-    let signature = sign_recovery(&secp, msg_digest, seckey).unwrap();
+    let signature = sign_recovery(msg_digest, seckey).unwrap();
 
     let (recovery_id, serialize_sig) = signature.serialize_compact();
 
-    assert_eq!(recover(&secp, msg_digest, serialize_sig, recovery_id.to_u8()), Ok(pubkey));
+    assert_eq!(recover(msg_digest, serialize_sig, recovery_id.to_u8()), Ok(pubkey));
 }

no_std_test/src/main.rs

@@ -51,26 +51,18 @@ 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::*;
-
-use serde_cbor::de;
 use serde_cbor::ser::SliceWrite;
-use serde_cbor::Serializer;
+use serde_cbor::{de, Serializer};
 
-fn abort() -> ! {
-    unsafe { libc::abort() }
-}
+fn abort() -> ! { unsafe { libc::abort() } }
 
 struct FakeRng;
 impl RngCore for FakeRng {
-    fn next_u32(&mut self) -> u32 {
-        57
-    }
-    fn next_u64(&mut self) -> u64 {
-        57
-    }
+    fn next_u32(&mut self) -> u32 { 57 }
+    fn next_u64(&mut self) -> u64 { 57 }
     fn fill_bytes(&mut self, dest: &mut [u8]) {
         for i in dest {
             *i = 57;
@@ -93,9 +85,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);
@@ -133,12 +125,7 @@ struct Print {
 }
 
 impl Print {
-    pub fn new() -> Self {
-        Self {
-            loc: 0,
-            buf: [0u8; 512],
-        }
-    }
+    pub fn new() -> Self { Self { loc: 0, buf: [0u8; 512] } }
 
     pub fn print(&self) {
         unsafe {

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<AlignedType>; MAX_PREALLOC_SIZE],
+        Option<NonNull<ffi::Context>>,
+    );
+
+    // 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<c_void> {
+            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<ffi::Context> {
+            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<SelfContainedContext> 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<SelfContainedContext> = SpinLock::<SelfContainedContext>::new();
+
+/// Borrows the global context and does 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, Ctx: Context, F: FnOnce(&Secp256k1<Ctx>) -> 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 does 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: FnOnce(NonNull<ffi::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)) }

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<Secp256k1<All>> = RefCell::new(Secp256k1::new());
+}
+
+/// Borrows the global context and does 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, Ctx: Context, F: FnOnce(&Secp256k1<Ctx>) -> 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 does 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: FnOnce(NonNull<ffi::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);
+        }
+    });
+}

src/context.rs renamed to src/context/mod.rs

@@ -10,6 +10,15 @@ use crate::ffi::types::{c_uint, c_void, AlignedType};
 use crate::ffi::{self, CPtr};
 use crate::{Error, Secp256k1};
 
+#[cfg_attr(feature = "std", path = "internal_std.rs")]
+#[cfg_attr(not(feature = "std"), path = "internal_nostd.rs")]
+mod internal;
+
+#[cfg(not(feature = "std"))]
+mod spinlock;
+
+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 {
@@ -369,7 +378,8 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
     /// * 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<ffi::Context>,
     ) -> ManuallyDrop<Secp256k1<AllPreallocated<'buf>>> {