Skip to content

Commit 5a06f1a

Browse files
benmabenma
committed
securechip: return granular error code from securechip_kdf()
So we can use it and show it to the user if it goes wrong in keystore_unlock().
1 parent d09951f commit 5a06f1a

File tree

10 files changed

+79
-40
lines changed

10 files changed

+79
-40
lines changed

‎src/keystore.c

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,19 @@ static uint8_t* _get_bip39_seed(void)
108108
return _retained_bip39_seed;
109109
}
110110

111-
static keystore_error_t _stretch_password(const char* password, uint8_t* kdf_out)
111+
/**
112+
* Stretch the user password using the securechip, putting the result in `kdf_out`, which must be 32
113+
* bytes. `securechip_result_out`, if not NULL, will contain the error code from `securechip_kdf()`
114+
* if there was a secure chip error, and 0 otherwise.
115+
*/
116+
static keystore_error_t _stretch_password(
117+
const char* password,
118+
uint8_t* kdf_out,
119+
int* securechip_result_out)
112120
{
121+
if (securechip_result_out != NULL) {
122+
*securechip_result_out = 0;
123+
}
113124
uint8_t password_salted_hashed[32] = {0};
114125
UTIL_CLEANUP_32(password_salted_hashed);
115126
if (!salt_hash_data(
@@ -126,13 +137,21 @@ static keystore_error_t _stretch_password(const char* password, uint8_t* kdf_out
126137

127138
// First KDF on SECURECHIP_SLOT_ROLLKEY increments the monotonic
128139
// counter. Call only once!
129-
if (!securechip_kdf(SECURECHIP_SLOT_ROLLKEY, kdf_in, 32, kdf_out)) {
140+
int securechip_result = securechip_kdf(SECURECHIP_SLOT_ROLLKEY, kdf_in, 32, kdf_out);
141+
if (securechip_result) {
142+
if (securechip_result_out != NULL) {
143+
*securechip_result_out = securechip_result;
144+
}
130145
return KEYSTORE_ERR_SC_KDF;
131146
}
132147
// Second KDF does not use the counter and we call it multiple times.
133148
for (int i = 0; i < KDF_NUM_ITERATIONS; i++) {
134149
memcpy(kdf_in, kdf_out, 32);
135-
if (!securechip_kdf(SECURECHIP_SLOT_KDF, kdf_in, 32, kdf_out)) {
150+
securechip_result = securechip_kdf(SECURECHIP_SLOT_KDF, kdf_in, 32, kdf_out);
151+
if (securechip_result) {
152+
if (securechip_result_out != NULL) {
153+
*securechip_result_out = securechip_result;
154+
}
136155
return KEYSTORE_ERR_SC_KDF;
137156
}
138157
}
@@ -153,11 +172,18 @@ static keystore_error_t _stretch_password(const char* password, uint8_t* kdf_out
153172
return KEYSTORE_OK;
154173
}
155174

175+
/**
176+
* Retrieves the encrypted seed and attempts to decrypt it using the password.
177+
*
178+
* `securechip_result_out`, if not NULL, will contain the error code from `securechip_kdf()` if
179+
* there was a secure chip error, and 0 otherwise.
180+
*/
156181
static keystore_error_t _get_and_decrypt_seed(
157182
const char* password,
158183
uint8_t* decrypted_seed_out,
159184
size_t* decrypted_seed_len_out,
160-
bool* password_correct_out)
185+
bool* password_correct_out,
186+
int* securechip_result_out)
161187
{
162188
uint8_t encrypted_seed_and_hmac[96];
163189
UTIL_CLEANUP_32(encrypted_seed_and_hmac);
@@ -167,7 +193,7 @@ static keystore_error_t _get_and_decrypt_seed(
167193
}
168194
uint8_t secret[32];
169195
UTIL_CLEANUP_32(secret);
170-
keystore_error_t result = _stretch_password(password, secret);
196+
keystore_error_t result = _stretch_password(password, secret, securechip_result_out);
171197
if (result != KEYSTORE_OK) {
172198
return result;
173199
}
@@ -199,7 +225,7 @@ static bool _verify_seed(
199225
size_t seed_len;
200226
UTIL_CLEANUP_32(decrypted_seed);
201227
bool password_correct = false;
202-
if (_get_and_decrypt_seed(password, decrypted_seed, &seed_len, &password_correct) !=
228+
if (_get_and_decrypt_seed(password, decrypted_seed, &seed_len, &password_correct, NULL) !=
203229
KEYSTORE_OK) {
204230
return false;
205231
}
@@ -235,7 +261,7 @@ bool keystore_encrypt_and_store_seed(
235261
}
236262
uint8_t secret[32] = {0};
237263
UTIL_CLEANUP_32(secret);
238-
if (_stretch_password(password, secret) != KEYSTORE_OK) {
264+
if (_stretch_password(password, secret, NULL) != KEYSTORE_OK) {
239265
return false;
240266
}
241267

@@ -302,7 +328,10 @@ static void _free_string(char** str)
302328
wally_free_string(*str);
303329
}
304330

305-
keystore_error_t keystore_unlock(const char* password, uint8_t* remaining_attempts_out)
331+
keystore_error_t keystore_unlock(
332+
const char* password,
333+
uint8_t* remaining_attempts_out,
334+
int* securechip_result_out)
306335
{
307336
if (!memory_is_seeded()) {
308337
return KEYSTORE_ERR_UNSEEDED;
@@ -323,7 +352,8 @@ keystore_error_t keystore_unlock(const char* password, uint8_t* remaining_attemp
323352
UTIL_CLEANUP_32(seed);
324353
size_t seed_len;
325354
bool password_correct = false;
326-
keystore_error_t result = _get_and_decrypt_seed(password, seed, &seed_len, &password_correct);
355+
keystore_error_t result =
356+
_get_and_decrypt_seed(password, seed, &seed_len, &password_correct, securechip_result_out);
327357
if (result != KEYSTORE_OK) {
328358
return result;
329359
}

‎src/keystore.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,15 @@ USE_RESULT bool keystore_create_and_store_seed(
9494
* If it is false, the keystore is not unlocked.
9595
* @param[out] remaining_attempts_out will have the number of remaining attempts.
9696
* If zero, the keystore is locked until the device is reset.
97+
* @param[out] securechip_result_out, if not NULL, will contain the error code from
98+
* `securechip_kdf()` if there was a secure chip error, and 0 otherwise.
9799
* @return
98100
* - KEYSTORE_OK if they keystore was successfully unlocked
99101
* - KEYSTORE_ERR_* if unsuccessful.
100102
* Only call this if memory_is_seeded() returns true.
101103
*/
102-
USE_RESULT keystore_error_t keystore_unlock(const char* password, uint8_t* remaining_attempts_out);
104+
USE_RESULT keystore_error_t
105+
keystore_unlock(const char* password, uint8_t* remaining_attempts_out, int* securechip_result_out);
103106

104107
/** Unlocks the bip39 seed.
105108
* @param[in] mnemonic_passphrase bip39 passphrase used in the derivation. Use the

‎src/rust/bitbox02/src/keystore.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ pub enum Error {
3838
MaxAttemptsExceeded,
3939
Unseeded,
4040
Memory,
41-
ScKdf,
41+
// Securechip error with the error code from securechip.c
42+
ScKdf(i32),
4243
Salt,
4344
Hash,
4445
SeedSize,
@@ -47,7 +48,14 @@ pub enum Error {
4748

4849
pub fn unlock(password: &SafeInputString) -> Result<(), Error> {
4950
let mut remaining_attempts: u8 = 0;
50-
match unsafe { bitbox02_sys::keystore_unlock(password.as_cstr(), &mut remaining_attempts) } {
51+
let mut securechip_result: i32 = 0;
52+
match unsafe {
53+
bitbox02_sys::keystore_unlock(
54+
password.as_cstr(),
55+
&mut remaining_attempts,
56+
&mut securechip_result,
57+
)
58+
} {
5159
keystore_error_t::KEYSTORE_OK => Ok(()),
5260
keystore_error_t::KEYSTORE_ERR_INCORRECT_PASSWORD => {
5361
Err(Error::IncorrectPassword { remaining_attempts })
@@ -56,7 +64,7 @@ pub fn unlock(password: &SafeInputString) -> Result<(), Error> {
5664
keystore_error_t::KEYSTORE_ERR_UNSEEDED => Err(Error::Unseeded),
5765
keystore_error_t::KEYSTORE_ERR_MEMORY => Err(Error::Memory),
5866
keystore_error_t::KEYSTORE_ERR_SEED_SIZE => Err(Error::SeedSize),
59-
keystore_error_t::KEYSTORE_ERR_SC_KDF => Err(Error::ScKdf),
67+
keystore_error_t::KEYSTORE_ERR_SC_KDF => Err(Error::ScKdf(securechip_result)),
6068
keystore_error_t::KEYSTORE_ERR_SALT => Err(Error::Salt),
6169
keystore_error_t::KEYSTORE_ERR_HASH => Err(Error::Hash),
6270
}

‎src/securechip/securechip.c

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -513,18 +513,18 @@ bool securechip_update_keys(void)
513513
return _update_kdf_key() == ATCA_SUCCESS;
514514
}
515515

516-
bool securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint8_t* kdf_out)
516+
int securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint8_t* kdf_out)
517517
{
518518
if (len > 127 || (slot != SECURECHIP_SLOT_ROLLKEY && slot != SECURECHIP_SLOT_KDF)) {
519-
return false;
519+
return SC_ERR_INVALID_ARGS;
520520
}
521521
if (msg == kdf_out) {
522-
return false;
522+
return SC_ERR_INVALID_ARGS;
523523
}
524524

525525
ATCA_STATUS result = _authorize_key();
526526
if (result != ATCA_SUCCESS) {
527-
return false;
527+
return result;
528528
}
529529

530530
uint8_t nonce_out[32] = {0};
@@ -555,7 +555,7 @@ bool securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint
555555
/* kdf_out, */
556556
/* nonce_out); */
557557
if (result != ATCA_SUCCESS) {
558-
return false;
558+
return result;
559559
}
560560
// Output is encrypted with the io protection key.
561561
uint8_t io_protection_key[32] = {0};
@@ -567,11 +567,7 @@ bool securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint
567567
.data = kdf_out,
568568
.data_size = 32,
569569
};
570-
result = atcah_io_decrypt(&io_dec_params);
571-
if (result != ATCA_SUCCESS) {
572-
return false;
573-
}
574-
return true;
570+
return atcah_io_decrypt(&io_dec_params);
575571
}
576572

577573
bool securechip_gen_attestation_key(uint8_t* pubkey_out)

‎src/securechip/securechip.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ typedef enum {
2929
SC_ERR_SLOT_UNLOCKED_AUTH = -5,
3030
SC_ERR_SLOT_UNLOCKED_ENC = -6,
3131
SC_ERR_IFS = -7,
32+
SC_ERR_INVALID_ARGS = -8,
3233
} securechip_error_t;
3334

3435
typedef struct {
@@ -87,9 +88,9 @@ USE_RESULT bool securechip_update_keys(void);
8788
* @param[in] len Must be <= 127.
8889
* @param[out] kdf_out Must have size 32. Result of the kdf will be stored here.
8990
* Cannot be the same as `msg`.
90-
* @return true on success.
91+
* @return values of `securechip_error_t` if negative, values of `ATCA_STATUS` if positive, 0 on
9192
*/
92-
USE_RESULT bool securechip_kdf(
93+
USE_RESULT int securechip_kdf(
9394
securechip_slot_t slot,
9495
const uint8_t* msg,
9596
size_t len,

‎src/workflow/restore.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ bool workflow_restore_backup(const RestoreBackupRequest* restore_request)
6060
return false;
6161
}
6262
uint8_t remaining_attempts;
63-
if (keystore_unlock(password, &remaining_attempts) != KEYSTORE_OK) {
63+
if (keystore_unlock(password, &remaining_attempts, NULL) != KEYSTORE_OK) {
6464
// This should/can never happen, but let's check anyway.
6565
Abort("workflow_restore_backup: unlock failed");
6666
}

‎test/device-test/src/test_backup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ int main(void)
131131
Abort("Failed to create keystore");
132132
}
133133
uint8_t remaining_attempts;
134-
if (keystore_unlock("device-test", &remaining_attempts) != KEYSTORE_OK) {
134+
if (keystore_unlock("device-test", &remaining_attempts, NULL) != KEYSTORE_OK) {
135135
Abort("Failed to unlock keystore");
136136
}
137137
_test_backup(_timestamp, _timestamp);

‎test/unit-test/framework/mock_securechip.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ bool securechip_update_keys(void)
3030
return true;
3131
}
3232

33-
bool securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint8_t* kdf_out)
33+
int securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint8_t* kdf_out)
3434
{
3535
check_expected(slot);
3636
check_expected(msg);
3737
// wally_sha256(msg, len, kdf_out, 32);
3838
memcpy(kdf_out, (const uint8_t*)mock(), 32);
39-
return true;
39+
return 0;
4040
}
4141

4242
bool securechip_u2f_counter_set(uint32_t counter)

‎test/unit-test/test_keystore.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -376,15 +376,15 @@ static void _test_keystore_create_and_unlock_twice(void** state)
376376

377377
will_return(__wrap_memory_is_seeded, true);
378378
_expect_stretch(true);
379-
assert_int_equal(KEYSTORE_OK, keystore_unlock(PASSWORD, &remaining_attempts));
379+
assert_int_equal(KEYSTORE_OK, keystore_unlock(PASSWORD, &remaining_attempts, NULL));
380380

381381
// Create new (different) seed.
382382
_expect_encrypt_and_store_seed();
383383
assert_true(keystore_encrypt_and_store_seed(_mock_seed_2, 32, PASSWORD));
384384

385385
will_return(__wrap_memory_is_seeded, true);
386386
_expect_stretch(true);
387-
assert_int_equal(KEYSTORE_OK, keystore_unlock(PASSWORD, &remaining_attempts));
387+
assert_int_equal(KEYSTORE_OK, keystore_unlock(PASSWORD, &remaining_attempts, NULL));
388388
}
389389

390390
static void _expect_seeded(bool seeded)
@@ -402,7 +402,7 @@ static void _perform_some_unlocks(void)
402402
_reset_reset_called = false;
403403
will_return(__wrap_memory_is_seeded, true);
404404
_expect_stretch(true);
405-
assert_int_equal(KEYSTORE_OK, keystore_unlock(PASSWORD, &remaining_attempts));
405+
assert_int_equal(KEYSTORE_OK, keystore_unlock(PASSWORD, &remaining_attempts, NULL));
406406
assert_int_equal(remaining_attempts, MAX_UNLOCK_ATTEMPTS);
407407
assert_false(_reset_reset_called);
408408
_expect_seeded(true);
@@ -417,7 +417,7 @@ static void _test_keystore_unlock(void** state)
417417
uint8_t remaining_attempts;
418418

419419
will_return(__wrap_memory_is_seeded, false);
420-
assert_int_equal(KEYSTORE_ERR_UNSEEDED, keystore_unlock(PASSWORD, &remaining_attempts));
420+
assert_int_equal(KEYSTORE_ERR_UNSEEDED, keystore_unlock(PASSWORD, &remaining_attempts, NULL));
421421
_expect_encrypt_and_store_seed();
422422
assert_true(keystore_encrypt_and_store_seed(_mock_seed, 32, PASSWORD));
423423
_expect_seeded(false);
@@ -432,7 +432,7 @@ static void _test_keystore_unlock(void** state)
432432
assert_int_equal(
433433
i >= MAX_UNLOCK_ATTEMPTS ? KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED
434434
: KEYSTORE_ERR_INCORRECT_PASSWORD,
435-
keystore_unlock(PASSWORD, &remaining_attempts));
435+
keystore_unlock(PASSWORD, &remaining_attempts, NULL));
436436
assert_int_equal(remaining_attempts, MAX_UNLOCK_ATTEMPTS - i);
437437
// Wrong password does not lock the keystore again if already unlocked.
438438
_expect_seeded(true);
@@ -444,7 +444,7 @@ static void _test_keystore_unlock(void** state)
444444
_reset_reset_called = false;
445445
will_return(__wrap_memory_is_seeded, true);
446446
assert_int_equal(
447-
KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED, keystore_unlock(PASSWORD, &remaining_attempts));
447+
KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED, keystore_unlock(PASSWORD, &remaining_attempts, NULL));
448448
assert_int_equal(remaining_attempts, 0);
449449
assert_true(_reset_reset_called);
450450
}

‎test/unit-test/test_keystore_functional.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ static uint8_t _salt_root[KEYSTORE_MAX_SEED_LENGTH] = {
4848
0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22,
4949
};
5050

51-
bool __wrap_securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint8_t* kdf_out)
51+
int __wrap_securechip_kdf(securechip_slot_t slot, const uint8_t* msg, size_t len, uint8_t* kdf_out)
5252
{
5353
assert_true(slot == SECURECHIP_SLOT_KDF || slot == SECURECHIP_SLOT_ROLLKEY);
5454
uint8_t key[3] = "key";
5555
assert_int_equal(WALLY_OK, wally_hmac_sha256(key, sizeof(key), msg, len, kdf_out, SHA256_LEN));
56-
return true;
56+
return 0;
5757
}
5858

5959
/** Reset the SmartEEPROM configuration. */
@@ -86,11 +86,12 @@ static void _test_seeds(void** state)
8686
will_return(__wrap_memory_is_seeded, true);
8787
assert_int_equal(
8888
KEYSTORE_ERR_INCORRECT_PASSWORD,
89-
keystore_unlock(_some_other_password, &remaining_attempts));
89+
keystore_unlock(_some_other_password, &remaining_attempts, NULL));
9090
// First time: unlock. After unlock, it becomes a password check.
9191
for (int i = 0; i < 3; i++) {
9292
will_return(__wrap_memory_is_seeded, true);
93-
assert_int_equal(KEYSTORE_OK, keystore_unlock(_some_password, &remaining_attempts));
93+
assert_int_equal(
94+
KEYSTORE_OK, keystore_unlock(_some_password, &remaining_attempts, NULL));
9495
}
9596
assert_true(keystore_copy_seed(read_seed, &read_seed_len));
9697
assert_int_equal(seed_size, read_seed_len);
@@ -158,7 +159,7 @@ static void _test_combination(
158159
uint8_t remaining_attempts;
159160
assert_true(keystore_is_locked());
160161
will_return(__wrap_memory_is_seeded, true);
161-
assert_int_equal(KEYSTORE_OK, keystore_unlock(_some_password, &remaining_attempts));
162+
assert_int_equal(KEYSTORE_OK, keystore_unlock(_some_password, &remaining_attempts, NULL));
162163
assert_true(keystore_is_locked());
163164
assert_true(keystore_unlock_bip39(mnemonic_passphrase));
164165
assert_false(keystore_is_locked());

0 commit comments

Comments
 (0)