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

Transaction signing payload is hashed first #1624

Merged
gavofyork merged 9 commits into from
Jan 31, 2019
Merged

Transaction signing payload is hashed first #1624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
442f985
Transaction signing payload is hashed first
gavofyork Jan 30, 2019
3390172
Fix tests
gavofyork Jan 30, 2019
5dd3b53
Rebuild wasm
gavofyork Jan 30, 2019
5788499
Fix tests
gavofyork Jan 30, 2019
20f1a77
Only take payload hash when payload.len > 256 bytes.
gavofyork Jan 31, 2019
6825824
Fix tests
gavofyork Jan 31, 2019
787b46e
Merge remote-tracking branch 'origin/master' into gav-tx-hash-payload
gavofyork Jan 31, 2019
1f28d58
Merge remote-tracking branch 'origin/master' into gav-tx-hash-payload
gavofyork Jan 31, 2019
3a4dbcc
Merge remote-tracking branch 'origin/master' into gav-tx-hash-payload
gavofyork Jan 31, 2019
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
54 changes: 34 additions & 20 deletions core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use std::fmt;

use rstd::prelude::*;
use runtime_io::blake2_256;
use codec::{Decode, Encode, Input, Compact};
use traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup,
Checkable, Extrinsic};
Expand Down Expand Up @@ -85,14 +86,20 @@ where
Some((signed, signature, index, era)) => {
let h = context.block_number_to_hash(BlockNumber::sa(era.birth(context.current_height().as_())))
.ok_or("transaction birth block ancient")?;
let payload = (index, self.function, era, h);
let signed = context.lookup(signed)?;
if !::verify_encoded_lazy(&signature, &payload, &signed) {
let raw_payload = (index, self.function, era, h);
if !raw_payload.using_encoded(|payload| {
if payload.len() > 256 {
signature.verify(&blake2_256(payload)[..], &signed)
} else {
signature.verify(payload, &signed)
}
}) {
return Err("bad signature in extrinsic")
}
CheckedExtrinsic {
signed: Some((signed, (payload.0).0)),
function: payload.1,
signed: Some((signed, (raw_payload.0).0)),
function: raw_payload.1,
}
}
None => CheckedExtrinsic {
Expand Down Expand Up @@ -182,6 +189,7 @@ impl<Address, Index, Call, Signature> fmt::Debug for UncheckedMortalCompactExtri
#[cfg(test)]
mod tests {
use super::*;
use runtime_io::blake2_256;

struct TestContext;
impl Lookup for TestContext {
Expand All @@ -208,78 +216,84 @@ mod tests {
}
}

const DUMMY_FUNCTION: u64 = 0;
const DUMMY_ACCOUNTID: u64 = 0;

type Ex = UncheckedMortalCompactExtrinsic<u64, u64, u64, TestSig>;
type CEx = CheckedExtrinsic<u64, u64, u64>;
type Ex = UncheckedMortalCompactExtrinsic<u64, u64, Vec<u8>, TestSig>;
type CEx = CheckedExtrinsic<u64, u64, Vec<u8>>;

#[test]
fn unsigned_codec_should_work() {
let ux = Ex::new_unsigned(DUMMY_FUNCTION);
let ux = Ex::new_unsigned(vec![0u8;0]);
let encoded = ux.encode();
assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux));
}

#[test]
fn signed_codec_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::immortal(), 0u64).encode()), Era::immortal());
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal());
let encoded = ux.encode();
assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux));
}

#[test]
fn large_signed_codec_should_work() {
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned()), Era::immortal());
let encoded = ux.encode();
assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux));
}

#[test]
fn unsigned_check_should_work() {
let ux = Ex::new_unsigned(DUMMY_FUNCTION);
let ux = Ex::new_unsigned(vec![0u8;0]);
assert!(!ux.is_signed().unwrap_or(false));
assert!(<Ex as Checkable<TestContext>>::check(ux, &TestContext).is_ok());
}

#[test]
fn badly_signed_check_should_fail() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal());
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal());
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err("bad signature in extrinsic"));
}

#[test]
fn immortal_signed_check_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), DUMMY_FUNCTION, Era::immortal(), 0u64).encode()), Era::immortal());
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal());
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: DUMMY_FUNCTION }));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }));
}

#[test]
fn mortal_signed_check_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), DUMMY_FUNCTION, Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: DUMMY_FUNCTION }));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }));
}

#[test]
fn later_mortal_signed_check_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), DUMMY_FUNCTION, Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: DUMMY_FUNCTION }));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }));
}

#[test]
fn too_late_mortal_signed_check_should_fail() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err("bad signature in extrinsic"));
}

#[test]
fn too_early_mortal_signed_check_should_fail() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err("bad signature in extrinsic"));
}

