[PATCH v2 1/2] xattrs: Skip security.evm extended attribute

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/2] xattrs: Skip security.evm extended attribute

Stefan Berger
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


--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/2] xattrs: Properly handle security.ima extended attribute

Stefan Berger
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


--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] xattrs: Skip security.evm extended attribute

Linda A. Walsh-2
In reply to this post by Stefan Berger
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).



--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] xattrs: Skip security.evm extended attribute

Stefan Berger
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


--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] xattrs: Skip security.evm extended attribute

Wayne Davison-2
On Mon, Jan 9, 2017 at 2:31 PM, Stefan Berger <[hidden email]> wrote:
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?

The latest git version now has a modifier to the filter options that lets you specify include/exclude rules that only apply to xattr names. These can be used to control what names get read from of the source files (sender-side rules) and what names get deleted from the destination files (receiver side rules). I've just done some simple testing, and it seems to work fine.

..wayne..


--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] xattrs: Skip security.evm extended attribute

Stefan Berger
On 01/22/2017 09:15 PM, Wayne Davison wrote:
On Mon, Jan 9, 2017 at 2:31 PM, Stefan Berger <[hidden email]> wrote:
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?

The latest git version now has a modifier to the filter options that lets you specify include/exclude rules that only apply to xattr names. These can be used to control what names get read from of the source files (sender-side rules) and what names get deleted from the destination files (receiver side rules). I've just done some simple testing, and it seems to work fine.

:-) Thanks. I'll try it when I get a chance.

   Stefan


--
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html