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

Adds HostFunctions to tendermint-light-client-verifier #1138

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Cargo.lock
# Proptest regressions dumps
**/*.proptest-regressions

.idea
seunlanlege marked this conversation as resolved.
Show resolved Hide resolved
seunlanlege marked this conversation as resolved.
Show resolved Hide resolved

# Light Client WASM
light-client-js/pkg/
light-client-js/examples/verifier-web/node_modules/
Expand Down
2 changes: 1 addition & 1 deletion light-client-js/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ default = ["console_error_panic_hook"]
serde = { version = "1.0", default-features = false, features = [ "derive" ] }
serde_json = { version = "1.0", default-features = false }
# TODO(thane): Remove once https://github.com/rustwasm/wasm-bindgen/issues/2508 is resolved
seunlanlege marked this conversation as resolved.
Show resolved Hide resolved
syn = { version = "=1.0.65", default-features = false }
syn = { version = "1.0.95", default-features = false }
seunlanlege marked this conversation as resolved.
Show resolved Hide resolved
tendermint = { version = "0.24.0-pre.2", default-features = false, path = "../tendermint" }
tendermint-light-client-verifier = { version = "0.24.0-pre.2", default-features = false, path = "../light-client-verifier" }
wasm-bindgen = { version = "0.2.63", default-features = false, features = [ "serde-serialize" ] }
Expand Down
13 changes: 13 additions & 0 deletions light-client-verifier/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,27 @@ all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[features]
std = [
"tendermint/std",
"serde/std",
"time/std",
"flex-error/std",
]
default = ["flex-error/std", "flex-error/eyre_tracer"]
secp256k1 = ["tendermint/secp256k1"]
test-helpers = [ "sp-core"]

[dependencies]
tendermint = { version = "0.24.0-pre.2", path = "../tendermint", default-features = false }
derive_more = { version = "0.99.5", default-features = false, features = ["display"] }
serde = { version = "1.0.106", default-features = false }
time = { version = "0.3.5", default-features = false }
flex-error = { version = "0.4.4", default-features = false }
sp-std = { version = "4.0.0", default-features = false }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be an optional dependency?

Copy link
Author

Choose a reason for hiding this comment

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

its a drop in replacement for the std lib, so no default features means core, std features means std

sp-core = { version = "6.0.0", features = ["full_crypto"], optional = true }
Comment on lines +43 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these dependencies really necessary?

Copy link
Author

Choose a reason for hiding this comment

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

yes its used for tests and std end-users

Copy link
Member

Choose a reason for hiding this comment

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

Can we instead use core and alloc crates? We would rather avoid introducing Substrate-specific dependencies into the light client.


[dev-dependencies]
tendermint-testgen = { path = "../testgen", default-features = false }
hex = "0.4.3"
sp-core = { version = "6.0.0", features = ["full_crypto"] }
hex-literal = "0.3.4"
81 changes: 81 additions & 0 deletions light-client-verifier/src/host_functions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//! Host function utilities

use sp_std::fmt::Debug;

/// Host functions that the light client needs for crypto operations.
pub trait HostFunctionsProvider: Send + Sync + Default + Debug + 'static {
/// sha256 hash function
fn sha2_256(preimage: &[u8]) -> [u8; 32];

/// Verify an ed25519 signature
fn ed25519_verify(sig: &[u8], msg: &[u8], pub_key: &[u8]) -> bool;

/// verify secp256k1 signatures
fn secp256k1_verify(sig: &[u8], message: &[u8], public: &[u8]) -> bool;
Copy link
Collaborator

@tony-iqlusion tony-iqlusion Jun 20, 2022

Choose a reason for hiding this comment

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

Using a bool for signature verification is a dangerous API which has lead to cryptographic vulnerabilities in practice:

https://github.com/libp2p/rust-libp2p/pull/1127/files

The signature crate, already used by tendermint-rs, provides a well thought-out type safe trait-based approach to encapsulating signing and verification providers for various digital signature algorithms.

TMKMS uses this to support hardware signers for signatures, for example (e.g. Ledger, YubiHSM2)

Likewise the digest crate provides an abstraction over hash functions.

}

#[cfg(any(feature = "test-helpers", test))]
pub mod helper {
use crate::host_functions::HostFunctionsProvider;

#[derive(Default, Debug)]
pub struct HostFunctionsManager;

impl HostFunctionsProvider for HostFunctionsManager {
fn sha2_256(preimage: &[u8]) -> [u8; 32] {
sp_core::hashing::sha2_256(preimage)
}

fn ed25519_verify(sig: &[u8], msg: &[u8], pub_key: &[u8]) -> bool {
use sp_core::{ed25519, ByteArray, Pair};

let result = ed25519::Signature::from_slice(sig)
.ok_or(())
.and_then(|sig| {
let public_key = ed25519::Public::from_slice(pub_key).map_err(|_| ())?;
Ok((sig, public_key))
});

if let Ok((sig, public_key)) = result {
return ed25519::Pair::verify(&sig, msg, &public_key);
}

false
}

fn secp256k1_verify(sig: &[u8], message: &[u8], public: &[u8]) -> bool {
use sp_core::{ecdsa, ByteArray, Pair};

let result = ecdsa::Signature::from_slice(sig.clone())
.ok_or(())
.and_then(|sig| {
let public = ecdsa::Public::from_slice(public).map_err(|_| ())?;
Ok((public, sig))
});

if let Ok((public, signature)) = result {
return ecdsa::Pair::verify_weak(&sig, message, &public);
}

false
}
}
}

#[cfg(test)]
mod tests {
use hex_literal::hex;

use crate::host_functions::{helper::HostFunctionsManager, HostFunctionsProvider};

#[test]
#[should_panic]
// not super sure what the problem is here but secpk256 is optional so 🤷🏾‍
fn test_secpk1256_verification() {
let public = hex!("043a3150798c8af69d1e6e981f3a45402ba1d732f4be8330c5164f49e10ec555b4221bd842bc5e4d97eff37165f60e3998a424d72a450cf95ea477c78287d0343a");
let msg = hex!("313233343030");
let sig = hex!("304402207fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a002207fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0");

assert!(HostFunctionsManager::secp256k1_verify(&sig, &msg, &public))
}
}
2 changes: 2 additions & 0 deletions light-client-verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ extern crate alloc;
mod prelude;

pub mod errors;
pub mod host_functions;
pub mod merkle;
pub mod operations;
pub mod options;
pub mod predicates;
Expand Down
139 changes: 139 additions & 0 deletions light-client-verifier/src/merkle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
//! Merkle tree used in Tendermint networks

use crate::{host_functions::HostFunctionsProvider, prelude::*};

/// Size of Merkle root hash
pub const HASH_SIZE: usize = 32;

/// Hash is the output of the cryptographic digest function
pub type Hash = [u8; HASH_SIZE];

/// Compute a simple Merkle root from vectors of arbitrary byte vectors.
/// The leaves of the tree are the bytes of the given byte vectors in
/// the given order.
pub fn simple_hash_from_byte_vectors<H: HostFunctionsProvider>(byte_vecs: Vec<Vec<u8>>) -> Hash {
simple_hash_from_byte_slices_inner::<H>(byte_vecs.as_slice())
}

// recurse into subtrees
fn simple_hash_from_byte_slices_inner<H: HostFunctionsProvider>(byte_slices: &[Vec<u8>]) -> Hash {
let length = byte_slices.len();
match length {
0 => empty_hash::<H>(),
1 => leaf_hash::<H>(byte_slices[0].as_slice()),
_ => {
let k = get_split_point(length);
let left = simple_hash_from_byte_slices_inner::<H>(&byte_slices[..k]);
let right = simple_hash_from_byte_slices_inner::<H>(&byte_slices[k..]);
inner_hash::<H>(&left, &right)
},
}
}

// returns the largest power of 2 less than length
fn get_split_point(length: usize) -> usize {
match length {
0 => panic!("tree is empty!"),
1 => panic!("tree has only one element!"),
2 => 1,
_ => length.next_power_of_two() / 2,
}
}

// tmhash({})
fn empty_hash<H: HostFunctionsProvider>() -> Hash {
// the empty string / byte slice
let empty = Vec::with_capacity(0);

// hash it !
H::sha2_256(&empty)
}

// tmhash(0x00 || leaf)
fn leaf_hash<H: HostFunctionsProvider>(bytes: &[u8]) -> Hash {
// make a new array starting with 0 and copy in the bytes
let mut leaf_bytes = Vec::with_capacity(bytes.len() + 1);
leaf_bytes.push(0x00);
leaf_bytes.extend_from_slice(bytes);

// hash it !
H::sha2_256(&leaf_bytes)
}

// tmhash(0x01 || left || right)
fn inner_hash<H: HostFunctionsProvider>(left: &[u8], right: &[u8]) -> Hash {
// make a new array starting with 0x1 and copy in the bytes
let mut inner_bytes = Vec::with_capacity(left.len() + right.len() + 1);
inner_bytes.push(0x01);
inner_bytes.extend_from_slice(left);
inner_bytes.extend_from_slice(right);

// hash it !
H::sha2_256(&inner_bytes)
}

#[cfg(test)]
mod tests {
use super::*;
use crate::host_functions::helper::HostFunctionsManager; // TODO: use non-subtle ?

#[test]
fn test_get_split_point() {
assert_eq!(get_split_point(2), 1);
assert_eq!(get_split_point(3), 2);
assert_eq!(get_split_point(4), 2);
assert_eq!(get_split_point(5), 4);
assert_eq!(get_split_point(10), 8);
assert_eq!(get_split_point(20), 16);
assert_eq!(get_split_point(100), 64);
assert_eq!(get_split_point(255), 128);
assert_eq!(get_split_point(256), 128);
assert_eq!(get_split_point(257), 256);
}

#[test]
fn test_rfc6962_empty_tree() {
let empty_tree_root_hex =
"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
let empty_tree_root = &hex::decode(empty_tree_root_hex).unwrap();
let empty_tree: Vec<Vec<u8>> = vec![vec![]; 0];

let root = simple_hash_from_byte_vectors::<HostFunctionsManager>(empty_tree);
assert_eq!(empty_tree_root, &root);
}

#[test]
fn test_rfc6962_empty_leaf() {
let empty_leaf_root_hex =
"6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d";
let empty_leaf_root = &hex::decode(empty_leaf_root_hex).unwrap();
let one_empty_leaf: Vec<Vec<u8>> = vec![vec![]; 1];

let root = simple_hash_from_byte_vectors::<HostFunctionsManager>(one_empty_leaf);
assert_eq!(empty_leaf_root, &root);
}

#[test]
fn test_rfc6962_leaf() {
let leaf_root_hex = "395aa064aa4c29f7010acfe3f25db9485bbd4b91897b6ad7ad547639252b4d56";
let leaf_string = "L123456";

let leaf_root = &hex::decode(leaf_root_hex).unwrap();
let leaf_tree: Vec<Vec<u8>> = vec![leaf_string.as_bytes().to_vec(); 1];

let root = simple_hash_from_byte_vectors::<HostFunctionsManager>(leaf_tree);
assert_eq!(leaf_root, &root);
}

#[test]
fn test_rfc6962_node() {
let node_hash_hex = "aa217fe888e47007fa15edab33c2b492a722cb106c64667fc2b044444de66bbb";
let left_string = "N123";
let right_string = "N456";

let node_hash = &hex::decode(node_hash_hex).unwrap();
let hash =
inner_hash::<HostFunctionsManager>(left_string.as_bytes(), right_string.as_bytes());
assert_eq!(node_hash, &hash);
}
}
3 changes: 0 additions & 3 deletions light-client-verifier/src/operations.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
//! Crypto function traits allowing mocking out during testing

pub mod hasher;
pub use self::hasher::*;

pub mod voting_power;
pub use self::voting_power::*;

Expand Down
26 changes: 10 additions & 16 deletions light-client-verifier/src/operations/commit_validator.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
//! Provides an interface and default implementation for the `CommitValidator` operation

use core::marker::PhantomData;

use tendermint::block::CommitSig;

use crate::{
errors::VerificationError,
operations::{Hasher, ProdHasher},
host_functions::HostFunctionsProvider,
merkle::simple_hash_from_byte_vectors,
types::{SignedHeader, ValidatorSet},
};

Expand All @@ -27,25 +30,15 @@ pub trait CommitValidator: Send + Sync {

/// Production-ready implementation of a commit validator
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ProdCommitValidator {
hasher: ProdHasher,
}

impl ProdCommitValidator {
/// Create a new commit validator using the given [`Hasher`]
/// to compute the hash of headers and validator sets.
pub fn new(hasher: ProdHasher) -> Self {
Self { hasher }
}
}
pub struct ProdCommitValidator<H: HostFunctionsProvider>(PhantomData<H>);

impl Default for ProdCommitValidator {
impl<H: HostFunctionsProvider> Default for ProdCommitValidator<H> {
fn default() -> Self {
Self::new(ProdHasher::default())
Self(PhantomData)
}
}

impl CommitValidator for ProdCommitValidator {
impl<H: HostFunctionsProvider> CommitValidator for ProdCommitValidator<H> {
fn validate(
&self,
signed_header: &SignedHeader,
Expand Down Expand Up @@ -94,9 +87,10 @@ impl CommitValidator for ProdCommitValidator {
};

if validator_set.validator(*validator_address) == None {
let bytes = validator_set.serialize_to_preimage();
return Err(VerificationError::faulty_signer(
*validator_address,
self.hasher.hash_validator_set(validator_set),
simple_hash_from_byte_vectors::<H>(bytes).into(),
));
}
}
Expand Down
24 changes: 0 additions & 24 deletions light-client-verifier/src/operations/hasher.rs

This file was deleted.

Loading