Mitigations against event type manipulation in UEFI eventlog

.packit.yaml

@@ -3,6 +3,6 @@ jobs:
   trigger: pull_request
   metadata:
     targets:
-    - fedora-all
+    - fedora-stable
     - centos-stream-9-x86_64
     skip_build: true

keylime/elchecking/example.py

@@ -100,7 +100,8 @@ def refstate_to_test(self, refstate: policies.RefState) -> tests.Test:
                 raise Exception(f'refstate lacks {req}')
 
         dispatcher = tests.Dispatcher(('PCRIndex', 'EventType'))
-        vd = tests.VariableDispatch()
+        vd_driver_config = tests.VariableDispatch()
+        vd_authority = tests.VariableDispatch()
 
         def bsa_test(kernel: dict) -> tests.Test:
             tt = [tests.DigestTest({"sha256": string_strip0x(kernel['shim_authcode_sha256'])}),
@@ -125,36 +126,50 @@ def bsa_test(kernel: dict) -> tests.Test:
                 ) for kernel in kernels]),
                 scrtm_and_bios_test),
             'kernel_cmdlines', 'bsas', 'ipl9s', 's_crtms', 'platform_firmware_blobs')
-        dispatcher.set((0, 'EV_NO_ACTION'), tests.AcceptAll())
+        # We only expect one EV_NO_ACTION event at the start.
+        dispatcher.set((0, 'EV_NO_ACTION'), tests.OnceTest(tests.AcceptAll()))
         dispatcher.set((0, 'EV_S_CRTM_VERSION'), events_final.get('s_crtms'))
         dispatcher.set((0, 'EV_EFI_PLATFORM_FIRMWARE_BLOB'),
                        events_final.get('platform_firmware_blobs'))
