Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-24 Thread Lennart Poettering 
On Thu, 20.02.14 12:01, Łukasz Stelmach (l.stelm...@samsung.com) wrote:

Heya!

I applied the three patches now, and made some clean-ups which I folded
into your last patch. AMong the changes I made is that I dropped is the
hook-up with label_context_set() since that is only for controlling what
label to use for files that are about to be created, which is something
SMACK doesn't support, right? (And the code certainly didn't do that,
since it tried to apply an xattr on the file which will necessarily
fail, given that is is for files that are to be created later, not for
files that already exist).

Pleas check if everythings is good for SMACK now!

Thanks,

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 v2 1/2] Smack - relabel directories and files created by systemd

2014-02-20 Thread Łukasz Stelmach 
It was 2014-02-19 śro 20:05, when Zbigniew Jędrzejewski-Szmek wrote:
 On Wed, Feb 19, 2014 at 04:17:15PM +0100, Łukasz Stelmach wrote:
 It was 2014-02-19 śro 16:05, when Zbigniew Jędrzejewski-Szmek wrote:
  On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
  How to have support for more than one security fw reasonably
  compiled in? (I think this is the moment to create the pattern).
  Why not? It would be rather constraining for a distribution which wants
  to support more than one. systemd should just perform the steps necessary
  for all compiled frameworks compiled in, silently ignoring failures coming
  from missing frameworks.
 

[...]

 The most robust way for systemd is:
 1) to check in runtime which frameworks are supported,
 We have use_selinux(), use_apparmor(), use_smack().

 2) to attempt an action for every one of them,
 3) to return an error if ANY of the actions fail.

 In general yes, but different frameworks need hooks in different places.
 So we generally insert a call to a function specific to a framework,
 and inside this function, a use_*() test is performed, and suitably,
 either nothing is done or the setup is performed. If an error happens,
 it is up to this function to decide whether silent failure, warning,
 or an error are warranted.

OK, how about this?

https://review.tizen.org/git/?p=platform/upstream/systemd.git;a=commitdiff;h=4879ed0a3b3942ed0188c2b5a5633f22847ebe76;hp=6300b3eca9e5261b73bd7f1bb9735992b127cd80

https://review.tizen.org/git/?p=platform/upstream/systemd.git;a=blob;f=src/shared/label.c;h=89939217e3d9bce011c125b504978571e7b57c22;hb=4879ed0a3b3942ed0188c2b5a5633f22847ebe76

-- 
Łukasz Stelmach
Samsung RD Institute Poland
Samsung Electronics


pgpPxAeTP7PJE.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Łukasz Stelmach 
From: Casey Schaufler ca...@schaufler-ca.com

Systemd creates directories in /dev. These directories will
get the label of systemd, which is the label of the System
domain, which is not accessable to everyone. Relabel the
directories, files and symlinks created so that they can be
generally used.

Signed-off-by: Casey Schaufler casey.schauf...@intel.com
Signed-off-by: Łukasz Stelmach l.stelm...@samsung.com
---
 src/shared/label.c |   60 +---
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/shared/label.c b/src/shared/label.c
index 4a26ba9..9a1916a 100644
--- a/src/shared/label.c
+++ b/src/shared/label.c
@@ -41,6 +41,48 @@
 static struct selabel_handle *label_hnd = NULL;
 
 #endif
+#ifdef HAVE_SMACK
+#include sys/xattr.h
+#include string.h
+#define FLOOR_LABEL_
+#define STAR_LABEL *
+
+static void smack_relabel_in_dev(const char *path) {
+struct stat sb;
+const char *label;
+int r;
+
+/*
+ * Path must be in /dev and must exist
+ */
+if (!path_equal(path, /dev) 
+!path_startswith(path, /dev))
+return;
+
+r = lstat(path, sb);
+if (r  0)
+return;
+
+/*
+ * Label directories and character devices *.
+ * Label symlinks _.
+ * Don't change anything else.
+ */
+if (S_ISDIR(sb.st_mode))
+label = STAR_LABEL;
+else if (S_ISLNK(sb.st_mode))
+label = FLOOR_LABEL;
+else if (S_ISCHR(sb.st_mode))
+label = STAR_LABEL;
+else
+return;
+
+r = setxattr(path, security.SMACK64, label, strlen(label), 0);
+if (r  0)
+log_error(Smack relabeling \%s\ %s, path, strerror(errno));
+return;
+}
+#endif
 
 int label_init(const char *prefix) {
 int r = 0;
@@ -130,6 +172,9 @@ int label_fix(const char *path, bool ignore_enoent, bool 
ignore_erofs) {
 r = security_getenforce() == 1 ? -errno : 0;
 }
 #endif
+#ifdef HAVE_SMACK
+smack_relabel_in_dev(path);
+#endif
 
 return r;
 }
