diff options
| -rw-r--r-- | ChangeLog | 25 | ||||
| -rw-r--r-- | elf/Makefile | 6 | ||||
| -rw-r--r-- | elf/dl-tunable-types.h | 15 | ||||
| -rw-r--r-- | elf/dl-tunables.c | 119 | ||||
| -rw-r--r-- | elf/dl-tunables.h | 15 | ||||
| -rw-r--r-- | elf/dl-tunables.list | 16 | ||||
| -rw-r--r-- | elf/tst-env-setuid-tunables.c | 60 | ||||
| -rw-r--r-- | elf/tst-env-setuid.c | 282 | ||||
| -rw-r--r-- | scripts/gen-tunables.awk | 8 |
9 files changed, 511 insertions, 35 deletions
diff --git a/ChangeLog b/ChangeLog index b2f41e3da6..aed8764272 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2017-02-02 Siddhesh Poyarekar <siddhesh@sourceware.org> + + [BZ #21073] + * elf/dl-tunable-types.h (tunable_seclevel_t): New enum. + * elf/dl-tunables.c (tunables_strdup): Remove. + (get_next_env): Also return the previous envp. + (parse_tunables): Erase tunables of category + TUNABLES_SECLEVEL_SXID_ERASE. + (maybe_enable_malloc_check): Make MALLOC_CHECK_ + TUNABLE_SECLEVEL_NONE if /etc/setuid-debug is accessible. + (__tunables_init)[TUNABLES_FRONTEND == + TUNABLES_FRONTEND_valstring]: Update GLIBC_TUNABLES envvar + after parsing. + [TUNABLES_FRONTEND != TUNABLES_FRONTEND_valstring]: Erase + tunable envvars of category TUNABLES_SECLEVEL_SXID_ERASE. + * elf/dl-tunables.h (struct _tunable): Change member is_secure + to security_level. + * elf/dl-tunables.list: Add security_level annotations for all + tunables. + * scripts/gen-tunables.awk: Recognize and generate enum values + for security_level. + * elf/tst-env-setuid.c: New test case. + * elf/tst-env-setuid-tunables: new test case. + * elf/Makefile (tests-static): Add them. + 2017-02-01 Richard Henderson <rth@twiddle.net> * sysdeps/alpha/memchr.c (__memchr): Use saturating arithmetic diff --git a/elf/Makefile b/elf/Makefile index c7a29693bd..61abeb59ee 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -149,7 +149,7 @@ tests-static = tst-tls1-static tst-tls2-static tst-stackguard1-static \ tst-leaks1-static tst-array1-static tst-array5-static \ tst-ptrguard1-static tst-dl-iter-static \ tst-tlsalign-static tst-tlsalign-extern-static \ - tst-linkall-static + tst-linkall-static tst-env-setuid tst-env-setuid-tunables ifeq (yes,$(build-shared)) tests-static += tst-tls9-static tst-tls9-static-ENV = \ @@ -1397,3 +1397,7 @@ $(objpfx)tst-nodelete-dlclose-plugin.so: $(objpfx)tst-nodelete-dlclose-dso.so $(objpfx)tst-nodelete-dlclose: $(objpfx)tst-nodelete-dlclose-dso.so $(objpfx)tst-nodelete-dlclose.out: $(objpfx)tst-nodelete-dlclose-dso.so \ $(objpfx)tst-nodelete-dlclose-plugin.so + +tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096 +tst-env-setuid-tunables-ENV = \ + GLIBC_TUNABLES=glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096 diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h index 5273dab48f..a986f0b593 100644 --- a/elf/dl-tunable-types.h +++ b/elf/dl-tunable-types.h @@ -43,4 +43,19 @@ typedef union const char *strval; } tunable_val_t; +/* Security level for tunables. This decides what to do with individual + tunables for AT_SECURE binaries. */ +typedef enum +{ + /* Erase the tunable for AT_SECURE binaries so that child processes don't + read it. */ + TUNABLE_SECLEVEL_SXID_ERASE = 0, + /* Ignore the tunable for AT_SECURE binaries, but don't erase it, so that + child processes can read it. */ + TUNABLE_SECLEVEL_SXID_IGNORE = 1, + /* Read the tunable. */ + TUNABLE_SECLEVEL_NONE = 2, +} tunable_seclevel_t; + + #endif diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index cbf4c8e8f2..a8d53d6a31 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -76,10 +76,12 @@ tunables_strdup (const char *in) #endif static char ** -get_next_env (char **envp, char **name, size_t *namelen, char **val) +get_next_env (char **envp, char **name, size_t *namelen, char **val, + char ***prev_envp) { while (envp != NULL && *envp != NULL) { + char **prev = envp; char *envline = *envp++; int len = 0; @@ -93,6 +95,7 @@ get_next_env (char **envp, char **name, size_t *namelen, char **val) *name = envline; *namelen = len; *val = &envline[len + 1]; + *prev_envp = prev; return envp; } @@ -243,8 +246,13 @@ tunable_initialize (tunable_t *cur, const char *strval) } #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring +/* Parse the tunable string TUNESTR and adjust it to drop any tunables that may + be unsafe for AT_SECURE processes so that it can be used as the new + environment variable value for GLIBC_TUNABLES. VALSTRING is the original + environment variable string which we use to make NULL terminated values so + that we don't have to allocate memory again for it. */ static void -parse_tunables (char *tunestr) +parse_tunables (char *tunestr, char *valstring) { if (tunestr == NULL || *tunestr == '\0') return; @@ -275,37 +283,65 @@ parse_tunables (char *tunestr) p += len + 1; - char *value = p; + /* Take the value from the valstring since we need to NULL terminate it. */ + char *value = &valstring[p - tunestr]; len = 0; while (p[len] != ':' && p[len] != '\0') len++; - char end = p[len]; - p[len] = '\0'; - /* Add the tunable if it exists. */ for (size_t i = 0; i < sizeof (tunable_list) / sizeof (tunable_t); i++) { tunable_t *cur = &tunable_list[i]; - /* If we are in a secure context (AT_SECURE) then ignore the tunable - unless it is explicitly marked as secure. Tunable values take - precendence over their envvar aliases. */ - if (__libc_enable_secure && !cur->is_secure) - continue; - if (is_name (cur->name, name)) { + /* If we are in a secure context (AT_SECURE) then ignore the tunable + unless it is explicitly marked as secure. Tunable values take + precendence over their envvar aliases. */ + if (__libc_enable_secure) + { + if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE) + { + if (p[len] == '\0') + { + /* Last tunable in the valstring. Null-terminate and + return. */ + *name = '\0'; + return; + } + else + { + /* Remove the current tunable from the string. We do + this by overwriting the string starting from NAME + (which is where the current tunable begins) with + the remainder of the string. We then have P point + to NAME so that we continue in the correct + position in the valstring. */ + char *q = &p[len + 1]; + p = name; + while (*q != '\0') + *name++ = *q++; + name[0] = '\0'; + len = 0; + } + } + + if (cur->security_level != TUNABLE_SECLEVEL_NONE) + break; + } + + value[len] = '\0'; tunable_initialize (cur, value); break; } } - if (end == ':') - p += len + 1; - else + if (p[len] == '\0') return; + else + p += len + 1; } } #endif @@ -320,8 +356,9 @@ static inline void __always_inline maybe_enable_malloc_check (void) { - if (__access_noerrno ("/etc/suid-debug", F_OK) == 0) - tunable_list[TUNABLE_ENUM_NAME(glibc, malloc, check)].is_secure = true; + tunable_id_t id = TUNABLE_ENUM_NAME (glibc, malloc, check); + if (__libc_enable_secure && __access_noerrno ("/etc/suid-debug", F_OK) == 0) + tunable_list[id].security_level = TUNABLE_SECLEVEL_NONE; } /* Initialize the tunables list from the environment. For now we only use the @@ -333,17 +370,21 @@ __tunables_init (char **envp) char *envname = NULL; char *envval = NULL; size_t len = 0; + char **prev_envp = envp; maybe_enable_malloc_check (); - while ((envp = get_next_env (envp, &envname, &len, &envval)) != NULL) + while ((envp = get_next_env (envp, &envname, &len, &envval, + &prev_envp)) != NULL) { #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring if (is_name (GLIBC_TUNABLES, envname)) { - char *val = tunables_strdup (envval); - if (val != NULL) - parse_tunables (val); + char *new_env = tunables_strdup (envname); + if (new_env != NULL) + parse_tunables (new_env + len + 1, envval); + /* Put in the updated envval. */ + *prev_envp = new_env; continue; } #endif @@ -354,8 +395,7 @@ __tunables_init (char **envp) /* Skip over tunables that have either been set already or should be skipped. */ - if (cur->strval != NULL || cur->env_alias == NULL - || (__libc_enable_secure && !cur->is_secure)) + if (cur->strval != NULL || cur->env_alias == NULL) continue; const char *name = cur->env_alias; @@ -363,6 +403,39 @@ __tunables_init (char **envp) /* We have a match. Initialize and move on to the next line. */ if (is_name (name, envname)) { + /* For AT_SECURE binaries, we need to check the security settings of + the tunable and decide whether we read the value and also whether + we erase the value so that child processes don't inherit them in + the environment. */ + if (__libc_enable_secure) + { + if (cur->security_level == TUNABLE_SECLEVEL_SXID_ERASE) + { + /* Erase the environment variable. */ + char **ep = prev_envp; + + while (*ep != NULL) + { + if (is_name (name, *ep)) + { + char **dp = ep; + + do + dp[0] = dp[1]; + while (*dp++); + } + else + ++ep; + } + /* Reset the iterator so that we read the environment again + from the point we erased. */ + envp = prev_envp; + } + + if (cur->security_level != TUNABLE_SECLEVEL_NONE) + continue; + } + tunable_initialize (cur, envval); break; } diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index e07825c443..f33adfb359 100644 --- a/elf/dl-tunables.h +++ b/elf/dl-tunables.h @@ -41,11 +41,16 @@ struct _tunable tunable_val_t val; /* The value. */ const char *strval; /* The string containing the value, points into envp. */ - bool is_secure; /* Whether the tunable must be read - even for setuid binaries. Note that - even if the tunable is read, it may - not get used by the target module if - the value is considered unsafe. */ + tunable_seclevel_t security_level; /* Specify the security level for the + tunable with respect to AT_SECURE + programs. See description of + tunable_seclevel_t to see a + description of the values. + + Note that even if the tunable is + read, it may not get used by the + target module if the value is + considered unsafe. */ /* Compatibility elements. */ const char *env_alias; /* The compatibility environment variable name. */ diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list index d8cd912559..cb9e8f173b 100644 --- a/elf/dl-tunables.list +++ b/elf/dl-tunables.list @@ -21,8 +21,13 @@ # minval: Optional minimum acceptable value # maxval: Optional maximum acceptable value # env_alias: An alias environment variable -# is_secure: Specify whether the environment variable should be read for -# setuid binaries. +# security_level: Specify security level of the tunable. Valid values are: +# +# SXID_ERASE: (default) Don't read for AT_SECURE binaries and +# removed so that child processes can't read it. +# SXID_IGNORE: Don't read for AT_SECURE binaries, but retained for +# non-AT_SECURE subprocesses. +# SXID_NONE: Read all the time. glibc { malloc { @@ -35,34 +40,41 @@ glibc { top_pad { type: SIZE_T env_alias: MALLOC_TOP_PAD_ + security_level: SXID_IGNORE } perturb { type: INT_32 minval: 0 maxval: 0xff env_alias: MALLOC_PERTURB_ + security_level: SXID_IGNORE } mmap_threshold { type: SIZE_T env_alias: MALLOC_MMAP_THRESHOLD_ + security_level: SXID_IGNORE } trim_threshold { type: SIZE_T env_alias: MALLOC_TRIM_THRESHOLD_ + security_level: SXID_IGNORE } mmap_max { type: INT_32 env_alias: MALLOC_MMAP_MAX_ + security_level: SXID_IGNORE } arena_max { type: SIZE_T env_alias: MALLOC_ARENA_MAX minval: 1 + security_level: SXID_IGNORE } arena_test { type: SIZE_T env_alias: MALLOC_ARENA_TEST minval: 1 + security_level: SXID_IGNORE } } } diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c new file mode 100644 index 0000000000..a563f69972 --- /dev/null +++ b/elf/tst-env-setuid-tunables.c @@ -0,0 +1,60 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* Verify that tunables correctly filter out unsafe tunables like + glibc.malloc.check and glibc.malloc.mmap_threshold but also retain + glibc.malloc.mmap_threshold in an unprivileged child. */ + +#define test_parent test_parent_tunables +#define test_child test_child_tunables + +static int test_child_tunables (void); +static int test_parent_tunables (void); + +#include "tst-env-setuid.c" + +#define CHILD_VALSTRING_VALUE "glibc.malloc.mmap_threshold=4096" +#define PARENT_VALSTRING_VALUE \ + "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096" + +static int +test_child_tunables (void) +{ + const char *val = getenv ("GLIBC_TUNABLES"); + + if (val != NULL && strcmp (val, CHILD_VALSTRING_VALUE) == 0) + return 0; + + if (val != NULL) + printf ("Unexpected GLIBC_TUNABLES VALUE %s\n", val); + + return 1; +} + +static int +test_parent_tunables (void) +{ + const char *val = getenv ("GLIBC_TUNABLES"); + + if (val != NULL && strcmp (val, PARENT_VALSTRING_VALUE) == 0) + return 0; + + if (val != NULL) + printf ("Unexpected GLIBC_TUNABLES VALUE %s\n", val); + + return 1; +} diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c new file mode 100644 index 0000000000..85d423d799 --- /dev/null +++ b/elf/tst-env-setuid.c @@ -0,0 +1,282 @@ +/* Copyright (C) 2012-2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +/* Verify that tunables correctly filter out unsafe environment variables like + MALLOC_CHECK_ and MALLOC_MMAP_THRESHOLD_ but also retain + MALLOC_MMAP_THRESHOLD_ in an unprivileged child. */ + +#include <errno.h> +#include <fcntl.h> +#include <stdlib.h> +#include <stdint.h> +#include <stdio.h> +#include <string.h> +#include <sys/stat.h> +#include <sys/wait.h> +#include <unistd.h> + +#include <support/support.h> +#include <support/test-driver.h> + +static char SETGID_CHILD[] = "setgid-child"; +#define CHILD_STATUS 42 + +/* Return a GID which is not our current GID, but is present in the + supplementary group list. */ +static gid_t +choose_gid (void) +{ + const int count = 64; + gid_t groups[count]; + int ret = getgroups (count, groups); + if (ret < 0) + { + printf ("getgroups: %m\n"); + exit (1); + } + gid_t current = getgid (); + for (int i = 0; i < ret; ++i) + { + if (groups[i] != current) + return groups[i]; + } + return 0; +} + +/* Spawn and execute a program and verify that it returns the CHILD_STATUS. */ +static pid_t +do_execve (char **args) +{ + pid_t kid = vfork (); + + if (kid < 0) + { + printf ("vfork: %m\n"); + return -1; + } + + if (kid == 0) + { + /* Child process. */ + execve (args[0], args, environ); + _exit (-errno); + } + + if (kid < 0) + return 1; + + int status; + + if (waitpid (kid, &status, 0) < 0) + { + printf ("waitpid: %m\n"); + return 1; + } + + if (!WIFEXITED (status) || WEXITSTATUS (status) != CHILD_STATUS) + { + printf ("Unexpected exit status %d from child process\n", + status); + return 1; + } + return 0; +} + +/* Copies the executable into a restricted directory, so that we can + safely make it SGID with the TARGET group ID. Then runs the + executable. */ +static int +run_executable_sgid (gid_t target) +{ + char *dirname = xasprintf ("%s/tst-tunables-setuid.%jd", + test_dir, (intmax_t) getpid ()); + char *execname = xasprintf ("%s/bin", dirname); + int infd = -1; + int outfd = -1; + int ret = 0; + if (mkdir (dirname, 0700) < 0) + { + printf ("mkdir: %m\n"); + goto err; + } + infd = open ("/proc/self/exe", O_RDONLY); + if (infd < 0) + { + printf ("open (/proc/self/exe): %m\n"); + goto err; + } + outfd = open (execname, O_WRONLY | O_CREAT | O_EXCL, 0700); + if (outfd < 0) + { + printf ("open (%s): %m\n", execname); + goto err; + } + char buf[4096]; + for (;;) + { + ssize_t rdcount = read (infd, buf, sizeof (buf)); + if (rdcount < 0) + { + printf ("read: %m\n"); + goto err; + } + if (rdcount == 0) + break; + char *p = buf; + char *end = buf + rdcount; + while (p != end) + { + ssize_t wrcount = write (outfd, buf, end - p); + if (wrcount == 0) + errno = ENOSPC; + if (wrcount <= 0) + { + printf ("write: %m\n"); + goto err; + } + p += wrcount; + } + } + if (fchown (outfd, getuid (), target) < 0) + { + printf ("fchown (%s): %m\n", execname); + goto err; + } + if (fchmod (outfd, 02750) < 0) + { + printf ("fchmod (%s): %m\n", execname); + goto err; + } + if (close (outfd) < 0) + { + printf ("close (outfd): %m\n"); + goto err; + } + if (close (infd) < 0) + { + printf ("close (infd): %m\n"); + goto err; + } + + char *args[] = {execname, SETGID_CHILD, NULL}; + + ret = do_execve (args); + +err: + if (outfd >= 0) + close (outfd); + if (infd >= 0) + close (infd); + if (execname) + { + unlink (execname); + free (execname); + } + if (dirname) + { + rmdir (dirname); + free (dirname); + } + return ret; +} + +#ifndef test_child +static int +test_child (void) +{ + if (getenv ("MALLOC_CHECK_") != NULL) + { + printf ("MALLOC_CHECK_ is still set\n"); + return 1; + } + + if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL) + { + printf ("MALLOC_MMAP_THRESHOLD_ lost\n"); + return 1; + } + + return 0; +} +#endif + +#ifndef test_parent +static int +test_parent (void) +{ + if (getenv ("MALLOC_CHECK_") == NULL) + { + printf ("MALLOC_CHECK_ lost\n"); + return 1; + } + + if (getenv ("MALLOC_MMAP_THRESHOLD_") == NULL) + { + printf ("MALLOC_MMAP_THRESHOLD_ lost\n"); + return 1; + } + + return 0; +} +#endif + +static int +do_test_prep (int argc, char **argv) +{ + /* Setgid child process. */ + if (argc == 2 && strcmp (argv[1], SETGID_CHILD) == 0) + { + if (getgid () == getegid ()) + { + /* This can happen if the file system is mounted nosuid. */ + fprintf (stderr, "SGID failed: GID and EGID match (%jd)\n", + (intmax_t) getgid ()); + exit (EXIT_UNSUPPORTED); + } + + int ret = test_child (); + + if (ret != 0) + exit (1); + + exit (CHILD_STATUS); + } + else + { + if (test_parent () != 0) + exit (1); + + /* Try running a setgid program. */ + gid_t target = choose_gid (); + if (target == 0) + { + fprintf (stderr, + "Could not find a suitable GID for user %jd, skipping test\n", + (intmax_t) getuid ()); + exit (0); + } + + if (run_executable_sgid (target) == 0) + exit (0); + } + + /* Something went wrong and our argv was corrupted. */ + _exit (1); +} + +#define TEST_FUNCTION_ARGV do_test_prep +#include <support/test-driver.c> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk index b65b5a4a33..e7bfc22b05 100644 --- a/scripts/gen-tunables.awk +++ b/scripts/gen-tunables.awk @@ -52,7 +52,7 @@ $1 == "}" { env_alias[top_ns][ns][tunable] = "NULL" } if (!is_secure[top_ns][ns][tunable]) { - is_secure[top_ns][ns][tunable] = "false" + is_secure[top_ns][ns][tunable] = "SXID_ERASE" } tunable = "" @@ -102,8 +102,8 @@ $1 == "}" { else if (attr == "env_alias") { env_alias[top_ns][ns][tunable] = sprintf("\"%s\"", val) } - else if (attr == "is_secure") { - if (val == "true" || val == "false") { + else if (attr == "security_level") { + if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") { is_secure[top_ns][ns][tunable] = val } else { @@ -146,7 +146,7 @@ END { for (n in types[t]) { for (m in types[t][n]) { printf (" {TUNABLE_NAME_S(%s, %s, %s)", t, n, m) - printf (", {TUNABLE_TYPE_%s, %s, %s}, {.numval = 0}, NULL, %s, %s},\n", + printf (", {TUNABLE_TYPE_%s, %s, %s}, {.numval = 0}, NULL, TUNABLE_SECLEVEL_%s, %s},\n", types[t][n][m], minvals[t][n][m], maxvals[t][n][m], is_secure[t][n][m], env_alias[t][n][m]); } |