lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <535A7CB9.6010000@samsung.com>
Date:	Fri, 25 Apr 2014 18:18:17 +0300
From:	Dmitry Kasatkin <d.kasatkin@...sung.com>
To:	Mimi Zohar <zohar@...ux.vnet.ibm.com>
Cc:	dhowells@...hat.com, jmorris@...ei.org, roberto.sassu@...ito.it,
	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 15/20] ima: path based policy loading interface

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@...sung.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
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