@@ -204,6 +249,9 @@ int label_context_set(const char *path, mode_t mode) {
 if (r  0  security_getenforce() == 0)
 r = 0;
 #endif
+#ifdef HAVE_SMACK
+smack_relabel_in_dev(path);
+#endif
 
 return r;
 }
@@ -257,10 +305,10 @@ void label_free(const char *label) {
 }
 
 int label_mkdir(const char *path, mode_t mode) {
+int r;
 
-/* Creates a directory and labels it according to the SELinux policy */
 #ifdef HAVE_SELINUX
-int r;
+/* Creates a directory and labels it according to the SELinux policy */
 security_context_t fcon = NULL;
 
 if (!use_selinux() || !label_hnd)
@@ -303,7 +351,13 @@ finish:
 
 skipped:
 #endif
-return mkdir(path, mode)  0 ? -errno : 0;
+r = mkdir(path, mode);
+if (r)
+return -errno;
+#ifdef HAVE_SMACK
+smack_relabel_in_dev(path);
+#endif
+return 0;
 }
 
 int label_bind(int fd, const struct sockaddr *addr, socklen_t addrlen) {
-- 
1.7.9.5

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

Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Lennart Poettering 
On Wed, 19.02.14 14:07, Łukasz Stelmach (l.stelm...@samsung.com) wrote:

 From: Casey Schaufler ca...@schaufler-ca.com
 
 Systemd creates directories in /dev. These directories will
 get the label of systemd, which is the label of the System
 domain, which is not accessable to everyone. Relabel the
 directories, files and symlinks created so that they can be
 generally used.
 
 Signed-off-by: Casey Schaufler casey.schauf...@intel.com
 Signed-off-by: Łukasz Stelmach l.stelm...@samsung.com
 ---
  src/shared/label.c |   60 
 +---
  1 file changed, 57 insertions(+), 3 deletions(-)
 
 diff --git a/src/shared/label.c b/src/shared/label.c
 index 4a26ba9..9a1916a 100644
 --- a/src/shared/label.c
 +++ b/src/shared/label.c
 @@ -41,6 +41,48 @@
  static struct selabel_handle *label_hnd = NULL;
  
  #endif
 +#ifdef HAVE_SMACK
 +#include sys/xattr.h
 +#include string.h

No includes in the middle of files please for normal API stuff.

Also, these files are not smack-specific. In order to avoid superfluous
#ifdefs, and to avoid uplicate inclusions later on, please just add
these to the top of the file, and include string.h unconditionally, and
xattr.h only if HAVE_XATTR is defined...

 +#define FLOOR_LABEL  _
 +#define STAR_LABEL   *

hmm, could we rename these to SMACK_LABEL_FLOOR and SMACK_LABEL_STAR?
That way they have a namespaced common namespace.

 +
 +static void smack_relabel_in_dev(const char *path) {
 +struct stat sb;
 +const char *label;
 +int r;
 +
 +/*
 + * Path must be in /dev and must exist
 + */
 +if (!path_equal(path, /dev) 
 +!path_startswith(path, /dev))
 +return;
 +
 +r = lstat(path, sb);
 +if (r  0)
 +return;
 +
 +/*
 + * Label directories and character devices *.
 + * Label symlinks _.
 + * Don't change anything else.
 + */
 +if (S_ISDIR(sb.st_mode))
 +label = STAR_LABEL;
 +else if (S_ISLNK(sb.st_mode))
 +label = FLOOR_LABEL;
 +else if (S_ISCHR(sb.st_mode))
 +label = STAR_LABEL;
 +else
 +return;
 +
 +r = setxattr(path, security.SMACK64, label, strlen(label), 0);
 +if (r  0)
 +log_error(Smack relabeling \%s\ %s, path, 
 strerror(errno));
 +return;

This return is unnecessary...

That said, I think it find it nicer if this call would actually return
an error, so that the caller decides whether it wants to ignore it, not
the function internally.

Also, please move the #ifdef HAVE_SMACK checks inside of this function
and make it a NOP on non-SMACK builds. That way we only have one #ifdef
check for this and not one for each invocation of the function. The
compiler should be smart away to suppress the function if it empty.

Otherwise looks good.

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 v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Łukasz Stelmach 
It was 2014-02-19 śro 14:30, when Lennart Poettering wrote:
 On Wed, 19.02.14 14:07, Łukasz Stelmach (l.stelm...@samsung.com) wrote:

 From: Casey Schaufler ca...@schaufler-ca.com
 
 Systemd creates directories in /dev. These directories will
 get the label of systemd, which is the label of the System
 domain, which is not accessable to everyone. Relabel the
 directories, files and symlinks created so that they can be
 generally used.
 
 Signed-off-by: Casey Schaufler casey.schauf...@intel.com
 Signed-off-by: Łukasz Stelmach l.stelm...@samsung.com
 ---
  src/shared/label.c |   60 
 +---
  1 file changed, 57 insertions(+), 3 deletions(-)
 
 diff --git a/src/shared/label.c b/src/shared/label.c
 index 4a26ba9..9a1916a 100644
 --- a/src/shared/label.c
 +++ b/src/shared/label.c
 @@ -41,6 +41,48 @@
  static struct selabel_handle *label_hnd = NULL;
  
  #endif
 +#ifdef HAVE_SMACK
 +#include sys/xattr.h
 +#include string.h

 No includes in the middle of files please for normal API stuff.

 Also, these files are not smack-specific. In order to avoid superfluous
 #ifdefs, and to avoid uplicate inclusions later on, please just add
 these to the top of the file, and include string.h unconditionally, and
 xattr.h only if HAVE_XATTR is defined...

 +#define FLOOR_LABEL _
 +#define STAR_LABEL  *

 hmm, could we rename these to SMACK_LABEL_FLOOR and SMACK_LABEL_STAR?
 That way they have a namespaced common namespace.

 +
 +static void smack_relabel_in_dev(const char *path) {
 +struct stat sb;
 +const char *label;
 +int r;
 +
 +/*
 + * Path must be in /dev and must exist
 + */
 +if (!path_equal(path, /dev) 
 +!path_startswith(path, /dev))
 +return;
 +
 +r = lstat(path, sb);
 +if (r  0)
 +return;
 +
 +/*
 + * Label directories and character devices *.
 + * Label symlinks _.
 + * Don't change anything else.
 + */
 +if (S_ISDIR(sb.st_mode))
 +label = STAR_LABEL;
 +else if (S_ISLNK(sb.st_mode))
 +label = FLOOR_LABEL;
 +else if (S_ISCHR(sb.st_mode))
 +label = STAR_LABEL;
 +else
 +return;
 +
 +r = setxattr(path, security.SMACK64, label, strlen(label), 0);
 +if (r  0)
 +log_error(Smack relabeling \%s\ %s, path, 
 strerror(errno));
 +return;

 This return is unnecessary...

 That said, I think it find it nicer if this call would actually return
 an error, so that the caller decides whether it wants to ignore it, not
 the function internally.

 Also, please move the #ifdef HAVE_SMACK checks inside of this function
 and make it a NOP on non-SMACK builds. That way we only have one #ifdef
 check for this and not one for each invocation of the function. The
 compiler should be smart away to suppress the function if it empty.

I am not sure about that. If we want smack_relabel_in_dev() to return a
value and call it from label_fix()

--8---cut here---start-8---
int label_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
int r = 0;

#ifdef HAVE_SELINUX
[...]
#endif
smack_relabel_in_dev(path);

return r;
}
--8---cut here---end---8---

then it seems better to write

--8---cut here---start-8---
#elif defined(HAVE_SMACK)
r = smack_relabel_in_dev(path);
#endif
--8---cut here---end---8---

and be able to add support for a yet undetermined security framework
below assuming systemd can have support for only one fw compiled in. How
to have support for more than one security fw reasonably compiled in? (I
think this is the moment to create the pattern).

-- 
Łukasz Stelmach
Samsung RD Institute Poland
Samsung Electronics


pgpnrFfEnCqYU.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Zbigniew Jędrzejewski-Szmek 
On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
 How to have support for more than one security fw reasonably
 compiled in? (I think this is the moment to create the pattern).
Why not? It would be rather constraining for a distribution which wants
to support more than one. systemd should just perform the steps necessary
for all compiled frameworks compiled in, silently ignoring failures coming
from missing frameworks.

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

Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Lennart Poettering 
On Wed, 19.02.14 15:44, Łukasz Stelmach (l.stelm...@samsung.com) wrote:

  Also, please move the #ifdef HAVE_SMACK checks inside of this function
  and make it a NOP on non-SMACK builds. That way we only have one #ifdef
  check for this and not one for each invocation of the function. The
  compiler should be smart away to suppress the function if it empty.
 
 I am not sure about that. If we want smack_relabel_in_dev() to return a
 value and call it from label_fix()
 
 --8---cut here---start-8---
 int label_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
 int r = 0;
 
 #ifdef HAVE_SELINUX
 [...]
 #endif
 smack_relabel_in_dev(path);
 
 return r;
 }
 --8---cut here---end---8---
 
 then it seems better to write
 
 --8---cut here---start-8---
 #elif defined(HAVE_SMACK)
 r = smack_relabel_in_dev(path);
 #endif
 --8---cut here---end---8---
 
 and be able to add support for a yet undetermined security framework
 below assuming systemd can have support for only one fw compiled in. How
 to have support for more than one security fw reasonably compiled in? (I
 think this is the moment to create the pattern).

Well, the other option is to simply place the smack relabelling code
directly in label_fix(), which would map 1:1 what we do for selinux.

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 v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Lennart Poettering 
On Wed, 19.02.14 16:05, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 
 On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
  How to have support for more than one security fw reasonably
  compiled in? (I think this is the moment to create the pattern).
 Why not? It would be rather constraining for a distribution which wants
 to support more than one. systemd should just perform the steps necessary
 for all compiled frameworks compiled in, silently ignoring failures coming
 from missing frameworks.

Yes, I agree fully with Zbigniew. A distribution like Debian is likely
to enable support for AppArmor, SMACK and SELinux in systemd, all at the
same time. That doesn't mean that all three will be active together
during runtime, as the kernel doesn't support that, however the binary
we build should support all three, and what is used is decided at
runtime at the discretion of the admin.

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 v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Łukasz Stelmach 
It was 2014-02-19 śro 16:05, when Zbigniew Jędrzejewski-Szmek wrote:
 On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
 How to have support for more than one security fw reasonably
 compiled in? (I think this is the moment to create the pattern).
 Why not? It would be rather constraining for a distribution which wants
 to support more than one. systemd should just perform the steps necessary
 for all compiled frameworks compiled in, silently ignoring failures coming
 from missing frameworks.

Hmm... silent ignoring makes things hard to debug.

The most robust way for systemd is:

1) to check in runtime which frameworks are supported,

