Messages in this thread | | | Subject | Re: [RFC v2 3/7] integrity: create and inititialize a keyring with builtin public key | From | Mimi Zohar <> | Date | Thu, 16 Aug 2012 15:28:52 -0400 |
| |
On Thu, 2012-08-16 at 14:37 -0400, Josh Boyer wrote: > 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.
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
| |