Re: [PATCH v5] ima: add crypto agility support for template-hash algorithm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2024-04-08 at 13:17 +0200, Enrico Bravi wrote:
> The template hash showed by the ascii_runtime_measurements and
> binary_runtime_measurements is the one calculated using sha1 and there is
> no possibility to change this value, despite the fact that the template
> hash is calculated using the hash algorithms corresponding to all the PCR
> banks configured in the TPM.
> 
> Add the support to retrieve the ima log with the template data hash
> calculated with a specific hash algorithm.
> Add a new file in the securityfs ima directory for each hash algo
> configured in a PCR bank of the TPM. Each new file has the name with
> the following structure:
> 
>         {binary, ascii}_runtime_measurements_<hash_algo_name>
> 
> Legacy files are kept, to avoid breaking existing applications, but as
> symbolic links which point to {binary, ascii}_runtime_measurements_sha1
> files. These two files are created even if a TPM chip is not detected or
> the sha1 bank is not configured in the TPM.
> 
> As example, in the case a TPM chip is present and sha256 is the only
> configured PCR bank, the listing of the securityfs ima directory is the
> following:
> 
> lr--r--r-- [...] ascii_runtime_measurements -> ascii_runtime_measurements_sha1
> -r--r----- [...] ascii_runtime_measurements_sha1
> -r--r----- [...] ascii_runtime_measurements_sha256
> lr--r--r-- [...] binary_runtime_measurements -> binary_runtime_measurements_sha1
> -r--r----- [...] binary_runtime_measurements_sha1
> -r--r----- [...] binary_runtime_measurements_sha256
> --w------- [...] policy
> -r--r----- [...] runtime_measurements_count
> -r--r----- [...] violations
> 
> Signed-off-by: Enrico Bravi <enrico.bravi@xxxxxxxxx>
> Signed-off-by: Silvia Sisinni <silvia.sisinni@xxxxxxxxx>

Reviewed-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>

Thanks

Roberto

