Skip to content

Commit

Permalink
Merge branch 'smarteeprom_fix'
Browse files Browse the repository at this point in the history
Smarteeprom fix
  • Loading branch information
conte91 committed Dec 16, 2019
2 parents 3ca0f45 + 2a0f318 commit cff33d0
Show file tree
Hide file tree
Showing 11 changed files with 392 additions and 68 deletions.
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ set(DBB-FIRMWARE-SOURCES
${CMAKE_SOURCE_DIR}/src/util.c
${CMAKE_SOURCE_DIR}/src/sd.c
${CMAKE_SOURCE_DIR}/src/hww.c
${CMAKE_SOURCE_DIR}/src/memory/bitbox02_smarteeprom.c
${CMAKE_SOURCE_DIR}/src/memory/memory.c
${CMAKE_SOURCE_DIR}/src/memory/mpu.c
${CMAKE_SOURCE_DIR}/src/memory/nvmctrl.c
Expand Down
2 changes: 2 additions & 0 deletions src/firmware.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "driver_init.h"
#include "firmware_main_loop.h"
#include "hardfault.h"
#include "memory/bitbox02_smarteeprom.h"
#include "platform_init.h"
#include "qtouch.h"
#include "screen.h"
Expand All @@ -35,6 +36,7 @@ int main(void)
screen_splash();
qtouch_init();
common_main();
bitbox02_smarteeprom_init();
traceln("%s", "Device initialized");
workflow_start_orientation_screen();
firmware_main_loop();
Expand Down
29 changes: 14 additions & 15 deletions src/keystore.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "cipher/cipher.h"
#include "hardfault.h"
#include "keystore.h"
#include "memory/bitbox02_smarteeprom.h"
#include "memory/memory.h"
#include "random.h"
#include "reset.h"
Expand All @@ -28,10 +29,6 @@
#include <wally_bip39.h>
#include <wally_crypto.h>

// After this many failed unlock attempts, the keystore becomes locked until a
// device reset.
#define MAX_UNLOCK_ATTEMPTS (10)

// This number of KDF iterations on the 2nd kdf slot when stretching the device
// password.
#define KDF_NUM_ITERATIONS (2)
Expand Down Expand Up @@ -289,7 +286,6 @@ bool keystore_create_and_store_seed(const char* password, const uint8_t* host_en
for (size_t i = 0; i < KEYSTORE_MAX_SEED_LENGTH; i++) {
seed[i] ^= password_salted_hashed[i];
}

return keystore_encrypt_and_store_seed(seed, KEYSTORE_MAX_SEED_LENGTH, password);
}

Expand All @@ -303,12 +299,18 @@ keystore_error_t keystore_unlock(const char* password, uint8_t* remaining_attemp
if (!memory_is_seeded()) {
return KEYSTORE_ERR_GENERIC;
}
uint8_t failed_attempts = memory_get_failed_unlock_attempts();
uint8_t failed_attempts = bitbox02_smarteeprom_get_unlock_attempts();
if (failed_attempts >= MAX_UNLOCK_ATTEMPTS) {
/*
* We reset the device as soon as the MAX_UNLOCK_ATTEMPTSth attempt
* is made. So we should never enter this branch...
* This is just an extraordinary measure for added resilience.
*/
*remaining_attempts_out = 0;
reset_reset(false);
return KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED;
}
bitbox02_smarteeprom_increment_unlock_attempts();
uint8_t seed[KEYSTORE_MAX_SEED_LENGTH] = {0};
UTIL_CLEANUP_32(seed);
size_t seed_len;
Expand All @@ -320,28 +322,25 @@ keystore_error_t keystore_unlock(const char* password, uint8_t* remaining_attemp
if (_is_unlocked_device) {
// Already unlocked. Fail if the seed changed under our feet (should never happen).
if (seed_len != _seed_length || !MEMEQ(_retained_seed, seed, _seed_length)) {
return KEYSTORE_ERR_GENERIC;
Abort("Seed has suddenly changed. This should never happen.");
}
} else {
memcpy(_retained_seed, seed, seed_len);
_seed_length = seed_len;
_is_unlocked_device = true;
}
if (!memory_reset_failed_unlock_attempts()) {
return KEYSTORE_ERR_GENERIC;
}
} else if (!memory_increment_failed_unlock_attempts()) {
return KEYSTORE_ERR_GENERIC;
bitbox02_smarteeprom_reset_unlock_attempts();
}
// Compute remaining attempts
failed_attempts = memory_get_failed_unlock_attempts();
if (failed_attempts >= MAX_UNLOCK_ATTEMPTS) { // checks for uint8 overflow
failed_attempts = bitbox02_smarteeprom_get_unlock_attempts();

if (failed_attempts >= MAX_UNLOCK_ATTEMPTS) {
*remaining_attempts_out = 0;
reset_reset(false);
return KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED;
}
*remaining_attempts_out = MAX_UNLOCK_ATTEMPTS - failed_attempts;

*remaining_attempts_out = MAX_UNLOCK_ATTEMPTS - failed_attempts;
return password_correct ? KEYSTORE_OK : KEYSTORE_ERR_INCORRECT_PASSWORD;
}

Expand Down
160 changes: 160 additions & 0 deletions src/memory/bitbox02_smarteeprom.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
// Copyright 2019 Shift Cryptosecurity AG
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "bitbox02_smarteeprom.h"

#include "hardfault.h"
#include "memory/mpu.h"
#include "memory/smarteeprom.h"

#include <stddef.h>
#include <string.h>

/**
* Current version of the data stored in SmartEEPROM.
*/
#define BITBOX02_SMARTEEPROM_DATA_VERSION (1)

#define BITBOX02_SMARTEEPROM_UNINITIALIZED_VERSION (0xFF)

/**
* Contents of the SmartEEPROM, as stored in the BitBox02.
* Version 1.
*
* Each field is stored in a separate SmartEEPROM page (32B) to help
* maximizing the lifetime of the device.
* The version is fixed at virtual address zero for every possible
* future structure (see _get_data_version).
*
* See smarteeprom_setup() for a description of how the SmartEEPROM
* is configured.
*/
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpacked"
typedef struct __attribute__((__packed__)) {
/** == 1 */
uint8_t version;
uint8_t padding[SMARTEEPROM_PAGE_SIZE - sizeof(uint8_t)];
/** Number of unlock attempts since last successful unlock. */
uint8_t unlock_attempts;
uint8_t padding_2[SMARTEEPROM_PAGE_SIZE - sizeof(uint8_t)];
} bitbox02_smarteeprom_image_v1_t;
#pragma GCC diagnostic pop

/**
* Reads the version field at address zero.
*
* The offset (in memory) of the version needs to stay out
* of the data structure. Otherwise, we need to know the version
* of the struct we're going to use in order to know the location
* of the version field... Which loses the point of the field itself.
*/
static uint8_t _get_data_version(void)
{
uint8_t result;
smarteeprom_read(0, sizeof(result), &result);
return result;
}

/**
* Reads the version field at address zero.
*/
static void _set_data_version(uint8_t new_version)
{
smarteeprom_write(0, sizeof(new_version), &new_version);
}

static void _init_v1(void)
{
bitbox02_smarteeprom_image_v1_t new_image;
smarteeprom_read(0, sizeof(new_image), (uint8_t*)&new_image);
/*
* Note that this forcefully resets the unlock counter
* the first and only time the device is upgraded.
* Not too bad...
*/
new_image.unlock_attempts = 0;
smarteeprom_write(0, sizeof(new_image), (uint8_t*)&new_image);
_set_data_version(0x01);
}

void bitbox02_smarteeprom_init(void)
{
uint8_t current_version = _get_data_version();
if (current_version == BITBOX02_SMARTEEPROM_DATA_VERSION) {
return;
}
/*
* Migrate from old versions.
* FUTURE: if the data structures are changed, add here the code
* to migrate from each version.
*/
if (current_version == BITBOX02_SMARTEEPROM_UNINITIALIZED_VERSION) {
_init_v1();
} else {
/*
* Incorrect version!
* Something has gone terribly wrong.
* Maybe reset the whole device?
*/
Abort("Unrecognized SmartEEPROM version.");
}
}

uint8_t bitbox02_smarteeprom_get_unlock_attempts(void)
{
uint8_t result;
smarteeprom_read(
offsetof(bitbox02_smarteeprom_image_v1_t, unlock_attempts),
sizeof(((bitbox02_smarteeprom_image_v1_t*)0)->unlock_attempts),
&result);
/*
* Sanity-check the value.
*
* At no point this number should be allowed to go above MAX_UNLOCK_ATTEMPTS.
*/
if (result > MAX_UNLOCK_ATTEMPTS) {
Abort("#Unlock attempts increased past the maximum allowed value.");
}
return result;
}

void bitbox02_smarteeprom_increment_unlock_attempts(void)
{
uint8_t unlock_attempts = bitbox02_smarteeprom_get_unlock_attempts();
/*
* The number of attempts can never grow past the maximum.
* Catch an attempt to increment it past the maximum before we
* do the actual increment.
*/
if (unlock_attempts == MAX_UNLOCK_ATTEMPTS) {
Abort("Tried to increment the number of unlocks past the maximum allowed value.");
}
unlock_attempts++;
smarteeprom_write(
offsetof(bitbox02_smarteeprom_image_v1_t, unlock_attempts),
sizeof(unlock_attempts),
&unlock_attempts);
}

void bitbox02_smarteeprom_reset_unlock_attempts(void)
{
/* Sanity-check the current value */
(void)bitbox02_smarteeprom_get_unlock_attempts();
uint8_t w_unlock_attempts = 0;
smarteeprom_write(
offsetof(bitbox02_smarteeprom_image_v1_t, unlock_attempts),
sizeof(w_unlock_attempts),
&w_unlock_attempts);
}
53 changes: 53 additions & 0 deletions src/memory/bitbox02_smarteeprom.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2019 Shift Cryptosecurity AG
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef _BITBOX02_SMARTEEPROM_H
#define _BITBOX02_SMARTEEPROM_H

#include <stdint.h>

/**
* After this many failed unlock attempts, the keystore becomes locked until a
* device reset.
*/
#define MAX_UNLOCK_ATTEMPTS (10)

/**
* Reads and validates the last recorded number of unlock attempts.
*/
uint8_t bitbox02_smarteeprom_get_unlock_attempts(void);

/**
* Increments the recorded number of unlock attempts.
*
* This will fail if the number of unlock attempts would
* be increased past the allowed maximum.
*/
void bitbox02_smarteeprom_increment_unlock_attempts(void);

/**
* Resets the recorded number of unlock attempts to 0.
*
* This will fail if the currently recorded number of unlock attempts
* is invalid.
*/
void bitbox02_smarteeprom_reset_unlock_attempts(void);

/**
* Makes sure that the contents of the SmartEEPROM in the BitBox02
* are up-to-date with the latest struct definition.
*/
void bitbox02_smarteeprom_init(void);

#endif // _BITBOX02_SMARTEEPROM_H
9 changes: 9 additions & 0 deletions src/memory/smarteeprom.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,13 @@ void smarteeprom_write(size_t address, size_t bytes, const uint8_t* buffer)
;
while (NVMCTRL->SEESTAT.bit.BUSY != 0)
;
/*
* Read back the buffer.
* Check that it matches what we've just written.
*/
uint8_t read_buf[bytes];
smarteeprom_read(address, bytes, read_buf);
if (memcmp(read_buf, buffer, bytes)) {
Abort("Write to SmartEEPROM failed to verify data.");
}
}
2 changes: 1 addition & 1 deletion test/unit-test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations -Wno-implicit-function-declaration -Wno-bad-function-cast")


set(DBB-FILTERED-SOURCES
${DBB-FIRMWARE-SOURCES}
${DBB-FIRMWARE-UI-SOURCES}
Expand Down Expand Up @@ -60,6 +59,7 @@ add_library(bitbox
framework/mock_component.c
framework/mock_ff.c
framework/mock_sd.c
framework/mock_smarteeprom.c
framework/mock_securechip.c
)

Expand Down
19 changes: 0 additions & 19 deletions test/unit-test/framework/mock_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,6 @@ bool __wrap_memory_is_seeded(void)
return mock();
}

static uint32_t _failed_unlock_attempts = 0;

uint8_t __wrap_memory_get_failed_unlock_attempts(void)
{
return (uint8_t)_failed_unlock_attempts;
}

bool __wrap_memory_reset_failed_unlock_attempts(void)
{
_failed_unlock_attempts = 0;
return true;
}

bool __wrap_memory_increment_failed_unlock_attempts(void)
{
_failed_unlock_attempts++;
return true;
}

static uint8_t _encrypted_seed_and_hmac[96];
static uint8_t _encrypted_seed_and_hmac_len;

Expand Down
Loading

0 comments on commit cff33d0

Please sign in to comment.