Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Enable signature verification in background task #10946

Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1d9df92
Enable signature verification in background task
librelois Nov 23, 2021
9a73a44
optimize : spawn all background checks before extrinsics execution
librelois Nov 29, 2021
77a767e
Merge branch 'master' into elois-background-sig-verif
librelois Dec 16, 2021
1ded1ba
Merge branch 'master' into elois-background-sig-verif
librelois Feb 28, 2022
9b974d8
fix warning: fn execute_extrinsics_with_book_keeping not used
librelois Feb 28, 2022
37fcf55
Merge branch 'master' into elois-background-sig-verif
librelois Apr 6, 2022
a222e60
fmt
librelois Apr 6, 2022
80baeb1
refactor in a more generic way
librelois Apr 13, 2022
4a02772
add ci job test-frame-executive-features-compile-to-wasm
librelois Apr 13, 2022
9409c4c
apply review suggestions
librelois May 12, 2022
a4765f4
apply review suggestions 2
librelois May 12, 2022
f3f30d0
Merge branch 'master' into elois-background-sig-verif
librelois May 12, 2022
787db84
Merge branch 'master' into elois-background-sig-verif
librelois Jun 2, 2022
0c4b002
rename SignatureBatching -> BackgroundVerifyContext
librelois Jun 2, 2022
f3557c4
Merge branch 'master' into elois-background-sig-verif
librelois Jul 1, 2022
1bde334
fix try runtime build
librelois Jul 21, 2022
f5a85b0
Merge branch 'master' into elois-background-sig-verif
librelois Jul 21, 2022
5727533
fix tests
librelois Jul 21, 2022
625ce06
fmt
librelois Jul 21, 2022
df3636e
fix warnings
librelois Jul 21, 2022
38a5d82
Merge branch 'master' into elois-background-sig-verif
librelois Aug 25, 2022
6062fa1
Merge branch 'master' into elois-background-sig-verif
librelois Sep 19, 2022
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
16 changes: 16 additions & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,22 @@ test-frame-examples-compile-to-wasm:
- cargo +nightly build --target=wasm32-unknown-unknown --no-default-features
- sccache -s

test-frame-executive-features-compile-to-wasm:
# into one job
stage: test
<<: *docker-env
<<: *test-refs
variables:
<<: *default-vars
# Enable debug assertions since we are running optimized builds for testing
# but still want to have debug assertions.
RUSTFLAGS: "-Cdebug-assertions=y"
RUST_BACKTRACE: 1
script:
- cd ./frame/executive
- cargo +nightly build --target=wasm32-unknown-unknown --no-default-features --features background-signature-verification
librelois marked this conversation as resolved.
Show resolved Hide resolved
- sccache -s

test-linux-stable-int:
<<: *test-linux
stage: test
Expand Down
5 changes: 4 additions & 1 deletion frame/executive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ sp-inherents = { version = "4.0.0-dev", path = "../../primitives/inherents" }

[features]
default = ["std"]
with-tracing = ["sp-tracing/with-tracing"]

# Enable signature verification in background task
background-signature-verification = []
std = [
"codec/std",
"scale-info/std",
Expand All @@ -48,3 +50,4 @@ std = [
"sp-std/std",
]
try-runtime = ["frame-support/try-runtime"]
with-tracing = ["sp-tracing/with-tracing"]
108 changes: 81 additions & 27 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,22 @@ use frame_support::{
},
weights::{DispatchClass, DispatchInfo, GetDispatchInfo},
};
#[cfg(not(feature = "background-signature-verification"))]
use sp_runtime::traits::Checkable;
#[cfg(feature = "background-signature-verification")]
use sp_runtime::traits::{BackgroundCheckable as Checkable, Checkable as _};
use sp_runtime::{
generic::Digest,
traits::{
self, Applyable, CheckEqual, Checkable, Dispatchable, Header, NumberFor, One, Saturating,
self, Applyable, CheckEqual, Dispatchable, Header, NumberFor, One, Saturating,
ValidateUnsigned, Zero,
},
transaction_validity::{TransactionSource, TransactionValidity},
ApplyExtrinsicResult,
};
use sp_std::{marker::PhantomData, prelude::*};

pub type CheckedOf<E, C> = <E as Checkable<C>>::Checked;
pub type CheckedOf<E, C> = <E as sp_runtime::traits::Checkable<C>>::Checked;
pub type CallOf<E, C> = <CheckedOf<E, C> as Applyable>::Call;
pub type OriginOf<E, C> = <CallOf<E, C> as Dispatchable>::Origin;

