Re: [PATCH 15/20] ima: path based policy loading interface

[Dmitry Kasatkin <d.kasatkin@samsung.com>]

On 25/04/14 00:03, Mimi Zohar wrote:
> On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
>> Currently policy is loaded by writing policy content to
>> '<securityfs>/ima/policy' file.
>>
>> This patch extends policy loading meachanism with possibility
>> to load signed policy using a path to the policy.
>> Policy signature must be available in the <policy>.sig file.
> Assuming (big assumption) you're permitted to open the policy file from
> the kernel, why are you verifying the signature inline based on a .sig?
> Shouldn't this be a new integrity/security hook?

What kind of hook do you mean?

Actually I was considering 2 approaches.

1. Introduce additional IMA function similar to ima_module_check() and
then retrieve verification status OK+sig
That would be using normal measurement code.
This requires anyway to open file from the kernel.
But it would be a bit tricky to move policy and signature together and
does not work with auto-generated initramfs
images on the distros as they do not have xattrs...

2. Read policy to the buffer as it is needed anyway and then just verify
the signature.
This was just simple enough as initial thought to implement.
Easy to copy policy and works with initramfs.


- Dmitry

> thanks,
>
> Mimi
>
>> Policy can be loaded like:
>> echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  security/integrity/ima/Kconfig      | 13 +++++++
>>  security/integrity/ima/ima.h        |  9 +++++
>>  security/integrity/ima/ima_fs.c     |  2 +-
>>  security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 97 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>> index 5474c47..465cef4 100644
>> --- a/security/integrity/ima/Kconfig
>> +++ b/security/integrity/ima/Kconfig
>> @@ -140,3 +140,16 @@ config IMA_LOAD_X509
>>  	help
>>  	   This option enables X509 certificate loading from the kernel
>>  	   to the '_ima' trusted keyring.
>> +
>> +config IMA_POLICY_LOADER
>> +	bool "Path based policy loading interface"
>> +	depends on IMA_TRUSTED_KEYRING
>> +	default n
>> +	help
>> +	  This option enables path based signed policy loading interface.
>> +	  Policy signature must be provided in the <policy>.sig file
>> +	  along with the policy. When this option is enabled, kernel
>> +	  tries to load default policy from /etc/ima_policy.
>> +
>> +	  Loading policy is like:
>> +	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index 3b90b60..f2722bb 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -170,6 +170,15 @@ bool ima_default_policy(void);
>>  ssize_t ima_parse_add_rule(char *);
>>  void ima_delete_rules(void);
>>
>> +#ifdef CONFIG_IMA_POLICY_LOADER
>> +ssize_t ima_read_policy(char *path);
>> +#else
>> +static inline ssize_t ima_read_policy(char *data)
>> +{
>> +	return ima_parse_add_rule(data);
>> +}
>> +#endif
>> +
>>  /* Appraise integrity measurements */
>>  #define IMA_APPRAISE_ENFORCE	0x01
>>  #define IMA_APPRAISE_FIX	0x02
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index 34ae5f2..bde7a0e 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
>>  	if (copy_from_user(data, buf, datalen))
>>  		goto out;
>>
>> -	result = ima_parse_add_rule(data);
>> +	result = ima_read_policy(data);
>>  out:
>>  	if (result < 0)
>>  		valid_policy = 0;
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index b24e7d1..c6da801 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -17,6 +17,9 @@
>>  #include <linux/parser.h>
>>  #include <linux/slab.h>
>>  #include <linux/genhd.h>
>> +#ifdef CONFIG_IMA_POLICY_LOADER
>> +#include <linux/file.h>
>> +#endif
>>
>>  #include "ima.h"
>>
>> @@ -747,3 +750,74 @@ void ima_delete_rules(void)
>>  	}
>>  	mutex_unlock(&ima_rules_mutex);
>>  }
>> +
>> +#ifdef CONFIG_IMA_POLICY_LOADER
>> +
>> +ssize_t ima_read_policy(char *path)
>> +{
>> +	char *data, *datap, *sig;
>> +	int rc, psize, pathlen = strlen(path);
>> +	char *p, *sigpath;
>> +	struct {
>> +		struct ima_digest_data hdr;
>> +		char digest[IMA_MAX_DIGEST_SIZE];
>> +	} hash;
>> +
>> +	if (path[0] != '/')
>> +		return ima_parse_add_rule(path);
>> +
>> +	/* remove \n */
>> +	datap = path;
>> +	strsep(&datap, "\n");
>> +
>> +	/* we always want signature? */
>> +	sigpath = __getname();
>> +	if (!sigpath)
>> +		return -ENOMEM;
>> +
>> +	rc = integrity_read_file(path, &data);
>> +	if (rc < 0)
>> +		goto free_path;
>> +
>> +	psize = rc;
>> +	datap = data;
>> +
>> +	sprintf(sigpath, "%s.sig", path);
>> +	/* we always want signature? */
>> +	rc = integrity_read_file(sigpath, &sig);
>> +	if (rc < 0)
>> +		goto free_data;
>> +
>> +	hash.hdr.algo = ima_hash_algo;
>> +	ima_get_hash_algo((void *)sig, rc, &hash.hdr);
>> +	ima_calc_buffer_hash(data, psize, &hash.hdr);
>> +	rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
>> +				     (const char *)sig, rc,
>> +				     hash.hdr.digest, hash.hdr.length);
>> +	if (rc) {
>> +		pr_err("integrity_digsig_verify() = %d\n", rc);
>> +		goto free_sig;
>> +	}
>> +
>> +	while (psize > 0 && (p = strsep(&datap, "\n"))) {
>> +		pr_debug("rule: %s\n", p);
>> +		rc = ima_parse_add_rule(p);
>> +		if (rc < 0)
>> +			break;
>> +		psize -= rc;
>> +	}
>> +free_sig:
>> +	kfree(sig);
>> +free_data:
>> +	kfree(data);
>> +free_path:
>> +	__putname(sigpath);
>> +	if (rc < 0)
>> +		return rc;
>> +	else if (psize)
>> +		return -EINVAL;
>> +	else
>> +		return pathlen;
>> +}
>> +
>> +#endif
>
>