linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ima: define '_ima' as a builtin 'trusted' keyring
@ 2013-10-30 18:54 Mimi Zohar
  2013-10-31  8:30 ` Dmitry Kasatkin
  0 siblings, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2013-10-30 18:54 UTC (permalink / raw)
  To: linux-security-module; +Cc: David Howells, linux-kernel

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@us.ibm.com>
---
 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
 
 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
-- 
1.8.1.4




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ima: define '_ima' as a builtin 'trusted' keyring
  2013-10-30 18:54 [PATCH] ima: define '_ima' as a builtin 'trusted' keyring Mimi Zohar
@ 2013-10-31  8:30 ` Dmitry Kasatkin
  2013-10-31 12:03   ` Mimi Zohar
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kasatkin @ 2013-10-31  8:30 UTC (permalink / raw)
  To: Mimi Zohar, linux-security-module, David Howells; +Cc: linux-kernel

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@us.ibm.com>
> ---
>  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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ima: define '_ima' as a builtin 'trusted' keyring
  2013-10-31  8:30 ` Dmitry Kasatkin
@ 2013-10-31 12:03   ` Mimi Zohar
  2013-10-31 12:23     ` Dmitry Kasatkin
  0 siblings, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2013-10-31 12:03 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: linux-security-module, David Howells, linux-kernel

On Thu, 2013-10-31 at 10:30 +0200, Dmitry Kasatkin wrote:
> 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@us.ibm.com>
> > ---
> >  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.

The existing keyring implementation permits userspace to create a new
keyring with the exact same name as a previously defined trusted
keyring.  For all practical purposes, replacing a trusted keyring with
an untrusted one.  The existing solution is to prohibit userspace from
creating a dot prefixed keyring.  

Allowing only signed keys to be added to the IMA keyring breaks the
existing userspace/kernel ABI, which has existed since linux-3.3.  At
some point, we could deprecate the non trusted keyring. 

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

Agreed, userspace should be permitted to create a trusted keyring, but
not change an existing keyring to trusted.

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

In the builtin initramfs case, the public key is included in the signed
image.  Where is the key stored that verifies the separately signed
initramfs?  Is there a signature chain of trust?

If there is a signature chain of trust and a local-ca signed the
initramfs, then the local-ca key could be added to the system keyring
and used to sign keys for the IMA keyring.

thanks,

Mimi

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ima: define '_ima' as a builtin 'trusted' keyring
  2013-10-31 12:03   ` Mimi Zohar
@ 2013-10-31 12:23     ` Dmitry Kasatkin
  2013-10-31 12:43       ` Mimi Zohar
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kasatkin @ 2013-10-31 12:23 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-security-module, David Howells, linux-kernel

On 31/10/13 14:03, Mimi Zohar wrote:
> On Thu, 2013-10-31 at 10:30 +0200, Dmitry Kasatkin wrote:
>> 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@us.ibm.com>
>>> ---
>>>  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.
> The existing keyring implementation permits userspace to create a new
> keyring with the exact same name as a previously defined trusted
> keyring.  For all practical purposes, replacing a trusted keyring with
> an untrusted one.  The existing solution is to prohibit userspace from
> creating a dot prefixed keyring.  
>
> Allowing only signed keys to be added to the IMA keyring breaks the
> existing userspace/kernel ABI, which has existed since linux-3.3.  At
> some point, we could deprecate the non trusted keyring. 
>
>> Setting trusted-only makes sense until we will get support of setting
>> trusted only from user-space using keyctl...
> Agreed, userspace should be permitted to create a trusted keyring, but
> not change an existing keyring to trusted.

Then all keys on that keyring must be signed..
This is not what I was saying...

It is always possible to specify keyring hierarchy and rules what
verifies what.
But may be better not to over-engineer...

It is how it is now.. Will see based on use-cases in the future...

>> 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..
> In the builtin initramfs case, the public key is included in the signed
> image.  Where is the key stored that verifies the separately signed
> initramfs?  Is there a signature chain of trust?

In prototype implementation I used kernel module verification
function... module key...

>
> If there is a signature chain of trust and a local-ca signed the
> initramfs, then the local-ca key could be added to the system keyring
> and used to sign keys for the IMA keyring.
>
> thanks,

You need to embed local-ca somehow into the kernel..
Or pass/read and verify it somehow...

- Dmitry

> Mimi
>
>> 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.
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ima: define '_ima' as a builtin 'trusted' keyring
  2013-10-31 12:23     ` Dmitry Kasatkin
@ 2013-10-31 12:43       ` Mimi Zohar
  2013-10-31 13:18         ` Dmitry Kasatkin
  0 siblings, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2013-10-31 12:43 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: linux-security-module, David Howells, linux-kernel

