On Fri, 2016-03-04 at 15:01 +0000, David Howells wrote:
> Provide a config option (IMA_PERMIT_ADD_TO_IMA_KEYRINGS) 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.

Here is an example of using the term "system trusted keyrings", which
the previous patch removes.

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

Only certificates signed by a key on the system keyring were added to
the IMA keyring, unless IMA_MOK_KEYRING was configured.  Then, the
certificate could be signed by a either a key on the system or ima_mok
keyrings.  To replicate this behavior, the default behavior should be to
only permit certificates signed by a key on the builtin keyring, unless
this new Kconfig is enabled.  Only then, permit certificates signed by a
key on either the builtin or secondary keyrings to be added to the IMA
keyring.

Mimi


> The .ima_mok keyring is removed.
> 
> Signed-off-by: David Howells <[email protected]>
> ---
> 
>  include/keys/system_keyring.h    |    9 ---------
>  security/integrity/digsig.c      |   31 +++++--------------------------
>  security/integrity/ima/Kconfig   |   36 ++++++++++++++++++++++++------------
>  security/integrity/ima/ima_mok.c |   12 ++----------
>  4 files changed, 31 insertions(+), 57 deletions(-)
> 
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index c6c05f1513f5..0764a8c1c131 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -29,22 +29,13 @@ extern int restrict_link_by_builtin_trusted(struct key 
> *keyring,
>  #endif
> 
>  #ifdef CONFIG_IMA_MOK_KEYRING
> -extern struct key *ima_mok_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;
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index ac2d39232567..aef3ac7be868 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_system_trusted(keyring, type, payload);
> -     if (ret != -ENOKEY)
> -             return ret;
> -
> -     return restrict_link_by_signature(get_ima_mok_keyring(),
> -                                       type, payload);
> -}
> +#ifdef CONFIG_IMA_PERMIT_ADD_TO_IMA_KEYRINGS
> +#define restrict_link_to_ima restrict_link_by_system_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_reject
>  #endif
> 
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int 
> siglen,
> @@ -114,7 +92,8 @@ 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..890e8aa8ccb4 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -155,23 +155,35 @@ config IMA_TRUSTED_KEYRING
> 
>          This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> 
> +config IMA_PERMIT_ADD_TO_IMA_KEYRINGS
> +     bool "Allow keys to be added to the ima keyrings, with restrictions"
> +     depends on IMA_APPRAISE
> +     depends on SYSTEM_TRUSTED_KEYRING
> +     select INTEGRITY_TRUSTED_KEYRING
> +     default n
> +     help
> +       This option permits keys to be added to the ima keyrings using
> +       add_key() or KEYCTL_LINK - with the restriction that the key must be
> +       validly signed by a key in one of the system trusted keyrings.
> +
> +       Intermediate keys between those the kernel has compiled in and the
> +       keys to be added may be added to the system secondary keyring if that
> +       is enabled - provided those are validly signed by a key in the system
> +       trusted keyrings.
> +
> +       If this is not set, attempts to add to the ima keyrings will be
> +       rejected with EPERM.
> +
>  config IMA_MOK_KEYRING
> -     bool "Create IMA machine owner keys (MOK) and blacklist keyrings"
> +     bool "Create IMA machine owner blacklist keyrings"
>       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 IMA blacklist keyring.  This 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.
> 
>  config IMA_LOAD_X509
>       bool "Load X509 certificate onto the '.ima' trusted keyring"
> diff --git a/security/integrity/ima/ima_mok.c 
> b/security/integrity/ima/ima_mok.c
> index be0da013622c..031d65a91297 100644
> --- a/security/integrity/ima/ima_mok.c
> +++ b/security/integrity/ima/ima_mok.c
> @@ -28,15 +28,7 @@ struct key *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_system_trusted, NULL);
> +     pr_notice("Allocating IMA blacklist keyrings.\n");
> 
>       ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
>                               KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
> @@ -46,7 +38,7 @@ __init int ima_mok_init(void)
>                               KEY_ALLOC_NOT_IN_QUOTA,
>                               restrict_link_by_system_trusted, NULL);
> 
> -     if (IS_ERR(ima_mok_keyring) || IS_ERR(ima_blacklist_keyring))
> +     if (IS_ERR(ima_blacklist_keyring))
>               panic("Can't allocate IMA MOK or blacklist keyrings.");
> 
>       set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


Reply via email to