Fix for keylime#849

A new parameter, `ima_hash_alg` is introduced under the `[tenant]`
section in `keylime.conf`. This parameter can be overriden by the CLI
option `--allowlist_hash_alg` on `keylime_tenant`. All this is, of
course, only applicable to "version 1" IMA allowlists.

In addition to that we fix the WARNING message on the agent. If the IMA
hash algorithm is other than SHA1, and the kernel is older 5.10,
communicate that there is potential problem.

Signed-off-by: Marcio Silva <marcio.a.silva@ibm.com>

keylime.conf

@@ -395,6 +395,13 @@ ima_allowlist = allowlist.txt
 # WARNING: use with great caution as it will affect the security of IMA.
 ima_excludelist = exclude.txt
 
+# If a plain (non-JSON) allowlist is being supplied (known as "version 1" in keylime),
+# the tenant will have to specify which hash algorithm should be used for IMA, 
+# since this information will not be extracted directly from the file. In case 
+# a JSON-formmatted allowlist file is supplied (with the hash algorithm already
+# specified there) this parameter will be simply ignored.
+ima_hash_alg = sha1
+
 # Specify the acceptable crypto algorithms to use with the TPM for this agent.
 # Only algorithms specified below will be allowed for usage by an agent.  If an
 # agent uses an algorithm not specified here, it will fail validation.

keylime/cmd/ima_emulator_adapter.py

