Re: [PATCH v3 2/2] Adds ima_root_ca keyring;

2015-10-09 Thread Petko Manolov 
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 
> > ---
> >  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),
> > +_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),
> >   _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

Re: [PATCH v3 2/2] Adds ima_root_ca keyring;

2015-10-02 Thread Mimi Zohar 
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 
> ---
>  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),
> +  _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),
> _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