2) to attempt an action for every one of them,

3) to return an error if ANY of the actions fail.

-- 
Łukasz Stelmach
Samsung RD Institute Poland
Samsung Electronics


pgpiM2yv81AYx.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Schaufler, Casey 
 -Original Message-
 From: Lennart Poettering [mailto:lenn...@poettering.net]
 Sent: Wednesday, February 19, 2014 7:08 AM
 To: Zbigniew Jędrzejewski-Szmek
 Cc: Łukasz Stelmach; Casey Schaufler; Schaufler, Casey; systemd-
 de...@lists.freedesktop.org
 Subject: Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and
 files created by systemd
 
 On Wed, 19.02.14 16:05, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
 wrote:
 
 
  On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
   How to have support for more than one security fw reasonably
   compiled in? (I think this is the moment to create the pattern).
  Why not? It would be rather constraining for a distribution which
  wants to support more than one. systemd should just perform the steps
  necessary for all compiled frameworks compiled in, silently ignoring
  failures coming from missing frameworks.
 
 Yes, I agree fully with Zbigniew. A distribution like Debian is likely to 
 enable
 support for AppArmor, SMACK and SELinux in systemd, all at the same time.
 That doesn't mean that all three will be active together during runtime, as 
 the
 kernel doesn't support that,

Yet. There is work in progress to enable multiple concurrent
security modules. At the current pace of development 2015
is the best guess for landing.

 however the binary we build should support all
 three, and what is used is decided at runtime at the discretion of the admin.
 
 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 v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Greg KH 
On Wed, Feb 19, 2014 at 04:10:49PM +, Schaufler, Casey wrote:
  -Original Message-
  From: Lennart Poettering [mailto:lenn...@poettering.net]
  Sent: Wednesday, February 19, 2014 7:08 AM
  To: Zbigniew Jędrzejewski-Szmek
  Cc: Łukasz Stelmach; Casey Schaufler; Schaufler, Casey; systemd-
  de...@lists.freedesktop.org
  Subject: Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and
  files created by systemd
  
  On Wed, 19.02.14 16:05, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
  wrote:
  
  
   On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
How to have support for more than one security fw reasonably
compiled in? (I think this is the moment to create the pattern).
   Why not? It would be rather constraining for a distribution which
   wants to support more than one. systemd should just perform the steps
   necessary for all compiled frameworks compiled in, silently ignoring
   failures coming from missing frameworks.
  
  Yes, I agree fully with Zbigniew. A distribution like Debian is likely to 
  enable
  support for AppArmor, SMACK and SELinux in systemd, all at the same time.
  That doesn't mean that all three will be active together during runtime, as 
  the
  kernel doesn't support that,
 
 Yet. There is work in progress to enable multiple concurrent
 security modules. At the current pace of development 2015
 is the best guess for landing.

We've been hearing that for how many decades now?  :)

