diff options
author | Stephen Cprek <smcprek@us.ibm.com> | 2017-11-21 16:09:22 -0600 |
---|---|---|
committer | Daniel M. Crowell <dcrowell@us.ibm.com> | 2017-12-05 14:37:19 -0500 |
commit | ca52131dad3de16f44b9c9f07b5413edf1e9742a (patch) | |
tree | 56a0fcd4357510dee0fa25883dea463cfdb1433b /src/usr/secureboot | |
parent | 89f7297255af3b70c6c1f7a3845498d13eff5cfd (diff) | |
download | talos-hostboot-ca52131dad3de16f44b9c9f07b5413edf1e9742a.tar.gz talos-hostboot-ca52131dad3de16f44b9c9f07b5413edf1e9742a.zip |
Handle ContainerHeader asserts more nicely with error logs
Change-Id: I2dfd02bd7c7f5b5356cd93ca967482c2d7f79ec1
RTC: 178520
RTC: 181899
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/49966
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@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 | 46 | ||||
-rw-r--r-- | src/usr/secureboot/common/containerheader.C | 195 | ||||
-rw-r--r-- | src/usr/secureboot/trusted/base/trustedboot_base.C | 8 |
3 files changed, 217 insertions, 32 deletions
diff --git a/src/usr/secureboot/base/test/securerommgrtest.H b/src/usr/secureboot/base/test/securerommgrtest.H index 1355cc75d..590258ec0 100644 --- a/src/usr/secureboot/base/test/securerommgrtest.H +++ b/src/usr/secureboot/base/test/securerommgrtest.H @@ -244,7 +244,14 @@ class SecureRomManagerTest : public CxxTest::TestSuite /*******************************************************************/ /* Parse Secure Container Header */ /*******************************************************************/ - SECUREBOOT::ContainerHeader l_conHdr(signedFile_pageAddr); + SECUREBOOT::ContainerHeader l_conHdr; + l_errl = l_conHdr.setHeader(signedFile_pageAddr); + if (l_errl) + { + TS_FAIL("SecureRomManagerTest::test_parse_container_header: failed to parse Container Header"); + errlCommit(l_errl, SECURE_COMP_ID); + break; + } // Check if container header seems valid if (!l_conHdr.iv_isValid) @@ -321,7 +328,14 @@ class SecureRomManagerTest : public CxxTest::TestSuite /* Parse Secure Container Header */ /*******************************************************************/ - SECUREBOOT::ContainerHeader l_conHdr(signedFile_pageAddr); + SECUREBOOT::ContainerHeader l_conHdr; + l_errl = l_conHdr.setHeader(signedFile_pageAddr); + if (l_errl) + { + TS_FAIL("SecureRomManagerTest::test_hash_page_table_verify: failed to parse Container Header"); + errlCommit(l_errl, SECURE_COMP_ID); + break; + } size_t l_payloadTextSize = l_conHdr.payloadTextSize(); TRACUCOMP(g_trac_secure, "SecureRomManagerTest::test_hash_page_table_verify ContainerHeader payload_size = 0x%X", l_payloadTextSize); @@ -447,6 +461,8 @@ class SecureRomManagerTest : public CxxTest::TestSuite break; } + // Could replace with SECUREBOOT::ContainerHeader ability to generate + // fake headers char pHeader[MAX_SECURE_HEADER_SIZE]={0}; memcpy(pHeader,signedFile_pageAddr,sizeof(pHeader)); @@ -462,7 +478,15 @@ class SecureRomManagerTest : public CxxTest::TestSuite { memset(pCompIdInContainer,0x00,compIdSize); strncpy(pCompIdInContainer,test.pActualCompId,compIdSize); - SECUREBOOT::ContainerHeader containerHeader(pHeader); + SECUREBOOT::ContainerHeader containerHeader; + pError = containerHeader.setHeader(pHeader); + if (pError) + { + errlCommit(pError, SECURE_COMP_ID); + TS_FAIL("SecureRomManagerTest::test_verifyContainer: failed to parse Container Header"); + break; + } + pError = SECUREBOOT::verifyComponent( containerHeader, @@ -536,20 +560,24 @@ class SecureRomManagerTest : public CxxTest::TestSuite // otherwise strncmp below needs a different size const char* l_compId = "FAKEHEADERTEST"; + do { // 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())) + SECUREBOOT::ContainerHeader l_fakeHdr; + errlHndl_t l_errl = l_fakeHdr.setFakeHeader(l_totalContainerSize, + l_compId); + if (l_errl) { - TS_FAIL("SecureRomManagerTest::test_fakeHeader: missing magic number"); + TS_FAIL("SecureRomManagerTest::test_fakeHeader: failed to parse Container Header"); + errlCommit(l_errl, SECURE_COMP_ID); + break; } // 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"); + break; } // Ensure the parsed component ID matches what was passed in through @@ -558,7 +586,9 @@ class SecureRomManagerTest : public CxxTest::TestSuite SW_HDR_COMP_ID_SIZE_BYTES) != 0) { TS_FAIL("SecureRomManagerTest::test_fakeHeader: component ID was not parsed correctly"); + break; } + } while(0); } }; diff --git a/src/usr/secureboot/common/containerheader.C b/src/usr/secureboot/common/containerheader.C index dd43551d2..cec5f8cce 100644 --- a/src/usr/secureboot/common/containerheader.C +++ b/src/usr/secureboot/common/containerheader.C @@ -24,6 +24,8 @@ /* IBM_PROLOG_END_TAG */ #include <secureboot/containerheader.H> #include "../common/securetrace.H" +#include <secureboot/secure_reasoncodes.H> +#include <pnor/pnor_reasoncodes.H> // Quick change for unit testing //#define TRACUCOMP(args...) TRACFCOMP(args) @@ -32,52 +34,111 @@ namespace SECUREBOOT { -void ContainerHeader::parse_header(const void* i_header) +errlHndl_t ContainerHeader::parse_header() { - assert(i_header != nullptr); - const uint8_t* l_hdr = reinterpret_cast<const uint8_t*>(i_header); + assert(iv_pHdrStart != nullptr, "Cannot parse header that is nullptr"); + const uint8_t* l_hdr = reinterpret_cast<const uint8_t*>(iv_pHdrStart); + errlHndl_t l_errl = nullptr; + + do { /*---- Parse ROM_container_raw ----*/ // The rom code has a placeholder for the prefix in the first struct size_t l_size = offsetof(ROM_container_raw, prefix); - safeMemCpyAndInc(&iv_headerInfo.hw_hdr, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.hw_hdr, l_hdr, l_size); + if(l_errl) + { + break; + } // Early check if magic number is valid, as a quick check to try and prevent // any storage exceptions while parsing header. - assert(iv_headerInfo.hw_hdr.magic_number == ROM_MAGIC_NUMBER, - "ContainerHeader: magic number = 0x%08X not valid", - iv_headerInfo.hw_hdr.magic_number); + if(iv_headerInfo.hw_hdr.magic_number != ROM_MAGIC_NUMBER) + { + TRACFCOMP(g_trac_secure,ERR_MRK"ContainerHeader::parse_header() Magic Number = 0x%X not valid to parse Container Header", + iv_headerInfo.hw_hdr.magic_number); + + /*@ + * @errortype ERRORLOG::ERRL_SEV_UNRECOVERABLE + * @moduleid SECUREBOOT::MOD_SECURE_CONT_HDR_PARSE + * @reasoncode PNOR::RC_BAD_SECURE_MAGIC_NUM + * @userdata1 Actual magic number + * @userdata2 Expected magic number + * @devdesc Error parsing secure header + * @custdesc Firmware Error + */ + l_errl = new ERRORLOG::ErrlEntry( + ERRORLOG::ERRL_SEV_UNRECOVERABLE, + SECUREBOOT::MOD_SECURE_CONT_HDR_PARSE, + PNOR::RC_BAD_SECURE_MAGIC_NUM, + iv_headerInfo.hw_hdr.magic_number, + ROM_MAGIC_NUMBER, + true/*SW Error*/); + l_errl->collectTrace(SECURE_COMP_NAME); + l_errl->collectTrace(PNOR_COMP_NAME); + break; + } /*---- Parse ROM_prefix_header_raw ----*/ l_size = offsetof(ROM_prefix_header_raw, ecid); - safeMemCpyAndInc(&iv_headerInfo.hw_prefix_hdr, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.hw_prefix_hdr, l_hdr, l_size); + if(l_errl) + { + break; + } // Get ECID array l_size = iv_headerInfo.hw_prefix_hdr.ecid_count * ECID_SIZE; - safeMemCpyAndInc(&iv_headerInfo.hw_prefix_hdr.ecid, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.hw_prefix_hdr.ecid, l_hdr, l_size); + if(l_errl) + { + break; + } /*---- Parse ROM_prefix_data_raw ----*/ l_size = offsetof(ROM_prefix_data_raw, sw_pkey_p); - safeMemCpyAndInc(&iv_headerInfo.hw_prefix_data, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.hw_prefix_data, l_hdr, l_size); + if(l_errl) + { + break; + } // Get SW keys l_size = iv_headerInfo.hw_prefix_hdr.sw_key_count * sizeof(ecc_key_t); // Cache total software keys size iv_totalSwKeysSize = l_size; - safeMemCpyAndInc(&iv_headerInfo.hw_prefix_data.sw_pkey_p, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.hw_prefix_data.sw_pkey_p, l_hdr, + l_size); + if(l_errl) + { + break; + } /*---- Parse ROM_sw_header_raw ----*/ l_size = offsetof(ROM_sw_header_raw, ecid); - safeMemCpyAndInc(&iv_headerInfo.sw_hdr, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.sw_hdr, l_hdr, l_size); + if(l_errl) + { + break; + } strncpy(iv_componentId,iv_headerInfo.sw_hdr.component_id, sizeof(iv_headerInfo.sw_hdr.component_id)); // Get ECID array l_size = iv_headerInfo.sw_hdr.ecid_count * ECID_SIZE; - safeMemCpyAndInc(&iv_headerInfo.sw_hdr.ecid, l_hdr, l_size); + l_errl = safeMemCpyAndInc(&iv_headerInfo.sw_hdr.ecid, l_hdr, l_size); + if(l_errl) + { + break; + } /*---- Parse ROM_sw_sig_raw ----*/ - safeMemCpyAndInc(&iv_headerInfo.sw_sig.sw_sig_p, l_hdr, iv_totalSwKeysSize); + l_errl = safeMemCpyAndInc(&iv_headerInfo.sw_sig.sw_sig_p, l_hdr, + iv_totalSwKeysSize); + if(l_errl) + { + break; + } // Parse hw and sw flags parseFlags(); @@ -88,10 +149,18 @@ void ContainerHeader::parse_header(const void* i_header) #endif // After parsing check if header is valid, do some quick bound checks - validate(); + l_errl = validate(); + if(l_errl) + { + break; + } // Debug printing print(); + + } while(0); + + return l_errl; } void ContainerHeader::initVars() @@ -153,7 +222,6 @@ void ContainerHeader::genFakeHeader(const size_t i_totalSize, // No-op already zeroed out iv_pHdrStart = reinterpret_cast<const uint8_t*>(iv_fakeHeader.data()); - parse_header(iv_fakeHeader.data()); } void ContainerHeader::print() const @@ -264,8 +332,10 @@ const SHA512_t* ContainerHeader::hwKeyHash() const return &iv_hwKeyHash; } -void ContainerHeader::validate() +errlHndl_t ContainerHeader::validate() { + errlHndl_t l_errl = nullptr; + iv_isValid = (iv_hdrBytesRead <= MAX_SECURE_HEADER_SIZE) && (iv_headerInfo.hw_hdr.magic_number == ROM_MAGIC_NUMBER) && (iv_headerInfo.hw_hdr.version == ROM_VERSION) @@ -275,24 +345,85 @@ void ContainerHeader::validate() && (iv_headerInfo.hw_prefix_hdr.sw_key_count >= SW_KEY_COUNT_MIN) && (iv_headerInfo.hw_prefix_hdr.sw_key_count <= SW_KEY_COUNT_MAX) && (iv_headerInfo.sw_hdr.payload_size != 0); + + if(!iv_isValid) + { + TRACFCOMP(g_trac_secure,ERR_MRK"ContainerHeader::validate() failed weak header verification"); + /*@ + * @errortype ERRORLOG::ERRL_SEV_UNRECOVERABLE + * @moduleid SECUREBOOT::MOD_SECURE_CONT_VALIDATE + * @reasoncode SECUREBOOT::RC_CONT_HDR_INVALID + * @userdata1[0::31] Magic Number + * @userdata1[32::63] ROM version + * @userdata2[0:15] Algorithm version + * @userdata2[16:31] Hash algorithm + * @userdata2[32:47] Signature algorithm + * @userdata2[48:63] SW key count + * @devdesc Error parsing secure header + * @custdesc Firmware Error + */ + l_errl = new ERRORLOG::ErrlEntry( + ERRORLOG::ERRL_SEV_UNRECOVERABLE, + SECUREBOOT::MOD_SECURE_CONT_VALIDATE, + SECUREBOOT::RC_CONT_HDR_INVALID, + TWO_UINT32_TO_UINT64(iv_headerInfo.hw_hdr.magic_number, + iv_headerInfo.hw_hdr.version), + FOUR_UINT16_TO_UINT64(iv_headerInfo.hw_prefix_hdr.ver_alg.version, + iv_headerInfo.hw_prefix_hdr.ver_alg.hash_alg, + iv_headerInfo.hw_prefix_hdr.ver_alg.sig_alg, + iv_headerInfo.hw_prefix_hdr.sw_key_count), + true/*SW Error*/); + l_errl->collectTrace(SECURE_COMP_NAME); + l_errl->collectTrace(PNOR_COMP_NAME); + } + + return l_errl; } -void ContainerHeader::safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr, +errlHndl_t ContainerHeader::safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr, const size_t i_size) { - 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"); + assert(i_dest != nullptr, "ContainerHeader: dest nullptr"); + assert(io_hdr != nullptr, "ContainerHeader: current header location nullptr"); + assert(iv_pHdrStart != nullptr, "ContainerHeader: start of header nullptr"); TRACDCOMP(g_trac_secure,"dest: 0x%X src: 0x%X size: 0x%X",i_dest, io_hdr, i_size); + errlHndl_t l_errl = nullptr; + do { // Determine if the memcpy is within the bounds of the container header iv_hdrBytesRead = io_hdr - iv_pHdrStart; - assert( (iv_hdrBytesRead + i_size) <= MAX_SECURE_HEADER_SIZE, - "ContainerHeader: memcpy is out of bounds of max header size"); + if((iv_hdrBytesRead + i_size) > MAX_SECURE_HEADER_SIZE) + { + TRACFCOMP(g_trac_secure,ERR_MRK"ContainerHeader::safeMemCpyAndInc parsed header is out of bounds of max header size of 0x%X", + MAX_SECURE_HEADER_SIZE); + /*@ + * @errortype ERRORLOG::ERRL_SEV_UNRECOVERABLE + * @moduleid SECUREBOOT::MOD_SECURE_CONT_HDR_CPY_INC + * @reasoncode SECUREBOOT::RC_CONT_HDR_NO_SPACE + * @userdata1 Size needed + * @userdata2 Max secure header size + * @devdesc Error parsing secure header + * @custdesc Firmware Error + */ + l_errl = new ERRORLOG::ErrlEntry( + ERRORLOG::ERRL_SEV_UNRECOVERABLE, + SECUREBOOT::MOD_SECURE_CONT_HDR_CPY_INC, + SECUREBOOT::RC_CONT_HDR_NO_SPACE, + iv_hdrBytesRead + i_size, + MAX_SECURE_HEADER_SIZE, + true/*SW Error*/); + l_errl->collectTrace(SECURE_COMP_NAME); + l_errl->collectTrace(PNOR_COMP_NAME); + break; + } memcpy(i_dest, io_hdr, i_size); io_hdr += i_size; + + } while(0); + + return l_errl; } bool ContainerHeader::isValid() const @@ -333,4 +464,22 @@ const uint8_t* ContainerHeader::fakeHeader() const return iv_fakeHeader.data(); } +errlHndl_t ContainerHeader::setHeader(const void* i_header) +{ + assert(i_header != nullptr, "Cannot set header to nullptr"); + iv_pHdrStart = reinterpret_cast<const uint8_t*>(i_header); + initVars(); + return parse_header(); +} + + +errlHndl_t ContainerHeader::setFakeHeader(const size_t i_totalSize, + const char* i_compId) +{ + initVars(); + genFakeHeader(i_totalSize, i_compId); + return parse_header(); +} + + }; //end of SECUREBOOT namespace diff --git a/src/usr/secureboot/trusted/base/trustedboot_base.C b/src/usr/secureboot/trusted/base/trustedboot_base.C index 806ecd91e..b94484bb8 100644 --- a/src/usr/secureboot/trusted/base/trustedboot_base.C +++ b/src/usr/secureboot/trusted/base/trustedboot_base.C @@ -445,7 +445,13 @@ errlHndl_t extendBaseImage() } // Build a container header object from the raw header - const SECUREBOOT::ContainerHeader hbbContainerHeader(pHbbHeader); + SECUREBOOT::ContainerHeader hbbContainerHeader; + pError = hbbContainerHeader.setHeader(pHbbHeader); + if (pError) + { + TRACFCOMP(g_trac_trustedboot, ERR_MRK"extendBaseImage() setheader failed"); + break; + } const void* pHbbVa = nullptr; if(!SECUREBOOT::enabled()) |