[Toybox] Squinting at LSM support in cp.

[enh]

On Fri, Jul 10, 2015 at 11:33 AM, Rob Landley <rob at landley.net> wrote:
>
>
> On 07/06/2015 11:30 PM, enh wrote:
>> 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.
>
> Yeah, that's the approach I'm taking. The --preserve=xattr stuff copies
> all extended attributes, the --preserve=context does setfscreatecon, and
> you can do both if you like.
>
>>>
>>> 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?
>
> I have to force the user writeable bit to populate read-only
> directories, and there's a parallel bit about "directory label says no
> touching its contents, but I have to put the contents in there first,
> but if the directory is world writeable other than the label then
> creating it with original permissions and adding the label later lets
> people add arbitrary stuff to it during that window..."
>
>> but that does raise an interesting question: how does one set an xattr
>> on a read-only file?
>
> We have read and write bits for file contents, and we have xattrs which
> aren't quite metadata and aren't quite file contents. Having the write
> bit control this "metadata" is retrofitted and not orthogonal.
>
> This extended attribute nonsense was invented for the macintosh "finder"
> in 1983 because if you move the icon around in its folder they wanted to
> attach the position to the file but not store it in the file contents,
> or in external .rc files, so they invented something. Unix has
> "everything is a file" and pipes and linear serializable file contents
> and the mac went "oh no, we need arbitrary complexity you interact with
> visually and no command line", and microsoft copied what the mac did and
> shoved it into OS/2 and then windows, and everybody else went "is that
> what we're doing now? Let's extend this ad infinitum".
>
> (Of course microsoft went "no, that's not enough strange sideband data,
> let's add the _registry_. Sigh. It's not unix. It's really, really not
> unix. Grrr.)
>
> I'm just trying to figure out what the right thing to do is when
> presented with obvious corner cases and race conditions. You'd think
> there would be a canned answer, but so far...
>
>>> 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 :-)
>
> Which has its own potential downsides (a label that says "you can't
> write in here" during directory creation of something that has contents).
>
> So I've got the xattr method which adds the labels after the fact and is
> thus insecure but reliable (populate the directory, _then_ label it from
> the COMEAGAIN callback), and I've got the context method that adds them
> up front and is thus secure but unreliable (locking yourself out of your
> own copy while making it).
>
> Welcome to linux security modules programming. I was trying to stay out
> of it until I figured out how to avoid these tradeoffs, but they seem to
> be inherent in the design of this nonsense. (If they're not, please tell
> me how. I'd really like to know.)
>
>>> 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.
>
> Security context is a specific subset of extended attributes. It's not
> all extended attributes.
>
>> If cp is built without xattr support, ignore this option.
>
> So always accept it and silently fail. How nice.
>
> Not doing that.
>
>> 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.
>
> Yes, but see above for exploitable race conditions with labeling things
> after they're created, especially on a system supporting inotify and
> friends.
>
>> ‘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.
>
> Squint. Stare. Ponder.
>
> Ok... I think it's saying --all once again allows xattr copies to
> silently fail, including "the new copy does not have all the security
> labels you thought it did but we're not going to tell you about it
> because pfffft security, right?"
>
> And of course --preserve=all doesn't behave quite the same as -a because
> FSF. (The man page defines -a as "same as -dR --preserve=all" and the
> info page says --preserve=all does not act like -a, but as I said,
> "because FSF".)
>
> Right. What's the _correct_ behavior?
>
> You can copy to a filesystem that doesn't support xattrs. If source
> files have xattrs that can't be applied, it should warn about it same as
> "can't set owner" does on fat. Wait, are they saying --preserve=all
> won't warn about that either?
>
> Grrr. dd if=/dev/zero of=blah.img bs=1M count=10; mkfs.vfat blah.img;
> sudo mount -o loop blah.img sub, cp -a www sub
>
> And it won't because it belongs to root and I can't chown or chmod a
> directory. (Both silently fail, this is the ubuntu host tools.)
>
> However, if I remount with -o uid=$UID and cp -a it then goes:
> cp: cannot create symbolic link ‘sub/www2/index.html’: Operation not
> permitted
>
> Does that mean the info page is full of it, or is "operation not
> permitted" (-EPERM, errno=1) different from "Operation not supported",
> the closest to which I can find is in asm-generic/errno.h:
> #define EOPNOTSUPP 95 /* Operation not supported on transport endpoint *
>
> Either way, same behavior for -a or -r --preserve=all so let's go back
> to ignoring the FSF's crazy and figuring out what the correct behavior
> should be.
>
> If you want to shut up the "can't preserve xattr" warnings, you can say
> "cp -a --no-preserve=xattr". (Note to self: implement --no-preserve.)
>
> Setting a default security context and then being unable to apply it due
> to the filesystem in question not supporting xattrs _presumably_ doesn't
> sabotage every system call made to such filesystems, so we've already
> got "silently fail" in there at the kernel level and I'm ok with that here.
>
> If you say "cp -a --no-preserve=context" you get the reliable but
> insecure copy, it will copy the xattrs and thus the security labels, but
> won't apply them at directory creation time thus avoiding "can't copy
> this hierarchy due to security labels" problem.
>
> Alas I have _no_ idea how to explain that last part concisely in the
> help text. I'm also aware --no-preserve=context seems like it should
> strip security context and if people want to argue for that instead, I'm
> going to rant about how this entire mess is badly designed and there
> should already be some xattr version of chmod -R that does a regex
> search and replace on extended attribute key/value pairs, and that if
> such a tool doesn't already exist to be copied then this LSM stuff
> doesn't get used by non-bureaucrats, and cp probably isn't that tool
> although maybe it can share infrastructure with it, and then I'd ask
> somebody to explain to me what cp --attributes-only is for exactly?

there's no chmod/chown equivalent in the literal sense, but
restorecon(8) is probably the closest. basically you don't trust
arbitrary operations, and you have a tool that applies the correct
labels based on its configuration. (so you have to get the
configuration right, but you can largely ignore arbitrary fs
modifications.)

note also that most processes can't set a file's label, and not having
a label is fail-safe in the sense of "that gets you nothing".

> We're going to have to go through this whole mess over again for "tar",
> and possibly cpio. (How does initramfs currently get xattrs? I was on an
> email thread with somebody trying to extend the cpio format last year,
> but the thread petered out without resolution as far as I know...)
>
>>> 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
>
> Yay. Thanks.
>
> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.

 1436905488.0