Re: [PATCH] fs: Return raw xattr for security.* if there is size disagreement with LSMs

On Thu, 2021-06-17 at 07:09 +0000, Roberto Sassu wrote:

quoted

From: Stefan Berger [mailto:stefanb@linux.ibm.com]
Sent: Wednesday, June 16, 2021 4:40 PM
On 6/16/21 9:22 AM, Roberto Sassu wrote:

quoted

vfs_getxattr() differs from vfs_setxattr() in the way it obtains the xattr
value. The former gives precedence to the LSMs, and if the LSMs don't
provide a value, obtains it from the filesystem handler. The latter does
the opposite, first invokes the filesystem handler, and if the filesystem
does not support xattrs, passes the xattr value to the LSMs.

The problem is that not necessarily the user gets the same xattr value that
he set. For example, if he sets security.selinux with a value not
terminated with '\0', he gets a value terminated with '\0' because SELinux
adds it during the translation from xattr to internal representation
(vfs_setxattr()) and from internal representation to xattr
(vfs_getxattr()).

Normally, this does not have an impact unless the integrity of xattrs is
verified with EVM. The kernel and the user see different values due to the
different functions used to obtain them:

kernel (EVM): uses vfs_getxattr_alloc() which obtains the xattr value from
               the filesystem handler (raw value);

user (ima-evm-utils): uses vfs_getxattr() which obtains the xattr value
                       from the LSMs (normalized value).

Maybe there should be another implementation similar to
vfs_getxattr_alloc() (or modify it) to behave like vfs_getxattr() but do
the memory allocation part so that the kernel sees what user space see
rather than modifying it with your patch so that user space now sees
something different than what it has been for years (previous
NUL-terminated SELinux xattr may not be NUL-terminated anymore)?

I'm concerned that this would break HMACs/digital signatures
calculated with raw values.

Which would happen if the LSM is not enabled (e.g. "lsm=" boot command
line option).

An alternative would be to do the EVM verification twice if the
first time didn't succeed (with vfs_getxattr_alloc() and with the
new function that behaves like vfs_getxattr()).

Unfortunately, I don't see an alternative.

thanks,

Mimi