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.

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

Passkey: check account signature on passkey #2036

Merged
merged 15 commits into from
Jun 24, 2024
2 changes: 2 additions & 0 deletions pallets/passkey/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ sp-std = { workspace = true }
frame-benchmarking = { workspace = true, optional = true }
sp-core = { workspace = true }
log = { workspace = true, default-features = false }

# Frequency related dependencies
common-primitives = { default-features = false, path = "../../common/primitives" }

[dev-dependencies]
Expand Down
62 changes: 55 additions & 7 deletions pallets/passkey/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
traits::IsSubType,
};
use frame_system::pallet_prelude::*;
use sp_runtime::traits::Dispatchable;
use sp_runtime::traits::{Convert, Dispatchable, Verify};
use sp_std::vec::Vec;

#[cfg(test)]
mod mock;
Expand All @@ -44,6 +45,9 @@

#[frame_support::pallet]
pub mod module {
use common_primitives::utils::wrap_binary_data;
use sp_runtime::{AccountId32, MultiSignature};

use super::*;

/// the storage version for this pallet
Expand All @@ -64,12 +68,15 @@

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

/// AccountId truncated to 32 bytes
type ConvertIntoAccountId32: Convert<Self::AccountId, AccountId32>;
}

#[pallet::error]
pub enum Error<T> {
/// PlaceHolder error
PlaceHolderError,
/// InvalidAccountSignature
InvalidAccountSignature,
}

#[pallet::event]
Expand Down Expand Up @@ -119,13 +126,54 @@
#[pallet::validate_unsigned]
impl<T: Config> ValidateUnsigned for Pallet<T> {
type Call = Call<T>;
fn validate_unsigned(
_source: TransactionSource,
_call: &Self::Call,
) -> TransactionValidity {
fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
Self::validate_signatures(call)?;
Ok(ValidTransaction::default())
}
}

impl<T: Config> Pallet<T> {
fn validate_signatures(call: &Call<T>) -> TransactionValidity {
match call {
Call::proxy { payload } => {
let signed_data = payload.passkey_public_key;
let signature = payload.passkey_call.account_ownership_proof.clone();
let signer = &payload.passkey_call.account_id;
match Self::check_account_signature(signer, &signed_data.into(), &signature) {
Ok(_) => Ok(ValidTransaction::default()),

Check warning on line 143 in pallets/passkey/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/passkey/src/lib.rs#L143

Added line #L143 was not covered by tests
Err(_e) => InvalidTransaction::BadSigner.into(),
}
},
_ => InvalidTransaction::Call.into(),

Check warning on line 147 in pallets/passkey/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/passkey/src/lib.rs#L147

Added line #L147 was not covered by tests
}
}

/// Check the signature on passkey public key by the account id
/// Returns Ok(()) if the signature is valid
/// Returns Err(InvalidAccountSignature) if the signature is invalid
/// # Arguments
/// * `signer` - The account id of the signer
/// * `signed_data` - The signed data
/// * `signature` - The signature
/// # Return
/// * `Ok(())` if the signature is valid
/// * `Err(InvalidAccountSignature)` if the signature is invalid
fn check_account_signature(
signer: &T::AccountId,
signed_data: &Vec<u8>,
signature: &MultiSignature,
) -> DispatchResult {
let key: AccountId32 = T::ConvertIntoAccountId32::convert((*signer).clone());
let signed_payload: Vec<u8> = wrap_binary_data(signed_data.clone().into());

let verified = signature.verify(&signed_payload[..], &key);
if verified {
Ok(())

Check warning on line 171 in pallets/passkey/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/passkey/src/lib.rs#L171

Added line #L171 was not covered by tests
} else {
Err(Error::<T>::InvalidAccountSignature.into())
}
saraswatpuneet marked this conversation as resolved.
Show resolved Hide resolved
}
}
saraswatpuneet marked this conversation as resolved.
Show resolved Hide resolved
}

impl<T: Config> Pallet<T> {}
10 changes: 6 additions & 4 deletions pallets/passkey/src/mock.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
//! Mocks for the Passkey module.

use super::*;
use frame_support::{
construct_runtime,
traits::{ConstU32, ConstU64, Everything},
};
use sp_core::H256;
use sp_runtime::{traits::IdentityLookup, BuildStorage};
use sp_runtime::{
traits::{ConvertInto, IdentityLookup},
BuildStorage,
};

use crate as pallet_passkey;

Expand Down Expand Up @@ -49,10 +50,11 @@ impl frame_system::Config for Test {
type MaxConsumers = ConstU32<16>;
}

