Re: [PATCH] id: show SMACK security context

[Pádraig Brady]

On 04/22/2013 02:09 PM, Jarkko Sakkinen wrote:
> Hi
> 
> And thanks for replying! 
> (and sorry for double reply)
> 
> On Mon, Apr 22, 2013, at 13:15, Pádraig Brady wrote:
>> On 04/17/2013 09:30 PM, Jarkko Sakkinen wrote:
>>> Enable showing SMACK security context with -Z command-line switch.
>>> Adds dependency to libsmack.
>>> ---
>>>  configure.ac |  5 +++++
>>>  src/id.c     | 21 +++++++++++++++++----
>>>  src/local.mk |  2 +-
>>>  3 files changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 3f0c58b..e001bd8 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -276,6 +276,11 @@ if test $ac_cv_func_syslog = no; then
>>>    done
>>>  fi
>>>  
>>> +AC_ARG_WITH([smack], AS_HELP_STRING([--with-smack], [Build with SMACK]))
>>> +if test "x$with_smack" = "xyes"; then
>>> +   PKG_CHECK_MODULES([LIBSMACK], [libsmack], [AC_DEFINE([HAVE_SMACK], [1], 
>>> [FIXME])])
>>> +fi
>>> +
>>
>> I think it's best to not introduce a new dependency on pkg-config.
>> pkg-config can simplify autoconf for the developer but pushes
>> complexity to the user through env variables etc. so is best avoided.
> 
> What direction should I take on detecting dependency here? Should
> I start looking into gnulib?

Probably not.
For the moment I'd proceed as we do now for libcap for example.
You can see the configure time checks for libcap in:
http://git.savannah.gnu.org/cgit/coreutils.git/plain/m4/jm-macros.m4
Then the code can do:

#ifdef HAVE_SMACK
# include <sys/smack.h>
#endif

>>> @@ -189,14 +199,17 @@ main (int argc, char **argv)
>>>       and we're not in POSIXLY_CORRECT mode, get our context.  Otherwise,
>>>       leave the context variable alone - it has been initialized to an
>>>       invalid value that will be not displayed in print_full_info().  */
>>> -  if (selinux_enabled
>>> -      && n_ids == 0
>>> +  if (n_ids == 0
>>>        && (just_context
>>>            || (default_format && ! getenv ("POSIXLY_CORRECT"))))
>>>      {
>>>        /* Report failure only if --context (-Z) was explicitly requested.  
>>> */
>>> -      if (getcon (&context) && just_context)
>>> +      if (selinux_enabled && getcon (&context) && just_context)
>>> +        error (EXIT_FAILURE, 0, _("can't get process context"));
>>> +#ifdef HAVE_SMACK
>>> +      else if (smack_enabled && smack_new_label_from_self ((char **) 
>>> &context))
>>>          error (EXIT_FAILURE, 0, _("can't get process context"));
>>> +#endif
>>
>> So smack defers to SELinux.
>> In that case you probably don't want --with-smack above,
>> and instead auto detect smack availability.
> 
> Well, actually you couldn't have SELinux and SMACK active in the
> kernel at the same time. Kernel can only have one LSM enabled at
> a time (and you cannot switch or disable LSM). So this essentially
> detects, which one is enabled in the kernel.

Cool, so we don't need --with-smack, but should support
--disable-smack as per the libcap example mentioned above

>> On a general note, smack is a simpler MAC system to SELinux,
>> used primarily in intel derived embedded platforms.
>> I notice small patch to ls here: https://github.com/promovicz/smack-util
>> and a more encompassing busybox patch given the more embedded
>> use of smack currently.
> 
> The target that I'm working these patches is Tizen PC. The utilities
> that you are referring to are legacy. The exception is that 
> coreutils-6.9 patch is used in Tizen Mobile. In Tizen PC we are 
> happily using GPLv3 licensed code.
> 
> Instead library and command-line utils come from
> https://github.com/smack-team/smack. In core utils we want to use
> libsmack
> to detect availability of SMACK LSM because it contains reliable code to
> do
> that and it would not feel right to replicate that code. If I had needed
> code for taking label from /proc/self/attr/current, I would not added a
> new dependency but instead would have wrote routines into coreutils.

The dependency is fine, and correct.

> 
> This library is also used in our command-line utilities and RPM plug-in
> that setups security contexes.
> 
>>
>> While overloading -Z to refer to both SELinux and smack seems OK,
>> I'm worried about "writeable" options.  I.E. should cp -Z copy
>> both smack and SELinux attributes? Should cp --context=... support
>> both also, and how would that impact portability of scripts?
> 
> Well, cp does not need patching (at least we don't need it)
> because it is already able to pass extended attributes through
> on request. That's enough for our needs.
> 
>>
>> As the lines blur between embedded and traditional systems,
>> I see coreutils being more applicable to this space (from a
>> technical point of view, rather than a licensing one).
>> Do you have a practical reason for this effort in an embeded space,
>> or are you interested in smack in the "traditional" space?
> 
> I guess you got your answer in my previous comments.

thanks,
Pádraig.