-        dispatcher.set((7, 'EV_EFI_VARIABLE_DRIVER_CONFIG'), vd)
-        vd.set('61dfe48b-ca93-d211-aa0d-00e098032b8c', 'SecureBoot',
+        dispatcher.set((7, 'EV_EFI_VARIABLE_DRIVER_CONFIG'), vd_driver_config)
+        vd_driver_config.set('61dfe48b-ca93-d211-aa0d-00e098032b8c', 'SecureBoot',
                tests.FieldTest('Enabled', tests.StringEqual('Yes')))
-        vd.set('61dfe48b-ca93-d211-aa0d-00e098032b8c', 'PK',
+        vd_driver_config.set('61dfe48b-ca93-d211-aa0d-00e098032b8c', 'PK',
                 tests.KeySubset('a159c0a5-e494-a74a-87b5-ab155c2bf072', sigs_strip0x(refstate['pk'])))
-        vd.set('61dfe48b-ca93-d211-aa0d-00e098032b8c', 'KEK',
+        vd_driver_config.set('61dfe48b-ca93-d211-aa0d-00e098032b8c', 'KEK',
                 tests.KeySubset('a159c0a5-e494-a74a-87b5-ab155c2bf072', sigs_strip0x(refstate['kek'])))
-        vd.set('cbb219d7-3a3d-9645-a3bc-dad00e67656f', 'db',
+        vd_driver_config.set('cbb219d7-3a3d-9645-a3bc-dad00e67656f', 'db',
                 tests.KeySubset('a159c0a5-e494-a74a-87b5-ab155c2bf072', sigs_strip0x(refstate['db'])))
-        vd.set('cbb219d7-3a3d-9645-a3bc-dad00e67656f', 'dbx',
+        vd_driver_config.set('cbb219d7-3a3d-9645-a3bc-dad00e67656f', 'dbx',
                 tests.KeySuperset('2616c4c1-4c50-9240-aca9-41f936934328', sigs_strip0x(refstate['dbx'])))
+        dispatcher.set((7, 'EV_EFI_VARIABLE_AUTHORITY'), vd_authority)
+        # Assume that the cert that was used to verify the Shim is always trusted.
+        # TODO: can we use the db entry for that instead of AcceptAll?
+        vd_authority.set('cbb219d7-3a3d-9645-a3bc-dad00e67656f', 'db',
+                         tests.OnceTest(tests.AcceptAll()))
+        # Accept all SbatLevels of the Shim, because we already checked the hash of the Shim itself.
+        vd_authority.set('50ab5d60-46e0-0043-abb6-3dd810dd8b23', 'SbatLevel',
+                         tests.OnceTest(tests.AcceptAll()))
+        # Accept all certificates that are used by the Shim to verify the next component,
+        # because we already checked the hash of the Shim itself.
+        vd_authority.set('50ab5d60-46e0-0043-abb6-3dd810dd8b23', 'Shim',
+                         tests.OnceTest(tests.AcceptAll()))
+
 
         # A list of allowed digests for firmware from device driver appears
         # in PCR2, event type EV_EFI_BOOT_SERVICES_DRIVER. Here we will just
-        # accept everything
+        # accept everything.
+        # This is fine because we do not use any other entry type from PCR 2 for validation.
         dispatcher.set((2, 'EV_EFI_BOOT_SERVICES_DRIVER'),
                                         tests.AcceptAll())
         dispatcher.set((1, 'EV_EFI_VARIABLE_BOOT'), tests.VariableTest(
             '61dfe48b-ca93-d211-aa0d-00e098032b8c',
             re.compile('BootOrder|Boot[0-9a-fA-F]+'),
             tests.AcceptAll()))
-        dispatcher.set((4, 'EV_EFI_ACTION'), tests.AcceptAll())
+        dispatcher.set((4, 'EV_EFI_ACTION'), tests.EvEfiActionTest(4))
         for pcr in range(8):
-            dispatcher.set((pcr, 'EV_SEPARATOR'), tests.AcceptAll())
-        dispatcher.set((7, 'EV_EFI_VARIABLE_AUTHORITY'), tests.AcceptAll())
-        dispatcher.set((5, 'EV_EFI_GPT_EVENT'), tests.AcceptAll())
+            dispatcher.set((pcr, 'EV_SEPARATOR'), tests.EvSeperatorTest())
+
         dispatcher.set((4, 'EV_EFI_BOOT_SERVICES_APPLICATION'),
                        events_final.get('bsas'))
         dispatcher.set((14, 'EV_IPL'), tests.Or(
@@ -181,7 +196,9 @@ def bsa_test(kernel: dict) -> tests.Test:
                 tests.RegExp('kernel_cmdline: .*'),
                 events_final.get('kernel_cmdlines'))
         ))))
-        dispatcher.set((5, 'EV_EFI_ACTION'), tests.AcceptAll())
+        dispatcher.set((5, 'EV_EFI_ACTION'), tests.EvEfiActionTest(5))
+        # Accept all UEFI_GPT_DATA. We only expect one entry for that.
+        dispatcher.set((5, 'EV_EFI_GPT_EVENT'), tests.OnceTest(tests.AcceptAll()))
         events_test = tests.FieldTest('events',
                                       tests.And(
                                           events_final.get_initializer(),

keylime/elchecking/tests.py

@@ -1,7 +1,10 @@
 import abc
+import codecs
 import re
 import typing
 
+from keylime.common.algorithms import Hash
+
 # This module defines the abstraction of a Test (of JSON data)
 # and several specific test classes.
 # A Test can be used multiple times, even concurrently.
@@ -593,3 +596,68 @@ def __init__(self, sig_type: str, keys: typing.Iterable[Signature]):
             FieldTest('Keys',
                       SupersetOfDicts(keys, ('SignatureOwner', 'SignatureData')))
         ))
+
+
+class OnceTest(Test):
+    """Tests that only works once"""
+
+    def __init__(self, test: Test):
+        self.executed = False
+        self.test = test
+
+    def why_not(self, globs: Globals, subject: Data) -> str:
+        if self.executed:
+            return 'test was already run once'
+
+        self.executed = True
+        return self.test.why_not(globs, subject)
+
+
+# Following tests are TCG PC Client Platform specific, but are common and reduce the use of AcceptAll
+
+class EvSeperatorTest(Or):
+    """Test for valid EV_SEPARATOR entry values"""
+    def __init__(self):
+        # See TCG PC Client Platform Firmware Profile (Table 9 Events)
+        valid_hex_values = ["00000000", "FFFFFFFF"]
+        tests = []
+        for value in valid_hex_values:
+            val_bytes = codecs.decode(value.encode(), 'hex')
+            event_test = FieldTest("Event", StringEqual(value))
+            digests = {}
+            for hash_alg in Hash:
+                digests[str(hash_alg)] = codecs.encode(hash_alg.hash(val_bytes), 'hex').decode("utf-8")
+            tests.append(And(event_test, DigestTest(digests)))
+        super().__init__(*tests)
+
+
+class EvEfiActionTest(Test):
+    """Test for valid EV_EFI_ACTION entry values"""
+
+    _expected_strings = {
+        4: ["Calling EFI Application from Boot Option", "Returning from EFI Application from Boot Option"],
+        5: ["Exit Boot Services Invocation", "Exit Boot Services Returned with Failure",
+            "Exit Boot Services Returned with Success"],
+        6: ["UEFI Debug Mode"]
+    }
+
+    def __init__(self, pcr: int):
+        self.pcr = pcr
+        if pcr not in self._expected_strings:
+            self.test = None
+            return
+
+        tests = []
+        for value in self._expected_strings[pcr]:
+            event_test = FieldTest("Event", StringEqual(value))
+            digests = {}
+            for hash_alg in Hash:
+                digests[str(hash_alg)] = codecs.encode(hash_alg.hash(value.encode()), 'hex').decode("utf-8")
+            tests.append(And(event_test, DigestTest(digests)))
+        self.test = Or(*tests)
+
+    def why_not(self, globs: Globals, subject: Data) -> str:
+        if self.test is None:
+            return f"No EV_EFI_ACTION event for {self.pcr} is expected in the spec"
+
+        return self.test.why_not(globs, subject)