Le vendredi 14 février 2014 à 12:31 +0100, Lennart Poettering a écrit :
> On Fri, 14.02.14 12:21, Michael Scherer (m...@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?
It did also look weird this morning when I reviewed the patch, but I
trusted more my past-self than my current-self, I will take a look and
send another patch if needed.
> > 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...
So, you want a 2nd property SELinuxcontextIgnore/AppArmorProfileIgnore
that would be True when SELinuxContext/AppArmorProfile is prefixed by
'-', or also when SELinux/AppArmor is disabled ?
Also, SELinuxContext would be the context without the leading '-',
correct ?
> > + if (context->apparmor_profile && use_apparmor()) {
> > + char* c = context->apparmor_profile;
> > + bool ignore = false;
> > + if (c[0] == '-') {
> > + c++;
> > + ignore = true;
>
> Indentation 8 chars please...
Will do
> > + }
> > +
> > + 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...
So that would requires a custom change to load-fragment.c, so I guess
that's a separate task as we need to apply that to selinuxcontext.
So I will provides the change for SELinuxContext, then once accepted,
send a v3 of the apparmor patch with the code following the "style" of
SELinuxContext, is this ok ?
--
Michael Scherer
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
