From a53a0b040f45a1086515e7a5c8a8326c0b1d1f74 Mon Sep 17 00:00:00 2001 From: Luigi Semenzato Date: Fri, 10 Jan 2014 16:26:08 -0800 Subject: vboot: use recovery button as dev mode switch confirmation We don't allow ENTER from a USB keyboard as the confirmation in the switch from normal to developer mode. For devices that have a physical recovery button, we require a recovery button press instead. For other devices, we require that ENTER be pressed on the internal keyboard. This prevents an "evil keyboard" attack in which a USB keyboard (or other USB device pretending to be a keyboard) sends a control-D/ENTER sequence shortly after every boot (followed by more evil keys). In that situation, when users power-on in recovery mode, they will be forced to dev mode even if it was not their intention. Further attacks are easy at that point. TESTING. On a panther device: 1. powered on with recovery button pressed -> booted in recovery mode 2. pressed control-D on external USB keyboard -> got to ToDev? screen 3. pressed ENTER -> system beeped 4. pressed recovery button -> system rebooted in DEV mode ... all as expected Also: 1. powered on with recovery button pressed and HELD recovery button 2. pressed control-D -> system beeped BUG=chrome-os-partner:21729 TEST=manual (see commit message) BRANCH=none CQ-DEPEND=CL:182420,CL:182946,CL:182357 Change-Id: Ib986d00d4567c2d447f8bbff0e5ccfec94596aa7 Reviewed-on: https://chromium-review.googlesource.com/182241 Reviewed-by: Luigi Semenzato Tested-by: Luigi Semenzato Commit-Queue: Luigi Semenzato --- tests/vboot_api_kernel2_tests.c | 81 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-) (limited to 'tests/vboot_api_kernel2_tests.c') diff --git a/tests/vboot_api_kernel2_tests.c b/tests/vboot_api_kernel2_tests.c index 21ea3063..9dd15ef3 100644 --- a/tests/vboot_api_kernel2_tests.c +++ b/tests/vboot_api_kernel2_tests.c @@ -34,9 +34,12 @@ static int vbexlegacy_called; static int trust_ec; static int virtdev_set; static uint32_t virtdev_retval; - static uint32_t mock_keypress[8]; +static uint32_t mock_keyflags[8]; static uint32_t mock_keypress_count; +static uint32_t mock_switches[8]; +static uint32_t mock_switches_count; +static int mock_switches_are_stuck; static uint32_t screens_displayed[8]; static uint32_t screens_count = 0; static uint32_t mock_num_disks[8]; @@ -81,8 +84,13 @@ static void ResetMocks(void) screens_count = 0; Memset(mock_keypress, 0, sizeof(mock_keypress)); + Memset(mock_keyflags, 0, sizeof(mock_keyflags)); mock_keypress_count = 0; + Memset(mock_switches, 0, sizeof(mock_switches)); + mock_switches_count = 0; + mock_switches_are_stuck = 0; + Memset(mock_num_disks, 0, sizeof(mock_num_disks)); mock_num_disks_count = 0; } @@ -101,8 +109,25 @@ uint32_t VbExIsShutdownRequested(void) uint32_t VbExKeyboardRead(void) { - if (mock_keypress_count < ARRAY_SIZE(mock_keypress)) + return VbExKeyboardReadWithFlags(NULL); +} + +uint32_t VbExKeyboardReadWithFlags(uint32_t *key_flags) +{ + if (mock_keypress_count < ARRAY_SIZE(mock_keypress)) { + if (key_flags != NULL) + *key_flags = mock_keyflags[mock_keypress_count]; return mock_keypress[mock_keypress_count++]; + } else + return 0; +} + +uint32_t VbExGetSwitches(uint32_t request_mask) +{ + if (mock_switches_are_stuck) + return mock_switches[0] & request_mask; + if (mock_switches_count < ARRAY_SIZE(mock_switches)) + return mock_switches[mock_switches_count++] & request_mask; else return 0; } @@ -190,13 +215,47 @@ static void VbUserConfirmsTest(void) ResetMocks(); mock_keypress[0] = ' '; shutdown_request_calls_left = 1; - TEST_EQ(VbUserConfirms(&cparams, 1), 0, "Space means no"); + TEST_EQ(VbUserConfirms(&cparams, VB_CONFIRM_SPACE_MEANS_NO), 0, + "Space means no"); ResetMocks(); mock_keypress[0] = ' '; shutdown_request_calls_left = 1; TEST_EQ(VbUserConfirms(&cparams, 0), -1, "Space ignored"); + ResetMocks(); + mock_keypress[0] = '\r'; + mock_keyflags[0] = VB_KEY_FLAG_TRUSTED_KEYBOARD; + TEST_EQ(VbUserConfirms(&cparams, VB_CONFIRM_MUST_TRUST_KEYBOARD), + 1, "Enter with trusted keyboard"); + + ResetMocks(); + mock_keypress[0] = '\r'; /* untrusted */ + mock_keypress[1] = ' '; + TEST_EQ(VbUserConfirms(&cparams, + VB_CONFIRM_SPACE_MEANS_NO | + VB_CONFIRM_MUST_TRUST_KEYBOARD), + 0, "Untrusted keyboard"); + + ResetMocks(); + mock_switches[0] = VB_INIT_FLAG_REC_BUTTON_PRESSED; + TEST_EQ(VbUserConfirms(&cparams, + VB_CONFIRM_SPACE_MEANS_NO | + VB_CONFIRM_MUST_TRUST_KEYBOARD), + 1, "Recovery button"); + + ResetMocks(); + mock_keypress[0] = '\r'; + mock_keypress[1] = 'y'; + mock_keypress[2] = 'z'; + mock_keypress[3] = ' '; + mock_switches[0] = VB_INIT_FLAG_REC_BUTTON_PRESSED; + mock_switches_are_stuck = 1; + TEST_EQ(VbUserConfirms(&cparams, + VB_CONFIRM_SPACE_MEANS_NO | + VB_CONFIRM_MUST_TRUST_KEYBOARD), + 0, "Recovery button stuck"); + printf("...done.\n"); } @@ -529,6 +588,20 @@ static void VbBootRecTest(void) TEST_NEQ(screens_displayed[1], VB_SCREEN_RECOVERY_TO_DEV, " todev screen"); + /* Ctrl+D ignored because the physical recovery switch is still pressed + * and we don't like that. + */ + ResetMocks(); + shared->flags = VBSD_BOOT_REC_SWITCH_ON; + trust_ec = 1; + shutdown_request_calls_left = 100; + mock_keypress[0] = 0x04; + mock_switches[0] = VB_INIT_FLAG_REC_BUTTON_PRESSED; + TEST_EQ(VbBootRecovery(&cparams, &lkp), VBERROR_SHUTDOWN_REQUESTED, + "Ctrl+D ignored if phys rec button is still pressed"); + TEST_NEQ(screens_displayed[1], VB_SCREEN_RECOVERY_TO_DEV, + " todev screen"); + /* Ctrl+D then space means don't enable */ ResetMocks(); shared->flags = VBSD_HONOR_VIRT_DEV_SWITCH | VBSD_BOOT_REC_SWITCH_ON; @@ -555,6 +628,7 @@ static void VbBootRecTest(void) trust_ec = 1; mock_keypress[0] = 0x04; mock_keypress[1] = '\r'; + mock_keyflags[1] = VB_KEY_FLAG_TRUSTED_KEYBOARD; TEST_EQ(VbBootRecovery(&cparams, &lkp), VBERROR_TPM_REBOOT_REQUIRED, "Ctrl+D todev confirm"); TEST_EQ(virtdev_set, 1, " virtual dev mode on"); @@ -567,6 +641,7 @@ static void VbBootRecTest(void) trust_ec = 1; mock_keypress[0] = 0x04; mock_keypress[1] = '\r'; + mock_keyflags[1] = VB_KEY_FLAG_TRUSTED_KEYBOARD; virtdev_retval = VBERROR_SIMULATED; TEST_EQ(VbBootRecovery(&cparams, &lkp), VBERROR_TPM_SET_BOOT_MODE_STATE, "Ctrl+D todev failure"); -- cgit v1.2.1