1
0
mirror of https://github.com/systemd/systemd.git synced 2024-12-23 21:35:11 +03:00

Merge pull request #31210 from poettering/chdir-hardening

WorkingDirectory= hardening
This commit is contained in:
Luca Boccassi 2024-02-06 12:41:43 +00:00 committed by GitHub
commit d50f58d641
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 146 additions and 57 deletions

View File

@ -784,3 +784,10 @@ int mount_option_supported(const char *fstype, const char *key, const char *valu
return true; /* works! */
}
bool path_below_api_vfs(const char *p) {
assert(p);
/* API VFS are either directly mounted on any of these three paths, or below it. */
return PATH_STARTSWITH_SET(p, "/dev", "/sys", "/proc");
}

View File

@ -73,3 +73,5 @@ bool mount_new_api_supported(void);
unsigned long ms_nosymfollow_supported(void);
int mount_option_supported(const char *fstype, const char *key, const char *value);
bool path_below_api_vfs(const char *p);

View File

@ -2721,8 +2721,9 @@ int bus_exec_context_set_transient_property(
return 1;
} else if (streq(name, "WorkingDirectory")) {
_cleanup_free_ char *simplified = NULL;
bool missing_ok, is_home;
const char *s;
bool missing_ok;
r = sd_bus_message_read(message, "s", &s);
if (r < 0)
@ -2734,23 +2735,33 @@ int bus_exec_context_set_transient_property(
} else
missing_ok = false;
if (!isempty(s) && !streq(s, "~") && !path_is_absolute(s))
return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "WorkingDirectory= expects an absolute path or '~'");
if (isempty(s))
is_home = false;
else if (streq(s, "~"))
is_home = true;
else {
if (!path_is_absolute(s))
return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "WorkingDirectory= expects an absolute path or '~'");
r = path_simplify_alloc(s, &simplified);
if (r < 0)
return r;
if (!path_is_normalized(simplified))
return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "WorkingDirectory= expects a normalized path or '~'");
if (path_below_api_vfs(simplified))
return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "WorkingDirectory= may not be below /proc/, /sys/ or /dev/.");
is_home = false;
}
if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
if (streq(s, "~")) {
c->working_directory = mfree(c->working_directory);
c->working_directory_home = true;
} else {
r = free_and_strdup(&c->working_directory, empty_to_null(s));
if (r < 0)
return r;
c->working_directory_home = false;
}
free_and_replace(c->working_directory, simplified);
c->working_directory_home = is_home;
c->working_directory_missing_ok = missing_ok;
unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "WorkingDirectory=%s%s", missing_ok ? "-" : "", s);
unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "WorkingDirectory=%s%s", missing_ok ? "-" : "", c->working_directory_home ? "+" : ASSERT_PTR(c->working_directory));
}
return 1;

View File

