Secure Boot: Fix lid load from HB reserved memory issues at runtime

- Force all PNOR sections we load from HB rserved memory to be secure
  Only exception is the RINGOVD section, in which we use a fake header
- Add fake header when Secureboot compiled out or a section is never
  signed as there is no secure header preserved in virtual memory

RTC: 171708
RTC: 180063
Change-Id: Ibbbd7be24ee7b199e73451c63b2c2d1f86a2c2d8
Reviewed-on: http://ralgit01.raleigh.ibm.com/gerrit1/49020
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Reviewed-by: Marshall J. Wilks <mjwilks@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>

3 files changed, 140 insertions, 44 deletions

diff --git a/src/usr/secureboot/base/test/securerommgrtest.H b/src/usr/secureboot/base/test/securerommgrtest.H
index 4a445b52f..1355cc75d 100644
--- a/src/usr/secureboot/base/test/securerommgrtest.H
+++ b/src/usr/secureboot/base/test/securerommgrtest.H
@@ -45,6 +45,7 @@
 #include "../../../vfs/vfsrp.H"
 #include <sys/vfs.h>
 #include <kernel/console.H>
+#include <pnor/pnorif.H>
 
 // Quick change for unit testing
 //#define TRACUCOMP(args...)  TRACFCOMP(args)
@@ -525,6 +526,40 @@ class SecureRomManagerTest : public CxxTest::TestSuite
             unloadSignedFile( signedFile_pageAddr, signedFile_size);
         }
     }
+
+    void test_fakeHeader(void)
+    {
+        TRACFCOMP(g_trac_secure,"SecureRomManagerTest::test_fakeHeader");
+
+        const size_t l_totalContainerSize = 0x10000;
+        // Purposely make a comp id larger than SW_HDR_COMP_ID_SIZE_BYTES
+        // otherwise strncmp below needs a different size
+        const char* l_compId = "FAKEHEADERTEST";
+
+        // 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()))
+        {
+            TS_FAIL("SecureRomManagerTest::test_fakeHeader: missing magic number");
+        }
+
+        // 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");
+        }
+
+        // Ensure the parsed component ID matches what was passed in through
+        // SW_HDR_COMP_ID_SIZE_BYTES
+        if(strncmp(l_fakeHdr.componentId(), l_compId,
+                   SW_HDR_COMP_ID_SIZE_BYTES) != 0)
+        {
+            TS_FAIL("SecureRomManagerTest::test_fakeHeader: component ID was not parsed correctly");
+        }
+    }
 };
 
 /**********************************************************************/
diff --git a/src/usr/secureboot/common/containerheader.C b/src/usr/secureboot/common/containerheader.C
index 069a587d9..dd43551d2 100644
--- a/src/usr/secureboot/common/containerheader.C
+++ b/src/usr/secureboot/common/containerheader.C
@@ -34,7 +34,7 @@ namespace SECUREBOOT
 
 void ContainerHeader::parse_header(const void* i_header)
 {
-    assert(i_header != NULL);
+    assert(i_header != nullptr);
     const uint8_t* l_hdr = reinterpret_cast<const uint8_t*>(i_header);
 
     /*---- Parse ROM_container_raw ----*/
@@ -94,6 +94,68 @@ void ContainerHeader::parse_header(const void* i_header)
     print();
 }
 