Expand Down Expand Up @@ -235,9 +239,19 @@ where
Self::initialize_block(block.header());
Self::initial_checks(&block);

#[cfg(feature = "background-signature-verification")]
let signature_batching = sp_runtime::SignatureBatching::start();

let (header, extrinsics) = block.deconstruct();
let checked_extrinsics = check_extrinsics(extrinsics);

Self::execute_checked_extrinsics_with_book_keeping(checked_extrinsics, *header.number());

Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());
#[cfg(feature = "background-signature-verification")]
// ensure that all background checks have been completed successfully.
if !signature_batching.verify() {
panic!("Signature verification failed.");
}

// do some of the checks that would normally happen in `final_checks`, but definitely skip
// the state root check.
Expand Down Expand Up @@ -351,6 +365,33 @@ where
}
}

fn check_extrinsics(
extrinsics: Vec<Block::Extrinsic>,
) -> Vec<(CheckedOf<Block::Extrinsic, Context>, Vec<u8>)> {
librelois marked this conversation as resolved.
Show resolved Hide resolved
extrinsics
.into_iter()
.map(|extrinsic| {
let encoded = extrinsic.encode();
#[cfg(feature = "background-signature-verification")]
match extrinsic.background_check(&Default::default()) {
Ok(checked_extrinsic) => (checked_extrinsic, encoded),
Err(e) => {
let err: &'static str = e.into();
panic!("{}", err)
},
}
#[cfg(not(feature = "background-signature-verification"))]
match extrinsic.check(&Default::default()) {
librelois marked this conversation as resolved.
Show resolved Hide resolved
Ok(checked_extrinsic) => (checked_extrinsic, encoded),
Err(e) => {
let err: &'static str = e.into();
panic!("{}", err)
},
}
})
.collect()
librelois marked this conversation as resolved.
Show resolved Hide resolved
}

/// Actually execute all transitions for `block`.
pub fn execute_block(block: Block) {
sp_io::init_tracing();
Expand All @@ -362,12 +403,18 @@ where
// any initial checks
Self::initial_checks(&block);

#[cfg(feature = "background-signature-verification")]
let signature_batching = sp_runtime::SignatureBatching::start();

// execute extrinsics
// check extrinsics in background
let (header, extrinsics) = block.deconstruct();
Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());
let checked_extrinsics = Self::check_extrinsics(extrinsics);

// execute extrinsics
Self::execute_checked_extrinsics_with_book_keeping(checked_extrinsics, *header.number());

#[cfg(feature = "background-signature-verification")]
// ensure that all background checks have been completed successfully.
librelois marked this conversation as resolved.
Show resolved Hide resolved
if !signature_batching.verify() {
panic!("Signature verification failed.");
}
Expand All @@ -377,17 +424,21 @@ where
}
}

