Skip to content

Commit edeefe9

Browse files
committed
keystore: encrypt the bip39 seed in RAM
This encrypts the bip39 seed in RAM in the same way as the seed is encrypted, using the secure chip for key stretching. Since decryption (key stretching) takes around 100ms, this has a performance impact on every operation that needs the an xprv or xpub: - computing xpubs - getting private keys to sign something - etc. This has an impact especially in Bitcoin transaction signing, as we need the xpub at every transaction input during the first round of input streaming and for every change output. The xprv is needed twice per input when signing the input (once for the antiklepto nonce commit, then for signing). The performance impact was mitigated previously by 463b1f6, which sped up the first round of input streaming by caching xpubs - only one xpub per script type is needed for the whole transaction for single sig, where we only support three script types (p2wpkh, p2wpkh-p2sh, p2tr). In multisig, it is only one xpub. Here are some performance numbers: If you had a 50 inputs tx signed with antiklepto enabled, my experiment resulted in: - loading tx (stream inputs): ~30s, before xpub caching was activated - loading tx (stream inputs): ~6s, after xpub caching was activated - signing tx: 50*2*100ms: ~10s. I.e. Xpub caching saved more than seed decrypting added. Another place where this has a performance impact: retrieving xpubs. When the user connects the BitBox02 for the first time to the BitBoxApp, the BitBoxApp gets 3 xpubs right away (one per single sig script type) for Bitcoin, 2 xpubs for Litecoin, 1 for Ethereum. Each xpub computation is also performed twice (`see _get_xprv_twice()`) to avoid catch potential bitflips. With 100ms per access this means a delay of ~1.2s, which is noticable. Maybe a progress animation can/should be added there.
1 parent be929a5 commit edeefe9

File tree

3 files changed

+113
-6
lines changed

3 files changed

+113
-6
lines changed

src/keystore.c

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@ static size_t _retained_seed_encrypted_len = 0;
4343

4444
// Change this ONLY via keystore_unlock_bip39().
4545
static bool _is_unlocked_bip39 = false;
46+
static uint8_t _unstretched_retained_bip39_seed_encryption_key[32] = {0};
4647
// Must be defined if _is_unlocked is true. ONLY ACCESS THIS WITH _copy_bip39_seed().
47-
static uint8_t _retained_bip39_seed[64] = {0};
48+
static uint8_t _retained_bip39_seed_encrypted[64 + 64] = {0};
49+
static size_t _retained_bip39_seed_encrypted_len = 0;
4850

