From d11454aa7a873b0ce8b51d31bdc590d4883e5d18 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Wed, 7 Aug 2024 14:09:38 -0400 Subject: [PATCH 1/6] feat(crypto): CRP-2560 Add BIP340 support to secp256k1 utility crate --- rs/crypto/ecdsa_secp256k1/src/lib.rs | 195 ++++++++++++++++------- rs/crypto/ecdsa_secp256k1/tests/tests.rs | 73 ++++++++- 2 files changed, 208 insertions(+), 60 deletions(-) diff --git a/rs/crypto/ecdsa_secp256k1/src/lib.rs b/rs/crypto/ecdsa_secp256k1/src/lib.rs index b672899c309..3ab7ac1f2a6 100644 --- a/rs/crypto/ecdsa_secp256k1/src/lib.rs +++ b/rs/crypto/ecdsa_secp256k1/src/lib.rs @@ -12,7 +12,7 @@ use k256::{ }, AffinePoint, Scalar, Secp256k1, }; -use rand::{CryptoRng, RngCore}; +use rand::{CryptoRng, Rng, RngCore}; use zeroize::ZeroizeOnDrop; /// An error indicating that decoding a key failed @@ -319,10 +319,10 @@ fn pem_encode(raw: &[u8], label: &'static str) -> String { }) } -/// An ECDSA private key +/// A secp256k1 public key, suitable for generating ECDSA and BIP340 signatures #[derive(Clone, ZeroizeOnDrop)] pub struct PrivateKey { - key: k256::ecdsa::SigningKey, + key: k256::SecretKey, } impl PrivateKey { @@ -334,7 +334,7 @@ impl PrivateKey { /// Generate a new random private key using some provided RNG pub fn generate_using_rng(rng: &mut R) -> Self { - let key = k256::ecdsa::SigningKey::random(rng); + let key = k256::SecretKey::random(rng); Self { key } } @@ -345,7 +345,7 @@ impl PrivateKey { KeyDecodingError::InvalidKeyEncoding(format!("invalid key size = {}.", bytes.len())) })?; - let key = k256::ecdsa::SigningKey::from_bytes(&GenericArray::from(byte_array)) + let key = k256::SecretKey::from_bytes(&GenericArray::from(byte_array)) .map_err(|e| KeyDecodingError::InvalidKeyEncoding(format!("{:?}", e)))?; Ok(Self { key }) } @@ -353,7 +353,7 @@ impl PrivateKey { /// Deserialize a private key encoded in PKCS8 format pub fn deserialize_pkcs8_der(der: &[u8]) -> Result { use k256::pkcs8::DecodePrivateKey; - let key = k256::ecdsa::SigningKey::from_pkcs8_der(der) + let key = k256::SecretKey::from_pkcs8_der(der) .map_err(|e| KeyDecodingError::InvalidKeyEncoding(format!("{:?}", e)))?; Ok(Self { key }) } @@ -416,17 +416,19 @@ impl PrivateKey { pem_encode(&self.serialize_rfc5915_der(), PEM_HEADER_RFC5915) } - /// Sign a message + /// Sign a message with ECDSA /// /// The message is hashed with SHA-256 and the signature is /// normalized (using the minimum-s approach of BitCoin) pub fn sign_message(&self, message: &[u8]) -> [u8; 64] { use k256::ecdsa::{signature::Signer, Signature}; - let sig: Signature = self.key.sign(message); + + let ecdsa = k256::ecdsa::SigningKey::from(&self.key); + let sig: Signature = ecdsa.sign(message); sig.to_bytes().into() } - /// Sign a message digest + /// Sign a message digest with ECDSA /// /// The signature is normalized (using the minimum-s approach of BitCoin) pub fn sign_digest(&self, digest: &[u8]) -> Option<[u8; 64]> { @@ -436,17 +438,42 @@ impl PrivateKey { } use k256::ecdsa::{signature::hazmat::PrehashSigner, Signature}; - let sig: Signature = self - .key - .sign_prehash(digest) - .expect("Failed to sign digest"); + let ecdsa = k256::ecdsa::SigningKey::from(&self.key); + let sig: Signature = ecdsa.sign_prehash(digest).expect("Failed to sign digest"); Some(sig.to_bytes().into()) } + /// Sign a message with BIP340 Schnorr + pub fn sign_bip340(&self, message: &[u8; 32], rng: &mut R) -> [u8; 64] { + let need_flip = self.public_key().serialize_sec1(true)[0] == 0x03; + + let bip340 = if need_flip { + let ns = self.key.to_nonzero_scalar().negate(); + let nz_ns = + k256::NonZeroScalar::new(ns).expect("Negation of non-zero is always non-zero"); + k256::schnorr::SigningKey::from(nz_ns) + } else { + k256::schnorr::SigningKey::from(&self.key) + }; + + loop { + /* + * The only way this function can fail is the (cryptographically unlikely) + * situation where k or s of zero is generated. If this occurs, simply retry + * with a new aux_rand + */ + let aux_rand = rng.gen::<[u8; 32]>(); + if let Ok(sig) = bip340.sign_prehash_with_aux_rand(message, &aux_rand) { + return sig.to_bytes().into(); + } + } + } + /// Return the public key corresponding to this private key pub fn public_key(&self) -> PublicKey { - let key = self.key.verifying_key(); - PublicKey { key: *key } + PublicKey { + key: self.key.public_key(), + } } /// Derive a private key from this private key using a derivation path @@ -484,27 +511,27 @@ impl PrivateKey { ) -> (Self, [u8; 32]) { use k256::NonZeroScalar; - let public_key: AffinePoint = *self.key.verifying_key().as_affine(); + let public_key: AffinePoint = self.key.public_key().to_projective().to_affine(); let (_pt, offset, derived_chain_code) = derivation_path.derive_offset(public_key, chain_code); - let derived_scalar = self.key.as_nonzero_scalar().as_ref().add(&offset); + let derived_scalar = self.key.to_nonzero_scalar().as_ref().add(&offset); let nz_ds = NonZeroScalar::new(derived_scalar).expect("Derivation always produces non-zero sum"); let derived_key = Self { - key: k256::ecdsa::SigningKey::from(nz_ds), + key: k256::SecretKey::from(nz_ds), }; (derived_key, derived_chain_code) } } -/// An ECDSA public key +/// A secp256k1 public key, suitable for verifying ECDSA or BIP340 signatures #[derive(Debug, Clone, PartialEq, Eq)] pub struct PublicKey { - key: k256::ecdsa::VerifyingKey, + key: k256::PublicKey, } impl PublicKey { @@ -515,15 +542,37 @@ impl PublicKey { /// /// See SEC1 section 2.3.3 for details of the format pub fn deserialize_sec1(bytes: &[u8]) -> Result { - let key = k256::ecdsa::VerifyingKey::from_sec1_bytes(bytes) + let key = k256::PublicKey::from_sec1_bytes(bytes) .map_err(|e| KeyDecodingError::InvalidKeyEncoding(format!("{:?}", e)))?; Ok(Self { key }) } + /// Deserialize a public key stored as BIP340 key + /// + /// This is just the encoding of the x coordinate of the point. Implicitly, + /// the y coordinate is the even choice. + /// + /// See BIP340 + /// for details + pub fn deserialize_bip340(bytes: &[u8]) -> Result { + if bytes.len() != 32 { + return Err(KeyDecodingError::InvalidKeyEncoding(format!( + "Expected 32 bytes got {}", + bytes.len() + ))); + } + + let mut sec1 = [0u8; 33]; + sec1[0] = 0x02; // even y + sec1[1..].copy_from_slice(bytes); + + Self::deserialize_sec1(&sec1) + } + /// Deserialize a public key stored in DER SubjectPublicKeyInfo format pub fn deserialize_der(bytes: &[u8]) -> Result { use k256::pkcs8::DecodePublicKey; - let key = k256::ecdsa::VerifyingKey::from_public_key_der(bytes) + let key = k256::PublicKey::from_public_key_der(bytes) .map_err(|e| KeyDecodingError::InvalidKeyEncoding(format!("{:?}", e)))?; Ok(Self { key }) } @@ -545,9 +594,24 @@ impl PublicKey { /// /// See SEC1 section 2.3.3 for details pub fn serialize_sec1(&self, compressed: bool) -> Vec { + use k256::elliptic_curve::sec1::ToEncodedPoint; self.key.to_encoded_point(compressed).as_bytes().to_vec() } + /// Serialize a public key in the style of BIP340 + /// + /// That is, with the x coordinate only and the y coordinate being implicit + /// + /// See BIP340 + /// for details + pub fn serialize_bip340(&self) -> Vec { + let sec1 = self.serialize_sec1(true); + + // Remove the leading byte of the SEC1 encoding, which indicates + // the sign of y, returning only the encoding of the x coordinate + sec1[1..].to_vec() + } + /// Serialize a public key in DER as a SubjectPublicKeyInfo pub fn serialize_der(&self) -> Vec { der_encode_ecdsa_spki_pubkey(&self.serialize_sec1(false)) @@ -570,33 +634,24 @@ impl PublicKey { Err(_) => return false, }; - /* - * In k256 0.11 and earlier, verify required that s be normalized. There is a regression in - * k256 0.12 (https://github.com/RustCrypto/elliptic-curves/issues/908) which causes either s - * to be accepted. Until this is fixed, include an explicit check on the sign of s. - */ - if signature.normalize_s().is_some() { - return false; - } - - self.key.verify(message, &signature).is_ok() + let ecdsa = k256::ecdsa::VerifyingKey::from(&self.key); + ecdsa.verify(message, &signature).is_ok() } /// Verify a (message,signature) pair /// /// The message is hashed with SHA-256 /// - /// This accepts signatures without s-normalization + /// This accepts signatures without requiring s-normalization /// - /// ECDSA signatures are a tuple of integers (r,s) which satisfy a certain + /// ECDSA signatures are a pair of integers (r,s) which satisfy a certain /// equation which involves also the public key and the message. A quirk of /// ECDSA is that if (r,s) is a valid signature then (r,-s) is also a valid /// signature (here negation is modulo the group order). /// /// This means that given a valid ECDSA signature, it is possible to create /// a "new" ECDSA signature that is also valid, without having access to the - /// public key. Unlike `verify_signature`, this function accepts either `s` - /// value. + /// key. Unlike `verify_signature`, this function accepts either `s` value. pub fn verify_signature_with_malleability(&self, message: &[u8], signature: &[u8]) -> bool { use k256::ecdsa::signature::Verifier; let signature = match k256::ecdsa::Signature::try_from(signature) { @@ -604,10 +659,11 @@ impl PublicKey { Err(_) => return false, }; + let ecdsa = k256::ecdsa::VerifyingKey::from(&self.key); if let Some(normalized) = signature.normalize_s() { - self.key.verify(message, &normalized).is_ok() + ecdsa.verify(message, &normalized).is_ok() } else { - self.key.verify(message, &signature).is_ok() + ecdsa.verify(message, &signature).is_ok() } } @@ -620,31 +676,22 @@ impl PublicKey { Err(_) => return false, }; - /* - * In k256 0.11 and earlier, verify required that s be normalized. There is a regression in - * k256 0.12 (https://github.com/RustCrypto/elliptic-curves/issues/908) which causes either s - * to be accepted. Until this is fixed, include an explicit check on the sign of s. - */ - if signature.normalize_s().is_some() { - return false; - } - - self.key.verify_prehash(digest, &signature).is_ok() + let ecdsa = k256::ecdsa::VerifyingKey::from(&self.key); + ecdsa.verify_prehash(digest, &signature).is_ok() } - /// Verify a (message digest,signature) pair + /// Verify a (digest,signature) pair /// - /// This accepts signatures without s-normalization + /// This accepts signatures without requiring s-normalization /// - /// ECDSA signatures are a tuple of integers (r,s) which satisfy a certain + /// ECDSA signatures are a pair of integers (r,s) which satisfy a certain /// equation which involves also the public key and the message. A quirk of /// ECDSA is that if (r,s) is a valid signature then (r,-s) is also a valid - /// signature (here negation is modulo the group order). + /// signature (here negation is modulo the group order) for the same message. /// /// This means that given a valid ECDSA signature, it is possible to create - /// a "new" ECDSA signature that is also valid, without having access to the - /// public key. Unlike `verify_signature_prehashed`, this function accepts either `s` - /// value. + /// a "new" ECDSA signature, without having access to the key. Unlike + /// `verify_signature_prehashed`, this function accepts either `s` value. pub fn verify_signature_prehashed_with_malleability( &self, digest: &[u8], @@ -657,10 +704,37 @@ impl PublicKey { Err(_) => return false, }; + let ecdsa = k256::ecdsa::VerifyingKey::from(&self.key); if let Some(normalized) = signature.normalize_s() { - self.key.verify_prehash(digest, &normalized).is_ok() + ecdsa.verify_prehash(digest, &normalized).is_ok() + } else { + ecdsa.verify_prehash(digest, &signature).is_ok() + } + } + + /// Verify a BIP340 (message,signature) pair + pub fn verify_bip340_signature(&self, message: &[u8], signature: &[u8]) -> bool { + use k256::elliptic_curve::point::AffineCoordinates; + use k256::schnorr::signature::hazmat::PrehashVerifier; + use std::ops::Neg; + + let signature = match k256::schnorr::Signature::try_from(signature) { + Ok(sig) => sig, + Err(_) => return false, + }; + + let pt = self.key.to_projective().to_affine(); + + let pt = if pt.y_is_odd().into() { pt.neg() } else { pt }; + + if let Ok(pk) = k256::PublicKey::from_affine(pt) { + if let Ok(bip340) = k256::schnorr::VerifyingKey::try_from(pk) { + bip340.verify_prehash(message, &signature).is_ok() + } else { + false + } } else { - self.key.verify_prehash(digest, &signature).is_ok() + false } } @@ -679,7 +753,10 @@ impl PublicKey { ) -> Result { let signature = k256::ecdsa::Signature::from_slice(signature) .map_err(|e| RecoveryError::SignatureParseError(e.to_string()))?; - k256::ecdsa::RecoveryId::trial_recovery_from_prehash(&self.key, digest, &signature) + + let ecdsa = k256::ecdsa::VerifyingKey::from(&self.key); + + k256::ecdsa::RecoveryId::trial_recovery_from_prehash(&ecdsa, digest, &signature) .map(|recid| RecoveryId { recid }) .map_err(|e| RecoveryError::WrongParameters(e.to_string())) } @@ -710,7 +787,7 @@ impl PublicKey { let (pt, _offset, chain_code) = derivation_path.derive_offset(public_key, chain_code); let derived_key = Self { - key: k256::ecdsa::VerifyingKey::from( + key: k256::PublicKey::from( k256::PublicKey::from_affine(pt).expect("Derived point is valid"), ), }; diff --git a/rs/crypto/ecdsa_secp256k1/tests/tests.rs b/rs/crypto/ecdsa_secp256k1/tests/tests.rs index b5cd397bd3c..0538c8eaf93 100644 --- a/rs/crypto/ecdsa_secp256k1/tests/tests.rs +++ b/rs/crypto/ecdsa_secp256k1/tests/tests.rs @@ -104,7 +104,7 @@ fn should_reject_long_x_when_deserializing_private_key() { } #[test] -fn should_accept_signatures_that_we_generate() { +fn should_accept_ecdsa_signatures_that_we_generate() { use rand::RngCore; let rng = &mut test_rng(); @@ -128,6 +128,26 @@ fn should_accept_signatures_that_we_generate() { } } +#[test] +fn should_accept_bip340_signatures_that_we_generate() { + use rand::RngCore; + + let mut rng = test_rng(); + + for _ in 0..100 { + let sk = PrivateKey::generate_using_rng(&mut rng); + + let pk = sk.public_key(); + println!("{}", hex::encode(pk.serialize_sec1(true))); + + let mut msg = [0u8; 32]; + rng.fill_bytes(&mut msg); + let sig = sk.sign_bip340(&msg, &mut rng); + + assert!(pk.verify_bip340_signature(&msg, &sig)); + } +} + #[test] fn should_reject_high_s_in_signature_unless_malleable() -> Result<(), KeyDecodingError> { let pk = PublicKey::deserialize_sec1(&hex::decode("04E38257CE81AB62AB1DF591E360AB0021D2D24E737299CF48317DBF31A3996A2A78DD07EA1996F24FE829B4EE968BA2700632D8F165E793E41AE37B8911FC83C9").unwrap())?; @@ -256,11 +276,62 @@ fn should_serialization_and_deserialization_round_trip_for_public_keys( assert_eq!(key_via_sec1c.serialize_sec1(false), expected); assert_eq!(key_via_der.serialize_sec1(false), expected); assert_eq!(key_via_pem.serialize_sec1(false), expected); + + // This only holds for keys with an even y coordinate + if key.serialize_sec1(false)[0] == 0x02 { + let key_via_bip340 = PublicKey::deserialize_bip340(&key.serialize_bip340())?; + assert_eq!(key_via_bip340.serialize_sec1(false), expected); + } } Ok(()) } +#[test] +fn should_match_bip340_reference_test_signatures() { + struct Bip340Test { + msg: Vec, + sig: Vec, + pk: Vec, + accept: bool, + } + + impl Bip340Test { + fn new(pk: &'static str, msg: &'static str, sig: &'static str, accept: bool) -> Self { + let pk = hex::decode(pk).unwrap(); + let msg = hex::decode(msg).unwrap(); + let sig = hex::decode(sig).unwrap(); + Self { + pk, + msg, + sig, + accept, + } + } + } + + let bip340_tests = [ + Bip340Test::new("F9308A019258C31049344F85F89D5229B531C845836F99B08601F113BCE036F9", "0000000000000000000000000000000000000000000000000000000000000000", "E907831F80848D1069A5371B402410364BDF1C5F8307B0084C55F1CE2DCA821525F66A4A85EA8B71E482A74F382D2CE5EBEEE8FDB2172F477DF4900D310536C0", true), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "6896BD60EEAE296DB48A229FF71DFE071BDE413E6D43F917DC8DCF8C78DE33418906D11AC976ABCCB20B091292BFF4EA897EFCB639EA871CFA95F6DE339E4B0A", true), + Bip340Test::new("DD308AFEC5777E13121FA72B9CC1B7CC0139715309B086C960E18FD969774EB8", "7E2D58D8B3BCDF1ABADEC7829054F90DDA9805AAB56C77333024B9D0A508B75C", "5831AAEED7B44BB74E5EAB94BA9D4294C49BCF2A60728D8B4C200F50DD313C1BAB745879A5AD954A72C45A91C3A51D3C7ADEA98D82F8481E0E1E03674A6F3FB7", true), + Bip340Test::new("25D1DFF95105F5253C4022F628A996AD3A0D95FBF21D468A1B33F8C160D8F517", "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", "7EB0509757E246F19449885651611CB965ECC1A187DD51B64FDA1EDC9637D5EC97582B9CB13DB3933705B32BA982AF5AF25FD78881EBB32771FC5922EFC66EA3", true), + Bip340Test::new("D69C3509BB99E412E68B0FE8544E72837DFA30746D8BE2AA65975F29D22DC7B9", "4DF3C3F68FCC83B27E9D42C90431A72499F17875C81A599B566C9889B9696703", "00000000000000000000003B78CE563F89A0ED9414F5AA28AD0D96D6795F9C6376AFB1548AF603B3EB45C9F8207DEE1060CB71C04E80F593060B07D28308D7F4", true), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "FFF97BD5755EEEA420453A14355235D382F6472F8568A18B2F057A14602975563CC27944640AC607CD107AE10923D9EF7A73C643E166BE5EBEAFA34B1AC553E2", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "1FA62E331EDBC21C394792D2AB1100A7B432B013DF3F6FF4F99FCB33E0E1515F28890B3EDB6E7189B630448B515CE4F8622A954CFE545735AAEA5134FCCDB2BD", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E177769961764B3AA9B2FFCB6EF947B6887A226E8D7C93E00C5ED0C1834FF0D0C2E6DA6", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "0000000000000000000000000000000000000000000000000000000000000000123DDA8328AF9C23A94C1FEECFD123BA4FB73476F0D594DCB65C6425BD186051", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "00000000000000000000000000000000000000000000000000000000000000017615FBAF5AE28864013C099742DEADB4DBA87F11AC6754F93780D5A1837CF197", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "4A298DACAE57395A15D0795DDBFD1DCB564DA82B0F269BC70A74F8220429BA1D69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B", false), + Bip340Test::new("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659", "243F6A8885A308D313198A2E03707344A4093822299F31D0082EFA98EC4E6C89", "6CFF5C3BA86C69EA4B7376F31A9BCB4F74C1976089B2D9963DA2E5543E177769FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", false), + ]; + + for tv in bip340_tests { + let pk = PublicKey::deserialize_bip340(&tv.pk).unwrap(); + assert_eq!(pk.verify_bip340_signature(&tv.msg, &tv.sig), tv.accept); + } +} + #[test] fn should_be_able_to_parse_openssl_rfc5915_format_key() { pub const SAMPLE_SECP256K1_PEM: &str = r#"-----BEGIN EC PRIVATE KEY----- From a9a9a82095c230999433409b0ac58c53ec83810a Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Thu, 8 Aug 2024 14:06:34 -0400 Subject: [PATCH 2/6] Fix a clippy --- rs/crypto/ecdsa_secp256k1/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/crypto/ecdsa_secp256k1/src/lib.rs b/rs/crypto/ecdsa_secp256k1/src/lib.rs index 3ab7ac1f2a6..efa8fc6705c 100644 --- a/rs/crypto/ecdsa_secp256k1/src/lib.rs +++ b/rs/crypto/ecdsa_secp256k1/src/lib.rs @@ -464,7 +464,7 @@ impl PrivateKey { */ let aux_rand = rng.gen::<[u8; 32]>(); if let Ok(sig) = bip340.sign_prehash_with_aux_rand(message, &aux_rand) { - return sig.to_bytes().into(); + return sig.to_bytes(); } } } From 94949d462d439e3b99a79d29b829d9de77568d47 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 9 Aug 2024 17:16:47 -0400 Subject: [PATCH 3/6] Rename functions, leaving compat forwards --- rs/crypto/ecdsa_secp256k1/src/lib.rs | 48 ++++++++++++++++--- rs/crypto/ecdsa_secp256k1/tests/tests.rs | 30 ++++++------ .../basic_sig/ecdsa_secp256k1/src/api.rs | 4 +- 3 files changed, 59 insertions(+), 23 deletions(-) diff --git a/rs/crypto/ecdsa_secp256k1/src/lib.rs b/rs/crypto/ecdsa_secp256k1/src/lib.rs index efa8fc6705c..a9c6d1299e4 100644 --- a/rs/crypto/ecdsa_secp256k1/src/lib.rs +++ b/rs/crypto/ecdsa_secp256k1/src/lib.rs @@ -416,11 +416,23 @@ impl PrivateKey { pem_encode(&self.serialize_rfc5915_der(), PEM_HEADER_RFC5915) } + #[deprecated(note="use sign_message_with_ecdsa")] + /// Deprecated alias of sign_message_with_ecdsa + pub fn sign_message(&self, message: &[u8]) -> [u8; 64] { + self.sign_message_with_ecdsa(message) + } + + #[deprecated(note="use sign_digest_with_ecdsa")] + /// Deprecated alias of sign_digest_with_ecdsa + pub fn sign_digest(&self, message: &[u8]) -> Option<[u8; 64]> { + self.sign_digest_with_ecdsa(message) + } + /// Sign a message with ECDSA /// /// The message is hashed with SHA-256 and the signature is /// normalized (using the minimum-s approach of BitCoin) - pub fn sign_message(&self, message: &[u8]) -> [u8; 64] { + pub fn sign_message_with_ecdsa(&self, message: &[u8]) -> [u8; 64] { use k256::ecdsa::{signature::Signer, Signature}; let ecdsa = k256::ecdsa::SigningKey::from(&self.key); @@ -431,7 +443,7 @@ impl PrivateKey { /// Sign a message digest with ECDSA /// /// The signature is normalized (using the minimum-s approach of BitCoin) - pub fn sign_digest(&self, digest: &[u8]) -> Option<[u8; 64]> { + pub fn sign_digest_with_ecdsa(&self, digest: &[u8]) -> Option<[u8; 64]> { if digest.len() < 16 { // k256 arbitrarily rejects digests that are < 128 bits return None; @@ -622,12 +634,36 @@ impl PublicKey { pem_encode(&self.serialize_der(), "PUBLIC KEY") } + #[deprecated(note="use verify_ecdsa_signature")] + /// Deprecated alias of verify_ecdsa_signature + pub fn verify_signature(&self, message: &[u8], signature: &[u8]) -> bool { + self.verify_ecdsa_signature(message, signature) + } + + #[deprecated(note="use verify_ecdsa_signature_with_malleability")] + /// Deprecated alias of verify_ecdsa_signature_with_malleability + pub fn verify_signature_with_malleability(&self, message: &[u8], signature: &[u8]) -> bool { + self.verify_ecdsa_signature_with_malleability(message, signature) + } + + #[deprecated(note="use verify_ecdsa_signature_prehashed")] + /// Deprecated alias of verify_ecdsa_signature_prehashed + pub fn verify_signature_prehashed(&self, digest: &[u8], signature: &[u8]) -> bool { + self.verify_ecdsa_signature_prehashed(digest, signature) + } + + #[deprecated(note="use verify_ecdsa_signature_prehashed_with_malleability")] + /// Deprecated alias of verify_ecdsa_signature_prehashed_with_malleability + pub fn verify_signature_prehashed_with_malleability(&self, digest: &[u8], signature: &[u8]) -> bool { + self.verify_ecdsa_signature_prehashed_with_malleability(digest, signature) + } + /// Verify a (message,signature) pair, requiring s-normalization /// /// If used to verify signatures generated by a library that does not /// perform s-normalization, this function will reject roughly half of all /// signatures. - pub fn verify_signature(&self, message: &[u8], signature: &[u8]) -> bool { + pub fn verify_ecdsa_signature(&self, message: &[u8], signature: &[u8]) -> bool { use k256::ecdsa::signature::Verifier; let signature = match k256::ecdsa::Signature::try_from(signature) { Ok(sig) => sig, @@ -652,7 +688,7 @@ impl PublicKey { /// This means that given a valid ECDSA signature, it is possible to create /// a "new" ECDSA signature that is also valid, without having access to the /// key. Unlike `verify_signature`, this function accepts either `s` value. - pub fn verify_signature_with_malleability(&self, message: &[u8], signature: &[u8]) -> bool { + pub fn verify_ecdsa_signature_with_malleability(&self, message: &[u8], signature: &[u8]) -> bool { use k256::ecdsa::signature::Verifier; let signature = match k256::ecdsa::Signature::try_from(signature) { Ok(sig) => sig, @@ -668,7 +704,7 @@ impl PublicKey { } /// Verify a (message digest,signature) pair - pub fn verify_signature_prehashed(&self, digest: &[u8], signature: &[u8]) -> bool { + pub fn verify_ecdsa_signature_prehashed(&self, digest: &[u8], signature: &[u8]) -> bool { use k256::ecdsa::signature::hazmat::PrehashVerifier; let signature = match k256::ecdsa::Signature::try_from(signature) { @@ -692,7 +728,7 @@ impl PublicKey { /// This means that given a valid ECDSA signature, it is possible to create /// a "new" ECDSA signature, without having access to the key. Unlike /// `verify_signature_prehashed`, this function accepts either `s` value. - pub fn verify_signature_prehashed_with_malleability( + pub fn verify_ecdsa_signature_prehashed_with_malleability( &self, digest: &[u8], signature: &[u8], diff --git a/rs/crypto/ecdsa_secp256k1/tests/tests.rs b/rs/crypto/ecdsa_secp256k1/tests/tests.rs index 0538c8eaf93..dc44b5e3ace 100644 --- a/rs/crypto/ecdsa_secp256k1/tests/tests.rs +++ b/rs/crypto/ecdsa_secp256k1/tests/tests.rs @@ -31,7 +31,7 @@ fn should_pass_wycheproof_ecdsa_secp256k1_verification_tests() -> Result<(), Key for test in &test_group.tests { // The Wycheproof ECDSA tests do not normalize s so we must use // the verification method that accepts either valid s - let accepted = pk.verify_signature_with_malleability(&test.msg, &test.sig); + let accepted = pk.verify_ecdsa_signature_with_malleability(&test.msg, &test.sig); assert_eq!(accepted, test.result == wycheproof::TestResult::Valid); } } @@ -48,13 +48,13 @@ fn test_sign_prehash_works_with_any_size_input_gte_16() { for i in 0..16 { let buf = vec![0x42; i]; - assert_eq!(sk.sign_digest(&buf), None); + assert_eq!(sk.sign_digest_with_ecdsa(&buf), None); } for i in 16..1024 { let buf = vec![0x42; i]; - let sig = sk.sign_digest(&buf).unwrap(); - assert!(pk.verify_signature_prehashed(&buf, &sig)); + let sig = sk.sign_digest_with_ecdsa(&buf).unwrap(); + assert!(pk.verify_ecdsa_signature_prehashed(&buf, &sig)); } } @@ -73,7 +73,7 @@ fn should_use_rfc6979_nonces_for_ecdsa_signature_generation() { let message = b"abc"; let expected_sig = "d8bdb0ddfc8ebb8be42649048e92edc8547d1587b2a8f721738a2ecc0733401c70e86d3042ebbb50dccfbfbdf6c0462c7be45bcd0208d33e34efec273a86eab9"; - let generated_sig = sk.sign_message(message); + let generated_sig = sk.sign_message_with_ecdsa(message); assert_eq!(hex::encode(generated_sig), expected_sig); // Now check the prehash variant: @@ -83,7 +83,7 @@ fn should_use_rfc6979_nonces_for_ecdsa_signature_generation() { sha256.update(message); sha256.finalize().into() }; - let generated_sig = sk.sign_digest(&message_hash).unwrap(); + let generated_sig = sk.sign_digest_with_ecdsa(&message_hash).unwrap(); assert_eq!(hex::encode(generated_sig), expected_sig); } @@ -115,16 +115,16 @@ fn should_accept_ecdsa_signatures_that_we_generate() { for m in 0..100 { let mut msg = vec![0u8; m]; rng.fill_bytes(&mut msg); - let sig = sk.sign_message(&msg); + let sig = sk.sign_message_with_ecdsa(&msg); assert_eq!( - sk.sign_message(&msg), + sk.sign_message_with_ecdsa(&msg), sig, "ECDSA signature generation is deterministic" ); - assert!(pk.verify_signature(&msg, &sig)); - assert!(pk.verify_signature_with_malleability(&msg, &sig)); + assert!(pk.verify_ecdsa_signature(&msg, &sig)); + assert!(pk.verify_ecdsa_signature_with_malleability(&msg, &sig)); } } @@ -154,8 +154,8 @@ fn should_reject_high_s_in_signature_unless_malleable() -> Result<(), KeyDecodin let msg = b"test"; let sig = hex::decode("6471F8E5E63D6055AA6F6D3A8EBF49935D1316D6A54B9B09465B3BEB38E3AC14CE0FFBABD8E3248BEEBD568DCBCC7861126B1AB88E721D0206E9D67ECD878C7C").unwrap(); - assert!(!pk.verify_signature(msg, &sig)); - assert!(pk.verify_signature_with_malleability(msg, &sig)); + assert!(!pk.verify_ecdsa_signature(msg, &sig)); + assert!(pk.verify_ecdsa_signature_with_malleability(msg, &sig)); // Test again using the pre-hashed variants: let msg_hash: [u8; 32] = { @@ -165,8 +165,8 @@ fn should_reject_high_s_in_signature_unless_malleable() -> Result<(), KeyDecodin sha256.finalize().into() }; - assert!(!pk.verify_signature_prehashed(&msg_hash, &sig)); - assert!(pk.verify_signature_prehashed_with_malleability(&msg_hash, &sig)); + assert!(!pk.verify_ecdsa_signature_prehashed(&msg_hash, &sig)); + assert!(pk.verify_ecdsa_signature_prehashed_with_malleability(&msg_hash, &sig)); Ok(()) } @@ -492,7 +492,7 @@ mod try_recovery_from_digest { let public_key = private_key.public_key(); let digest = rng.gen::<[u8; 32]>(); let signature = private_key - .sign_digest(&digest) + .sign_digest_with_ecdsa(&digest) .expect("cannot fail because digest > 16 bytes"); let recid = public_key diff --git a/rs/crypto/internal/crypto_lib/basic_sig/ecdsa_secp256k1/src/api.rs b/rs/crypto/internal/crypto_lib/basic_sig/ecdsa_secp256k1/src/api.rs index d078ea91b36..f12b05a89d1 100644 --- a/rs/crypto/internal/crypto_lib/basic_sig/ecdsa_secp256k1/src/api.rs +++ b/rs/crypto/internal/crypto_lib/basic_sig/ecdsa_secp256k1/src/api.rs @@ -120,7 +120,7 @@ pub fn sign(msg: &[u8], sk: &types::SecretKeyBytes) -> CryptoResult Ok(()), false => Err(CryptoError::SignatureVerification { algorithm: AlgorithmId::EcdsaSecp256k1, From c1a7c10864a5f121db6203c32fdbc450b6760378 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 9 Aug 2024 18:17:19 -0400 Subject: [PATCH 4/6] Revert deprecation --- rs/crypto/ecdsa_secp256k1/src/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/rs/crypto/ecdsa_secp256k1/src/lib.rs b/rs/crypto/ecdsa_secp256k1/src/lib.rs index a9c6d1299e4..b25a021b45c 100644 --- a/rs/crypto/ecdsa_secp256k1/src/lib.rs +++ b/rs/crypto/ecdsa_secp256k1/src/lib.rs @@ -416,13 +416,11 @@ impl PrivateKey { pem_encode(&self.serialize_rfc5915_der(), PEM_HEADER_RFC5915) } - #[deprecated(note="use sign_message_with_ecdsa")] /// Deprecated alias of sign_message_with_ecdsa pub fn sign_message(&self, message: &[u8]) -> [u8; 64] { self.sign_message_with_ecdsa(message) } - #[deprecated(note="use sign_digest_with_ecdsa")] /// Deprecated alias of sign_digest_with_ecdsa pub fn sign_digest(&self, message: &[u8]) -> Option<[u8; 64]> { self.sign_digest_with_ecdsa(message) @@ -634,25 +632,21 @@ impl PublicKey { pem_encode(&self.serialize_der(), "PUBLIC KEY") } - #[deprecated(note="use verify_ecdsa_signature")] /// Deprecated alias of verify_ecdsa_signature pub fn verify_signature(&self, message: &[u8], signature: &[u8]) -> bool { self.verify_ecdsa_signature(message, signature) } - #[deprecated(note="use verify_ecdsa_signature_with_malleability")] /// Deprecated alias of verify_ecdsa_signature_with_malleability pub fn verify_signature_with_malleability(&self, message: &[u8], signature: &[u8]) -> bool { self.verify_ecdsa_signature_with_malleability(message, signature) } - #[deprecated(note="use verify_ecdsa_signature_prehashed")] /// Deprecated alias of verify_ecdsa_signature_prehashed pub fn verify_signature_prehashed(&self, digest: &[u8], signature: &[u8]) -> bool { self.verify_ecdsa_signature_prehashed(digest, signature) } - #[deprecated(note="use verify_ecdsa_signature_prehashed_with_malleability")] /// Deprecated alias of verify_ecdsa_signature_prehashed_with_malleability pub fn verify_signature_prehashed_with_malleability(&self, digest: &[u8], signature: &[u8]) -> bool { self.verify_ecdsa_signature_prehashed_with_malleability(digest, signature) From 50d43bcaa69d7fc63fc09a08042a188a11ba63e6 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 9 Aug 2024 18:19:32 -0400 Subject: [PATCH 5/6] fmt --- rs/crypto/ecdsa_secp256k1/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/rs/crypto/ecdsa_secp256k1/src/lib.rs b/rs/crypto/ecdsa_secp256k1/src/lib.rs index b25a021b45c..dda7e922186 100644 --- a/rs/crypto/ecdsa_secp256k1/src/lib.rs +++ b/rs/crypto/ecdsa_secp256k1/src/lib.rs @@ -648,7 +648,11 @@ impl PublicKey { } /// Deprecated alias of verify_ecdsa_signature_prehashed_with_malleability - pub fn verify_signature_prehashed_with_malleability(&self, digest: &[u8], signature: &[u8]) -> bool { + pub fn verify_signature_prehashed_with_malleability( + &self, + digest: &[u8], + signature: &[u8], + ) -> bool { self.verify_ecdsa_signature_prehashed_with_malleability(digest, signature) } @@ -682,7 +686,11 @@ impl PublicKey { /// This means that given a valid ECDSA signature, it is possible to create /// a "new" ECDSA signature that is also valid, without having access to the /// key. Unlike `verify_signature`, this function accepts either `s` value. - pub fn verify_ecdsa_signature_with_malleability(&self, message: &[u8], signature: &[u8]) -> bool { + pub fn verify_ecdsa_signature_with_malleability( + &self, + message: &[u8], + signature: &[u8], + ) -> bool { use k256::ecdsa::signature::Verifier; let signature = match k256::ecdsa::Signature::try_from(signature) { Ok(sig) => sig, From cc661df6e2af9d3a763af5551eb087dad3872f6b Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 9 Aug 2024 18:20:13 -0400 Subject: [PATCH 6/6] Randomize test input --- rs/crypto/ecdsa_secp256k1/tests/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/crypto/ecdsa_secp256k1/tests/tests.rs b/rs/crypto/ecdsa_secp256k1/tests/tests.rs index dc44b5e3ace..670a812b196 100644 --- a/rs/crypto/ecdsa_secp256k1/tests/tests.rs +++ b/rs/crypto/ecdsa_secp256k1/tests/tests.rs @@ -140,7 +140,7 @@ fn should_accept_bip340_signatures_that_we_generate() { let pk = sk.public_key(); println!("{}", hex::encode(pk.serialize_sec1(true))); - let mut msg = [0u8; 32]; + let mut msg = rng.gen::<[u8; 32]>(); rng.fill_bytes(&mut msg); let sig = sk.sign_bip340(&msg, &mut rng);