picnoir
/
Systemd
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #6300 from keszybz/refuse-to-load-some-units 

Refuse to load some units
This commit is contained in:
Lennart Poettering 2017-07-12 09:28:20 +02:00 committed by GitHub
parent 7f7ab22892 b023856884
commit 6297d07b82
5 changed files with 123 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
man/systemd.exec.xml
View File

@ -1053,14 +1053,19 @@
<varname>After=</varname> dependencies on all mount units necessary to access <filename>/tmp</filename> and
<filename>/var/tmp</filename>. Moreover an implicitly <varname>After=</varname> ordering on
<citerefentry><refentrytitle>systemd-tmpfiles-setup.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>
is added.</para></listitem>
is added.</para>
<para>Note that the implementation of this setting might be impossible (for example if mount namespaces
are not available), and the unit should be written in a way that does not solely rely on this setting for
security.</para></listitem>
</varlistentry>
<varlistentry>
<term><varname>PrivateDevices=</varname></term>
<listitem><para>Takes a boolean argument. If true, sets up a new /dev namespace for the executed processes and
only adds API pseudo devices such as <filename>/dev/null</filename>, <filename>/dev/zero</filename> or
<listitem><para>Takes a boolean argument. If true, sets up a new <filename>/dev</filename> mount for the
executed processes and only adds API pseudo devices such as <filename>/dev/null</filename>,
<filename>/dev/zero</filename> or
<filename>/dev/random</filename> (as well as the pseudo TTY subsystem) to it, but no physical devices such as
<filename>/dev/sda</filename>, system memory <filename>/dev/mem</filename>, system ports
<filename>/dev/port</filename> and others. This is useful to securely turn off physical device access by the
@ -1071,8 +1076,8 @@
<citerefentry><refentrytitle>systemd.resource-control</refentrytitle><manvolnum>5</manvolnum></citerefentry>
for details). Note that using this setting will disconnect propagation of mounts from the service to the host
(propagation in the opposite direction continues to work). This means that this setting may not be used for
services which shall be able to install mount points in the main mount namespace. The /dev namespace will be
mounted read-only and 'noexec'. The latter may break old programs which try to set up executable memory by
services which shall be able to install mount points in the main mount namespace. The new <filename>/dev</filename>
will be mounted read-only and 'noexec'. The latter may break old programs which try to set up executable memory by
using <citerefentry><refentrytitle>mmap</refentrytitle><manvolnum>2</manvolnum></citerefentry> of
<filename>/dev/zero</filename> instead of using <constant>MAP_ANON</constant>. This setting is implied if
<varname>DynamicUser=</varname> is set. For this setting the same restrictions regarding mount propagation and
@ -1080,7 +1085,11 @@
If turned on and if running in user mode, or in system mode, but without the <constant>CAP_SYS_ADMIN</constant>
capability (e.g. setting <varname>User=</varname>), <varname>NoNewPrivileges=yes</varname>
is implied.
</para></listitem>
</para>
<para>Note that the implementation of this setting might be impossible (for example if mount namespaces
are not available), and the unit should be written in a way that does not solely rely on this setting for
security.</para></listitem>
</varlistentry>
<varlistentry>
@ -1091,7 +1100,7 @@
configures only the loopback network device
<literal>lo</literal> inside it. No other network devices will
be available to the executed process. This is useful to
securely turn off network access by the executed process.
turn off network access by the executed process.
Defaults to false. It is possible to run two or more units
within the same private network namespace by using the
<varname>JoinsNamespaceOf=</varname> directive, see
@ -1101,7 +1110,11 @@
The latter has the effect that AF_UNIX sockets in the abstract
socket namespace will become unavailable to the processes
(however, those located in the file system will continue to be
accessible).</para></listitem>
accessible).</para>
<para>Note that the implementation of this setting might be impossible (for example if network namespaces
are not available), and the unit should be written in a way that does not solely rely on this setting for
security.</para></listitem>
</varlistentry>
<varlistentry>
@ -1123,7 +1136,11 @@
<para>This setting is particularly useful in conjunction with
<varname>RootDirectory=</varname>/<varname>RootImage=</varname>, as the need to synchronize the user and group
databases in the root directory and on the host is reduced, as the only users and groups who need to be matched
are <literal>root</literal>, <literal>nobody</literal> and the unit's own user and group.</para></listitem>
are <literal>root</literal>, <literal>nobody</literal> and the unit's own user and group.</para>
<para>Note that the implementation of this setting might be impossible (for example if user namespaces
are not available), and the unit should be written in a way that does not solely rely on this setting for
security.</para></listitem>
</varlistentry>
<varlistentry>

6
src/core/load-fragment-gperf.gperf.m4
View File