On Thu, 2013-10-31 at 14:23 +0200, Dmitry Kasatkin wrote:
> On 31/10/13 14:03, Mimi Zohar wrote:
> > On Thu, 2013-10-31 at 10:30 +0200, Dmitry Kasatkin wrote:
> >> 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@us.ibm.com>
> >>> ---
> >>>  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.
> > The existing keyring implementation permits userspace to create a new
> > keyring with the exact same name as a previously defined trusted
> > keyring.  For all practical purposes, replacing a trusted keyring with
> > an untrusted one.  The existing solution is to prohibit userspace from
> > creating a dot prefixed keyring.  
> >
> > Allowing only signed keys to be added to the IMA keyring breaks the
> > existing userspace/kernel ABI, which has existed since linux-3.3.  At
> > some point, we could deprecate the non trusted keyring. 
> >
> >> Setting trusted-only makes sense until we will get support of setting
> >> trusted only from user-space using keyctl...
> > Agreed, userspace should be permitted to create a trusted keyring, but
> > not change an existing keyring to trusted.
> 
> Then all keys on that keyring must be signed..
> This is not what I was saying...
> 
> It is always possible to specify keyring hierarchy and rules what
> verifies what.
> But may be better not to over-engineer...
> 
> It is how it is now.. Will see based on use-cases in the future...

Right, keys can be loaded onto the existing keyring; and the keyring can
be locked in the initramfs.  Moving forward, a trusted keyring implies a
HW based certificate chain of trust.

> >> 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..
> > In the builtin initramfs case, the public key is included in the signed
> > image.  Where is the key stored that verifies the separately signed
> > initramfs?  Is there a signature chain of trust?
> 
> In prototype implementation I used kernel module verification
> function... module key...

This implies that you rebuilt the kernel. :)  In that case, add your
local-ca public key to the root build tree.  All .x509 suffixed files
are included in the image and loaded on the system keyring.

> >
> > If there is a signature chain of trust and a local-ca signed the
> > initramfs, then the local-ca key could be added to the system keyring
> > and used to sign keys for the IMA keyring.
> >
> > thanks,
> 
> You need to embed local-ca somehow into the kernel..
> Or pass/read and verify it somehow...

Exactly.

Mimi


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ima: define '_ima' as a builtin 'trusted' keyring
  2013-10-31 12:43       ` Mimi Zohar
@ 2013-10-31 13:18         ` Dmitry Kasatkin
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Kasatkin @ 2013-10-31 13:18 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-security-module, David Howells, linux-kernel

On 31/10/13 14:43, Mimi Zohar wrote:
> On Thu, 2013-10-31 at 14:23 +0200, Dmitry Kasatkin wrote:
>> On 31/10/13 14:03, Mimi Zohar wrote:
>>> On Thu, 2013-10-31 at 10:30 +0200, Dmitry Kasatkin wrote:
>>>> 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@us.ibm.com>
>>>>> ---
>>>>>  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.
>>> The existing keyring implementation permits userspace to create a new
>>> keyring with the exact same name as a previously defined trusted
>>> keyring.  For all practical purposes, replacing a trusted keyring with
>>> an untrusted one.  The existing solution is to prohibit userspace from
>>> creating a dot prefixed keyring.  
>>>
>>> Allowing only signed keys to be added to the IMA keyring breaks the
>>> existing userspace/kernel ABI, which has existed since linux-3.3.  At
>>> some point, we could deprecate the non trusted keyring. 
>>>
>>>> Setting trusted-only makes sense until we will get support of setting
>>>> trusted only from user-space using keyctl...
>>> Agreed, userspace should be permitted to create a trusted keyring, but
>>> not change an existing keyring to trusted.
>> Then all keys on that keyring must be signed..
>> This is not what I was saying...
>>
>> It is always possible to specify keyring hierarchy and rules what
>> verifies what.
>> But may be better not to over-engineer...
>>
>> It is how it is now.. Will see based on use-cases in the future...
> Right, keys can be loaded onto the existing keyring; and the keyring can
> be locked in the initramfs.  Moving forward, a trusted keyring implies a
> HW based certificate chain of trust.
>
>>>> 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..
>>> In the builtin initramfs case, the public key is included in the signed
>>> image.  Where is the key stored that verifies the separately signed
>>> initramfs?  Is there a signature chain of trust?
>> In prototype implementation I used kernel module verification
>> function... module key...
> This implies that you rebuilt the kernel. :)  In that case, add your
> local-ca public key to the root build tree.  All .x509 suffixed files
> are included in the image and loaded on the system keyring.
>

No.. Key stays on the filesystem... It is re-generated if it is missing...

- Dmitry

>>> If there is a signature chain of trust and a local-ca signed the
>>> initramfs, then the local-ca key could be added to the system keyring
>>> and used to sign keys for the IMA keyring.
>>>
>>> thanks,
>> You need to embed local-ca somehow into the kernel..
>> Or pass/read and verify it somehow...
> Exactly.
>
> Mimi
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-31 13:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-30 18:54 [PATCH] ima: define '_ima' as a builtin 'trusted' keyring Mimi Zohar
2013-10-31  8:30 ` Dmitry Kasatkin
2013-10-31 12:03   ` Mimi Zohar
2013-10-31 12:23     ` Dmitry Kasatkin
2013-10-31 12:43       ` Mimi Zohar
2013-10-31 13:18         ` Dmitry Kasatkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).