> ---
> 
> v5:
>  - Added lookup_algo_by_dentry() function to select the hash algo during
>    measurements dump. (suggested by Roberto)
>  - Renamed remove_measurements_list_files() to
>    remove_securityfs_measurement_lists() and create_measurements_list_files()
>    to create_securityfs_measurement_lists(), and marked both as __init.
>    (suggested by Mimi)
>  - Renamed ima_ascii_measurements_files to ascii_securityfs_measurement_lists,
>    ima_binary_measurements_files to binary_securityfs_measurement_lists and
>    ima_measurements_files_count to securityfs_measurement_list_count, and
>    marked them as __ro_after_init. (suggested by Mimi)
>  - Added missing NULL assignment for file.file in ima_dump_measurement_list()
>    during kexec.
> 
> v4:
>  - Added NULL check on m->file for measurements list dump called by
>    ima_dump_measurement_list() on kexec.
>  - Exported ima_algo_array and struct ima_algo_desc declaration from
>    ima_crypto.c to access this information in ima_fs.c.
>  - Added ima_measurements_files_count global variable to avoid extra
>    logic each time the number of measurements file is needed.
> 
> v3:
>  - Added create_measurements_list_files function for measurements files creation.
>  - Parameterized the remove_measurements_list_files function and add NULL
>    check before freeing files' list.
>  - Removed algorithm selection based on file name during ima_measurements_show
>    and ima_ascii_measurements_show, and selecting it comparing dentry address.
>  - Allocate also sha1 file following the schema
>    {binary, ascii}_runtime_measurements_<hash_algo_name> and keep legacy
>    files as symbolic links to those files.
>  - Allocate measurements files lists even if a TPM chip is not detected,
>    adding only sha1 files.
> 
> v2:
>  - Changed the behavior of configuring at boot time the template data hash
>    algorithm.
>  - Removed template data hash algo name prefix.
>  - Removed ima_template_hash command line option.
>  - Introducing a new file in the securityfs ima subdir for each PCR banks
>    algorithm configured in the TPM.
>    (suggested by Roberto)
> 
> ---
>  security/integrity/ima/ima.h        |  10 +++
>  security/integrity/ima/ima_crypto.c |   7 +-
>  security/integrity/ima/ima_fs.c     | 129 +++++++++++++++++++++++++---
>  security/integrity/ima/ima_kexec.c  |   1 +
>  4 files changed, 129 insertions(+), 18 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c29db699c996..e5e45cce4d0e 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -54,6 +54,16 @@ extern int ima_hash_algo __ro_after_init;
>  extern int ima_sha1_idx __ro_after_init;
>  extern int ima_hash_algo_idx __ro_after_init;
>  extern int ima_extra_slots __ro_after_init;
> +
> +/* IMA hash algorithm description */
> +struct ima_algo_desc {
> +	struct crypto_shash *tfm;
> +	enum hash_algo algo;
> +};
> +
> +/* hash algorithms configured in IMA */
> +extern struct ima_algo_desc *ima_algo_array;
> +
>  extern int ima_appraise;
>  extern struct tpm_chip *ima_tpm_chip;
>  extern const char boot_aggregate_name[];
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index f3738b2c8bcd..3606931fc525 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -57,11 +57,6 @@ MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size");
>  static struct crypto_shash *ima_shash_tfm;
>  static struct crypto_ahash *ima_ahash_tfm;
>  
> -struct ima_algo_desc {
> -	struct crypto_shash *tfm;
> -	enum hash_algo algo;
> -};
> -
>  int ima_sha1_idx __ro_after_init;
>  int ima_hash_algo_idx __ro_after_init;
>  /*
> @@ -70,7 +65,7 @@ int ima_hash_algo_idx __ro_after_init;
>   */
>  int ima_extra_slots __ro_after_init;
>  
> -static struct ima_algo_desc *ima_algo_array;
> +struct ima_algo_desc *ima_algo_array;
>  
>  static int __init ima_init_ima_crypto(void)
>  {
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index cd1683dad3bf..d0c8f7daa2e2 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -116,9 +116,30 @@ void ima_putc(struct seq_file *m, void *data, int datalen)
>  		seq_putc(m, *(char *)data++);
>  }
>  
> +static struct dentry **ascii_securityfs_measurement_lists __ro_after_init;
> +static struct dentry **binary_securityfs_measurement_lists __ro_after_init;
> +static int securityfs_measurement_list_count __ro_after_init;
> +
> +static void lookup_algo_by_dentry(int *algo_idx, enum hash_algo *algo,
> +				  struct seq_file *m, struct dentry **dentry_list)
> +{
> +	struct dentry *dentry;
> +	int i;
> +
> +	dentry = file_dentry(m->file);
> +
> +	for (i = 0; i < securityfs_measurement_list_count; i++) {
> +		if (dentry == dentry_list[i]) {
> +			*algo_idx = i;
> +			*algo = ima_algo_array[i].algo;
> +			break;
> +		}
> +	}
> +}
> +
>  /* print format:
>   *       32bit-le=pcr#
> - *       char[20]=template digest
> + *       char[n]=template digest
>   *       32bit-le=template name size
>   *       char[n]=template name
>   *       [eventdata length]
> @@ -132,7 +153,14 @@ int ima_measurements_show(struct seq_file *m, void *v)
>  	char *template_name;
>  	u32 pcr, namelen, template_data_len; /* temporary fields */
>  	bool is_ima_template = false;
> -	int i;
> +	int i, algo_idx;
> +	enum hash_algo algo;
> +
> +	algo_idx = ima_sha1_idx;
> +	algo = HASH_ALGO_SHA1;
> +
> +	if (m->file != NULL)
> +		lookup_algo_by_dentry(&algo_idx, &algo, m,
> +				      binary_securityfs_measurement_lists);
>  
>  	/* get entry */
>  	e = qe->entry;
> @@ -151,7 +179,7 @@ int ima_measurements_show(struct seq_file *m, void *v)
>  	ima_putc(m, &pcr, sizeof(e->pcr));
>  
>  	/* 2nd: template digest */
> -	ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE);
> +	ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
>  
>  	/* 3rd: template name size */
>  	namelen = !ima_canonical_fmt ? strlen(template_name) :
> @@ -220,7 +248,14 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
>  	struct ima_queue_entry *qe = v;
>  	struct ima_template_entry *e;
>  	char *template_name;
> -	int i;
> +	int i, algo_idx;
> +	enum hash_algo algo;
> +
> +	algo_idx = ima_sha1_idx;
> +	algo = HASH_ALGO_SHA1;
> +
> +	if (m->file != NULL)
> +		lookup_algo_by_dentry(&algo_idx, &algo, m,
> +				      ascii_securityfs_measurement_lists);
>  
>  	/* get entry */
>  	e = qe->entry;
> @@ -233,8 +268,8 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
>  	/* 1st: PCR used (config option) */
>  	seq_printf(m, "%2d ", e->pcr);
>  
> -	/* 2nd: SHA1 template hash */
> -	ima_print_digest(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE);
> +	/* 2nd: template hash */
> +	ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
>  
>  	/* 3th:  template name */
>  	seq_printf(m, " %s", template_name);
> @@ -379,6 +414,69 @@ static const struct seq_operations ima_policy_seqops = {
>  };
>  #endif
>  
> +static void __init remove_securityfs_measurement_lists(struct dentry **dentry_list)
> +{
> +	int i;
> +
> +	if (dentry_list) {
> +		for (i = 0; i < securityfs_measurement_list_count; i++)
> +			securityfs_remove(dentry_list[i]);
> +
> +		kfree(dentry_list);
> +	}
> +
> +	securityfs_measurement_list_count = 0;
> +}
> +
> +static int __init create_securityfs_measurement_lists(void)
> +{
> +	int i;
> +	u16 algo;
> +	char file_name[NAME_MAX + 1];
> +	struct dentry *dentry;
> +
> +	securityfs_measurement_list_count = NR_BANKS(ima_tpm_chip);
> +
> +	if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
> +		securityfs_measurement_list_count++;
> +
> +	ascii_securityfs_measurement_lists = kcalloc(securityfs_measurement_list_count,
> +						     sizeof(struct dentry *), GFP_KERNEL);
> +	if (!ascii_securityfs_measurement_lists)
> +		return -ENOMEM;
> +
> +	binary_securityfs_measurement_lists = kcalloc(securityfs_measurement_list_count,
> +						      sizeof(struct dentry *), GFP_KERNEL);
> +	if (!binary_securityfs_measurement_lists)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < securityfs_measurement_list_count; i++) {
> +		algo = ima_algo_array[i].algo;
> +
> +		sprintf(file_name, "ascii_runtime_measurements_%s",
> +			hash_algo_name[algo]);
> +		dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
> +						ima_dir, NULL,
> +						&ima_ascii_measurements_ops);
> +		if (IS_ERR(dentry))
> +			return PTR_ERR(dentry);
> +
> +		ascii_securityfs_measurement_lists[i] = dentry;
> +
> +		sprintf(file_name, "binary_runtime_measurements_%s",
> +			hash_algo_name[algo]);
> +		dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
> +						ima_dir, NULL,
> +						&ima_measurements_ops);
> +		if (IS_ERR(dentry))
> +			return PTR_ERR(dentry);
> +
> +		binary_securityfs_measurement_lists[i] = dentry;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * ima_open_policy: sequentialize access to the policy file
>   */
> @@ -454,6 +552,9 @@ int __init ima_fs_init(void)
>  {
>  	int ret;
>  
> +	ascii_securityfs_measurement_lists = NULL;
> +	binary_securityfs_measurement_lists = NULL;
> +
>  	ima_dir = securityfs_create_dir("ima", integrity_dir);
>  	if (IS_ERR(ima_dir))
>  		return PTR_ERR(ima_dir);
> @@ -465,19 +566,21 @@ int __init ima_fs_init(void)
>  		goto out;
>  	}
>  
> +	ret = create_securityfs_measurement_lists();
> +	if (ret != 0)
> +		goto out;
> +
>  	binary_runtime_measurements =
> -	    securityfs_create_file("binary_runtime_measurements",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> -				   &ima_measurements_ops);
> +		securityfs_create_symlink("binary_runtime_measurements", ima_dir,
> +					  "binary_runtime_measurements_sha1", NULL);
>  	if (IS_ERR(binary_runtime_measurements)) {
>  		ret = PTR_ERR(binary_runtime_measurements);
>  		goto out;
>  	}
>  
>  	ascii_runtime_measurements =
> -	    securityfs_create_file("ascii_runtime_measurements",
> -				   S_IRUSR | S_IRGRP, ima_dir, NULL,
> -				   &ima_ascii_measurements_ops);
> +		securityfs_create_symlink("ascii_runtime_measurements", ima_dir,
> +					  "ascii_runtime_measurements_sha1", NULL);
>  	if (IS_ERR(ascii_runtime_measurements)) {
>  		ret = PTR_ERR(ascii_runtime_measurements);
>  		goto out;
> @@ -515,6 +618,8 @@ int __init ima_fs_init(void)
>  	securityfs_remove(runtime_measurements_count);
>  	securityfs_remove(ascii_runtime_measurements);
>  	securityfs_remove(binary_runtime_measurements);
> +	remove_securityfs_measurement_lists(ascii_securityfs_measurement_lists);
> +	remove_securityfs_measurement_lists(binary_securityfs_measurement_lists);
>  	securityfs_remove(ima_symlink);
>  	securityfs_remove(ima_dir);
>  
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index dadc1d138118..52e00332defe 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -30,6 +30,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>  		goto out;
>  	}
>  
> +	file.file = NULL;
>  	file.size = segment_size;
>  	file.read_pos = 0;
>  	file.count = sizeof(khdr);	/* reserved space */
> 
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba






[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux