Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: eip-191 hash calculation fix #2248

Merged
merged 1 commit into from
Jan 3, 2025
Merged

bugfix: eip-191 hash calculation fix #2248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 14 additions & 36 deletions common/primitives/src/signatures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use frame_support::{
__private::{codec, RuntimeDebug},
pallet_prelude::{Decode, Encode, MaxEncodedLen, TypeInfo},
};
use scale_info::prelude::format;
use parity_scale_codec::alloc::string::ToString;
use sp_core::{
crypto,
crypto::{AccountId32, FromEntropy},
Expand All @@ -19,6 +19,9 @@ use sp_runtime::{
MultiSignature,
};

/// Ethereum message prefix eip-191
const ETHEREUM_MESSAGE_PREFIX: &[u8; 26] = b"\x19Ethereum Signed Message:\n";

/// Signature verify that can work with any known signature types.
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[derive(Eq, PartialEq, Clone, Encode, Decode, MaxEncodedLen, RuntimeDebug, TypeInfo)]
Expand Down Expand Up @@ -254,10 +257,11 @@ fn check_secp256k1_signature(signature: &[u8; 65], msg: &[u8; 32], signer: &Acco
}
}

fn eth_message_hash(message: &str) -> [u8; 32] {
let prefixed = format!("{}{}{}", "\x19Ethereum Signed Message:\n", message.len(), message);
log::debug!(target:"ETHEREUM", "prefixed {:?}",prefixed);
sp_io::hashing::keccak_256(prefixed.as_bytes())
fn eth_message_hash(message: &[u8]) -> [u8; 32] {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matches https://github.com/ethers-io/ethers.js/blob/48cdcef2925ffca4f60412697791dac268f06d0d/src.ts/hash/message.ts#L35

let only_len = (message.len() as u32).to_string().into_bytes();
let concatenated = [ETHEREUM_MESSAGE_PREFIX.as_slice(), only_len.as_slice(), message].concat();
log::debug!(target:"ETHEREUM", "prefixed {:?}",concatenated);
sp_io::hashing::keccak_256(concatenated.as_slice())
}

fn check_ethereum_signature<L: Lazy<[u8]>>(
Expand All @@ -270,21 +274,14 @@ fn check_ethereum_signature<L: Lazy<[u8]>>(
};

// signature of ethereum prefixed message eip-191
let message_prefixed = eth_message_hash(&format!("0x{:?}", HexDisplay::from(&msg.get())));
let message_prefixed = eth_message_hash(&msg.get());
if verify_signature(&signature.as_ref(), &message_prefixed, signer) {
return true
}

// signature of raw payload, compatible with polkadotJs signatures
let hashed = sp_io::hashing::keccak_256(&msg.get());
if verify_signature(signature.as_ref(), &hashed, signer) {
return true
}

// frequency wrapped for Metamask compatibility
let frequency_wrapped =
eth_message_hash(&format!("<Frequency>0x{:?}</Frequency>", HexDisplay::from(&msg.get())));
verify_signature(&signature.as_ref(), &frequency_wrapped, signer)
Comment on lines -284 to -287
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this custom wrapping for now. We might not need it and we will cross that bridge in future in case it was necessary.

verify_signature(signature.as_ref(), &hashed, signer)
}

#[cfg(test)]
Expand All @@ -308,14 +305,14 @@ mod tests {
#[test]
fn ethereum_prefixed_eip191_signatures_should_work() {
// payload is random and the signature is generated over that payload by a standard EIP-191 signer
let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe454c1f285c69f563934857a63463571d57723fbad6ac7de44611ed674f02c04c2ae00").expect("Should convert");
let signature_raw = from_hex("0x056ca64d31251a1f20733ce2a741e2963c87a9674a35f8619b6b97210ae8c8b54c2853da1b943dd95ac3b893b37f69ca7dc38c13f8ec92a235b00ae03426505900").expect("Should convert");
let payload = b"test eip-191 message payload";
let signature_raw = from_hex("0x276dcc9c69da24dd8441ba3acc9b60d8aae0cb39f0bc5ad92c723a31bf11575031d860978280191a0a97a1f74336ca0c79a8b1b3aab013fb58a27f113b73b2081b").expect("Should convert");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this signature is generated using metamask and the payload above

let unified_signature = UnifiedSignature::from(ecdsa::Signature::from_raw(
signature_raw.try_into().expect("should convert"),
));

let public_key = ecdsa::Public::from_raw(
from_hex("0x025b107c7f38d5ac7d618e626f9fa57eec683adf373b1352cd20e5e5c684747079")
from_hex("0x03be5b145e12c5fb95151374ed919eb445ade57637d729dd2d73bf161d4bc10329")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compressed public key associated with the account used in metamask for signing 0x6906405649a9c73f28BB3D9bC3B647280D822025

.expect("should convert")
.try_into()
.expect("invalid size"),
Expand Down Expand Up @@ -343,25 +340,6 @@ mod tests {
assert!(unified_signature.verify(&payload[..], &unified_signer.into_account()));
}

#[test]
fn ethereum_custom_wrapped_signatures_should_work() {
// payload is random and the signature is generated over that payload by Metamask signer
let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe4548de1bcb12a1d42e58b218a7abb03cb629111625cf3449640d837c5aa98b87d8e00").expect("Should convert");
let signature_raw = from_hex("0x9633e747bcd951bdb9d98ff84c65562e1f62bd059c578a942859e1695f2472aa0dbaab48c28f6dbc795baa73c27252d97e8dc2170fd7d69694d5cd1863fb968c00").expect("Should convert");
let unified_signature = UnifiedSignature::from(ecdsa::Signature::from_raw(
signature_raw.try_into().expect("should convert"),
));

let public_key = ecdsa::Public::from_raw(
from_hex("0x025b107c7f38d5ac7d618e626f9fa57eec683adf373b1352cd20e5e5c684747079")
.expect("should convert")
.try_into()
.expect("invalid size"),
);
let unified_signer = UnifiedSigner::from(public_key);
assert!(unified_signature.verify(&payload[..], &unified_signer.into_account()));
}

#[test]
fn ethereum_invalid_signatures_should_fail() {
let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe4548de1bcb12a1d42e58b218a7abb03cb629111625cf3449640d837c5aa98b87d8e00").expect("Should convert");
Expand Down
2 changes: 1 addition & 1 deletion e2e/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 36 additions & 1 deletion pallets/passkey/src/tests_v2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::test_common::{
utilities::*,
};
use pallet_balances::Call as BalancesCall;
use sp_core::{sr25519, sr25519::Public, Pair};
use sp_core::{bytes::from_hex, ecdsa, sr25519, sr25519::Public, Pair};
use sp_runtime::{traits::One, DispatchError::BadOrigin};

struct TestPasskeyPayloadBuilder {
Expand Down Expand Up @@ -523,3 +523,38 @@ fn pre_dispatch_with_exceeding_weight_should_fail() {
assert_err!(v, InvalidTransaction::ExhaustsResources);
});
}

#[test]
fn passkey_example_should_work() {
new_test_ext().execute_with(|| {
// arrange
let account_id = AccountId32::new(from_hex("0x000000000000000000000000cf613044ccd8c1c60f561b99bd1fd2daef89625f").unwrap().try_into().unwrap());
let pass_key_public_key = PasskeyPublicKey(from_hex("0x029bd263885e5eeaea31fa3b2e78ab1106d2cb1995045777fca3b38913a755d250").unwrap().try_into().unwrap());
let payload = PasskeyPayloadV2 {
passkey_public_key: pass_key_public_key,
verifiable_passkey_signature: VerifiablePasskeySignature {
signature: from_hex("0x30440220498c9d752c98f4cec77bb386f48dc52ccc6523f386c8942b3d50ff9dbdb4046a02205d2f2e3fa8bf27fd5c8f0d7f1ae8724a30dab1dac758019459114a447357c54b").unwrap().try_into().unwrap(),
authenticator_data: from_hex("0x49960de5880e8c687434170f6476605b8fe4aeb9a28632c7995cf3ba831d97630500000003").unwrap().try_into().unwrap(),
client_data_json: from_hex("0x7b2274797065223a22776562617574686e2e676574222c226368616c6c656e6765223a227372576f2d4a49393564336332393362774e756469747a53687a4f636849577a584d336478376d554f4351222c226f726967696e223a22687474703a2f2f6c6f63616c686f73743a3536353135222c2263726f73734f726967696e223a66616c73657d").unwrap().try_into().unwrap(),
},
account_ownership_proof: MultiSignature::Ecdsa(ecdsa::Signature::from_raw(from_hex("0xa0d9b3fe775b4be9d2b52f37f8a21c30be037a858f85cca82bce58ac05a95d9621063978dc086a83532e839882e701f2b7134541a4814526c888b8650c3eb5f81b").unwrap().try_into().unwrap())),
passkey_call: PasskeyCallV2 {
account_id: account_id.clone(),
account_nonce: 0,
call: Box::new(RuntimeCall::System(SystemCall::remark { remark: vec![1, 2, 3u8] })),
},
};
assert_ok!(Balances::force_set_balance(
RawOrigin::Root.into(),
account_id.into(),
10000000000
));

// act
let res =
Passkey::validate_unsigned(TransactionSource::InBlock, &Call::proxy_v2 { payload });

// assert
assert!(res.is_ok());
});
}
4 changes: 2 additions & 2 deletions runtime/frequency/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("frequency"),
impl_name: create_runtime_str!("frequency"),
authoring_version: 1,
spec_version: 140,
spec_version: 141,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand All @@ -415,7 +415,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("frequency-testnet"),
impl_name: create_runtime_str!("frequency"),
authoring_version: 1,
spec_version: 140,
spec_version: 141,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down
Loading