From bbbeb3892f6a9c196674a521ff543aab94b509ed Mon Sep 17 00:00:00 2001
From: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date: Wed, 1 Feb 2017 15:48:32 +0530
Subject: [PATCH 1/2] tunables: Fix environment variable processing for setuid
binaries
Florian Weimer pointed out that we have three different kinds of
environment variables (and hence tunables):
1. Variables that are removed for setxid processes
2. Variables that are ignored in setxid processes but is passed on to
child processes
3. Variables that are passed on to child processes all the time
Tunables currently only does (2) and (3) when it should be doing (1)
for MALLOC_CHECK_. This patch enhances the is_secure flag in tunables
to an enum value that can specify which of the above three categories
the tunable (and its envvar alias) belongs to.
The default is for tunables to be in (1). Hence, all of the malloc
tunables barring MALLOC_CHECK_ are explicitly specified to belong to
category (2). There were discussions around abolishing category (2)
completely but we can do that as a separate exercise in 2.26.
Tested on x86_64 to verify that there are no regressions.
[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.
---
elf/Makefile | 6 +-
elf/dl-tunable-types.h | 15 +++
elf/dl-tunables.c | 119 ++++++++++++++----
elf/dl-tunables.h | 15 ++-
elf/dl-tunables.list | 16 ++-
elf/tst-env-setuid-tunables.c | 60 +++++++++
elf/tst-env-setuid.c | 282 ++++++++++++++++++++++++++++++++++++++++++
scripts/gen-tunables.awk | 8 +-
8 files changed, 486 insertions(+), 35 deletions(-)
create mode 100644 elf/tst-env-setuid-tunables.c
create mode 100644 elf/tst-env-setuid.c
@@ -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
@@ -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
@@ -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 == ':')
+ if (p[len] == '\0')
+ return;
+ else
p += len + 1;
- else
- return;
}
}
#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;
}
@@ -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. */
@@ -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
}
}
}
new file mode 100644
@@ -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;
+}
new file mode 100644
@@ -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>
@@ -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]);
}
--
2.7.4