Skip to content

Commit 6ca9f58

Browse files
committed
Merge #806: context: introduce new global context API with rerandomization
19cfe16 recovery: rewrite API to not use context objects (Andrew Poelstra) 4f600db key: update a couple arbitrary API functions to no longer take a context (Andrew Poelstra) 362495b test: remove a ton of rand feature-gating (Andrew Poelstra) 5fa32b3 key: remove std/alloc/global-context gates from serde::deserialize and FromStr (Andrew Poelstra) 0e42950 context: add nostd version of global context (Andrew Poelstra) 979aa1a context: introduce spinlock that gives up after a few iterations (Andrew Poelstra) 9d54872 context: introduce global rerandomizable context (std only) (Andrew Poelstra) 1e45d4c 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](#346 (comment)) but in the [following several messages](#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 19cfe16 Tree-SHA512: 5b0be1472ef7a52221a01c141ac58f080c85f954515c567e2ecba6549f2d970996a0f7ce3c5349c2391b1eee3b504b695efdddf86a5cc70ab411dd5f3a40704b
2 parents b111209 + 19cfe16 commit 6ca9f58

File tree

15 files changed

+793
-337
lines changed

15 files changed

+793
-337
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)', 'cfg(secp256k1_fu
5757

5858
[[example]]
5959
name = "sign_verify_recovery"
60-
required-features = ["recovery", "std"]
60+
required-features = ["recovery"]
6161

6262
[[example]]
6363
name = "sign_verify"

examples/sign_verify_recovery.rs

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,25 @@
11
extern crate secp256k1;
22

3-
use secp256k1::{ecdsa, Error, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification};
3+
use secp256k1::{ecdsa, Error, Message, PublicKey, SecretKey};
44

5-
fn recover<C: Verification>(
6-
secp: &Secp256k1<C>,
7-
msg_digest: [u8; 32],
8-
sig: [u8; 64],
9-
recovery_id: u8,
10-
) -> Result<PublicKey, Error> {
11-
let msg = Message::from_digest(msg_digest);
5+
fn recover(msg_digest: [u8; 32], sig: [u8; 64], recovery_id: u8) -> Result<PublicKey, Error> {
126
let id = ecdsa::RecoveryId::try_from(i32::from(recovery_id))?;
137
let sig = ecdsa::RecoverableSignature::from_compact(&sig, id)?;
8+
let msg = Message::from_digest(msg_digest);
149

15-
secp.recover_ecdsa(msg, &sig)
10+
sig.recover_ecdsa(msg)
1611
}
1712

18-
fn sign_recovery<C: Signing>(
19-
secp: &Secp256k1<C>,
13+
fn sign_recovery(
2014
msg_digest: [u8; 32],
2115
seckey: [u8; 32],
2216
) -> Result<ecdsa::RecoverableSignature, Error> {
2317
let msg = Message::from_digest(msg_digest);
2418
let seckey = SecretKey::from_byte_array(seckey)?;
25-
Ok(secp.sign_ecdsa_recoverable(msg, &seckey))
19+
Ok(ecdsa::RecoverableSignature::sign_ecdsa_recoverable(msg, &seckey))
2620
}
2721

2822
fn main() {
29-
let secp = Secp256k1::new();
30-
3123
let seckey = [
3224
59, 148, 11, 85, 134, 130, 61, 253, 2, 174, 59, 70, 27, 180, 51, 107, 94, 203, 174, 253,
3325
102, 39, 170, 146, 46, 252, 4, 143, 236, 12, 136, 28,
@@ -39,9 +31,9 @@ fn main() {
3931
.unwrap();
4032
let msg_digest = *b"this must be secure hash output.";
4133

42-
let signature = sign_recovery(&secp, msg_digest, seckey).unwrap();
34+
let signature = sign_recovery(msg_digest, seckey).unwrap();
4335

4436
let (recovery_id, serialize_sig) = signature.serialize_compact();
4537

46-
assert_eq!(recover(&secp, msg_digest, serialize_sig, recovery_id.to_u8()), Ok(pubkey));
38+
assert_eq!(recover(msg_digest, serialize_sig, recovery_id.to_u8()), Ok(pubkey));
4739
}

no_std_test/src/main.rs

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,26 +51,18 @@ use core::panic::PanicInfo;
5151

5252
use secp256k1::ecdh::{self, SharedSecret};
5353
use secp256k1::ffi::types::AlignedType;
54-
use secp256k1::rand::{self, RngCore};
54+
use secp256k1::rand::RngCore;
5555
use secp256k1::serde::Serialize;
5656
use secp256k1::*;
57-
58-
use serde_cbor::de;
5957
use serde_cbor::ser::SliceWrite;
60-
use serde_cbor::Serializer;
58+
use serde_cbor::{de, Serializer};
6159

62-
fn abort() -> ! {
63-
unsafe { libc::abort() }
64-
}
60+
fn abort() -> ! { unsafe { libc::abort() } }
6561

6662
struct FakeRng;
6763
impl RngCore for FakeRng {
68-
fn next_u32(&mut self) -> u32 {
69-
57
70-
}
71-
fn next_u64(&mut self) -> u64 {
72-
57
73-
}
64+
fn next_u32(&mut self) -> u32 { 57 }
65+
fn next_u64(&mut self) -> u64 { 57 }
7466
fn fill_bytes(&mut self, dest: &mut [u8]) {
7567
for i in dest {
7668
*i = 57;
@@ -93,9 +85,9 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
9385
let sig = secp.sign_ecdsa(message, &secret_key);
9486
assert!(secp.verify_ecdsa(&sig, message, &public_key).is_ok());
9587

96-
let rec_sig = secp.sign_ecdsa_recoverable(message, &secret_key);
88+
let rec_sig = ecdsa::RecoverableSignature::sign_ecdsa_recoverable(message, &secret_key);
9789
assert!(secp.verify_ecdsa(&rec_sig.to_standard(), message, &public_key).is_ok());
98-
assert_eq!(public_key, secp.recover_ecdsa(message, &rec_sig).unwrap());
90+
assert_eq!(public_key, rec_sig.recover_ecdsa(message).unwrap());
9991
let (rec_id, data) = rec_sig.serialize_compact();
10092
let new_rec_sig = ecdsa::RecoverableSignature::from_compact(&data, rec_id).unwrap();
10193
assert_eq!(rec_sig, new_rec_sig);
@@ -133,12 +125,7 @@ struct Print {
133125
}
134126

135127
impl Print {
136-
pub fn new() -> Self {
137-
Self {
138-
loc: 0,
139-
buf: [0u8; 512],
140-
}
141-
}
128+
pub fn new() -> Self { Self { loc: 0, buf: [0u8; 512] } }
142129

143130
pub fn print(&self) {
144131
unsafe {

src/context/internal_nostd.rs

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
// SPDX-License-Identifier: CC0-1.0
2+
3+
use core::marker::PhantomData;
4+
use core::mem::ManuallyDrop;
5+
use core::ptr::NonNull;
6+
7+
use crate::context::spinlock::SpinLock;
8+
use crate::{ffi, Context, Secp256k1};
9+
10+
mod self_contained_context {
11+
use core::mem::MaybeUninit;
12+
use core::ptr::NonNull;
13+
14+
use crate::ffi::types::{c_void, AlignedType};
15+
use crate::{ffi, AllPreallocated, Context as _};
16+
17+
const MAX_PREALLOC_SIZE: usize = 16; // measured at 208 bytes on Andrew's 64-bit system
18+
19+
/// A secp256k1 context object which can be allocated on the stack or in static storage.
20+
pub struct SelfContainedContext(
21+
[MaybeUninit<AlignedType>; MAX_PREALLOC_SIZE],
22+
Option<NonNull<ffi::Context>>,
23+
);
24+
25+
// SAFETY: the context object owns all its own data.
26+
unsafe impl Send for SelfContainedContext {}
27+
28+
impl SelfContainedContext {
29+
/// Creates a new uninitialized self-contained context.
30+
pub const fn new_uninitialized() -> Self {
31+
Self([MaybeUninit::uninit(); MAX_PREALLOC_SIZE], None)
32+
}
33+
34+
/// Accessor for the underlying raw context pointer
35+
fn buf(&mut self) -> NonNull<c_void> {
36+
NonNull::new(self.0.as_mut_ptr() as *mut c_void).unwrap()
37+
}
38+
39+
pub fn clone_into(&mut self, other: &mut SelfContainedContext) {
40+
// SAFETY: just FFI calls
41+
unsafe {
42+
let other = other.raw_ctx().as_ptr();
43+
assert!(
44+
ffi::secp256k1_context_preallocated_clone_size(other)
45+
<= core::mem::size_of::<[AlignedType; MAX_PREALLOC_SIZE]>(),
46+
"prealloc size exceeds our guessed compile-time upper bound",
47+
);
48+
ffi::secp256k1_context_preallocated_clone(other, self.buf());
49+
}
50+
}
51+
52+
/// Accessor for the context as a raw context pointer.
53+
///
54+
/// On the first call, this will create the context.
55+
pub fn raw_ctx(&mut self) -> NonNull<ffi::Context> {
56+
let buf = self.buf();
57+
*self.1.get_or_insert_with(|| {
58+
// SAFETY: just FFI calls
59+
unsafe {
60+
assert!(
61+
ffi::secp256k1_context_preallocated_size(AllPreallocated::FLAGS)
62+
<= core::mem::size_of::<[AlignedType; MAX_PREALLOC_SIZE]>(),
63+
"prealloc size exceeds our guessed compile-time upper bound",
64+
);
65+
ffi::secp256k1_context_preallocated_create(buf, AllPreallocated::FLAGS)
66+
}
67+
})
68+
}
69+
}
70+
}
71+
// Needs to be pub(super) so that we can define a constructor for
72+
// SpinLock<SelfContainedContext> in the spinlock module. (We cannot do so generically
73+
// because we need a const constructor.)
74+
pub(super) use self_contained_context::SelfContainedContext;
75+
76+
static SECP256K1: SpinLock<SelfContainedContext> = SpinLock::<SelfContainedContext>::new();
77+
78+
/// Borrows the global context and does some operation on it.
79+
///
80+
/// If `randomize_seed` is provided, it is used to rerandomize the context after the
81+
/// operation is complete. If it is not provided, randomization is skipped.
82+
///
83+
/// Only a bit or two per signing operation is needed; if you have any entropy at all,
84+
/// you should provide it, even if you can't provide 32 random bytes.
85+
pub fn with_global_context<T, Ctx: Context, F: FnOnce(&Secp256k1<Ctx>) -> T>(
86+
f: F,
87+
rerandomize_seed: Option<&[u8; 32]>,
88+
) -> T {
89+
with_raw_global_context(
90+
|ctx| {
91+
let secp = ManuallyDrop::new(Secp256k1 { ctx, phantom: PhantomData });
92+
f(&*secp)
93+
},
94+
rerandomize_seed,
95+
)
96+
}
97+
98+
/// Borrows the global context as a raw pointer and does some operation on it.
99+
///
100+
/// If `randomize_seed` is provided, it is used to rerandomize the context after the
101+
/// operation is complete. If it is not provided, randomization is skipped.
102+
///
103+
/// Only a bit or two per signing operation is needed; if you have any entropy at all,
104+
/// you should provide it, even if you can't provide 32 random bytes.
105+
pub fn with_raw_global_context<T, F: FnOnce(NonNull<ffi::Context>) -> T>(
106+
f: F,
107+
rerandomize_seed: Option<&[u8; 32]>,
108+
) -> T {
109+
// Our function may be expensive, so before calling it, we copy the global
110+
// context into this local buffer on the stack. Then we can release it,
111+
// allowing other callers to use it simultaneously.
112+
let mut ctx = SelfContainedContext::new_uninitialized();
113+
let mut have_global_ctx = false;
114+
if let Some(mut guard) = SECP256K1.try_lock() {
115+
let global_ctx = &mut *guard;
116+
ctx.clone_into(global_ctx);
117+
have_global_ctx = true;
118+
// (the lock is now dropped)
119+
}
120+
121+
// Obtain a raw pointer to the context, creating one if it has not been already,
122+
// and call the function.
123+
let ctx_ptr = ctx.raw_ctx();
124+
let ret = f(ctx_ptr);
125+
126+
// ...then rerandomize the local copy, and try to replace the global one
127+
// with this. Note that even if we got the lock above, we may fail to get
128+
// it now; in that case, we don't rerandomize and leave the contexct in
129+
// the state that we found it in.
130+
//
131+
// We do this, rather than holding the lock continuously through the call
132+
// to `f`, to minimize the likelihood of contention. If we fail to randomize,
133+
// that really isn't a big deal since this is a "defense in depth" measure
134+
// whose value is likely to obtain even if it only succeeds a small fraction
135+
// of the time.
136+
//
137+
// Contention, meanwhile, will lead to users using a stack-local copy of
138+
// the context rather than the global one, which aside from being inefficient,
139+
// means that the context they use won't be rerandomized at all. So there
140+
// isn't even any benefit.
141+
if have_global_ctx {
142+
if let Some(seed) = rerandomize_seed {
143+
// SAFETY: just a FFI call
144+
unsafe {
145+
assert_eq!(ffi::secp256k1_context_randomize(ctx_ptr, seed.as_ptr()), 1);
146+
}
147+
if let Some(ref mut guard) = SECP256K1.try_lock() {
148+
guard.clone_into(&mut ctx);
149+
}
150+
}
151+
}
152+
ret
153+
}
154+
155+
/// Rerandomize the global context, using the given data as a seed.
156+
///
157+
/// The provided data will be mixed with the entropy from previous calls in a timing
158+
/// analysis resistant way. It is safe to directly pass secret data to this function.
159+
pub fn rerandomize_global_context(seed: &[u8; 32]) { with_raw_global_context(|_| {}, Some(seed)) }

src/context/internal_std.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// SPDX-License-Identifier: CC0-1.0
2+
3+
use std::cell::RefCell;
4+
use std::marker::PhantomData;
5+
use std::mem::ManuallyDrop;
6+
use std::ptr::NonNull;
7+
8+
use secp256k1_sys as ffi;
9+
10+
use crate::{All, Context, Secp256k1};
11+
12+
thread_local! {
13+
static SECP256K1: RefCell<Secp256k1<All>> = RefCell::new(Secp256k1::new());
14+
}
15+
16+
/// Borrows the global context and does some operation on it.
17+
///
18+
/// If provided, after the operation is complete, [`rerandomize_global_context`]
19+
/// is called on the context. If you have some random data available,
20+
pub fn with_global_context<T, Ctx: Context, F: FnOnce(&Secp256k1<Ctx>) -> T>(
21+
f: F,
22+
rerandomize_seed: Option<&[u8; 32]>,
23+
) -> T {
24+
with_raw_global_context(
25+
|ctx| {
26+
let secp = ManuallyDrop::new(Secp256k1 { ctx, phantom: PhantomData });
27+
f(&*secp)
28+
},
29+
rerandomize_seed,
30+
)
31+
}
32+
33+
/// Borrows the global context as a raw pointer and does some operation on it.
34+
///
35+
/// If provided, after the operation is complete, [`rerandomize_global_context`]
36+
/// is called on the context. If you have some random data available,
37+
pub fn with_raw_global_context<T, F: FnOnce(NonNull<ffi::Context>) -> T>(
38+
f: F,
39+
rerandomize_seed: Option<&[u8; 32]>,
40+
) -> T {
41+
SECP256K1.with(|secp| {
42+
let borrow = secp.borrow();
43+
let ret = f(borrow.ctx);
44+
drop(borrow);
45+
46+
if let Some(seed) = rerandomize_seed {
47+
rerandomize_global_context(seed);
48+
}
49+
ret
50+
})
51+
}
52+
53+
/// Rerandomize the global context, using the given data as a seed.
54+
///
55+
/// The provided data will be mixed with the entropy from previous calls in a timing
56+
/// analysis resistant way. It is safe to directly pass secret data to this function.
57+
pub fn rerandomize_global_context(seed: &[u8; 32]) {
58+
SECP256K1.with(|secp| {
59+
let mut borrow = secp.borrow_mut();
60+
61+
// If we have access to the thread rng then use it as well.
62+
#[cfg(feature = "rand")]
63+
{
64+
let mut new_seed: [u8; 32] = rand::random();
65+
for (new, byte) in new_seed.iter_mut().zip(seed.iter()) {
66+
*new ^= *byte;
67+
}
68+
borrow.seeded_randomize(&new_seed);
69+
}
70+
#[cfg(not(feature = "rand"))]
71+
{
72+
borrow.seeded_randomize(seed);
73+
}
74+
});
75+
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ use crate::ffi::types::{c_uint, c_void, AlignedType};
1010
use crate::ffi::{self, CPtr};
1111
use crate::{Error, Secp256k1};
1212

13+
#[cfg_attr(feature = "std", path = "internal_std.rs")]
14+
#[cfg_attr(not(feature = "std"), path = "internal_nostd.rs")]
15+
mod internal;
16+
17+
#[cfg(not(feature = "std"))]
18+
mod spinlock;
19+
20+
pub use internal::{rerandomize_global_context, with_global_context, with_raw_global_context};
21+
1322
#[cfg(all(feature = "global-context", feature = "std"))]
1423
/// Module implementing a singleton pattern for a global `Secp256k1` context.
1524
pub mod global {
@@ -369,7 +378,8 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
369378
/// * The version of `libsecp256k1` used to create `raw_ctx` must be **exactly the one linked
370379
/// into this library**.
371380
/// * The lifetime of the `raw_ctx` pointer must outlive `'buf`.
372-
/// * `raw_ctx` must point to writable memory (cannot be `ffi::secp256k1_context_no_precomp`).
381+
/// * `raw_ctx` must point to writable memory (cannot be `ffi::secp256k1_context_no_precomp`),
382+
/// **or** the user must never attempt to rerandomize the context.
373383
pub unsafe fn from_raw_all(
374384
raw_ctx: NonNull<ffi::Context>,
375385
) -> ManuallyDrop<Secp256k1<AllPreallocated<'buf>>> {

0 commit comments

Comments
 (0)