Skip to content

Commit 1f572ae

Browse files
benmabenma
committed
Merge branch 'ethereum-testnet'
2 parents a365c67 + 18f2c90 commit 1f572ae

File tree

9 files changed

+276
-113
lines changed

9 files changed

+276
-113
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
77
## Firmware
88

99
### [Unreleased]
10+
- Allow mainnet keypaths (`m/44'/60'/0'/0/*`) Rinkeby and Ropsten, and testnet keypaths (`m/44'/1'/0'/0/*`) for Ethereum mainnet
1011
- Display granular error codes when unlock fails unexpectedly
1112
- Remove RandomNumber API endpoint
1213

‎src/rust/apps/ethereum/src/keypath.rs

Lines changed: 82 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,23 @@ use util::bip32::HARDENED;
1717
const ACCOUNT_MAX: u32 = 99; // 100 accounts
1818

1919
/// Does limit checks the keypath, whitelisting bip44 purpose, account and change.
20-
/// Only allows the well-known xpub of m'/44'/60'/0'/0 for now.
20+
/// Only allows the well-known xpubs of m'/44'/60'/0'/0 and m'/44'/1'/0'/0 for now.
2121
/// Since ethereum doesn't use the "change" path part it is always 0 and have become part of the
2222
/// xpub keypath.
2323
/// @return true if the keypath is valid, false if it is invalid.
24-
pub fn is_valid_keypath_xpub(keypath: &[u32], expected_coin: u32) -> bool {
25-
keypath.len() == 4 && keypath[..4] == [44 + HARDENED, expected_coin, 0 + HARDENED, 0]
24+
pub fn is_valid_keypath_xpub(keypath: &[u32]) -> bool {
25+
keypath.len() == 4
26+
&& (keypath[..4] == [44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0]
27+
|| keypath[..4] == [44 + HARDENED, 1 + HARDENED, 0 + HARDENED, 0])
2628
}
2729

2830
/// Does limit checks the keypath, whitelisting bip44 purpose, account and change.
2931
/// Returns true if the keypath is valid, false if it is invalid.
30-
pub fn is_valid_keypath_address(keypath: &[u32], expected_coin: u32) -> bool {
32+
pub fn is_valid_keypath_address(keypath: &[u32]) -> bool {
3133
if keypath.len() != 5 {
3234
return false;
3335
}
34-
if !is_valid_keypath_xpub(&keypath[..4], expected_coin) {
36+
if !is_valid_keypath_xpub(&keypath[..4]) {
3537
return false;
3638
}
3739
if keypath[4] > ACCOUNT_MAX {
@@ -46,66 +48,98 @@ mod tests {
4648

4749
#[test]
4850
fn test_is_valid_keypath_xpub() {
49-
let expected_coin = 60 + HARDENED;
50-
assert!(is_valid_keypath_xpub(
51-
&[44 + HARDENED, expected_coin, 0 + HARDENED, 0],
52-
expected_coin
53-
));
51+
assert!(is_valid_keypath_xpub(&[
52+
44 + HARDENED,
53+
60 + HARDENED,
54+
0 + HARDENED,
55+
0
56+
]));
57+
assert!(is_valid_keypath_xpub(&[
58+
44 + HARDENED,
59+
1 + HARDENED,
60+
0 + HARDENED,
61+
0
62+
]));
5463
// wrong coin.
55-
assert!(!is_valid_keypath_xpub(
56-
&[44 + HARDENED, expected_coin, 0 + HARDENED, 0],
57-
expected_coin + 1,
58-
));
64+
assert!(!is_valid_keypath_xpub(&[
65+
44 + HARDENED,
66+
0 + HARDENED,
67+
0 + HARDENED,
68+
0
69+
]));
5970
// too short
60-
assert!(!is_valid_keypath_xpub(
61-
&[44 + HARDENED, expected_coin, 0 + HARDENED],
62-
expected_coin + 1,
63-
));
71+
assert!(!is_valid_keypath_xpub(&[
72+
44 + HARDENED,
73+
60 + HARDENED,
74+
0 + HARDENED
75+
]));
6476
// too long
65-
assert!(!is_valid_keypath_xpub(
66-
&[44 + HARDENED, expected_coin, 0 + HARDENED, 0, 0],
67-
expected_coin + 1,
68-
));
77+
assert!(!is_valid_keypath_xpub(&[
78+
44 + HARDENED,
79+
60 + HARDENED,
80+
0 + HARDENED,
81+
0,
82+
0
83+
]));
6984
}
7085

7186
#[test]
7287
fn test_is_valid_keypath_address() {
73-
let expected_coin = 60 + HARDENED;
74-
let keypath_for_account =
75-
|account| [44 + HARDENED, expected_coin, 0 + HARDENED, 0, account];
76-
7788
// 100 good paths.
7889
for account in 0..100 {
79-
assert!(is_valid_keypath_address(
80-
&keypath_for_account(account),
81-
expected_coin
82-
));
90+
assert!(is_valid_keypath_address(&[
91+
44 + HARDENED,
92+
60 + HARDENED,
93+
0 + HARDENED,
94+
0,
95+
account
96+
]));
97+
assert!(is_valid_keypath_address(&[
98+
44 + HARDENED,
99+
1 + HARDENED,
100+
0 + HARDENED,
101+
0,
102+
account
103+
]));
83104
// wrong coin
84-
assert!(!is_valid_keypath_address(
85-
&keypath_for_account(account),
86-
expected_coin + 1
87-
));
105+
assert!(!is_valid_keypath_address(&[
106+
44 + HARDENED,
107+
0 + HARDENED,
108+
0 + HARDENED,
109+
0,
110+
account
111+
]));
88112
}
89-
assert!(!is_valid_keypath_address(
90-
&keypath_for_account(100),
91-
expected_coin
92-
));
113+
// account too high
114+
assert!(!is_valid_keypath_address(&[
115+
44 + HARDENED,
116+
60 + HARDENED,
117+
0 + HARDENED,
118+
0,
119+
100
120+
]));
93121

94122
// too short
95-
assert!(!is_valid_keypath_address(
96-
&[44 + HARDENED, expected_coin, 0 + HARDENED, 0],
97-
expected_coin
98-
));
123+
assert!(!is_valid_keypath_address(&[
124+
44 + HARDENED,
125+
60 + HARDENED,
126+
0 + HARDENED,
127+
0
128+
]));
99129
// too long
100-
assert!(!is_valid_keypath_address(
101-
&[44 + HARDENED, expected_coin, 0 + HARDENED, 0, 0, 0],
102-
expected_coin
103-
));
130+
assert!(!is_valid_keypath_address(&[
131+
44 + HARDENED,
132+
60 + HARDENED,
133+
0 + HARDENED,
134+
0,
135+
0,
136+
0
137+
]));
104138
// tweak keypath elements
105139
for i in 0..4 {
106-
let mut keypath = keypath_for_account(0);
140+
let mut keypath = [44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0];
107141
keypath[i] += 1;
108-
assert!(!is_valid_keypath_address(&keypath, expected_coin));
142+
assert!(!is_valid_keypath_address(&keypath));
109143
}
110144
}
111145
}

‎src/rust/bitbox02-rust-c/src/app_ethereum.rs

Lines changed: 0 additions & 58 deletions
This file was deleted.

‎src/rust/bitbox02-rust-c/src/lib.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ mod sha2;
4040
#[cfg(feature = "firmware")]
4141
mod workflow;
4242

43-
#[cfg(feature = "app-ethereum")]
44-
pub mod app_ethereum;
45-
4643
// Whenever execution reaches somewhere it isn't supposed to rust code will "panic". Our panic
4744
// handler will print the available information on the screen. If we compile with `panic=abort`
4845
// this code will never get executed.

‎src/rust/bitbox02-rust/src/hww/api/ethereum.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ compile_error!(
1818
);
1919

2020
mod amount;
21+
mod keypath;
2122
mod pubrequest;
2223
mod sign;
2324
mod signmsg;

‎src/rust/bitbox02-rust/src/hww/api/ethereum/keypath.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2021 Shift Crypto AG
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use super::Error;
16+
use crate::workflow::confirm;
17+
use bitbox02::app_eth::Params;
18+
19+
/// If the second element of `keypath` does not match the expected bip44 coin value for the given
20+
/// coin, we warn the user about an unusual keypath.
21+
///
22+
/// The keypath is already assumed to be validated and that the second element is one of the
23+
/// Ethereum bip44 values, e.g. `60'` or `1'`.
24+
///
25+
/// A warning suffices so the user does not accidentally send e.g. mainnet coins to a testnet path
26+
/// (m/44'/1'/...). It is safe to make a transaction on the 'wrong' keypath as the chain id is
27+
/// unique and part of the transaction sighash.
28+
pub async fn warn_unusual_keypath(
29+
params: &Params,
30+
title: &str,
31+
keypath: &[u32],
32+
) -> Result<(), Error> {
33+
if keypath.len() < 2 {
34+
return Err(Error::InvalidInput);
35+
}
36+
if keypath[1] != params.bip44_coin {
37+
let body = format!(
38+
"Unusual keypath warning: {}. Proceed only if you know what you are doing.",
39+
util::bip32::to_string(keypath)
40+
);
41+
let params = confirm::Params {
42+
title,
43+
body: &body,
44+
scrollable: true,
45+
..Default::default()
46+
};
47+
return Ok(confirm::confirm(&params).await?);
48+
}
49+
Ok(())
50+
}

‎src/rust/bitbox02-rust/src/hww/api/ethereum/pubrequest.rs

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ async fn process_address(request: &pb::EthPubRequest) -> Result<Response, Error>
4242
)
4343
};
4444

45-
if !ethereum::keypath::is_valid_keypath_address(&request.keypath, params.bip44_coin) {
45+
if !ethereum::keypath::is_valid_keypath_address(&request.keypath) {
4646
return Err(Error::InvalidInput);
4747
}
4848
let pubkey = bitbox02::keystore::secp256k1_pubkey_uncompressed(&request.keypath)
@@ -54,6 +54,7 @@ async fn process_address(request: &pb::EthPubRequest) -> Result<Response, Error>
5454
Some(erc20_params) => erc20_params.name,
5555
None => params.name,
5656
};
57+
super::keypath::warn_unusual_keypath(&params, title, &request.keypath).await?;
5758
let params = confirm::Params {
5859
title,
5960
title_autowrap: true,
@@ -73,8 +74,8 @@ fn process_xpub(request: &pb::EthPubRequest) -> Result<Response, Error> {
7374
return Err(Error::InvalidInput);
7475
}
7576

76-
let params = bitbox02::app_eth::params_get(request.coin as _).ok_or(Error::InvalidInput)?;
77-
if !ethereum::keypath::is_valid_keypath_xpub(&request.keypath, params.bip44_coin) {
77+
bitbox02::app_eth::params_get(request.coin as _).ok_or(Error::InvalidInput)?;
78+
if !ethereum::keypath::is_valid_keypath_xpub(&request.keypath) {
7879
return Err(Error::InvalidInput);
7980
}
8081
let xpub = keystore::encode_xpub_at_keypath(&request.keypath, keystore::xpub_type_t::XPUB)
@@ -201,6 +202,44 @@ mod tests {
201202
}))
202203
);
203204

205+
static mut CONFIRM_COUNTER: u32 = 0;
206+
207+
// All good, with display, unusual keypath.
208+
mock(Data {
209+
ui_confirm_create: Some(Box::new(|params| {
210+
match unsafe {
211+
CONFIRM_COUNTER += 1;
212+
CONFIRM_COUNTER
213+
} {
214+
1 => {
215+
assert_eq!(params.title, "Ropsten");
216+
assert_eq!(params.body, "Unusual keypath warning: m/44'/60'/0'/0/0. Proceed only if you know what you are doing.");
217+
}
218+
2 => {
219+
assert_eq!(params.title, "Ropsten");
220+
assert_eq!(params.body, ADDRESS);
221+
}
222+
_ => panic!("too many user confirmations"),
223+
}
224+
true
225+
})),
226+
..Default::default()
227+
});
228+
mock_unlocked();
229+
assert_eq!(
230+
block_on(process(&pb::EthPubRequest {
231+
output_type: OutputType::Address as _,
232+
keypath: [44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0].to_vec(),
233+
coin: pb::EthCoin::RopstenEth as _,
234+
display: true,
235+
contract_address: b"".to_vec(),
236+
})),
237+
Ok(Response::Pub(pb::PubResponse {
238+
r#pub: ADDRESS.into()
239+
}))
240+
);
241+
assert_eq!(unsafe { CONFIRM_COUNTER }, 2);
242+
204243
// Keystore locked.
205244
mock(Data {
206245
ui_confirm_create: Some(Box::new(|_| true)),

0 commit comments

Comments
 (0)