Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tor Gitlab #40020 - maint-0.3.5]: Fix seccomp sandbox rules for opening directories #2013

Open
wants to merge 3 commits into
base: maint-0.3.5
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changes/bug27315
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
o Minor bugfixes (linux seccomp2 sandbox):
- Fix a regression on sandboxing rules for the openat() syscall.
The fix for bug 25440 fixed the problem on systems with glibc >=
2.27 but broke tor on previous versions of glibc. We now apply
the correct seccomp rule according to the running glibc version.
Patch from Daniel Pinto. Fixes bug 27315; bugfix on 0.3.5.11.
9 changes: 9 additions & 0 deletions changes/bug40020
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
o Minor bugfixes (linux seccomp2 sandbox):
- Makes the seccomp sandbox allow the correct syscall for opendir
according to the running glibc version. The opendir function
either uses open or openat but the current code does not
differenciate between opendir and open calls. This adds a new
seccomp sandbox rule for opendir. This fixes crashes when
reloading torrc with sandbox enabled when running on glibc
2.15 to 2.21 and 2.26. Patch from Daniel Pinto. Fixes bug 40020;
bugfix on 0.3.5.11.
17 changes: 14 additions & 3 deletions src/app/main/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,9 @@ sandbox_init_filter(void)
#define OPEN(name) \
sandbox_cfg_allow_open_filename(&cfg, tor_strdup(name))

#define OPENDIR(dir) \
sandbox_cfg_allow_opendir_dirname(&cfg, tor_strdup(dir))

#define OPEN_DATADIR(name) \
sandbox_cfg_allow_open_filename(&cfg, get_datadir_fname(name))

Expand All @@ -1005,8 +1008,10 @@ sandbox_init_filter(void)
OPEN_DATADIR2(name, name2 suffix); \
} while (0)

// KeyDirectory is a directory, but it is only opened in check_private_dir
// which calls open instead of opendir
#define OPEN_KEY_DIRECTORY() \
sandbox_cfg_allow_open_filename(&cfg, tor_strdup(options->KeyDirectory))
OPEN(options->KeyDirectory)
#define OPEN_CACHEDIR(name) \
sandbox_cfg_allow_open_filename(&cfg, get_cachedir_fname(name))
#define OPEN_CACHEDIR_SUFFIX(name, suffix) do { \
Expand All @@ -1020,6 +1025,8 @@ sandbox_init_filter(void)
OPEN_KEYDIR(name suffix); \
} while (0)

// DataDirectory is a directory, but it is only opened in check_private_dir
// which calls open instead of opendir
OPEN(options->DataDirectory);
OPEN_KEY_DIRECTORY();

Expand Down Expand Up @@ -1067,7 +1074,11 @@ sandbox_init_filter(void)
}

SMARTLIST_FOREACH(options->FilesOpenedByIncludes, char *, f, {
OPEN(f);
if (file_status(f) == FN_DIR) {
OPENDIR(f);
} else {
OPEN(f);
}
});

#define RENAME_SUFFIX(name, suffix) \
Expand Down Expand Up @@ -1180,7 +1191,7 @@ sandbox_init_filter(void)
* directory that holds it. */
char *dirname = tor_strdup(port->unix_addr);
if (get_parent_directory(dirname) == 0) {
OPEN(dirname);
OPENDIR(dirname);
}
tor_free(dirname);
sandbox_cfg_allow_chmod_filename(&cfg, tor_strdup(port->unix_addr));
Expand Down
105 changes: 94 additions & 11 deletions src/lib/sandbox/sandbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ static sandbox_cfg_t *filter_dynamic = NULL;
* the high bits of the value might get masked out improperly. */
#define SCMP_CMP_MASKED(a,b,c) \
SCMP_CMP4((a), SCMP_CMP_MASKED_EQ, ~(scmp_datum_t)(b), (c))
/* For negative constants, the rule to add depends on the glibc version. */
#define SCMP_CMP_NEG(a,op,b) (libc_negative_constant_needs_cast() ? \
(SCMP_CMP((a), (op), (unsigned int)(b))) : \
(SCMP_CMP_STR((a), (op), (b))))

/** Variable used for storing all syscall numbers that will be allowed with the
* stage 1 general Tor sandbox.
Expand Down Expand Up @@ -272,6 +276,12 @@ static int filter_nopar_gen[] = {
SCMP_SYS(poll)
};

/* opendir is not a syscall but it will use either open or openat. We do not
* want the decision to allow open/openat to be the callers reponsability, so
* we create a phony syscall number for opendir and sb_opendir will choose the
* correct syscall. */
#define PHONY_OPENDIR_SYSCALL -2

