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

2014-02-21 Thread Michael Scherer 
Le vendredi 21 février 2014 à 03:48 +0100, Lennart Poettering a écrit :
 On Thu, 20.02.14 16:19, m...@zarb.org (m...@zarb.org) wrote:
 
  From: Michael Scherer m...@zarb.org
  
  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.
 
 Applied! I made some changes though, there were some missing
 bits to make sure the config hookup works correctly. I don't have any
 apparmor available though. Could you check if everything works
 correctly?

I will, I do have a opensuse VM for that, and I think intrigeri in CC,
likely does too.

 I figure the only missing bit to get apparmor up to the same level of
 support in systemd as SELinux, SMACK and IMA have would be policy
 uploading during early boot.

Yeah, but this requires call to a external binary, I was wondering is
using some unit wouldn't be enough. Upstart also do provides a way to
load a policy specificied in a job, which is maye something we could
support, like on demand module loading for selinux . 

What do people think about it ? 
( for on demand loading of profile/module )
-- 
Michael Scherer

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-21 Thread intrigeri 
Hi,

Michael Scherer wrote (21 Feb 2014 08:39:12 GMT) :
 Le vendredi 21 février 2014 à 03:48 +0100, Lennart Poettering a écrit :
 I don't have any apparmor available though. Could you check if
 everything works correctly?

 I will, I do have a opensuse VM for that, and I think intrigeri in CC,
 likely does too.

I'll happily test this (in our upcoming Tor unit file) as soon as we
have a version of systemd in Debian, that this patchset cleanly
applies to.

Cheers,
-- 
  intrigeri
  | GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
  | OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-21 Thread Lennart Poettering 
On Fri, 21.02.14 09:39, Michael Scherer (m...@zarb.org) wrote:

  Applied! I made some changes though, there were some missing
  bits to make sure the config hookup works correctly. I don't have any
  apparmor available though. Could you check if everything works
  correctly?
 
 I will, I do have a opensuse VM for that, and I think intrigeri in CC,
 likely does too.
 
  I figure the only missing bit to get apparmor up to the same level of
  support in systemd as SELinux, SMACK and IMA have would be policy
  uploading during early boot.
 
 Yeah, but this requires call to a external binary, I was wondering is
 using some unit wouldn't be enough. Upstart also do provides a way to

Well, MAC policies sound like something one really should upload at a
time where no process but PID 1 is around, so that it is guaranteed to
apply to every process on the system. Uploading it in a normal unit
loads it releatively late and in parallel to other servics.

I am happy to add code that uploads the AppArmor policy the same way we
upload SELinux, IMA, SMACK, but either this uploading must be so simple
that we can easily implement this in our own code (which is the way we
went for IMA or SMACK), or they must provide us with some library, but
doing this via invoking a binary is something that I don't want to see
in systemd upstream.

 load a policy specificied in a job, which is maye something we could

What is different from what we have now with AppArmorProfile=?

 support, like on demand module loading for selinux . 

Hmm, on-demand module loading for selinux? What do you mean by that?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-20 Thread misc 
3rd version of the patch, taking in account the feedback from Lennart. 
See 
http://lists.freedesktop.org/archives/systemd-devel/2014-January/015975.html 
and 
http://lists.freedesktop.org/archives/systemd-devel/2014-February/016916.html
for details
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-20 Thread Lennart Poettering 
On Thu, 20.02.14 16:19, m...@zarb.org (m...@zarb.org) wrote:

 From: Michael Scherer m...@zarb.org
 
 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.

Applied! I made some changes though, there were some missing
bits to make sure the config hookup works correctly. I don't have any
apparmor available though. Could you check if everything works
correctly?

I figure the only missing bit to get apparmor up to the same level of
support in systemd as SELinux, SMACK and IMA have would be policy
uploading during early boot.

Thanks!

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-20 Thread misc 
From: Michael Scherer m...@zarb.org

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   |  2 ++
 configure.ac  | 13 ++
 man/systemd.exec.xml  | 13 ++
 src/core/build.h  |  8 +-
 src/core/dbus-execute.c   | 19 ++
 src/core/execute.c| 23 
 src/core/execute.h|  3 +++
 src/core/load-fragment-gperf.gperf.m4 |  3 +++
 src/core/load-fragment.c  | 49 +++
 src/core/load-fragment.h  |  1 +
 src/shared/exit-status.c  |  3 +++
 src/shared/exit-status.h  |  3 ++-
 12 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index c71367d..4ac2122 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1016,6 +1016,7 @@ libsystemd_core_la_CFLAGS = \
$(AUDIT_CFLAGS) \
$(CAP_CFLAGS) \
$(KMOD_CFLAGS) \
+   $(APPARMOR_CFLAGS) \
$(SECCOMP_CFLAGS) \
-pthread
 
