diff --git a/corelib/src/starknet/secp256_trait.cairo b/corelib/src/starknet/secp256_trait.cairo index fe8c0ced65b..dafcf42de75 100644 --- a/corelib/src/starknet/secp256_trait.cairo +++ b/corelib/src/starknet/secp256_trait.cairo @@ -1,5 +1,4 @@ use array::ArrayTrait; -use keccak::keccak_u256s_be_inputs; use math::{u256_mul_mod_n, inv_mod}; use option::OptionTrait; use starknet::{eth_address::U256IntoEthAddress, EthAddress, SyscallResult, SyscallResultTrait}; @@ -35,6 +34,9 @@ trait Secp256Trait { fn secp256_ec_get_point_from_x_syscall( x: u256, y_parity: bool ) -> SyscallResult>; + fn secp256_ec_get_point_from_xy_syscall( + x: u256, y: u256 + ) -> SyscallResult>; } trait Secp256PointTrait { @@ -43,6 +45,43 @@ trait Secp256PointTrait { fn mul(self: Secp256Point, scalar: u256) -> SyscallResult; } +/// Checks whether `value` is in the range [1, N), where N is the size of the curve. +fn is_signature_entry_valid< + Secp256Point, +Drop, impl Secp256Impl: Secp256Trait +>( + value: u256 +) -> bool { + value != 0_u256 && value < Secp256Impl::get_curve_size() +} + +fn verify_signature< + Secp256Point, + +Drop, + impl Secp256Impl: Secp256Trait, + +Secp256PointTrait +>( + msg_hash: u256, r: u256, s: u256, public_key: Secp256Point +) -> Result { + if !is_signature_entry_valid::(r) { + return Result::Err('Signature out of range'); + } + if !is_signature_entry_valid::(s) { + return Result::Err('Signature out of range'); + } + + let n_nz = Secp256Impl::get_curve_size().try_into().unwrap(); + let s_inv = inv_mod(s.try_into().unwrap(), n_nz).unwrap(); + let u1 = u256_mul_mod_n(msg_hash, s_inv, n_nz); + let u2 = u256_mul_mod_n(r, s_inv, n_nz); + + let generator_point = Secp256Impl::get_generator_point(); + let point1 = generator_point.mul(u1).unwrap_syscall(); + let point2 = public_key.mul(u2).unwrap_syscall(); + let sum = point1.add(point2).unwrap_syscall(); + + let (x, y) = sum.get_coordinates().unwrap_syscall(); + return Result::Ok(x == r); +} /// Receives a signature and the signed message hash. /// Returns the public key associated with the signer, represented as a point on the curve. @@ -85,69 +124,3 @@ fn secp256_ec_negate_scalar< ) -> u256 { Secp256Impl::get_curve_size() - c } - - -/// Checks a Secp256 ECDSA signature. -/// Also verifies that r and s components of the signature are in the range (0, N), -/// where N is the size of the curve. -/// Returns a Result with an error string if the signature is invalid. -fn is_eth_signature_valid< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - msg_hash: u256, signature: Signature, eth_address: EthAddress -) -> Result<(), felt252> { - if !is_signature_entry_valid::(signature.r) { - return Result::Err('Signature out of range'); - } - if !is_signature_entry_valid::(signature.s) { - return Result::Err('Signature out of range'); - } - - let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); - let calculated_eth_address = public_key_point_to_eth_address(:public_key_point); - if eth_address != calculated_eth_address { - return Result::Err('Invalid signature'); - } - Result::Ok(()) -} - -/// Asserts that a Secp256 ECDSA signature is valid. -/// Also verifies that r and s components of the signature are in the range (0, N), -/// where N is the size of the curve. -fn verify_eth_signature< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - msg_hash: u256, signature: Signature, eth_address: EthAddress -) { - match is_eth_signature_valid::(:msg_hash, :signature, :eth_address) { - Result::Ok(()) => {}, - Result::Err(err) => panic_with_felt252(err), - } -} - -/// Checks whether `value` is in the range [1, N), where N is the size of the curve. -fn is_signature_entry_valid< - Secp256Point, +Drop, impl Secp256Impl: Secp256Trait ->( - value: u256 -) -> bool { - value != 0_u256 && value < Secp256Impl::get_curve_size() -} - -/// Converts a public key point to the corresponding Ethereum address. -fn public_key_point_to_eth_address< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - public_key_point: Secp256Point -) -> EthAddress { - let (x, y) = public_key_point.get_coordinates().unwrap_syscall(); - - // Keccak output is little endian. - let point_hash_le = keccak_u256s_be_inputs(array![x, y].span()); - let point_hash = u256 { - low: integer::u128_byte_reverse(point_hash_le.high), - high: integer::u128_byte_reverse(point_hash_le.low) - }; - - point_hash.into() -} diff --git a/corelib/src/starknet/secp256k1.cairo b/corelib/src/starknet/secp256k1.cairo index 7030844f6de..38e88764b72 100644 --- a/corelib/src/starknet/secp256k1.cairo +++ b/corelib/src/starknet/secp256k1.cairo @@ -3,8 +3,14 @@ use option::OptionTrait; use starknet::{ - EthAddress, secp256_trait::{Secp256Trait, Secp256PointTrait}, SyscallResult, SyscallResultTrait + EthAddress, + secp256_trait::{ + Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature + }, + SyscallResult, SyscallResultTrait }; +use keccak::keccak_u256s_be_inputs; + #[derive(Copy, Drop)] extern type Secp256k1Point; @@ -32,6 +38,17 @@ impl Secp256k1Impl of Secp256Trait { ) -> SyscallResult> { secp256k1_get_point_from_x_syscall(x, y_parity) } + fn secp256_ec_get_point_from_xy_syscall( + x: u256, y: u256 + ) -> SyscallResult> { + let point = secp256k1_get_point_from_x_syscall(x, false).unwrap_syscall().unwrap(); + let (_, point_y) = point.get_coordinates().unwrap_syscall(); + if (point_y == y) { + return SyscallResult::Ok(Option::Some(point)); + } else { + return secp256k1_get_point_from_x_syscall(x, true); + } + } } impl Secp256k1PointImpl of Secp256PointTrait { @@ -72,3 +89,54 @@ extern fn secp256k1_get_point_from_x_syscall( extern fn secp256k1_get_xy_syscall( p: Secp256k1Point ) -> SyscallResult<(u256, u256)> implicits(GasBuiltin, System) nopanic; + +/// Asserts that a Secp256 ECDSA signature is valid. +/// Also verifies that r and s components of the signature are in the range (0, N), +/// where N is the size of the curve. +fn verify_eth_signature(msg_hash: u256, signature: Signature, eth_address: EthAddress) { + match is_eth_signature_valid(:msg_hash, :signature, :eth_address) { + Result::Ok(()) => {}, + Result::Err(err) => panic_with_felt252(err), + } +} + +/// Checks a Secp256 ECDSA signature. +/// Also verifies that r and s components of the signature are in the range (0, N), +/// where N is the size of the curve. +/// Returns a Result with an error string if the signature is invalid. +fn is_eth_signature_valid( + msg_hash: u256, signature: Signature, eth_address: EthAddress +) -> Result<(), felt252> { + if !is_signature_entry_valid::(signature.r) { + return Result::Err('Signature out of range'); + } + if !is_signature_entry_valid::(signature.s) { + return Result::Err('Signature out of range'); + } + + let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); + let calculated_eth_address = public_key_point_to_eth_address(:public_key_point); + if eth_address != calculated_eth_address { + return Result::Err('Invalid signature'); + } + Result::Ok(()) +} + + +/// Converts a public key point to the corresponding Ethereum address. +fn public_key_point_to_eth_address< + Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait +>( + public_key_point: Secp256Point +) -> EthAddress { + let (x, y) = public_key_point.get_coordinates().unwrap_syscall(); + + // Keccak output is little endian. + let point_hash_le = keccak_u256s_be_inputs(array![x, y].span()); + let point_hash = u256 { + low: integer::u128_byte_reverse(point_hash_le.high), + high: integer::u128_byte_reverse(point_hash_le.low) + }; + + point_hash.into() +} diff --git a/corelib/src/starknet/secp256r1.cairo b/corelib/src/starknet/secp256r1.cairo index 65fdfcc86a6..44ec6059c3a 100644 --- a/corelib/src/starknet/secp256r1.cairo +++ b/corelib/src/starknet/secp256r1.cairo @@ -32,6 +32,17 @@ impl Secp256r1Impl of Secp256Trait { ) -> SyscallResult> { secp256r1_get_point_from_x_syscall(x, y_parity) } + fn secp256_ec_get_point_from_xy_syscall( + x: u256, y: u256 + ) -> SyscallResult> { + let point = secp256r1_get_point_from_x_syscall(x, false).unwrap_syscall().unwrap(); + let (_, point_y) = point.get_coordinates().unwrap_syscall(); + if (point_y == y) { + return SyscallResult::Ok(Option::Some(point)); + } else { + return secp256r1_get_point_from_x_syscall(x, true); + } + } } impl Secp256r1PointImpl of Secp256PointTrait { diff --git a/corelib/src/test/secp256k1_test.cairo b/corelib/src/test/secp256k1_test.cairo index 537cd9bff2a..e551b617273 100644 --- a/corelib/src/test/secp256k1_test.cairo +++ b/corelib/src/test/secp256k1_test.cairo @@ -2,9 +2,9 @@ use starknet::{ eth_address::U256IntoEthAddress, EthAddress, secp256k1::Secp256k1Impl, SyscallResultTrait }; use starknet::secp256_trait::{ - Signature, recover_public_key, verify_eth_signature, Secp256PointTrait, signature_from_vrs + Signature, recover_public_key, Secp256PointTrait, signature_from_vrs, verify_signature }; -use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl}; +use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl, verify_eth_signature}; #[test] #[available_gas(100000000)] @@ -70,7 +70,7 @@ fn test_verify_eth_signature() { get_message_and_signature( :y_parity ); - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); } #[test] @@ -83,7 +83,7 @@ fn test_verify_eth_signature_wrong_eth_address() { :y_parity ); let eth_address = (eth_address.into() + 1).try_into().unwrap(); - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); } #[test] @@ -96,7 +96,7 @@ fn test_verify_eth_signature_overflowing_signature_r() { :y_parity ); signature.r = Secp256k1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); } #[test] @@ -109,5 +109,37 @@ fn test_verify_eth_signature_overflowing_signature_s() { :y_parity ); signature.s = Secp256k1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); +} + +#[test] +#[available_gas(100000000)] +fn test_verify_signature() { + let (msg_hash, signature, public_key_x, public_key_y, _) = get_message_and_signature(false); + + let public_key = Secp256k1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + .unwrap_syscall() + .unwrap(); + + let is_valid = verify_signature::( + msg_hash, signature.r, signature.s, public_key + ) + .unwrap(); + assert(is_valid, 'Signature should be valid'); +} + +#[test] +#[available_gas(100000000)] +fn test_verify_signature_invalid_signature() { + let (msg_hash, signature, public_key_x, public_key_y, _) = get_message_and_signature(false); + + let public_key = Secp256k1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + .unwrap_syscall() + .unwrap(); + + let is_valid = verify_signature::( + msg_hash, signature.r + 1, signature.s, public_key + ) + .unwrap(); + assert(!is_valid, 'Signature should be invalid'); } diff --git a/corelib/src/test/secp256r1_test.cairo b/corelib/src/test/secp256r1_test.cairo index ef1bdb3ae12..84306862b28 100644 --- a/corelib/src/test/secp256r1_test.cairo +++ b/corelib/src/test/secp256r1_test.cairo @@ -1,9 +1,5 @@ -use starknet::{ - eth_address::U256IntoEthAddress, EthAddress, secp256r1::Secp256r1Impl, SyscallResultTrait -}; -use starknet::secp256_trait::{ - recover_public_key, verify_eth_signature, Secp256PointTrait, Signature -}; +use starknet::{secp256r1::Secp256r1Impl, SyscallResultTrait}; +use starknet::secp256_trait::{recover_public_key, Secp256PointTrait, Signature, verify_signature}; use starknet::secp256r1::{Secp256r1Point, Secp256r1PointImpl}; use test::test_utils::assert_eq; @@ -20,7 +16,7 @@ fn test_secp256r1_recover_public_key() { /// Returns a golden valid message hash and its signature, for testing. -fn get_message_and_signature() -> (u256, Signature, u256, u256, EthAddress) { +fn get_message_and_signature() -> (u256, Signature, u256, u256, Secp256r1Point) { // msg = "" // public key: (0x04aaec73635726f213fb8a9e64da3b8632e41495a944d0045b522eba7240fad5, // 0x0087d9315798aaa3a5ba01775787ced05eaaf7b4e09fc81d6d1aa546e8365d525d) @@ -32,47 +28,60 @@ fn get_message_and_signature() -> (u256, Signature, u256, u256, EthAddress) { 0x04aaec73635726f213fb8a9e64da3b8632e41495a944d0045b522eba7240fad5, 0x0087d9315798aaa3a5ba01775787ced05eaaf7b4e09fc81d6d1aa546e8365d525d ); - let eth_address = 0x492882426e1cda979008bfaf874ff796eb3bb1c0_u256.into(); - (msg_hash, Signature { r, s, y_parity: true }, public_key_x, public_key_y, eth_address) + let public_key = Secp256r1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + .unwrap_syscall() + .unwrap(); + + (msg_hash, Signature { r, s, y_parity: true }, public_key_x, public_key_y, public_key) } #[test] #[available_gas(100000000)] -fn test_verify_eth_signature() { - let (msg_hash, signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - verify_eth_signature::(:msg_hash, :signature, :eth_address); +fn test_verify_signature() { + let (msg_hash, signature, _, _, public_key) = get_message_and_signature(); + let is_valid = verify_signature::( + msg_hash, signature.r, signature.s, public_key + ) + .unwrap(); + assert(is_valid, 'Signature should be valid'); } #[test] -#[should_panic(expected: ('Invalid signature',))] #[available_gas(100000000)] -fn test_verify_eth_signature_wrong_eth_address() { - let (msg_hash, signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - let eth_address = (eth_address.into() + 1).try_into().unwrap(); - verify_eth_signature::(:msg_hash, :signature, :eth_address); +fn test_verify_signature_invalid_signature() { + let (msg_hash, signature, _, _, public_key) = get_message_and_signature(); + let is_valid = verify_signature::( + msg_hash, signature.r + 1, signature.s, public_key + ) + .unwrap(); + assert(!is_valid, 'Signature should be invalid'); } #[test] #[should_panic(expected: ('Signature out of range',))] #[available_gas(100000000)] -fn test_verify_eth_signature_overflowing_signature_r() { - let (msg_hash, mut signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - signature.r = Secp256r1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); +fn test_verify_signature_overflowing_signature_r() { + let (msg_hash, mut signature, _, _, public_key) = get_message_and_signature(); + match verify_signature::( + msg_hash, Secp256r1Impl::get_curve_size() + 1, signature.s, public_key + ) { + Result::Ok(_) => {}, + Result::Err(err) => panic_with_felt252(err) + } } #[test] #[should_panic(expected: ('Signature out of range',))] #[available_gas(100000000)] -fn test_verify_eth_signature_overflowing_signature_s() { - let (msg_hash, mut signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - signature.s = Secp256r1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); +fn test_verify_signature_overflowing_signature_s() { + let (msg_hash, mut signature, _, _, public_key) = get_message_and_signature(); + match verify_signature::( + msg_hash, signature.r, Secp256r1Impl::get_curve_size() + 1, public_key + ) { + Result::Ok(_) => {}, + Result::Err(err) => panic_with_felt252(err) + } }