From b1916a7bd27a77382cbfa8b1c9f8d98989824270 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 23 Dec 2020 18:00:34 +0000 Subject: [PATCH] fuzz: implement recoverable signatures, get all tests passing, run them in CI --- contrib/test.sh | 9 ++- secp256k1-sys/src/lib.rs | 7 -- secp256k1-sys/src/recovery.rs | 123 ++++++++++++++++++++-------------- src/lib.rs | 2 + src/recovery.rs | 1 + src/schnorrsig.rs | 1 + 6 files changed, 83 insertions(+), 60 deletions(-) diff --git a/contrib/test.sh b/contrib/test.sh index 7befc21bd..c65c26f19 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -33,12 +33,17 @@ if [ "$DO_FEATURE_MATRIX" = true ]; then cargo test --all --features="$feature" done - # Other combos + # Other combos RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --all - RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --all --features="recovery" + RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --all --features="$FEATURES" cargo test --all --features="rand rand-std" cargo test --all --features="rand serde" + if [ "$DO_BENCH" = true ]; then # proxy for us having a nightly compiler + cargo test --all --all-features + RUSTFLAGS='--cfg=rust_secp_fuzz' RUSTDOCFLAGS='--cfg=rust_secp_fuzz' cargo test --all --all-features + fi + # Examples cargo run --example sign_verify cargo run --example sign_verify_recovery --features=recovery diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 7a8d66621..72fe4ec49 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -97,13 +97,6 @@ pub type SchnorrNonceFn = Option u32 { - self.0 as u32 - } -} - /// Library-internal representation of a Secp256k1 public key #[repr(C)] pub struct PublicKey([c_uchar; 64]); diff --git a/secp256k1-sys/src/recovery.rs b/secp256k1-sys/src/recovery.rs index 9b9464b81..683eef43a 100644 --- a/secp256k1-sys/src/recovery.rs +++ b/secp256k1-sys/src/recovery.rs @@ -16,8 +16,7 @@ //! # FFI of the recovery module use ::types::*; -#[cfg(not(rust_secp_fuzz))] -use ::{Context, Signature, NonceFn, PublicKey}; +use {Context, Signature, NonceFn, PublicKey}; /// Library-internal representation of a Secp256k1 signature + recovery ID #[repr(C)] @@ -36,7 +35,6 @@ impl Default for RecoverableSignature { } } -#[cfg(not(rust_secp_fuzz))] extern "C" { #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_recoverable_signature_parse_compact")] pub fn secp256k1_ecdsa_recoverable_signature_parse_compact(cx: *const Context, sig: *mut RecoverableSignature, @@ -52,6 +50,10 @@ extern "C" { pub fn secp256k1_ecdsa_recoverable_signature_convert(cx: *const Context, sig: *mut Signature, input: *const RecoverableSignature) -> c_int; +} + +#[cfg(not(rust_secp_fuzz))] +extern "C" { #[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_3_1_ecdsa_sign_recoverable")] pub fn secp256k1_ecdsa_sign_recoverable(cx: *const Context, sig: *mut RecoverableSignature, @@ -72,58 +74,77 @@ extern "C" { #[cfg(rust_secp_fuzz)] mod fuzz_dummy { - extern crate std; - use self::std::ptr; - use super::RecoverableSignature; - use types::*; - use ::{Signature, Context, PublicKey, NonceFn, secp256k1_ec_seckey_verify, - SECP256K1_START_NONE, SECP256K1_START_VERIFY, SECP256K1_START_SIGN}; - - pub unsafe fn secp256k1_ecdsa_recoverable_signature_parse_compact(_cx: *const Context, _sig: *mut RecoverableSignature, - _input64: *const c_uchar, _recid: c_int) - -> c_int { - unimplemented!(); - } - - pub unsafe fn secp256k1_ecdsa_recoverable_signature_serialize_compact(_cx: *const Context, _output64: *mut c_uchar, - _recid: *mut c_int, _sig: *const RecoverableSignature) - -> c_int { - unimplemented!(); - } - - pub unsafe fn secp256k1_ecdsa_recoverable_signature_convert(_cx: *const Context, _sig: *mut Signature, - _input: *const RecoverableSignature) - -> c_int { - unimplemented!(); - } - - /// Sets sig to (2|3)||msg32||sk - pub unsafe fn secp256k1_ecdsa_sign_recoverable(cx: *const Context, - sig: *mut RecoverableSignature, - msg32: *const c_uchar, - sk: *const c_uchar, - _noncefn: NonceFn, - _noncedata: *const c_void) - -> c_int { - assert!(!cx.is_null() && (*cx).flags() & !(SECP256K1_START_NONE | SECP256K1_START_VERIFY | SECP256K1_START_SIGN) == 0); - assert!((*cx).flags() & SECP256K1_START_SIGN == SECP256K1_START_SIGN); - if secp256k1_ec_seckey_verify(cx, sk) != 1 { return 0; } - if *sk.offset(0) > 0x7f { - (*sig).0[0] = 2; - } else { - (*sig).0[0] = 3; + use super::*; + use std::slice; + + use secp256k1_ec_pubkey_create; + use secp256k1_ec_pubkey_parse; + use secp256k1_ec_pubkey_serialize; + use SECP256K1_SER_COMPRESSED; + + /// Sets sig to msg32||full pk + pub unsafe fn secp256k1_ecdsa_sign_recoverable( + cx: *const Context, + sig: *mut RecoverableSignature, + msg32: *const c_uchar, + sk: *const c_uchar, + _noncefn: NonceFn, + _noncedata: *const c_void, + ) -> c_int { + // 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 { + return 0; } - ptr::copy(msg32, (*sig).0[1..33].as_mut_ptr(), 32); - ptr::copy(sk, (*sig).0[33..65].as_mut_ptr(), 32); + // Sign + let sig_sl = slice::from_raw_parts_mut(sig as *mut u8, 65); + let msg_sl = slice::from_raw_parts(msg32 as *const u8, 32); + sig_sl[..32].copy_from_slice(msg_sl); + let mut out_len: size_t = 33; + secp256k1_ec_pubkey_serialize(cx, sig_sl[32..].as_mut_ptr(), &mut out_len, &new_pk, SECP256K1_SER_COMPRESSED); + // Encode the parity of the pubkey in the final byte as 0/1, + // which is the same encoding (though the parity is computed + // differently) as real recoverable signatures. + sig_sl.swap(32, 64); + sig_sl[64] -= 2; 1 } - pub unsafe fn secp256k1_ecdsa_recover(_cx: *const Context, - _pk: *mut PublicKey, - _sig: *const RecoverableSignature, - _msg32: *const c_uchar) - -> c_int { - unimplemented!(); + pub unsafe fn secp256k1_ecdsa_recover( + cx: *const Context, + pk: *mut PublicKey, + sig: *const RecoverableSignature, + msg32: *const c_uchar + ) -> c_int { + let sig_sl = slice::from_raw_parts(sig as *const u8, 65); + let msg_sl = slice::from_raw_parts(msg32 as *const u8, 32); + println!("HMM0"); + + if sig_sl[64] > 4 { + return 0; + } + // Pull the original pk out of the siganture + let mut pk_ser = [0; 33]; + pk_ser.copy_from_slice(&sig_sl[32..]); + pk_ser.swap(0, 32); + pk_ser[0] += 2; + // Check that it parses (in a real sig, this would be the R value, + // so it is actually required to be a valid point) + if secp256k1_ec_pubkey_parse(cx, pk, pk_ser.as_ptr(), 33) == 0 { + return 0; + } + // Munge it up so that a different message will give a different pk + for i in 0..32 { + pk_ser[i + 1] ^= sig_sl[i] ^ msg_sl[i]; + } + // If any munging happened, this will fail parsing half the time, so + // tweak-and-loop until we find a key that works. + let mut idx = 0; + while secp256k1_ec_pubkey_parse(cx, pk, pk_ser.as_ptr(), 33) == 0 { + pk_ser[1 + idx / 8] ^= 1 << (idx % 8); + idx += 1; + } + 1 } } #[cfg(rust_secp_fuzz)] diff --git a/src/lib.rs b/src/lib.rs index b4eddd740..bfbb1eb69 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -103,6 +103,7 @@ //! 0xc9, 0x42, 0x8f, 0xca, 0x69, 0xc1, 0x32, 0xa2, //! ]).expect("compact signatures are 64 bytes; DER signatures are 68-72 bytes"); //! +//! # #[cfg(not(rust_secp_fuzz))] //! assert!(secp.verify(&message, &sig, &public_key).is_ok()); //! ``` //! @@ -1221,6 +1222,7 @@ mod tests { } #[cfg(feature = "serde")] + #[cfg(not(rust_secp_fuzz))] // fixed sig vectors can't work with fuzz-sigs #[test] fn test_signature_serde() { use serde_test::{Configure, Token, assert_tokens}; diff --git a/src/recovery.rs b/src/recovery.rs index 5eb6f57a2..e598ca863 100644 --- a/src/recovery.rs +++ b/src/recovery.rs @@ -235,6 +235,7 @@ mod tests { } #[test] + #[cfg(not(rust_secp_fuzz))] // fixed sig vectors can't work with fuzz-sigs fn sign() { let mut s = Secp256k1::new(); s.randomize(&mut thread_rng()); diff --git a/src/schnorrsig.rs b/src/schnorrsig.rs index f63745696..12ab2f25c 100644 --- a/src/schnorrsig.rs +++ b/src/schnorrsig.rs @@ -722,6 +722,7 @@ mod tests { } #[cfg(feature = "serde")] + #[cfg(not(rust_secp_fuzz))] // fixed sig vectors can't work with fuzz-sigs #[test] fn test_signature_serde() { use serde_test::{assert_tokens, Configure, Token};