Re: [PATCH v0] KEYS: Security LSM Hook for key_create_or_update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/18/2019 12:53 PM, Lakshmi Ramasubramanian wrote:
> Problem Statement:
> key_create_or_update function currently does not have
> a security LSM hook. The hook is needed to allow security
> subsystems to use key create or update information.

What security module(s) do you expect to use this?

> security_key_alloc LSM hook that is currently available is not
> providing enough information about the key (the key payload,
> the target keyring, etc.). Also, this LSM hook is only available
> for key create.
>
> Changes made:
> Adding a new LSM hook for key key_create_or_update,
> security_key_create_or_update, which is called after
>    => A newly created key is instantiated and linked to the target
>       keyring (__key_instantiate_and_link).
>    => An existing key is updated with a new payload (__key_update)
>
> security_key_create_or_update is passed the target keyring, key,
> cred, key creation flags, and a boolean flag indicating whether
> the key was newly created or updated.
>
> Security subsystems can use this hook for handling key create or update.
> For example, IMA subsystem can measure the key when it is created or
> updated.

IMA is not a Linux Security Module.

> Testing performed:
>   * Booted the kernel with this change.
>   * Executed keyctl tests from the Linux Test Project (LTP)
>   * Added a new key to a keyring and verified "key create" code path.
>     => In this case added a key to builtin_trusted_keys keyring.
>   * Added the same key again and verified "key update" code path.
>     => Add the same key to builtin_trusted_keys keyring.
>     => find_key_to_update found the key and LSM hook was
>        called for key update (create flag set to false).
>   * Forced the LSM hook (security_key_create_or_update) to
>     return an error and verified that the key was not added to
>     the keyring ("keyctl list <keyring>" does not list the key).
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
> ---
>  include/linux/lsm_hooks.h | 13 +++++++
>  include/linux/security.h  | 10 +++++
>  security/keys/key.c       | 78 ++++++++++++++++++++++++++++++++++-----
>  security/security.c       |  8 ++++

You don't have a security module that provides this hook.
We don't accept interfaces without users.