+void ContainerHeader::initVars()
+{
+    memset(&iv_headerInfo, 0x00, sizeof(iv_headerInfo));
+    memset(iv_hwKeyHash, 0, sizeof(SHA512_t));
+    memset(iv_componentId,0x00,sizeof(iv_componentId));
+}
+
+void ContainerHeader::genFakeHeader(const size_t i_totalSize,
+                                    const char* const i_compId)
+{
+    SecureHeaderInfo info {};
+    assert(iv_fakeHeader.data() != nullptr, "Internal fake header buffer nullptr");
+
+    uint8_t* l_hdr = reinterpret_cast<uint8_t*>(iv_fakeHeader.data());
+
+     /*---- ROM_container_raw ----*/
+    info.hw_hdr.magic_number = ROM_MAGIC_NUMBER;
+    info.hw_hdr.version = CONTAINER_VERSION;
+    info.hw_hdr.container_size = i_totalSize;
+    // The rom code has a placeholder for the prefix in the first struct so
+    // skip it
+    size_t l_size = offsetof(ROM_container_raw, prefix);
+    memcpy(l_hdr, &info.hw_hdr, l_size);
+    l_hdr += l_size;
+
+    /*---- ROM_prefix_header_raw ----*/
+    info.hw_prefix_hdr.ver_alg.version = HEADER_VERSION;
+    info.hw_prefix_hdr.ver_alg.hash_alg = HASH_ALG_SHA512;
+    info.hw_prefix_hdr.ver_alg.sig_alg = SIG_ALG_ECDSA521;
+    info.hw_prefix_hdr.sw_key_count = 1;
+    info.hw_prefix_hdr.payload_size = sizeof(ecc_key_t);
+
+    l_size = offsetof(ROM_prefix_header_raw, ecid);
+    l_size += info.hw_prefix_hdr.ecid_count * ECID_SIZE;
+    memcpy(l_hdr, &info.hw_prefix_hdr, l_size);
+    l_hdr += l_size;
+
+    /*---- Parse ROM_prefix_data_raw ----*/
+    // Skip over variable number of sw keys as they are already zeroed out
+    l_size = offsetof(ROM_prefix_data_raw, sw_pkey_p);
+    l_size += info.hw_prefix_hdr.sw_key_count * sizeof(ecc_key_t);
+    l_hdr += l_size;
+
+    /*---- ROM_sw_header_raw ----*/
+    info.sw_hdr.ver_alg.version = 1;
+    strncpy(info.sw_hdr.component_id, i_compId,SW_HDR_COMP_ID_SIZE_BYTES);
+    info.sw_hdr.ver_alg.hash_alg = HASH_ALG_SHA512;
+    info.sw_hdr.ver_alg.sig_alg = SIG_ALG_ECDSA521;
+    info.sw_hdr.payload_size = i_totalSize - PAGE_SIZE;
+
+    l_size = offsetof(ROM_sw_header_raw, ecid);
+    l_size += info.hw_prefix_hdr.ecid_count * ECID_SIZE;
+    memcpy(l_hdr, &info.sw_hdr, l_size);
+    l_hdr += l_size;
+
+    /*---- Parse ROM_sw_sig_raw ----*/
+    // No-op already zeroed out
+
+    iv_pHdrStart = reinterpret_cast<const uint8_t*>(iv_fakeHeader.data());
+    parse_header(iv_fakeHeader.data());
+}
+
 void ContainerHeader::print() const
 {
 #ifdef HOSTBOOT_DEBUG
@@ -218,9 +280,9 @@ void ContainerHeader::validate()
 void ContainerHeader::safeMemCpyAndInc(void* i_dest, const uint8_t* &io_hdr,
                                        const size_t i_size)
 {
-    assert(i_dest != NULL, "ContainerHeader: dest ptr NULL");
-    assert(io_hdr != NULL, "ContainerHeader: current header location ptr NULL");
-    assert(iv_pHdrStart != NULL, "ContainerHeader: start of header ptr NULL");
+    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");
 
     TRACDCOMP(g_trac_secure,"dest: 0x%X src: 0x%X size: 0x%X",i_dest, io_hdr, i_size);
 
@@ -265,4 +327,10 @@ void ContainerHeader::genHwKeyHash()
 }
 #endif
 
+const uint8_t* ContainerHeader::fakeHeader() const
+{
+    assert(iv_fakeHeader.data() != nullptr, "Fake header should not be nullptr");
+    return iv_fakeHeader.data();
+}
+
 }; //end of SECUREBOOT namespace
diff --git a/src/usr/secureboot/runtime/test/testsecureboot_rt.H b/src/usr/secureboot/runtime/test/testsecureboot_rt.H
index 33ca4cd48..f728357e2 100644
--- a/src/usr/secureboot/runtime/test/testsecureboot_rt.H
+++ b/src/usr/secureboot/runtime/test/testsecureboot_rt.H
@@ -44,6 +44,7 @@
 #include <secureboot/settings.H>
 #include <pnor/pnorif.H>
 #include <pnor/pnor_reasoncodes.H>
