Re: [systemd-devel] systemd unit checker tool

2015-04-08 Thread Przemyslaw Kedzierski

On 04/07/2015 07:02 PM, Zbigniew Jędrzejewski-Szmek wrote:

On Tue, Apr 07, 2015 at 12:36:31PM -0400, Rahul Sundaram wrote:



Perhaps packaging guidelines should recommend running this command or it
should be part of the macro that packages include that logs warnings when
unit files has any of these issues




We are interested in a tool that could be run automatically,
like rpmlint, at finish of build process.
It could read and check unit files from one service alone or in context 
of all of them. This should be controlled by options.

There will be defined a set of rules, for both methods

Of course should be configurable the source of unit files used to build 
a graph - currently installed in build system, from other rpm's,
maybe from image? (what to do if target systemd will be too new than 
running on build system)


Is there only possible to check typos and syntax errors in case of 
testing single unit file?
For example, problems like these: 
http://lists.freedesktop.org/archives/systemd-devel/2011-March/001800.html


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


[systemd-devel] systemd unit checker tool

2015-04-07 Thread Przemyslaw Kedzierski

Hello,

We started work on a tool dedicated to check systemd unit files.
Its main purpose will be to parse all or a subset of them, look for
common errors, suboptimal designs.
It will also check single unit file separately or sets of them
referencing one another (for example cycles).

We need some help. Could you please describe types of mistakes
 that are common, harmful and annoying?

Thank you

Przemek Kedzierski

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


[systemd-devel] [PATCH] bus-proxy: cloning smack label

2014-12-09 Thread Przemyslaw Kedzierski
When dbus client connects to systemd-bus-proxyd through
Unix domain socket proxy takes client's smack label and sets for itself.

It is done before and independent of dropping privileges.

The reason of such soluton is fact that tests of access rights
performed by lsm may take place inside kernel, not only
in userspace of recipient of message.

The bus-proxyd needs CAP_MAC_ADMIN to manipulate its label.

In case of systemd running in system mode, CAP_MAC_ADMIN
should be added to CapabilityBoundingSet in service file of bus-proxyd.

In case of systemd running in user mode ('systemd --user')
it can be achieved by addition
Capabilities=cap_mac_admin=i and SecureBits=keep-caps
to user@.service file
and setting cap_mac_admin+ei on bus-proxyd binary.
---
 Makefile.am | 11 +--
 configure.ac|  4 
 src/bus-proxyd/bus-proxyd.c | 20 
 src/shared/capability.c | 18 ++
 src/shared/capability.h |  2 ++
 units/systemd-bus-pro...@.service.in| 22 --
 units/systemd-bus-pro...@.service.m4.in | 22 ++
 units/u...@.service.in  | 19 ---
 units/u...@.service.m4.in   | 23 +++
 9 files changed, 98 insertions(+), 43 deletions(-)
 delete mode 100644 units/systemd-bus-pro...@.service.in
 create mode 100644 units/systemd-bus-pro...@.service.m4.in
 delete mode 100644 units/u...@.service.in
 create mode 100644 units/u...@.service.m4.in

diff --git a/Makefile.am b/Makefile.am
index 7b43733..78cf4a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -591,7 +591,7 @@ EXTRA_DIST += \
units/systemd-f...@.service.in \
units/systemd-fsck-root.service.in \
units/systemd-machine-id-commit.service.in \
-   units/u...@.service.in \
+   units/u...@.service.m4.in \
units/debug-shell.service.in \
units/systemd-suspend.service.in \
units/quotaon.service.in \
@@ -2579,9 +2579,16 @@ dist_userunit_DATA += \
 endif
 
 EXTRA_DIST += \
-   units/systemd-bus-pro...@.service.in \
+   units/systemd-bus-pro...@.service.m4.in \
units/user/systemd-bus-pro...@.service.in
 
+if HAVE_SMACK
+bus-proxyd-set-cap-hook:
+   $(SETCAP) cap_mac_admin+ei 
$(DESTDIR)$(rootlibexecdir)/systemd-bus-proxyd
+
+INSTALL_EXEC_HOOKS += bus-proxyd-set-cap-hook
+endif
+
 # 
--
 systemd_tty_ask_password_agent_SOURCES = \
src/tty-ask-password-agent/tty-ask-password-agent.c
diff --git a/configure.ac b/configure.ac
index 356a3c3..94b4a02 100644
--- a/configure.ac
+++ b/configure.ac
@@ -90,6 +90,8 @@ AC_PATH_PROG([XSLTPROC], [xsltproc])
 AC_PATH_PROG([QUOTAON], [quotaon], [/usr/sbin/quotaon], 
[$PATH:/usr/sbin:/sbin])
 AC_PATH_PROG([QUOTACHECK], [quotacheck], [/usr/sbin/quotacheck], 
[$PATH:/usr/sbin:/sbin])
 