/// Execute given extrinsics and take care of post-extrinsics book-keeping.
fn execute_extrinsics_with_book_keeping(
extrinsics: Vec<Block::Extrinsic>,
/// Execute given checked extrinsics and take care of post-extrinsics book-keeping.
fn execute_checked_extrinsics_with_book_keeping(
checked_extrinsics: Vec<(CheckedOf<Block::Extrinsic, Context>, Vec<u8>)>,
librelois marked this conversation as resolved.
Show resolved Hide resolved
block_number: NumberFor<Block>,
) {
extrinsics.into_iter().for_each(|e| {
if let Err(e) = Self::apply_extrinsic(e) {
let err: &'static str = e.into();
panic!("{}", err)
}
});
checked_extrinsics.into_iter().for_each(
librelois marked this conversation as resolved.
Show resolved Hide resolved
|(checked_extrinsic, unchecked_extrinsic_encoded)| {
if let Err(e) =
Self::apply_extrinsic_with_len(checked_extrinsic, unchecked_extrinsic_encoded)
{
let err: &'static str = e.into();
panic!("{}", err)
}
},
);

// post-extrinsics book-keeping
<frame_system::Pallet<System>>::note_finished_extrinsics();
Expand Down Expand Up @@ -431,34 +482,37 @@ where
///
/// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt
/// hashes.
pub fn apply_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult {
pub fn apply_extrinsic(unchecked_extrinsic: Block::Extrinsic) -> ApplyExtrinsicResult {
sp_io::init_tracing();
let encoded = uxt.encode();
let encoded_len = encoded.len();
Self::apply_extrinsic_with_len(uxt, encoded_len, encoded)
let unchecked_extrinsic_encoded = unchecked_extrinsic.encode();

// Verify that the signature is good.
let checked_extrinsic = unchecked_extrinsic.check(&Default::default())?;

Self::apply_extrinsic_with_len(checked_extrinsic, unchecked_extrinsic_encoded)
}

/// Actually apply an extrinsic given its `encoded_len`; this doesn't note its hash.
fn apply_extrinsic_with_len(
uxt: Block::Extrinsic,
encoded_len: usize,
to_note: Vec<u8>,
checked_extrinsic: CheckedOf<Block::Extrinsic, Context>,
unchecked_extrinsic_encoded: Vec<u8>,
) -> ApplyExtrinsicResult {
sp_tracing::enter_span!(sp_tracing::info_span!("apply_extrinsic",
ext=?sp_core::hexdisplay::HexDisplay::from(&uxt.encode())));
// Verify that the signature is good.
let xt = uxt.check(&Default::default())?;
ext=?sp_core::hexdisplay::HexDisplay::from(&unchecked_extrinsic_encoded)));

let encoded_len = unchecked_extrinsic_encoded.len();

// We don't need to make sure to `note_extrinsic` only after we know it's going to be
// executed to prevent it from leaking in storage since at this point, it will either
// execute or panic (and revert storage changes).
<frame_system::Pallet<System>>::note_extrinsic(to_note);
<frame_system::Pallet<System>>::note_extrinsic(unchecked_extrinsic_encoded);

// AUDIT: Under no circumstances may this function panic from here onwards.

// Decode parameters and dispatch
let dispatch_info = xt.get_dispatch_info();
let r = Applyable::apply::<UnsignedValidator>(xt, &dispatch_info, encoded_len)?;
let dispatch_info = checked_extrinsic.get_dispatch_info();
let r =
Applyable::apply::<UnsignedValidator>(checked_extrinsic, &dispatch_info, encoded_len)?;

<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info);

Expand Down
34 changes: 32 additions & 2 deletions primitives/runtime/src/generic/unchecked_extrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
use crate::{
generic::CheckedExtrinsic,
traits::{
self, Checkable, Extrinsic, ExtrinsicMetadata, IdentifyAccount, MaybeDisplay, Member,
SignedExtension,
self, BackgroundCheckable, Checkable, Extrinsic, ExtrinsicMetadata, IdentifyAccount,
MaybeDisplay, Member, SignedExtension,
},
transaction_validity::{InvalidTransaction, TransactionValidityError},
OpaqueExtrinsic,
Expand Down Expand Up @@ -163,6 +163,36 @@ where
}
}

impl<Address, AccountId, Call, Signature, Extra, Lookup> BackgroundCheckable<Lookup>
for UncheckedExtrinsic<Address, Call, Signature, Extra>
where
Address: Member + MaybeDisplay,
Call: Encode + Member,
Signature: Member + traits::BackgroundVerify,
<Signature as traits::Verify>::Signer: IdentifyAccount<AccountId = AccountId>,
Extra: SignedExtension<AccountId = AccountId>,
AccountId: Member + MaybeDisplay,
Lookup: traits::Lookup<Source = Address, Target = AccountId>,
{
fn background_check(self, lookup: &Lookup) -> Result<Self::Checked, TransactionValidityError> {
Ok(match self.signature {
Some((signed, signature, extra)) => {
let signed = lookup.lookup(signed)?;
let raw_payload = SignedPayload::new(self.function, extra)?;
if !raw_payload
.using_encoded(|payload| signature.background_verify(payload, &signed))
{
return Err(InvalidTransaction::BadProof.into())
}

let (function, extra, _) = raw_payload.deconstruct();
CheckedExtrinsic { signed: Some((signed, extra)), function }
},
None => CheckedExtrinsic { signed: None, function: self.function },
})
}
}

impl<Address, Call, Signature, Extra> ExtrinsicMetadata
for UncheckedExtrinsic<Address, Call, Signature, Extra>
where
Expand Down
30 changes: 30 additions & 0 deletions primitives/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,36 @@ impl Verify for MultiSignature {
}
}

impl crate::traits::BackgroundVerify for MultiSignature {
fn background_verify<L: Lazy<[u8]>>(&self, mut msg: L, signer: &AccountId32) -> bool {
match (self, signer) {
(Self::Ed25519(ref sig), who) =>
if let Ok(pubkey) = ed25519::Public::from_slice(who.as_ref()) {
sig.background_verify(msg, &pubkey)
} else {
false
},
(Self::Sr25519(ref sig), who) =>
if let Ok(pubkey) = sr25519::Public::from_slice(who.as_ref()) {
sig.background_verify(msg, &pubkey)
} else {
false
},
(Self::Ecdsa(ref sig), who) => {
// Unfortunatly, ecdsa signature can't be verified in a background task
// because we don't known the public key.
let m = sp_io::hashing::blake2_256(msg.get());
match sp_io::crypto::secp256k1_ecdsa_recover_compressed(sig.as_ref(), &m) {
Ok(pubkey) =>
&sp_io::hashing::blake2_256(pubkey.as_ref()) ==
<dyn AsRef<[u8; 32]>>::as_ref(who),
_ => false,
}
},
}
}
}