/* These macros help avoid the error where the number of filters we add on a
* single rule don't match the arg_cnt param. */
#define seccomp_rule_add_0(ctx,act,call) \
Expand Down Expand Up @@ -424,39 +434,67 @@ sb_mmap2(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
#endif
#endif

/* Return true if we think we're running with a libc that always uses
* openat on linux. */
/* Return true the libc version is greater or equal than
* <b>major</b>.<b>minor</b>. Returns false otherwise. */
static int
libc_uses_openat_for_everything(void)
is_libc_at_least(int major, int minor)
{
#ifdef CHECK_LIBC_VERSION
const char *version = gnu_get_libc_version();
if (version == NULL)
return 0;

int major = -1;
int minor = -1;
int libc_major = -1;
int libc_minor = -1;

tor_sscanf(version, "%d.%d", &major, &minor);
if (major >= 3)
tor_sscanf(version, "%d.%d", &libc_major, &libc_minor);
if (libc_major > major)
return 1;
else if (major == 2 && minor >= 26)
else if (libc_major == major && libc_minor >= minor)
return 1;
else
return 0;
#else /* !(defined(CHECK_LIBC_VERSION)) */
(void)major;
(void)minor;
return 0;
#endif /* defined(CHECK_LIBC_VERSION) */
}

/* Return true if we think we're running with a libc that uses openat for the
* open function on linux. */
static int
libc_uses_openat_for_open(void)
{
return is_libc_at_least(2, 26);
}

/* Return true if we think we're running with a libc that uses openat for the
* opendir function on linux. */
static int
libc_uses_openat_for_opendir(void)
{
// libc 2.27 and above or between 2.15 (inclusive) and 2.22 (exclusive)
return is_libc_at_least(2, 27) ||
(is_libc_at_least(2, 15) && !is_libc_at_least(2, 22));
}

/* Return true if we think we're running with a libc that needs to cast
* negative arguments like AT_FDCWD for seccomp rules. */
static int
libc_negative_constant_needs_cast(void)
{
return is_libc_at_least(2, 27);
}

/** Allow a single file to be opened. If <b>use_openat</b> is true,
* we're using a libc that remaps all the opens into openats. */
static int
allow_file_open(scmp_filter_ctx ctx, int use_openat, const char *file)
{
if (use_openat) {
return seccomp_rule_add_2(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat),
SCMP_CMP(0, SCMP_CMP_EQ, (unsigned int)AT_FDCWD),
SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD),
SCMP_CMP_STR(1, SCMP_CMP_EQ, file));
} else {
return seccomp_rule_add_1(ctx, SCMP_ACT_ALLOW, SCMP_SYS(open),
Expand All @@ -474,7 +512,7 @@ sb_open(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
int rc;
sandbox_cfg_t *elem = NULL;

int use_openat = libc_uses_openat_for_everything();
int use_openat = libc_uses_openat_for_open();

// for each dynamic parameter filters
for (elem = filter; elem != NULL; elem = elem->next) {
Expand Down Expand Up @@ -592,7 +630,7 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
if (param != NULL && param->prot == 1 && param->syscall
== SCMP_SYS(openat)) {
rc = seccomp_rule_add_3(ctx, SCMP_ACT_ALLOW, SCMP_SYS(openat),
SCMP_CMP(0, SCMP_CMP_EQ, AT_FDCWD),
SCMP_CMP_NEG(0, SCMP_CMP_EQ, AT_FDCWD),
SCMP_CMP_STR(1, SCMP_CMP_EQ, param->value),
SCMP_CMP(2, SCMP_CMP_EQ, O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|
O_CLOEXEC));
Expand All @@ -607,6 +645,30 @@ sb_openat(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
return 0;
}

static int
sb_opendir(scmp_filter_ctx ctx, sandbox_cfg_t *filter)
{
int rc;
sandbox_cfg_t *elem = NULL;

// for each dynamic parameter filters
for (elem = filter; elem != NULL; elem = elem->next) {
smp_param_t *param = elem->param;

if (param != NULL && param->prot == 1 && param->syscall
== PHONY_OPENDIR_SYSCALL) {
rc = allow_file_open(ctx, libc_uses_openat_for_opendir(), param->value);
if (rc != 0) {
log_err(LD_BUG,"(Sandbox) failed to add openat syscall, received "
"libseccomp error %d", rc);
return rc;
}
}
}

return 0;
}

/**
* Function responsible for setting up the socket syscall for
* the seccomp filter sandbox.
Expand Down Expand Up @@ -1106,6 +1168,7 @@ static sandbox_filter_func_t filter_func[] = {
sb_chmod,
sb_open,
sb_openat,
sb_opendir,
sb_rename,
#ifdef __NR_fcntl64
sb_fcntl64,
Expand Down Expand Up @@ -1425,6 +1488,19 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file)
return 0;
}

int
sandbox_cfg_allow_opendir_dirname(sandbox_cfg_t **cfg, char *dir)
{
sandbox_cfg_t *elem = NULL;

elem = new_element(PHONY_OPENDIR_SYSCALL, dir);

elem->next = *cfg;
*cfg = elem;

return 0;
}

/**
* Function responsible for going through the parameter syscall filters and
* call each function pointer in the list.
Expand Down Expand Up @@ -1730,6 +1806,13 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file)
return 0;
}

int
sandbox_cfg_allow_opendir_dirname(sandbox_cfg_t **cfg, char *dir)
{
(void)cfg; (void)dir;
return 0;
}

int
sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file)
{
Expand Down
7 changes: 7 additions & 0 deletions src/lib/sandbox/sandbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2);
*/
int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file);

/**
* Function used to add a opendir allowed filename to a supplied configuration.
* The (char*) specifies the path to the allowed dir; we steal the pointer to
* that dir.
*/
int sandbox_cfg_allow_opendir_dirname(sandbox_cfg_t **cfg, char *dir);

/**
* Function used to add a stat/stat64 allowed filename to a configuration.
* The (char*) specifies the path to the allowed file; that pointer is stolen.
Expand Down