diff options
28 files changed, 383 insertions, 287 deletions
diff --git a/src/bootloader/bootloader.C b/src/bootloader/bootloader.C index 55969fa0b..99ac0b1ec 100644 --- a/src/bootloader/bootloader.C +++ b/src/bootloader/bootloader.C @@ -198,18 +198,35 @@ namespace Bootloader{ { BOOTLOADER_TRACE(BTLDR_TRC_MAIN_VERIFY_SAB_UNSET); } - // # @TODO RTC:170136 terminate in this case - // Ensure SecureRom is actually present + // Terminate if a valid securerom is not present else if ( !g_blData->secureRomValid ) { +#ifdef CONFIG_SECUREBOOT_BEST_EFFORT BOOTLOADER_TRACE(BTLDR_TRC_MAIN_VERIFY_NO_EYECATCH); +#else + BOOTLOADER_TRACE(BTLDR_TRC_MAIN_VERIFY_INVALID_SECROM); + /*@ + * @errortype + * @moduleid Bootloader::MOD_BOOTLOADER_VERIFY + * @reasoncode SECUREBOOT::RC_SECROM_INVALID + * @userdata1[0:15] TI_WITH_SRC + * @userdata1[16:31] TI_BOOTLOADER + * @userdata1[32:63] Failing address = 0 + * @devdesc Valid securerom not present + * @custdesc Security failure occurred while running processor + * boot code. + */ + bl_terminate(Bootloader::MOD_BOOTLOADER_VERIFY, + SECUREBOOT::RC_SECROM_INVALID); +#endif } - // # @TODO RTC:170136 terminate in this case +#ifdef CONFIG_SECUREBOOT_BEST_EFFORT else if ( !PNOR::cmpSecurebootMagicNumber( reinterpret_cast<const uint8_t*>(i_pContainer))) { BOOTLOADER_TRACE(BTLDR_TRC_MAIN_VERIFY_NO_MAGIC_NUM); } +#endif else { // Set startAddr to ROM_verify() function at an offset of Secure ROM diff --git a/src/build/configs/fsprelease.config b/src/build/configs/fsprelease.config index 4c50b930f..431671a6a 100644 --- a/src/build/configs/fsprelease.config +++ b/src/build/configs/fsprelease.config @@ -16,3 +16,5 @@ set FSP_BUILD # OpenPower checkstop analysis unset ENABLE_CHECKSTOP_ANALYSIS unset IPLTIME_CHECKSTOP_ANALYSIS + +set SECUREBOOT_BEST_EFFORT diff --git a/src/build/debug/Hostboot/BlTrace.pm b/src/build/debug/Hostboot/BlTrace.pm index 8aa93a951..c285ec03d 100644 --- a/src/build/debug/Hostboot/BlTrace.pm +++ b/src/build/debug/Hostboot/BlTrace.pm @@ -79,6 +79,7 @@ my %traceText = ( "F9" => "PNOR Access getHBBSection findTOC error", "FA" => "PNOR Access getHBBSection findTOC no HBB section", "FB" => "main verifyBaseImage failed", + "FC" => "main verifyBaseImage secure rom invalid", ); sub formatTrace diff --git a/src/build/tools/verify-commit b/src/build/tools/verify-commit index 6f04dd97d..93a471784 100755 --- a/src/build/tools/verify-commit +++ b/src/build/tools/verify-commit @@ -293,12 +293,6 @@ sub verifyCommitMsg # Verify format of RTC message. if ($line =~ m/^\s*RTC:\s*[0-9]+(.*)/) { - if ("" ne $rtcTag) - { - error("Commit Message",$line,$lineCount, - "Duplicate RTC tag found."); - } - $rtcTag = $line; if ("" ne $1) { @@ -309,12 +303,6 @@ sub verifyCommitMsg if ($line =~ m/^\s*CQ:\s*[A-Z][A-Z][0-9]+(.*)/) { - if ("" ne $cqTag) - { - error("Commit Message",$line,$lineCount, - "Duplicate CQ tag found."); - } - $cqTag = $line; if ("" ne $1) { diff --git a/src/include/bootloader/bootloader_trace.H b/src/include/bootloader/bootloader_trace.H index 269a904f5..129b9a303 100644 --- a/src/include/bootloader/bootloader_trace.H +++ b/src/include/bootloader/bootloader_trace.H @@ -181,6 +181,9 @@ enum BootloaderTraces /** Bootloader main verifyContainer failed */ BTLDR_TRC_MAIN_VERIFY_FAIL = 0xFB, + + /** Bootloader main verifyContainer secure rom invalid */ + BTLDR_TRC_MAIN_VERIFY_INVALID_SECROM = 0xFC, }; #ifndef BOOTLOADER_TRACE diff --git a/src/include/kernel/bltohbdatamgr.H b/src/include/kernel/bltohbdatamgr.H index cbbdb40d2..6dd9d55f5 100644 --- a/src/include/kernel/bltohbdatamgr.H +++ b/src/include/kernel/bltohbdatamgr.H @@ -34,14 +34,6 @@ class BlToHbDataManager { private: - /** - * @brief Performs a printk along with a kassert to be more verbose if - * Data is not valid. - * - * @return N/A - */ - void validAssert() const; - /* * @brief Prints, via printkd, important parts of the structure * diff --git a/src/include/usr/pnor/pnor_reasoncodes.H b/src/include/usr/pnor/pnor_reasoncodes.H index f01291a24..3eee83930 100644 --- a/src/include/usr/pnor/pnor_reasoncodes.H +++ b/src/include/usr/pnor/pnor_reasoncodes.H @@ -177,6 +177,7 @@ namespace PNOR RC_BAD_SECURE_MAGIC_NUM = PNOR_COMP_ID | 0x31, RC_MBOX_BAD_SEQUENCE = PNOR_COMP_ID | 0x32, RC_MBOX_ERROR_STATUS = PNOR_COMP_ID | 0x33, + RC_UNSIGNED_PNOR_SECTION = PNOR_COMP_ID | 0x34, //@fixme-RTC:131607-Temporary value to allow HWSV compile //termination_rc diff --git a/src/include/usr/pnor/pnorif.H b/src/include/usr/pnor/pnorif.H index 3a2d3e53b..b6f616b92 100644 --- a/src/include/usr/pnor/pnorif.H +++ b/src/include/usr/pnor/pnorif.H @@ -180,7 +180,7 @@ errlHndl_t validateAltMaster( void ); */ void getPnorInfo( PnorInfo_t& o_pnorInfo ); -// @TODO RTC:155374 Remove this in the future + /** * @brief Check if PNOR section appears to be secure and sets the * internal TOC of PnorRp accordingly. diff --git a/src/include/usr/secureboot/header.H b/src/include/usr/secureboot/header.H index 34300ba9a..0478fa675 100644 --- a/src/include/usr/secureboot/header.H +++ b/src/include/usr/secureboot/header.H @@ -59,7 +59,7 @@ namespace SECUREBOOT iv_data=NULL; } - // @TODO RTC 168021 Converge to single method of reading + // @TODO RTC 178520 Converge to single method of reading // secure header /** @@ -72,7 +72,7 @@ namespace SECUREBOOT */ void loadSecurely(); - // @TODO RTC 168021 Converge to single method of reading + // @TODO RTC 178520 Converge to single method of reading // secure header /** diff --git a/src/include/usr/secureboot/secure_reasoncodes.H b/src/include/usr/secureboot/secure_reasoncodes.H index 59761275a..98fe38d3c 100644 --- a/src/include/usr/secureboot/secure_reasoncodes.H +++ b/src/include/usr/secureboot/secure_reasoncodes.H @@ -54,6 +54,7 @@ namespace SECUREBOOT RC_ROM_SHA512 = SECURE_COMP_ID | 0x08, RC_SECURE_BAD_TARGET = SECURE_COMP_ID | 0x09, RC_SECURE_BOOT_DISABLED = SECURE_COMP_ID | 0x0A, + RC_SECROM_INVALID = SECURE_COMP_ID | 0x0B, // Reason codes 0xA0 - 0xEF reserved for trustedboot_reasoncodes.H }; diff --git a/src/include/usr/secureboot/service.H b/src/include/usr/secureboot/service.H index 27c35f6d4..0258b5706 100644 --- a/src/include/usr/secureboot/service.H +++ b/src/include/usr/secureboot/service.H @@ -299,6 +299,13 @@ namespace SECUREBOOT */ bool allowAttrOverrides(); + /* Definition in settings.H */ + bool bestEffortPolicy(); + + /* Definition in securerommgr.H */ + bool secureRomValidPolicy(); + + } #endif diff --git a/src/include/usr/secureboot/settings.H b/src/include/usr/secureboot/settings.H index 417e14d96..491c607d3 100644 --- a/src/include/usr/secureboot/settings.H +++ b/src/include/usr/secureboot/settings.H @@ -90,7 +90,8 @@ namespace SECUREBOOT class Settings { public: - Settings() : iv_enabled(false) { _init(); }; + Settings() : iv_enabled(false), + iv_bestEffortPolicy(false) { _init(); }; ~Settings() {}; /** @brief Determine if Secureboot is enabled. */ @@ -135,6 +136,13 @@ namespace SECUREBOOT TARGETING::Target* i_pProc = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL) const; + /** @brief Determines if the Secureboot best effort policy is + * enabled. Uses knowledge of compile config options and + * secure mode enabled + * @return bool - True if enabled, false otherwise + */ + bool getBestEffortPolicy() const; + private: void _init(); @@ -184,6 +192,7 @@ namespace SECUREBOOT /** Cached secure boot enabled value */ bool iv_enabled; + bool iv_bestEffortPolicy; }; } diff --git a/src/kernel/bltohbdatamgr.C b/src/kernel/bltohbdatamgr.C index 3d33715b2..e33fab6ef 100644 --- a/src/kernel/bltohbdatamgr.C +++ b/src/kernel/bltohbdatamgr.C @@ -44,15 +44,6 @@ bool BlToHbDataManager::iv_initialized = false; bool BlToHbDataManager::iv_dataValid = false; size_t BlToHbDataManager::iv_preservedSize = 0; -void BlToHbDataManager::validAssert() const -{ - if(!iv_dataValid) - { - printk("E> BlToHbDataManager is invalid, cannot access\n"); - kassert(iv_dataValid); - } -} - void BlToHbDataManager::print() const { if(iv_dataValid) @@ -269,13 +260,21 @@ const uint64_t BlToHbDataManager::getBranchtableOffset() const const void* BlToHbDataManager::getSecureRom() const { - validAssert(); + if(!iv_dataValid) + { + printk("E> BlToHbDataManager is invalid, cannot access SecureRom\n"); + crit_assert(iv_dataValid); + } return iv_data.secureRom; } const uint64_t BlToHbDataManager::getSecureRomAddr() const { - validAssert(); + if(!iv_dataValid) + { + printk("E> BlToHbDataManager is invalid, cannot access SecureRomAddr\n"); + crit_assert(iv_dataValid); + } return reinterpret_cast<uint64_t>(iv_data.secureRom); } @@ -286,13 +285,21 @@ const size_t BlToHbDataManager::getSecureRomSize() const const void* BlToHbDataManager::getHwKeysHash() const { - validAssert(); + if(!iv_dataValid) + { + printk("E> BlToHbDataManager is invalid, cannot access HwKeysHash\n"); + crit_assert(iv_dataValid); + } return iv_data.hwKeysHash; } const uint64_t BlToHbDataManager::getHwKeysHashAddr() const { - validAssert(); + if(!iv_dataValid) + { + printk("E> BlToHbDataManager is invalid, cannot access HwKeysHashAddr\n"); + crit_assert(iv_dataValid); + } return reinterpret_cast<uint64_t>(iv_data.hwKeysHash); } @@ -303,13 +310,21 @@ const size_t BlToHbDataManager::getHwKeysHashSize() const const void* BlToHbDataManager::getHbbHeader() const { - validAssert(); + if(!iv_dataValid) + { + printk("E> BlToHbDataManager is invalid, cannot access HbbHeader\n"); + crit_assert(iv_dataValid); + } return iv_data.hbbHeader; } const uint64_t BlToHbDataManager::getHbbHeaderAddr() const { - validAssert(); + if(!iv_dataValid) + { + printk("E> BlToHbDataManager is invalid, cannot access HbbHeaderAddr\n"); + crit_assert(iv_dataValid); + } return reinterpret_cast<uint64_t>(iv_data.hbbHeader); } diff --git a/src/usr/pnor/pnor_common.C b/src/usr/pnor/pnor_common.C index addf0e491..20d6bd760 100644 --- a/src/usr/pnor/pnor_common.C +++ b/src/usr/pnor/pnor_common.C @@ -304,31 +304,31 @@ errlHndl_t PNOR::parseTOC( uint8_t* i_tocBuffer,SectionData_t * o_TOC) } - // @TODO RTC 168021 Remove legacy extensions when all - // secure sections are supported +#ifndef __HOSTBOOT_RUNTIME if (PNOR::hasNonSecureHeader(o_TOC[l_secId])) { // Never extend the base image through this path, it will be // handled elsewhere if(l_secId != PNOR::HB_BASE_CODE) { - // For non-secure sections with a SHA512 header, the - // flash address has incremented past the header, so - // back up by the header size (accounting for ECC) in order - // to extend the header - auto addr = o_TOC[l_secId].flashAddr; - size_t headerSize = + // For non-secure sections with a SHA512 header, the + // flash address has incremented past the header, so + // back up by the header size (accounting for ECC) in order + // to extend the header + auto addr = o_TOC[l_secId].flashAddr; + size_t headerSize = (o_TOC[l_secId].integrity == FFS_INTEG_ECC_PROTECT) ? PAGESIZE_PLUS_ECC : PAGESIZE; - addr -= headerSize; + addr -= headerSize; - l_errhdl = PNOR::extendHash(addr, headerSize, l_secId); - if (l_errhdl) - { - break; - } + l_errhdl = PNOR::extendHash(addr, headerSize, l_secId); + if (l_errhdl) + { + break; + } } } +#endif } for(int tmpId = 0; @@ -346,50 +346,49 @@ errlHndl_t PNOR::parseTOC( uint8_t* i_tocBuffer,SectionData_t * o_TOC) return l_errhdl; } -// @TODO RTC 168021 Remove legacy extensions when all secure sections are -// supported -errlHndl_t PNOR::extendHash(uint64_t i_addr, - size_t i_size, - const PNOR::SectionId i_sectionId) +#ifndef __HOSTBOOT_RUNTIME +errlHndl_t PNOR::extendHash(uint64_t i_addr, size_t i_size, + const PNOR::SectionId i_sectionId) { errlHndl_t l_errhdl = NULL; do { - #ifndef __HOSTBOOT_RUNTIME - const char* l_name = PNOR::SectionIdToString(i_sectionId); - - // Read data from the PNOR DD - uint8_t* l_buf = new uint8_t[i_size](); - TARGETING::Target* l_target = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL; - l_errhdl = DeviceFW::deviceRead(l_target, l_buf, i_size, - DEVICE_PNOR_ADDRESS(0,i_addr)); - if (l_errhdl) - { - break; - } - SHA512_t l_hash = {0}; - SECUREBOOT::hashBlob(l_buf, i_size, l_hash); - l_errhdl = TRUSTEDBOOT::pcrExtend(TRUSTEDBOOT::PCR_0, - PNOR::PAYLOAD == i_sectionId? - TRUSTEDBOOT::EV_COMPACT_HASH: - (PNOR::isCoreRootOfTrustSection(i_sectionId)? - TRUSTEDBOOT::EV_S_CRTM_CONTENTS: - TRUSTEDBOOT::EV_POST_CODE), - l_hash, - sizeof(SHA512_t), - l_name); - delete[] l_buf; + const char* l_name = PNOR::SectionIdToString(i_sectionId); + + // Read data from the PNOR DD + uint8_t* l_buf = new uint8_t[i_size](); + TARGETING::Target* l_target = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL; + l_errhdl = DeviceFW::deviceRead(l_target, l_buf, i_size, + DEVICE_PNOR_ADDRESS(0,i_addr)); + if (l_errhdl) + { + break; + } + + SHA512_t l_hash = {0}; + SECUREBOOT::hashBlob(l_buf, i_size, l_hash); + l_errhdl = TRUSTEDBOOT::pcrExtend(TRUSTEDBOOT::PCR_0, + PNOR::PAYLOAD == i_sectionId? + TRUSTEDBOOT::EV_COMPACT_HASH: + (PNOR::isCoreRootOfTrustSection(i_sectionId)? + TRUSTEDBOOT::EV_S_CRTM_CONTENTS: + TRUSTEDBOOT::EV_POST_CODE), + l_hash, + sizeof(SHA512_t), + l_name); + delete[] l_buf; + + if (l_errhdl) + { + break; + } - if (l_errhdl) - { - break; - } - #endif } while(0); return l_errhdl; } +#endif bool PNOR::isInhibitedSection(const uint32_t i_section) { @@ -445,7 +444,7 @@ bool PNOR::isInhibitedSection(const uint32_t i_section) #endif } -// @TODO RTC:155374 Remove this in the future + errlHndl_t PNOR::setSecure(const uint32_t i_secId, PNOR::SectionData_t* io_TOC) { @@ -458,30 +457,32 @@ errlHndl_t PNOR::setSecure(const uint32_t i_secId, // Set secure field based on enforced policy io_TOC[i_secId].secure = PNOR::isEnforcedSecureSection(i_secId); + // HBRT does not support best effort policy. Use enforced secure policy only. #ifndef __HOSTBOOT_RUNTIME -#ifdef CONFIG_SECUREBOOT_BEST_EFFORT - if (io_TOC[i_secId].secure) + if(SECUREBOOT::bestEffortPolicy()) { - // Apply best effort policy by checking if the section appears to have a - // secure header - size_t l_size = sizeof(ROM_MAGIC_NUMBER); - uint8_t l_buf[l_size] = {0}; - auto l_target = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL; - // Read first 4 bytes of section data from the PNOR DD - // Note: Do not need to worry about ECC as the 9th byte is the first - // ECC byte. - l_errhdl = DeviceFW::deviceRead(l_target, l_buf, l_size, - DEVICE_PNOR_ADDRESS(0,io_TOC[i_secId].flashAddr)); - if (l_errhdl) + if (io_TOC[i_secId].secure) { - break; - } + // Apply best effort policy by checking if the section appears to have a + // secure header + size_t l_size = sizeof(ROM_MAGIC_NUMBER); + uint8_t l_buf[l_size] = {0}; + auto l_target = TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL; + // Read first 4 bytes of section data from the PNOR DD + // Note: Do not need to worry about ECC as the 9th byte is the first + // ECC byte. + l_errhdl = DeviceFW::deviceRead(l_target, l_buf, l_size, + DEVICE_PNOR_ADDRESS(0,io_TOC[i_secId].flashAddr)); + if (l_errhdl) + { + break; + } - // Check if first 4 bytes match the Secureboot Magic Number - io_TOC[i_secId].secure &= PNOR::cmpSecurebootMagicNumber(l_buf); + // Check if first 4 bytes match the Secureboot Magic Number + io_TOC[i_secId].secure &= PNOR::cmpSecurebootMagicNumber(l_buf); + } } #endif -#endif } while (0); diff --git a/src/usr/pnor/pnor_common.H b/src/usr/pnor/pnor_common.H index fa7e037cc..448d44cc5 100644 --- a/src/usr/pnor/pnor_common.H +++ b/src/usr/pnor/pnor_common.H @@ -73,9 +73,8 @@ namespace PNOR { void physicalToMmioOffset(uint64_t i_hbbAddress, uint64_t& o_mmioOffset); - // @TODO RTC 168021 Remove legacy extensions when all secure sections - // are supported via story 168021 - + // @TODO RTC 178520 Remove legacy extensions when all secure sections + // are supported /** * @brief Reads version header of section, hashes it, and extends to tpm * buffer list. @@ -86,9 +85,11 @@ namespace PNOR { * * @return Error from operation */ +#ifndef __HOSTBOOT_RUNTIME errlHndl_t extendHash(uint64_t i_addr, size_t i_size, const PNOR::SectionId i_sectionId); +#endif } diff --git a/src/usr/pnor/pnor_utils.C b/src/usr/pnor/pnor_utils.C index 0047b2016..1ba448639 100644 --- a/src/usr/pnor/pnor_utils.C +++ b/src/usr/pnor/pnor_utils.C @@ -345,7 +345,7 @@ PNOR::parseEntries (ffs_hdr* i_ffs_hdr, #ifdef BOOTLOADER io_TOC[secId].secure = PNOR::isEnforcedSecureSection(secId); -#elif !defined(__HOSTBOOT_RUNTIME) // runtime is handled by rt_pnor code +#else // Check if PNOR section has a secureHeader or not. l_errhdl = PNOR::setSecure(secId, io_TOC); if (l_errhdl) diff --git a/src/usr/pnor/pnorrp.C b/src/usr/pnor/pnorrp.C index 42082aab7..d138d5e39 100644 --- a/src/usr/pnor/pnorrp.C +++ b/src/usr/pnor/pnorrp.C @@ -356,7 +356,7 @@ void PnorRP::initDaemon() break; } - // @TODO RTC 168021 Remove the non-secure extension path and + // @TODO RTC 178520 Remove the non-secure extension path and // always used the converged HBB extension path. // If secured, extend base image (HBB) when Hostboot first starts. diff --git a/src/usr/pnor/runtime/rt_pnor.C b/src/usr/pnor/runtime/rt_pnor.C index c65ad905d..ae187a057 100644 --- a/src/usr/pnor/runtime/rt_pnor.C +++ b/src/usr/pnor/runtime/rt_pnor.C @@ -50,9 +50,6 @@ extern trace_desc_t* g_trac_pnor; */ TASK_ENTRY_MACRO( RtPnor::init ); -// @TODO RTC:155374 Remove this in the future -const size_t BEST_EFFORT_NUM_BYTES = 32; - /** * @brief Return the size and address of a given section of PNOR data */ @@ -159,33 +156,42 @@ errlHndl_t RtPnor::getSectionInfo(PNOR::SectionId i_section, do { bool l_inhibited = false; - #ifdef CONFIG_SECUREBOOT + bool l_secure = false; +#ifdef CONFIG_SECUREBOOT l_inhibited = PNOR::isInhibitedSection(i_section); - #endif - if (i_section == PNOR::INVALID_SECTION || l_inhibited) + l_secure = iv_TOC[i_section].secure; +#endif + if (i_section == PNOR::INVALID_SECTION || l_inhibited || l_secure) { - TRACFCOMP(g_trac_pnor, "RtPnor::getSectionInfo: Invalid Section" - " %d", (int)i_section); - #ifdef CONFIG_SECUREBOOT + TRACFCOMP(g_trac_pnor, "RtPnor::getSectionInfo: Invalid Section %d", + static_cast<int>(i_section)); +#ifdef CONFIG_SECUREBOOT if (l_inhibited) { - TRACFCOMP(g_trac_pnor, "RtPnor::getSectionInfo: " - "attribute overrides inhibited by secureboot"); + TRACFCOMP(g_trac_pnor, ERR_MRK"RtPnor::getSectionInfo: attribute overrides inhibited by secureboot"); + } + else if (l_secure) + { + TRACFCOMP(g_trac_pnor, ERR_MRK"RtPnor::getSectionInfo: secure sections should be loaded via Hostboot Reserved Memory"); } - #endif +#endif /*@ * @errortype * @moduleid PNOR::MOD_RTPNOR_GETSECTIONINFO * @reasoncode PNOR::RC_RTPNOR_INVALID_SECTION * @userdata1 PNOR::SectionId - * @userdata2 Inhibited by secureboot + * @userdata2[0:31] Inhibited by secureboot + * @userdata2[32:63] Indication of a secure section * @devdesc invalid section passed to getSectionInfo or * section prohibited by secureboot */ l_err = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE, PNOR::MOD_RTPNOR_GETSECTIONINFO, PNOR::RC_RTPNOR_INVALID_SECTION, - i_section, l_inhibited, true); + i_section, + TWO_UINT32_TO_UINT64(l_inhibited, + l_secure), + true); break; } @@ -258,16 +264,6 @@ errlHndl_t RtPnor::getSectionInfo(PNOR::SectionId i_section, (iv_TOC[i_section].version & FFS_VERS_SHA512) ? true : false; o_info.sha512perEC = (iv_TOC[i_section].version & FFS_VERS_SHA512_PER_EC) ? true : false; -#ifdef CONFIG_SECUREBOOT - o_info.secure = iv_TOC[i_section].secure; - // We don't verify PNOR sections at runtime, but we - // still have to bypass the secure header - if(o_info.secure) - { - o_info.vaddr += PAGESIZE; - o_info.size -= PAGESIZE; - } -#endif } while (0); TRACFCOMP(g_trac_pnor, EXIT_MRK"RtPnor::getSectionInfo"); @@ -722,15 +718,6 @@ errlHndl_t RtPnor::readTOC () TRACFCOMP(g_trac_pnor, "RtPnor::readTOC: parseTOC failed"); break; } - - // Check if PNOR section has a secureHeader or not. - // Cannot do a device read during parseTOC in Runtime, so do after. - l_err = setSecure(l_toc0Buffer, iv_TOC); - if (l_err) - { - TRACFCOMP(g_trac_pnor, "RtPnor::readTOC: setSecure failed"); - break; - } } } while (0); @@ -743,73 +730,6 @@ errlHndl_t RtPnor::readTOC () return l_err; } -// @TODO RTC:155374 Remove this in the future -errlHndl_t RtPnor::setSecure(const uint8_t* i_tocBuffer, - PNOR::SectionData_t* io_TOC) const -{ - errlHndl_t l_errhdl = nullptr; - - assert(i_tocBuffer != nullptr, "RtPnor::setSecure received a NULL tocBuffer to read"); - assert(io_TOC != nullptr, "RtPnor::setSecure received a NULL toc to modify"); - - do { - // Set secure flag for each section after the TOC - // Walk through all the entries in the table and parse the data. - auto const l_ffs_hdr = reinterpret_cast<const ffs_hdr*>(i_tocBuffer); - for(uint32_t i=0; i<l_ffs_hdr->entry_count; ++i) - { - PNOR::SectionId l_secId = PNOR::INVALID_SECTION; - - // Get current entry section id - auto cur_entry = &(l_ffs_hdr->entries[i]); - PNOR::getSectionEnum(cur_entry, &l_secId); - if(l_secId == PNOR::INVALID_SECTION) - { - TRACFCOMP(g_trac_pnor, "RtPnor::setSecure Unrecognized Section name(%s), skipping",cur_entry->name); - continue; - } - - // Set secure field based on enforced policy - io_TOC[l_secId].secure = PNOR::isEnforcedSecureSection(l_secId); - -#ifdef CONFIG_SECUREBOOT_BEST_EFFORT - if (io_TOC[l_secId].secure) - { - // Apply best effort policy by checking if the section appears to have a - // secure header - // Need to read first 4 bytes of data to check version header - // Note: For OPAL and PHYP need to read 8 bytes for ECC checking - // In CXX test a pnorDD read is called and requires a - // multiple of 4 bytes. If the section as ECC then it - // needs to be a multiple of 4 bytes after ECC. - // 32 Bytes fulfills both requirements. - size_t l_size = BEST_EFFORT_NUM_BYTES; - uint8_t l_buf[l_size] = {0}; - - bool l_ecc = io_TOC[l_secId].integrity & FFS_INTEG_ECC_PROTECT; - // Read first 8 bytes of section data from PNOR - l_errhdl = readFromDevice(iv_masterProcId, - static_cast<PNOR::SectionId>(l_secId), - 0, l_size, l_ecc, l_buf); - if (l_errhdl) - { - break; - } - - // Check if first 4 bytes match the Secureboot Magic Number - io_TOC[l_secId].secure &= PNOR::cmpSecurebootMagicNumber(l_buf); - } -#endif - } - if (l_errhdl) - { - break; - } - } while(0); - - return l_errhdl; -} - /***********************************************************/ RtPnor& RtPnor::getInstance() { diff --git a/src/usr/pnor/runtime/rt_pnor.H b/src/usr/pnor/runtime/rt_pnor.H index b49aa3aaf..6af111995 100644 --- a/src/usr/pnor/runtime/rt_pnor.H +++ b/src/usr/pnor/runtime/rt_pnor.H @@ -177,23 +177,6 @@ class RtPnor */ errlHndl_t readTOC(); - // @TODO RTC:155374 Remove this in the future - /** - * @brief Check if PNOR sections appear to be secure and sets the - * internal TOC of PnorRp accordingly. - * Note: The setting of the flag is based on the Secureboot policy. - * Note: Done separately from pnor common code to prevent PNOR - * deadlock when reading section bytes. - * - * @param[in] io_TOC Pointer to internal array of section data that - * represents the TOC of pnor flash - * Asserts if nullptr - * - * @return errlHndl_t Error log if request was invalid - */ - errlHndl_t setSecure(const uint8_t* i_tocBuffer, - PNOR::SectionData_t* io_TOC) const; - /** * @brief Get processor id of master proc and cache in internal variable * diff --git a/src/usr/pnor/spnorrp.C b/src/usr/pnor/spnorrp.C index d6dd6fefb..0b298afeb 100644 --- a/src/usr/pnor/spnorrp.C +++ b/src/usr/pnor/spnorrp.C @@ -284,18 +284,38 @@ uint64_t SPnorRP::verifySections(SectionId i_id, LoadRecord* o_rec) if (!l_info.secure) { -#ifdef CONFIG_SECUREBOOT_BEST_EFFORT - TRACFCOMP(g_trac_pnor,"PNOR::verifySections> called on unsecured section - Best effort policy skipping"); - break; -#else - TRACFCOMP(g_trac_pnor,ERR_MRK"PNOR::verifySections> called on " - "unsecured section"); - - // TODO securebootp9 revisit this assert code and replace with error log - // code if it is deemed that this assert could happen in the field - assert(false,"PNOR::loadSection> section %i is not a secure section", - i_id); -#endif + if(SECUREBOOT::bestEffortPolicy()) + { + TRACFCOMP(g_trac_pnor,"PNOR::verifySections> called on unsecured section - Best effort policy skipping"); + break; + } + else + { + TRACFCOMP(g_trac_pnor,ERR_MRK"PNOR::verifySections> called on " + "unsecured section"); + + /*@ + * @errortype + * @severity ERRL_SEV_CRITICAL_SYS_TERM + * @moduleid PNOR::MOD_SPNORRP_VERIFYSECTIONS + * @reasoncode PNOR::RC_UNSIGNED_PNOR_SECTION + * @userdata1 PNOR section requested to verify + * @userdata2 0 + * @devdesc Cannot verify unsigned PNOR section + * @custdesc Security failure: unable to securely load + * requested firmware. + */ + l_errhdl = new ERRORLOG::ErrlEntry( + ERRORLOG::ERRL_SEV_CRITICAL_SYS_TERM, + PNOR::MOD_SPNORRP_VERIFYSECTIONS, + PNOR::RC_UNSIGNED_PNOR_SECTION, + TO_UINT64(i_id), + 0, + true /*Add HB SW Callout*/); + l_errhdl->collectTrace(PNOR_COMP_NAME); + l_errhdl->collectTrace(SECURE_COMP_NAME); + break; + } } l_info.vaddr -= PAGESIZE; // back up a page to expose the secure header diff --git a/src/usr/secureboot/HBconfig b/src/usr/secureboot/HBconfig index 425593e74..7a6c29cf3 100644 --- a/src/usr/secureboot/HBconfig +++ b/src/usr/secureboot/HBconfig @@ -4,9 +4,9 @@ config SECUREBOOT help Enable and enforce secure boot -# @TODO RTC:155374 Remove this in the future +# @TODO RTC:178520 Remove this in the future config SECUREBOOT_BEST_EFFORT - default y if SECUREBOOT + default n depends on SECUREBOOT help Enable Best effort Secureboot. Should only be used for diff --git a/src/usr/secureboot/base/header.C b/src/usr/secureboot/base/header.C index 2f62f804c..f2c41069c 100644 --- a/src/usr/secureboot/base/header.C +++ b/src/usr/secureboot/base/header.C @@ -36,7 +36,7 @@ namespace SECUREBOOT return Singleton<Header>::instance(); } - // @TODO RTC 168021 Converge on a single method of reading the secure + // @TODO RTC 178520 Converge on a single method of reading the secure // header void Header::loadSecurely() { @@ -49,7 +49,7 @@ namespace SECUREBOOT _set(pSecureHeader); } - // @TODO RTC 168021 Converge on a single method of reading the secure + // @TODO RTC 178520 Converge on a single method of reading the secure // header void Header::setNonSecurely( const void* const i_pHeader) diff --git a/src/usr/secureboot/base/securerommgr.C b/src/usr/secureboot/base/securerommgr.C index dc8e6155c..d596d5c64 100644 --- a/src/usr/secureboot/base/securerommgr.C +++ b/src/usr/secureboot/base/securerommgr.C @@ -64,6 +64,11 @@ errlHndl_t initializeSecureRomManager(void) return Singleton<SecureRomManager>::instance().initialize(); } +bool secureRomValidPolicy() +{ + return Singleton<SecureRomManager>::instance().secureRomValidPolicy(); +} + /** * @brief Verify Signed Container */ @@ -71,8 +76,7 @@ errlHndl_t verifyContainer(void * i_container, const SHA512_t* i_hwKeyHash) { errlHndl_t l_errl = nullptr; - // @TODO RTC:170136 remove isValid check - if(Singleton<SecureRomManager>::instance().isValid()) + if(Singleton<SecureRomManager>::instance().secureRomValidPolicy()) { l_errl = Singleton<SecureRomManager>::instance(). verifyContainer(i_container,i_hwKeyHash); @@ -87,12 +91,15 @@ errlHndl_t verifyContainer(void * i_container, const SHA512_t* i_hwKeyHash) */ void hashBlob(const void * i_blob, size_t i_size, SHA512_t o_buf) { - // @TODO RTC:170136 remove isValid check - if(Singleton<SecureRomManager>::instance().isValid()) + if(Singleton<SecureRomManager>::instance().secureRomValidPolicy()) { return Singleton<SecureRomManager>::instance(). hashBlob(i_blob, i_size, o_buf); } + else + { + memset(o_buf, 0, sizeof(SHA512_t)); + } } /** @@ -101,8 +108,7 @@ void hashBlob(const void * i_blob, size_t i_size, SHA512_t o_buf) */ void hashConcatBlobs(const blobPair_t &i_blobs, SHA512_t o_buf) { - // @TODO RTC:170136 remove isValid check - if(Singleton<SecureRomManager>::instance().isValid()) + if(Singleton<SecureRomManager>::instance().secureRomValidPolicy()) { return Singleton<SecureRomManager>::instance(). hashConcatBlobs(i_blobs, o_buf); @@ -115,7 +121,7 @@ void hashConcatBlobs(const blobPair_t &i_blobs, SHA512_t o_buf) void getHwKeyHash(SHA512_t o_hash) { // @TODO RTC:170136 remove isValid check - if(Singleton<SecureRomManager>::instance().isValid()) + if(Singleton<SecureRomManager>::instance().secureRomValidPolicy()) { return Singleton<SecureRomManager>::instance().getHwKeyHash(o_hash); } @@ -148,31 +154,54 @@ using namespace SECUREBOOT; */ errlHndl_t SecureRomManager::initialize() { - TRACDCOMP(g_trac_secure,ENTER_MRK"SecureRomManager::initialize()"); + TRACFCOMP(g_trac_secure,ENTER_MRK"SecureRomManager::initialize()"); errlHndl_t l_errl = nullptr; uint32_t l_rc = 0; do{ - // @TODO RTC:170136 terminate in initialize if the securebit is on - // and code is not valid. Remove all isValid() checks in rest of - // SecureRomManager. - // Check if secureboot data is valid. + // Check if bootloader to hostboot data is valid. iv_secureromValid = g_BlToHbDataManager.isValid(); + if (!iv_secureromValid) { - // The Secure ROM has already been initialized - TRACDCOMP(g_trac_secure,"SecureRomManager::initialize(): SecureROM invalid, skipping functionality"); - + // Allow skipping functionality if secure rom is invalid if best + // effort policy enabled + if(SECUREBOOT::bestEffortPolicy()) + { + TRACFCOMP(g_trac_secure,INFO_MRK"SecureRomManager::initialize(): SecureROM invalid, skipping functionality"); #ifdef CONFIG_CONSOLE - CONSOLE::displayf(SECURE_COMP_NAME, "SecureROM invalid - skipping functionality"); + CONSOLE::displayf(SECURE_COMP_NAME, "SecureROM invalid - skipping functionality"); #endif - - // Can skip the rest of this function - break; + printk("SecureRomManager SecureROM invalid -- skipping functionality\n"); + // Can skip the rest of this function + break; + } + // Otherwise enforce securerom to be valid. + else + { + TRACFCOMP(g_trac_secure,ERR_MRK"SecureRomManager::initialize(): SecureROM invalid"); +#ifdef CONFIG_CONSOLE + CONSOLE::displayf(SECURE_COMP_NAME, ERR_MRK"SecureROM invalid"); +#endif + printk("ERR> SecureRomManager SecureROM invalid\n"); + /*@ + * @errortype + * @moduleid SECUREBOOT::MOD_SECURE_ROM_INIT + * @reasoncode SECUREBOOT::RC_SECROM_INVALID + * @devdesc Valid securerom not present + * @custdesc Security failure occurred during the IPL of + * the system. + */ + l_errl = new ERRORLOG::ErrlEntry( ERRORLOG::ERRL_SEV_UNRECOVERABLE, + SECUREBOOT::MOD_SECURE_ROM_INIT, + SECUREBOOT::RC_SECROM_INVALID); + l_errl->collectTrace(SECURE_COMP_NAME,ERROR_TRACE_SIZE); + break; + } } - TRACDCOMP(g_trac_secure,"SecureRomManager::initialize(): SecureROM valid, enabling functionality"); + TRACFCOMP(g_trac_secure,"SecureRomManager::initialize(): SecureROM valid, enabling functionality"); #ifdef CONFIG_CONSOLE CONSOLE::displayf(SECURE_COMP_NAME, "SecureROM valid - enabling functionality"); #endif @@ -281,12 +310,11 @@ errlHndl_t SecureRomManager::verifyContainer(void * i_container, do{ // Check if secureboot data is valid. - if (!iv_secureromValid) + if (!secureRomValidPolicy()) { // Can skip the rest of this function break; } - // Check to see if ROM has already been initialized // This should have been done early in IPL so assert if this // is not the case as system is in a bad state @@ -393,7 +421,7 @@ void SecureRomManager::hashBlob(const void * i_blob, size_t i_size, SHA512_t o_b TRACDCOMP(g_trac_secure,INFO_MRK"SecureRomManager::hashBlob()"); // Check if secureboot data is valid. - if (iv_secureromValid) + if (secureRomValidPolicy()) { // Check to see if ROM has already been initialized // This should have been done early in IPL so assert if this @@ -426,7 +454,7 @@ void SecureRomManager::hashConcatBlobs(const blobPair_t &i_blobs, SHA512_t o_buf) const { // Check if secureboot data is valid. - if (iv_secureromValid) + if (secureRomValidPolicy()) { std::vector<uint8_t> concatBuf; for (const auto &it : i_blobs) @@ -443,9 +471,24 @@ void SecureRomManager::hashConcatBlobs(const blobPair_t &i_blobs, } } -bool SecureRomManager::isValid() +bool SecureRomManager::secureRomValidPolicy() const { - return iv_secureromValid; + bool l_policy = true; + if (bestEffortPolicy()) + { + // Set policy based on secure ROM status + l_policy = iv_secureromValid; + } + else + { + // Assert secure rom is valid in this mode. + // The initialize function should have created an error log already if + // this case is false, so this code path should not be hit. + assert(iv_secureromValid==true, "SecureRomManager cannot operate with invalid secure rom"); + l_policy = true; + } + + return l_policy; } /******************** @@ -458,7 +501,7 @@ bool SecureRomManager::isValid() void SecureRomManager::getHwKeyHash() { // Check if secureboot data is valid. - if (iv_secureromValid) + if (secureRomValidPolicy()) { iv_key_hash = reinterpret_cast<const SHA512_t*>( g_BlToHbDataManager.getHwKeysHash()); @@ -471,7 +514,7 @@ void SecureRomManager::getHwKeyHash() void SecureRomManager::getHwKeyHash(SHA512_t o_hash) { // Check if secureboot data is valid. - if (iv_secureromValid) + if (secureRomValidPolicy()) { memcpy(o_hash, iv_key_hash, sizeof(SHA512_t)); } diff --git a/src/usr/secureboot/base/securerommgr.H b/src/usr/secureboot/base/securerommgr.H index bf8812342..b221d2c10 100644 --- a/src/usr/secureboot/base/securerommgr.H +++ b/src/usr/secureboot/base/securerommgr.H @@ -97,11 +97,13 @@ class SecureRomManager void hashConcatBlobs (const blobPair_t &i_blobs, SHA512_t o_buf) const; /* - * @brief Getter for private "is valid" variable + * @brief Determines if best effort policy is enabled and allowed when + * SecureROM is invalid. + * Asserts secure rom is valid if bestEffortPolicy is false * - * @return bool - True if valid, false otherwise + * @return bool - True if enabled, false otherwise */ - bool isValid(); + bool secureRomValidPolicy() const; /* * @brief Get offset of function from the start of the SecureROM diff --git a/src/usr/secureboot/base/service.C b/src/usr/secureboot/base/service.C index 6d0bf8ff3..98a750c98 100644 --- a/src/usr/secureboot/base/service.C +++ b/src/usr/secureboot/base/service.C @@ -92,13 +92,8 @@ void* initializeBase(void* unused) do { - - // Load original secureboot header. - if (enabled()) - { - Singleton<Header>::instance().loadSecurely(); - } - + // SecureROM manager verifies if the content necessary for secureboot in + // the BltoHbData is valid or not. So initialize before anything else. // Don't enable SecureRomManager in VPO #ifndef CONFIG_P9_VPO_COMPILE @@ -109,6 +104,12 @@ void* initializeBase(void* unused) break; } #endif + + // Load original secureboot header. + if (enabled()) + { + Singleton<Header>::instance().loadSecurely(); + } } while(0); return l_errl; @@ -121,6 +122,11 @@ bool enabled() } #endif +bool bestEffortPolicy() +{ + return Singleton<Settings>::instance().getBestEffortPolicy(); +} + errlHndl_t getSecuritySwitch(uint64_t& o_regValue, TARGETING::Target* i_pProc) { return Singleton<Settings>::instance().getSecuritySwitch(o_regValue, @@ -177,6 +183,9 @@ void handleSecurebootFailure(errlHndl_t &io_err, bool i_waitForShutdown) HWAS::SRCI_PRIORITY_HIGH); // Add Security related user details + // @TODO RTC: 176134 A chain of calls leads to a portion of code in the ext + // img. If we get an HBI page verify failure and the ext + // image is corrupted, we will hang. addSecureUserDetailsToErrolog(io_err); io_err->collectTrace(SECURE_COMP_NAME,MAX_ERROR_TRACE_SIZE); diff --git a/src/usr/secureboot/base/settings.C b/src/usr/secureboot/base/settings.C index 078b9e1ed..0e2e2ea02 100644 --- a/src/usr/secureboot/base/settings.C +++ b/src/usr/secureboot/base/settings.C @@ -101,6 +101,21 @@ namespace SECUREBOOT securitySwitchValue,cbsValue); } #endif + +#ifdef CONFIG_SECUREBOOT_BEST_EFFORT + iv_bestEffortPolicy = true; +#else + if (iv_enabled) + { + iv_bestEffortPolicy = false; + } + else + { + iv_bestEffortPolicy = true; + } +#endif + SB_INF("getBestEffortPolicy() state:%i",iv_bestEffortPolicy); + printk("SECUREBOOT::bestEffortPolicy() state:%i\n", iv_bestEffortPolicy); } bool Settings::getEnabled() const @@ -108,6 +123,11 @@ namespace SECUREBOOT return iv_enabled; } + bool Settings::getBestEffortPolicy() const + { + return iv_bestEffortPolicy; + } + errlHndl_t Settings::getJumperState(SecureJumperState& o_state, Target* i_pProc) const { @@ -364,5 +384,4 @@ namespace SECUREBOOT return l_errl; } - } diff --git a/src/usr/secureboot/runtime/test/testsecureboot_rt.H b/src/usr/secureboot/runtime/test/testsecureboot_rt.H index a7bd93830..6d63b4fd7 100644 --- a/src/usr/secureboot/runtime/test/testsecureboot_rt.H +++ b/src/usr/secureboot/runtime/test/testsecureboot_rt.H @@ -42,6 +42,8 @@ #include "common/securetrace.H" #include <secureboot/service.H> #include <secureboot/settings.H> +#include <pnor/pnorif.H> +#include <pnor/pnor_reasoncodes.H> class SecurebootRtTestSuite: public CxxTest::TestSuite { @@ -158,6 +160,66 @@ class SecurebootRtTestSuite: public CxxTest::TestSuite SB_EXIT("SecurebootRtTestSuite::testBaseInterfaces"); } + void testAccessSecurePnorSection() + { + SB_ENTER("testAccessSecurePnorSection"); + + errlHndl_t l_err = nullptr; + PNOR::SectionId l_id = PNOR::OCC; + PNOR::SectionInfo_t l_info; + + // Ensure we cannot read secure sections from PNOR at Runtime + l_err = PNOR::getSectionInfo(l_id, l_info); + if(l_err) + { + if (l_err->reasonCode() == PNOR::RC_RTPNOR_INVALID_SECTION) + { + delete l_err; + l_err = nullptr; + } + else + { + TS_FAIL("testAccessSecurePnorSection: unexpected reason code for Secure Section %s. Expected RC 0x%.4X Actual RC 0x%.4X", + PNOR::SectionIdToString(l_id), + PNOR::RC_RTPNOR_INVALID_SECTION, + l_err->reasonCode()); + errlCommit(l_err, SECURE_COMP_ID); + } + } + else + { + TS_FAIL("testAccessSecurePnorSection: Did not catch illegal PNOR access of Secure Section %s", + PNOR::SectionIdToString(l_id)); + } + + l_id = PNOR::HB_EXT_CODE; + l_err = PNOR::getSectionInfo(l_id, l_info); + if(l_err) + { + if (l_err->reasonCode() == PNOR::RC_RTPNOR_INVALID_SECTION) + { + delete l_err; + l_err = nullptr; + } + else + { + TS_FAIL("testAccessSecurePnorSection: unexpected reason code for Secure Section %s. Expected RC 0x%.4X Actual RC 0x%.4X", + PNOR::SectionIdToString(l_id), + PNOR::RC_RTPNOR_INVALID_SECTION, + l_err->reasonCode()); + errlCommit(l_err, SECURE_COMP_ID); + } + } + else + { + TS_FAIL("testAccessSecurePnorSection: Did not catch illegal PNOR access of Secure Section %s", + PNOR::SectionIdToString(l_id)); + } + + + SB_EXIT("testAccessSecurePnorSection"); + } + private: diff --git a/src/usr/vfs/vfsrp.C b/src/usr/vfs/vfsrp.C index 63f2a15e7..0682460ee 100644 --- a/src/usr/vfs/vfsrp.C +++ b/src/usr/vfs/vfsrp.C @@ -362,9 +362,9 @@ void VfsRp::msgHandler() // Failed to pass secureboot verification if(l_errl) { + msg->data[1] = -EACCES; SECUREBOOT::handleSecurebootFailure( l_errl,false); - msg->data[1] = -EACCES; break; } } |
