diff options
author | Mike Baiocchi <mbaiocch@us.ibm.com> | 2017-07-01 01:43:12 -0500 |
---|---|---|
committer | Daniel M. Crowell <dcrowell@us.ibm.com> | 2017-08-09 13:47:00 -0400 |
commit | 776d1086a7ed224c482d2da3c49b2c597b8776ab (patch) | |
tree | 1da81d9f9edec18550b4bd69dcfd95140e741fa1 /src | |
parent | e676209189922c5105629a9785a25958ba0972a9 (diff) | |
download | talos-hostboot-776d1086a7ed224c482d2da3c49b2c597b8776ab.tar.gz talos-hostboot-776d1086a7ed224c482d2da3c49b2c597b8776ab.zip |
Secureboot: Inhibit attribute overrides and sync exposures
For Secureboot purposes, we don't consider the FSP a secure source. So
this commit inhibts attribute overrides and any sort of attribute syncing
from the FSP.
Change-Id: I941ab5083d3055bc29237839aaaf4b723a2b0e90
RTC:175071
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/42687
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Reviewed-by: Stephen M. Cprek <smcprek@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/include/kernel/bltohbdatamgr.H | 3 | ||||
-rw-r--r-- | src/include/usr/secureboot/service.H | 9 | ||||
-rw-r--r-- | src/usr/fapi2/plat_attr_override_sync.C | 48 | ||||
-rw-r--r-- | src/usr/initservice/istepdispatcher/istepdispatcher.C | 11 | ||||
-rw-r--r-- | src/usr/secureboot/base/service.C | 32 | ||||
-rw-r--r-- | src/usr/secureboot/runtime/rt_secureboot.C | 35 | ||||
-rw-r--r-- | src/usr/secureboot/runtime/test/testsecureboot_rt.H | 20 | ||||
-rw-r--r-- | src/usr/targeting/attrPlatOverride.C | 8 | ||||
-rwxr-xr-x | src/usr/targeting/attrrp.C | 7 | ||||
-rw-r--r-- | src/usr/targeting/attrsync.C | 27 | ||||
-rw-r--r-- | src/usr/targeting/runtime/attrPlatOverride_rt.C | 2 | ||||
-rw-r--r-- | src/usr/targeting/test/testattrtank.H | 14 |
12 files changed, 202 insertions, 14 deletions
diff --git a/src/include/kernel/bltohbdatamgr.H b/src/include/kernel/bltohbdatamgr.H index 6b6f8b831..42ded91cc 100644 --- a/src/include/kernel/bltohbdatamgr.H +++ b/src/include/kernel/bltohbdatamgr.H @@ -66,6 +66,9 @@ class BlToHbDataManager // Converts HBB header pointer to a 64-bit address const uint64_t getHbbHeaderAddr() const; + // Needed for testcases + friend class AttrTankTest; + public: /** diff --git a/src/include/usr/secureboot/service.H b/src/include/usr/secureboot/service.H index 2f0430d83..27c35f6d4 100644 --- a/src/include/usr/secureboot/service.H +++ b/src/include/usr/secureboot/service.H @@ -290,6 +290,15 @@ namespace SECUREBOOT */ void addSecureUserDetailsToErrolog(errlHndl_t & io_err); + /* + * @brief Determines if Attribute Overrides are Allowed + * If Secureboot is enabled, check allowAttrOverrides setting; + * If Secureboot is not enabled, always allow Attribute Overrides + * + * @return bool TRUE if Attribute Overrides Are Allowed; FALSE otherwise + */ + bool allowAttrOverrides(); + } #endif diff --git a/src/usr/fapi2/plat_attr_override_sync.C b/src/usr/fapi2/plat_attr_override_sync.C index 27659dc06..91842a3ec 100644 --- a/src/usr/fapi2/plat_attr_override_sync.C +++ b/src/usr/fapi2/plat_attr_override_sync.C @@ -51,6 +51,7 @@ #include <fapi2_attribute_service.H> #include <util/utilmbox_scratch.H> #include <plat_utils.H> +#include <secureboot/service.H> namespace fapi2 { @@ -72,6 +73,13 @@ uint8_t g_attrOverrideFapiTank = 0; //****************************************************************************** void directOverride() { + if (!SECUREBOOT::allowAttrOverrides()) + { + FAPI_INF("directOverride: skipping since " + "attribute overrides are not allowed"); + return; + } + #ifndef __HOSTBOOT_RUNTIME uint32_t l_targetType = TARGETING::TYPE_NA; // Apply the attribute override @@ -239,7 +247,14 @@ void AttrOverrideSync::monitorForFspMessages() l_chunk.iv_size = l_pMsg->data[1]; l_chunk.iv_pAttributes = static_cast<uint8_t *>(l_pMsg->extra_data); - if (l_chunk.iv_pAttributes == NULL) + if (SECUREBOOT::allowAttrOverrides() == false) + { + FAPI_ERR("monitorForFspMessages: Ignoring Set Overrides " + "Message (0x%X) from FSP since attribute overrides " + "are not allowed", + l_pMsg->type); + } + else if (l_chunk.iv_pAttributes == NULL) { FAPI_ERR("monitorForFspMessages: tank %d, size %d, NULL data pointer", l_tank, l_chunk.iv_size); @@ -364,6 +379,7 @@ errlHndl_t AttrOverrideSync::sendAttrsToFsp( //****************************************************************************** void AttrOverrideSync::sendAttrOverridesAndSyncsToFsp() { + #ifndef __HOSTBOOT_RUNTIME const uint32_t MAILBOX_CHUNK_SIZE = 4096; @@ -495,6 +511,7 @@ void AttrOverrideSync::sendAttrOverridesAndSyncsToFsp() //****************************************************************************** void AttrOverrideSync::sendFapiAttrSyncs() { + #ifndef __HOSTBOOT_RUNTIME const uint32_t MAILBOX_CHUNK_SIZE = 4096; @@ -540,9 +557,15 @@ void AttrOverrideSync::getAttrOverridesFromFsp() { #ifndef __HOSTBOOT_RUNTIME FAPI_IMP("Requesting Attribute Overrides from the FSP"); - errlHndl_t l_pErr = NULL; + if (!SECUREBOOT::allowAttrOverrides()) + { + FAPI_INF("AttrOverrideSync::getAttrOverridesFromFsp: skipping " + "since attribute overrides are not allowed"); + return; + } + msg_t * l_pMsg = msg_allocate(); l_pMsg->type = MSG_GET_OVERRIDES; l_pMsg->data[0] = 0; @@ -582,6 +605,14 @@ bool AttrOverrideSync::getAttrOverride(const AttributeId i_attrId, return false; } + // This check after previous two for performance reasons + if (!SECUREBOOT::allowAttrOverrides()) + { + FAPI_INF("AttrOverrideSync::getAttrOverride: skipping " + "since attribute overrides are not allowed"); + return false; + } + // Do the work of figuring out the target's type/position/node and find out // if there is an override for this target uint32_t l_targetType = getTargetType(i_pTarget); @@ -778,6 +809,12 @@ void AttrOverrideSync::triggerAttrSync() //****************************************************************************** void AttrOverrideSync::clearAttrOverrides() { + if (!SECUREBOOT::allowAttrOverrides()) + { + FAPI_INF("AttrOverrideSync::clearAttrOverrides: skipping clear calls" + "since attribute overrides are not allowed"); + return; + } #ifndef __HOSTBOOT_RUNTIME // Debug Channel is clearing all attribute overrides. FAPI_INF("Debug Channel CLEAR_ALL_OVERRIDES"); @@ -789,6 +826,13 @@ void AttrOverrideSync::clearAttrOverrides() //****************************************************************************** void AttrOverrideSync::dynSetAttrOverrides() { + if (!SECUREBOOT::allowAttrOverrides()) + { + FAPI_INF("AttrOverrideSync::dynSetAttrOverrides: skipping since " + "attribute overrides are not allowed"); + return; + } + #ifndef __HOSTBOOT_RUNTIME errlHndl_t err = NULL; int64_t rc = 0; diff --git a/src/usr/initservice/istepdispatcher/istepdispatcher.C b/src/usr/initservice/istepdispatcher/istepdispatcher.C index 5edf4f835..8c0db6bcb 100644 --- a/src/usr/initservice/istepdispatcher/istepdispatcher.C +++ b/src/usr/initservice/istepdispatcher/istepdispatcher.C @@ -331,7 +331,7 @@ void IStepDispatcher::init(errlHndl_t &io_rtaskRetErrl) l_attrOverridesExist = l_pTopLevelTarget-> getAttr<TARGETING::ATTR_PLCK_IPL_ATTR_OVERRIDES_EXIST>(); - if (l_attrOverridesExist) + if (l_attrOverridesExist && SECUREBOOT::allowAttrOverrides()) { fapi2::theAttrOverrideSync().getAttrOverridesFromFsp(); } @@ -343,7 +343,6 @@ void IStepDispatcher::init(errlHndl_t &io_rtaskRetErrl) } err = executeAllISteps(); - if(err) { TRACFCOMP(g_trac_initsvc, "ERROR: Failed executing all isteps," @@ -1744,7 +1743,7 @@ void IStepDispatcher::handleIStepRequestMsg(msg_t * & io_pMsg) uint8_t istep = ((io_pMsg->data[0] & 0x000000FF00000000) >> 32); uint8_t substep = (io_pMsg->data[0] & 0x00000000000000FF); - TRACFCOMP(g_trac_initsvc, ENTER_MRK"handleIstepRequestMsg: 0x%016x, istep: %d, substep: %d", + TRACFCOMP(g_trac_initsvc, ENTER_MRK"handleIStepRequestMsg: 0x%016x, istep: %d, substep: %d", io_pMsg->data[0], istep, substep); // Transfer ownership of the message pointer to iv_pIstepMsg because if the @@ -1813,7 +1812,7 @@ void IStepDispatcher::handleIStepRequestMsg(msg_t * & io_pMsg) // In istep mode we cannot do a reconfigure of any sort, so create // an error. - TRACFCOMP(g_trac_initsvc, ERR_MRK"handleIstepRequestMsg: IStep success and deconfigs, creating error"); + TRACFCOMP(g_trac_initsvc, ERR_MRK"handleIStepRequestMsg: IStep success and deconfigs, creating error"); err = failedDueToDeconfig(istep, substep, newIstep, newSubstep); } @@ -1840,14 +1839,14 @@ void IStepDispatcher::handleIStepRequestMsg(msg_t * & io_pMsg) if (io_pMsg == NULL) { // An IStep already responded to the message!! - TRACFCOMP(g_trac_initsvc, ERR_MRK"handleIstepRequestMsg: message response already sent!"); + TRACFCOMP(g_trac_initsvc, ERR_MRK"handleIStepRequestMsg: message response already sent!"); } else { if (msg_is_async(io_pMsg)) { // Unexpected - TRACFCOMP(g_trac_initsvc, ERR_MRK"handleIstepRequestMsg: async istep message!"); + TRACFCOMP(g_trac_initsvc, ERR_MRK"handleIStepRequestMsg: async istep message!"); } else { diff --git a/src/usr/secureboot/base/service.C b/src/usr/secureboot/base/service.C index fc4498908..6d0bf8ff3 100644 --- a/src/usr/secureboot/base/service.C +++ b/src/usr/secureboot/base/service.C @@ -41,6 +41,7 @@ #include "purge.H" #include <kernel/misc.H> #include <kernel/console.H> +#include <kernel/bltohbdatamgr.H> #include <console/consoleif.H> #include <util/misc.H> @@ -559,4 +560,35 @@ void addSecureUserDetailsToErrolog(errlHndl_t & io_err) //Note: adding UdTargetHwKeyHash left to Extended image } +#ifndef __HOSTBOOT_RUNTIME +bool allowAttrOverrides() +{ + bool retVal = false; + + if (enabled()) + { + if (g_BlToHbDataManager.getAllowAttrOverrides()) + { + retVal = true; + SB_INF("allowAttrOverrides: Allowing Attr Overrides in " + "Secure Mode: retVal=%d", retVal); + } + else + { + retVal = false; + SB_INF("allowAttrOverrides: DO NOT Allow Attr Overrides in " + "Secure Mode: retVal=%d", retVal); + } + } + else + { + retVal = true; + SB_DBG("allowAttrOverrides: Allow Attr Overrides in " + "Unsecure Mode: retVal=%d", retVal); + } + + return retVal; +}; +#endif + } //namespace SECUREBOOT diff --git a/src/usr/secureboot/runtime/rt_secureboot.C b/src/usr/secureboot/runtime/rt_secureboot.C index 1c84c2bf1..2277cce29 100644 --- a/src/usr/secureboot/runtime/rt_secureboot.C +++ b/src/usr/secureboot/runtime/rt_secureboot.C @@ -40,7 +40,7 @@ #include <targeting/common/commontargeting.H> #include <targeting/common/targetservice.H> #include <devicefw/userif.H> - +#include <util/misc.H> namespace SECUREBOOT { @@ -87,6 +87,39 @@ bool enabled() } #endif +#ifdef __HOSTBOOT_RUNTIME +bool allowAttrOverrides() +{ + bool retVal = false; + + if (enabled()) + { + // Check attribute to see if overrides are allowed in secure mode + if ( Util::isTargetingLoaded() ) + { + TARGETING::TargetService& tS = TARGETING::targetService(); + TARGETING::Target* sys = nullptr; + (void) tS.getTopLevelTarget( sys ); + assert(sys, "SECUREBOOT::allowAttrOverrides() system target is NULL"); + + retVal = sys->getAttr< + TARGETING::ATTR_ALLOW_ATTR_OVERRIDES_IN_SECURE_MODE>(); + + SB_INF("SECUREBOOT::allowAttrOverrides: " + "Inside Attr check: retVal=0x%X", + retVal); + } + } + else + { + // Allow Attribute Overrides in unsecure mode + retVal = true; + } + + return retVal; +} +#endif + int verify_container( const void* i_pContainer, const void* i_pHwKeyHash, diff --git a/src/usr/secureboot/runtime/test/testsecureboot_rt.H b/src/usr/secureboot/runtime/test/testsecureboot_rt.H index ef9a641a9..4173c0855 100644 --- a/src/usr/secureboot/runtime/test/testsecureboot_rt.H +++ b/src/usr/secureboot/runtime/test/testsecureboot_rt.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2016 */ +/* Contributors Listed Below - COPYRIGHT 2016,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -36,6 +36,7 @@ #include <config.h> #include "common/securetrace.H" +#include <secureboot/service.H> class SecurebootRtTestSuite: public CxxTest::TestSuite { @@ -95,6 +96,23 @@ class SecurebootRtTestSuite: public CxxTest::TestSuite SB_EXIT("SecurebootRtTestSuite::testVerifyContainer"); } + void testBaseInterfaces() + { + SB_ENTER("SecurebootRtTestSuite::testBaseInterfaces"); + + do { + + SB_INF("SECUREBOOT::enabled() = %d", SECUREBOOT::enabled()); + + SB_INF("SECUREBOOT::allowAttrOverrides() = %d", + SECUREBOOT::allowAttrOverrides()); + + } while(0); + + SB_EXIT("SecurebootRtTestSuite::testBaseInterfaces"); + } + + private: }; diff --git a/src/usr/targeting/attrPlatOverride.C b/src/usr/targeting/attrPlatOverride.C index a5f2a18c0..dafb27fb2 100644 --- a/src/usr/targeting/attrPlatOverride.C +++ b/src/usr/targeting/attrPlatOverride.C @@ -27,6 +27,7 @@ #include <targeting/common/trace.H> #include <targeting/common/targreasoncodes.H> #include <errl/errlmanager.H> +#include <secureboot/service.H> namespace TARGETING { @@ -71,6 +72,13 @@ errlHndl_t getAttrOverrides(PNOR::SectionInfo_t &i_sectionInfo, do { + if (!SECUREBOOT::allowAttrOverrides()) + { + TRACFCOMP(g_trac_targeting,"attrPlatOverride::getAttrOverrides: " + "skipping since Attribute Overrides are not allowed"); + break; + } + TRACFCOMP( g_trac_targeting, "Section id=%d, size=%d", i_sectionInfo.id, i_sectionInfo.size ); uint32_t l_index = 0; diff --git a/src/usr/targeting/attrrp.C b/src/usr/targeting/attrrp.C index 66be9a33f..2c9f0b6fe 100755 --- a/src/usr/targeting/attrrp.C +++ b/src/usr/targeting/attrrp.C @@ -52,6 +52,7 @@ #include <fapi2/plat_attr_override_sync.H> #include <targeting/attrPlatOverride.H> #include <config.h> +#include <secureboot/service.H> using namespace INITSERVICE; using namespace ERRORLOG; @@ -929,6 +930,12 @@ namespace TARGETING size_t l_maxSize = io_size; io_size = 0; + if (!SECUREBOOT::allowAttrOverrides()) + { + TRACFCOMP( g_trac_targeting, "AttrRP::_saveOverrides: skipping " + "since Attribute Overrides are not allowed."); + } + // Save the fapi and temp overrides // Note: no need to look at PERM because those were added to // the base targeting model diff --git a/src/usr/targeting/attrsync.C b/src/usr/targeting/attrsync.C index 6faca132a..4201776be 100644 --- a/src/usr/targeting/attrsync.C +++ b/src/usr/targeting/attrsync.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2012,2016 */ +/* Contributors Listed Below - COPYRIGHT 2012,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -27,7 +27,7 @@ #include <targeting/common/trace.H> #include <initservice/initserviceif.H> #include <errl/hberrltypes.H> - +#include <secureboot/service.H> using namespace ERRORLOG; @@ -67,6 +67,14 @@ namespace TARGETING "section type %u total pages %d", iv_section_to_sync, iv_total_pages ); + if (!SECUREBOOT::allowAttrOverrides()) + { + TARG_INF("AttributeSync::updateSectionData(): skipping since " + "attribute overrides are not allowed and we don't " + "trust the FSP, but still returning ATTR_SYNC_SUCCESS"); + return ATTR_SYNC_SUCCESS; + } + ATTR_SYNC_RC l_rc = ATTR_SYNC_SUCCESS; // call the targeting function here to get context. @@ -167,6 +175,13 @@ namespace TARGETING memset( &l_page, 0, sizeof(TARGETING::sectionRefData) ); do{ + if (!SECUREBOOT::allowAttrOverrides()) + { + TARG_INF("AttributeSync::syncSectionFromFsp(): skipping since " + "attribute overrides are not allowed and we don't " + "trust the FSP"); + break; + } // send a request to FSP to sync to Hostboot l_errl = sendSyncToHBRequestMessage(); @@ -554,6 +569,14 @@ namespace TARGETING break; } + if (!SECUREBOOT::allowAttrOverrides()) + { + TARG_INF("syncAllAttributesFromFsp(): skipping since " + "attribute overrides are not allowed and we don't " + "trust the FSP"); + break; + } + // create Hostboot message queue msg_q_t l_pHbMsgQ = msg_q_create(); diff --git a/src/usr/targeting/runtime/attrPlatOverride_rt.C b/src/usr/targeting/runtime/attrPlatOverride_rt.C index 6b9fc5a8f..fcd6aabc1 100644 --- a/src/usr/targeting/runtime/attrPlatOverride_rt.C +++ b/src/usr/targeting/runtime/attrPlatOverride_rt.C @@ -54,7 +54,7 @@ int apply_attr_override(uint8_t* i_data, bool l_allowOverrides = true; #ifdef CONFIG_SECUREBOOT - l_allowOverrides = !SECUREBOOT::enabled(); + l_allowOverrides = SECUREBOOT::allowAttrOverrides(); #endif if (l_allowOverrides) diff --git a/src/usr/targeting/test/testattrtank.H b/src/usr/targeting/test/testattrtank.H index 247076e58..e1a2b22af 100644 --- a/src/usr/targeting/test/testattrtank.H +++ b/src/usr/targeting/test/testattrtank.H @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2013,2014 */ +/* Contributors Listed Below - COPYRIGHT 2013,2017 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -36,6 +36,7 @@ #include <targeting/common/attributeTank.H> #include <targeting/attrPlatOverride.H> #include <pnor/pnorif.H> +#include <kernel/bltohbdatamgr.H> using namespace TARGETING; @@ -1171,6 +1172,13 @@ public: l_attrPermSec.name = "Test Attr Perm"; l_attrPermSec.vaddr = reinterpret_cast<uint64_t>(malloc(3*l_chunkSize)); + + // This test needs Attribute Overrides to work, so save global variable + // before setting it to 'true' for the duration of this test + bool save_allowAttrOverrides = g_BlToHbDataManager.iv_data.allowAttrOverrides; + g_BlToHbDataManager.iv_data.allowAttrOverrides = true; + TS_TRACE("testBMCAttrOverride: saved Allow Attr Override (%d) and set to true", save_allowAttrOverrides); + do { // Create local AttributeTanks @@ -1448,6 +1456,10 @@ public: } while(0); + // Restore Allow Attr Override Setting + g_BlToHbDataManager.iv_data.allowAttrOverrides = save_allowAttrOverrides; + TS_TRACE("testBMCAttrOverride: restored Allow Attr Override (%d)", save_allowAttrOverrides); + // Free memory free (reinterpret_cast<char *>(l_attrTmpSec.vaddr)); free (reinterpret_cast<char *>(l_attrPermSec.vaddr)); |