Re: [PATCH] id: show SMACK security context

[Pádraig Brady]

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.

>  AC_CACHE_CHECK([for 3-argument setpriority function],
>    [utils_cv_func_setpriority],
>    [AC_LINK_IFELSE(
> diff --git a/src/id.c b/src/id.c
> index b5a7214..86b63b4 100644
> --- a/src/id.c
> +++ b/src/id.c
> @@ -24,6 +24,7 @@
>  #include <grp.h>
>  #include <getopt.h>
>  #include <selinux/selinux.h>
> +#include <sys/smack.h>

You can't unconditionally include here.
See the selinux-h module in gnulib that supports its unconditional include.

>  
>  #include "system.h"
>  #include "error.h"
> @@ -107,6 +108,9 @@ main (int argc, char **argv)
>  {
>    int optc;
>    int selinux_enabled = (is_selinux_enabled () > 0);
> +#ifdef HAVE_SMACK
> +  int smack_enabled = (smack_smackfs_path () != NULL);
> +#endif
>  
>    /* If true, output the list of all group IDs. -G */
>    bool just_group_list = false;
> @@ -134,10 +138,16 @@ main (int argc, char **argv)
>            break;
>  
>          case 'Z':
> -          /* politely decline if we're not on a selinux-enabled kernel. */
> +          /* politely decline if we're not on a SELinux/SMACK-enabled 
> kernel. */
> +#ifdef HAVE_SMACK
> +          if (!selinux_enabled && !smack_enabled)
> +            error (EXIT_FAILURE, 0,
> +                   _("--context (-Z) works only on an SELinux/SMACK-enabled 
> kernel"));
> +#else
>            if (!selinux_enabled)
>              error (EXIT_FAILURE, 0,
>                     _("--context (-Z) works only on an SELinux-enabled 
> kernel"));
> +#endif
>            just_context = 1;
>            break;
>  
> @@ -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.

>      }
>  
>    if (n_ids == 1)
> diff --git a/src/local.mk b/src/local.mk
> index 1ae9eff..67d6693 100644
> --- a/src/local.mk
> +++ b/src/local.mk
> @@ -227,7 +227,7 @@ src_test_LDADD += $(LIB_EACCESS)
>  copy_ldadd += $(LIB_SELINUX)
>  src_chcon_LDADD += $(LIB_SELINUX)
>  src_ginstall_LDADD += $(LIB_SELINUX)
> -src_id_LDADD += $(LIB_SELINUX)
> +src_id_LDADD += $(LIB_SELINUX) $(LIBSMACK_LIBS)
>  src_ls_LDADD += $(LIB_SELINUX)
>  src_mkdir_LDADD += $(LIB_SELINUX)
>  src_mkfifo_LDADD += $(LIB_SELINUX)
> 

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.

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?

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?

thanks,
Pádraig.