#[test]
fn encoding_matches_vec() {
let ex = Ex::new_unsigned(DUMMY_FUNCTION);
let ex = Ex::new_unsigned(vec![0u8;0]);
let encoded = ex.encode();
let decoded = Ex::decode(&mut encoded.as_slice()).unwrap();
assert_eq!(decoded, ex);
Expand Down
55 changes: 35 additions & 20 deletions core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use std::fmt;

use rstd::prelude::*;
use runtime_io::blake2_256;
use codec::{Decode, Encode, Input};
use traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup,
Checkable, Extrinsic};
Expand Down Expand Up @@ -84,14 +85,21 @@ where
Some((signed, signature, index, era)) => {
let h = context.block_number_to_hash(BlockNumber::sa(era.birth(context.current_height().as_())))
.ok_or("transaction birth block ancient")?;
let payload = (index, self.function, era, h);
let signed = context.lookup(signed)?;
if !::verify_encoded_lazy(&signature, &payload, &signed) {
let raw_payload = (index, self.function, era, h);

if !raw_payload.using_encoded(|payload| {
if payload.len() > 256 {
signature.verify(&blake2_256(payload)[..], &signed)
} else {
signature.verify(payload, &signed)
}
}) {
return Err("bad signature in extrinsic")
}
CheckedExtrinsic {
signed: Some((signed, payload.0)),
function: payload.1,
signed: Some((signed, raw_payload.0)),
function: raw_payload.1,
}
}
None => CheckedExtrinsic {
Expand Down Expand Up @@ -180,6 +188,7 @@ impl<Address, Index, Call, Signature> fmt::Debug for UncheckedMortalExtrinsic<Ad
#[cfg(test)]
mod tests {
use super::*;
use runtime_io::blake2_256;

struct TestContext;
impl Lookup for TestContext {
Expand All @@ -206,78 +215,84 @@ mod tests {
}
}

const DUMMY_FUNCTION: u64 = 0;
const DUMMY_ACCOUNTID: u64 = 0;

type Ex = UncheckedMortalExtrinsic<u64, u64, u64, TestSig>;
type CEx = CheckedExtrinsic<u64, u64, u64>;
type Ex = UncheckedMortalExtrinsic<u64, u64, Vec<u8>, TestSig>;
type CEx = CheckedExtrinsic<u64, u64, Vec<u8>>;

#[test]
fn unsigned_codec_should_work() {
let ux = Ex::new_unsigned(DUMMY_FUNCTION);
let ux = Ex::new_unsigned(vec![0u8;0]);
let encoded = ux.encode();
assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux));
}

#[test]
fn signed_codec_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::immortal(), 0u64).encode()), Era::immortal());
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal());
let encoded = ux.encode();
assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux));
}

#[test]
fn large_signed_codec_should_work() {
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned()), Era::immortal());
let encoded = ux.encode();
assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux));
}

#[test]
fn unsigned_check_should_work() {
let ux = Ex::new_unsigned(DUMMY_FUNCTION);
let ux = Ex::new_unsigned(vec![0u8;0]);
assert!(!ux.is_signed().unwrap_or(false));
assert!(<Ex as Checkable<TestContext>>::check(ux, &TestContext).is_ok());
}

#[test]
fn badly_signed_check_should_fail() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal());
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal());
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err("bad signature in extrinsic"));
}

#[test]
fn immortal_signed_check_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::immortal(), 0u64).encode()), Era::immortal());
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal());
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: DUMMY_FUNCTION }));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }));
}

#[test]
fn mortal_signed_check_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: DUMMY_FUNCTION }));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }));
}

#[test]
fn later_mortal_signed_check_should_work() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: DUMMY_FUNCTION }));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }));
}

#[test]
fn too_late_mortal_signed_check_should_fail() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err("bad signature in extrinsic"));
}

#[test]
fn too_early_mortal_signed_check_should_fail() {
let ux = Ex::new_signed(0, DUMMY_FUNCTION, DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, DUMMY_FUNCTION, Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43));
let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43));
assert!(ux.is_signed().unwrap_or(false));
assert_eq!(<Ex as Checkable<TestContext>>::check(ux, &TestContext), Err("bad signature in extrinsic"));
}

#[test]
fn encoding_matches_vec() {
let ex = Ex::new_unsigned(DUMMY_FUNCTION);
let ex = Ex::new_unsigned(vec![0u8;0]);
let encoded = ex.encode();
let decoded = Ex::decode(&mut encoded.as_slice()).unwrap();
assert_eq!(decoded, ex);
Expand Down
Binary file modified ...st-runtime/wasm/target/wasm32-unknown-unknown/release/substrate_test_runtime.compact.wasm
View file
Open in desktop
Binary file not shown.
6 changes: 5 additions & 1 deletion node/executor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ mod tests {
let era = Era::mortal(256, 0);
let payload = (index.into(), xt.function, era, GENESIS_HASH);
let pair = Pair::from(Keyring::from_public(Public::from_raw(signed.clone().into())).unwrap());
let signature = pair.sign(&payload.encode()).into();
let signature = payload.using_encoded(|b| if b.len() > 256 {
pair.sign(&runtime_io::blake2_256(b))
} else {
pair.sign(b)
}).into();
UncheckedExtrinsic {
signature: Some((indices::address::Address::Id(signed), signature, payload.0, era)),
function: payload.1,
Expand Down
4 changes: 2 additions & 2 deletions node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("node"),
impl_name: create_runtime_str!("substrate-node"),
authoring_version: 10,
spec_version: 17,
impl_version: 17,
spec_version: 18,
impl_version: 18,
apis: RUNTIME_API_VERSIONS,
};

Expand Down
Binary file modified node/runtime/wasm/target/wasm32-unknown-unknown/release/node_runtime.compact.wasm
View file
Open in desktop
Binary file not shown.