diff --git a/changes/bug27315 b/changes/bug27315
new file mode 100644
index 00000000000..8af3ac85591
--- /dev/null
+++ b/changes/bug27315
@@ -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.
diff --git a/changes/bug40020 b/changes/bug40020
new file mode 100644
index 00000000000..ca6ee2b85b8
--- /dev/null
+++ b/changes/bug40020
@@ -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.
diff --git a/src/app/main/main.c b/src/app/main/main.c
index 67f2181cd5c..3f35d4d23ff 100644
--- a/src/app/main/main.c
+++ b/src/app/main/main.c
@@ -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))
@@ -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 { \
@@ -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();
@@ -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) \
@@ -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));
diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c
index 8f577b06604..2f26c5429be 100644
--- a/src/lib/sandbox/sandbox.c
+++ b/src/lib/sandbox/sandbox.c
@@ -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.
@@ -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) \
@@ -424,31 +434,59 @@ 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
+ * major.minor. 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 use_openat is true,
* we're using a libc that remaps all the opens into openats. */
static int
@@ -456,7 +494,7 @@ 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),
@@ -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) {
@@ -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));
@@ -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.
@@ -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,
@@ -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.
@@ -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)
{
diff --git a/src/lib/sandbox/sandbox.h b/src/lib/sandbox/sandbox.h
index 5bec09a36a2..8542b57f9c1 100644
--- a/src/lib/sandbox/sandbox.h
+++ b/src/lib/sandbox/sandbox.h
@@ -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.