[Toybox] integration of SMACK

[José Bollo]

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".

> 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)


 1430983872.0