summaryrefslogtreecommitdiff
path: root/src/core/execute.c
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2016-10-25 15:52:54 +0200
committerLennart Poettering <lennart@poettering.net>2016-11-02 08:55:00 -0600
commit5cd9cd3537d1afca85877103615e61e6c03e7079 (patch)
tree0ba41e172281c11897f2ef880543134c242461a5 /src/core/execute.c
parent133ddbbeae74fc06173633605b3e612e934bc2dd (diff)
execute: apply seccomp filters after changing selinux/aa/smack contexts
Seccomp is generally an unprivileged operation, changing security contexts is most likely associated with some form of policy. Moreover, while seccomp may influence our own flow of code quite a bit (much more than the security context change) make sure to apply the seccomp filters immediately before executing the binary to invoke. This also moves enforcement of NNP after the security context change, so that NNP cannot affect it anymore. (However, the security policy now has to permit the NNP change). This change has a good chance of breaking current SELinux/AA/SMACK setups, because the policy might not expect this change of behaviour. However, it's technically the better choice I think and should hence be applied. Fixes: #3993
Diffstat (limited to 'src/core/execute.c')
-rw-r--r--src/core/execute.c70
1 files changed, 39 insertions, 31 deletions
diff --git a/src/core/execute.c b/src/core/execute.c
index ae9df41b99..c73e2e7220 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2538,12 +2538,6 @@ static int exec_child(
(void) umask(context->umask);
if ((params->flags & EXEC_APPLY_PERMISSIONS) && !command->privileged) {
- r = setup_smack(context, command);
- if (r < 0) {
- *exit_status = EXIT_SMACK_PROCESS_LABEL;
- return r;
- }
-
if (context->pam_name && username) {
r = setup_pam(context->pam_name, username, uid, gid, context->tty_path, &accum_env, fds, n_fds);
if (r < 0) {
@@ -2693,6 +2687,41 @@ static int exec_child(
}
}
+ /* Apply the MAC contexts late, but before seccomp syscall filtering, as those should really be last to
+ * influence our own codepaths as little as possible. Moreover, applying MAC contexts usually requires
+ * syscalls that are subject to seccomp filtering, hence should probably be applied before the syscalls
+ * are restricted. */
+
+#ifdef HAVE_SELINUX
+ if (mac_selinux_use()) {
+ char *exec_context = mac_selinux_context_net ?: context->selinux_context;
+
+ if (exec_context) {
+ r = setexeccon(exec_context);
+ if (r < 0) {
+ *exit_status = EXIT_SELINUX_CONTEXT;
+ return r;
+ }
+ }
+ }
+#endif
+
+ r = setup_smack(context, command);
+ if (r < 0) {
+ *exit_status = EXIT_SMACK_PROCESS_LABEL;
+ return r;
+ }
+
+#ifdef HAVE_APPARMOR
+ if (context->apparmor_profile && mac_apparmor_use()) {
+ r = aa_change_onexec(context->apparmor_profile);
+ if (r < 0 && !context->apparmor_profile_ignore) {
+ *exit_status = EXIT_APPARMOR_PROFILE;
+ return -errno;
+ }
+ }
+#endif
+
/* PR_GET_SECUREBITS is not privileged, while
* PR_SET_SECUREBITS is. So to suppress
* potential EPERMs we'll try not to call
@@ -2758,6 +2787,8 @@ static int exec_child(
}
}
+ /* This really should remain the last step before the execve(), to make sure our own code is unaffected
+ * by the filter as little as possible. */
if (context_has_syscall_filters(context)) {
r = apply_seccomp(unit, context);
if (r < 0) {
@@ -2766,30 +2797,6 @@ static int exec_child(
}
}
#endif
-
-#ifdef HAVE_SELINUX
- if (mac_selinux_use()) {
- char *exec_context = mac_selinux_context_net ?: context->selinux_context;
-
- if (exec_context) {
- r = setexeccon(exec_context);
- if (r < 0) {
- *exit_status = EXIT_SELINUX_CONTEXT;
- return r;
- }
- }
- }
-#endif
-
-#ifdef HAVE_APPARMOR
- if (context->apparmor_profile && mac_apparmor_use()) {
- r = aa_change_onexec(context->apparmor_profile);
- if (r < 0 && !context->apparmor_profile_ignore) {
- *exit_status = EXIT_APPARMOR_PROFILE;
- return -errno;
- }
- }
-#endif
}
final_argv = replace_env_argv(argv, accum_env);
@@ -3611,7 +3618,8 @@ char *exec_command_line(char **argv) {
STRV_FOREACH(a, argv)
k += strlen(*a)+3;
- if (!(n = new(char, k)))
+ n = new(char, k);
+ if (!n)
return NULL;
p = n;