+AC_PATH_PROG([SETCAP], [setcap], [/usr/sbin/setcap], [$PATH:/usr/sbin:/sbin])
+
 AC_PATH_PROG([KILL], [kill], [/usr/bin/kill], [$PATH:/usr/sbin:/sbin])
 
 AC_PATH_PROG([KMOD], [kmod], [/usr/bin/kmod], [$PATH:/usr/sbin:/sbin])
@@ -674,6 +676,8 @@ if test x${have_smack} = xyes ; then
 AC_DEFINE(HAVE_SMACK, 1, [Define if SMACK is available])
 fi
 
+AM_CONDITIONAL([HAVE_SMACK], [test x$have_smack = xyes])
+
 # 
--
 AC_ARG_ENABLE([gcrypt],
 AS_HELP_STRING([--disable-gcrypt],[Disable optional GCRYPT support]),
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 42fb0da..a9940f1 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -46,6 +46,7 @@
 #include capability.h
 #include bus-policy.h
 #include bus-control.h
+#include smack-util.h
 
 static char *arg_address = NULL;
 static char *arg_command_line_buffer = NULL;
@@ -1235,6 +1236,18 @@ static int patch_sender(sd_bus *a, sd_bus_message *m) {
 return 0;
 }
 
+static int mac_smack_apply_label_and_drop_cap_mac_admin(pid_t its_pid, const 
char *new_label) {
+
+int r1 = 0, r2 = 0;
+if (mac_smack_use()) {
+if (new_label  its_pid)
+r1 = mac_smack_apply_pid(its_pid, new_label);
+
+r2 = drop_capability(CAP_MAC_ADMIN);
+}
+return (r1  0) ? r1 : r2;
+}
+
 int main(int argc, char *argv[]) {
 
 _cleanup_bus_close_unref_ sd_bus *a = NULL, *b = NULL;
@@ -1274,6 +1287,13 @@ int main(int argc, char *argv[]) {
 if (is_unix) {
 (void) getpeercred(in_fd, ucred);
 (void) getpeersec(in_fd, peersec);
+
+#ifdef HAVE_SMACK
+r = mac_smack_apply_label_and_drop_cap_mac_admin(getpid(), 
peersec);
+if (r  0)
+log_warning(Failed to set SMACK label (%s) and drop
+ CAP_MAC_ADMIN : %s, peersec, 

Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label

2014-12-01 Thread Przemyslaw Kedzierski

Hello
Could you take a look at my patch?
Regards

Przemyslaw Kedzierski

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


[systemd-devel] [PATCH] bus-proxy: cloning smack label

2014-11-13 Thread Przemyslaw Kedzierski
When dbus client connects to systemd-bus-proxyd through
Unix domain socket proxy takes client's smack label and sets for itself.

It is done before and independent of dropping privileges.

The reason of such soluton is fact that tests of access rights
performed by lsm may take place inside kernel, not only
in userspace of recipient of message.

The bus-proxyd needs CAP_MAC_ADMIN to manipulate its label.

In case of systemd running in system mode, CAP_MAC_ADMIN
should be added to CapabilityBoundingSet in service file of bus-proxyd.

In case of systemd running in user mode ('systemd --user')
it can be achieved by addition
Capabilities=cap_mac_admin=i and SecureBits=keep-caps
to user@.service file
and setting cap_mac_admin+ei on bus-proxyd binary.

Change-Id: I0acb5ec0d9ce4aecf25ddb0ca0e137b7a187a02f
---
 Makefile.am |  4 ++--
 src/bus-proxyd/bus-proxyd.c | 17 +
 src/shared/capability.c | 18 ++
 src/shared/capability.h |  2 ++
 units/systemd-bus-pro...@.service.in| 22 --
 units/systemd-bus-pro...@.service.m4.in | 22 ++
 units/u...@.service.in  | 19 ---
 units/u...@.service.m4.in   | 23 +++
 8 files changed, 84 insertions(+), 43 deletions(-)
 delete mode 100644 units/systemd-bus-pro...@.service.in
 create mode 100644 units/systemd-bus-pro...@.service.m4.in
 delete mode 100644 units/u...@.service.in
 create mode 100644 units/u...@.service.m4.in

diff --git a/Makefile.am b/Makefile.am
index 701666c..e9db1f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -588,7 +588,7 @@ EXTRA_DIST += \
units/user/systemd-exit.service.in \
units/systemd-f...@.service.in \
units/systemd-fsck-root.service.in \
-   units/u...@.service.in \
+   units/u...@.service.m4.in \
units/debug-shell.service.in \
units/systemd-suspend.service.in \
units/quotaon.service.in \
@@ -2528,7 +2528,7 @@ dist_userunit_DATA += \
 endif
 
 EXTRA_DIST += \
-   units/systemd-bus-pro...@.service.in \
+   units/systemd-bus-pro...@.service.m4.in \
units/user/systemd-bus-pro...@.service.in
 
 # 
--
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index d6607ed..08e2c7c 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -45,6 +45,7 @@
 #include def.h
 #include capability.h
 #include bus-policy.h
+#include smack-util.h
 
 static char *arg_address = NULL;
 static char *arg_command_line_buffer = NULL;
@@ -1234,6 +1235,22 @@ int main(int argc, char *argv[]) {
 if (is_unix) {
 (void) getpeercred(in_fd, ucred);
 (void) getpeersec(in_fd, peersec);
+
+#ifdef HAVE_SMACK
+if (mac_smack_use()) {
+if (peersec) {
+
+r = mac_smack_apply_pid(getpid(), peersec);
+if (r  0)
+log_warning(Failed to set SMACK label 
%s : %s, peersec, strerror(-r));
+} else
+log_warning(Invalid SMACK label);
+
+r = drop_capability(CAP_MAC_ADMIN);
+if (r  0)
+log_warning(Failed to drop CAP_MAC_ADMIN: 
%s, strerror(-r));
+}
+#endif
 }
 
 if (arg_drop_privileges) {
diff --git a/src/shared/capability.c b/src/shared/capability.c
index 0226542..9dc42ec 100644
--- a/src/shared/capability.c
+++ b/src/shared/capability.c
@@ -285,3 +285,21 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t 
keep_capabilities) {
 
 return 0;
 }
+
+int drop_capability(cap_value_t cv) {
+_cleanup_cap_free_ cap_t tmp_cap = NULL;
+
+tmp_cap = cap_get_proc();
+if (!tmp_cap)
+return -errno;
+
+if ((cap_set_flag(tmp_cap, CAP_INHERITABLE, 1, cv, CAP_CLEAR)  0) ||
+(cap_set_flag(tmp_cap, CAP_PERMITTED, 1, cv, CAP_CLEAR)  0) ||
+(cap_set_flag(tmp_cap, CAP_EFFECTIVE, 1, cv, CAP_CLEAR)  0))
+return -errno;
+
+if (cap_set_proc(tmp_cap)  0)
+return -errno;
+
+return 0;
+}
diff --git a/src/shared/capability.h b/src/shared/capability.h
index 3e6d999..6f2f6f9 100644
--- a/src/shared/capability.h
+++ b/src/shared/capability.h
@@ -34,6 +34,8 @@ int capability_bounding_set_drop_usermode(uint64_t drop);
 
 int drop_privileges(uid_t uid, gid_t gid, uint64_t keep_capabilites);
 
+int drop_capability(cap_value_t cv);
+
 DEFINE_TRIVIAL_CLEANUP_FUNC(cap_t, cap_free);
 #define _cleanup_cap_free_ _cleanup_(cap_freep)
 
diff --git a/units/systemd-bus-pro...@.service.in 
b/units/systemd-bus-pro...@.service.in
deleted file mode 100644
index eef703f..000
--- 

[systemd-devel] [PATCH] bus-proxy: cloning smack label

2014-11-06 Thread Przemyslaw Kedzierski
When dbus client connects to systemd-bus-proxyd through
Unix domain socket proxy takes client's smack label and sets for itself.

It is done before and independent of dropping privileges.

The reason of such soluton is fact that tests of access rights
performed by lsm may take place inside kernel, not only
in userspace of recipient of message.

The bus-proxyd needs CAP_MAC_ADMIN to manipulate its label.

In case of systemd running in system mode, CAP_MAC_ADMIN
should be added to CapabilityBoundingSet in service file of bus-proxyd.

In case of systemd running in user mode ('systemd --user')
it can be achieved by addition
Capabilities=cap_mac_admin=i and SecureBits=keep-caps
to user@.service file
and setting cap_mac_admin+ei on bus-proxyd binary.

Signed-off-by: Przemyslaw Kedzierski p.kedzier...@samsung.com
---
 src/bus-proxyd/bus-proxyd.c  | 16 
 src/shared/capability.c  | 18 ++
 src/shared/capability.h  |  2 ++
 src/shared/smack-util.c  | 18 ++
 src/shared/smack-util.h  |  1 +
 units/systemd-bus-pro...@.service.in |  2 +-
 units/u...@.service.in   |  2 ++
 7 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index d10de2f..066232e 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -45,6 +45,7 @@
 #include def.h
 #include capability.h
 #include bus-policy.h
+#include smack-util.h
 
 static char *arg_address = NULL;
 static char *arg_command_line_buffer = NULL;
@@ -1168,6 +1169,21 @@ int main(int argc, char *argv[]) {
 if (is_unix) {
 (void) getpeercred(in_fd, ucred);
 (void) getpeersec(in_fd, peersec);
+
+if (mac_smack_use()) {
+if (peersec) {
+
+r = mac_smack_set_process_label(peersec);
+if (r  0)
+log_warning(Failed to set SMACK label 
%s : %s, peersec, strerror(-r));
+} else {
+log_warning(Invalid SMACK label);
+}
+
+r = drop_capability(CAP_MAC_ADMIN);
+if (r  0)
+log_warning(Failed to drop CAP_MAC_ADMIN: 
%s, strerror(-r));
+}
 }
 
 if (arg_drop_privileges) {
diff --git a/src/shared/capability.c b/src/shared/capability.c
index 0226542..9dc42ec 100644
--- a/src/shared/capability.c
+++ b/src/shared/capability.c
@@ -285,3 +285,21 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t 
keep_capabilities) {
 
 return 0;
 }
+
+int drop_capability(cap_value_t cv) {
+_cleanup_cap_free_ cap_t tmp_cap = NULL;
+
+tmp_cap = cap_get_proc();
+if (!tmp_cap)
+return -errno;
+
+if ((cap_set_flag(tmp_cap, CAP_INHERITABLE, 1, cv, CAP_CLEAR)  0) ||
+(cap_set_flag(tmp_cap, CAP_PERMITTED, 1, cv, CAP_CLEAR)  0) ||
+(cap_set_flag(tmp_cap, CAP_EFFECTIVE, 1, cv, CAP_CLEAR)  0))
+return -errno;
+
+if (cap_set_proc(tmp_cap)  0)
+return -errno;
+
+return 0;
+}
diff --git a/src/shared/capability.h b/src/shared/capability.h
index 3e6d999..6f2f6f9 100644
--- a/src/shared/capability.h
+++ b/src/shared/capability.h
@@ -34,6 +34,8 @@ int capability_bounding_set_drop_usermode(uint64_t drop);
 
 int drop_privileges(uid_t uid, gid_t gid, uint64_t keep_capabilites);
 
+int drop_capability(cap_value_t cv);
+
 DEFINE_TRIVIAL_CLEANUP_FUNC(cap_t, cap_free);
 #define _cleanup_cap_free_ _cleanup_(cap_freep)
 
diff --git a/src/shared/smack-util.c b/src/shared/smack-util.c
index a8dccd1..3e9810c 100644
--- a/src/shared/smack-util.c
+++ b/src/shared/smack-util.c
@@ -181,3 +181,21 @@ int mac_smack_fix(const char *path, bool ignore_enoent, 
bool ignore_erofs) {
 
 return r;
 }
+
+int mac_smack_set_process_label(char *label) {
+#ifdef HAVE_SMACK
+int ret;
+
+if (!label) {
+log_debug(No SMACK label given);
+return -1;
+}
+ret = write_string_file(/proc/self/attr/current, label);
+if (ret) {
+log_debug(Failed to set current SMACK label \%s\ on self: 
%s,
+  label, strerror(-ret));
+return ret;
+}
+#endif
+return 0;
+}
diff --git a/src/shared/smack-util.h b/src/shared/smack-util.h
index 68778da..a49405b 100644
--- a/src/shared/smack-util.h
+++ b/src/shared/smack-util.h
@@ -33,3 +33,4 @@ int mac_smack_apply(const char *path, const char *label);
 int mac_smack_apply_fd(int fd, const char *label);
 int mac_smack_apply_ip_in_fd(int fd, const char *label);
 int mac_smack_apply_ip_out_fd(int fd, const char *label);
+int mac_smack_set_process_label(char *label);
diff --git a/units/systemd-bus-pro...@.service.in

[systemd-devel] [PATCH] bus-proxy: --clone-smack-label option

2014-11-05 Thread Przemyslaw Kedzierski
This patch adds a '--clone-smack-label' option to systemd-bus-proxyd.
When dbus client connects to systemd-bus-proxyd through Unix domain socket
and this option is enabled
proxy takes client's smack label and sets for itself.

It is done before and independent of dropping privileges.

The reason of such soluton is fact that tests of access rights
performed by lsm may take place inside kernel, not only
in userspace of recipient of message.

The bus-proxyd needs CAP_MAC_ADMIN to manipulate its label.

In case of systemd running in system mode, CAP_MAC_ADMIN
should be added to CapabilityBoundingSet in service file of bus-proxyd.

In case of systemd running in user mode ('systemd --user')
it can be achieved by addition
Capabilities=cap_mac_admin=i and SecureBits=keep-caps
to user@.service file
and setting cap_mac_admin+ei on bus-proxyd binary.

Change-Id: I5a2c77348d4d293dd3707e82349cf624ddaf744a
Signed-off-by: Przemyslaw Kedzierski p.kedzier...@samsung.com
---
 man/systemd-bus-proxyd.xml  |  9 +
 src/bus-proxyd/bus-proxyd.c | 37 +
 src/shared/capability.c | 18 ++
 src/shared/capability.h |  2 ++
 src/shared/smack-util.c | 18 ++
 src/shared/smack-util.h |  1 +
 6 files changed, 85 insertions(+)

diff --git a/man/systemd-bus-proxyd.xml b/man/systemd-bus-proxyd.xml
index f9400f0..0aa24cf 100644
--- a/man/systemd-bus-proxyd.xml
+++ b/man/systemd-bus-proxyd.xml
@@ -87,6 +87,15 @@ along with systemd; If not, see 
http://www.gnu.org/licenses/.
 /listitem
   /varlistentry
 
+  varlistentry
+termoption--clone-smack-label/option/term
+
+listitem
+  paraTake client's smack label and set for itself.
+  The commandsystemd-bus-proxyd/command needs CAP_MAC_ADMIN to 
manipulate it./para
+/listitem
+  /varlistentry
+
   xi:include href=standard-options.xml xpointer=help /
   xi:include href=standard-options.xml xpointer=version /
 /variablelist
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index d10de2f..ae8cd02 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -45,11 +45,13 @@
 #include def.h
 #include capability.h
 #include bus-policy.h
+#include smack-util.h
 
 static char *arg_address = NULL;
 static char *arg_command_line_buffer = NULL;
 static bool arg_drop_privileges = false;
 static char **arg_configuration = NULL;
+static bool arg_clone_smack_label = false;
 
 static int help(void) {
 
@@ -58,6 +60,7 @@ static int help(void) {
  -h --help   Show this help\n
 --versionShow package version\n
 --drop-privilegesDrop privileges\n
+--clone-smack-label  Clone smack label\n
 --configuration=PATH Configuration file or directory\n
 --machine=MACHINEConnect to specified machine\n
 --address=ADDRESSConnect to the bus specified by 
ADDRESS\n
@@ -75,6 +78,7 @@ static int parse_argv(int argc, char *argv[]) {
 ARG_DROP_PRIVILEGES,
 ARG_CONFIGURATION,
 ARG_MACHINE,
+ARG_CLONE_SMACK_LABEL,
 };
 
 static const struct option options[] = {
@@ -84,6 +88,7 @@ static int parse_argv(int argc, char *argv[]) {
 { drop-privileges, no_argument,   NULL, 
ARG_DROP_PRIVILEGES },
 { configuration,   required_argument, NULL, 
ARG_CONFIGURATION   },
 { machine, required_argument, NULL, ARG_MACHINE  
   },
+{ clone-smack-label, no_argument, NULL, 
ARG_CLONE_SMACK_LABEL },
 {},
 };
 
@@ -149,6 +154,9 @@ static int parse_argv(int argc, char *argv[]) {
 break;
 }
 
+case ARG_CLONE_SMACK_LABEL:
+arg_clone_smack_label = true;
+break;
 case '?':
 return -EINVAL;
 
@@ -1168,6 +1176,35 @@ int main(int argc, char *argv[]) {
 if (is_unix) {
 (void) getpeercred(in_fd, ucred);
 (void) getpeersec(in_fd, peersec);
+
+if (arg_clone_smack_label) {
+
+if (!mac_smack_use()) {
+log_warning(No SMACK found);
+goto exit_clone_smack_label;
+}
+
+if (!peersec) {
+log_warning(Invalid SMACK label);
+goto exit_clone_smack_label;
+}
+
+r = have_effective_cap(CAP_MAC_ADMIN);
+if (r = 0) {
+log_warning(No CAP_MAC_ADMIN capability);
+goto exit_clone_smack_label