rsync - Jan 2017 - [PATCH v2 1/2] xattrs: Skip security.evm extended attribute

The security.evm extended attribute is fully owned by the Linux kernel
and cannot be directly written from userspace. Therefore, we can always
skip it.
---
 xattrs.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/xattrs.c b/xattrs.c
index b105392..3b72e61 100644
--- a/xattrs.c
+++ b/xattrs.c
@@ -255,6 +255,9 @@ static int rsync_xal_get(const char *fname, item_list *xalp)
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 
 		/* No rsync.%FOO attributes are copied w/o 2 -X options. */
@@ -358,6 +361,9 @@ int copy_xattrs(const char *source, const char *dest)
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 
 		datum_len = 0;
@@ -828,7 +834,9 @@ void receive_xattr(int f, struct file_struct *file)
 		}
 #ifdef HAVE_LINUX_XATTRS
 		/* Non-root can only save the user namespace. */
-		if (am_root <= 0 && !HAS_PREFIX(name, USER_PREFIX)) {
+		if (am_root <= 0 &&
+		    (!HAS_PREFIX(name, USER_PREFIX) ||
+		     !strcmp(name, "security.evm"))) {
 			if (!am_root) {
 				free(ptr);
 				continue;
@@ -962,6 +970,11 @@ static int rsync_xal_set(const char *fname, item_list
*xalp,
 	for (i = 0; i < xalp->count; i++) {
 		name = rxas[i].name;
 
+#ifdef HAVE_LINUX_XATTRS
+		if (!strcmp(name, "security.evm"))
+			continue;
+#endif
+
 		if (XATTR_ABBREV(rxas[i])) {
 			/* See if the fnamecmp version is identical. */
 			len = name_len = rxas[i].name_len;
@@ -1030,6 +1043,9 @@ static int rsync_xal_set(const char *fname, item_list
*xalp,
 		if (user_only ? !HAS_PREFIX(name, USER_PREFIX)
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
+
+		if (!strcmp(name, "security.evm"))
+			continue;
 #endif
 		if (am_root < 0 && name_len > RPRE_LEN
 		 && name[RPRE_LEN] == '%' && strcmp(name, XSTAT_ATTR)
== 0)
-- 
2.7.4

This patch addresses the proper handling of the security.ima
extended attribute in the following two cases:

- The security.ima extended attribute is not writeable if its value
represents a hash, since hash values are only writeable by the kernel.
We therefore ignore errors when security.ima could not be written.

- Similarly, when the kernel creates a security.ima extended
attribute with a hash value for a new file, we don't want to erase
the security.ima xattr (erasing is possible).
---
 xattrs.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/xattrs.c b/xattrs.c
index 3b72e61..64fc84a 100644
--- a/xattrs.c
+++ b/xattrs.c
@@ -1024,10 +1024,16 @@ static int rsync_xal_set(const char *fname, item_list
*xalp,
 		}
 
 		if (sys_lsetxattr(fname, name, rxas[i].datum, rxas[i].datum_len) < 0) {
-			rsyserr(FERROR_XFER, errno,
-				"rsync_xal_set: lsetxattr(\"%s\",\"%s\")
failed",
-				full_fname(fname), name);
-			ret = -1;
+			if (!strcmp(name, "security.ima")) {
+				/* security.ima may not be writeable
+				 * if it's a hash -- skip error output
+				 */
+			} else {
+				rsyserr(FERROR_XFER, errno,
+					"rsync_xal_set: lsetxattr(\"%s\",\"%s\")
failed",
+					full_fname(fname), name);
+				ret = -1;
+			}
 		} else /* make sure caller sets mtime */
 			sxp->st.st_mtime = (time_t)-1;
 	}
@@ -1044,7 +1050,8 @@ static int rsync_xal_set(const char *fname, item_list
*xalp,
 			      : HAS_PREFIX(name, SYSTEM_PREFIX))
 			continue;
 
-		if (!strcmp(name, "security.evm"))
+		if (!strcmp(name, "security.evm") ||
+		    !strcmp(name, "security.ima"))
 			continue;
 #endif
 		if (am_root < 0 && name_len > RPRE_LEN
-- 
2.7.4

Stefan Berger wrote:
> The security.evm extended attribute is fully owned by the Linux kernel
> and cannot be directly written from userspace. Therefore, we can always
> skip it.
>   
---  (see below "...")...

    Please put this on a switch or option.

The security.evm field seems only special on Mandatory Access
systems (from https://lwn.net/Articles/449719/), and seems like it
should be copyable by root on non-Mandatory Access systems.

At the very least, a "dd" from one file system to another, would copy
it,
so the security doesn't rely on it being copied WITH the rest of
its attrs, but with the field being a check on those fields not being
modified.

....

Reading further, a better solution might be to provide a list
of extended attributes to ***exclude*** from copying, making your
patch "general case", as well as an option to ONLY copy a list of
xattrs, that match an expression or list.

I'm against hardcoding specific cases into rsync, as they won't apply
to all systems rsync runs on as well as hard-coding current trends
in integrity-measurement (which may be subject to change).

On 01/06/2017 12:27 AM, L. A. Walsh wrote:
> Stefan Berger wrote:
>> The security.evm extended attribute is fully owned by the Linux kernel
>> and cannot be directly written from userspace. Therefore, we can always
>> skip it.
> ---  (see below "...")...
>
>    Please put this on a switch or option.
>
> The security.evm field seems only special on Mandatory Access
> systems (from https://lwn.net/Articles/449719/), and seems like it
> should be copyable by root on non-Mandatory Access systems.
>
> At the very least, a "dd" from one file system to another, would
copy it,
> so the security doesn't rely on it being copied WITH the rest of
> its attrs, but with the field being a check on those fields not being
> modified.
>
> ....
>
> Reading further, a better solution might be to provide a list
> of extended attributes to ***exclude*** from copying, making your
> patch "general case", as well as an option to ONLY copy a list of
> xattrs, that match an expression or list.
libattr for example has a config file that contains descriptions on how 
to handle individual extended attributes.

http://git.savannah.gnu.org/cgit/attr.git/tree/xattr.conf

Here we list security.evm as one that cannot be written by the kernel. 
This may change in the future. So, one other general solution may be to 
ignore xattr write failures and continue.

GNU tar for example requires to use --xattrs-include=pattern to indicate 
which extended attributes to put into the archive. It also support 
--xattrs-exclude=pattern. Maybe something along those lines could work?

https://www.gnu.org/software/tar/manual/html_node/Extended-File-Attributes.html

rsync also has the issue that it may end up removing an extended 
attribute, such as security.ima, that is set by the kernel once a file 
appears but that was not read from the source file. How would we handle 
this case? Another option?

>
> I'm against hardcoding specific cases into rsync, as they won't
apply
> to all systems rsync runs on as well as hard-coding current trends
> in integrity-measurement (which may be subject to change).
Ok.

    Stefan