Re: [PATCH v6 2/5] IMA: block writes of the security.ima xattr with unsupported algorithms

[Lakshmi Ramasubramanian <nramas@linux.microsoft.com>]

On 8/4/2021 2:20 AM, THOBY Simon wrote:
> By default, writes to the extended attributes security.ima will be
> allowed even if the hash algorithm used for the xattr is not compiled
> in the kernel (which does not make sense because the kernel would not
> be able to appraise that file as it lacks support for validating the
> hash).
> 
> Prevent writes to the security.ima xattr if the hash algorithm used is
> not available in the current kernel. Lo an audit message if such an
> operation is attempted.
Typo: Lo => Log

Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

> 
> Signed-off-by: Simon Thoby <simon.thoby@viveris.fr>
> ---
>   security/integrity/ima/ima.h          |  2 +-
>   security/integrity/ima/ima_appraise.c | 49 ++++++++++++++++++++++++++-
>   2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 2f4c20b16ad7..829478dabeeb 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -319,7 +319,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
>   void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
>   enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
>   					   enum ima_hooks func);
> -enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> +enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
>   				 int xattr_len);
>   int ima_read_xattr(struct dentry *dentry,
>   		   struct evm_ima_xattr_data **xattr_value);
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 63bec42c353f..ed1a98f6ee19 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -171,7 +171,7 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
>   	}
>   }
>   
> -enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> +enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
>   				 int xattr_len)
>   {
>   	struct signature_v2_hdr *sig;
> @@ -575,6 +575,51 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
>   		clear_bit(IMA_DIGSIG, &iint->atomic_flags);
>   }
>   
> +/**
> + * validate_hash_algo() - Block setxattr with invalid digests
> + * @dentry: object being setxattr()'ed
> + * @xattr_value: value supplied by userland for the xattr
> + * @xattr_value_len: length of xattr_value
> + *
> + * Context: called when the user tries to write the security.ima xattr.
> + * The xattr value is mapped to some hash algorithm, and this algorithm
> + * must be built in the kernel for the setxattr to be allowed.
> + *
> + * Emit an audit message when the algorithm is invalid.
> + *
> + * Return: 0 on success, else an error.
> + */
> +static int validate_hash_algo(struct dentry *dentry,
> +				   const struct evm_ima_xattr_data *xattr_value,
> +				   size_t xattr_value_len)
> +{
> +	int result = 0;
> +	char *path = NULL, *pathbuf = NULL;
> +	enum hash_algo xattr_hash_algo;
> +
> +	xattr_hash_algo = ima_get_hash_algo(xattr_value, xattr_value_len);
> +
> +	if (likely(xattr_hash_algo == ima_hash_algo ||
> +		   crypto_has_alg(hash_algo_name[xattr_hash_algo], 0, 0)))
> +		return result;
> +
> +	result = -EACCES;
> +
> +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> +	if (!pathbuf)
> +		return result;
> +
> +	path = dentry_path(dentry, pathbuf, PATH_MAX);
> +
> +	integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry), path,
> +			    "collect_data", "unavailable-hash-algorithm",
> +			    result, 0);
> +
> +	kfree(pathbuf);
> +
> +	return result;
> +}
> +
>   int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
>   		       const void *xattr_value, size_t xattr_value_len)
>   {
> @@ -595,6 +640,8 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
>   		ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
>   		if (result == 1)
>   			result = 0;
> +
> +		result = validate_hash_algo(dentry, xvalue, xattr_value_len);
>   	}
>   	return result;
>   }
>