>  4 files changed, 100 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index df1318d85f7d..2f2e95df62f3 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1066,6 +1066,15 @@
>   *
>   * Security hooks affecting all Key Management operations
>   *
> + * @key_create_or_update:
> + *      Notification of key create or update.
> + *      @keyring points to the keyring to which the key belongs
> + *      @key points to the key being created or updated
> + *      @cred current cred
> + *      @flags is the allocation flags
> + *      @create flag set to true if the key was created.
> + *              flag set to false if the key was updated.
> + *      Return 0 if permission is granted, -ve error otherwise.
>   * @key_alloc:
>   *	Permit allocation of a key and assign security data. Note that key does
>   *	not have a serial number assigned at this point.
> @@ -1781,6 +1790,9 @@ union security_list_options {
>  
>  	/* key management security hooks */
>  #ifdef CONFIG_KEYS
> +	int (*key_create_or_update)(struct key *keyring, struct key *key,
> +				    const struct cred *cred,
> +				    unsigned long flags, bool create);
>  	int (*key_alloc)(struct key *key, const struct cred *cred,
>  				unsigned long flags);
>  	void (*key_free)(struct key *key);
> @@ -2026,6 +2038,7 @@ struct security_hook_heads {
>  	struct hlist_head xfrm_decode_session;
>  #endif	/* CONFIG_SECURITY_NETWORK_XFRM */
>  #ifdef CONFIG_KEYS
> +	struct hlist_head key_create_or_update;
>  	struct hlist_head key_alloc;
>  	struct hlist_head key_free;
>  	struct hlist_head key_permission;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 5f7441abbf42..27e1c0a3057b 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1672,6 +1672,9 @@ static inline int security_path_chroot(const struct path *path)
>  #ifdef CONFIG_KEYS
>  #ifdef CONFIG_SECURITY
>  
> +int security_key_create_or_update(struct key *keyring, struct key *key,
> +				  const struct cred *cred,
> +				  unsigned long flags, bool create);
>  int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags);
>  void security_key_free(struct key *key);
>  int security_key_permission(key_ref_t key_ref,
> @@ -1680,6 +1683,13 @@ int security_key_getsecurity(struct key *key, char **_buffer);
>  
>  #else
>  
> +int security_key_create_or_update(struct key *keyring, struct key *key,
> +				  const struct cred *cred,
> +				  unsigned long flags, bool create)
> +{
> +	return 0;
> +}
> +
>  static inline int security_key_alloc(struct key *key,
>  				     const struct cred *cred,
>  				     unsigned long flags)
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 764f4c57913e..b913feaf196e 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -781,7 +781,7 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
>  }
>  
>  /**
> - * key_create_or_update - Update or create and instantiate a key.
> + * __key_create_or_update - Update or create and instantiate a key.
>   * @keyring_ref: A pointer to the destination keyring with possession flag.
>   * @type: The type of key.
>   * @description: The searchable description for the key.
> @@ -789,6 +789,8 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
>   * @plen: The length of @payload.
>   * @perm: The permissions mask for a new key.
>   * @flags: The quota flags for a new key.
> + * @create: Set to true if the key was newly created.
> + *          Set to false if the key was updated.
>   *
>   * Search the destination keyring for a key of the same description and if one
>   * is found, update it, otherwise create and instantiate a new one and create a
> @@ -805,13 +807,14 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
>   * On success, the possession flag from the keyring ref will be tacked on to
>   * the key ref before it is returned.
>   */
> -key_ref_t key_create_or_update(key_ref_t keyring_ref,
> -			       const char *type,
> -			       const char *description,
> -			       const void *payload,
> -			       size_t plen,
> -			       key_perm_t perm,
> -			       unsigned long flags)
> +static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
> +					const char *type,
> +					const char *description,
> +					const void *payload,
> +					size_t plen,
> +					key_perm_t perm,
> +					unsigned long flags,
> +					bool *create)
>  {
>  	struct keyring_index_key index_key = {
>  		.description	= description,
> @@ -936,6 +939,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
>  		goto error_link_end;
>  	}
>  
> +	*create = true;
>  	key_ref = make_key_ref(key, is_key_possessed(keyring_ref));
>  
>  error_link_end:
> @@ -948,7 +952,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
>  error:
>  	return key_ref;
>  
> - found_matching_key:
> +found_matching_key:
>  	/* we found a matching key, so we're going to try to update it
>  	 * - we can drop the locks first as we have the key pinned
>  	 */
> @@ -964,9 +968,65 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
>  		}
>  	}
>  
> +	*create = false;
>  	key_ref = __key_update(key_ref, &prep);
>  	goto error_free_prep;
>  }
> +
> +/**
> + * key_create_or_update - Update or create and instantiate a key.
> + * @keyring_ref: A pointer to the destination keyring with possession flag.
> + * @type: The type of key.
> + * @description: The searchable description for the key.
> + * @payload: The data to use to instantiate or update the key.
> + * @plen: The length of @payload.
> + * @perm: The permissions mask for a new key.
> + * @flags: The quota flags for a new key.
> + *
> + * Calls the internal function __key_create_or_update.
> + * If successful calls the security LSM hook.
> + */
> +key_ref_t key_create_or_update(key_ref_t keyring_ref,
> +			       const char *type,
> +			       const char *description,
> +			       const void *payload,
> +			       size_t plen,
> +			       key_perm_t perm,
> +			       unsigned long flags)
> +{
> +	key_ref_t key_ref;
> +	struct key *keyring, *key = NULL;
> +	int ret = 0;
> +	bool create = false;
> +
> +	key_ref = __key_create_or_update(keyring_ref, type, description,
> +					 payload, plen, perm, flags,
> +					 &create);
> +	if (IS_ERR(key_ref))
> +		goto out;
> +
> +	keyring = key_ref_to_ptr(keyring_ref);
> +	key = key_ref_to_ptr(key_ref);
> +
> +	/* let the security module know about
> +	 * the created or updated key.
> +	 */
> +	ret = security_key_create_or_update(keyring, key,
> +					    current_cred(),
> +					    flags, create);
> +	if (ret < 0)
> +		goto security_error;
> +	else
> +		goto out;
> +
> +security_error:
> +	key_unlink(keyring, key);
> +	key_put(key);
> +	key_ref = ERR_PTR(ret);
> +
> +out:
> +	return key_ref;
> +}
>  EXPORT_SYMBOL(key_create_or_update);
>  
>  /**
> diff --git a/security/security.c b/security/security.c
> index 250ee2d76406..fc1e4984fb53 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2280,6 +2280,14 @@ EXPORT_SYMBOL(security_skb_classify_flow);
>  
>  #ifdef CONFIG_KEYS
>  
> +int security_key_create_or_update(struct key *keyring, struct key *key,
> +				  const struct cred *cred,
> +				  unsigned long flags, bool create)
> +{
> +	return call_int_hook(key_create_or_update, 0,
> +			     keyring, key, cred, flags, create);
> +}
> +
>  int security_key_alloc(struct key *key, const struct cred *cred,
>  		       unsigned long flags)
>  {



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux