lkml.org 
[lkml]   [2020]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 09/12] evm: Allow setxattr() and setattr() if metadata digest won't change
From
Date
Hi Roberto,

"if" doesn't belong in the subject line. In this case, instead of "if
metadata ...", how about something like "for unmodified metadata"?

On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
> With the patch to allow xattr/attr operations if a portable signature
> verification fails, cp and tar can copy all xattrs/attrs so that at the
> end of the process verification succeeds.
>
> However, it might happen that xattrs/attrs are already set to the correct

^ the xattrs/attrs

> value (taken at signing time) and signature verification succeeds before
> the copy is completed. For example, an archive might contains files owned

^ has completed.

> by root and the archive is extracted by root.
>
> Then, since portable signatures are immutable, all subsequent operations
> fail (e.g. fchown()), even if the operation is legitimate (does not alter
> the current value).
>
> This patch avoids this problem by reporting successful operation to user
> space when that operation does not alter the current value of xattrs/attrs.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

<snip>

> +/*
> + * evm_xattr_change - check if passed xattr value differs from current value
> + * @dentry: pointer to the affected dentry
> + * @xattr_name: requested xattr
> + * @xattr_value: requested xattr value
> + * @xattr_value_len: requested xattr value length
> + *
> + * Check if passed xattr value differs from current value.
> + *
> + * Returns 1 if passed xattr value differs from current value, 0 otherwise.
> + */
> +static int evm_xattr_change(struct dentry *dentry, const char *xattr_name,
> + const void *xattr_value, size_t xattr_value_len)
> +{
> + char *xattr_data = NULL;
> + int rc = 0;
> +
> + if (posix_xattr_acl(xattr_name))
> + return evm_xattr_acl_change(dentry, xattr_name, xattr_value,
> + xattr_value_len);
> +
> + rc = vfs_getxattr_alloc(dentry, xattr_name, &xattr_data, 0, GFP_NOFS);
> + if (rc < 0)
> + return 1;
> +
> + if (rc == xattr_value_len)
> + rc = memcmp(xattr_value, xattr_data, rc);

This should probably be changed to crypto_memneq(). Refer to commit
613317bd212c ("EVM: Use crypto_memneq() for digest comparisons").

thanks,

Mimi

> + else
> + rc = 1;
> +
> + kfree(xattr_data);
> + return rc;
> +}
> +
>

\
 
 \ /
  Last update: 2020-09-17 15:19    [W:0.180 / U:0.544 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site