I'm not holding my breath...

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

Re: [systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

2014-02-19 Thread Zbigniew Jędrzejewski-Szmek 
On Wed, Feb 19, 2014 at 04:17:15PM +0100, Łukasz Stelmach wrote:
 It was 2014-02-19 śro 16:05, when Zbigniew Jędrzejewski-Szmek wrote:
  On Wed, Feb 19, 2014 at 03:44:32PM +0100, Łukasz Stelmach wrote:
  How to have support for more than one security fw reasonably
  compiled in? (I think this is the moment to create the pattern).
  Why not? It would be rather constraining for a distribution which wants
  to support more than one. systemd should just perform the steps necessary
  for all compiled frameworks compiled in, silently ignoring failures coming
  from missing frameworks.
 
 Hmm... silent ignoring makes things hard to debug.
 
Verbose failure is that what is done currently. (In general, I'm sure
that there are various places where stuff could be improved). When
systemd is started, it'll try to initialize SELinux, AppArmor, etc. If
there's no configuration for a specific LSM, it is silently
ignored. If there is configuration, but it is cannot be loaded, errors
are printed. Likewise for unit files: if specific directives cannot be
executed, warnings are printed. Depending on the specific directive,
this can result in continued execution or in an error. If support for
a specific security framework is not compiled into systemd, warnings
are printed.

 The most robust way for systemd is:
 1) to check in runtime which frameworks are supported,
We have use_selinux(), use_apparmor(), use_smack().

 2) to attempt an action for every one of them,
 3) to return an error if ANY of the actions fail.

In general yes, but different frameworks need hooks in different places.
So we generally insert a call to a function specific to a framework,
and inside this function, a use_*() test is performed, and suitably,
either nothing is done or the setup is performed. If an error happens,
it is up to this function to decide whether silent failure, warning,
or an error are warranted.

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