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

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

Reply via email to