[systemd-devel] [PATCH] Add AppArmor profile switching

[Lennart Poettering]

On Fri, 14.02.14 12:21, Michael Scherer (misc at zarb.org) wrote:

> This permit to switch to a specific apparmor profile when starting a daemon. This
> will result in a non operation if apparmor is disabled.
> It also add a new build requirement on libapparmor for using this feature.
> ---
>  Makefile.am                           |  7 +++++++
>  configure.ac                          | 13 +++++++++++++
>  man/systemd.exec.xml                  | 13 +++++++++++++
>  src/core/build.h                      |  8 +++++++-
>  src/core/dbus-execute.c               |  1 +
>  src/core/execute.c                    | 30 ++++++++++++++++++++++++++++++
>  src/core/execute.h                    |  2 ++
>  src/core/load-fragment-gperf.gperf.m4 |  3 ++-
>  src/shared/exit-status.c              |  3 +++
>  src/shared/exit-status.h              |  3 ++-
>  10 files changed, 80 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 79c49e6..79d355c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -776,6 +776,13 @@ libsystemd_shared_la_SOURCES += \
>  	src/shared/seccomp-util.c
>  endif
>  
> +libsystemd_shared_la_CFLAGS = \
> +       $(AM_CFLAGS) \
> +       $(APPARMOR_CFLAGS)
> +
> +libsystemd_shared_la_LIBADD = \
> +       $(APPARMOR_LIBS)
> +

Why is this in libsystemd-shared? This really should be added to the
core la, not shared... Or am I missing something?

>          SD_BUS_PROPERTY("SELinuxContext", "s", NULL, offsetof(ExecContext, selinux_context), SD_BUS_VTABLE_PROPERTY_CONST),
> +        SD_BUS_PROPERTY("AppArmorProfile", "s", NULL,
> offsetof(ExecContext, apparmor_profile),
> SD_BUS_VTABLE_PROPERTY_CONST),

Hmm, so thinking about this, we should normalize both these options and
turn the "s" signature into "(bs)", i.e. a structure made of a bool and
the label, where the bool inidcates whether a non-existing label shall
be ignored or not. We have the same split up when serializing exec
commands, and we should do that here too...

> +                        if (context->apparmor_profile && use_apparmor()) {
> +                                char* c = context->apparmor_profile;
> +                                bool ignore = false;
> +                                if (c[0] == '-') {
> +                                    c++;
> +                                    ignore = true;

Indentation 8 chars please...

> +                                }
> +
> +                                err = aa_change_onexec(context->apparmor_profile);
> +                                if (err < 0 && !ignore) {
> +                                        r = EXIT_APPARMOR;
> +                                        goto fail_child;
> +                                }
> +                        }
> +#endif
>                  }
>  
> @@ -140,6 +140,8 @@ struct ExecContext {
>  
>          char *selinux_context;
>  
> +        char *apparmor_profile;
> +

Similar as above, I'd like this to be stored normalized, i.e.:

  bool selinux_context_ignore;
  char *selinux_context;

  bool apparmor_profile_ignore;
  char *apparmor_profile;

Or similar...

Lennart

-- 
Lennart Poettering, Red Hat