@@ -33,7 +33,7 @@ def measure_list(file_path, position, ima_hash_alg, pcr_hash_alg, search_val=Non
         line = line.strip()
         position += 1
 
-        entry = ima_ast.Entry(line, None, ima_hash_alg=ima_hash_alg, pcr_hash_alg=pcr_hash_alg)
+        entry = ima_ast.Entry(line, None, ima_filedata_hash_alg=ima_hash_alg, pcr_hash_alg=pcr_hash_alg)
 
         if search_val is None:
             val = codecs.encode(entry.pcr_template_hash, 'hex').decode("utf8")

keylime/ima.py

@@ -269,7 +269,7 @@ def _process_measurement_list(agentAttestState, lines, hash_alg, lists=None, m2w
             continue
 
         try:
-            entry = ima_ast.Entry(line, ima_validator, ima_hash_alg=ima_log_hash_alg, pcr_hash_alg=hash_alg)
+            entry = ima_ast.Entry(line, ima_validator, ima_filedata_hash_alg=ima_log_hash_alg, pcr_hash_alg=hash_alg)
 
             # update hash
             running_hash = hash_alg.hash(running_hash + entry.pcr_template_hash)
@@ -328,7 +328,7 @@ def process_measurement_list(agentAttestState, lines, lists=None, m2w=None, pcrv
 
     return running_hash, failure
 
-def update_allowlist(allowlist):
+def update_allowlist(allowlist, ima_hash_alg="sha1"):
     """ Update the allowlist to the latest version adding default values for missing fields """
     allowlist["meta"]["version"] = ALLOWLIST_CURRENT_VERSION
 
@@ -345,8 +345,8 @@ def update_allowlist(allowlist):
         allowlist["ima-buf"] = {}
     # version 5 added 'log_hash_alg'
     if not "log_hash_alg" in allowlist["ima"]:
-        allowlist["ima"]["log_hash_alg"] = "sha1"
-
+        logger.debug("Hash algorithm for IMA was not specified in allowlist, and is assumed to be \"" + ima_hash_alg + "\"")
+        allowlist["ima"]["log_hash_alg"] = ima_hash_alg
     return allowlist
 
 def process_allowlists(allowlist, exclude):
@@ -382,11 +382,10 @@ def process_allowlists(allowlist, exclude):
     "keyrings": {},
     "ima": {
         "ignored_keyrings": [],
-        "log_hash_alg": "sha1"
     }
 }
 
-def read_allowlist(al_path=None, checksum="", gpg_sig_file=None, gpg_key_file=None):
+def read_allowlist(al_path=None, checksum="", gpg_sig_file=None, gpg_key_file=None, ima_hash_alg="sha1"):
     if al_path is None:
         al_path = config.get('tenant', 'ima_allowlist')
         if config.STUB_IMA:
@@ -471,7 +470,7 @@ def read_allowlist(al_path=None, checksum="", gpg_sig_file=None, gpg_key_file=No
             else:
                 alist[entrytype][path] = [checksum_hash]
 
-    alist = update_allowlist(alist)
+    alist = update_allowlist(alist, ima_hash_alg)
 
     return alist
 

keylime/ima_ast.py

@@ -315,7 +315,8 @@ class Entry:
     mode: Mode
     _bytes: bytes
     _validator: Validator
-    _ima_hash_alg: Hash
+    _ima_template_hash_alg: Hash
+    _ima_filedata_hash_alg: Hash
     _pcr_hash_alg: Hash
 
     _mode_lookup = {
@@ -325,9 +326,11 @@ class Entry:
         "ima-buf": ImaBuf
     }
 
-    def __init__(self, data: str, validator=None, ima_hash_alg: Hash = Hash.SHA1, pcr_hash_alg: Hash = Hash.SHA1):
+    def __init__(self, data: str, validator=None, ima_filedata_hash_alg: Hash = Hash.SHA1, pcr_hash_alg: Hash = Hash.SHA1):
         self._validator = validator
-        self._ima_hash_alg = ima_hash_alg
+        self._ima_filedata_hash_alg = ima_filedata_hash_alg
+        # Template hash will be, for the foreseeable future, always sha1, even for ima-ng and ima-sg
+        self._ima_template_hash_alg = Hash.SHA1
         self._pcr_hash_alg = pcr_hash_alg
         tokens = data.split(maxsplit=3)
         if len(tokens) != 4:
@@ -347,8 +350,8 @@ def __init__(self, data: str, validator=None, ima_hash_alg: Hash = Hash.SHA1, pc
         # Set correct hash for time of measure, time of use (ToMToU) errors
         # and if a file is already opened for write.
         # https://elixir.bootlin.com/linux/v5.12.12/source/security/integrity/ima/ima_main.c#L101
-        if self.ima_template_hash == get_START_HASH(ima_hash_alg):
-            self.ima_template_hash = get_FF_HASH(ima_hash_alg)
+        if self.ima_template_hash == get_START_HASH(self._ima_template_hash_alg):
+            self.ima_template_hash = get_FF_HASH(self._ima_template_hash_alg)
             self.pcr_template_hash = get_FF_HASH(pcr_hash_alg)
 
     def invalid(self):
@@ -359,16 +362,16 @@ def invalid(self):
                                           "expected": str(config.IMA_PCR), "got": self.pcr}, True)
 
         # Ignore template hash for ToMToU errors
-        if self.ima_template_hash == get_FF_HASH(self._ima_hash_alg):
+        if self.ima_template_hash == get_FF_HASH(self._ima_template_hash_alg):
             logger.warning("Skipped template_hash validation entry with FF_HASH")
             # By default ToMToU errors are not treated as a failure
             if config.getboolean("cloud_verifier", "tomtou_errors", fallback=False):
                 failure.add_event("tomtou", "hash validation was skipped", True)
             return failure
-        if self.ima_template_hash != self._ima_hash_alg.hash(self._bytes):
+        if self.ima_template_hash != self._ima_template_hash_alg.hash(self._bytes):
             failure.add_event("ima_hash",
-                              {"message": "IMA hash does not match the calculated hash.",
-                               "expected": self.ima_template_hash, "got": self.mode.bytes()}, True)
+                              {"message": "IMA template hash does not match the calculated hash.",
+                               "expected": str(self.ima_template_hash), "got": str(self.mode.bytes())}, True)
             return failure
         if self._validator is None:
             failure.add_event("no_validator", "No validator specified", True)

keylime/keylime_agent.py

@@ -609,9 +609,10 @@ def main():
     # Warn if kernel version is <5.10 and another algorithm than SHA1 is used,
     # because otherwise IMA will not work
     kernel_version = tuple(platform.release().split("-")[0].split("."))
-    if kernel_version < ("5", "10", "0") and instance_tpm.defaults["hash"] != algorithms.Hash.SHA1:
+    if tuple(map(int,kernel_version)) < (5, 10, 0) and instance_tpm.defaults["hash"] != algorithms.Hash.SHA1:
         logger.warning("IMA attestation only works on kernel versions <5.10 with SHA1 as tpm_hash_alg. "
-                       "Current algorithm is: %s", instance_tpm.defaults["hash"])
+                       "Even if ascii_runtime_measurements shows \"%s\" as the "
+                       "algorithm, it might be just padding zeros", (instance_tpm.defaults["hash"]))
 
     if ekcert is None:
         if virtual_agent:

keylime/tenant.py

@@ -159,6 +159,11 @@ def process_allowlist(self, args):
         if "vtpm_policy" in args and args["vtpm_policy"] is not None:
             vtpm_policy = args["vtpm_policy"]
         self.vtpm_policy = TPM_Utilities.readPolicy(vtpm_policy)
+
+        ima_hash_alg = config.get('tenant', 'ima_hash_alg')
+        if "ima_hash_alg" in args and args["ima_hash_alg"] is not None:
+            ima_hash_alg = args["ima_hash_alg"]
+
         logger.info("TPM PCR Mask from policy is %s", self.vtpm_policy['mask'])
 
         if len(args.get("ima_sign_verification_keys")) > 0:
@@ -190,7 +195,7 @@ def process_allowlist(self, args):
             if isinstance(args["allowlist"], str):
                 if args["allowlist"] == "default":
                     args["allowlist"] = config.get('tenant', 'allowlist')
-                al_data = ima.read_allowlist(args["allowlist"], args["allowlist_checksum"], args["allowlist_sig"], args["allowlist_sig_key"])
+                al_data = ima.read_allowlist(args["allowlist"], args["allowlist_checksum"], args["allowlist_sig"], args["allowlist_sig_key"], ima_hash_alg)
             elif isinstance(args["allowlist"], list):
                 al_data = args["allowlist"]
             else:
@@ -1194,6 +1199,8 @@ def main(argv=sys.argv):
                         help="Include additional files in provided directory in certificate zip file.  Must be specified with --cert")
     parser.add_argument('--allowlist', action='store', dest='allowlist',
                         default=None, help="Specify the file path of an allowlist")
+    parser.add_argument('--allowlist_hash_alg', action='store', dest='ima_hash_alg',
+                        default="sha1", help="Specify the IMA hash algorithm used on the creation of pain (\"vesion 1\" allowlists")
     parser.add_argument('--signature-verification-key', '--sign_verification_key', action='append', dest='ima_sign_verification_keys',
                         default=[], help="Specify an IMA file signature verification key")
     parser.add_argument('--signature-verification-key-sig', action='append', dest='ima_sign_verification_key_sigs',

test/test_ima_ast.py

@@ -52,7 +52,7 @@ def test_valid_entry_construction(self):
         for name, (expected_mode, data) in VALID_ENTRIES.items():
             err = None
             try:
-                entry = ima_ast.Entry(data, AlwaysTrueValidator, ima_hash_alg=hash_alg, pcr_hash_alg=hash_alg)
+                entry = ima_ast.Entry(data, AlwaysTrueValidator, ima_filedata_hash_alg=hash_alg, pcr_hash_alg=hash_alg)
                 self.assertTrue(entry.pcr == "10", f"Expected pcr 10 for {name}. Got: {entry.pcr}")
                 self.assertTrue(isinstance(entry.mode, expected_mode)) # pylint: disable=isinstance-second-argument-not-valid-type
                 self.assertTrue(entry.ima_template_hash == hash_alg.hash(entry.mode.bytes()),