@ -18,8 +18,8 @@ struct ConfigPerfItem;
m4_dnl Define the context options only once
m4_define(`EXEC_CONTEXT_CONFIG_ITEMS',
`$1.WorkingDirectory, config_parse_working_directory, 0, offsetof($1, exec_context)
$1.RootDirectory, config_parse_unit_path_printf, 0, offsetof($1, exec_context.root_directory)
$1.RootImage, config_parse_unit_path_printf, 0, offsetof($1, exec_context.root_image)
$1.RootDirectory, config_parse_unit_path_printf, true, offsetof($1, exec_context.root_directory)
$1.RootImage, config_parse_unit_path_printf, true, offsetof($1, exec_context.root_image)
$1.User, config_parse_user_group, 0, offsetof($1, exec_context.user)
$1.Group, config_parse_user_group, 0, offsetof($1, exec_context.group)
$1.SupplementaryGroups, config_parse_user_group_strv, 0, offsetof($1, exec_context.supplementary_groups)
@ -35,7 +35,7 @@ $1.UMask, config_parse_mode, 0,
$1.Environment, config_parse_environ, 0, offsetof($1, exec_context.environment)
$1.EnvironmentFile, config_parse_unit_env_file, 0, offsetof($1, exec_context.environment_files)
$1.PassEnvironment, config_parse_pass_environ, 0, offsetof($1, exec_context.pass_environment)
$1.DynamicUser, config_parse_bool, 0, offsetof($1, exec_context.dynamic_user)
$1.DynamicUser, config_parse_bool, true, offsetof($1, exec_context.dynamic_user)
$1.StandardInput, config_parse_exec_input, 0, offsetof($1, exec_context)
$1.StandardOutput, config_parse_exec_output, 0, offsetof($1, exec_context)
$1.StandardError, config_parse_exec_output, 0, offsetof($1, exec_context)

123
src/core/load-fragment.c
View File

@ -242,6 +242,7 @@ int config_parse_unit_path_printf(
_cleanup_free_ char *k = NULL;
Unit *u = userdata;
int r;
bool fatal = ltype;
assert(filename);
assert(lvalue);
@ -250,8 +251,10 @@ int config_parse_unit_path_printf(
r = unit_full_printf(u, rvalue, &k);
if (r < 0) {
log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s, ignoring: %m", rvalue);
return 0;
log_syntax(unit, LOG_ERR, filename, line, r,
"Failed to resolve unit specifiers on %s%s: %m",
fatal ? "" : ", ignoring", rvalue);
return fatal ? -ENOEXEC : 0;
}
return config_parse_path(unit, filename, line, section, section_line, lvalue, ltype, k, data, userdata);
@ -636,26 +639,36 @@ int config_parse_exec(
r = unit_full_printf(u, f, &path);
if (r < 0) {
log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s, ignoring: %m", f);
return 0;
log_syntax(unit, LOG_ERR, filename, line, r,
"Failed to resolve unit specifiers on %s%s: %m",
f, ignore ? ", ignoring" : "");
return ignore ? 0 : -ENOEXEC;
}
if (isempty(path)) {
/* 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(path)) {
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(path)) {
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(path, "/")) {
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 (!separate_argv0) {
@ -708,12 +721,14 @@ int config_parse_exec(
if (r == 0)
break;
if (r < 0)
return 0;
return ignore ? 0 : -ENOEXEC;
r = unit_full_printf(u, word, &resolved);
if (r < 0) {
log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to resolve unit specifiers on %s, ignoring: %m", word);
return 0;
log_syntax(unit, LOG_ERR, filename, line, r,
"Failed to resolve unit specifiers on %s%s: %m",
word, ignore ? ", ignoring" : "");
return ignore ? 0 : -ENOEXEC;
}
if (!GREEDY_REALLOC(n, nbufsize, nlen + 2))
@ -724,8 +739,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);
@ -1332,8 +1349,10 @@ int config_parse_exec_selinux_context(
r = unit_full_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);
@ -1380,8 +1399,10 @@ int config_parse_exec_apparmor_profile(
r = unit_full_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);
@ -1428,8 +1449,10 @@ int config_parse_exec_smack_process_label(
r = unit_full_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);
@ -1647,19 +1670,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);
@ -1907,13 +1930,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;
@ -1971,19 +1994,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);
@ -2142,25 +2165,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;
@ -4456,8 +4482,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) {

16
src/shared/conf-parser.c
View File

@ -615,6 +615,7 @@ int config_parse_bool(const char* unit,
int k;
bool *b = data;
bool fatal = ltype;
assert(filename);
assert(lvalue);
@ -623,8 +624,10 @@ int config_parse_bool(const char* unit,
k = parse_boolean(rvalue);
if (k < 0) {
log_syntax(unit, LOG_ERR, filename, line, k, "Failed to parse boolean value, ignoring: %s", rvalue);
return 0;
log_syntax(unit, LOG_ERR, filename, line, k,
"Failed to parse boolean value%s: %s",
fatal ? "" : ", ignoring", rvalue);
return fatal ? -ENOEXEC : 0;
}
*b = !!k;
@ -715,6 +718,7 @@ int config_parse_path(
void *userdata) {
char **s = data, *n;
bool fatal = ltype;
assert(filename);
assert(lvalue);
@ -723,12 +727,14 @@ int config_parse_path(
if (!utf8_is_valid(rvalue)) {
log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, rvalue);
return 0;
return fatal ? -ENOEXEC : 0;
}
if (!path_is_absolute(rvalue)) {
log_syntax(unit, LOG_ERR, filename, line, 0, "Not an absolute path, ignoring: %s", rvalue);
return 0;
log_syntax(unit, LOG_ERR, filename, line, 0,
"Not an absolute path%s: %s",
fatal ? "" : ", ignoring", rvalue);
return fatal ? -ENOEXEC : 0;
}
n = strdup(rvalue);

14
src/test/test-unit-file.c
View File

@ -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 */");