Merge rust-bitcoin/rust-bitcoin#1821: fuzz: replace `cfg(fuzzing)` weth `cfg(bitcoin-hashes-fuzz)`.

ab4a48c8ba ci: use new fuzzing cfg flags when fuzzing bitcoin (but not hashes) (Andrew Poelstra)
6649e15193 add README note explaining how to disable crypto for fuzzing (Andrew Poelstra)
283b7d6e51 hashes: rename fuzzing cfg parameter to bitcoin_hashes_fuzz (Andrew Poelstra)

Pull request description:

  A custom `cfg` flag can be turned on or off by the user. Our current use of `cfg(fuzzing)` is impossible to turn off when using honggfuzz, which makes it impossible to fuzz without the broken crypto. This causes trouble for some downstream crates and also makes it hard for us to fuzz our own library.

  Companion to rust-secp PR (TODO open this) which does effectively the same thing.

  Fixes #1587.

ACKs for top commit:
  tcharding:
    ACK ab4a48c8ba
  Kixunil:
    ACK ab4a48c8ba

Tree-SHA512: c873fbd7d39fc74ae4e67a28534b253b4a09b37b5985fefde944a3c2fbe74da7200ab666b8eae6b6a4916ceff3a8d0c6278d12abd3ae85884017de1c69c5dffe

.github/workflows/fuzz.yml

@@ -53,6 +53,7 @@ hashes_sha1,
           override: true
           profile: minimal
       - name: fuzz
+        run: if [[ "${{ matrix.fuzz_target }}" =~ ^bitcoin ]]; then export RUSTFLAGS='--cfg=hashes_fuzz --cfg=secp256k1_fuzz'; fi
         run: cd fuzz && ./fuzz.sh "${{ matrix.fuzz_target }}"
       - run: echo "${{ matrix.fuzz_target }}" >executed_${{ matrix.fuzz_target }}
       - uses: actions/upload-artifact@v2

fuzz/Cargo.toml

@@ -9,7 +9,7 @@ publish = false
 cargo-fuzz = true
 
 [dependencies]
-honggfuzz = { version = "0.5", default-features = false }
+honggfuzz = { version = "0.5.55", default-features = false }
 bitcoin = { version = "0.30.0", features = [ "serde" ] }
 
 serde = { version = "1.0.103", features = [ "derive" ] }

fuzz/README.md

@@ -20,6 +20,29 @@ On Nix, you can obtain these libraries by running
 
 and then run fuzz.sh as above.
 
+# Fuzzing with weak cryptography
+
+You may wish to replace the hashing and signing code with broken crypto,
+which will be faster and enable the fuzzer to do otherwise impossible
+things such as forging signatures or finding preimages to hashes.
+
+Doing so may result in spurious bug reports since the broken crypto does
+not respect the encoding or algebraic invariants upheld by the real crypto. We
+would like to improve this but it's a nontrivial problem -- though not
+beyond the abilities of a motivated student with a few months of time.
+Please let us know if you are interested in taking this on!
+
+Meanwhile, to use the broken crypto, simply compile (and run the fuzzing
+scripts) with
+
+    RUSTFLAGS="--cfg=hashes_fuzz --cfg=secp256k1_fuzz"
+
+which will replace the hashing library with broken hashes, and the
+secp256k1 library with broken cryptography.
+
+Needless to say, NEVER COMPILE REAL CODE WITH THESE FLAGS because if a
+fuzzer can break your crypto, so can anybody.
+
 # Long-term fuzzing
 
 To see the full list of targets, the most straightforward way is to run
@@ -85,9 +108,8 @@ The final line is a hex-encoded version of the input that caused the crash. You
 can test this directly by editing the `duplicate_crash` test to copy/paste the
 hex output into the call to `extend_vec_from_hex`. Then run the test with
 
-    RUSTFLAGS=--cfg=fuzzing cargo test
+    cargo test
 
-It is important to add the `cfg=fuzzing` flag, which tells rustc to compile the
-library as though it were running a fuzztest. In particular, this will disable
-or weaken all the cryptography.
+Note that if you set your `RUSTFLAGS` while fuzzing (see above) you must make
+sure they are set the same way when running `cargo test`.
 

