Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #4]

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

On Thu, 2016-04-07 at 09:59 +0100, David Howells wrote:
> Add a config option (IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY)
> that, when enabled, allows keys to be added to the IMA keyrings by
> userspace - with the restriction that each must be signed by a key in the
> system trusted keyrings.
> 
> EPERM will be returned if this option is disabled, ENOKEY will be returned if
> no authoritative key can be found and EKEYREJECTED will be returned if the
> signature doesn't match.  Other errors such as ENOPKG may also be returned.
> 
> If this new option is enabled, the builtin system keyring is searched, as is
> the secondary system keyring if that is also enabled.  Intermediate keys
> between the builtin system keyring and the key being added can be added to
> the secondary keyring (which replaces .ima_mok) to form a trust chain -
> provided they are also validly signed by a key in one of the trusted keyrings.
> 
> The .ima_mok keyring is then removed and the IMA blacklist keyring gets its
> own config option (IMA_BLACKLIST_KEYRING).
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Eventually we'll want to be able to load keys directly on the IMA
keyring, but until that code is added, this is good.  Thanks!

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>

Mimi

> ---
> 
>  include/keys/system_keyring.h    |   13 ++-----------
>  security/integrity/digsig.c      |   30 ++++--------------------------
>  security/integrity/ima/Kconfig   |   35 ++++++++++++++++++++++-------------
>  security/integrity/ima/Makefile  |    2 +-
>  security/integrity/ima/ima_mok.c |   17 ++++-------------
>  5 files changed, 33 insertions(+), 64 deletions(-)
> 
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 614424029de7..fbd4647767e9 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -33,28 +33,19 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
>  #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
>  #endif
> 
> -#ifdef CONFIG_IMA_MOK_KEYRING
> -extern struct key *ima_mok_keyring;
> +#ifdef CONFIG_IMA_BLACKLIST_KEYRING
>  extern struct key *ima_blacklist_keyring;
> 
> -static inline struct key *get_ima_mok_keyring(void)
> -{
> -	return ima_mok_keyring;
> -}
>  static inline struct key *get_ima_blacklist_keyring(void)
>  {
>  	return ima_blacklist_keyring;
>  }
>  #else
> -static inline struct key *get_ima_mok_keyring(void)
> -{
> -	return NULL;
> -}
>  static inline struct key *get_ima_blacklist_keyring(void)
>  {
>  	return NULL;
>  }
> -#endif /* CONFIG_IMA_MOK_KEYRING */
> +#endif /* CONFIG_IMA_BLACKLIST_KEYRING */
> 
> 
>  #endif /* _KEYS_SYSTEM_KEYRING_H */
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 98ee4c752cf5..4304372b323f 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -42,32 +42,10 @@ static bool init_keyring __initdata = true;
>  static bool init_keyring __initdata;
>  #endif
> 
> -#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
> -/*
> - * Restrict the addition of keys into the IMA keyring.
> - *
> - * Any key that needs to go in .ima keyring must be signed by CA in
> - * either .system or .ima_mok keyrings.
> - */
> -static int restrict_link_by_ima_mok(struct key *keyring,
> -				    const struct key_type *type,
> -				    const union key_payload *payload)
> -{
> -	int ret;
> -
> -	ret = restrict_link_by_builtin_trusted(keyring, type, payload);
> -	if (ret != -ENOKEY)
> -		return ret;
> -
> -	return restrict_link_by_signature(get_ima_mok_keyring(),
> -					  type, payload);
> -}
> +#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> +#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
>  #else
> -/*
> - * If there's no system trusted keyring, then keys cannot be loaded into
> - * .ima_mok and added keys cannot be marked trusted.
> - */
> -#define restrict_link_by_ima_mok restrict_link_reject
> +#define restrict_link_to_ima restrict_link_by_builtin_trusted
>  #endif
> 
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> @@ -114,7 +92,7 @@ int __init integrity_init_keyring(const unsigned int id)
>  				     KEY_USR_VIEW | KEY_USR_READ |
>  				     KEY_USR_WRITE | KEY_USR_SEARCH),
>  				    KEY_ALLOC_NOT_IN_QUOTA,
> -				    restrict_link_by_ima_mok, NULL);
> +				    restrict_link_to_ima, NULL);
>  	if (IS_ERR(keyring[id])) {
>  		err = PTR_ERR(keyring[id]);
>  		pr_info("Can't allocate %s keyring (%d)\n",
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index e54a8a8dae94..aab9b0a53edf 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -155,23 +155,32 @@ config IMA_TRUSTED_KEYRING
> 
>  	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> 
> -config IMA_MOK_KEYRING
> -	bool "Create IMA machine owner keys (MOK) and blacklist keyrings"
> +config IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> +	bool "Permit keys validly signed by a built-in or secondary CA cert (EXPERIMENTAL)"
> +	depends on SYSTEM_TRUSTED_KEYRING
> +	depends on SECONDARY_TRUSTED_KEYRING
> +	select INTEGRITY_TRUSTED_KEYRING
> +	default n
> +	help
> +	  Keys may be added to the IMA or IMA blacklist keyrings, if the
> +	  key is validly signed by a CA cert in the system built-in or
> +	  secondary trusted keyrings.
> +
> +	  Intermediate keys between those the kernel has compiled in and the
> +	  IMA keys to be added may be added to the system secondary keyring,
> +	  provided they are validly signed by a key already resident in the
> +	  built-in or secondary trusted keyrings.
> +
> +config IMA_BLACKLIST_KEYRING
> +	bool "Create IMA machine owner blacklist keyrings (EXPERIMENTAL)"
>  	depends on SYSTEM_TRUSTED_KEYRING
>  	depends on IMA_TRUSTED_KEYRING
>  	default n
>  	help
> -	   This option creates IMA MOK and blacklist keyrings.  IMA MOK is an
> -	   intermediate keyring that sits between .system and .ima keyrings,
> -	   effectively forming a simple CA hierarchy.  To successfully import a
> -	   key into .ima_mok it must be signed by a key which CA is in .system
> -	   keyring.  On turn any key that needs to go in .ima keyring must be
> -	   signed by CA in either .system or .ima_mok keyrings. IMA MOK is empty
> -	   at kernel boot.
> -
> -	   IMA blacklist keyring contains all revoked IMA keys.  It is consulted
> -	   before any other keyring.  If the search is successful the requested
> -	   operation is rejected and error is returned to the caller.
> +	   This option creates an IMA blacklist keyring, which contains all
> +	   revoked IMA keys.  It is consulted before any other keyring.  If
> +	   the search is successful the requested operation is rejected and
> +	   an error is returned to the caller.
> 
>  config IMA_LOAD_X509
>  	bool "Load X509 certificate onto the '.ima' trusted keyring"
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index a8539f9e060f..9aeaedad1e2b 100644
> --- a/security/integrity/ima/Makefile
> +++ b/security/integrity/ima/Makefile
> @@ -8,4 +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_MOK_KEYRING) += ima_mok.o
> +obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
> diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
> index 2988726d30d6..74a279957464 100644
> --- a/security/integrity/ima/ima_mok.c
> +++ b/security/integrity/ima/ima_mok.c
> @@ -20,23 +20,14 @@
>  #include <keys/system_keyring.h>
> 
> 
> -struct key *ima_mok_keyring;
>  struct key *ima_blacklist_keyring;
> 
>  /*
> - * Allocate the IMA MOK and blacklist keyrings
> + * Allocate the IMA blacklist keyring
>   */
>  __init int ima_mok_init(void)
>  {
> -	pr_notice("Allocating IMA MOK and blacklist keyrings.\n");
> -
> -	ima_mok_keyring = keyring_alloc(".ima_mok",
> -			      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,
> -			      restrict_link_by_builtin_trusted, NULL);
> +	pr_notice("Allocating IMA blacklist keyring.\n");
> 
>  	ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
>  				KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
> @@ -46,8 +37,8 @@ __init int ima_mok_init(void)
>  				KEY_ALLOC_NOT_IN_QUOTA,
>  				restrict_link_by_builtin_trusted, NULL);
> 
> -	if (IS_ERR(ima_mok_keyring) || IS_ERR(ima_blacklist_keyring))
> -		panic("Can't allocate IMA MOK or blacklist keyrings.");
> +	if (IS_ERR(ima_blacklist_keyring))
> +		panic("Can't allocate IMA blacklist keyring.");
> 
>  	set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags);
>  	return 0;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>