@@ -1031,6 +1032,7 @@ libsystemd_core_la_LIBADD = \
$(AUDIT_LIBS) \
$(CAP_LIBS) \
$(KMOD_LIBS) \
+   $(APPARMOR_CFLAGS) \
$(SECCOMP_LIBS)
 
 if HAVE_SECCOMP
diff --git a/configure.ac b/configure.ac
index 05ee098..2521741 100644
--- a/configure.ac
+++ b/configure.ac
@@ -385,6 +385,18 @@ if test x$enable_selinux != xno; then
 fi
 AM_CONDITIONAL(HAVE_SELINUX, [test $have_selinux = yes])
 
+have_apparmor=no
+AC_ARG_ENABLE(apparmor, AS_HELP_STRING([--disable-apparmor], [Disable optional 
AppArmor support]))
+if test x$enable_apparmor != xno; then
+PKG_CHECK_MODULES([APPARMOR], [libapparmor],
+[AC_DEFINE(HAVE_APPARMOR, 1, [Define if AppArmor is 
available]) have_apparmor=yes], have_apparmor=no)
+if test x$have_apparmor = xno -a x$enable_apparmor = xyes; then
+AC_MSG_ERROR([*** AppArmor support requested but libraries not 
found])
+fi
+fi
+AM_CONDITIONAL(HAVE_APPARMOR, [test $have_apparmor = yes])
+
+
 AC_ARG_WITH(debug-shell,
 AS_HELP_STRING([--with-debug-shell=PATH],
 [Path to debug shell binary]),
@@ -1110,6 +1122,7 @@ AC_MSG_RESULT([
 PAM: ${have_pam}
 AUDIT:   ${have_audit}
 IMA: ${have_ima}
+AppArmor:${have_apparmor}
 SELinux: ${have_selinux}
 SECCOMP: ${have_seccomp}
 SMACK:   ${have_smack}
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 7dbe05d..1983993 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -968,6 +968,19 @@
 /varlistentry
 
 varlistentry
+
termvarnameAppArmorProfile=/varname/term
+
+listitemparaTake a profile name as 
argument.
+The process executed by the unit will switch to
+this profile when started. Profiles must 
already
+be loaded in the kernel, or the unit will fail.
+This result in a non operation if AppArmor is 
not
+enabled. If prefixed by literal-/literal, 
all errors
+will be ignored.
+/para/listitem
+/varlistentry
+
+varlistentry
 termvarnameIgnoreSIGPIPE=/varname/term
 
 listitemparaTakes a boolean
diff --git a/src/core/build.h b/src/core/build.h
index c8117ed..3d7cd3e 100644
--- a/src/core/build.h
+++ b/src/core/build.h
@@ -45,6 +45,12 @@
 #define _SELINUX_FEATURE_ -SELINUX
 #endif
 
+#ifdef HAVE_APPARMOR
+#define _APPARMOR_FEATURE_ +APPARMOR
+#else
+#define _APPARMOR_FEATURE_ -APPARMOR
+#endif
+
 #ifdef HAVE_IMA
 #define _IMA_FEATURE_ +IMA
 #else
@@ -87,4 +93,4 @@
 #define _SECCOMP_FEATURE_ -SECCOMP
 #endif
 
-#define SYSTEMD_FEATURES _PAM_FEATURE_   _LIBWRAP_FEATURE_   
_AUDIT_FEATURE_   _SELINUX_FEATURE_   _IMA_FEATURE_   _SYSVINIT_FEATURE_ 
  _LIBCRYPTSETUP_FEATURE_   _GCRYPT_FEATURE_   _ACL_FEATURE_   
_XZ_FEATURE_   _SECCOMP_FEATURE_
+#define SYSTEMD_FEATURES _PAM_FEATURE_   _LIBWRAP_FEATURE_   
_AUDIT_FEATURE_   _SELINUX_FEATURE_   _IMA_FEATURE_   _SYSVINIT_FEATURE_ 
  _LIBCRYPTSETUP_FEATURE_   _GCRYPT_FEATURE_   _ACL_FEATURE_   
_XZ_FEATURE_   _SECCOMP_FEATURE_   _APPARMOR_FEATURE_
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 41dbbab..935c62b 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -482,6 +482,24 @@ static int

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

2014-02-14 Thread Michael Scherer 
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)
+
 # 
--
 noinst_LTLIBRARIES += \
libsystemd-units.la
diff --git a/configure.ac b/configure.ac
index 48d63e8..38dfa91 100644
--- a/configure.ac
+++ b/configure.ac
@@ -382,6 +382,18 @@ if test x$enable_selinux != xno; then
 fi
 AM_CONDITIONAL(HAVE_SELINUX, [test $have_selinux = yes])
 
+have_apparmor=no
+AC_ARG_ENABLE(apparmor, AS_HELP_STRING([--disable-apparmor], [Disable optional 
AppArmor support]))
+if test x$enable_apparmor != xno; then
+PKG_CHECK_MODULES([APPARMOR], [libapparmor],
+[AC_DEFINE(HAVE_APPARMOR, 1, [Define if AppArmor is 
available]) have_apparmor=yes], have_apparmor=no)
+if test x$have_apparmor = xno -a x$enable_apparmor = xyes; then
+AC_MSG_ERROR([*** AppArmor support requested but libraries not 
found])
+fi
+fi
+AM_CONDITIONAL(HAVE_APPARMOR, [test $have_apparmor = yes])
+
+
 AC_ARG_WITH(debug-shell,
 AS_HELP_STRING([--with-debug-shell=PATH],
 [Path to debug shell binary]),
@@ -1104,6 +1116,7 @@ AC_MSG_RESULT([
 PAM: ${have_pam}
 AUDIT:   ${have_audit}
 IMA: ${have_ima}
+AppArmor:${have_apparmor}
 SELinux: ${have_selinux}
 SECCOMP: ${have_seccomp}
 SMACK:   ${have_smack}
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 01356e4..262638d 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -968,6 +968,19 @@
 /varlistentry
 
 varlistentry
+
termvarnameAppArmorProfile=/varname/term
+
+listitemparaTake a profile name as 
argument.
+The process executed by the unit will switch to
+this profile when started. Profiles must 
already
+be loaded in the kernel, or the unit will fail.
+This result in a non operation if AppArmor is 
not
+enabled. If prefixed by literal-/literal, 
all errors
+will be ignored.
+/para/listitem
+/varlistentry
+
+varlistentry
 termvarnameIgnoreSIGPIPE=/varname/term
 
 listitemparaTakes a boolean
diff --git a/src/core/build.h b/src/core/build.h
index f04f03f..3d7cd3e 100644
--- a/src/core/build.h
+++ b/src/core/build.h
@@ -45,6 +45,12 @@
 #define _SELINUX_FEATURE_ -SELINUX
 #endif
 
+#ifdef HAVE_APPARMOR
+#define _APPARMOR_FEATURE_ +APPARMOR
+#else
+#define _APPARMOR_FEATURE_ -APPARMOR
+#endif
+
 #ifdef HAVE_IMA
 #define _IMA_FEATURE_ +IMA
 #else
@@ -87,4 +93,4 @@
 #define _SECCOMP_FEATURE_ -SECCOMP
 #endif
 
-#define SYSTEMD_FEATURES _PAM_FEATURE_   _LIBWRAP_FEATURE_   
_AUDIT_FEATURE_   _SELINUX_FEATURE_   _IMA_FEATURE_   _SYSVINIT_FEATURE_ 
  _LIBCRYPTSETUP_FEATURE_   _GCRYPT_FEATURE_   _ACL_FEATURE_   
_XZ_FEATURE_ _SECCOMP_FEATURE_
+#define SYSTEMD_FEATURES _PAM_FEATURE_   _LIBWRAP_FEATURE_   
_AUDIT_FEATURE_   _SELINUX_FEATURE_   _IMA_FEATURE_   _SYSVINIT_FEATURE_ 
  _LIBCRYPTSETUP_FEATURE_   _GCRYPT_FEATURE_   _ACL_FEATURE_   
_XZ_FEATURE_   _SECCOMP_FEATURE_   _APPARMOR_FEATURE_
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index a62f517..286ae7d 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -524,6 +524,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
 SD_BUS_PROPERTY(SameProcessGroup, b, bus_property_get_bool, 
offsetof(ExecContext, same_pgrp), SD_BUS_VTABLE_PROPERTY_CONST),

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

2014-02-14 Thread Lennart Poettering 
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?

  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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-14 Thread Michael Scherer 
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

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

2014-02-14 Thread Michael Scherer 
Le vendredi 14 février 2014 à 14:05 +0100, Michael Scherer a écrit :
 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:

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 ?

Mhh no,
you want 1 single property, but with a struct rather than 1 string,
forget about that, I misread.

-- 
Michael Scherer

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-02-14 Thread Lennart Poettering 
On Fri, 14.02.14 14:05, Michael Scherer (m...@zarb.org) wrote:

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 ?

Nope. Just one property as struct.

 Also, SELinuxContext would be the context without the leading '-',
 correct ?

Correct. The - would basically just be a way to denote this in the
unit file, but never stored like that internally nor exported over the bus.

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

Yupp, there would be two parse functions that can handle
this. (internally, they can probably just invoke a single function that
takes pointers to both the char* and the bool variable)

 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 ?

Feel free to just send them in a series.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel