On Thu, 2012-08-16 at 14:37 -0400, Josh Boyer wrote:
> On Wed, Aug 15, 2012 at 2:43 PM, Dmitry Kasatkin
> <dmitry.kasat...@intel.com> wrote:
> > From: Mimi Zohar <zo...@linux.vnet.ibm.com>
> >
> > Create and initialize a keyring with the builtin public key. This could
> > be an ephemeral key, created and destroyed during module install for
> > custom built kernels, or a key used to label the entire filesystem for
> > EVM/IMA-appraisal. Uses .incbin based on David Howell's post.
> >
> > Load the builtin public key on the specified keyring, creating the
> > keyring if it doesn't already exist.
> >
> > Signed-off-by: Mimi Zohar <zo...@us.ibm.com>
> > Signed-off-by: Dmitry Kasatkin <dmitry.kasat...@intel.com>
> > ---
> > security/integrity/Kconfig | 10 ++++
> > security/integrity/Makefile | 17 +++++++
> > security/integrity/digsig_pubkey.c | 96
> > ++++++++++++++++++++++++++++++++++++
> > security/integrity/integrity.h | 10 ++++
> > 4 files changed, 133 insertions(+)
> > create mode 100644 security/integrity/digsig_pubkey.c
> >
> > diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> > index 5bd1cc1..f789018 100644
> > --- a/security/integrity/Kconfig
> > +++ b/security/integrity/Kconfig
> > @@ -17,5 +17,15 @@ config INTEGRITY_SIGNATURE
> > This is useful for evm and module keyrings, when keys are
> > usually only added from initramfs.
> >
> > +config INTEGRITY_PUBKEY
> > + boolean "Create a keyring and initialize with builtin public key"
> > + depends on INTEGRITY_SIGNATURE
> > + default n
> > + help
> > + Create and initialize a keyring with the builtin public key. This
> > could
> > + be an ephemeral key, created and destroyed during module install
> > for
> > + custom built kernels, or a key used to label the entire
> > filesystem for
> > + EVM/IMA-appraisal.
> > +
>
> It could also be a key that is reused explicitly for signing kernels
> and kernel modules but has nothing to do with EVM/IMA filesystem
> labels, right? E.g. a distro key. I think the commit log and help
> text is a bit too restrictive here.
Definitely! The cover letter included the distro scenario, but I missed
updating it here.
> > +int integrity_init_keyring(const unsigned int id)
> > +{
> > + const struct cred *cred = current_cred();
> > + struct user_struct *user = cred->user;
> > + struct key *new_keyring, *key;
> > + u8 digest[SHA1_DIGEST_SIZE];
> > + char keyid[20];
> > + int ret, pubkey_size = pubkey_end - pubkey;
> > +
> > + if (pubkey_size == 0) {
> > + pr_info("pubkey is missing, skipping...\n");
> > + return 0;
> > + }
> > +
> > + new_keyring = keyring_alloc(keyring_name[id], user->uid, (gid_t) -1,
> > + cred, KEY_ALLOC_NOT_IN_QUOTA,
> > + user->uid_keyring);
> > + if (IS_ERR(new_keyring)) {
> > + ret = PTR_ERR(new_keyring);
> > + goto out;
> > + }
> > +
> > + ret = integrity_calc_digest("sha1", pubkey, pubkey_size, digest);
> > + if (ret < 0)
> > + goto out;
> > +
> > + sprintf(keyid, "%llX", __be64_to_cpup((uint64_t *)(digest+12)));
>
> keyid is only 20 bytes. Is there a guarantee somewhere that restricts
> the digest+12 value to be 20 bytes or less and NUL termintated? If
> not, should you use snprintf?
Correct, and SHA1 shouldn't be hardcoded either, but configurable.
>
> > +
> > + key = key_alloc(&key_type_user, keyid, 0, 0, current_cred(),
> > + (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> > + KEY_USR_VIEW | KEY_USR_SEARCH | KEY_USR_READ,
> > + KEY_ALLOC_NOT_IN_QUOTA);
> > + if (IS_ERR(key)) {
> > + ret = PTR_ERR(key);
> > + goto out;
>
> You leak new_keyring if here, right?
thanks
> > + }
> > +
> > + ret = key_instantiate_and_link(key, pubkey, pubkey_end - pubkey,
> > + new_keyring, NULL);
> > +out:
> > + pr_info("integrity: loaded public key %s on %s %s\n", keyid,
> > + keyring_name[id], !ret ? "succeeded" : "failed");
> > + return ret;
> > +}
>
> josh
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
