fuzz: implement recoverable signatures, get all tests passing, run them in CI

Author: apoelstra

contrib/test.sh

@@ -33,12 +33,17 @@ if [ "$DO_FEATURE_MATRIX" = true ]; then
         cargo test --all --features="$feature"
     done
 
-    # Other combos
-    RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --all
-    RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --no-run --all --features="recovery"
+    # Other combos 
+    RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --all
+    RUSTFLAGS='--cfg=rust_secp_fuzz' cargo test --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

secp256k1-sys/src/lib.rs

@@ -97,13 +97,6 @@ pub type SchnorrNonceFn = Option<unsafe extern "C" fn(
 #[derive(Clone, Debug)]
 #[repr(C)] pub struct Context(c_int);
 
-#[cfg(rust_secp_fuzz)]
-impl Context {
-    pub fn flags(&self) -> u32 {
-        self.0 as u32
-    }
-}
-
 /// Library-internal representation of a Secp256k1 public key
 #[repr(C)]
 pub struct PublicKey([c_uchar; 64]);

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)]

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};

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());

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};