Skip to content

Commit

Permalink
Prevent re-onboarding (#115)
Browse files Browse the repository at this point in the history 
- Preventing SEED and WIPE commands from executing if device is already onboarded
- Preventing onboarding tool from executing onboarding if device is already onboarded
- Added unit test cases for firmware and middleware changes
- Improving output for HSM2DongleErrorResult errors
  • Loading branch information
@amendelzon
amendelzon authored Jan 13, 2023
1 parent 1a8eb39 commit c51154c
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 15 deletions.
8 changes: 8 additions & 0 deletions ledger/src/ui/src/bootloader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ static unsigned char current_cmd;
// Flag used to prevent executing commands after the onboard is performed
static bool onboard_performed = false;

// Macro that throws an error unless
// the device is not onboarded
#define REQUIRE_NOT_ONBOARDED() \
if (os_perso_isonboarded() == 1) \
THROW(ERR_DEVICE_ONBOARDED);

/*
* Reset all reseteable operations, only if the given operation is starting.
*
Expand Down Expand Up @@ -96,6 +102,7 @@ unsigned int bootloader_process_apdu(volatile unsigned int rx,
// unauthenticated instruction
switch (APDU_CMD()) {
case RSK_SEED_CMD: // Send wordlist
REQUIRE_NOT_ONBOARDED();
reset_if_starting(RSK_META_CMD_UIOP);
tx = set_host_seed(rx, &onboard_ctx);
break;
Expand All @@ -108,6 +115,7 @@ unsigned int bootloader_process_apdu(volatile unsigned int rx,
tx = is_onboarded();
break;
case RSK_WIPE: //--- wipe and onboard device ---
REQUIRE_NOT_ONBOARDED();
reset_if_starting(RSK_META_CMD_UIOP);
tx = onboard_device(&onboard_ctx);
clear_pin();
Expand Down
1 change: 1 addition & 0 deletions ledger/src/ui/src/err.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ typedef enum {
ERR_INVALID_CLA = 0x6E22,
ERR_INTERNAL = 0x6A99,
ERR_INVALID_PIN = 0x69A0,
ERR_DEVICE_ONBOARDED = 0x69A1,

EX_BOOTLOADER_RSK_END = 0x90FF,
} err_code_ui_t;
Expand Down
107 changes: 107 additions & 0 deletions ledger/src/ui/test/bootloader/test_bootloader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ bolos_ux_context_t G_bolos_ux_context;
static bootloader_mode_t G_bootloader_mode = BOOTLOADER_MODE_DEFAULT;
static bool G_host_seed_is_set = false;
static bool G_pin_buffer_updated = false;
static unsigned int G_device_onboarded = 0;
static bool G_is_onboarded = false;
static bool G_is_pin_set = false;
static bool G_is_pin_buffer_cleared = false;
Expand All @@ -62,6 +63,7 @@ static void reset_flags() {
G_host_seed_is_set = false;
G_pin_buffer_updated = false;
G_is_onboarded = false;
G_device_onboarded = 0;
G_is_pin_set = false;
G_is_pin_buffer_cleared = false;
G_get_attestation_called = false;
Expand All @@ -88,6 +90,10 @@ unsigned int update_pin_buffer(volatile unsigned int rx) {
return 3;
}

unsigned int os_perso_isonboarded(void) {
return G_device_onboarded;
}

unsigned int is_onboarded() {
G_is_onboarded_called = true;
SET_APDU_AT(1, G_is_onboarded);
Expand Down Expand Up @@ -195,6 +201,36 @@ void test_seed() {
assert(RESET_IF_STARTED_CALLED());
}

void test_seed_onboarded() {
printf("Test RSK_SEED_CMD when onboarded...\n");

unsigned int rx;
unsigned int tx;
bootloader_init();
reset_flags();
G_bootloader_mode = BOOTLOADER_MODE_DEFAULT;
G_device_onboarded = 1;
G_host_seed_is_set = false;
SET_APDU("\x80\x44", rx); // RSK_SEED_CMD
BEGIN_TRY {
TRY {
bootloader_process_apdu(rx, G_bootloader_mode);
// bootloader_process_apdu should throw EX_BOOTLOADER_RSK_END
ASSERT_FAIL();
}
CATCH(ERR_DEVICE_ONBOARDED) {
assert(!G_host_seed_is_set);
return;
}
CATCH_OTHER(e) {
ASSERT_FAIL();
}
FINALLY {
}
}
END_TRY;
}

void test_pin() {
printf("Test RSK_PIN_CMD...\n");

Expand Down Expand Up @@ -258,6 +294,40 @@ void test_wipe_default_mode() {
assert(RESET_IF_STARTED_CALLED());
}

void test_wipe_default_mode_onboarded() {
printf("Test RSK_WIPE (default mode) when onboarded...\n");

unsigned int rx;
unsigned int tx;
bootloader_init();
reset_flags();
G_bootloader_mode = BOOTLOADER_MODE_DEFAULT;
G_device_onboarded = 1;
G_is_onboarded = false;
G_is_pin_buffer_cleared = false;
G_is_pin_set = true;
SET_APDU("\x80\x07", rx); // RSK_WIPE
BEGIN_TRY {
TRY {
bootloader_process_apdu(rx, G_bootloader_mode);
// bootloader_process_apdu should throw EX_BOOTLOADER_RSK_END
ASSERT_FAIL();
}
CATCH(ERR_DEVICE_ONBOARDED) {
assert(!G_is_onboarded);
assert(!G_is_pin_buffer_cleared);
assert(G_is_pin_set);
return;
}
CATCH_OTHER(e) {
ASSERT_FAIL();
}
FINALLY {
}
}
END_TRY;
}

void test_wipe_onboard_mode() {
printf("Test RSK_WIPE (onboard mode)...\n");

Expand All @@ -277,6 +347,40 @@ void test_wipe_onboard_mode() {
assert(RESET_IF_STARTED_CALLED());
}

void test_wipe_onboard_mode_onboarded() {
printf("Test RSK_WIPE (onboard mode) when onboarded...\n");

unsigned int rx;
unsigned int tx;
bootloader_init();
reset_flags();
G_bootloader_mode = BOOTLOADER_MODE_ONBOARD;
G_device_onboarded = 1;
G_is_onboarded = false;
G_is_pin_buffer_cleared = false;
G_is_pin_set = true;
SET_APDU("\x80\x07", rx); // RSK_WIPE
BEGIN_TRY {
TRY {
bootloader_process_apdu(rx, G_bootloader_mode);
// bootloader_process_apdu should throw EX_BOOTLOADER_RSK_END
ASSERT_FAIL();
}
CATCH(ERR_DEVICE_ONBOARDED) {
assert(!G_is_onboarded);
assert(!G_is_pin_buffer_cleared);
assert(G_is_pin_set);
return;
}
CATCH_OTHER(e) {
ASSERT_FAIL();
}
FINALLY {
}
}
END_TRY;
}

void test_newpin() {
printf("Test RSK_NEWPIN...\n");

Expand Down Expand Up @@ -559,10 +663,13 @@ void test_onboard_mode() {
int main() {
test_init();
test_seed();
test_seed_onboarded();
test_pin();
test_is_onboard();
test_wipe_default_mode();
test_wipe_default_mode_onboarded();
test_wipe_onboard_mode();
test_wipe_onboard_mode_onboarded();
test_newpin();
test_echo();
test_mode();
Expand Down
7 changes: 3 additions & 4 deletions middleware/admin/onboard.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,9 @@ def do_onboard(options):
info(f"Onboarded: {bls(is_onboarded)}")

if is_onboarded:
message = ("WARNING: The following operation will wipe the device and "
"generate a new seed. This cannot be undone.")
else:
message = "The following operation will onboard the device."
raise AdminError("Device already onboarded")

message = "The following operation will onboard the device."
head([
message,
"Do you want to proceed? Yes/No",
Expand Down
3 changes: 3 additions & 0 deletions middleware/ledger/hsm2dongle.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ class HSM2DongleErrorResult(HSM2DongleBaseError):
def error_code(self):
return self.args[0]

def __str__(self):
return f"Dongle returned error code {hex(self.error_code)}"


# Handles low-level communication with a powHSM dongle
class HSM2Dongle:
Expand Down
41 changes: 30 additions & 11 deletions middleware/tests/admin/test_onboard.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def test_onboard(self, readline, get_hsm_unlock, get_admin_hsm,
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(side_effect=[False, True])
self.dongle.get_device_key = Mock(return_value=self.DEVICE_KEY)
self.dongle.setup_endorsement_key = Mock(return_value=self.ENDORSEMENT_KEY)
self.dongle.handshake = Mock()
Expand All @@ -112,7 +112,7 @@ def test_onboard(self, readline, get_hsm_unlock, get_admin_hsm,
with patch("builtins.open", mock_open()) as file_mock:
do_onboard(self.default_options)

self.assertEqual(info_mock.call_args_list[5][0][0], "Onboarded: Yes")
self.assertEqual(info_mock.call_args_list[5][0][0], "Onboarded: No")
self.assertEqual(info_mock.call_args_list[10][0][0], "Onboarded")
self.assertEqual(info_mock.call_args_list[14][0][0], "Device key gathered")
self.assertEqual(info_mock.call_args_list[16][0][0],
Expand All @@ -125,6 +125,25 @@ def test_onboard(self, readline, get_hsm_unlock, get_admin_hsm,
self.assertTrue(self.dongle.onboard.called)
self.assertTrue(self.dongle.handshake.called)

@patch("admin.onboard.get_admin_hsm")
@patch("admin.unlock.get_hsm")
@patch("sys.stdin.readline")
def test_onboard_already_onboarded(self, readline, get_hsm_unlock, get_admin_hsm,
get_hsm_onboard, info_mock, *_):
get_hsm_onboard.return_value = self.dongle
get_hsm_unlock.return_value = self.dongle
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)

with self.assertRaises(AdminError) as e:
do_onboard(self.default_options)

self.assertEqual(info_mock.call_args_list[5][0][0], "Onboarded: Yes")
self.assertEqual(e.exception.args[0], "Device already onboarded")
self.assertFalse(self.dongle.onboard.called)

@patch("admin.onboard.get_admin_hsm")
@patch("admin.unlock.get_hsm")
@patch("sys.stdin.readline")
Expand All @@ -135,7 +154,7 @@ def test_onboard_onboard_error(self, readline, get_hsm_unlock, get_admin_hsm,
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(return_value=False)
self.dongle.get_device_key = Mock()
self.dongle.setup_endorsement_key = Mock()
self.dongle.handshake = Mock()
Expand Down Expand Up @@ -163,7 +182,7 @@ def test_onboard_handshake_error(self, readline, get_hsm_unlock, get_admin_hsm,
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(side_effect=[False, True])
self.dongle.get_device_key = Mock()
self.dongle.setup_endorsement_key = Mock()
self.dongle.handshake = Mock(side_effect=HSM2DongleError("error-msg"))
Expand Down Expand Up @@ -191,7 +210,7 @@ def test_onboard_getkey_error(self, readline, get_hsm_unlock, get_admin_hsm,
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(side_effect=[False, True])
self.dongle.get_device_key = Mock(side_effect=HSM2DongleError("error-msg"))
self.dongle.setup_endorsement_key = Mock()
self.dongle.handshake = Mock()
Expand Down Expand Up @@ -219,7 +238,7 @@ def test_onboard_setupkey_error(self, readline, get_hsm_unlock, get_admin_hsm,
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(side_effect=[False, True])
self.dongle.get_device_key = Mock()
self.dongle.setup_endorsement_key = Mock(side_effect=HSM2DongleError("error-msg"))
self.dongle.handshake = Mock()
Expand All @@ -245,7 +264,7 @@ def test_onboard_user_cancelled(self, readline, hsm_unlock, hsm_admin,
hsm_onboard.return_value = self.dongle
hsm_unlock.return_value = self.dongle
self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(return_value=False)
self.dongle.onboard = Mock()
hsm_admin.return_value = self.dongle
readline.return_value = "no\n"
Expand All @@ -266,7 +285,7 @@ def test_onboard_no_output_file(self, readline, get_hsm, *_):
get_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(return_value=False)
self.dongle.onboard = Mock()

options = self.default_options
Expand All @@ -288,7 +307,7 @@ def test_onboard_invalid_pin(self, *_):
def test_onboard_invalid_mode(self, get_hsm, *_):
get_hsm.return_value = self.dongle
self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.APP)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(return_value=False)

with self.assertRaises(AdminError) as e:
do_onboard(self.default_options)
Expand All @@ -305,7 +324,7 @@ def test_onboard_invalid_device_key(self, readline, get_hsm_unlock, get_admin_hs
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(side_effect=[False, True])
self.dongle.get_device_key = Mock(return_value=self.INVALID_KEY)
self.dongle.setup_endorsement_key = Mock(return_value=self.ENDORSEMENT_KEY)
readline.return_value = "yes\n"
Expand All @@ -326,7 +345,7 @@ def test_onboard_invalid_attestation_key(self, readline, get_hsm_unlock,
get_admin_hsm.return_value = self.dongle

self.dongle.get_current_mode = Mock(return_value=HSM2Dongle.MODE.BOOTLOADER)
self.dongle.is_onboarded = Mock(return_value=True)
self.dongle.is_onboarded = Mock(side_effect=[False, True])
self.dongle.get_device_key = Mock(return_value=self.DEVICE_KEY)
self.dongle.setup_endorsement_key = Mock(return_value=self.INVALID_KEY)
readline.return_value = "yes\n"
Expand Down

0 comments on commit c51154c

Please sign in to comment.