+#include "../../../pnor/pnor_utils.H"
 
 class SecurebootRtTestSuite: public CxxTest::TestSuite
 {
@@ -163,70 +164,62 @@ class SecurebootRtTestSuite: public CxxTest::TestSuite
     /**
      * @brief Helper to test case that runs getSectionInfo scenarios and checks
      *        for desired results.
+     *        If secure, should throw an error
+     *        Otherwise no error
      * @param[in] i_id, Pnor Section ID
-     * @param[in] i_secure, Indicates if section is expected to be secure or not
      *
      * @return N/A
      */
-    void runAccessSecurePnorTest(PNOR::SectionId i_id, bool i_secure)
+    void runAccessSecurePnorTest(PNOR::SectionId i_id)
     {
         errlHndl_t l_errl = nullptr;
+        bool l_secure = PNOR::isEnforcedSecureSection(i_id);
         PNOR::SectionInfo_t l_info;
 
-        l_errl = PNOR::getSectionInfo(i_id, l_info);
-        if(l_errl)
-        {
-            TS_FAIL("testAccessSecurePnorSection: Failed for section %s",
-                        PNOR::SectionIdToString(i_id));
-            errlCommit(l_errl, SECURE_COMP_ID);
-        }
+        SB_ENTER("runAccessSecurePnorTest %s", PNOR::SectionIdToString(i_id));
 
-        // TODO: RTC:180063 change this test case back to how it was before
-        //                  having secure sections return vaddr = 0
-        //                  previously in HB commit cefc4c
-        // If we expect the section to be secure, make sure it returns secure
-        // and a vaddr of 0
-        if (i_secure)
+        l_errl = PNOR::getSectionInfo(i_id, l_info);
+        if(l_secure)
         {
-            if (l_info.secure != 1)
+            SB_INF("runAccessSecurePnorTest is secure");
+            if (l_errl &&
+                (l_errl->reasonCode() == PNOR::RC_RTPNOR_INVALID_SECTION))
             {
-                TS_FAIL("testAccessSecurePnorSection: Did not return %s as a secure section",
-                            PNOR::SectionIdToString(i_id));
+                SB_INF("runAccessSecurePnorTest caught correct error");
+                delete l_errl;
+                l_errl = nullptr;
             }
-            else if (l_info.vaddr != 0)
+            else
             {
-                TS_FAIL("testAccessSecurePnorSection: Did not return a vaddr of 0 for secure section %s",
-                            PNOR::SectionIdToString(i_id));
+                TS_FAIL("testAccessSecurePnorSection: unexpected reason code for Secure Section %s. Expected RC 0x%.4X Actual RC 0x%.4X",
+                        PNOR::SectionIdToString(i_id),
+                        PNOR::RC_RTPNOR_INVALID_SECTION,
+                        l_errl->reasonCode());
+                errlCommit(l_errl, SECURE_COMP_ID);
             }
         }
-        // If we expect the section to be secure, make sure it returns secure
-        // and a vaddr of 0
-        else
+        else if(l_errl)
         {
-            if (l_info.vaddr == 0)
-            {
-                TS_FAIL("testAccessSecurePnorSection: Did not return a vaddr of non-zero for a non-secure section %s",
-                            PNOR::SectionIdToString(i_id));
-            }
+            TS_FAIL("testAccessSecurePnorSection: getSectionInfo failed for section %s",
+                        PNOR::SectionIdToString(i_id));
+            errlCommit(l_errl, SECURE_COMP_ID);
         }
+
+        SB_EXIT("runAccessSecurePnorTest");
     }
 
-    // TODO: RTC:180063 change this test case back to how it was before
-    //                  having secure sections return vaddr = 0 previously
-    //                  in HB commit cefc4c
     void testAccessSecurePnorSection()
     {
+#ifdef CONFIG_SECUREBOOT
         SB_ENTER("testAccessSecurePnorSection");
 
+        // Should thow an error for trying to read a secure section
+        runAccessSecurePnorTest(PNOR::OCC);
+        runAccessSecurePnorTest(PNOR::HB_EXT_CODE);
 
-        // Ensure we get a vaddr of 0 at Runtime
-        runAccessSecurePnorTest(PNOR::OCC, true);
-        runAccessSecurePnorTest(PNOR::HB_EXT_CODE, true);
-
-        // Ensure we get a vaddr of  at Runtime
-        runAccessSecurePnorTest(PNOR::TEST, false);
-
-        SB_EXIT("testAccessSecurePnorSection");
+        // No error for trying to read a secure section
+        runAccessSecurePnorTest(PNOR::TEST);
+#endif
     }