[Toybox] Squinting at LSM support in cp.

[Rob Landley]

So now I'm up to the cp -p security blanket label support, and it's fiddly.

If LSM means anything, then we don't want a race window between creating
a file and labeling the file. The optimal thing to do is create the file
with permissions 000 (or 700, I.E. just us), apply the label to the
filehandle, then chmod the file to the normal permissions. (If you
create a readable/writeable file people can fiddle with it before the
label blocks them. If you don't do it via filehandle they can swap in a
different file before you apply the label.)

Except... can the label prevent us from doing the chmod? In theory, once
we stamp a label on the file it can prevent arbitrary further actions
from occurring to the file. (I dunno what the security modules are
actually doing.)

The alternative (and what I've been using) is to set the file creation
context so the original open applies the label (presumably atomically).
But Jose said there's something like a sticky bit on parent directories,
where the context can override the default label (but not manually set
labels on specific files).

In addition, the cp patch I got a while back is based on the idea you
can have buckets of labels on a file because once you've opened this can
of worms infinite complexity will spiral out:

diff --git a/toys/posix/cp.c b/toys/posix/cp.c
index 2959f67..5be9f3c 100644
--- a/toys/posix/cp.c
+++ b/toys/posix/cp.c
@@ -86,6 +86,8 @@ config INSTALL
 #define FOR_cp
 #include "toys.h"

+#include <sys/xattr.h>
+
 GLOBALS(
   // install's options
   char *group;
@@ -249,6 +251,61 @@ int cp_node(struct dirtree *try)
           fdout = openat(cfd, catch, O_RDWR|O_CREAT|O_TRUNC,
try->st.st_mode);
           if (fdout >= 0) {
             xsendfile(fdin, fdout);
+
+            // Duplicate xattrs for new file
+            if (flags & FLAG_p) {
+              ssize_t buffer_len = flistxattr(fdin, NULL, 0);
+
+              if (buffer_len > 0) {
+                char *xattrs_buffer = malloc(buffer_len);
+
+                // If we don't succeed, then we don't copy the xattrs
+                if (xattrs_buffer != NULL) {
+                  buffer_len = flistxattr(fdin, xattrs_buffer, buffer_len);
+                  char *tmp_buffer = xattrs_buffer;
+
+                  while (buffer_len > 0) {
+                    int len = strlen(tmp_buffer)+1;
+                    char *xattr_value = NULL;
+                    //Fetch size of xattr value
+                    ssize_t xattr_len = fgetxattr(fdin, tmp_buffer,
xattr_value, 0);
+
+                    if (xattr_len == -1)
+                      goto exit_xattr;
+
+                    errno = 0;
+                    xattr_value = malloc(xattr_len);
+
+                    if (xattr_value == NULL)
+                      goto exit_xattr;
+
+                    xattr_len = fgetxattr(fdin, tmp_buffer,
xattr_value, xattr_len);
+
+                    if (xattr_len == -1) {
+                      free(xattr_value);
+                      goto exit_xattr;
+                    }
+
+                    int ret = fsetxattr(fdout, tmp_buffer, xattr_value,
xattr_len, 0);
+                    free(xattr_value);
+
+                    if (ret == -1) {
+                      //Something failed, we cannot fix it anyway,
hence we break the loop
+                      fprintf(stderr, "cp: cannot apply extended
attributes, \
+                        probably not supported by target filesystem");
+                      break;
+                    };
+
+                    tmp_buffer += len;
+                    buffer_len -= len;
+                  }
+
+exit_xattr:
+                free(xattrs_buffer);
+                errno = 0;
+                }
+              }
+            }
             err = 0;
           }
           close(fdin);

But later I also have this pending patch:

http://lists.landley.net/pipermail/toybox-landley.net/2015-May/004184.html

Both of those are only copying xattrs for _files_, not for devices or
for directories. Can they not be applied to those? I don't understand
what the correct behavior is here.

Since this is operating on raw xattrs instead of "security" labels, I'm
guessing this is correct for both smack and selinux on the theory that
it's using linux system calls instead of their respective wrapper
infrastructure?

I haven't got an flistxattr man page (of course not, even though grep on
the kernel source says it's a linux system call, that would imply
somebody cared about it enough to document it as if it's a thing that
gets used). It's also not one of the functions I currently have wrapped
in lib/lsm.h. But hey, I can syscall(_NR_thingy) it if necessary. If I
can figure out what the correct behavior here is...

(Also, I'm adding a config symbol of some kind. Unconditional include of
xattr.h makes me a bit uncomfortable here. Trying to figure out what the
least disruptive version of _that_ looks like too...)

Rob

 1435645098.0