From cea6cf84416a651ffb458469b80da2da885fe1f5 Mon Sep 17 00:00:00 2001 From: Alin Tomescu Date: Tue, 25 Jun 2024 16:09:45 -0700 Subject: [PATCH] Revert "fix:Correct ECDSA signature malleability handling (#13544)" (#13818) This reverts commit 6e4a3f9aeeb6d3afab3f3ae9efc625a008382223. --- .../src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs | 10 +++++----- .../src/unit_tests/secp256r1_ecdsa_test.rs | 10 ++++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs b/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs index 40b5c230b5eed..28cfb76105170 100644 --- a/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs +++ b/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs @@ -65,14 +65,14 @@ impl Signature { if bytes.len() != SIGNATURE_LENGTH { return Err(CryptoMaterialError::WrongLengthError); } - if !Signature::check_s_le_order_half(&bytes[32..]) { + if !Signature::check_s_lt_order_half(&bytes[32..]) { return Err(CryptoMaterialError::CanonicalRepresentationError); } Ok(()) } - /// Check if S <= ORDER_HALF to capture invalid signatures. - fn check_s_le_order_half(s: &[u8]) -> bool { + /// Check if S < ORDER_HALF to capture invalid signatures. + fn check_s_lt_order_half(s: &[u8]) -> bool { for i in 0..32 { match s[i].cmp(&ORDER_HALF[i]) { Ordering::Less => return true, @@ -80,8 +80,8 @@ impl Signature { _ => {}, } } - // At this stage S == ORDER_HALF , it is still considered a canonical value. - true + // At this stage S == ORDER_HALF which implies a non canonical S. + false } /// If the signature {R,S} does not have S < n/2 where n is the Ristretto255 order, return diff --git a/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs b/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs index b4581a54ae4d9..79d5d32fddf95 100644 --- a/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs +++ b/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs @@ -208,11 +208,13 @@ proptest! { let sig_unchecked = Signature::from_bytes_unchecked(&serialized); prop_assert!(sig_unchecked.is_ok()); - // S = ORDER_HALF should be a canonical signature. + // Update the signature by setting S = L to make it invalid. serialized[32..].copy_from_slice(&ORDER_HALF); - let canonical: &[u8] = &serialized; - prop_assert!( - Signature::try_from(canonical).is_ok() + let serialized_malleable_l: &[u8] = &serialized; + // try_from will fail with CanonicalRepresentationError. + prop_assert_eq!( + Signature::try_from(serialized_malleable_l), + Err(CryptoMaterialError::CanonicalRepresentationError) ); } }