Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

fix: eip712 signing with ledger hw #518

Merged
merged 11 commits into from
Oct 19, 2021
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
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ethers-contract/tests/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ mod eth_tests {
};

let sig = wallet
.sign_typed_data(foo_bar.clone())
.sign_typed_data(&foo_bar)
.await
.expect("failed to sign typed data");

Expand Down
4 changes: 4 additions & 0 deletions ethers-signers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ rand = { version = "0.8.4", default-features = false }
yubihsm = { version = "0.39.0", features = ["secp256k1", "http", "usb"], optional = true }
futures-util = "0.3.17"
futures-executor = "0.3.17"
semver = "1.0.4"

# aws
rusoto_core = { version = "0.47.0", optional = true }
Expand All @@ -39,6 +40,9 @@ spki = { version = "0.4.1", optional = true }
eth-keystore = { version = "0.3.0" }

[dev-dependencies]
ethers-contract = { version = "^0.5.0", path = "../ethers-contract", features = ["eip712", "abigen"]}
ethers-derive-eip712 = { version = "0.1.0", path = "../ethers-core/ethers-derive-eip712" }
serde_json = { version = "1.0.64" }
tracing-subscriber = "0.2.25"
yubihsm = { version = "0.39.0", features = ["secp256k1", "usb", "mockhsm"] }

