Messages in this thread | | | Date | Thu, 16 Aug 2012 14:37:52 -0400 | Subject | Re: [RFC v2 3/7] integrity: create and inititialize a keyring with builtin public key | From | Josh Boyer <> |
| |
On Wed, Aug 15, 2012 at 2:43 PM, Dmitry Kasatkin <dmitry.kasatkin@intel.com> wrote: > From: Mimi Zohar <zohar@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 <zohar@us.ibm.com> > Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@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.
> +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?
> + > + 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?
> + } > + > + 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
| |