Re: [RFC PATCH v4 3/4] ima: define '.ima' as a builtin 'trusted' keyring

[Mimi Zohar]

On Fri, 2014-05-30 at 19:05 +0300, Dmitry Kasatkin wrote:
> On 28 May 2014 22:26, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
> > On Wed, 2014-05-28 at 21:55 +0300, Dmitry Kasatkin wrote:
> >> On 28 May 2014 18:09, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
> >> > Require all keys added to the IMA keyring be signed by an
> >> > existing trusted key on the system trusted keyring.
> >> >
> >> > Changelog v1:
> >> > - don't link IMA trusted keyring to user 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)
> >> > - only make the IMA keyring a trusted keyring (reported-by D. Kastatkin)
> >> > - define stub integrity_init_keyring() definition based on
> >> >   CONFIG_INTEGRITY_SIGNATURE, not CONFIG_INTEGRITY_ASYMMETRIC_KEYS.
> >> >   (reported-by Jim Davis)
> >> >
> >> > Signed-off-by: Mimi Zohar<zohar@xxxxxxxxxxxxxxxxxx>
> >> > ---
> >> >  security/integrity/digsig.c           | 26 +++++++++++++++++++++++++-
> >> >  security/integrity/ima/Kconfig        |  8 ++++++++
> >> >  security/integrity/ima/ima_appraise.c | 11 +++++++++++
> >> >  security/integrity/integrity.h        |  5 +++++
> >> >  4 files changed, 49 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> >> > index b4af4eb..7da5f9c 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>
> >> >
> >> > @@ -24,7 +26,11 @@ static struct key *keyring[INTEGRITY_KEYRING_MAX];
> >> >  static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
> >> >         "_evm",
> >> >         "_module",
> >> > +#ifndef CONFIG_IMA_TRUSTED_KEYRING
> >> >         "_ima",
> >> > +#else
> >> > +       ".ima",
> >> > +#endif
> >> >  };
> >> >
> >> >  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> >> > @@ -35,7 +41,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 +62,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();
> >> > +
> >> > +       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_USR_WRITE | KEY_USR_SEARCH),
> >> > +                                   KEY_ALLOC_NOT_IN_QUOTA, NULL);
> >>
> >> Last parameter "destination" is NULL. It makes keyring "unsearchable"
> >> from user space.
> >> It prevents loading trusted keys from user-space, e.g. initramfs...
> >>
> >> Should it be "cred->user->uid_keyring"??
> >
> > David extended keyctl with the '%keyring' option.  For example,
> > "keyctl show %keyring:.ima" returns the .ima keyring id with a list of
> > all the keys.
> >
> 
> That is not kernel feature, but keyctl feature as I can see.
> It will not find keyring from user space..
> 
> keyutils.c 3.5.7 has this kind of thing
> f = fopen("/proc/keys", "r");
> 
> But it would require CONFIG_PROC_KEYS to be enabled.
> 
> May be David may comment...

David commented on an prior patch set, which defined a new id for the
system trusted keyring. For hjs comments, refer to
http://marc.info/?l=linux-security-module&m=137829415530503&w=2

thanks,

Mimi

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