fuzz/generate-files.sh

@@ -20,7 +20,7 @@ publish = false
 cargo-fuzz = true
 
 [dependencies]
-honggfuzz = { version = "0.5", default-features = false }
+honggfuzz = { version = "0.5.55", default-features = false }
 bitcoin = { version = "0.30.0", features = [ "serde" ] }
 
 serde = { version = "1.0.103", features = [ "derive" ] }
@@ -78,6 +78,7 @@ $(for name in $(listTargetNames); do echo "$name,"; done)
           override: true
           profile: minimal
       - name: fuzz
+        run: if [[ "\${{ matrix.fuzz_target }}" =~ ^bitcoin ]]; then export RUSTFLAGS='--cfg=hashes_fuzz --cfg=secp256k1_fuzz'; fi
         run: cd fuzz && ./fuzz.sh "\${{ matrix.fuzz_target }}"
       - run: echo "\${{ matrix.fuzz_target }}" >executed_\${{ matrix.fuzz_target }}
       - uses: actions/upload-artifact@v2

hashes/src/lib.rs

@@ -77,7 +77,7 @@
 #![allow(ellipsis_inclusive_range_patterns)]
 #![cfg_attr(all(not(test), not(feature = "std")), no_std)]
 // Instead of littering the codebase for non-fuzzing code just globally allow.
-#![cfg_attr(fuzzing, allow(dead_code, unused_imports))]
+#![cfg_attr(hashes_fuzz, allow(dead_code, unused_imports))]
 
 #[cfg(all(feature = "alloc", not(feature = "std")))]
 extern crate alloc;

hashes/src/ripemd160.rs

@@ -17,7 +17,7 @@ crate::internal_macros::hash_type! {
     "crate::util::json_hex_string::len_20"
 }
 