@ -3337,31 +3337,39 @@ static int apply_working_directory(
const char *home,
int *exit_status) {
const char *d, *wd;
const char *wd;
int r;
assert(context);
assert(exit_status);
if (context->working_directory_home) {
if (!home) {
*exit_status = EXIT_CHDIR;
return -ENXIO;
}
wd = home;
} else
wd = empty_to_root(context->working_directory);
if (params->flags & EXEC_APPLY_CHROOT)
d = wd;
else
d = prefix_roota((runtime ? runtime->ephemeral_copy : NULL) ?: context->root_directory, wd);
r = RET_NERRNO(chdir(wd));
else {
_cleanup_close_ int dfd = -EBADF;
if (chdir(d) < 0 && !context->working_directory_missing_ok) {
r = chase(wd,
(runtime ? runtime->ephemeral_copy : NULL) ?: context->root_directory,
CHASE_PREFIX_ROOT|CHASE_AT_RESOLVE_IN_ROOT,
/* ret_path= */ NULL,
&dfd);
if (r >= 0)
r = RET_NERRNO(fchdir(dfd));
}
if (r < 0 && !context->working_directory_missing_ok) {
*exit_status = EXIT_CHDIR;
return -errno;
return r;
}
return 0;
@ -5032,8 +5040,10 @@ int exec_invoke(
}
}
/* Apply working directory here, because the working directory might be on NFS and only the user running
* this service might have the correct privilege to change to the working directory */
/* Apply working directory here, because the working directory might be on NFS and only the user
* running this service might have the correct privilege to change to the working directory. Also, it
* is absolutely 💣 crucial 💣 we applied all mount namespacing rearrangements before this, so that
* the cwd cannot be used to pin directories outside of the sandbox. */
r = apply_working_directory(context, params, runtime, home, exit_status);
if (r < 0)
return log_exec_error_errno(context, params, r, "Changing to the requested working directory failed: %m");

View File

@ -606,7 +606,7 @@ int config_parse_socket_listen(
return 0;
}
r = path_simplify_and_warn(k, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue);
r = path_simplify_and_warn(k, PATH_CHECK_ABSOLUTE|PATH_CHECK_NON_API_VFS, unit, filename, line, lvalue);
if (r < 0)
return 0;
@ -2660,7 +2660,7 @@ int config_parse_working_directory(
return missing_ok ? 0 : -ENOEXEC;
}
r = path_simplify_and_warn(k, PATH_CHECK_ABSOLUTE | (missing_ok ? 0 : PATH_CHECK_FATAL), unit, filename, line, lvalue);
r = path_simplify_and_warn(k, PATH_CHECK_ABSOLUTE|PATH_CHECK_NON_API_VFS|(missing_ok ? 0 : PATH_CHECK_FATAL), unit, filename, line, lvalue);
if (r < 0)
return missing_ok ? 0 : -ENOEXEC;
@ -5422,7 +5422,7 @@ int config_parse_mount_images(
continue;
}
r = path_simplify_and_warn(sresolved, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue);
r = path_simplify_and_warn(sresolved, PATH_CHECK_ABSOLUTE|PATH_CHECK_NON_API_VFS, unit, filename, line, lvalue);
if (r < 0)
continue;
@ -5438,7 +5438,7 @@ int config_parse_mount_images(
continue;
}
r = path_simplify_and_warn(dresolved, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue);
r = path_simplify_and_warn(dresolved, PATH_CHECK_ABSOLUTE|PATH_CHECK_NON_API_VFS, unit, filename, line, lvalue);
if (r < 0)
continue;
@ -5580,7 +5580,7 @@ int config_parse_extension_images(
continue;
}
r = path_simplify_and_warn(sresolved, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue);
r = path_simplify_and_warn(sresolved, PATH_CHECK_ABSOLUTE|PATH_CHECK_NON_API_VFS, unit, filename, line, lvalue);
if (r < 0)
continue;
@ -5801,7 +5801,7 @@ int config_parse_pid_file(
return log_oom();
/* Check that the result is a sensible path */
r = path_simplify_and_warn(n, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue);
r = path_simplify_and_warn(n, PATH_CHECK_ABSOLUTE|PATH_CHECK_NON_API_VFS, unit, filename, line, lvalue);
if (r < 0)
return r;

View File

@ -1368,17 +1368,27 @@ static int parse_argv(int argc, char *argv[]) {
break;
case ARG_CHDIR:
case ARG_CHDIR: {
_cleanup_free_ char *wd = NULL;
if (!path_is_absolute(optarg))
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"Working directory %s is not an absolute path.", optarg);
r = free_and_strdup(&arg_chdir, optarg);
r = path_simplify_alloc(optarg, &wd);
if (r < 0)
return log_oom();
return log_error_errno(r, "Failed to simplify path %s: %m", optarg);
if (!path_is_normalized(wd))
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Working dirctory path is not normalized: %s", wd);
if (path_below_api_vfs(wd))
return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Working directory is below API VFS, refusing: %s", wd);
free_and_replace(arg_chdir, wd);
arg_settings_mask |= SETTING_WORKING_DIRECTORY;
break;
}
case ARG_PIVOT_ROOT:
r = pivot_root_parse(&arg_pivot_root_new, &arg_pivot_root_old, optarg);
@ -3512,6 +3522,9 @@ static int inner_child(
if (!barrier_place_and_sync(barrier)) /* #5 */
return log_error_errno(SYNTHETIC_ERRNO(ESRCH), "Parent died too early");
/* Note, this should be done this late (💣 and not moved earlier! 💣), so that all namespacing
* changes are already in effect by now, so that any resolved paths here definitely reference
* resources inside the container, and not outside of them. */
if (arg_chdir)
if (chdir(arg_chdir) < 0)
return log_error_errno(errno, "Failed to change to specified working directory %s: %m", arg_chdir);

View File

@ -4,6 +4,7 @@
#include "extract-word.h"
#include "ip-protocol-list.h"
#include "log.h"
#include "mountpoint-util.h"
#include "parse-helpers.h"
#include "parse-util.h"
#include "path-util.h"
@ -11,47 +12,56 @@
int path_simplify_and_warn(
char *path,
unsigned flag,
PathSimplifyWarnFlags flags,
const char *unit,
const char *filename,
unsigned line,
const char *lvalue) {
bool fatal = flag & PATH_CHECK_FATAL;
bool fatal = flags & PATH_CHECK_FATAL;
int level = fatal ? LOG_ERR : LOG_WARNING;
assert(!FLAGS_SET(flag, PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE));
assert(path);
assert(!FLAGS_SET(flags, PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE));
assert(lvalue);
if (!utf8_is_valid(path))
return log_syntax_invalid_utf8(unit, LOG_ERR, filename, line, path);
if (flag & (PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)) {
if (flags & (PATH_CHECK_ABSOLUTE | PATH_CHECK_RELATIVE)) {
bool absolute;
absolute = path_is_absolute(path);
if (!absolute && (flag & PATH_CHECK_ABSOLUTE))
return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL),
if (!absolute && (flags & PATH_CHECK_ABSOLUTE))
return log_syntax(unit, level, filename, line, SYNTHETIC_ERRNO(EINVAL),
"%s= path is not absolute%s: %s",
lvalue, fatal ? "" : ", ignoring", path);
if (absolute && (flag & PATH_CHECK_RELATIVE))
return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL),
if (absolute && (flags & PATH_CHECK_RELATIVE))
return log_syntax(unit, level, filename, line, SYNTHETIC_ERRNO(EINVAL),
"%s= path is absolute%s: %s",
lvalue, fatal ? "" : ", ignoring", path);
}
path_simplify_full(path, flag & PATH_KEEP_TRAILING_SLASH ? PATH_SIMPLIFY_KEEP_TRAILING_SLASH : 0);
path_simplify_full(path, flags & PATH_KEEP_TRAILING_SLASH ? PATH_SIMPLIFY_KEEP_TRAILING_SLASH : 0);
if (!path_is_valid(path))
return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL),
return log_syntax(unit, level, filename, line, SYNTHETIC_ERRNO(EINVAL),
"%s= path has invalid length (%zu bytes)%s.",
lvalue, strlen(path), fatal ? "" : ", ignoring");
if (!path_is_normalized(path))
return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL),
return log_syntax(unit, level, filename, line, SYNTHETIC_ERRNO(EINVAL),
"%s= path is not normalized%s: %s",
lvalue, fatal ? "" : ", ignoring", path);
if (FLAGS_SET(flags, PATH_CHECK_NON_API_VFS) && path_below_api_vfs(path))
return log_syntax(unit, level, filename, line, SYNTHETIC_ERRNO(EINVAL),
"%s= path is below API VFS%s: %s",
lvalue, fatal ? ", refusing" : ", ignoring",
path);
return 0;
}