/// Signature verify that can work with any known signature types..
#[derive(Eq, PartialEq, Clone, Default, Encode, Decode, RuntimeDebug)]
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
Expand Down
63 changes: 63 additions & 0 deletions primitives/runtime/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,57 @@ where
}
}

/// A signature that supports background verification.
librelois marked this conversation as resolved.
Show resolved Hide resolved
pub trait BackgroundVerify: Verify {
bkchr marked this conversation as resolved.
Show resolved Hide resolved
/// Register a signature for background verification.
///
/// This requires that background verification is enabled by doing XYZ.
///
/// Returns `true` when the signature was successfully registered for background verification
/// or if background verification is not enabled the signature could be verified successfully
/// immediately.
///
/// # Warning
///
/// This requires that the background verification is finished by calling finalize_verify to
/// check the result of all submitted signature verifications.
Copy link
Member

@bkchr bkchr Apr 12, 2022

Choose a reason for hiding this comment

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

Suggested change
/// Register a signature for background verification.
///
/// This requires that background verification is enabled by doing XYZ.
///
/// Returns `true` when the signature was successfully registered for background verification
/// or if background verification is not enabled the signature could be verified successfully
/// immediately.
///
/// # Warning
///
/// This requires that the background verification is finished by calling finalize_verify to
/// check the result of all submitted signature verifications.
/// Register a signature for background verification.
///
/// Returns `true` when the signature was successfully registered for background verification
/// or if background verification is not enabled the signature could be verified successfully
/// immediately.
///
/// # Security
///
/// It is required that this is called in a [`SignatureBatching`](crate::SignatureBatching) context.

The SignatureBatching should then be renamed.

fn background_verify<L: Lazy<[u8]>>(
&self,
msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool;
}

impl BackgroundVerify for sp_core::ed25519::Signature {
fn background_verify<L: Lazy<[u8]>>(
&self,
mut msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool {
sp_io::crypto::ed25519_batch_verify(self, msg.get(), signer)
}
}

impl BackgroundVerify for sp_core::sr25519::Signature {
fn background_verify<L: Lazy<[u8]>>(
&self,
mut msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool {
sp_io::crypto::sr25519_batch_verify(self, msg.get(), signer)
}
}

impl BackgroundVerify for sp_core::ecdsa::Signature {
fn background_verify<L: Lazy<[u8]>>(
&self,
mut msg: L,
signer: &<Self::Signer as IdentifyAccount>::AccountId,
) -> bool {
sp_io::crypto::ecdsa_batch_verify(self, msg.get(), signer)
}
}

/// An error type that indicates that the origin is invalid.
#[derive(Encode, Decode, RuntimeDebug)]
pub struct BadOrigin;
Expand Down Expand Up @@ -769,6 +820,18 @@ pub trait Checkable<Context>: Sized {
fn check(self, c: &Context) -> Result<Self::Checked, TransactionValidityError>;
}

/// A piece of information "checkable" in a background task, used by the standard Substrate
/// Executive in order to check the validity of a piece of extrinsic information, usually by
/// verifying the signature. Implement for pieces of information that require some additional
librelois marked this conversation as resolved.
Show resolved Hide resolved
/// context `Context` in order to be checked.
pub trait BackgroundCheckable<Context>: Checkable<Context> {
/// Check self in a background tas, given an instance of Context.
fn background_check(
Comment on lines +1048 to +1049
Copy link
Member

@bkchr bkchr Apr 12, 2022

Choose a reason for hiding this comment

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

Suggested change
/// Check self in a background tas, given an instance of Context.
fn background_check(
/// Check `self` using the given `context`.
///
/// Any signature will be checked using [`BackgroundVerify`].
///
/// # Security
///
/// It is required that this is called in a [`SignatureBatching`](crate::SignatureBatching) context.
fn batch_verification_check(

self,
c: &Context,
) -> Result<<Self as Checkable<Context>>::Checked, TransactionValidityError>;
}

/// A "checkable" piece of information, used by the standard Substrate Executive in order to
/// check the validity of a piece of extrinsic information, usually by verifying the signature.
/// Implement for pieces of information that don't require additional context in order to be
Expand Down