From 4bcb8ceb90fba5b15ab5fc650fb09314ded47d63 Mon Sep 17 00:00:00 2001 From: Joel Kitching Date: Fri, 16 Aug 2019 16:03:50 +0800 Subject: vboot/secdata: move permissions and uid check to ReadSpaceKernel Relocate permissions and uid check from RollbackKernelRead to ReadSpaceKernel. Restructure test code to set default values in ResetMocks. BUG=b:124141368, chromium:972956 TEST=make clean && make runtests BRANCH=none Change-Id: I72c536042b89684c6db5099412344678e3d9d920 Signed-off-by: Joel Kitching Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1758146 Reviewed-by: Julius Werner Tested-by: Joel Kitching Commit-Queue: Joel Kitching --- firmware/lib/rollback_index.c | 43 +++++++++---------- tests/rollback_index2_tests.c | 98 ++++++++++++++++++++++++++----------------- 2 files changed, 78 insertions(+), 63 deletions(-) diff --git a/firmware/lib/rollback_index.c b/firmware/lib/rollback_index.c index b7549bba..b0d4eb58 100644 --- a/firmware/lib/rollback_index.c +++ b/firmware/lib/rollback_index.c @@ -141,6 +141,22 @@ vb2_error_t SetVirtualDevMode(int val) uint32_t ReadSpaceKernel(RollbackSpaceKernel *rsk) { +#ifndef TPM2_MODE + /* + * Before reading the kernel space, verify its permissions. If the + * kernel space has the wrong permission, we give up. This will need + * to be fixed by the recovery kernel. We will have to worry about + * this because at any time (even with PP turned off) the TPM owner can + * remove and redefine a PP-protected space (but not write to it). + */ + uint32_t perms; + + RETURN_ON_FAILURE(TlclGetPermissions(KERNEL_NV_INDEX, &perms)); + + if (perms != TPM_NV_PER_PPWRITE) + return TPM_E_CORRUPTED_STATE; +#endif + uint32_t r; r = TlclRead(KERNEL_NV_INDEX, rsk, sizeof(RollbackSpaceKernel)); @@ -153,6 +169,9 @@ uint32_t ReadSpaceKernel(RollbackSpaceKernel *rsk) if (rsk->struct_version < ROLLBACK_SPACE_FIRMWARE_VERSION) return TPM_E_STRUCT_VERSION; + if (rsk->uid != ROLLBACK_SPACE_KERNEL_UID) + return TPM_E_CORRUPTED_STATE; + if (rsk->crc8 != vb2_crc8(rsk, offsetof(RollbackSpaceKernel, crc8))) { VB2_DEBUG("TPM: bad secdatak CRC\n"); return TPM_E_CORRUPTED_STATE; @@ -207,31 +226,7 @@ uint32_t RollbackFwmpRead(struct RollbackSpaceFwmp *fwmp) uint32_t RollbackKernelRead(uint32_t* version) { RollbackSpaceKernel rsk; - - /* - * Read the kernel space and verify its permissions. If the kernel - * space has the wrong permission, or it doesn't contain the right - * identifier, we give up. This will need to be fixed by the - * recovery kernel. We have to worry about this because at any time - * (even with PP turned off) the TPM owner can remove and redefine a - * PP-protected space (but not write to it). - */ RETURN_ON_FAILURE(ReadSpaceKernel(&rsk)); -#ifndef TPM2_MODE - /* - * TODO(vbendeb): restore this when it is defined how the kernel space - * gets protected. - */ - { - uint32_t perms, uid; - - RETURN_ON_FAILURE(TlclGetPermissions(KERNEL_NV_INDEX, &perms)); - memcpy(&uid, &rsk.uid, sizeof(uid)); - if (TPM_NV_PER_PPWRITE != perms || - ROLLBACK_SPACE_KERNEL_UID != uid) - return TPM_E_CORRUPTED_STATE; - } -#endif memcpy(version, &rsk.kernel_versions, sizeof(*version)); VB2_DEBUG("TPM: RollbackKernelRead 0x%x\n", (int)*version); return TPM_SUCCESS; diff --git a/tests/rollback_index2_tests.c b/tests/rollback_index2_tests.c index a092406d..9df8b06d 100644 --- a/tests/rollback_index2_tests.c +++ b/tests/rollback_index2_tests.c @@ -75,9 +75,20 @@ static void ResetMocks(int fail_on_call, uint32_t fail_with_err) fail_with_error = fail_with_err; memset(&mock_pflags, 0, sizeof(mock_pflags)); + memset(&mock_rsf, 0, sizeof(mock_rsf)); + mock_rsf.struct_version = ROLLBACK_SPACE_FIRMWARE_VERSION; + mock_rsf.crc8 = vb2_crc8(&mock_rsf, + offsetof(RollbackSpaceFirmware, crc8)); + memset(&mock_rsk, 0, sizeof(mock_rsk)); - mock_permissions = 0; + mock_rsk.uid = ROLLBACK_SPACE_KERNEL_UID; + mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; + mock_rsk.kernel_versions = 0x87654321; + mock_rsk.crc8 = vb2_crc8(&mock_rsk, + offsetof(RollbackSpaceKernel, crc8)); + + mock_permissions = TPM_NV_PER_PPWRITE; memset(mock_fwmp.buf, 0, sizeof(mock_fwmp.buf)); mock_fwmp.fwmp.struct_size = sizeof(mock_fwmp.fwmp); @@ -250,31 +261,30 @@ static void FirmwareSpaceTest(void) { RollbackSpaceFirmware rsf; - /* A read with old version should fail */ + /* Old version, valid CRC */ ResetMocks(0, 0); - mock_rsf.struct_version = ROLLBACK_SPACE_FIRMWARE_VERSION - 1; + mock_rsf.struct_version -= 1; + mock_rsf.crc8 = vb2_crc8(&mock_rsf, + offsetof(RollbackSpaceFirmware, crc8)); TEST_EQ(ReadSpaceFirmware(&rsf), TPM_E_STRUCT_VERSION, "ReadSpaceFirmware(), old version"); TEST_STR_EQ(mock_calls, "TlclRead(0x1007, 10)\n", "tlcl calls"); - /* A read with current version should fail on bad CRC */ + /* Current version, bad CRC */ ResetMocks(0, 0); - mock_rsf.struct_version = ROLLBACK_SPACE_FIRMWARE_VERSION; + mock_rsf.crc8 = 0; TEST_EQ(ReadSpaceFirmware(&rsf), TPM_E_CORRUPTED_STATE, - "ReadSpaceFirmware(), current version, bad CRC"); + "ReadSpaceFirmware(), bad CRC"); TEST_STR_EQ(mock_calls, "TlclRead(0x1007, 10)\n", "tlcl calls"); - /* Current version with valid CRC */ + /* Current version, valid CRC */ ResetMocks(0, 0); - mock_rsf.struct_version = ROLLBACK_SPACE_FIRMWARE_VERSION; - mock_rsf.crc8 = vb2_crc8(&mock_rsf, - offsetof(RollbackSpaceFirmware, crc8)); TEST_EQ(ReadSpaceFirmware(&rsf), 0, - "ReadSpaceFirmware(), current version, good CRC"); + "ReadSpaceFirmware(), successful read"); TEST_STR_EQ(mock_calls, "TlclRead(0x1007, 10)\n", "tlcl calls"); @@ -287,32 +297,55 @@ static void KernelSpaceTest(void) { RollbackSpaceKernel rsk; - /* A read with version old version should fail */ + /* Current version, bad perms, valid CRC, valid UID */ + ResetMocks(0, 0); + mock_permissions = 0; + TEST_EQ(ReadSpaceKernel(&rsk), TPM_E_CORRUPTED_STATE, + "ReadSpaceKernel(), bad permissions"); + TEST_STR_EQ(mock_calls, + "TlclGetPermissions(0x1008)\n", + "tlcl calls"); + + /* Old version, good perms, valid CRC, valid UID */ ResetMocks(0, 0); - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION - 1; + mock_rsk.struct_version -= 1; + mock_rsk.crc8 = vb2_crc8(&mock_rsk, + offsetof(RollbackSpaceKernel, crc8)); TEST_EQ(ReadSpaceKernel(&rsk), TPM_E_STRUCT_VERSION, "ReadSpaceKernel(), old version"); TEST_STR_EQ(mock_calls, + "TlclGetPermissions(0x1008)\n" "TlclRead(0x1008, 13)\n", "tlcl calls"); - /* A read with current version should fail on bad CRC */ + /* Current version, good perms, bad CRC, valid UID */ ResetMocks(0, 0); - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; + mock_rsk.crc8 = 0; TEST_EQ(ReadSpaceKernel(&rsk), TPM_E_CORRUPTED_STATE, - "ReadSpaceKernel(), current version, bad CRC"); + "ReadSpaceKernel(), bad CRC"); TEST_STR_EQ(mock_calls, + "TlclGetPermissions(0x1008)\n" "TlclRead(0x1008, 13)\n", "tlcl calls"); - /* Current version with valid CRC */ + /* Current version, good perms, valid CRC, bad UID */ ResetMocks(0, 0); - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; + mock_rsk.uid = 0; mock_rsk.crc8 = vb2_crc8(&mock_rsk, offsetof(RollbackSpaceKernel, crc8)); + TEST_EQ(ReadSpaceKernel(&rsk), TPM_E_CORRUPTED_STATE, + "ReadSpaceKernel(), bad UID"); + TEST_STR_EQ(mock_calls, + "TlclGetPermissions(0x1008)\n" + "TlclRead(0x1008, 13)\n", + "tlcl calls"); + + /* Current version, good perms, valid CRC, valid UID */ + ResetMocks(0, 0); TEST_EQ(ReadSpaceKernel(&rsk), 0, - "ReadSpaceKernel(), current version, good CRC"); + "ReadSpaceKernel(), successful read"); TEST_STR_EQ(mock_calls, + "TlclGetPermissions(0x1008)\n" "TlclRead(0x1008, 13)\n", "tlcl calls"); } @@ -364,16 +397,10 @@ static void RollbackKernelTest(void) /* Normal read */ ResetMocks(0, 0); - mock_rsk.uid = ROLLBACK_SPACE_KERNEL_UID; - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; - mock_rsk.kernel_versions = 0x87654321; - mock_rsk.crc8 = vb2_crc8(&mock_rsk, - offsetof(RollbackSpaceKernel, crc8)); - mock_permissions = TPM_NV_PER_PPWRITE; TEST_EQ(RollbackKernelRead(&version), 0, "RollbackKernelRead()"); TEST_STR_EQ(mock_calls, - "TlclRead(0x1008, 13)\n" - "TlclGetPermissions(0x1008)\n", + "TlclGetPermissions(0x1008)\n" + "TlclRead(0x1008, 13)\n", "tlcl calls"); TEST_EQ(version, 0x87654321, "RollbackKernelRead() version"); @@ -381,34 +408,27 @@ static void RollbackKernelTest(void) ResetMocks(1, TPM_E_IOERROR); TEST_EQ(RollbackKernelRead(&version), TPM_E_IOERROR, "RollbackKernelRead() error"); - TEST_STR_EQ(mock_calls, - "TlclRead(0x1008, 13)\n", - "tlcl calls"); /* Wrong permission or UID will return error */ ResetMocks(0, 0); - mock_rsk.uid = ROLLBACK_SPACE_KERNEL_UID + 1; - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; - mock_permissions = TPM_NV_PER_PPWRITE; + mock_rsk.uid = 0; + mock_rsk.crc8 = vb2_crc8(&mock_rsk, + offsetof(RollbackSpaceKernel, crc8)); TEST_EQ(RollbackKernelRead(&version), TPM_E_CORRUPTED_STATE, "RollbackKernelRead() bad uid"); ResetMocks(0, 0); - mock_rsk.uid = ROLLBACK_SPACE_KERNEL_UID; - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; - mock_permissions = TPM_NV_PER_PPWRITE + 1; + mock_permissions = 0; TEST_EQ(RollbackKernelRead(&version), TPM_E_CORRUPTED_STATE, "RollbackKernelRead() bad permissions"); /* Test write */ ResetMocks(0, 0); - mock_rsk.struct_version = ROLLBACK_SPACE_KERNEL_VERSION; - mock_rsk.crc8 = vb2_crc8(&mock_rsk, - offsetof(RollbackSpaceKernel, crc8)); TEST_EQ(RollbackKernelWrite(0xBEAD4321), 0, "RollbackKernelWrite()"); TEST_EQ(mock_rsk.kernel_versions, 0xBEAD4321, "RollbackKernelWrite() version"); TEST_STR_EQ(mock_calls, + "TlclGetPermissions(0x1008)\n" "TlclRead(0x1008, 13)\n" "TlclWrite(0x1008, 13)\n", "tlcl calls"); -- cgit v1.2.1