tunables: Fix environment variable processing for setuid binaries (bz #21073)

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.
This commit is contained in:
Siddhesh Poyarekar 2017-02-02 15:46:01 +05:30
parent 9c8e644853
commit 8b9e9c3c0b
9 changed files with 511 additions and 35 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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;
}

View File

@ -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. */

View File

@ -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
}
}
}

View File

@ -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;
}

282
elf/tst-env-setuid.c Normal file
View File

@ -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>

View File

@ -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]);
}