[Toybox] integration of SMACK

[enh]

On Thu, May 7, 2015 at 12:31 AM, José Bollo <jobol at nonadev.net> wrote:

> Le mercredi 06 mai 2015 à 13:19 -0700, James McMechan a écrit :
> > Looking at the fixes:
>
> Hello all, Hello Jim,
>
> First of all thank you for you fiddly review ^^
>
> > toys/other/stat.c for group name outputting the user name seems like a
> good catch
> >
> > toys/posix/cp.c you are treating -p like --preserve=all not like -p
> which only does mode,ownership,timestamps
> > according to the man page -p does not copy over xattrs at all, which
> seems kind of odd...
>
> You are absolutely right. I guess that Krysztof Sasiak that made the
> initial commit has not expected to enter in the deep logic of arguments
> and just added to functionnality here as is, breaking the common
> semantic.
>
> To be correct, from my cp man page, I see that "-p" is same as
> "--preserve=mode,ownership,timestamps" and that "--preserve[=ATTR_LIST]"
> accepts the attributes: "context",  "links", "xattr", "mode",
> "ownership", "timestamps", and  "all".


coincidentally, someone on Android asked for cp --preserve this week.
internal bug 21121352. i'll get around to that at some point if no one
beats me to it (but not before Google I/O).


>
> > toys/posix/id.c seems to lose the TOYBOX_USR flag which I think insures
> it is in /usr/bin like the regular version.
>
> Good catch. thx.
>
> > this is the second case where your are using
> > (TOYBOX_SELINUX || TOYBOX_SMACK)
> > perhaps you should add a hidden symbol like TOYBOX_SECURITY that could
> have all the various versions || together and use that in place of doing it
> by hand each time for the ??_SECURITY symbol.
>
> This is a good idea. But I'm affraid that after that, persnickety people
> will then replace TOYBOX_SELINUX with TOYBOX_SECURITY_SELINUX and
> TOYBOX_SMACK by TOYBOX_SECURITY_SMACK. But why being frighten by only
> few hours of work?
>
> > also you pulled the
> > if (CFG_TOYBOX_FREE) free(context);
> > up into the two if statements rather leaving it after both like it was.
> It should work as a good compiler will do tail combining but just having it
> sitting at the end is both clearer and shorter because both cases fall
> through to the same code.
>
> Size versus speed (or the opposite).
>
> > The lines 151-169 look like a extra string e.g. "LSM Smack Disabled" vs
> "SELinux Disabled" and a slight shuffling of the if statements could make
> that much shorter both halves seem to be almost identical...
>
> It is advocating to founding a security module in lib. That was
> suggested by Elliott Hughes.
>
> Why not. I am seeing now a good reason to make it. From the viewpoint of
> the security, our expect was that toybox is compiled:
>  - without security stuff
>  - with security SELinux (android)
>  - with security Smack (tizen)
>
> But it may only be true after answering this questions:
>
> - What other security model will enter in toybox? none?
> - Should toybox be compiled in an agnostic way that adapts itself on the
> kernel it runs on?
>
> > toybox/posix/mkdir.c does the normal mkdir really reset the process
> label for all future use when you use -p & -Z ? should it set it back to
> normal when the command is done? or is that per process context that
> vanishes on exit?
>
> good catch. Yes, it vanish on the process die but it would forbid to use
> it in library.
>
> >               toys/posix/mkfifo.c lost TOYBOX_USR
>
> thx
>
> > I could not find any way to comment on git hub directly though.
>
> Hum! Arrrr... maybe because it is not a pull request...
>
> Best regards
> José Bollo
>
> > Jim
> (snip)
>
>


-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20150513/bf254c27/attachment-0003.htm>