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@xxxxxxxxxx>
<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;
> +}
> +
>
