diff --git a/Cargo.lock b/Cargo.lock index a8c92ef6d6..a66b696d4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6059,6 +6059,7 @@ dependencies = [ "serde", "sha2", "tiny-keccak", + "wycheproof", "zeroize", ] @@ -10417,6 +10418,17 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" +[[package]] +name = "wycheproof" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "efb3be19abfb206c6adcbdf2007b09b0e8ca1f6530db40c03b42ce8ed4719894" +dependencies = [ + "data-encoding", + "serde", + "serde_json", +] + [[package]] name = "wyz" version = "0.5.1" diff --git a/Cargo.toml b/Cargo.toml index 80828466b9..55e433a3a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -229,6 +229,7 @@ tracing-test = "0.2" unicode-normalization = "0.1" url = "2.5.0" wasm-bindgen = "0.2" +wycheproof = "0.6.0" zerocopy = { version = "0.8", features = ["derive"] } zeroize = { version = "1.7", features = ["derive"] } zstd = "0.13" diff --git a/monad-secp/Cargo.toml b/monad-secp/Cargo.toml index f04af5b04f..f135faa106 100644 --- a/monad-secp/Cargo.toml +++ b/monad-secp/Cargo.toml @@ -36,6 +36,8 @@ rand = { workspace = true } rstest = { workspace = true } tiny-keccak = { workspace = true, features = ["keccak"] } +wycheproof = { workspace = true } + [[bench]] name = "secp" harness = false diff --git a/monad-secp/src/secp.rs b/monad-secp/src/secp.rs index dfff25484e..a606d5398e 100644 --- a/monad-secp/src/secp.rs +++ b/monad-secp/src/secp.rs @@ -97,6 +97,17 @@ fn msg_hash(msg: &[u8]) -> secp256k1::Message { secp256k1::Message::from_digest(hash.0) } +#[cfg(test)] +fn msg_hash_sha256(msg: &[u8]) -> secp256k1::Message { + use monad_crypto::hasher::{Hasher, Sha256Hash}; + let mut hasher = Sha256Hash::new(); + hasher.update(SD::PREFIX); + hasher.update(msg); + let hash = hasher.hash(); + + secp256k1::Message::from_digest(hash.0) +} + impl KeyPair { pub fn generate(rng: &mut R) -> Self { let keypair = secp256k1::Keypair::new(secp256k1::SECP256K1, rng); @@ -104,7 +115,7 @@ impl KeyPair { } /// Create a keypair from a secret key slice. The secret is zero-ized after - /// use. The secret must be 32 byytes. + /// use. The secret must be 32 bytes. pub fn from_bytes(secret: &mut [u8]) -> Result { let secret_array: [u8; 32] = secret .try_into() @@ -190,6 +201,22 @@ impl PubKey { ) .map_err(Error) } + + /// Verify that the message is correctly signed using SHA256 hash + #[cfg(test)] + pub fn verify_sha256( + &self, + msg: &[u8], + signature: &SecpSignature, + ) -> Result<(), Error> { + Secp256k1::verify_ecdsa( + secp256k1::SECP256K1, + msg_hash_sha256::(msg), + &signature.0.to_standard(), + &self.0, + ) + .map_err(Error) + } } impl SecpSignature { @@ -253,9 +280,14 @@ impl Drop for KeyPair { #[cfg(test)] mod tests { - use monad_crypto::signing_domain; + use monad_crypto::{signing_domain, signing_domain::SigningDomain}; use proptest::prelude::*; + use sha2::{Digest, Sha256}; use tiny_keccak::Hasher; + use wycheproof::{ + ecdsa::{TestName::EcdsaSecp256k1Sha256, TestSet}, + TestResult, + }; use super::{KeyPair, PubKey, SecpSignature}; @@ -317,6 +349,29 @@ mod tests { .is_err()); } + #[test] + fn test_domain_separation() { + struct AnotherDomain; + impl SigningDomain for AnotherDomain { + const PREFIX: &'static [u8] = b"another_domain"; + } + + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + + let msg = b"hello world"; + let signature = keypair.sign::(msg); + + assert!(keypair + .pubkey() + .verify::(msg, &signature) + .is_ok()); + assert!(keypair + .pubkey() + .verify::(msg, &signature) + .is_err()); + } + #[test] fn test_recovery() { let mut privkey: [u8; 32] = [127; 32]; @@ -439,6 +494,453 @@ mod tests { insta::assert_debug_snapshot!(profiles); } + fn map_wycheproof_result(result: TestResult) -> TestResult { + match result { + TestResult::Valid => TestResult::Valid, + TestResult::Invalid => TestResult::Invalid, + TestResult::Acceptable => TestResult::Valid, + } + } + + #[test] + fn test_secp_wycheproof_conformance() { + // Define a special signing domain for Wycheproof that uses raw SHA256 + struct WycheproofRawSha256; + impl SigningDomain for WycheproofRawSha256 { + const PREFIX: &'static [u8] = b""; + } + + let test_set = TestSet::load(EcdsaSecp256k1Sha256).unwrap(); + + for test_group in test_set.test_groups { + let pk = PubKey::from_slice(&test_group.key.key).unwrap(); + + for test in test_group.tests { + let Ok(mut sig_der) = secp256k1::ecdsa::Signature::from_der(&test.sig) else { + assert_eq!( + map_wycheproof_result(test.result), + TestResult::Invalid, + "failed to decode signature in test {}: {}", + test.tc_id, + test.comment + ); + continue; + }; + // We need to normalize the signature because rust-secp256k1 (via libsecp256k1) + // treats signatures with s in the upper half of the curve order as invalid (“low-S only”) + // to eliminate ECDSA’s malleability. + sig_der.normalize_s(); + + let sig_bytes = sig_der.serialize_compact(); + + // Wycheproof tests do not provide a recovery id, iterate over all possible ones to verify. + let mut n_bytes = [0u8; 65]; + n_bytes[..64].copy_from_slice(&sig_bytes); + + let mut res = TestResult::Invalid; + for i in 0..4 { + n_bytes[64] = i; + let sig = SecpSignature::deserialize(&n_bytes).unwrap(); + + if pk + .verify_sha256::(&test.msg, &sig) + .is_ok() + { + res = TestResult::Valid; + break; + } else { + continue; + } + } + assert_eq!( + map_wycheproof_result(test.result), + res, + "test {} failed: {}", + test.tc_id, + test.comment + ); + } + } + } + + #[test] + fn test_original_secp256k1_wycheproof_conformance() { + let test_set = TestSet::load(EcdsaSecp256k1Sha256).unwrap(); + + for test_group in test_set.test_groups { + let pk = secp256k1::PublicKey::from_slice(&test_group.key.key).unwrap(); + + for test in test_group.tests { + let Ok(mut sig_der) = secp256k1::ecdsa::Signature::from_der(&test.sig) else { + assert_eq!( + map_wycheproof_result(test.result), + TestResult::Invalid, + "failed to decode signature in test {}: {}", + test.tc_id, + test.comment + ); + continue; + }; + // We need to normalize the signature because rust-secp256k1 (via libsecp256k1) + // treats signatures with s in the upper half of the curve order as invalid (“low-S only”) + // to eliminate ECDSA’s malleability. + sig_der.normalize_s(); + + let msg = secp256k1::Message::from_digest(Sha256::digest(&test.msg).into()); + let res = + if secp256k1::Secp256k1::verify_ecdsa(secp256k1::SECP256K1, msg, &sig_der, &pk) + .is_ok() + { + TestResult::Valid + } else { + TestResult::Invalid + }; + assert_eq!( + map_wycheproof_result(test.result), + res, + "test {} failed: {}", + test.tc_id, + test.comment + ); + } + } + } + + #[test] + fn test_pubkey_operations() { + use std::{ + cmp::Ordering, + collections::hash_map::DefaultHasher, + hash::{Hash, Hasher}, + }; + fn hash_pk(pk: &PubKey) -> u64 { + let mut h = DefaultHasher::new(); + pk.hash(&mut h); + h.finish() + } + + // Deterministic key material for reproducibility + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + let pubkey1 = keypair.pubkey(); + + // Roundtrip via canonical encodings + let pubkey2 = PubKey::from_slice(&pubkey1.bytes()).unwrap(); + let pubkey3 = PubKey::from_slice(&pubkey1.bytes_compressed()).unwrap(); + + // Equality must hold + assert_eq!( + pubkey1, pubkey2, + "PubKey equality broke after uncompressed roundtrip" + ); + assert_eq!( + pubkey1, pubkey3, + "PubKey equality broke after compressed roundtrip" + ); + assert_eq!( + pubkey1.cmp(&pubkey2), + Ordering::Equal, + "Ord/Eq contract violated for uncompressed roundtrip" + ); + assert_eq!( + pubkey1.cmp(&pubkey3), + Ordering::Equal, + "Ord/Eq contract violated for compressed roundtrip" + ); + + let h1 = hash_pk(&pubkey1); + let h2 = hash_pk(&pubkey2); + let h3 = hash_pk(&pubkey3); + assert_eq!( + h1, h2, + "Hash/Eq contract violated for uncompressed roundtrip" + ); + assert_eq!(h1, h3, "Hash/Eq contract violated for compressed roundtrip"); + assert_eq!(h1, hash_pk(&pubkey1), "Hash not stable within a run"); + } + + #[test] + fn test_special_private_keys() { + let mut zero = + hex::decode("0000000000000000000000000000000000000000000000000000000000000000") + .unwrap(); + assert!(KeyPair::from_bytes(&mut zero).is_err()); + + let mut one = + hex::decode("0000000000000000000000000000000000000000000000000000000000000001") + .unwrap(); + assert!(KeyPair::from_bytes(&mut one).is_ok()); + + let mut order_minus_one = + hex::decode("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364140") + .unwrap(); + assert!(KeyPair::from_bytes(&mut order_minus_one).is_ok()); + + let mut order = + hex::decode("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141") + .unwrap(); + assert!(KeyPair::from_bytes(&mut order).is_err()); + } + + #[test] + fn test_non_malleability() { + // Big-endian subtraction: a - b (assuming a >= b) + fn sub_be_32(a: &[u8; 32], b: &[u8; 32]) -> [u8; 32] { + let mut out = [0u8; 32]; + let mut borrow: u16 = 0; + for i in (0..32).rev() { + let ai = a[i] as u16; + let bi = b[i] as u16; + let tmp = ai.wrapping_sub(bi).wrapping_sub(borrow); + out[i] = (tmp & 0xFF) as u8; + borrow = if ai < bi + borrow { 1 } else { 0 }; + } + out + } + + // 1) Generate a key and sign a message + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + let pubkey = keypair.pubkey(); + + let msg = b"malleability test message"; + let sig = keypair.sign::(msg); + + assert!(pubkey.verify::(msg, &sig).is_ok()); + + // 2) Assert the produced signature is already "low-S" + // (normalize_s() should not change it) + let std_sig = sig.0.to_standard(); + let mut std_sig_norm = std_sig; + std_sig_norm.normalize_s(); + assert_eq!( + std_sig.serialize_compact(), + std_sig_norm.serialize_compact(), + "Signer produced a high-S signature; expected low-S" + ); + + // 3) Construct malleable signature: (r, s') with s' = n - s + let comp = std_sig.serialize_compact(); // 64 bytes = r||s + let mut r = [0u8; 32]; + let mut s = [0u8; 32]; + r.copy_from_slice(&comp[..32]); + s.copy_from_slice(&comp[32..]); + + assert_ne!(s, [0u8; 32], "unexpected s=0 (should never happen)"); + + let n: [u8; 32] = secp256k1::constants::CURVE_ORDER; + let s_malleable = sub_be_32(&n, &s); + + // Build 65-byte wrapper encoding: r||s'||recid + let mut mal_bytes = [0u8; 65]; + mal_bytes[..32].copy_from_slice(&r); + mal_bytes[32..64].copy_from_slice(&s_malleable); + mal_bytes[64] = sig.serialize()[64]; + + // 4) The malleable signature must be rejected: + let mal_sig = SecpSignature::deserialize(&mal_bytes).unwrap(); + assert!( + pubkey.verify::(msg, &mal_sig).is_err(), + "High-S malleable signature successfully verified; signature is malleable" + ); + } + + #[test] + fn test_signature_deserialize_rejects_bad_lengths_and_recid() { + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + let msg = b"negative signature parsing"; + let sig = keypair.sign::(msg); + let good = sig.serialize(); + + // Bad lengths (DoS hardening: must return Err, never panic) + for len in [0usize, 1, 10, 64, 66, 100] { + let buf = vec![0u8; len]; + assert!( + SecpSignature::deserialize(&buf).is_err(), + "expected error for length {}", + len + ); + } + + // Bad recid values must be rejected (last byte is RecoveryId) + for bad_recid in 4u8..=255u8 { + let mut b = good; + b[64] = bad_recid; + assert!( + SecpSignature::deserialize(&b).is_err(), + "expected error for bad recid {}", + bad_recid + ); + } + } + + #[test] + fn test_signature_bitflip_corruption_is_rejected_or_fails_verify() { + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + let pk = keypair.pubkey(); + + let msg = b"bitflip corruption test"; + let sig = keypair.sign::(msg); + let good = sig.serialize(); + + // Flip one bit in r/s bytes and require: either parse fails or verify fails. + for i in [0usize, 5, 16, 31, 32, 40, 63] { + let mut b = good; + b[i] ^= 0x01; + + if let Ok(corrupted) = SecpSignature::deserialize(&b) { + assert!( + pk.verify::(msg, &corrupted).is_err(), + "corrupted signature unexpectedly verified (index {})", + i + ); + } + } + + // Flip the recid bit: verification ignores recid, but recovery should differ or fail. + for bit in [0x01u8, 0x02u8, 0x04u8, 0x08u8] { + let mut b = good; + b[64] ^= bit; + + if let Ok(corrupted) = SecpSignature::deserialize(&b) { + let recovered = corrupted.recover_pubkey::(msg); + // For a different (valid) recid, recovery should not yield the original key. + if let Ok(rpk) = recovered { + assert_ne!( + rpk.bytes(), + pk.bytes(), + "mutating recid should not recover original pubkey" + ); + } + } + } + } + + #[test] + fn test_pubkey_from_slice_rejects_invalid_encodings() { + // Wrong lengths (must be 33 or 65). + // This is only a necessary condition, not sufficient. + for len in [0usize, 1, 10, 32, 34, 64, 66, 100, 128, 255] { + let buf = vec![0u8; len]; + assert!( + PubKey::from_slice(&buf).is_err(), + "expected error for pubkey length {}", + len + ); + } + + // Generator G (compressed, 33 bytes) + { + let pk_bytes = + hex::decode("0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798") + .unwrap(); + let pk = PubKey::from_slice(&pk_bytes).expect("valid compressed pubkey should parse"); + assert_eq!(pk.bytes_compressed(), pk_bytes.as_slice()); + } + + // Generator G (uncompressed, 65 bytes) + { + let pk_bytes = hex::decode( + "0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8", + ).unwrap(); + let pk = PubKey::from_slice(&pk_bytes).expect("valid uncompressed pubkey should parse"); + assert_eq!(pk.bytes(), pk_bytes.as_slice()); + } + + // Invalid prefix for compressed (must be 0x02 or 0x03) + { + let pk = + hex::decode("0479be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798") + .unwrap(); + assert!( + PubKey::from_slice(&pk).is_err(), + "bad compressed prefix accepted" + ); + } + + // Invalid prefix for uncompressed (must be 0x04) + { + let pk = hex::decode( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8", + ) + .unwrap(); + assert!( + PubKey::from_slice(&pk).is_err(), + "bad uncompressed prefix accepted" + ); + } + + // Uncompressed point not on curve: (x=1, y=1) is not on secp256k1 + { + let mut pk = [0u8; 65]; + pk[0] = 0x04; + pk[32] = 0x01; // x = 1 + pk[64] = 0x01; // y = 1 + assert!( + PubKey::from_slice(&pk).is_err(), + "non-curve point (1,1) accepted" + ); + } + + // Compressed x out of field range: x = p (field prime) must be rejected (x must be < p) + { + // secp256k1 field prime p: + // FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFE FFFFFC2F + let pk = + hex::decode("02FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F") + .unwrap(); + assert!( + PubKey::from_slice(&pk).is_err(), + "compressed x=p (out of range) accepted" + ); + } + } + + #[test] + fn test_signing_is_deterministic_for_same_key_domain_and_message() { + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + + let msg = b"determinism check"; + let sig1 = keypair.sign::(msg).serialize(); + let sig2 = keypair.sign::(msg).serialize(); + + assert_eq!( + sig1, sig2, + "signing is not deterministic; nonce behavior changed (or RNG introduced)" + ); + } + + #[test] + fn test_recovery_misuse() { + let mut privkey: [u8; 32] = [127; 32]; + let keypair = KeyPair::from_bytes(&mut privkey).unwrap(); + let pk = keypair.pubkey(); + + let msg1 = b"recovery misuse msg1"; + let msg2 = b"recovery misuse msg2"; + + let sig = keypair.sign::(msg1); + + // Correct recovery should return the signing pubkey + let recovered1 = sig.recover_pubkey::(msg1).unwrap(); + assert_eq!(recovered1.bytes(), pk.bytes()); + + // Wrong-message recovery should not yield the original pubkey + let recovered2 = sig.recover_pubkey::(msg2).unwrap(); + assert_ne!( + recovered2.bytes(), + pk.bytes(), + "wrong-message recovery unexpectedly produced original pubkey" + ); + + // Verification is message-bound + assert!(pk.verify::(msg1, &sig).is_ok()); + assert!(pk.verify::(msg2, &sig).is_err()); + } + proptest! { #[test] fn proptest_from_ikm(ikm: [u8; 32]) { @@ -455,5 +957,23 @@ mod tests { } } } + + #[test] + fn proptest_no_panic_signature_deserialize(_case in any::<[u8; 65]>()) { + // Must never panic + let _ = SecpSignature::deserialize(&_case); + } + + #[test] + fn proptest_no_panic_pubkey_from_slice(bytes in proptest::collection::vec(any::(), 0..100)) { + // Must never panic + let _ = PubKey::from_slice(&bytes); + } + + #[test] + fn proptest_no_panic_rlp_decode(bytes in proptest::collection::vec(any::(), 0..200)) { + // Must never panic + let _ = alloy_rlp::decode_exact::(&bytes); + } } }