systemd: refuse to load units with errors (CVE-2017-1000082)
If a unit has a statement such as User=0day where the username exists but is strictly speaking invalid, the unit will be started as the root user instead. Backport a patch from upstream to mitigate this by refusing to start units such as this. (From OE-Core rev: a6eaef0f179a341c0b96bb30aaec2d80862a11d6) Signed-off-by: Ross Burton <ross.burton@intel.com> Signed-off-by: Armin Kuster <akuster808@gmail.com> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
This commit is contained in:
parent
bc3fa28e93
commit
b7e7b5e294
|
@ -0,0 +1,329 @@
|
|||
If a user is created with a strictly-speaking invalid name such as '0day' and a
|
||||
unit created to run as that user, systemd rejects the username and runs the unit
|
||||
as root.
|
||||
|
||||
CVE: CVE-2017-1000082
|
||||
Upstream-Status: Backport
|
||||
Signed-off-by: Ross Burton <ross.burton@intel.com>
|
||||
|
||||
From d8e1310e1ed7b6f122bc7eb8ba061fbd088783c0 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
||||
Date: Thu, 6 Jul 2017 13:28:19 -0400
|
||||
Subject: [PATCH] core/load-fragment: refuse units with errors in certain
|
||||
directives
|
||||
|
||||
If an error is encountered in any of the Exec* lines, WorkingDirectory,
|
||||
SELinuxContext, ApparmorProfile, SmackProcessLabel, Service (in .socket
|
||||
units), User, or Group, refuse to load the unit. If the config stanza
|
||||
has support, ignore the failure if '-' is present.
|
||||
|
||||
For those configuration directives, even if we started the unit, it's
|
||||
pretty likely that it'll do something unexpected (like write files
|
||||
in a wrong place, or with a wrong context, or run with wrong permissions,
|
||||
etc). It seems better to refuse to start the unit and have the admin
|
||||
clean up the configuration without giving the service a chance to mess
|
||||
up stuff.
|
||||
|
||||
Note that all "security" options that restrict what the unit can do
|
||||
(Capabilities, AmbientCapabilities, Restrict*, SystemCallFilter, Limit*,
|
||||
PrivateDevices, Protect*, etc) are _not_ treated like this. Such options are
|
||||
only supplementary, and are not always available depending on the architecture
|
||||
and compilation options, so unit authors have to make sure that the service
|
||||
runs correctly without them anyway.
|
||||
|
||||
Fixes #6237, #6277.
|
||||
|
||||
Signed-off-by: Ross Burton <ross.burton@intel.com>
|
||||
---
|
||||
src/core/load-fragment.c | 104 ++++++++++++++++++++++++++++------------------
|
||||
src/test/test-unit-file.c | 14 +++----
|
||||
2 files changed, 70 insertions(+), 48 deletions(-)
|
||||
|
||||
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
|
||||
index cbc826809..2047974f4 100644
|
||||
--- a/src/core/load-fragment.c
|
||||
+++ b/src/core/load-fragment.c
|
||||
@@ -630,20 +630,28 @@ int config_parse_exec(
|
||||
|
||||
if (isempty(f)) {
|
||||
/* First word is either "-" or "@" with no command. */
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Empty path in command line, ignoring: \"%s\"", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0,
|
||||
+ "Empty path in command line%s: \"%s\"",
|
||||
+ ignore ? ", ignoring" : "", rvalue);
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
if (!string_is_safe(f)) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Executable path contains special characters, ignoring: %s", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0,
|
||||
+ "Executable path contains special characters%s: %s",
|
||||
+ ignore ? ", ignoring" : "", rvalue);
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
if (!path_is_absolute(f)) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Executable path is not absolute, ignoring: %s", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0,
|
||||
+ "Executable path is not absolute%s: %s",
|
||||
+ ignore ? ", ignoring" : "", rvalue);
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
if (endswith(f, "/")) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Executable path specifies a directory, ignoring: %s", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0,
|
||||
+ "Executable path specifies a directory%s: %s",
|
||||
+ ignore ? ", ignoring" : "", rvalue);
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
if (f == firstword) {
|
||||
@@ -699,7 +707,7 @@ int config_parse_exec(
|
||||
if (r == 0)
|
||||
break;
|
||||
else if (r < 0)
|
||||
- return 0;
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
|
||||
if (!GREEDY_REALLOC(n, nbufsize, nlen + 2))
|
||||
return log_oom();
|
||||
@@ -709,8 +717,10 @@ int config_parse_exec(
|
||||
}
|
||||
|
||||
if (!n || !n[0]) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Empty executable name or zeroeth argument, ignoring: %s", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0,
|
||||
+ "Empty executable name or zeroeth argument%s: %s",
|
||||
+ ignore ? ", ignoring" : "", rvalue);
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
nce = new0(ExecCommand, 1);
|
||||
@@ -1315,8 +1325,10 @@ int config_parse_exec_selinux_context(
|
||||
|
||||
r = unit_name_printf(u, rvalue, &k);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers, ignoring: %m");
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r,
|
||||
+ "Failed to resolve specifiers%s: %m",
|
||||
+ ignore ? ", ignoring" : "");
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
free(c->selinux_context);
|
||||
@@ -1363,8 +1375,10 @@ int config_parse_exec_apparmor_profile(
|
||||
|
||||
r = unit_name_printf(u, rvalue, &k);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers, ignoring: %m");
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r,
|
||||
+ "Failed to resolve specifiers%s: %m",
|
||||
+ ignore ? ", ignoring" : "");
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
free(c->apparmor_profile);
|
||||
@@ -1411,8 +1425,10 @@ int config_parse_exec_smack_process_label(
|
||||
|
||||
r = unit_name_printf(u, rvalue, &k);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers, ignoring: %m");
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r,
|
||||
+ "Failed to resolve specifiers%s: %m",
|
||||
+ ignore ? ", ignoring" : "");
|
||||
+ return ignore ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
free(c->smack_process_label);
|
||||
@@ -1630,19 +1646,19 @@ int config_parse_socket_service(
|
||||
|
||||
r = unit_name_printf(UNIT(s), rvalue, &p);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers, ignoring: %s", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve specifiers: %s", rvalue);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
if (!endswith(p, ".service")) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Unit must be of type service, ignoring: %s", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0, "Unit must be of type service: %s", rvalue);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
r = manager_load_unit(UNIT(s)->manager, p, NULL, &error, &x);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to load unit %s, ignoring: %s", rvalue, bus_error_message(&error, r));
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r, "Failed to load unit %s: %s", rvalue, bus_error_message(&error, r));
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
unit_ref_set(&s->service, x);
|
||||
@@ -1893,13 +1909,13 @@ int config_parse_user_group(
|
||||
|
||||
r = unit_full_printf(u, rvalue, &k);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in %s, ignoring: %m", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in %s: %m", rvalue);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
if (!valid_user_group_name_or_id(k)) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID, ignoring: %s", k);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID: %s", k);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
n = k;
|
||||
@@ -1957,19 +1973,19 @@ int config_parse_user_group_strv(
|
||||
if (r == -ENOMEM)
|
||||
return log_oom();
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Invalid syntax, ignoring: %s", rvalue);
|
||||
- break;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r, "Invalid syntax: %s", rvalue);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
r = unit_full_printf(u, word, &k);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in %s, ignoring: %m", word);
|
||||
- continue;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in %s: %m", word);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
if (!valid_user_group_name_or_id(k)) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID, ignoring: %s", k);
|
||||
- continue;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0, "Invalid user/group name or numeric ID: %s", k);
|
||||
+ return -ENOEXEC;
|
||||
}
|
||||
|
||||
r = strv_push(users, k);
|
||||
@@ -2128,25 +2144,28 @@ int config_parse_working_directory(
|
||||
|
||||
r = unit_full_printf(u, rvalue, &k);
|
||||
if (r < 0) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers in working directory path '%s', ignoring: %m", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, r,
|
||||
+ "Failed to resolve unit specifiers in working directory path '%s'%s: %m",
|
||||
+ rvalue, missing_ok ? ", ignoring" : "");
|
||||
+ return missing_ok ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
path_kill_slashes(k);
|
||||
|
||||
if (!utf8_is_valid(k)) {
|
||||
log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue);
|
||||
- return 0;
|
||||
+ return missing_ok ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
if (!path_is_absolute(k)) {
|
||||
- log_syntax(unit, LOG_ERR, filename, line, 0, "Working directory path '%s' is not absolute, ignoring.", rvalue);
|
||||
- return 0;
|
||||
+ log_syntax(unit, LOG_ERR, filename, line, 0,
|
||||
+ "Working directory path '%s' is not absolute%s.",
|
||||
+ rvalue, missing_ok ? ", ignoring" : "");
|
||||
+ return missing_ok ? 0 : -ENOEXEC;
|
||||
}
|
||||
|
||||
- free_and_replace(c->working_directory, k);
|
||||
-
|
||||
c->working_directory_home = false;
|
||||
+ free_and_replace(c->working_directory, k);
|
||||
}
|
||||
|
||||
c->working_directory_missing_ok = missing_ok;
|
||||
@@ -4228,8 +4247,11 @@ int unit_load_fragment(Unit *u) {
|
||||
return r;
|
||||
|
||||
r = load_from_path(u, k);
|
||||
- if (r < 0)
|
||||
+ if (r < 0) {
|
||||
+ if (r == -ENOEXEC)
|
||||
+ log_unit_notice(u, "Unit configuration has fatal error, unit will not be started.");
|
||||
return r;
|
||||
+ }
|
||||
|
||||
if (u->load_state == UNIT_STUB) {
|
||||
SET_FOREACH(t, u->names, i) {
|
||||
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
|
||||
index 12f48bf43..fd797b587 100644
|
||||
--- a/src/test/test-unit-file.c
|
||||
+++ b/src/test/test-unit-file.c
|
||||
@@ -146,7 +146,7 @@ static void test_config_parse_exec(void) {
|
||||
r = config_parse_exec(NULL, "fake", 4, "section", 1,
|
||||
"LValue", 0, "/RValue/ argv0 r1",
|
||||
&c, u);
|
||||
- assert_se(r == 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
assert_se(c1->command_next == NULL);
|
||||
|
||||
log_info("/* honour_argv0 */");
|
||||
@@ -161,7 +161,7 @@ static void test_config_parse_exec(void) {
|
||||
r = config_parse_exec(NULL, "fake", 3, "section", 1,
|
||||
"LValue", 0, "@/RValue",
|
||||
&c, u);
|
||||
- assert_se(r == 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
assert_se(c1->command_next == NULL);
|
||||
|
||||
log_info("/* no command, whitespace only, reset */");
|
||||
@@ -220,7 +220,7 @@ static void test_config_parse_exec(void) {
|
||||
"-@/RValue argv0 r1 ; ; "
|
||||
"/goo/goo boo",
|
||||
&c, u);
|
||||
- assert_se(r >= 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
c1 = c1->command_next;
|
||||
check_execcommand(c1, "/RValue", "argv0", "r1", NULL, true);
|
||||
|
||||
@@ -374,7 +374,7 @@ static void test_config_parse_exec(void) {
|
||||
r = config_parse_exec(NULL, "fake", 4, "section", 1,
|
||||
"LValue", 0, path,
|
||||
&c, u);
|
||||
- assert_se(r == 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
assert_se(c1->command_next == NULL);
|
||||
}
|
||||
|
||||
@@ -401,21 +401,21 @@ static void test_config_parse_exec(void) {
|
||||
r = config_parse_exec(NULL, "fake", 4, "section", 1,
|
||||
"LValue", 0, "/path\\",
|
||||
&c, u);
|
||||
- assert_se(r == 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
assert_se(c1->command_next == NULL);
|
||||
|
||||
log_info("/* missing ending ' */");
|
||||
r = config_parse_exec(NULL, "fake", 4, "section", 1,
|
||||
"LValue", 0, "/path 'foo",
|
||||
&c, u);
|
||||
- assert_se(r == 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
assert_se(c1->command_next == NULL);
|
||||
|
||||
log_info("/* missing ending ' with trailing backslash */");
|
||||
r = config_parse_exec(NULL, "fake", 4, "section", 1,
|
||||
"LValue", 0, "/path 'foo\\",
|
||||
&c, u);
|
||||
- assert_se(r == 0);
|
||||
+ assert_se(r == -ENOEXEC);
|
||||
assert_se(c1->command_next == NULL);
|
||||
|
||||
log_info("/* invalid space between modifiers */");
|
||||
--
|
||||
2.11.0
|
|
@ -33,6 +33,7 @@ SRC_URI += " \
|
|||
file://0018-check-for-uchar.h-in-configure.patch \
|
||||
file://0019-socket-util-don-t-fail-if-libc-doesn-t-support-IDN.patch \
|
||||
file://0020-back-port-233-don-t-use-the-unified-hierarchy-for-the-systemd.patch \
|
||||
file://0001-core-load-fragment-refuse-units-with-errors-in-certa.patch \
|
||||
"
|
||||
SRC_URI_append_libc-uclibc = "\
|
||||
file://0002-units-Prefer-getty-to-agetty-in-console-setup-system.patch \
|
||||
|
|
Loading…
Reference in New Issue