-#[cfg(not(fuzzing))]
+#[cfg(not(hashes_fuzz))]
 fn from_engine(mut e: HashEngine) -> Hash {
     // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining
     let data_len = e.length as u64;
@@ -37,7 +37,7 @@ fn from_engine(mut e: HashEngine) -> Hash {
     Hash(e.midstate())
 }
 
-#[cfg(fuzzing)]
+#[cfg(hashes_fuzz)]
 fn from_engine(e: HashEngine) -> Hash {
     let mut res = e.midstate();
     res[0] ^= (e.length & 0xff) as u8;
@@ -67,7 +67,7 @@ impl Default for HashEngine {
 impl crate::HashEngine for HashEngine {
     type MidState = [u8; 20];
 
-    #[cfg(not(fuzzing))]
+    #[cfg(not(hashes_fuzz))]
     fn midstate(&self) -> [u8; 20] {
         let mut ret = [0; 20];
         for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) {
@@ -76,7 +76,7 @@ impl crate::HashEngine for HashEngine {
         ret
     }
 
-    #[cfg(fuzzing)]
+    #[cfg(hashes_fuzz)]
     fn midstate(&self) -> [u8; 20] {
         let mut ret = [0; 20];
         ret.copy_from_slice(&self.buffer[..20]);

hashes/src/sha1.rs

@@ -59,7 +59,7 @@ impl Default for HashEngine {
 impl crate::HashEngine for HashEngine {
     type MidState = [u8; 20];
 
-    #[cfg(not(fuzzing))]
+    #[cfg(not(hashes_fuzz))]
     fn midstate(&self) -> [u8; 20] {
         let mut ret = [0; 20];
         for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) {
@@ -68,7 +68,7 @@ impl crate::HashEngine for HashEngine {
         ret
     }
 
-    #[cfg(fuzzing)]
+    #[cfg(hashes_fuzz)]
     fn midstate(&self) -> [u8; 20] {
         let mut ret = [0; 20];
         ret.copy_from_slice(&self.buffer[..20]);

hashes/src/sha256.rs

@@ -17,7 +17,7 @@ crate::internal_macros::hash_type! {
     "crate::util::json_hex_string::len_32"
 }
 
-#[cfg(not(fuzzing))]
+#[cfg(not(hashes_fuzz))]
 fn from_engine(mut e: HashEngine) -> Hash {
     // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining
     let data_len = e.length as u64;
@@ -37,7 +37,7 @@ fn from_engine(mut e: HashEngine) -> Hash {
     Hash(e.midstate().to_byte_array())
 }
 
-#[cfg(fuzzing)]
+#[cfg(hashes_fuzz)]
 fn from_engine(e: HashEngine) -> Hash {
     let mut hash = e.midstate().to_byte_array();
     if hash == [0; 32] {
@@ -74,7 +74,7 @@ impl Default for HashEngine {
 impl crate::HashEngine for HashEngine {
     type MidState = Midstate;
 
-    #[cfg(not(fuzzing))]
+    #[cfg(not(hashes_fuzz))]
     fn midstate(&self) -> Midstate {
         let mut ret = [0; 32];
         for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) {
@@ -83,7 +83,7 @@ impl crate::HashEngine for HashEngine {
         Midstate(ret)
     }
 
-    #[cfg(fuzzing)]
+    #[cfg(hashes_fuzz)]
     fn midstate(&self) -> Midstate {
         let mut ret = [0; 32];
         ret.copy_from_slice(&self.buffer[..32]);

hashes/src/sha512.rs

@@ -39,7 +39,7 @@ impl Default for HashEngine {
 impl crate::HashEngine for HashEngine {
     type MidState = [u8; 64];
 
-    #[cfg(not(fuzzing))]
+    #[cfg(not(hashes_fuzz))]
     fn midstate(&self) -> [u8; 64] {
         let mut ret = [0; 64];
         for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(8)) {
@@ -48,7 +48,7 @@ impl crate::HashEngine for HashEngine {
         ret
     }
 
-    #[cfg(fuzzing)]
+    #[cfg(hashes_fuzz)]
     fn midstate(&self) -> [u8; 64] {
         let mut ret = [0; 64];
         ret.copy_from_slice(&self.buffer[..64]);
@@ -80,7 +80,7 @@ impl Hash {
     fn internal_engine() -> HashEngine { Default::default() }
 }
 
-#[cfg(not(fuzzing))]
+#[cfg(not(hashes_fuzz))]
 pub(crate) fn from_engine(mut e: HashEngine) -> Hash {
     // pad buffer with a single 1-bit then all 0s, until there are exactly 16 bytes remaining
     let data_len = e.length as u64;
@@ -101,7 +101,7 @@ pub(crate) fn from_engine(mut e: HashEngine) -> Hash {
     Hash(e.midstate())
 }
 
-#[cfg(fuzzing)]
+#[cfg(hashes_fuzz)]
 pub(crate) fn from_engine(e: HashEngine) -> Hash {
     let mut hash = e.midstate();
     hash[0] ^= 0xff; // Make this distinct from SHA-256

hashes/src/siphash24.rs

@@ -16,10 +16,10 @@ crate::internal_macros::hash_type! {
     "crate::util::json_hex_string::len_8"
 }
 
-#[cfg(not(fuzzing))]
+#[cfg(not(hashes_fuzz))]
 fn from_engine(e: HashEngine) -> Hash { Hash::from_u64(Hash::from_engine_to_u64(e)) }
 
-#[cfg(fuzzing)]
+#[cfg(hashes_fuzz)]
 fn from_engine(e: HashEngine) -> Hash {
     let state = e.midstate();
     Hash::from_u64(state.v0 ^ state.v1 ^ state.v2 ^ state.v3)

hashes/src/util.rs

@@ -68,7 +68,7 @@ macro_rules! borrow_slice_impl(
 
 macro_rules! engine_input_impl(
     () => (
-        #[cfg(not(fuzzing))]
+        #[cfg(not(hashes_fuzz))]
         fn input(&mut self, mut inp: &[u8]) {
             while !inp.is_empty() {
                 let buf_idx = self.length % <Self as crate::HashEngine>::BLOCK_SIZE;
@@ -85,7 +85,7 @@ macro_rules! engine_input_impl(
             }
         }
 
-        #[cfg(fuzzing)]
+        #[cfg(hashes_fuzz)]
         fn input(&mut self, inp: &[u8]) {
             for c in inp {
                 self.buffer[0] ^= *c;