On 15-10-02 13:15:49, Mimi Zohar wrote:
> On Thu, 2015-09-10 at 14:17 +0300, Petko Manolov wrote:
> > The .system keyring is populated at kernel build time and read-only while
> > the
> > system is running. There is no way to dynamically add other user's CA so
> > .ima_root_ca was introduced as read-write keyring that stores these
> > certificates. CA hierarchy is achieved by allowing import of key material
> > that
> > has been signed by CA already present in the .system keyring.
> >
> > The new .ima_blacklist is a keyring that holds all revoked IMA keys. It is
> > consulted first, then the .ima keyring.
>
> A couple of minor comments inline below...
Now that I am back from the ELC I will address both of the comments and send
you
the new patch-set.
Petko
> > Signed-off-by: Petko Manolov <pet...@mip-labs.com>
> > ---
> > crypto/asymmetric_keys/x509_public_key.c | 2 ++
> > include/keys/system_keyring.h | 13 ++++++++
> > security/integrity/digsig_asymmetric.c | 13 ++++++++
> > security/integrity/ima/Kconfig | 11 +++++++
> > security/integrity/ima/Makefile | 1 +
> > security/integrity/ima/ima_root_ca.c | 56
> > ++++++++++++++++++++++++++++++++
> > security/integrity/integrity.h | 13 ++++++++
> > 7 files changed, 109 insertions(+)
> > create mode 100644 security/integrity/ima/ima_root_ca.c
> >
> > diff --git a/crypto/asymmetric_keys/x509_public_key.c
> > b/crypto/asymmetric_keys/x509_public_key.c
> > index 6d88dd1..e39ca38 100644
> > --- a/crypto/asymmetric_keys/x509_public_key.c
> > +++ b/crypto/asymmetric_keys/x509_public_key.c
> > @@ -319,6 +319,8 @@ static int x509_key_preparse(struct
> > key_preparsed_payload *prep)
> > goto error_free_cert;
> > } else if (!prep->trusted) {
> > ret = x509_validate_trust(cert, get_system_trusted_keyring());
> > + if (ret)
> > + ret = x509_validate_trust(cert,
> > get_ima_root_ca_keyring());
> > if (!ret)
> > prep->trusted = 1;
> > }
> > diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> > index b20cd88..774de6c 100644
> > --- a/include/keys/system_keyring.h
> > +++ b/include/keys/system_keyring.h
> > @@ -35,4 +35,17 @@ extern int system_verify_data(const void *data, unsigned
> > long len,
> > enum key_being_used_for usage);
> > #endif
> >
> > +#ifdef CONFIG_IMA_ROOT_CA_KEYRING
> > +extern struct key *ima_root_ca_keyring;
> > +static inline struct key *get_ima_root_ca_keyring(void)
> > +{
> > + return ima_root_ca_keyring;
> > +}
> > +#else
> > +static inline struct key *get_ima_root_ca_keyring(void)
> > +{
> > + return NULL;
> > +}
> > +#endif /* CONFIG_IMA_ROOT_CA_KEYRING */
> > +
> > #endif /* _KEYS_SYSTEM_KEYRING_H */
> > diff --git a/security/integrity/digsig_asymmetric.c
> > b/security/integrity/digsig_asymmetric.c
> > index 4fec181..52377d9 100644
> > --- a/security/integrity/digsig_asymmetric.c
> > +++ b/security/integrity/digsig_asymmetric.c
> > @@ -32,9 +32,22 @@ static struct key *request_asymmetric_key(struct key
> > *keyring, uint32_t keyid)
> >
> > pr_debug("key search: \"%s\"\n", name);
> >
> > + key = get_ima_blacklist_keyring();
> > + if (key) {
> > + key_ref_t kref;
> > +
> > + kref = keyring_search(make_key_ref(key, 1),
> > + &key_type_asymmetric, name);
> > + if (!IS_ERR(kref)) {
> > + pr_err("Key '%s' is in ima_blacklist_keyring\n", name);
> > + return ERR_PTR(-EKEYREJECTED);
> > + }
> > + }
> > +
> > if (keyring) {
> > /* search in specific keyring */
> > key_ref_t kref;
> > +
> > kref = keyring_search(make_key_ref(keyring, 1),
> > &key_type_asymmetric, name);
> > if (IS_ERR(kref))
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index ebe7a907..69426ce 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -146,6 +146,17 @@ config IMA_TRUSTED_KEYRING
> > This option requires that all keys added to the .ima
> > keyring be signed by a key on the system trusted keyring.
> >
> > +config IMA_ROOT_CA_KEYRING
> > + bool "Create IMA Root CA keyring"
> > + depends on IMA_TRUSTED_KEYRING
> > + default y
> > + help
> > + This option creates IMA Root CA keyring. This is intermediate
> > + keyring which sits between the .system and .ima keyrings, effectively
> > + creating a simple CA hierarchy. All keys in it must be signed either
> > + by a key in the .system keyring or one which is already in
> > + .ima_root_ca_keyring.
> > +
> > config IMA_LOAD_X509
> > bool "Load X509 certificate onto the '.ima' trusted keyring"
> > depends on IMA_TRUSTED_KEYRING
> > diff --git a/security/integrity/ima/Makefile
> > b/security/integrity/ima/Makefile
> > index d79263d..b2f9aa0 100644
> > --- a/security/integrity/ima/Makefile
> > +++ b/security/integrity/ima/Makefile
> > @@ -8,3 +8,4 @@ obj-$(CONFIG_IMA) += ima.o
> > ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o
> > \
> > ima_policy.o ima_template.o ima_template_lib.o
> > ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> > +obj-$(CONFIG_IMA_ROOT_CA_KEYRING) += ima_root_ca.o
> > diff --git a/security/integrity/ima/ima_root_ca.c
> > b/security/integrity/ima/ima_root_ca.c
> > new file mode 100644
> > index 0000000..23fccdc
> > --- /dev/null
> > +++ b/security/integrity/ima/ima_root_ca.c
> > @@ -0,0 +1,56 @@
> > +/*
> > + * Copyright (C) 2015 Juniper Networks, Inc.
> > + *
> > + * Author:
> > + * Petko Manolov <petko.mano...@konsulko.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation, version 2 of the
> > + * License.
> > + *
> > + */
> > +
> > +#include <linux/export.h>
> > +#include <linux/kernel.h>
> > +#include <linux/sched.h>
> > +#include <linux/cred.h>
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <keys/asymmetric-type.h>
> > +
> > +
> > +struct key *ima_root_ca_keyring;
> > +EXPORT_SYMBOL_GPL(ima_root_ca_keyring);
> > +struct key *ima_blacklist_keyring;
> > +EXPORT_SYMBOL_GPL(ima_blacklist_keyring);
>
> Do these keyrings need to be exported?
>
> > +/*
> > + * Allocate the IMA Root CA keyring
> > + */
> > +__init int ima_root_ca_init(void)
> > +{
> > + pr_notice("Initialise IMA Root CA keyring.\n");
> > +
> > + ima_root_ca_keyring = keyring_alloc(".ima_root_ca",
> > + KUIDT_INIT(0), KGIDT_INIT(0), current_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);
> > +
> > + ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
> > + KUIDT_INIT(0), KGIDT_INIT(0), current_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);
> > +
> > + if (IS_ERR(ima_root_ca_keyring) || IS_ERR(ima_blacklist_keyring))
> > + panic("Can't allocate IMA Root CA or blacklist keyrings.");
> > + set_bit(KEY_FLAG_TRUSTED_ONLY, &ima_root_ca_keyring->flags);
> > + set_bit(KEY_FLAG_TRUSTED_ONLY, &ima_blacklist_keyring->flags);
>
> Why not use the existing ima/integrity_init_keyring() function?
>
> Mimi
>
> > + return 0;
> > +}
> > +
> > +module_init(ima_root_ca_init);
> > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> > index 9c61687..79d4a33 100644
> > --- a/security/integrity/integrity.h
> > +++ b/security/integrity/integrity.h
> > @@ -184,3 +184,16 @@ static inline void integrity_audit_msg(int
> > audit_msgno, struct inode *inode,
> > {
> > }
> > #endif
> > +
> > +#ifdef CONFIG_IMA_ROOT_CA_KEYRING
> > +extern struct key *ima_blacklist_keyring;
> > +static inline struct key *get_ima_blacklist_keyring(void)
> > +{
> > + return ima_blacklist_keyring;
> > +}
> > +#else
> > +static inline struct key *get_ima_blacklist_keyring(void)
> > +{
> > + return NULL;
> > +}
> > +#endif
>
>
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
