[Toybox] Squinting at LSM support in cp.
enh
enh at google.com
Mon Jul 6 21:30:18 PDT 2015
On Mon, Jun 29, 2015 at 11:18 PM, Rob Landley <rob at landley.net> wrote:
>
> 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.)
i think you want to setfscreatecon before you create the file.
>
> 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.)
chmod? wouldn't you pass the right mode to open?
but that does raise an interesting question: how does one set an xattr
on a read-only file?
>
> The alternative (and what I've been using) is to set the file creation
> context so the original open applies the label (presumably atomically).
ah, yes, that sounds right :-)
>
> 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?
they're all the same. from the GNU man page^W^Winfo nonsense
(https://www.gnu.org/software/coreutils/manual/html_node/cp-invocation.html):
‘context’
Preserve SELinux security context of the file, or fail with full diagnostics.
‘xattr’
Preserve extended attributes of the file, or fail with full
diagnostics. If cp is built without xattr support, ignore this option.
If SELinux context, ACLs or Capabilities are implemented using xattrs,
they are preserved implicitly by this option as well, i.e., even
without specifying --preserve=mode or --preserve=context.
‘all’
Preserve all file attributes. Equivalent to specifying all of the
above, but with the difference that failure to preserve SELinux
security context or extended attributes does not change cp’s exit
status. In contrast to -a, all but ‘Operation not supported’ warnings
are output.
>
> 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...
http://man7.org/linux/man-pages/man2/listxattr.2.html
> (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
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
1436243418.0
More information about the Toybox
mailing list