diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index a4f92775ae..d28de2d0f2 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1053,14 +1053,19 @@ After= dependencies on all mount units necessary to access /tmp and /var/tmp. Moreover an implicitly After= ordering on systemd-tmpfiles-setup.service8 - is added. + is added. + + 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. PrivateDevices= - Takes a boolean argument. If true, sets up a new /dev namespace for the executed processes and - only adds API pseudo devices such as /dev/null, /dev/zero or + Takes a boolean argument. If true, sets up a new /dev mount for the + executed processes and only adds API pseudo devices such as /dev/null, + /dev/zero or /dev/random (as well as the pseudo TTY subsystem) to it, but no physical devices such as /dev/sda, system memory /dev/mem, system ports /dev/port and others. This is useful to securely turn off physical device access by the @@ -1071,8 +1076,8 @@ systemd.resource-control5 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 /dev + will be mounted read-only and 'noexec'. The latter may break old programs which try to set up executable memory by using mmap2 of /dev/zero instead of using MAP_ANON. This setting is implied if DynamicUser= 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 CAP_SYS_ADMIN capability (e.g. setting User=), NoNewPrivileges=yes is implied. - + + + 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. @@ -1091,7 +1100,7 @@ configures only the loopback network device lo 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 JoinsNamespaceOf= 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). + accessible). + + 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. @@ -1123,7 +1136,11 @@ This setting is particularly useful in conjunction with RootDirectory=/RootImage=, 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 root, nobody and the unit's own user and group. + are root, nobody and the unit's own user and group. + + 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. diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index c43b7885be..5b5a86250e 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -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) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 6a6dadda2b..9d5c39b3dd 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -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) { diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 44df7493e2..e08402e3d2 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -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); diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index 12f48bf435..fd797b587e 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 */");