View File

@ -3,27 +3,28 @@
#include <stdint.h>
enum {
PATH_CHECK_FATAL = 1 << 0, /* If not set, then error message is appended with 'ignoring'. */
PATH_CHECK_ABSOLUTE = 1 << 1,
PATH_CHECK_RELATIVE = 1 << 2,
typedef enum PathSimplifyWarnFlags {
PATH_CHECK_FATAL = 1 << 0, /* If not set, then error message is appended with 'ignoring'. */
PATH_CHECK_ABSOLUTE = 1 << 1,
PATH_CHECK_RELATIVE = 1 << 2,
PATH_KEEP_TRAILING_SLASH = 1 << 3,
};
PATH_CHECK_NON_API_VFS = 1 << 4,
} PathSimplifyWarnFlags;
int path_simplify_and_warn(
char *path,
unsigned flag,
PathSimplifyWarnFlags flags,
const char *unit,
const char *filename,
unsigned line,
const char *lvalue);
int parse_socket_bind_item(
const char *str,
int *address_family,
int *ip_protocol,
uint16_t *nr_ports,
uint16_t *port_min);
const char *str,
int *address_family,
int *ip_protocol,
uint16_t *nr_ports,
uint16_t *port_min);
int config_parse_path_or_ignore(
const char *unit,

View File

@ -95,7 +95,7 @@ int mount_points_list_get(const char *mountinfo, MountPoint **head) {
* we might lack the rights to unmount these things, hence don't bother. */
if (mount_point_is_api(path) ||
mount_point_ignore(path) ||
PATH_STARTSWITH_SET(path, "/dev", "/sys", "/proc"))
path_below_api_vfs(path))
continue;
is_api_vfs = fstype_is_api_vfs(fstype);

View File

@ -93,4 +93,39 @@ TEST(invalid_items) {
test_invalid_item("ipv6:tcp:6666\n zupa");
}
static int test_path_simplify_and_warn_one(const char *p, const char *q, PathSimplifyWarnFlags f) {
_cleanup_free_ char *s = ASSERT_PTR(strdup(p));
int a, b;
a = path_simplify_and_warn(s, f, /* unit= */ NULL, /* filename= */ NULL, /* line= */ 0, "Foobar=");
assert(streq_ptr(s, q));
free(s);
s = ASSERT_PTR(strdup(p));
b = path_simplify_and_warn(s, f|PATH_CHECK_FATAL, /* unit= */ NULL, /* filename= */ NULL, /* line= */ 0, "Foobar=");
assert(streq_ptr(s, q));
assert(a == b);
return a;
}
TEST(path_simplify_and_warn) {
assert_se(test_path_simplify_and_warn_one("", "", 0) == -EINVAL);
assert_se(test_path_simplify_and_warn_one("/", "/", 0) == 0);
assert_se(test_path_simplify_and_warn_one("/foo/../bar", "/foo/../bar", 0) == -EINVAL);
assert_se(test_path_simplify_and_warn_one("/foo/./bar", "/foo/bar", 0) == 0);
assert_se(test_path_simplify_and_warn_one("/proc/self///fd", "/proc/self/fd", 0) == 0);
assert_se(test_path_simplify_and_warn_one("/proc/self///fd", "/proc/self/fd", PATH_CHECK_NON_API_VFS) == -EINVAL);
assert_se(test_path_simplify_and_warn_one("aaaa", "aaaa", 0) == 0);
assert_se(test_path_simplify_and_warn_one("aaaa", "aaaa", PATH_CHECK_ABSOLUTE) == -EINVAL);
assert_se(test_path_simplify_and_warn_one("aaaa", "aaaa", PATH_CHECK_RELATIVE) == 0);
assert_se(test_path_simplify_and_warn_one("/aaaa", "/aaaa", 0) == 0);
assert_se(test_path_simplify_and_warn_one("/aaaa", "/aaaa", PATH_CHECK_ABSOLUTE) == 0);
assert_se(test_path_simplify_and_warn_one("/aaaa", "/aaaa", PATH_CHECK_RELATIVE) == -EINVAL);
}
DEFINE_TEST_MAIN(LOG_INFO);