4951
/**
5052
* We allow seeds of 16, 24 or 32 bytes.
@@ -119,13 +121,37 @@ static bool _copy_bip39_seed(uint8_t* bip39_seed_out)
119121
if (!_is_unlocked_bip39) {
120122
return false;
121123
}
124+
125+
uint8_t retained_bip39_seed_encryption_key[32] = {0};
126+
UTIL_CLEANUP_32(retained_bip39_seed_encryption_key);
127+
if (_stretch_retained_seed_encryption_key(
128+
_unstretched_retained_bip39_seed_encryption_key,
129+
"keystore_retained_bip39_seed_access_in",
130+
"keystore_retained_bip39_seed_access_out",
131+
retained_bip39_seed_encryption_key) != KEYSTORE_OK) {
132+
return false;
133+
}
134+
size_t len = _retained_bip39_seed_encrypted_len - 48;
135+
bool password_correct = cipher_aes_hmac_decrypt(
136+
_retained_bip39_seed_encrypted,
137+
_retained_bip39_seed_encrypted_len,
138+
bip39_seed_out,
139+
&len,
140+
retained_bip39_seed_encryption_key);
141+
if (!password_correct) {
142+
// Should never happen.
143+
return false;
144+
}
145+
if (len != 64) {
146+
// Should never happen.
147+
return false;
148+
}
122149
// sanity check
123150
uint8_t zero[64] = {0};
124151
util_zero(zero, 64);
125-
if (MEMEQ(_retained_bip39_seed, zero, sizeof(_retained_bip39_seed))) {
152+
if (MEMEQ(bip39_seed_out, zero, 64)) {
126153
return false;
127154
}
128-
memcpy(bip39_seed_out, _retained_bip39_seed, sizeof(_retained_bip39_seed));
129155
return true;
130156
}
131157

@@ -370,7 +396,26 @@ USE_RESULT static keystore_error_t _retain_seed(const uint8_t* seed, size_t seed
370396

371397
USE_RESULT static bool _retain_bip39_seed(const uint8_t* bip39_seed)
372398
{
373-
memcpy(_retained_bip39_seed, bip39_seed, sizeof(_retained_bip39_seed));
399+
random_32_bytes(_unstretched_retained_bip39_seed_encryption_key);
400+
uint8_t retained_bip39_seed_encryption_key[32] = {0};
401+
UTIL_CLEANUP_32(retained_bip39_seed_encryption_key);
402+
if (_stretch_retained_seed_encryption_key(
403+
_unstretched_retained_bip39_seed_encryption_key,
404+
"keystore_retained_bip39_seed_access_in",
405+
"keystore_retained_bip39_seed_access_out",
406+
retained_bip39_seed_encryption_key) != KEYSTORE_OK) {
407+
return false;
408+
}
409+
size_t len = sizeof(_retained_bip39_seed_encrypted);
410+
if (!cipher_aes_hmac_encrypt(
411+
bip39_seed,
412+
64,
413+
_retained_bip39_seed_encrypted,
414+
&len,
415+
retained_bip39_seed_encryption_key)) {
416+
return false;
417+
}
418+
_retained_bip39_seed_encrypted_len = len;
374419
return true;
375420
}
376421

@@ -380,7 +425,12 @@ static void _delete_retained_seeds(void)
380425
_unstretched_retained_seed_encryption_key,
381426
sizeof(_unstretched_retained_seed_encryption_key));
382427
util_zero(_retained_seed_encrypted, sizeof(_retained_seed_encrypted));
383-
util_zero(_retained_bip39_seed, sizeof(_retained_bip39_seed));
428+
_retained_seed_encrypted_len = 0;
429+
util_zero(
430+
_unstretched_retained_bip39_seed_encryption_key,
431+
sizeof(_unstretched_retained_seed_encryption_key));
432+
util_zero(_retained_bip39_seed_encrypted, sizeof(_retained_bip39_seed_encrypted));
433+
_retained_bip39_seed_encrypted_len = 0;
384434
}
385435

386436
keystore_error_t keystore_unlock(
@@ -872,7 +922,9 @@ void keystore_mock_unlocked(const uint8_t* seed, size_t seed_len, const uint8_t*
872922
}
873923
_is_unlocked_bip39 = bip39_seed != NULL;
874924
if (bip39_seed != NULL) {
875-
memcpy(_retained_bip39_seed, bip39_seed, sizeof(_retained_bip39_seed));
925+
if (!_retain_bip39_seed(bip39_seed)) {
926+
Abort("couldn't retain bip39 seed");
927+
}
876928
}
877929
}
878930

@@ -881,4 +933,10 @@ const uint8_t* keystore_test_get_retained_seed_encrypted(size_t* len_out)
881933
*len_out = _retained_seed_encrypted_len;
882934
return _retained_seed_encrypted;
883935
}
936+
937+
const uint8_t* keystore_test_get_retained_bip39_seed_encrypted(size_t* len_out)
938+
{
939+
*len_out = _retained_bip39_seed_encrypted_len;
940+
return _retained_bip39_seed_encrypted;
941+
}
884942
#endif

src/keystore.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ USE_RESULT bool keystore_secp256k1_schnorr_bip86_sign(
285285
void keystore_mock_unlocked(const uint8_t* seed, size_t seed_len, const uint8_t* bip39_seed);
286286

287287
const uint8_t* keystore_test_get_retained_seed_encrypted(size_t* len_out);
288+
const uint8_t* keystore_test_get_retained_bip39_seed_encrypted(size_t* len_out);
288289
#endif
289290

290291
#endif

test/unit-test/test_keystore.c

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ static uint8_t _unstretched_retained_seed_encryption_key[32] =
6161
"\xfe\x09\x76\x01\x14\x52\xa7\x22\x12\xe4\xb8\xbd\x57\x2b\x5b\xe3\x01\x41\xa3\x56\xf1\x13\x37"
6262
"\xd2\x9d\x35\xea\x8f\xf9\x97\xbe\xfc";
6363

64+
static uint8_t _unstretched_retained_bip39_seed_encryption_key[32] =
65+
"\x9b\x44\xc7\x04\x88\x93\xfa\xaf\x6e\x2d\x76\x25\xd1\x3d\x8f\x1c\xab\x07\x65\xfd\x61\xf1\x59"
66+
"\xd9\x71\x3e\x08\x15\x5d\x06\x71\x7c";
67+
6468
static const uint32_t _keypath[] = {
6569
44 + BIP32_INITIAL_HARDENED_CHILD,
6670
0 + BIP32_INITIAL_HARDENED_CHILD,
@@ -78,6 +82,10 @@ static uint8_t _expected_retained_seed_secret[32] =
7882
"\xb1\x56\xbe\x41\x65\x30\xc6\xfc\x00\x01\x88\x44\x16\x17\x74\xa3\x54\x6a\x53\xac\x6d\xd4\xa0"
7983
"\x46\x26\x08\x83\x8e\x21\x60\x08\xf7";
8084

85+
const uint8_t _expected_retained_bip39_seed_secret[32] =
86+
"\x85\x6d\x9a\x8c\x1e\xa4\x2a\x69\xae\x76\x32\x42\x44\xac\xe6\x74\x39\x7f\xf1\x36\x0a\x4b\xa4"
87+
"\xc8\x5f\xfb\xd4\x2c\xee\x8a\x7f\x29";
88+
8189
static uint8_t _expected_secret[32] =
8290
"\xa8\xf4\xfe\x54\x33\x0e\x1a\xb7\xa0\xe3\xbe\x8a\x8d\x75\xd2\x22\xb2\xae\xc2\xb3\xab\x41\xca"
8391
"\x2a\x04\x0e\xa0\x08\x60\x6b\xaf\xce";
@@ -137,11 +145,19 @@ static void _expect_retain_seed(void)
137145
will_return(__wrap_random_32_bytes, _unstretched_retained_seed_encryption_key);
138146
}
139147

148+
static void _expect_retain_bip39_seed(void)
149+
{
150+
will_return(__wrap_random_32_bytes, _unstretched_retained_bip39_seed_encryption_key);
151+
}
152+
140153
void _mock_unlocked(const uint8_t* seed, size_t seed_len, const uint8_t* bip39_seed)
141154
{
142155
if (seed != NULL) {
143156
_expect_retain_seed();
144157
}
158+
if (bip39_seed != NULL) {
159+
_expect_retain_bip39_seed();
160+
}
145161
keystore_mock_unlocked(seed, seed_len, bip39_seed);
146162
}
147163

@@ -390,6 +406,36 @@ static void _test_keystore_unlock(void** state)
390406
assert_true(_reset_reset_called);
391407
}
392408

409+
static void _test_keystore_unlock_bip39(void** state)
410+
{
411+
keystore_lock();
412+
assert_false(keystore_unlock_bip39(""));
413+
414+
_mock_unlocked(_mock_seed, sizeof(_mock_seed), NULL);
415+
assert_true(keystore_is_locked());
416+
417+
_expect_retain_bip39_seed();
418+
assert_true(keystore_unlock_bip39("foo"));
419+
// Check that the retained bip39 seed was encrypted with the expected encryption key.
420+
size_t encrypted_len = 0;
421+
const uint8_t* retained_bip39_seed_encrypted =
422+
keystore_test_get_retained_bip39_seed_encrypted(&encrypted_len);
423+
size_t decrypted_len = encrypted_len - 48;
424+
uint8_t out[decrypted_len];
425+
assert_true(cipher_aes_hmac_decrypt(
426+
retained_bip39_seed_encrypted,
427+
encrypted_len,
428+
out,
429+
&decrypted_len,
430+
_expected_retained_bip39_seed_secret));
431+
assert_int_equal(decrypted_len, 64);
432+
const uint8_t expected_bip39_seed[64] =
433+
"\x2b\x3c\x63\xde\x86\xf0\xf2\xb1\x3c\xc6\xa3\x6c\x1b\xa2\x31\x4f\xbc\x1b\x40\xc7\x7a\xb9"
434+
"\xcb\x64\xe9\x6b\xa4\xd5\xc6\x2f\xc2\x04\x74\x8c\xa6\x62\x6a\x9f\x03\x5e\x7d\x43\x1b\xce"
435+
"\x8c\x92\x10\xec\x0b\xdf\xfc\x2e\x7d\xb8\x73\xde\xe5\x6c\x8a\xc2\x15\x3e\xee\x9a";
436+
assert_memory_equal(out, expected_bip39_seed, decrypted_len);
437+
}
438+
393439
static void _test_keystore_lock(void** state)
394440
{
395441
_mock_unlocked(NULL, 0, NULL);
@@ -474,6 +520,7 @@ static void _mock_with_mnemonic(const char* mnemonic, const char* passphrase)
474520
assert_true(keystore_bip39_mnemonic_to_seed(mnemonic, seed, &seed_len));
475521

476522
_mock_unlocked(seed, seed_len, NULL);
523+
_expect_retain_bip39_seed();
477524
assert_true(keystore_unlock_bip39(passphrase));
478525
}
479526

@@ -691,6 +738,7 @@ int main(void)
691738
cmocka_unit_test(_test_keystore_encrypt_and_store_seed),
692739
cmocka_unit_test(_test_keystore_create_and_unlock_twice),
693740
cmocka_unit_test(_test_keystore_unlock),
741+
cmocka_unit_test(_test_keystore_unlock_bip39),
694742
cmocka_unit_test(_test_keystore_lock),
695743
cmocka_unit_test(_test_keystore_get_bip39_mnemonic),
696744
cmocka_unit_test(_test_keystore_create_and_store_seed),

0 commit comments

Comments
 (0)