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

Feature/passkey proxy param shift 2241 2 #2243

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
165 changes: 63 additions & 102 deletions pallets/passkey/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
/// The majority of these checks are the same as `SignedExtra` list in defined in runtime
fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
let valid_tx = ValidTransaction::default();
let payload = Self::filter_valid_calls(&call)?;
let (payload, is_legacy_call) = Self::filter_valid_calls(&call)?;

let frame_system_validity =
FrameSystemChecks(payload.passkey_call.account_id.clone(), call.clone())
Expand All @@ -199,14 +199,8 @@
)
.validate()?;
// this is the last since it is the heaviest
// Requires different calls for v1 vs v2
let signature_validity = match call {
Call::proxy { payload } =>
PasskeySignatureCheck::new(payload.clone()).validate()?,
Call::proxy_v2 { payload } =>
PasskeySignatureCheckV2::new(payload.clone()).validate()?,
_ => return Err(InvalidTransaction::Call.into()),
};
let signature_validity =
PasskeySignatureCheck::new(payload.clone(), is_legacy_call).validate()?;

let valid_tx = valid_tx
.combine_with(frame_system_validity)
Expand All @@ -220,22 +214,15 @@
/// Checking and executing a list of operations pre_dispatch
/// The majority of these checks are the same as `SignedExtra` list in defined in runtime
fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> {
let payload = Self::filter_valid_calls(&call)?;
let (payload, is_legacy_call) = Self::filter_valid_calls(&call)?;
FrameSystemChecks(payload.passkey_call.account_id.clone(), call.clone())
.pre_dispatch()?;
PasskeyNonceCheck::new(payload.passkey_call.clone()).pre_dispatch()?;
PasskeyWeightCheck::new(call.clone()).pre_dispatch()?;
ChargeTransactionPayment::<T>(payload.passkey_call.account_id.clone(), call.clone())
.pre_dispatch()?;
// this is the last since it is the heaviest
// Requires different calls for v1 vs v2
match call {
Call::proxy { payload } =>
PasskeySignatureCheck::new(payload.clone()).pre_dispatch(),
Call::proxy_v2 { payload } =>
PasskeySignatureCheckV2::new(payload.clone()).pre_dispatch(),
_ => return Err(InvalidTransaction::Call.into()),
}
PasskeySignatureCheck::new(payload.clone(), is_legacy_call).pre_dispatch()
}
}
}
Expand All @@ -246,15 +233,18 @@
<T as frame_system::Config>::RuntimeCall:
From<Call<T>> + Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
{
/// Filtering the valid calls and extracting the Payload V2 from inside the call
fn filter_valid_calls(call: &Call<T>) -> Result<PasskeyPayloadV2<T>, TransactionValidityError> {
/// Filtering the valid calls and extracting the Payload V2 from inside the call and returning if this
/// is a legacy call or not
fn filter_valid_calls(
call: &Call<T>,
) -> Result<(PasskeyPayloadV2<T>, bool), TransactionValidityError> {
match call {
Call::proxy { payload }
if T::PasskeyCallFilter::contains(&payload.clone().passkey_call.call) =>
return Ok(payload.clone().into()),
return Ok((payload.clone().into(), true)),
Call::proxy_v2 { payload }
if T::PasskeyCallFilter::contains(&payload.clone().passkey_call.call) =>
return Ok(payload.clone()),
return Ok((payload.clone(), false)),
_ => return Err(InvalidTransaction::Call.into()),
}
}
Expand Down Expand Up @@ -299,99 +289,70 @@
/// 2. Passkey P256 signature of the account public key
#[derive(Encode, Decode, Clone, TypeInfo)]
#[scale_info(skip_type_params(T))]
struct PasskeySignatureCheck<T: Config>(pub PasskeyPayload<T>);

/// Passkey V2 signatures check which verifies 2 signatures
/// 1. Account signature of the P256 public key
/// 2. Passkey P256 signature of the account public key
#[derive(Encode, Decode, Clone, TypeInfo)]
#[scale_info(skip_type_params(T))]
struct PasskeySignatureCheckV2<T: Config>(pub PasskeyPayloadV2<T>);

/// 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<T: Config>(
signer: &T::AccountId,
signed_data: &Vec<u8>,
signature: &MultiSignature,
) -> DispatchResult {
let key = T::ConvertIntoAccountId32::convert((*signer).clone());

if !check_signature(signature, key, signed_data.clone()) {
return Err(Error::<T>::InvalidAccountSignature.into());
}

Ok(())
}

/// Validation logic for the two signatures for a passkey call
/// Returns Ok(()) if the signatures are valid
fn validate_payload_pieces<T: Config>(
account_ownership_proof: &MultiSignature,
signer: &T::AccountId,
p256_signer: &PasskeyPublicKey,
p256_signature: &VerifiablePasskeySignature,
p256_signed_data: &Vec<u8>,
) -> TransactionValidity {
// checking account signature to verify ownership of the account used
check_account_signature::<T>(signer, &p256_signer.inner().to_vec(), &account_ownership_proof)
.map_err(|_e| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;
p256_signature
.try_verify(&p256_signed_data, &p256_signer)
.map_err(|e| match e {
PasskeyVerificationError::InvalidProof =>
TransactionValidityError::Invalid(InvalidTransaction::BadSigner),
_ => TransactionValidityError::Invalid(InvalidTransaction::Custom(e.into())),
})?;

Ok(ValidTransaction::default())
struct PasskeySignatureCheck<T: Config> {
payload: PasskeyPayloadV2<T>,
is_legacy_payload: bool,
}

impl<T: Config> PasskeySignatureCheck<T> {
pub fn new(passkey_payload: PasskeyPayload<T>) -> Self {
Self(passkey_payload)
pub fn new(passkey_payload: PasskeyPayloadV2<T>, is_legacy_payload: bool) -> Self {
Self { payload: passkey_payload, is_legacy_payload }
}

pub fn validate(&self) -> TransactionValidity {
validate_payload_pieces::<T>(
&self.0.passkey_call.account_ownership_proof,
&self.0.passkey_call.account_id,
&self.0.passkey_public_key,
&self.0.verifiable_passkey_signature,
&self.0.passkey_call.encode(),
)
// checking account signature to verify ownership of the account used
let signed_data = self.payload.passkey_public_key.clone();
let signature = self.payload.account_ownership_proof.clone();
let signer = &self.payload.passkey_call.account_id;

Self::check_account_signature(signer, &signed_data.inner().to_vec(), &signature)
.map_err(|_e| TransactionValidityError::Invalid(InvalidTransaction::BadSigner))?;

// checking the passkey signature to ensure access to the passkey
let p256_signed_data = match self.is_legacy_payload {
true => PasskeyPayload::from(self.payload.clone()).passkey_call.encode(),
false => self.payload.passkey_call.encode(),
};
let p256_signature = self.payload.verifiable_passkey_signature.clone();
let p256_signer = self.payload.passkey_public_key.clone();

p256_signature
.try_verify(&p256_signed_data, &p256_signer)
.map_err(|e| match e {
PasskeyVerificationError::InvalidProof =>
TransactionValidityError::Invalid(InvalidTransaction::BadSigner),
_ => TransactionValidityError::Invalid(InvalidTransaction::Custom(e.into())),

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

View check run for this annotation

Codecov / codecov/patch

pallets/passkey/src/lib.rs#L324

Added line #L324 was not covered by tests
})?;

Ok(ValidTransaction::default())
}

pub fn pre_dispatch(&self) -> Result<(), TransactionValidityError> {
let _ = self.validate()?;
Ok(())
}
}

impl<T: Config> PasskeySignatureCheckV2<T> {
pub fn new(passkey_payload: PasskeyPayloadV2<T>) -> Self {
Self(passkey_payload)
}

pub fn validate(&self) -> TransactionValidity {
validate_payload_pieces::<T>(
&self.0.account_ownership_proof,
&self.0.passkey_call.account_id,
&self.0.passkey_public_key,
&self.0.verifiable_passkey_signature,
&self.0.passkey_call.encode(),
)
}
/// 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 = T::ConvertIntoAccountId32::convert((*signer).clone());

if !check_signature(signature, key, signed_data.clone()) {
return Err(Error::<T>::InvalidAccountSignature.into());
}

pub fn pre_dispatch(&self) -> Result<(), TransactionValidityError> {
let _ = self.validate()?;
Ok(())
}
}
Expand Down
3 changes: 3 additions & 0 deletions pallets/passkey/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ impl TestPasskeyPayloadBuilder {
}

#[test]
#[allow(deprecated)]
fn proxy_call_with_signed_origin_should_fail() {
new_test_ext().execute_with(|| {
// arrange
Expand All @@ -134,6 +135,7 @@ fn proxy_call_with_signed_origin_should_fail() {
}

#[test]
#[allow(deprecated)]
fn proxy_call_with_unsigned_origin_should_work() {
new_test_ext().execute_with(|| {
// arrange
Expand Down Expand Up @@ -285,6 +287,7 @@ fn validate_unsigned_should_fee_removed_on_successful_validation() {
}

#[test]
#[allow(deprecated)]
fn fee_withdrawn_for_failed_call() {
new_test_ext().execute_with(|| {
// arrange
Expand Down
17 changes: 11 additions & 6 deletions pallets/passkey/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,17 @@ impl<T: Config> From<PasskeyPayload<T>> for PasskeyPayloadV2<T> {
}
}

impl<T: Config> From<PasskeyCall<T>> for PasskeyCallV2<T> {
fn from(call: PasskeyCall<T>) -> Self {
PasskeyCallV2 {
account_id: call.account_id,
account_nonce: call.account_nonce,
call: call.call,
impl<T: Config> From<PasskeyPayloadV2<T>> for PasskeyPayload<T> {
fn from(payload: PasskeyPayloadV2<T>) -> Self {
PasskeyPayload {
passkey_public_key: payload.passkey_public_key,
verifiable_passkey_signature: payload.verifiable_passkey_signature,
passkey_call: PasskeyCall {
account_id: payload.passkey_call.account_id,
account_ownership_proof: payload.account_ownership_proof,
account_nonce: payload.passkey_call.account_nonce,
call: payload.passkey_call.call,
},
}
}
}
Expand Down
Loading