Kernel fix IMA hash entries

[williamcroberts]

Fix this below (see this thread https://lists.01.org/hyperkitty/list/tpm2@lists.01.org/thread/FUBD3MY5U5YICNUYSF3NE2STO3YAW7Y4/):

As today, IMA is harcoded to make the boot_aggregate entry in SHA1

https://github.com/torvalds/linux/blob/ac438771ccb4479528594c7e19f2c39cf1814a86/security/integrity/ima/ima_init.c#L59

So the ima_hash=sha256 option is activated after the boot_aggregate. It is the same for me in Fedora 31. It would be nice if somebody contributed to the kernel and fixes this, or at least harcode it to sha256 :)

What I can see from you initial message is that you get all the digest from the measured boot process (PCR 0 to 7) in both SHA1 and SHA256 PCRs, which means that your BIOS to TPM interaction is working fine. In Fedora, you would see additional measurements in the PCR 8 and 9 corresponding to the digests of the components that grub2 reads (config, kernel and kernel config, and initiramfs).

But when IMA is measuring stuff, you get only PCR SHA1 digests. I think this is related to the 4.4 kernel version. The oldest kernel I used to validate IMA was a 4.17, and I am currently using 5.6. I believe there is no option to control which PCR banks IMA uses, it should measure in all the available PCR 10s by default. Is upgrading to Ubuntu 18.04 or 20.04 possible for you? Also, Ubuntu 16.04 is EOL since April 2019, so you have other good reasons to upgrade :) _______________________________________________
tpm2 mailing list -- tpm2@lists.01.org

[tstruk]

I think the reason it is hardcoded to SHA1 is that is still needs to support TPM1.2. For this reason it can not be changed to SHA256.

[williamcroberts]

I would wager it knows what tpm it's talking to, considering that the extend functions all have different and extra parameters. So the fact that it's not doing it, is likely an oversight.

[tstruk]

I don't think it does, or even cares, it just calls the same tpm api - tpm_pcr_extend().
See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/integrity/ima/ima_queue.c#n137

[williamcroberts]

Yes currently it doesn't care, and a lot of the TPM API is abstracted between versions, but you can query it and if it's 2.0 set the hash to sha256 (ie the code knows what it's talking to, IMA just doesn't do anything even though that knowledge is available).

You can see how to do this in the sysfs driver for the caps file:

	/* TPM 1.2 */
	if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
			 "attempting to determine the 1.2 version",
			 sizeof(cap.version2))) {
		version = &cap.version2.version;
		goto out_print;
	}

	/* TPM 1.1 */
	if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
			"attempting to determine the 1.1 version",
			sizeof(cap.version1))) {
		goto out_ops;
	}

	version = &cap.version1;

out_print:
	str += sprintf(str,
		       "TCG version: %d.%d\nFirmware version: %d.%d\n",
		       version->major, version->minor,
		       version->rev_major, version->rev_minor);

I don't know why they made getting the version so hard, I would create a new tpm helper that is, get me the version that both sysfs caps can call and that IMA can call and get the version of the TPM chip it's using. Then it can just pick the algorithm off of that. We also might want to consider populating in tpm_chip structure with that information cached on tpm_chip initialization.

[williamcroberts]

Actually they have an API and cache the is tpm2 in flags, much more sane:

int tpm_is_tpm2(struct tpm_chip *chip)
{
	int rc;

	chip = tpm_find_get_ops(chip);
	if (!chip)
		return -ENODEV;

	rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;

	tpm_put_ops(chip);

	return rc;
}
EXPORT_SYMBOL_GPL(tpm_is_tpm2);

So one can just call tpm_is_tpm2 and set the hash alg.

[williamcroberts]

Just offhand, this patch should hopefully do it (untested)

+#include <linux/printk.h>
 
 #include "ima.h"
 
@@ -59,6 +60,16 @@ static int __init ima_add_boot_aggregate(void)
        iint->ima_hash->length = SHA1_DIGEST_SIZE;
 
        if (ima_tpm_chip) {
+               result = tpm_is_tpm2(ima_tpm_chip);
+               if (result > 0) {
+                       /* yes it's a TPM2 chipset use sha256 */
+                       iint->ima_hash->algo = HASH_ALGO_SHA256;
+                       iint->ima_hash->length = SHA256_DIGEST_SIZE;
+               } else if (result < 0) {
+                       /* ignore errors here, as we can just move on with SHA1 */
+                       pr_warn("Could not query TPM chip version, got: %d\n", result);
+               }
+
                result = ima_calc_boot_aggregate(&hash.hdr);
                if (result < 0) {
                        audit_cause = "hashing_error";

[williamcroberts]

This was mentioned in the mailing list, the option below is post this call, so it doesn't matter.... However, it could also query the default option set at build time, not sure why its not doing that.

FYI There apears to be a command line option:

Select the default hash algorithm used for the measurement                                                                                                           │  
  │ list, integrity appraisal and audit log.  The compiled default                                                                                                       │  
  │ hash algorithm can be overwritten using the kernel command                                                                                                           │  
  │ line 'ima_hash=' option.                                                                                                                                             │  
  │                                                                                                                                                                      │  
  │                                                                                                                                                                      │  
  │ Defined at security/integrity/ima/Kconfig:90                                                                                                                         │  
  │   Prompt: Default integrity hash algorithm                                                                                                                           │  
  │   Depends on: INTEGRITY [=y] && IMA [=y]                                                                                                                             │  
  │   Location:                                                                                                                                                          │  
  │     -> Security options                                                                                                                                              │  
  │       -> Integrity subsystem (INTEGRITY [=y])                                                                                                                        │  
  │         -> Integrity Measurement Architecture(IMA) (IMA [=y])                                                                                                        │  
  │ Selected by [m]:                                                                                                                                                     │  
  │   - INTEGRITY [=y] && IMA [=y] && m        

[williamcroberts]

FYI I sent a mailer to the ima list to ask why and what we can do to fix it:
https://lore.kernel.org/linux-integrity/476DC76E7D1DF2438D32BFADF679FC5649EDCB91@ORSMSX101.amr.corp.intel.com/T/#u

[williamcroberts]

This is actually already being done on IMA mailing list as we speak, yeah!

https://lore.kernel.org/linux-integrity/20200325104712.25694-1-roberto.sassu@huawei.com/