Expand Down
2 changes: 1 addition & 1 deletion ethers-signers/src/aws/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ impl<'a> super::Signer for AwsSigner<'a> {

async fn sign_typed_data<T: Eip712 + Send + Sync>(
&self,
payload: T,
payload: &T,
) -> Result<EthSig, Self::Error> {
let hash = payload
.encode_eip712()
Expand Down
86 changes: 82 additions & 4 deletions ethers-signers/src/ledger/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use futures_util::lock::Mutex;

use ethers_core::{
types::{
transaction::eip2718::TypedTransaction, Address, NameOrAddress, Signature, Transaction,
TransactionRequest, TxHash, H256, U256,
transaction::{eip2718::TypedTransaction, eip712::Eip712},
Address, NameOrAddress, Signature, Transaction, TransactionRequest, TxHash, H256, U256,
},
utils::keccak256,
};
Expand All @@ -29,6 +29,8 @@ pub struct LedgerEthereum {
pub(crate) address: Address,
}

const EIP712_MIN_VERSION: &str = ">=1.6.0";

impl LedgerEthereum {
/// Instantiate the application by acquiring a lock on the ledger device.
///
Expand Down Expand Up @@ -136,7 +138,38 @@ impl LedgerEthereum {
self.sign_payload(INS::SIGN_PERSONAL_MESSAGE, payload).await
}

// Helper function for signing either transaction data or personal messages
/// Signs an EIP712 encoded domain separator and message
pub async fn sign_typed_struct<T>(&self, payload: &T) -> Result<Signature, LedgerError>
where
T: Eip712,
{
// See comment for v1.6.0 requirement
// https://github.com/LedgerHQ/app-ethereum/issues/105#issuecomment-765316999
let req = semver::VersionReq::parse(EIP712_MIN_VERSION)?;
let version = semver::Version::parse(&self.version().await?)?;

// Enforce app version is greater than EIP712_MIN_VERSION
if !req.matches(&version) {
return Err(LedgerError::UnsupportedAppVersion(
EIP712_MIN_VERSION.to_string(),
));
}

let domain_separator = payload
.domain_separator()
.map_err(|e| LedgerError::Eip712Error(e.to_string()))?;
let struct_hash = payload
.struct_hash()
.map_err(|e| LedgerError::Eip712Error(e.to_string()))?;

let mut payload = Self::path_to_bytes(&self.derivation);
payload.extend_from_slice(&domain_separator);
payload.extend_from_slice(&struct_hash);

self.sign_payload(INS::SIGN_ETH_EIP_712, payload).await
}

// Helper function for signing either transaction data, personal messages or EIP712 derived structs
pub async fn sign_payload(
&self,
command: INS,
Expand Down Expand Up @@ -200,9 +233,30 @@ impl LedgerEthereum {
mod tests {
use super::*;
use crate::Signer;
use ethers_core::types::{Address, TransactionRequest};
use ethers_contract::EthAbiType;
use ethers_core::types::{
transaction::eip712::Eip712, Address, TransactionRequest, I256, U256,
};
use ethers_derive_eip712::*;
use std::str::FromStr;

#[derive(Debug, Clone, Eip712, EthAbiType)]
#[eip712(
name = "Eip712Test",
version = "1",
chain_id = 1,
verifying_contract = "0x0000000000000000000000000000000000000001",
salt = "eip712-test-75F0CCte"
)]
struct FooBar {
foo: I256,
bar: U256,
fizz: Vec<u8>,
buzz: [u8; 32],
far: String,
out: Address,
}

#[tokio::test]
#[ignore]
// Replace this with your ETH addresses.
Expand Down Expand Up @@ -269,4 +323,28 @@ mod tests {
let addr = ledger.get_address().await.unwrap();
sig.verify(message, addr).unwrap();
}

#[tokio::test]
#[ignore]
async fn test_sign_eip712_struct() {
let ledger = LedgerEthereum::new(DerivationType::LedgerLive(0), 1u64)
.await
.unwrap();

let foo_bar = FooBar {
foo: I256::from(10),
bar: U256::from(20),
fizz: b"fizz".to_vec(),
buzz: keccak256("buzz"),
far: String::from("space"),
out: Address::from([0; 20]),
};

let sig = ledger
.sign_typed_struct(&foo_bar)
.await
.expect("failed to sign typed data");
let foo_bar_hash = foo_bar.encode_eip712().unwrap();
sig.verify(foo_bar_hash, ledger.address).unwrap();
}
}
11 changes: 3 additions & 8 deletions ethers-signers/src/ledger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,12 @@ impl Signer for LedgerEthereum {
self.sign_tx(message).await
}

/// Signs a EIP712 derived struct
async fn sign_typed_data<T: Eip712 + Send + Sync>(
&self,
payload: T,
payload: &T,
) -> Result<Signature, Self::Error> {
let hash = payload
.encode_eip712()
.map_err(|e| Self::Error::Eip712Error(e.to_string()))?;

let sig = self.sign_message(hash).await?;

Ok(sig)
self.sign_typed_struct(payload).await
}

/// Returns the signer's Ethereum Address
Expand Down
7 changes: 7 additions & 0 deletions ethers-signers/src/ledger/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,15 @@ pub enum LedgerError {
#[error(transparent)]
/// Error when converting from a hex string
HexError(#[from] hex::FromHexError),
#[error(transparent)]
/// Error when converting a semver requirement
SemVerError(#[from] semver::Error),
/// Error type from Eip712Error message
#[error("error encoding eip712 struct: {0:?}")]
Eip712Error(String),
/// Error when signing EIP712 struct with not compatible Ledger ETH app
#[error("Ledger ethereum app requires at least version: {0:?}")]
UnsupportedAppVersion(String),
}

pub const P1_FIRST: u8 = 0x00;
Expand All @@ -57,6 +63,7 @@ pub enum INS {
SIGN = 0x04,
GET_APP_CONFIGURATION = 0x06,
SIGN_PERSONAL_MESSAGE = 0x08,
SIGN_ETH_EIP_712 = 0x0C,
}

#[repr(u8)]
Expand Down
2 changes: 1 addition & 1 deletion ethers-signers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub trait Signer: std::fmt::Debug + Send + Sync {
/// Payload must implement Eip712 trait.
async fn sign_typed_data<T: Eip712 + Send + Sync>(
&self,
payload: T,
payload: &T,
) -> Result<Signature, Self::Error>;

/// Returns the signer's Ethereum Address
Expand Down
2 changes: 1 addition & 1 deletion ethers-signers/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<D: Sync + Send + DigestSigner<Sha256Proxy, RecoverableSignature>> Signer fo

async fn sign_typed_data<T: Eip712 + Send + Sync>(
&self,
payload: T,
payload: &T,
) -> Result<Signature, Self::Error> {
let encoded = payload
.encode_eip712()
Expand Down