diff options
author | Stephen Cprek <smcprek@us.ibm.com> | 2017-10-31 13:01:30 -0500 |
---|---|---|
committer | Daniel M. Crowell <dcrowell@us.ibm.com> | 2017-11-19 15:54:51 -0500 |
commit | 81279c1d146d8ee920494c7817cdd72f165dd373 (patch) | |
tree | d616d0914823c8c25592e8276e0610ba1c9d2a28 /src/usr/secureboot | |
parent | 63a026113332464fc3bcc73369ba35bfe8f62b6f (diff) | |
download | talos-hostboot-81279c1d146d8ee920494c7817cdd72f165dd373.tar.gz talos-hostboot-81279c1d146d8ee920494c7817cdd72f165dd373.zip |
Secure Boot: Fix lid load from HB reserved memory issues at runtime
- Force all PNOR sections we load from HB rserved memory to be secure
Only exception is the RINGOVD section, in which we use a fake header
- Add fake header when Secureboot compiled out or a section is never
signed as there is no secure header preserved in virtual memory
RTC: 171708
RTC: 180063
Change-Id: Ibbbd7be24ee7b199e73451c63b2c2d1f86a2c2d8
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/49020
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Marshall J. Wilks <mjwilks@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Reviewed-by: Michael Baiocchi <mbaiocch@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Diffstat (limited to 'src/usr/secureboot')
-rw-r--r-- | src/usr/secureboot/base/test/securerommgrtest.H | 35 | ||||
-rw-r--r-- | src/usr/secureboot/common/containerheader.C | 76 | ||||
-rw-r--r-- | src/usr/secureboot/runtime/test/testsecureboot_rt.H | 73 |
3 files changed, 140 insertions, 44 deletions
diff --git a/src/usr/secureboot/base/test/securerommgrtest.H b/src/usr/secureboot/base/test/securerommgrtest.H index 4a445b52f..1355cc75d 100644 --- a/src/usr/secureboot/base/test/securerommgrtest.H +++ b/src/usr/secureboot/base/test/securerommgrtest.H @@ -45,6 +45,7 @@ #include "../../../vfs/vfsrp.H" #include <sys/vfs.h> #include <kernel/console.H> +#include <pnor/pnorif.H> // Quick change for unit testing //#define TRACUCOMP(args...) TRACFCOMP(args) @@ -525,6 +526,40 @@ class SecureRomManagerTest : public CxxTest::TestSuite unloadSignedFile( signedFile_pageAddr, signedFile_size); } } + + void test_fakeHeader(void) + { + TRACFCOMP(g_trac_secure,"SecureRomManagerTest::test_fakeHeader"); + + const size_t l_totalContainerSize = 0x10000; + // Purposely make a comp id larger than SW_HDR_COMP_ID_SIZE_BYTES + // otherwise strncmp below needs a different size + const char* l_compId = "FAKEHEADERTEST"; + + // Simple call constructor to create fake header and make sure it + // does not cause an error + SECUREBOOT::ContainerHeader l_fakeHdr(l_totalContainerSize, l_compId); + + // Check if Header is mising + if (!PNOR::cmpSecurebootMagicNumber(l_fakeHdr.fakeHeader())) + { + TS_FAIL("SecureRomManagerTest::test_fakeHeader: missing magic number"); + } + + // Payload Text Size should be the total container size minus the header + if(l_fakeHdr.payloadTextSize() != (l_totalContainerSize - PAGE_SIZE)) + { + TS_FAIL("SecureRomManagerTest::test_fakeHeader: payload text size was not parsed correctly"); + } + + // Ensure the parsed component ID matches what was passed in through + // SW_HDR_COMP_ID_SIZE_BYTES + if(strncmp(l_fakeHdr.componentId(), l_compId, + SW_HDR_COMP_ID_SIZE_BYTES) != 0) + { + TS_FAIL("SecureRomManagerTest::test_fakeHeader: component ID was not parsed correctly"); + } + } }; /**********************************************************************/ diff --git a/src/usr/secureboot/common/containerheader.C b/src/usr/secureboot/common/containerheader.C index 069a587d9..dd43551d2 100644 --- a/src/usr/secureboot/common/containerheader.C +++ b/src/usr/secureboot/common/containerheader.C @@ -34,7 +34,7 @@ namespace SECUREBOOT void ContainerHeader::parse_header(const void* i_header) { - assert(i_header != NULL); + assert(i_header != nullptr); const uint8_t* l_hdr = reinterpret_cast<const uint8_t*>(i_header); /*---- Parse ROM_container_raw ----*/ @@ -94,6 +94,68 @@ void ContainerHeader::parse_header(const void* i_header) print(); } +void ContainerHeader::initVars() +{ + memset(&iv_headerInfo, 0x00, sizeof(iv_headerInfo)); + memset(iv_hwKeyHash, 0, sizeof(SHA512_t)); + memset(iv_componentId,0x00,sizeof(iv_componentId)); +} + +void ContainerHeader::genFakeHeader(const size_t i_totalSize, + const char* const i_compId) +{ + SecureHeaderInfo info {}; + assert(iv_fakeHeader.data() != nullptr, "Internal fake header buffer nullptr"); + + uint8_t* l_hdr = reinterpret_cast<uint8_t*>(iv_fakeHeader.data()); + + /*---- ROM_container_raw ----*/ + info.hw_hdr.magic_number = ROM_MAGIC_NUMBER; + info.hw_hdr.version = CONTAINER_VERSION; + info.hw_hdr.container_size = i_totalSize; + // The rom code has a placeholder for the prefix in the first struct so + // skip it + size_t l_size = offsetof(ROM_container_raw, prefix); + memcpy(l_hdr, &info.hw_hdr, l_size); + l_hdr += l_size; + + /*---- ROM_prefix_header_raw ----*/ + info.hw_prefix_hdr.ver_alg.version = HEADER_VERSION; + info.hw_prefix_hdr.ver_alg.hash_alg = HASH_ALG_SHA512; + info.hw_prefix_hdr.ver_alg.sig_alg = SIG_ALG_ECDSA521; + info.hw_prefix_hdr.sw_key_count = 1; + info.hw_prefix_hdr.payload_size = sizeof(ecc_key_t); + + l_size = offsetof(ROM_prefix_header_raw, ecid); + l_size += info.hw_prefix_hdr.ecid_count * ECID_SIZE; + memcpy(l_hdr, &info.hw_prefix_hdr, l_size); + l_hdr += l_size; + + /*---- Parse ROM_prefix_data_raw ----*/ + // Skip over variable number of sw keys as they are already zeroed out + l_size = offsetof(ROM_prefix_data_raw, sw_pkey_p); + l_size += info.hw_prefix_hdr.sw_key_count * sizeof(ecc_key_t); + l_hdr += l_size; + + /*---- ROM_sw_header_raw ----*/ + info.sw_hdr.ver_alg.version = 1; + strncpy(info.sw_hdr.component_id, i_compId,SW_HDR_COMP_ID_SIZE_BYTES); + info.sw_hdr.ver_alg.hash_alg = HASH_ALG_SHA512; + info.sw_hdr.ver_alg.sig_alg = SIG_ALG_ECDSA521; + info.sw_hdr.payload_size = i_totalSize - PAGE_SIZE; + + l_size = offsetof(ROM_sw_header_raw, ecid); + l_size += info.hw_prefix_hdr.ecid_count * ECID_SIZE; + memcpy(l_hdr, &info.sw_hdr, l_size); + l_hdr += l_size; + + /*---- Parse ROM_sw_sig_raw ----*/ + // No-op already zeroed out + + iv_pHdrStart = reinterpret_cast<const uint8_t*>(iv_fakeHeader.data()); + parse_header(iv_fakeHeader.data()); +} + void ContainerHeader::print() const { #ifdef HOSTBOOT_DEBUG @@ -218,9 +280,9 @@ void ContainerHeader::validate() void ContainerHeader::safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr, const size_t i_size) { - assert(i_dest != NULL, "ContainerHeader: dest ptr NULL"); - assert(io_hdr != NULL, "ContainerHeader: current header location ptr NULL"); - assert(iv_pHdrStart != NULL, "ContainerHeader: start of header ptr NULL"); + assert(i_dest != nullptr, "ContainerHeader: dest ptr NULL"); + assert(io_hdr != nullptr, "ContainerHeader: current header location ptr NULL"); + assert(iv_pHdrStart != nullptr, "ContainerHeader: start of header ptr NULL"); TRACDCOMP(g_trac_secure,"dest: 0x%X src: 0x%X size: 0x%X",i_dest, io_hdr, i_size); @@ -265,4 +327,10 @@ void ContainerHeader::genHwKeyHash() } #endif +const uint8_t* ContainerHeader::fakeHeader() const +{ + assert(iv_fakeHeader.data() != nullptr, "Fake header should not be nullptr"); + return iv_fakeHeader.data(); +} + }; //end of SECUREBOOT namespace diff --git a/src/usr/secureboot/runtime/test/testsecureboot_rt.H b/src/usr/secureboot/runtime/test/testsecureboot_rt.H index 33ca4cd48..f728357e2 100644 --- a/src/usr/secureboot/runtime/test/testsecureboot_rt.H +++ b/src/usr/secureboot/runtime/test/testsecureboot_rt.H @@ -44,6 +44,7 @@ #include <secureboot/settings.H> #include <pnor/pnorif.H> #include <pnor/pnor_reasoncodes.H> +#include "../../../pnor/pnor_utils.H" class SecurebootRtTestSuite: public CxxTest::TestSuite { @@ -163,70 +164,62 @@ class SecurebootRtTestSuite: public CxxTest::TestSuite /** * @brief Helper to test case that runs getSectionInfo scenarios and checks * for desired results. + * If secure, should throw an error + * Otherwise no error * @param[in] i_id, Pnor Section ID - * @param[in] i_secure, Indicates if section is expected to be secure or not * * @return N/A */ - void runAccessSecurePnorTest(PNOR::SectionId i_id, bool i_secure) + void runAccessSecurePnorTest(PNOR::SectionId i_id) { errlHndl_t l_errl = nullptr; + bool l_secure = PNOR::isEnforcedSecureSection(i_id); PNOR::SectionInfo_t l_info; - l_errl = PNOR::getSectionInfo(i_id, l_info); - if(l_errl) - { - TS_FAIL("testAccessSecurePnorSection: Failed for section %s", - PNOR::SectionIdToString(i_id)); - errlCommit(l_errl, SECURE_COMP_ID); - } + SB_ENTER("runAccessSecurePnorTest %s", PNOR::SectionIdToString(i_id)); - // TODO: RTC:180063 change this test case back to how it was before - // having secure sections return vaddr = 0 - // previously in HB commit cefc4c - // If we expect the section to be secure, make sure it returns secure - // and a vaddr of 0 - if (i_secure) + l_errl = PNOR::getSectionInfo(i_id, l_info); + if(l_secure) { - if (l_info.secure != 1) + SB_INF("runAccessSecurePnorTest is secure"); + if (l_errl && + (l_errl->reasonCode() == PNOR::RC_RTPNOR_INVALID_SECTION)) { - TS_FAIL("testAccessSecurePnorSection: Did not return %s as a secure section", - PNOR::SectionIdToString(i_id)); + SB_INF("runAccessSecurePnorTest caught correct error"); + delete l_errl; + l_errl = nullptr; } - else if (l_info.vaddr != 0) + else { - TS_FAIL("testAccessSecurePnorSection: Did not return a vaddr of 0 for secure section %s", - PNOR::SectionIdToString(i_id)); + TS_FAIL("testAccessSecurePnorSection: unexpected reason code for Secure Section %s. Expected RC 0x%.4X Actual RC 0x%.4X", + PNOR::SectionIdToString(i_id), + PNOR::RC_RTPNOR_INVALID_SECTION, + l_errl->reasonCode()); + errlCommit(l_errl, SECURE_COMP_ID); } } - // If we expect the section to be secure, make sure it returns secure - // and a vaddr of 0 - else + else if(l_errl) { - if (l_info.vaddr == 0) - { - TS_FAIL("testAccessSecurePnorSection: Did not return a vaddr of non-zero for a non-secure section %s", - PNOR::SectionIdToString(i_id)); - } + TS_FAIL("testAccessSecurePnorSection: getSectionInfo failed for section %s", + PNOR::SectionIdToString(i_id)); + errlCommit(l_errl, SECURE_COMP_ID); } + + SB_EXIT("runAccessSecurePnorTest"); } - // TODO: RTC:180063 change this test case back to how it was before - // having secure sections return vaddr = 0 previously - // in HB commit cefc4c void testAccessSecurePnorSection() { +#ifdef CONFIG_SECUREBOOT SB_ENTER("testAccessSecurePnorSection"); + // Should thow an error for trying to read a secure section + runAccessSecurePnorTest(PNOR::OCC); + runAccessSecurePnorTest(PNOR::HB_EXT_CODE); - // Ensure we get a vaddr of 0 at Runtime - runAccessSecurePnorTest(PNOR::OCC, true); - runAccessSecurePnorTest(PNOR::HB_EXT_CODE, true); - - // Ensure we get a vaddr of at Runtime - runAccessSecurePnorTest(PNOR::TEST, false); - - SB_EXIT("testAccessSecurePnorSection"); + // No error for trying to read a secure section + runAccessSecurePnorTest(PNOR::TEST); +#endif } |