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 <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
diff --git a/tests/vboot_api_kernel2_tests.c b/tests/vboot_api_kernel2_tests.c
index 21ea306..9dd15ef 100644
--- a/tests/vboot_api_kernel2_tests.c
+++ b/tests/vboot_api_kernel2_tests.c
@@ -34,9 +34,12 @@
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 @@
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 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 @@
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 @@
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 @@
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 @@
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");