From f2e4b297e12b7b9b0f8f487e60040fda326cc3ad Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Feb 2021 09:50:58 -0500 Subject: [PATCH 1/4] Do not test secret->public derivation or pk validity in fuzzing cfg In the next commit the secret->public key derivation in fuzzing cfg is changed to be simpler, as well as the validity rules of public keys relaxed. This adds a new test to ensure random keys can be added, not just the hard-coded keys test that exists today. --- src/key.rs | 31 +++++++++++++++++++++++++++++-- src/schnorrsig.rs | 12 ++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/key.rs b/src/key.rs index 4b8ef45..a919eff 100644 --- a/src/key.rs +++ b/src/key.rs @@ -690,7 +690,13 @@ mod test { let s = Secp256k1::signing_only(); let sk = SecretKey::from_slice(&SK_BYTES).expect("sk"); + + // In fuzzing mode secret->public key derivation is different, so + // hard-code the epected result. + #[cfg(not(fuzzing))] let pk = PublicKey::from_secret_key(&s, &sk); + #[cfg(fuzzing)] + let pk = PublicKey::from_slice(&[0x02, 0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, 0x1c, 0x97, 0x09, 0xe2, 0x30, 0x92, 0x06, 0x7d, 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66]).expect("pk"); assert_eq!( sk.to_string(), @@ -733,6 +739,9 @@ mod test { } #[test] + // In fuzzing mode the Y coordinate is expected to match the X, so this + // test uses invalid public keys. + #[cfg(not(fuzzing))] fn test_pubkey_serialize() { struct DumbRng(u32); impl RngCore for DumbRng { @@ -841,7 +850,7 @@ mod test { assert_eq!(set.len(), COUNT); } - #[test] + #[cfg_attr(not(fuzzing), test)] fn pubkey_combine() { let compressed1 = PublicKey::from_slice( &hex!("0241cc121c419921942add6db6482fb36243faf83317c866d2a28d8c6d7089f7ba"), @@ -861,7 +870,7 @@ mod test { assert_eq!(sum1.unwrap(), exp_sum); } - #[test] + #[cfg_attr(not(fuzzing), test)] fn pubkey_combine_keys() { let compressed1 = PublicKey::from_slice( &hex!("0241cc121c419921942add6db6482fb36243faf83317c866d2a28d8c6d7089f7ba"), @@ -884,6 +893,24 @@ mod test { assert_eq!(sum1.unwrap(), exp_sum); } + #[test] + fn create_pubkey_combine() { + let s = Secp256k1::new(); + + let (mut sk1, pk1) = s.generate_keypair(&mut thread_rng()); + let (sk2, pk2) = s.generate_keypair(&mut thread_rng()); + + let sum1 = pk1.combine(&pk2); + assert!(sum1.is_ok()); + let sum2 = pk2.combine(&pk1); + assert!(sum2.is_ok()); + assert_eq!(sum1, sum2); + + assert!(sk1.add_assign(&sk2.as_ref()[..]).is_ok()); + let sksum = PublicKey::from_secret_key(&s, &sk1); + assert_eq!(Ok(sksum), sum1); + } + #[test] fn pubkey_equal() { let pk1 = PublicKey::from_slice( diff --git a/src/schnorrsig.rs b/src/schnorrsig.rs index 9088bf3..ab3b6f4 100644 --- a/src/schnorrsig.rs +++ b/src/schnorrsig.rs @@ -706,6 +706,9 @@ mod tests { PublicKey::from_slice(&[0xff; constants::SCHNORRSIG_PUBLIC_KEY_SIZE]), Err(InvalidPublicKey) ); + // In fuzzing mode restrictions on public key validity are much more + // relaxed, thus the invalid check below is expected to fail. + #[cfg(not(fuzzing))] assert_eq!( PublicKey::from_slice(&[0x55; constants::SCHNORRSIG_PUBLIC_KEY_SIZE]), Err(InvalidPublicKey) @@ -724,7 +727,13 @@ mod tests { let s = Secp256k1::signing_only(); let sk = KeyPair::from_seckey_slice(&secp, &SK_BYTES).expect("sk"); + + // In fuzzing mode secret->public key derivation is different, so + // hard-code the epected result. + #[cfg(not(fuzzing))] let pk = PublicKey::from_keypair(&s, &sk); + #[cfg(fuzzing)] + let pk = PublicKey::from_slice(&[0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, 0x1c, 0x97, 0x09, 0xe2, 0x30, 0x92, 0x06, 0x7d, 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66]).expect("pk"); assert_eq!( pk.to_string(), @@ -762,6 +771,9 @@ mod tests { } #[test] + // In fuzzing mode secret->public key derivation is different, so + // this test will never correctly derive the static pubkey. + #[cfg(not(fuzzing))] fn test_pubkey_serialize() { struct DumbRng(u32); impl RngCore for DumbRng { From 940a51c2c607ed683f4b759db5ee9126b92f260a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Feb 2021 09:55:46 -0500 Subject: [PATCH 2/4] Reduce cryptography usage in --cfg=fuzzing This reduces the usage of real cryptography in --cfg=fuzzing, specifically replacing the secret->public key derivation with a simple copy and ECDH with XOR of the public and private parts (plus a stream of 1s to make a test pass that expected non-0 output). It leaves secret tweak addition/multiplication as-is. It also changes the context creation to over-allocate and store the context flags at the end of the context buffer, allowing us to easily test context flags in each function. While it would be nice to have something fancier (eg XOR-based), its not immediately obvious how to accomplish this, and better to fix the issues I have than spend too much time on it. Fixes #271. This partially reverts b811ec133a0628f9db0e452abf84b8e302737f58 --- secp256k1-sys/src/lib.rs | 476 +++++++++++++++++++++++++++++++++------ src/key.rs | 7 +- 2 files changed, 412 insertions(+), 71 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index d8ec281..fb15622 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -275,38 +275,13 @@ extern "C" { pub static secp256k1_context_no_precomp: *const Context; // Contexts - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_size")] - pub fn secp256k1_context_preallocated_size(flags: c_uint) -> size_t; - - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_create")] - pub fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context; - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_destroy")] pub fn secp256k1_context_preallocated_destroy(cx: *mut Context); - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_clone_size")] - pub fn secp256k1_context_preallocated_clone_size(cx: *const Context) -> size_t; - - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_clone")] - pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_randomize")] pub fn secp256k1_context_randomize(cx: *mut Context, seed32: *const c_uchar) -> c_int; - - // Pubkeys - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_pubkey_parse")] - pub fn secp256k1_ec_pubkey_parse(cx: *const Context, pk: *mut PublicKey, - input: *const c_uchar, in_len: size_t) - -> c_int; - - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_pubkey_serialize")] - pub fn secp256k1_ec_pubkey_serialize(cx: *const Context, output: *mut c_uchar, - out_len: *mut size_t, pk: *const PublicKey, - compressed: c_uint) - -> c_int; - // Signatures #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ecdsa_signature_parse_der")] pub fn secp256k1_ecdsa_signature_parse_der(cx: *const Context, sig: *mut Signature, @@ -338,6 +313,7 @@ extern "C" { in_sig: *const Signature) -> c_int; + // Secret Keys #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_seckey_verify")] pub fn secp256k1_ec_seckey_verify(cx: *const Context, sk: *const c_uchar) -> c_int; @@ -376,6 +352,34 @@ extern "C" { sk: *mut c_uchar, tweak: *const c_uchar) -> c_int; +} + +#[cfg(not(fuzzing))] +extern "C" { + // Contexts + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_size")] + pub fn secp256k1_context_preallocated_size(flags: c_uint) -> size_t; + + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_create")] + pub fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context; + + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_clone_size")] + pub fn secp256k1_context_preallocated_clone_size(cx: *const Context) -> size_t; + + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_clone")] + pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; + + // Pubkeys + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_pubkey_parse")] + pub fn secp256k1_ec_pubkey_parse(cx: *const Context, pk: *mut PublicKey, + input: *const c_uchar, in_len: size_t) + -> c_int; + + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_pubkey_serialize")] + pub fn secp256k1_ec_pubkey_serialize(cx: *const Context, output: *mut c_uchar, + out_len: *mut size_t, pk: *const PublicKey, + compressed: c_uint) + -> c_int; // EC #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_pubkey_create")] @@ -417,6 +421,42 @@ extern "C" { data: *mut c_void, ) -> c_int; + // ECDSA + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ecdsa_verify")] + pub fn secp256k1_ecdsa_verify(cx: *const Context, + sig: *const Signature, + msg32: *const c_uchar, + pk: *const PublicKey) + -> c_int; + + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ecdsa_sign")] + pub fn secp256k1_ecdsa_sign(cx: *const Context, + sig: *mut Signature, + msg32: *const c_uchar, + sk: *const c_uchar, + noncefn: NonceFn, + noncedata: *const c_void) + -> c_int; + + // Schnorr Signatures + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_schnorrsig_sign")] + pub fn secp256k1_schnorrsig_sign( + cx: *const Context, + sig: *mut c_uchar, + msg32: *const c_uchar, + keypair: *const KeyPair, + noncefp: SchnorrNonceFn, + noncedata: *const c_void + ) -> c_int; + + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_schnorrsig_verify")] + pub fn secp256k1_schnorrsig_verify( + cx: *const Context, + sig64: *const c_uchar, + msg32: *const c_uchar, + pubkey: *const XOnlyPublicKey, + ) -> c_int; + // Extra keys #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_keypair_create")] pub fn secp256k1_keypair_create( @@ -480,46 +520,6 @@ extern "C" { ) -> c_int; } -#[cfg(not(fuzzing))] -extern "C" { - // ECDSA - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ecdsa_verify")] - pub fn secp256k1_ecdsa_verify(cx: *const Context, - sig: *const Signature, - msg32: *const c_uchar, - pk: *const PublicKey) - -> c_int; - - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ecdsa_sign")] - pub fn secp256k1_ecdsa_sign(cx: *const Context, - sig: *mut Signature, - msg32: *const c_uchar, - sk: *const c_uchar, - noncefn: NonceFn, - noncedata: *const c_void) - -> c_int; - - // Schnorr Signatures - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_schnorrsig_sign")] - pub fn secp256k1_schnorrsig_sign( - cx: *const Context, - sig: *mut c_uchar, - msg32: *const c_uchar, - keypair: *const KeyPair, - noncefp: SchnorrNonceFn, - noncedata: *const c_void - ) -> c_int; - - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_schnorrsig_verify")] - pub fn secp256k1_schnorrsig_verify( - cx: *const Context, - sig64: *const c_uchar, - msg32: *const c_uchar, - pubkey: *const XOnlyPublicKey, - ) -> c_int; -} - - /// A reimplementation of the C function `secp256k1_context_create` in rust. /// /// This function allocates memory, the pointer should be deallocated using `secp256k1_context_destroy` @@ -670,6 +670,223 @@ impl CPtr for [T] { mod fuzz_dummy { use super::*; + #[cfg(rust_secp_no_symbol_renaming)] compile_error!("We do not support fuzzing with rust_secp_no_symbol_renaming"); + + extern "C" { + fn rustsecp256k1_v0_4_0_context_preallocated_size(flags: c_uint) -> size_t; + fn rustsecp256k1_v0_4_0_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context; + fn rustsecp256k1_v0_4_0_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; + } + + const CTX_SIZE: usize = 1024 * 1024 * 2; + // Contexts + pub unsafe fn secp256k1_context_preallocated_size(flags: c_uint) -> size_t { + assert!(rustsecp256k1_v0_4_0_context_preallocated_size(flags) + std::mem::size_of::() <= CTX_SIZE); + CTX_SIZE + } + pub unsafe fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context { + let ptr = (prealloc as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::()); + (ptr as *mut c_uint).write(flags); + rustsecp256k1_v0_4_0_context_preallocated_create(prealloc, flags) + } + pub unsafe fn secp256k1_context_preallocated_clone_size(_cx: *const Context) -> size_t { CTX_SIZE } + pub unsafe fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context { + let orig_ptr = (cx as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::()); + let new_ptr = (prealloc as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::()); + let flags = (orig_ptr as *mut c_uint).read(); + (new_ptr as *mut c_uint).write(flags); + rustsecp256k1_v0_4_0_context_preallocated_clone(cx, prealloc) + } + + unsafe fn check_context_flags(cx: *const Context, required_flags: c_uint) { + assert!(!cx.is_null()); + let cx_flags = if cx == secp256k1_context_no_precomp { + 1 + } else { + let ptr = (cx as *const u8).add(CTX_SIZE).sub(std::mem::size_of::()); + (ptr as *const c_uint).read() + }; + assert_eq!(cx_flags & 1, 1); // SECP256K1_FLAGS_TYPE_CONTEXT + assert_eq!(cx_flags & required_flags, required_flags); + } + + /// Checks that pk != 0xffff...ffff and pk[1..32] == pk[33..64] + unsafe fn test_pk_validate(cx: *const Context, + pk: *const PublicKey) -> c_int { + check_context_flags(cx, 0); + if (*pk).0[1..32] != (*pk).0[33..64] || + ((*pk).0[32] != 0 && (*pk).0[32] != 0xff) || + secp256k1_ec_seckey_verify(cx, (*pk).0[0..32].as_ptr()) == 0 { + 0 + } else { + 1 + } + } + unsafe fn test_cleanup_pk(pk: *mut PublicKey) { + (*pk).0[32..].copy_from_slice(&(*pk).0[..32]); + if (*pk).0[32] <= 0x7f { + (*pk).0[32] = 0; + } else { + (*pk).0[32] = 0xff; + } + } + + // Pubkeys + pub unsafe fn secp256k1_ec_pubkey_parse(cx: *const Context, pk: *mut PublicKey, + input: *const c_uchar, in_len: size_t) + -> c_int { + check_context_flags(cx, 0); + match in_len { + 33 => { + if *input != 2 && *input != 3 { + 0 + } else { + ptr::copy(input.offset(1), (*pk).0[0..32].as_mut_ptr(), 32); + ptr::copy(input.offset(2), (*pk).0[33..64].as_mut_ptr(), 31); + if *input == 3 { + (*pk).0[32] = 0xff; + } else { + (*pk).0[32] = 0; + } + test_pk_validate(cx, pk) + } + }, + 65 => { + if *input != 4 && *input != 6 && *input != 7 { + 0 + } else { + ptr::copy(input.offset(1), (*pk).0.as_mut_ptr(), 64); + test_cleanup_pk(pk); + test_pk_validate(cx, pk) + } + }, + _ => 0 + } + } + + /// Serialize PublicKey back to 33/65 byte pubkey + pub unsafe fn secp256k1_ec_pubkey_serialize(cx: *const Context, output: *mut c_uchar, + out_len: *mut size_t, pk: *const PublicKey, + compressed: c_uint) + -> c_int { + check_context_flags(cx, 0); + assert_eq!(test_pk_validate(cx, pk), 1); + if compressed == SECP256K1_SER_COMPRESSED { + assert_eq!(*out_len, 33); + if (*pk).0[32] <= 0x7f { + *output = 2; + } else { + *output = 3; + } + ptr::copy((*pk).0.as_ptr(), output.offset(1), 32); + } else if compressed == SECP256K1_SER_UNCOMPRESSED { + assert_eq!(*out_len, 65); + *output = 4; + ptr::copy((*pk).0.as_ptr(), output.offset(1), 64); + } else { + panic!("Bad flags"); + } + 1 + } + + // EC + /// Sets pk to sk||sk + pub unsafe fn secp256k1_ec_pubkey_create(cx: *const Context, pk: *mut PublicKey, + sk: *const c_uchar) -> c_int { + check_context_flags(cx, SECP256K1_START_SIGN); + if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } + ptr::copy(sk, (*pk).0[0..32].as_mut_ptr(), 32); + test_cleanup_pk(pk); + assert_eq!(test_pk_validate(cx, pk), 1); + 1 + } + + pub unsafe fn secp256k1_ec_pubkey_negate(cx: *const Context, + pk: *mut PublicKey) -> c_int { + check_context_flags(cx, 0); + assert_eq!(test_pk_validate(cx, pk), 1); + if secp256k1_ec_seckey_negate(cx, (*pk).0[..32].as_mut_ptr()) != 1 { return 0; } + test_cleanup_pk(pk); + assert_eq!(test_pk_validate(cx, pk), 1); + 1 + } + + /// The PublicKey equivalent of secp256k1_ec_privkey_tweak_add + pub unsafe fn secp256k1_ec_pubkey_tweak_add(cx: *const Context, + pk: *mut PublicKey, + tweak: *const c_uchar) + -> c_int { + check_context_flags(cx, SECP256K1_START_VERIFY); + assert_eq!(test_pk_validate(cx, pk), 1); + if secp256k1_ec_seckey_tweak_add(cx, (*pk).0[..32].as_mut_ptr(), tweak) != 1 { return 0; } + test_cleanup_pk(pk); + assert_eq!(test_pk_validate(cx, pk), 1); + 1 + } + + /// The PublicKey equivalent of secp256k1_ec_privkey_tweak_mul + pub unsafe fn secp256k1_ec_pubkey_tweak_mul(cx: *const Context, + pk: *mut PublicKey, + tweak: *const c_uchar) + -> c_int { + check_context_flags(cx, 0); + assert_eq!(test_pk_validate(cx, pk), 1); + if secp256k1_ec_seckey_tweak_mul(cx, (*pk).0[..32].as_mut_ptr(), tweak) != 1 { return 0; } + test_cleanup_pk(pk); + assert_eq!(test_pk_validate(cx, pk), 1); + 1 + } + + pub unsafe fn secp256k1_ec_pubkey_combine(cx: *const Context, + out: *mut PublicKey, + ins: *const *const PublicKey, + n: c_int) + -> c_int { + check_context_flags(cx, 0); + assert!(n >= 1); + (*out) = **ins; + for i in 1..n { + assert_eq!(test_pk_validate(cx, *ins.offset(i as isize)), 1); + if secp256k1_ec_seckey_tweak_add(cx, (*out).0[..32].as_mut_ptr(), (**ins.offset(i as isize)).0[..32].as_ptr()) != 1 { + return 0; + } + } + test_cleanup_pk(out); + assert_eq!(test_pk_validate(cx, out), 1); + 1 + } + + /// Sets out to point^scalar^1s + pub unsafe fn secp256k1_ecdh( + cx: *const Context, + out: *mut c_uchar, + point: *const PublicKey, + scalar: *const c_uchar, + hashfp: EcdhHashFn, + data: *mut c_void, + ) -> c_int { + check_context_flags(cx, 0); + assert_eq!(test_pk_validate(cx, point), 1); + if secp256k1_ec_seckey_verify(cx, scalar) != 1 { return 0; } + + let scalar_slice = slice::from_raw_parts(scalar, 32); + let pk_slice = &(*point).0[..32]; + + let mut res_arr = [0; 32]; + for i in 0..32 { + res_arr[i] = scalar_slice[i] ^ pk_slice[i] ^ 1; + } + + if let Some(hashfn) = hashfp { + (hashfn)(out, res_arr.as_ptr(), res_arr.as_ptr(), data); + } else { + res_arr[16] = 0x00; // result should always be a valid secret key + let out_slice = slice::from_raw_parts_mut(out, 32); + out_slice.copy_from_slice(&res_arr); + } + 1 + } + // ECDSA /// Verifies that sig is msg32||pk[..32] pub unsafe fn secp256k1_ecdsa_verify(cx: *const Context, @@ -677,9 +894,7 @@ mod fuzz_dummy { msg32: *const c_uchar, pk: *const PublicKey) -> c_int { - // Check context is built for verification - let mut new_pk = (*pk).clone(); - let _ = secp256k1_ec_pubkey_tweak_add(cx, &mut new_pk, msg32); + check_context_flags(cx, SECP256K1_START_VERIFY); // Actually verify let sig_sl = slice::from_raw_parts(sig as *const u8, 64); let msg_sl = slice::from_raw_parts(msg32 as *const u8, 32); @@ -698,6 +913,7 @@ mod fuzz_dummy { _noncefn: NonceFn, _noncedata: *const c_void) -> c_int { + check_context_flags(cx, SECP256K1_START_SIGN); // Check context is built for signing (and compute pk) let mut new_pk = PublicKey::new(); if secp256k1_ec_pubkey_create(cx, &mut new_pk, sk) != 1 { @@ -711,6 +927,7 @@ mod fuzz_dummy { 1 } + // Schnorr Signatures /// Verifies that sig is msg32||pk[32..] pub unsafe fn secp256k1_schnorrsig_verify( cx: *const Context, @@ -718,6 +935,7 @@ mod fuzz_dummy { msg32: *const c_uchar, pubkey: *const XOnlyPublicKey, ) -> c_int { + check_context_flags(cx, SECP256K1_START_VERIFY); // Check context is built for verification let mut new_pk = PublicKey::new(); let _ = secp256k1_xonly_pubkey_tweak_add(cx, &mut new_pk, pubkey, msg32); @@ -737,9 +955,10 @@ mod fuzz_dummy { sig64: *mut c_uchar, msg32: *const c_uchar, keypair: *const KeyPair, - noncefp: SchnorrNonceFn, - noncedata: *const c_void + _noncefp: SchnorrNonceFn, + _noncedata: *const c_void ) -> c_int { + check_context_flags(cx, SECP256K1_START_SIGN); // Check context is built for signing let mut new_kp = KeyPair::new(); if secp256k1_keypair_create(cx, &mut new_kp, (*keypair).0.as_ptr()) != 1 { @@ -753,6 +972,123 @@ mod fuzz_dummy { sig_sl[32..].copy_from_slice(&new_kp.0[32..64]); 1 } + + // Extra keys + pub unsafe fn secp256k1_keypair_create( + cx: *const Context, + keypair: *mut KeyPair, + seckey: *const c_uchar, + ) -> c_int { + check_context_flags(cx, SECP256K1_START_SIGN); + if secp256k1_ec_seckey_verify(cx, seckey) == 0 { return 0; } + + let mut pk = PublicKey::new(); + if secp256k1_ec_pubkey_create(cx, &mut pk, seckey) == 0 { return 0; } + + let seckey_slice = slice::from_raw_parts(seckey, 32); + (*keypair).0[..32].copy_from_slice(seckey_slice); + (*keypair).0[32..].copy_from_slice(&pk.0); + 1 + } + + pub unsafe fn secp256k1_xonly_pubkey_parse( + cx: *const Context, + pubkey: *mut XOnlyPublicKey, + input32: *const c_uchar, + ) -> c_int { + check_context_flags(cx, 0); + let inslice = slice::from_raw_parts(input32, 32); + (*pubkey).0[..32].copy_from_slice(inslice); + (*pubkey).0[32..].copy_from_slice(inslice); + test_cleanup_pk(pubkey as *mut PublicKey); + test_pk_validate(cx, pubkey as *mut PublicKey) + } + + pub unsafe fn secp256k1_xonly_pubkey_serialize( + cx: *const Context, + output32: *mut c_uchar, + pubkey: *const XOnlyPublicKey, + ) -> c_int { + check_context_flags(cx, 0); + let outslice = slice::from_raw_parts_mut(output32, 32); + outslice.copy_from_slice(&(*pubkey).0[..32]); + 1 + } + + pub unsafe fn secp256k1_xonly_pubkey_from_pubkey( + cx: *const Context, + xonly_pubkey: *mut XOnlyPublicKey, + pk_parity: *mut c_int, + pubkey: *const PublicKey, + ) -> c_int { + check_context_flags(cx, 0); + if !pk_parity.is_null() { + *pk_parity = ((*pubkey).0[32] == 0).into(); + } + (*xonly_pubkey).0.copy_from_slice(&(*pubkey).0); + assert_eq!(test_pk_validate(cx, pubkey), 1); + 1 + } + + pub unsafe fn secp256k1_xonly_pubkey_tweak_add( + cx: *const Context, + output_pubkey: *mut PublicKey, + internal_pubkey: *const XOnlyPublicKey, + tweak32: *const c_uchar, + ) -> c_int { + check_context_flags(cx, SECP256K1_START_VERIFY); + (*output_pubkey).0.copy_from_slice(&(*internal_pubkey).0); + secp256k1_ec_pubkey_tweak_add(cx, output_pubkey, tweak32) + } + + pub unsafe fn secp256k1_keypair_xonly_pub( + cx: *const Context, + pubkey: *mut XOnlyPublicKey, + pk_parity: *mut c_int, + keypair: *const KeyPair + ) -> c_int { + check_context_flags(cx, 0); + if !pk_parity.is_null() { + *pk_parity = ((*keypair).0[32] == 0).into(); + } + (*pubkey).0.copy_from_slice(&(*keypair).0[32..]); + 1 + } + + pub unsafe fn secp256k1_keypair_xonly_tweak_add( + cx: *const Context, + keypair: *mut KeyPair, + tweak32: *const c_uchar, + ) -> c_int { + check_context_flags(cx, SECP256K1_START_VERIFY); + let mut pk = PublicKey::new(); + pk.0.copy_from_slice(&(*keypair).0[32..]); + let mut sk = [0; 32]; + sk.copy_from_slice(&(*keypair).0[..32]); + assert_eq!(secp256k1_ec_pubkey_tweak_add(cx, &mut pk, tweak32), 1); + assert_eq!(secp256k1_ec_seckey_tweak_add(cx, (&mut sk[..]).as_mut_ptr(), tweak32), 1); + (*keypair).0[..32].copy_from_slice(&sk); + (*keypair).0[32..].copy_from_slice(&pk.0); + 1 + } + + pub unsafe fn secp256k1_xonly_pubkey_tweak_add_check( + cx: *const Context, + tweaked_pubkey32: *const c_uchar, + tweaked_pubkey_parity: c_int, + internal_pubkey: *const XOnlyPublicKey, + tweak32: *const c_uchar, + ) -> c_int { + check_context_flags(cx, SECP256K1_START_VERIFY); + let mut tweaked_pk = PublicKey::new(); + assert_eq!(secp256k1_xonly_pubkey_tweak_add(cx, &mut tweaked_pk, internal_pubkey, tweak32), 1); + let in_slice = slice::from_raw_parts(tweaked_pubkey32, 32); + if &tweaked_pk.0[..32] == in_slice && tweaked_pubkey_parity == (tweaked_pk.0[32] == 0).into() { + 1 + } else { + 0 + } + } } #[cfg(fuzzing)] diff --git a/src/key.rs b/src/key.rs index a919eff..0491b8e 100644 --- a/src/key.rs +++ b/src/key.rs @@ -958,9 +958,14 @@ mod test { "; let s = Secp256k1::new(); - let sk = SecretKey::from_slice(&SK_BYTES).unwrap(); + + // In fuzzing mode secret->public key derivation is different, so + // hard-code the epected result. + #[cfg(not(fuzzing))] let pk = PublicKey::from_secret_key(&s, &sk); + #[cfg(fuzzing)] + let pk = PublicKey::from_slice(&PK_BYTES).expect("pk"); assert_tokens(&sk.compact(), &[Token::BorrowedBytes(&SK_BYTES[..])]); assert_tokens(&sk.compact(), &[Token::Bytes(&SK_BYTES)]); From c486ca10c71f861136400037741e1b336c1fb513 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 26 Feb 2021 23:20:16 -0500 Subject: [PATCH 3/4] Use a global static context in fuzzing, reducing overhead --- secp256k1-sys/src/lib.rs | 44 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index fb15622..1bda95d 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -669,6 +669,7 @@ impl CPtr for [T] { #[cfg(fuzzing)] mod fuzz_dummy { use super::*; + use core::sync::atomic::{AtomicUsize, Ordering}; #[cfg(rust_secp_no_symbol_renaming)] compile_error!("We do not support fuzzing with rust_secp_no_symbol_renaming"); @@ -678,16 +679,55 @@ mod fuzz_dummy { fn rustsecp256k1_v0_4_0_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; } - const CTX_SIZE: usize = 1024 * 1024 * 2; + #[cfg(feature = "lowmemory")] + const CTX_SIZE: usize = 1024 * 65; + #[cfg(not(feature = "lowmemory"))] + const CTX_SIZE: usize = 1024 * (1024 + 128); // Contexts pub unsafe fn secp256k1_context_preallocated_size(flags: c_uint) -> size_t { assert!(rustsecp256k1_v0_4_0_context_preallocated_size(flags) + std::mem::size_of::() <= CTX_SIZE); CTX_SIZE } + + static HAVE_PREALLOCATED_CONTEXT: AtomicUsize = AtomicUsize::new(0); + const HAVE_CONTEXT_NONE: usize = 0; + const HAVE_CONTEXT_WORKING: usize = 1; + const HAVE_CONTEXT_DONE: usize = 2; + static mut PREALLOCATED_CONTEXT: [u8; CTX_SIZE] = [0; CTX_SIZE]; pub unsafe fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context { + // While applications should generally avoid creating too many contexts, sometimes fuzzers + // perform tasks repeatedly which real applications may only do rarely. Thus, we want to + // avoid being overly slow here. We do so by having a static context and copying it into + // new buffers instead of recalculating it. Because we shouldn't rely on std, we use a + // simple hand-written OnceFlag built out of an atomic to gate the global static. + let mut have_ctx = HAVE_PREALLOCATED_CONTEXT.load(Ordering::Relaxed); + while have_ctx != HAVE_CONTEXT_DONE { + if have_ctx == HAVE_CONTEXT_NONE { + have_ctx = HAVE_PREALLOCATED_CONTEXT.swap(HAVE_CONTEXT_WORKING, Ordering::AcqRel); + if have_ctx == HAVE_CONTEXT_NONE { + assert!(rustsecp256k1_v0_4_0_context_preallocated_size(SECP256K1_START_SIGN | SECP256K1_START_VERIFY) + std::mem::size_of::() <= CTX_SIZE); + assert_eq!(rustsecp256k1_v0_4_0_context_preallocated_create( + PREALLOCATED_CONTEXT[..].as_ptr() as *mut c_void, + SECP256K1_START_SIGN | SECP256K1_START_VERIFY), + PREALLOCATED_CONTEXT[..].as_ptr() as *mut Context); + assert_eq!(HAVE_PREALLOCATED_CONTEXT.swap(HAVE_CONTEXT_DONE, Ordering::AcqRel), + HAVE_CONTEXT_WORKING); + } else if have_ctx == HAVE_CONTEXT_DONE { + // Another thread finished while we were swapping. + HAVE_PREALLOCATED_CONTEXT.store(HAVE_CONTEXT_DONE, Ordering::Release); + } + } else { + // Another thread is building, just busy-loop until they're done. + assert_eq!(have_ctx, HAVE_CONTEXT_WORKING); + have_ctx = HAVE_PREALLOCATED_CONTEXT.load(Ordering::Acquire); + #[cfg(feature = "std")] + std::thread::yield_now(); + } + } + ptr::copy_nonoverlapping(PREALLOCATED_CONTEXT[..].as_ptr(), prealloc as *mut u8, CTX_SIZE); let ptr = (prealloc as *mut u8).add(CTX_SIZE).sub(std::mem::size_of::()); (ptr as *mut c_uint).write(flags); - rustsecp256k1_v0_4_0_context_preallocated_create(prealloc, flags) + prealloc as *mut Context } pub unsafe fn secp256k1_context_preallocated_clone_size(_cx: *const Context) -> size_t { CTX_SIZE } pub unsafe fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context { From 79119e8123f35852bb42edc1a4dfd81be029def0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 26 Feb 2021 23:20:48 -0500 Subject: [PATCH 4/4] Skip context randomization in fuzzing to improve performance --- secp256k1-sys/src/lib.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 1bda95d..55aeac2 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -278,10 +278,6 @@ extern "C" { #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_destroy")] pub fn secp256k1_context_preallocated_destroy(cx: *mut Context); - #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_randomize")] - pub fn secp256k1_context_randomize(cx: *mut Context, - seed32: *const c_uchar) - -> c_int; // Signatures #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ecdsa_signature_parse_der")] pub fn secp256k1_ecdsa_signature_parse_der(cx: *const Context, sig: *mut Signature, @@ -369,6 +365,10 @@ extern "C" { #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_preallocated_clone")] pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; + #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_context_randomize")] + pub fn secp256k1_context_randomize(cx: *mut Context, + seed32: *const c_uchar) + -> c_int; // Pubkeys #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_4_0_ec_pubkey_parse")] pub fn secp256k1_ec_pubkey_parse(cx: *const Context, pk: *mut PublicKey, @@ -738,6 +738,14 @@ mod fuzz_dummy { rustsecp256k1_v0_4_0_context_preallocated_clone(cx, prealloc) } + pub unsafe fn secp256k1_context_randomize(cx: *mut Context, + _seed32: *const c_uchar) + -> c_int { + // This function is really slow, and unsuitable for fuzzing + check_context_flags(cx, 0); + 1 + } + unsafe fn check_context_flags(cx: *const Context, required_flags: c_uint) { assert!(!cx.is_null()); let cx_flags = if cx == secp256k1_context_no_precomp {