From 0b02cc8314bebe97354a57614fa5464ec931363e Mon Sep 17 00:00:00 2001 From: Nick Bofferding Date: Mon, 5 Mar 2018 23:58:01 -0600 Subject: Secure Boot: Check integrity of dynamically sized secure header copies When reading a secure header, the container header object can overrun a buffer when number of ECIDs or software keys specified is greater than the supported amount. This change implements hard enforcement to ensure that this is no longer possible. Change-Id: Ife9194763f858b37e2de6f12fa01d74da1145df3 CQ: SW419735 Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/55088 CI-Ready: Nicholas E. Bofferding Tested-by: Jenkins Server Tested-by: Jenkins OP Build CI Tested-by: Jenkins OP HW Reviewed-by: ILYA SMIRNOV Reviewed-by: Michael Baiocchi Reviewed-by: Marshall J. Wilks Tested-by: FSP CI Jenkins Reviewed-by: William G. Hoffa --- src/include/usr/secureboot/containerheader.H | 53 ++++++++++++++++++++----- src/include/usr/secureboot/secure_reasoncodes.H | 3 +- 2 files changed, 46 insertions(+), 10 deletions(-) (limited to 'src/include') diff --git a/src/include/usr/secureboot/containerheader.H b/src/include/usr/secureboot/containerheader.H index daebbcfb6..d95d6cb86 100644 --- a/src/include/usr/secureboot/containerheader.H +++ b/src/include/usr/secureboot/containerheader.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016,2017 */ +/* Contributors Listed Below - COPYRIGHT 2016,2018 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -47,6 +47,16 @@ class ContainerHeader { public: + /** + * @brief Enum specifying a specific ECID count field from the secure + * header + */ + enum class ECID_COUNT_FIELD : uint8_t + { + HW_HEADER = 0x00, // ECID count field in the HW header + SW_HEADER = 0x01, // ECID count field in the SW header + }; + /** * @brief Default Constructor */ @@ -269,22 +279,47 @@ class ContainerHeader */ errlHndl_t parse_header(); + /** + * @brief Validate that the specified ECID count field from the secure + * header is 0 + * + * @param[in] i_ecidCountField Indicates which secure header ECID count + * field (HW header or SW header) should be validated + * @param[in] i_ecidCount The actual value of the ECID count field + * + * @return errlHndl_t Error log handle indicating success or failure + * @retval nullptr Success; the ECID count field is 0 (and valid) + * @retval !nullptr Error; the ECID count field had a value other + * than 0 and the error log handle points to a valid error log + */ + errlHndl_t validateEcidCount( + const ECID_COUNT_FIELD i_ecidCountField, + const uint8_t i_ecidCount) const; + /** * @brief Checks bounds of parsing before mempy and increments pointer * * Ensures that we don't memcpy more bytes than the max size of a - * secure container header. Error log created on out of bounds memcpy. + * secure container header. Optionally ensures that the requested copy + * does not exceed a supplied maximum size, in order to prevent + * dynamically sized data area overruns. Error log created on any + * violation of the above constraints. * - * @param[in] i_dest Pointer to the memory location to copy to - * nullptr input will assert - * @param[in] io_hdr Pointer to current location of container header - * nullptr input will assert - * @param[in] i_size Number of bytes to copy + * @param[in] i_dest Pointer to the memory location to copy to + * nullptr input will assert + * @param[in] io_hdr Pointer to current location of container header + * nullptr input will assert + * @param[in] i_size Number of bytes to copy + * @param[in] i_maxSize Maximum transaction size in bytes for this + * single copy. Default=secure header size. * * @return Error handle if error; otherwise nullptr */ - errlHndl_t safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr, - const size_t i_size); + errlHndl_t safeMemCpyAndInc( + void* i_dest, + const uint8_t*& io_hdr, + size_t i_size, + size_t i_maxSize=MAX_SECURE_HEADER_SIZE); // Pointer to fake header generated std::array iv_fakeHeader; diff --git a/src/include/usr/secureboot/secure_reasoncodes.H b/src/include/usr/secureboot/secure_reasoncodes.H index bee232ee7..5dcb4bf5a 100644 --- a/src/include/usr/secureboot/secure_reasoncodes.H +++ b/src/include/usr/secureboot/secure_reasoncodes.H @@ -47,6 +47,7 @@ namespace SECUREBOOT MOD_SECURE_SET_SBE_SECURE_MODE = 0x0D, MOD_SECURE_GET_ALL_SEC_REGS = 0x0E, MOD_SECURE_LOAD_HEADER = 0x0F, + MOD_SECURE_VALIDATE_ECID_COUNT = 0x10, }; enum SECUREReasonCode @@ -70,7 +71,7 @@ namespace SECUREBOOT RC_PROC_NOT_SCOMABLE = SECURE_COMP_ID | 0x10, RC_DEVICE_READ_ERR = SECURE_COMP_ID | 0x11, RC_INVALID_BASE_HEADER = SECURE_COMP_ID | 0x12, - + RC_INVALID_ECID_COUNT = SECURE_COMP_ID | 0x13, // Reason codes 0xA0 - 0xEF reserved for trustedboot_reasoncodes.H }; -- cgit v1.2.1