Re: [systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack

2013-05-11 Thread Kok, Auke-jan H
On Wed, May 8, 2013 at 8:20 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Wed, May 08, 2013 at 11:42:34AM -0700, Kok, Auke-jan H wrote:
 On Tue, May 7, 2013 at 5:29 AM, Karol Lewandowski
 k.lewando...@samsung.com wrote:
  On 05/07/2013 01:32 PM, Lennart Poettering wrote:
  On Tue, 07.05.13 13:21, Karol Lewandowski (k.lewando...@samsung.com) 
  wrote:
 
  Heya,
 
  Hmm, does that directory always exist? Or only if AppArmor is actually
  runtime enabled?
 
  /sys/fs/smackfs is only registered when smack lsm is actually enabled:
 

  https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/security/smack/smackfs.c?id=e93072374112db9dc86635934ee761249be28370#n2179
 
  I.e. this check should ideally only return true if SMACK is not only
  built into the kernel, but actually really enabled during
  runtime. That's what the SELinux check does and what the most useful
  semantics are.
 
  Ok, I see that libselinux will consider selinux to be disabled also when
  policy is not loaded:
 

  http://userspace.selinuxproject.org/trac/browser/libselinux/src/enabled.c#L12
 
  I guess we could do something similar (inspect /proc/self/attr/current)
  but honestly, I don't think it's really needed.  Rafał, could you correct 
  me
  if I'm wrong?

 smack is different as in that it can function without any loaded
 policies, so looking at policies isn't the right thing for smack. So
 likely looking at the presence of smackfs is exactly the same as
 looking at the preference of /proc/self/attr/current, except the
 latter is more complex, so less desirable imho.
 Applied, with a commit message based on this explanation.

FYI, I just added similar code for IMA allowing ConditionSecurity=ima.

I will take the AR to ask our Intel security folks if we don't need to
do more - as in
verify that IMA actually has a policy loaded, but the policy interface for IMA
is write-only, so there is no way to find out if a policy was
previously written.

Cheers,

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


Re: [systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack

2013-05-08 Thread Zbigniew Jędrzejewski-Szmek
On Wed, May 08, 2013 at 11:42:34AM -0700, Kok, Auke-jan H wrote:
 On Tue, May 7, 2013 at 5:29 AM, Karol Lewandowski
 k.lewando...@samsung.com wrote:
  On 05/07/2013 01:32 PM, Lennart Poettering wrote:
  On Tue, 07.05.13 13:21, Karol Lewandowski (k.lewando...@samsung.com) wrote:
 
  Heya,
 
  Hmm, does that directory always exist? Or only if AppArmor is actually
  runtime enabled?
 
  /sys/fs/smackfs is only registered when smack lsm is actually enabled:
 

  https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/security/smack/smackfs.c?id=e93072374112db9dc86635934ee761249be28370#n2179
 
  I.e. this check should ideally only return true if SMACK is not only
  built into the kernel, but actually really enabled during
  runtime. That's what the SELinux check does and what the most useful
  semantics are.
 
  Ok, I see that libselinux will consider selinux to be disabled also when
  policy is not loaded:
 

  http://userspace.selinuxproject.org/trac/browser/libselinux/src/enabled.c#L12
 
  I guess we could do something similar (inspect /proc/self/attr/current)
  but honestly, I don't think it's really needed.  Rafał, could you correct me
  if I'm wrong?
 
 smack is different as in that it can function without any loaded
 policies, so looking at policies isn't the right thing for smack. So
 likely looking at the presence of smackfs is exactly the same as
 looking at the preference of /proc/self/attr/current, except the
 latter is more complex, so less desirable imho.
Applied, with a commit message based on this explanation.

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


[systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack

2013-05-07 Thread Karol Lewandowski
Signed-off-by: Karol Lewandowski k.lewando...@samsung.com

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index 49103da..256c813 100644
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -984,8 +984,9 @@
 may be used to check whether the given
 security module is enabled on the
 system.  Currently the only recognized
-values are varnameselinux/varname
-and varnameapparmor/varname.
+values are varnameselinux/varname,
+varnameapparmor/varname and
+varnamesmack/varname.
 The test may be negated by prepending
 an exclamation
 mark./para
diff --git a/src/core/condition.c b/src/core/condition.c
index 4aa5530..16cae6d 100644
--- a/src/core/condition.c
+++ b/src/core/condition.c
@@ -164,6 +164,8 @@ static bool test_security(const char *parameter) {
 #endif
if (streq(parameter, apparmor))
return access(/sys/kernel/security/apparmor/, F_OK) == 0;
+   if (streq(parameter, smack))
+   return access(/sys/fs/smackfs, F_OK) == 0;
 return false;
 }
 
-- 
1.7.10.4

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


Re: [systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack

2013-05-07 Thread Lennart Poettering
On Tue, 07.05.13 13:21, Karol Lewandowski (k.lewando...@samsung.com) wrote:

Heya,

Hmm, does that directory always exist? Or only if AppArmor is actually
runtime enabled?

I.e. this check should ideally only return true if SMACK is not only
built into the kernel, but actually really enabled during
runtime. That's what the SELinux check does and what the most useful
semantics are.

 Signed-off-by: Karol Lewandowski k.lewando...@samsung.com
 
 diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
 index 49103da..256c813 100644
 --- a/man/systemd.unit.xml
 +++ b/man/systemd.unit.xml
 @@ -984,8 +984,9 @@
  may be used to check whether the given
  security module is enabled on the
  system.  Currently the only recognized
 -values are varnameselinux/varname
 -and varnameapparmor/varname.
 +values are varnameselinux/varname,
 +varnameapparmor/varname and
 +varnamesmack/varname.
  The test may be negated by prepending
  an exclamation
  mark./para
 diff --git a/src/core/condition.c b/src/core/condition.c
 index 4aa5530..16cae6d 100644
 --- a/src/core/condition.c
 +++ b/src/core/condition.c
 @@ -164,6 +164,8 @@ static bool test_security(const char *parameter) {
  #endif
   if (streq(parameter, apparmor))
   return access(/sys/kernel/security/apparmor/, F_OK) == 0;
 + if (streq(parameter, smack))
 + return access(/sys/fs/smackfs, F_OK) == 0;
  return false;
  }
  


Lennart

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


Re: [systemd-devel] [PATCH] condition, man: Add support for ConditionSecurity=smack

2013-05-07 Thread Karol Lewandowski
On 05/07/2013 01:32 PM, Lennart Poettering wrote:
 On Tue, 07.05.13 13:21, Karol Lewandowski (k.lewando...@samsung.com) wrote:
 
 Heya,
 
 Hmm, does that directory always exist? Or only if AppArmor is actually
 runtime enabled?

/sys/fs/smackfs is only registered when smack lsm is actually enabled:

  
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/security/smack/smackfs.c?id=e93072374112db9dc86635934ee761249be28370#n2179

 I.e. this check should ideally only return true if SMACK is not only
 built into the kernel, but actually really enabled during
 runtime. That's what the SELinux check does and what the most useful
 semantics are.

Ok, I see that libselinux will consider selinux to be disabled also when
policy is not loaded:

  http://userspace.selinuxproject.org/trac/browser/libselinux/src/enabled.c#L12

I guess we could do something similar (inspect /proc/self/attr/current)
but honestly, I don't think it's really needed.  Rafał, could you correct me
if I'm wrong?

Cheers

 
 Signed-off-by: Karol Lewandowski k.lewando...@samsung.com

 diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
 index 49103da..256c813 100644
 --- a/man/systemd.unit.xml
 +++ b/man/systemd.unit.xml
 @@ -984,8 +984,9 @@
  may be used to check whether the given
  security module is enabled on the
  system.  Currently the only recognized
 -values are varnameselinux/varname
 -and varnameapparmor/varname.
 +values are varnameselinux/varname,
 +varnameapparmor/varname and
 +varnamesmack/varname.
  The test may be negated by prepending
  an exclamation
  mark./para
 diff --git a/src/core/condition.c b/src/core/condition.c
 index 4aa5530..16cae6d 100644
 --- a/src/core/condition.c
 +++ b/src/core/condition.c
 @@ -164,6 +164,8 @@ static bool test_security(const char *parameter) {
  #endif
  if (streq(parameter, apparmor))
  return access(/sys/kernel/security/apparmor/, F_OK) == 0;
 +if (streq(parameter, smack))
 +return access(/sys/fs/smackfs, F_OK) == 0;
  return false;
  }
  
 
 
 Lennart
 

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