frequency-chain / frequency

Frequency: A Polkadot Parachain
https://www.frequency.xyz
Apache License 2.0
51 stars 19 forks source link

Signature Verification: Support Unwrapped Signatures in addtion to <Bytes> Wrapped #2134

Closed wilwade closed 1 month ago

wilwade commented 3 months ago

Summary

The account_ownership_proof parameter in PasskeyCall signifies the signature of <Bytes>PASSKEY_PUBLIC_KEY</Bytes>. This means that the wallet client or browser must use this format during registration.

Issue details

Currently, the polkadot-js-extension wraps all raw signed bytes inside the <Bytes> tag, and the Frequency protocol follows the same approach during passkey registration. Specifically, it signs the payload <Bytes>PASSKEY_PUBLIC_KEY</Bytes> using the private key of the user's Substrate Account. During the transaction process, it wraps the passkey_public_key inside <Bytes>.

However, since any entity can develop an infrastructure for passkey registration and transaction submission, there is a risk that others might not wrap the PASSKEY_PUBLIC_KEY in <Bytes>, potentially causing confusion.

Risk

The current method could lead to interoperability issues if different implementations do not use the <Bytes> wrapping for the PASSKEY_PUBLIC_KEY. This inconsistency could result in failed signature verification and transaction errors.

Mitigation

The check_account_signature function should verify signatures for both the raw signed_data and <Bytes>signed_data</Bytes>. This dual-check approach, as implemented in pallet-nfts, would ensure compatibility across different implementations and reduce the risk of verification failures.

Suggestion

This should be fixed everywhere

aramikm commented 1 month ago

What is the benefit of applying this outside passkeys? Isn't applying this everywhere going to weaken the following claim?

As a convention the polkadot-js-extension always wraps raw data in tags to prevent malicious apps from tricking users into signing unintended transactions.