Re: [PATCH] ima: define '_ima' as a builtin 'trusted' keyring

[Dmitry Kasatkin]

On 30/10/13 20:54, Mimi Zohar wrote:
> Require all keys added to the IMA keyring be signed by an
> existing trusted key on the system trusted keyring.
>
> Changelog:
> - define stub integrity_init_keyring() function (reported-by Fengguang Wu)
> - differentiate between regular and trusted keyring names.
> - replace printk with pr_info (D. Kasatkin)
>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxx>
> ---
>  security/integrity/digsig.c           | 30 +++++++++++++++++++++++++++++-
>  security/integrity/ima/Kconfig        |  8 ++++++++
>  security/integrity/ima/ima_appraise.c | 11 +++++++++++
>  security/integrity/integrity.h        |  7 +++++++
>  4 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index b4af4eb..77ca965 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -13,7 +13,9 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/err.h>
> +#include <linux/sched.h>
>  #include <linux/rbtree.h>
> +#include <linux/cred.h>
>  #include <linux/key-type.h>
>  #include <linux/digsig.h>
>  
> @@ -21,11 +23,19 @@
>  
>  static struct key *keyring[INTEGRITY_KEYRING_MAX];
>  
> +#ifdef CONFIG_IMA_TRUSTED_KEYRING
> +static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
> +	".evm",
> +	".module",
> +	".ima",
> +};
> +#else
>  static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
>  	"_evm",
>  	"_module",
>  	"_ima",
>  };
> +#endif

Hello,

I am not sure if having 2 different names "_" and "." makes sense.

Setting trusted-only makes sense until we will get support of setting
trusted only from user-space using keyctl...

David, do you remember our discussion in Edinburgh?
Can you provide a way to set keyring as trusted-only from user space..

Motivation...

In many embedded systems, initramfs is built into the kerâânel image.
Kernel image is signed and obviously initramfs as well..
Or initramfs may be signed separately like in my prototype implementation...

Note that non-x86 systems - embedded, mobile, etc has no UEFI, MOK.
Initial keys cannot be verified. (we should not rely on using kernel
modules key)
Thus keys on the protected initramfs may not be required to be signed..

It must be a way to add "initial keys" from user-space...
This is like "setting initial trust"..
This kind of functionality also useful for ".system" keyring itself.


- Dmitry

>  
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  			    const char *digest, int digestlen)
> @@ -35,7 +45,7 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  
>  	if (!keyring[id]) {
>  		keyring[id] =
> -			request_key(&key_type_keyring, keyring_name[id], NULL);
> +		    request_key(&key_type_keyring, keyring_name[id], NULL);
>  		if (IS_ERR(keyring[id])) {
>  			int err = PTR_ERR(keyring[id]);
>  			pr_err("no %s keyring: %d\n", keyring_name[id], err);
> @@ -56,3 +66,21 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  
>  	return -EOPNOTSUPP;
>  }
> +
> +int integrity_init_keyring(const unsigned int id)
> +{
> +	const struct cred *cred = current_cred();
> +	const struct user_struct *user = cred->user;
> +
> +	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
> +				    KGIDT_INIT(0), cred,
> +				    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +				     KEY_USR_VIEW | KEY_USR_READ),
> +				    KEY_ALLOC_NOT_IN_QUOTA, user->uid_keyring);
> +	if (!IS_ERR(keyring[id]))
> +		set_bit(KEY_FLAG_TRUSTED_ONLY, &keyring[id]->flags);
> +	else
> +		pr_info("Can't allocate %s keyring (%ld)\n",
> +			keyring_name[id], PTR_ERR(keyring[id]));
> +	return 0;
> +}
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 81a2797..dad8d4c 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -123,3 +123,11 @@ config IMA_APPRAISE
>  	  For more information on integrity appraisal refer to:
>  	  <http://linux-ima.sourceforge.net>
>  	  If unsure, say N.
> +
> +config IMA_TRUSTED_KEYRING
> +	bool "Require all keys on the _ima keyring be signed"
> +	depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
> +	default y
> +	help
> +	   This option requires that all keys added to the _ima
> +	   keyring be signed by a key on the system trusted keyring.
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 734e946..46353ee 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -381,3 +381,14 @@ int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name)
>  	}
>  	return result;
>  }
> +
> +#ifdef CONFIG_IMA_TRUSTED_KEYRING
> +static int __init init_ima_keyring(void)
> +{
> +	int ret;
> +
> +	ret = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
> +	return 0;
> +}
> +late_initcall(init_ima_keyring);
> +#endif
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 2fb5e53..b9e7c13 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -137,12 +137,19 @@ static inline int integrity_digsig_verify(const unsigned int id,
>  #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
>  int asymmetric_verify(struct key *keyring, const char *sig,
>  		      int siglen, const char *data, int datalen);
> +
> +int integrity_init_keyring(const unsigned int id);
>  #else
>  static inline int asymmetric_verify(struct key *keyring, const char *sig,
>  				    int siglen, const char *data, int datalen)
>  {
>  	return -EOPNOTSUPP;
>  }
> +
> +static int integrity_init_keyring(const unsigned int id)
> +{
> +	return 0;
> +}
>  #endif
>  
>  #ifdef CONFIG_INTEGRITY_AUDIT

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