impl Config for Test {
impl pallet_passkey::Config for Test {
type RuntimeEvent = RuntimeEvent;
type WeightInfo = ();
type RuntimeCall = RuntimeCall;
type ConvertIntoAccountId32 = ConvertInto;
}

impl pallet_balances::Config for Test {
Expand Down
46 changes: 41 additions & 5 deletions pallets/passkey/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Unit tests for the passkey module.
use super::*;
use common_primitives::utils::wrap_binary_data;
use frame_support::{assert_noop, assert_ok};
use frame_system::Call as SystemCall;
use mock::*;
Expand All @@ -13,8 +14,10 @@ fn proxy_call_with_signed_origin_should_fail() {
// arrange
let (test_account_1_key_pair, _) = sr25519::Pair::generate();
let (test_account_2_key_pair, _) = sr25519::Pair::generate();

let signature: MultiSignature = test_account_1_key_pair.sign(b"sdsds").into();
let passkey_public_key = [0u8; 33];
let wrapped_binary = wrap_binary_data(passkey_public_key.to_vec());
let signature: MultiSignature =
test_account_1_key_pair.sign(wrapped_binary.as_slice()).into();
let call: PasskeyCall<Test> = PasskeyCall {
account_id: test_account_1_key_pair.public().into(),
account_nonce: 3,
Expand All @@ -25,7 +28,7 @@ fn proxy_call_with_signed_origin_should_fail() {
})),
};
let payload = PasskeyPayload {
passkey_public_key: [0u8; 33],
passkey_public_key,
verifiable_passkey_signature: VerifiablePasskeySignature {
saraswatpuneet marked this conversation as resolved.
Show resolved Hide resolved
signature: PasskeySignature::default(),
client_data_json: PasskeyClientDataJson::default(),
Expand All @@ -47,15 +50,18 @@ fn proxy_call_with_unsigned_origin_should_work() {
new_test_ext().execute_with(|| {
// arrange
let (test_account_1_key_pair, _) = sr25519::Pair::generate();
let signature: MultiSignature = test_account_1_key_pair.sign(b"sdsds").into();
let passkey_public_key = [0u8; 33];
let wrapped_binary = wrap_binary_data(passkey_public_key.to_vec());
let signature: MultiSignature =
test_account_1_key_pair.sign(wrapped_binary.as_slice()).into();
let call: PasskeyCall<Test> = PasskeyCall {
account_id: test_account_1_key_pair.public().into(),
account_nonce: 3,
account_ownership_proof: signature,
call: Box::new(RuntimeCall::System(SystemCall::remark { remark: vec![1, 2, 3u8] })),
};
let payload = PasskeyPayload {
passkey_public_key: [0u8; 33],
passkey_public_key,
verifiable_passkey_signature: VerifiablePasskeySignature {
signature: PasskeySignature::default(),
client_data_json: PasskeyClientDataJson::default(),
Expand All @@ -68,3 +74,33 @@ fn proxy_call_with_unsigned_origin_should_work() {
assert_ok!(Passkey::proxy(RuntimeOrigin::none(), payload));
});
}

#[test]
fn test_proxy_call_with_bad_signature_should_fail() {
new_test_ext().execute_with(|| {
// arrange
let (test_account_1_key_pair, _) = sr25519::Pair::generate();
let passkey_public_key = [0u8; 33];
let wrapped_binary = wrap_binary_data("bad data".as_bytes().to_vec());
let signature: MultiSignature =
test_account_1_key_pair.sign(wrapped_binary.as_slice()).into();
let call: PasskeyCall<Test> = PasskeyCall {
account_id: test_account_1_key_pair.public().into(),
account_nonce: 3,
account_ownership_proof: signature,
call: Box::new(RuntimeCall::System(SystemCall::remark { remark: vec![1, 2, 3u8] })),
};
let payload = PasskeyPayload {
passkey_public_key,
verifiable_passkey_signature: VerifiablePasskeySignature {
signature: PasskeySignature::default(),
client_data_json: PasskeyClientDataJson::default(),
authenticator_data: PasskeyAuthenticatorData::default(),
},
passkey_call: call,
};
let res = Passkey::validate_unsigned(TransactionSource::InBlock, &Call::proxy { payload });
// assert
assert_eq!(res, InvalidTransaction::BadSigner.into());
});
}
5 changes: 3 additions & 2 deletions runtime/frequency/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("frequency"),
impl_name: create_runtime_str!("frequency"),
authoring_version: 1,
spec_version: 86,
spec_version: 87,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand All @@ -357,7 +357,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("frequency-testnet"),
impl_name: create_runtime_str!("frequency"),
authoring_version: 1,
spec_version: 86,
spec_version: 87,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down Expand Up @@ -913,6 +913,7 @@ impl pallet_passkey::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type RuntimeCall = RuntimeCall;
type WeightInfo = pallet_passkey::weights::SubstrateWeight<Runtime>;
type ConvertIntoAccountId32 = ConvertInto;
}

#[cfg(any(not(feature = "frequency-no-relay"), feature = "frequency-lint-check"))]
Expand Down