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>

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));