* [PATCH v2] rtld: Add glibc.rtld.enable_secure tunable. @ 2023-12-05 17:45 Joe Simmons-Talbott 2023-12-12 19:48 ` Siddhesh Poyarekar 0 siblings, 1 reply; 3+ messages in thread From: Joe Simmons-Talbott @ 2023-12-05 17:45 UTC (permalink / raw) To: libc-alpha; +Cc: Joe Simmons-Talbott Add a tunable for setting __libc_enable_secure to 1. Does not set __libc_enable_secure to 0 if the tunable is set to 0. Ignores any tunables following glib.rtld.enable_secure. One use-case for this addition is to enable testing code paths that depend on __libc_eanble_secure being set without the need to use setxid binaries. --- Changes to v1: * handle the tunable for the dynamic loader as well. NOTE: I'm not certain I've picked the appropriate place to handle glibc.rtld.enable_secure. I tried to make it happen as early as possible to minimize and places where __libc_enable_secure might be checked before the tunable initialization takes place. NEWS | 4 + csu/libc-start.c | 4 + elf/Makefile | 2 + elf/dl-tunables.c | 8 +- elf/dl-tunables.h | 11 +++ elf/dl-tunables.list | 6 ++ elf/tst-rtld-list-tunables.exp | 1 + elf/tst-tunables-enable_secure.c | 115 ++++++++++++++++++++++++++++ sysdeps/unix/sysv/linux/dl-sysdep.c | 3 + 9 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 elf/tst-tunables-enable_secure.c diff --git a/NEWS b/NEWS index 8c1c149f91..b04fb15064 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,10 @@ Major new features: on thread stack created by pthread_create or memory allocated by malloc). +* A new tunable, glibc.rtld.enable_secure, used to set __libc_enable_secure + to 1. Setting the tunable to 0 does *NOT* set __libc_enable_secure to 0. + Once the tunable is encountered all following tunables are ignored. + Deprecated and removed features, and other changes affecting compatibility: * The ldconfig program now skips file names containing ';' or ending in diff --git a/csu/libc-start.c b/csu/libc-start.c index c3bb6d09bc..2f546a3677 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -266,6 +266,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), __tunables_init (__environ); + int32_t tes = TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL); + if (tes == 1) + __libc_enable_secure = 1; + ARCH_INIT_CPU_FEATURES (); /* Do static pie self relocation after tunables and cpu features diff --git a/elf/Makefile b/elf/Makefile index afec7be084..6c38a72b0d 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -285,6 +285,7 @@ tests-static-internal := \ tst-tls1-static \ tst-tls1-static-non-pie \ tst-tunables \ + tst-tunables-enable_secure \ # tests-static-internal CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o @@ -2672,6 +2673,7 @@ $(objpfx)tst-glibc-hwcaps-mask.out: \ $(objpfx)tst-glibc-hwcaps-cache.out: $(objpfx)tst-glibc-hwcaps tst-tunables-ARGS = -- $(host-test-program-cmd) +tst-tunables-enable_secure-ARGS = -- $(host-test-program-cmd) $(objpfx)list-tunables.out: tst-rtld-list-tunables.sh $(objpfx)ld.so $(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' \ diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 644d21d1b0..d6c55cfaf2 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -250,7 +250,13 @@ parse_tunables (char *valstring) } for (int i = 0; i < ntunables; i++) - tunable_initialize (tunables[i].t, tunables[i].value); + { + tunable_initialize (tunables[i].t, tunables[i].value); + + /* Ignore all tunables after encountering "glibc.rtld.enable_secure" */ + if (tunable_strcmp(tunables[i].t->name, "glibc.rtld.enable_secure") == 0) + break; + } } /* Initialize the tunables list from the environment. For now we only use the diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index 0df4dde24e..2d4fc84d74 100644 --- a/elf/dl-tunables.h +++ b/elf/dl-tunables.h @@ -164,4 +164,15 @@ tunable_is_name (const char *orig, const char *envname) return false; } +/* Compare two strings. */ +static __always_inline int +tunable_strcmp (const char *left, const char *right) +{ + for (;*left != '\0' && *right != '\0'; right++, left++) + if (*left != *right) + break; + + return *left - *right; +} + #endif diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list index 1b23fc9473..cb1b35854e 100644 --- a/elf/dl-tunables.list +++ b/elf/dl-tunables.list @@ -136,6 +136,12 @@ glibc { minval: 0 default: 512 } + enable_secure { + type: INT_32 + minval: 0 + maxval: 1 + default: 0 + } } mem { diff --git a/elf/tst-rtld-list-tunables.exp b/elf/tst-rtld-list-tunables.exp index 2233ea9c7c..db0e1c86e9 100644 --- a/elf/tst-rtld-list-tunables.exp +++ b/elf/tst-rtld-list-tunables.exp @@ -12,5 +12,6 @@ glibc.malloc.tcache_unsorted_limit: 0x0 (min: 0x0, max: 0x[f]+) glibc.malloc.top_pad: 0x20000 (min: 0x0, max: 0x[f]+) glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0x[f]+) glibc.rtld.dynamic_sort: 2 (min: 1, max: 2) +glibc.rtld.enable_secure: 0 (min: 0, max: 1) glibc.rtld.nns: 0x4 (min: 0x1, max: 0x10) glibc.rtld.optional_static_tls: 0x200 (min: 0x0, max: 0x[f]+) diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c new file mode 100644 index 0000000000..e749ec9f17 --- /dev/null +++ b/elf/tst-tunables-enable_secure.c @@ -0,0 +1,115 @@ +/* Check GLIBC_TUNABLES parsing for enable_secure. + Copyright (C) 2023 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 + <https://www.gnu.org/licenses/>. */ + +#include <array_length.h> +#include <dl-tunables.h> +#include <getopt.h> +#include <intprops.h> +#include <stdint.h> +#include <stdlib.h> +#include <support/capture_subprocess.h> +#include <support/check.h> +#include <sys/auxv.h> +#include <unistd.h> + +static int restart; +#define CMDLINE_OPTIONS \ + { "restart", no_argument, &restart, 1 }, + +static const struct test_t +{ + const char *env; + int32_t expected_malloc_check; + int32_t expected_enable_secure; +} tests[] = +{ + /* Expected tunable format. */ + { + "glibc.malloc.check=2:glibc.rtld.enable_secure=1", + 2, + 1, + }, + /* Tunables encountered after enable_secure should be ignored. */ + { + "glibc.rtld.enable_secure=1:glibc.malloc.check=2", + 0, + 1, + }, +}; + +static int +handle_restart (int i) +{ + TEST_COMPARE (tests[i].expected_malloc_check, + TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL)); + TEST_COMPARE (tests[i].expected_enable_secure, + TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL)); + if (tests[i].expected_enable_secure == 1) + { + TEST_COMPARE (1, __libc_enable_secure); + } + return 0; +} + +static int +do_test (int argc, char *argv[]) +{ + /* We must have either: + - One or four parameters left if called initially: + + path to ld.so optional + + "--library-path" optional + + the library path optional + + the application name + + the test to check */ + + TEST_VERIFY_EXIT (argc == 2 || argc == 5); + + if (restart) + return handle_restart (atoi (argv[1])); + + char nteststr[INT_BUFSIZE_BOUND (int)]; + + char *spargv[10]; + { + int i = 0; + for (; i < argc - 1; i++) + spargv[i] = argv[i + 1]; + spargv[i++] = (char *) "--direct"; + spargv[i++] = (char *) "--restart"; + spargv[i++] = nteststr; + spargv[i] = NULL; + } + + for (int i = 0; i < array_length (tests); i++) + { + snprintf (nteststr, sizeof nteststr, "%d", i); + + printf ("[%d] Spawned test for %s\n", i, tests[i].env); + setenv ("GLIBC_TUNABLES", tests[i].env, 1); + struct support_capture_subprocess result + = support_capture_subprogram (spargv[0], spargv); + support_capture_subprocess_check (&result, "tst-tunables-enable_secure", + 0, sc_allow_stderr); + support_capture_subprocess_free (&result); + } + + return 0; +} + +#define TEST_FUNCTION_ARGV do_test +#include <support/test-driver.c> diff --git a/sysdeps/unix/sysv/linux/dl-sysdep.c b/sysdeps/unix/sysv/linux/dl-sysdep.c index 1b3dd869b5..d3a30e8d7a 100644 --- a/sysdeps/unix/sysv/linux/dl-sysdep.c +++ b/sysdeps/unix/sysv/linux/dl-sysdep.c @@ -108,6 +108,9 @@ _dl_sysdep_start (void **start_argptr, dl_hwcap_check (); __tunables_init (_environ); + int32_t tes = TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL); + if (tes == 1) + __libc_enable_secure = 1; /* Initialize DSO sorting algorithm after tunables. */ _dl_sort_maps_init (); -- 2.41.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] rtld: Add glibc.rtld.enable_secure tunable. 2023-12-05 17:45 [PATCH v2] rtld: Add glibc.rtld.enable_secure tunable Joe Simmons-Talbott @ 2023-12-12 19:48 ` Siddhesh Poyarekar 2024-01-02 21:16 ` Joe Simmons-Talbott 0 siblings, 1 reply; 3+ messages in thread From: Siddhesh Poyarekar @ 2023-12-12 19:48 UTC (permalink / raw) To: Joe Simmons-Talbott, libc-alpha On 2023-12-05 12:45, Joe Simmons-Talbott wrote: > Add a tunable for setting __libc_enable_secure to 1. Does not set > __libc_enable_secure to 0 if the tunable is set to 0. Ignores any > tunables following glib.rtld.enable_secure. One use-case for this > addition is to enable testing code paths that depend on > __libc_eanble_secure being set without the need to use setxid binaries. > --- > Changes to v1: > * handle the tunable for the dynamic loader as well. > > NOTE: I'm not certain I've picked the appropriate place to handle > glibc.rtld.enable_secure. I tried to make it happen as early as > possible to minimize and places where __libc_enable_secure might be > checked before the tunable initialization takes place. > > NEWS | 4 + > csu/libc-start.c | 4 + > elf/Makefile | 2 + > elf/dl-tunables.c | 8 +- > elf/dl-tunables.h | 11 +++ > elf/dl-tunables.list | 6 ++ > elf/tst-rtld-list-tunables.exp | 1 + > elf/tst-tunables-enable_secure.c | 115 ++++++++++++++++++++++++++++ > sysdeps/unix/sysv/linux/dl-sysdep.c | 3 + > 9 files changed, 153 insertions(+), 1 deletion(-) > create mode 100644 elf/tst-tunables-enable_secure.c > > diff --git a/NEWS b/NEWS > index 8c1c149f91..b04fb15064 100644 > --- a/NEWS > +++ b/NEWS > @@ -43,6 +43,10 @@ Major new features: > on thread stack created by pthread_create or memory allocated by > malloc). > > +* A new tunable, glibc.rtld.enable_secure, used to set __libc_enable_secure > + to 1. Setting the tunable to 0 does *NOT* set __libc_enable_secure to 0. > + Once the tunable is encountered all following tunables are ignored. > + __libc_enable_secure is an internal detail. How about rephrasing it in terms of the feature this implements, i.e. "A new tunable, ... used to run a program as if it were a setuid process, enabling a number of security features. This is currently a testing tool to allow more extensive verification tests for AT_SECURE programs and not meant to be a security feature". > Deprecated and removed features, and other changes affecting compatibility: > > * The ldconfig program now skips file names containing ';' or ending in > diff --git a/csu/libc-start.c b/csu/libc-start.c > index c3bb6d09bc..2f546a3677 100644 > --- a/csu/libc-start.c > +++ b/csu/libc-start.c > @@ -266,6 +266,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > __tunables_init (__environ); > > + int32_t tes = TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL); > + if (tes == 1) > + __libc_enable_secure = 1; > + OK, but I wonder if it's easier to just do this in __tunables_init so that it's done in a single place. > ARCH_INIT_CPU_FEATURES (); > > /* Do static pie self relocation after tunables and cpu features > diff --git a/elf/Makefile b/elf/Makefile > index afec7be084..6c38a72b0d 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -285,6 +285,7 @@ tests-static-internal := \ > tst-tls1-static \ > tst-tls1-static-non-pie \ > tst-tunables \ > + tst-tunables-enable_secure \ > # tests-static-internal > > CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o > @@ -2672,6 +2673,7 @@ $(objpfx)tst-glibc-hwcaps-mask.out: \ > $(objpfx)tst-glibc-hwcaps-cache.out: $(objpfx)tst-glibc-hwcaps > > tst-tunables-ARGS = -- $(host-test-program-cmd) > +tst-tunables-enable_secure-ARGS = -- $(host-test-program-cmd) > > $(objpfx)list-tunables.out: tst-rtld-list-tunables.sh $(objpfx)ld.so > $(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' \ > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 644d21d1b0..d6c55cfaf2 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -250,7 +250,13 @@ parse_tunables (char *valstring) > } > > for (int i = 0; i < ntunables; i++) > - tunable_initialize (tunables[i].t, tunables[i].value); > + { > + tunable_initialize (tunables[i].t, tunables[i].value); > + > + /* Ignore all tunables after encountering "glibc.rtld.enable_secure" */ > + if (tunable_strcmp(tunables[i].t->name, "glibc.rtld.enable_secure") == 0) > + break; This still has the problem that Szabolcs pointed out, i.e. one only needs to reorder tunables to be before enable_secure to avoid being ignored. What you need to do is when you encounter enable_secure in parse_tunable_string (look for the tunable_is_name call in there), you return 0. That will ensure that no tunables are set at all and you've successfully emulated setxid for tunables. If you set __libc_enable_secure there, all code that follows will run as if the program were AT_SECURE. > + } > } > > /* Initialize the tunables list from the environment. For now we only use the > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > index 0df4dde24e..2d4fc84d74 100644 > --- a/elf/dl-tunables.h > +++ b/elf/dl-tunables.h > @@ -164,4 +164,15 @@ tunable_is_name (const char *orig, const char *envname) > return false; > } > > +/* Compare two strings. */ > +static __always_inline int > +tunable_strcmp (const char *left, const char *right) > +{ > + for (;*left != '\0' && *right != '\0'; right++, left++) > + if (*left != *right) > + break; > + > + return *left - *right; > +} > + > #endif > diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list > index 1b23fc9473..cb1b35854e 100644 > --- a/elf/dl-tunables.list > +++ b/elf/dl-tunables.list > @@ -136,6 +136,12 @@ glibc { > minval: 0 > default: 512 > } > + enable_secure { > + type: INT_32 > + minval: 0 > + maxval: 1 > + default: 0 > + } > } > > mem { > diff --git a/elf/tst-rtld-list-tunables.exp b/elf/tst-rtld-list-tunables.exp > index 2233ea9c7c..db0e1c86e9 100644 > --- a/elf/tst-rtld-list-tunables.exp > +++ b/elf/tst-rtld-list-tunables.exp > @@ -12,5 +12,6 @@ glibc.malloc.tcache_unsorted_limit: 0x0 (min: 0x0, max: 0x[f]+) > glibc.malloc.top_pad: 0x20000 (min: 0x0, max: 0x[f]+) > glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0x[f]+) > glibc.rtld.dynamic_sort: 2 (min: 1, max: 2) > +glibc.rtld.enable_secure: 0 (min: 0, max: 1) > glibc.rtld.nns: 0x4 (min: 0x1, max: 0x10) > glibc.rtld.optional_static_tls: 0x200 (min: 0x0, max: 0x[f]+) > diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c > new file mode 100644 > index 0000000000..e749ec9f17 > --- /dev/null > +++ b/elf/tst-tunables-enable_secure.c > @@ -0,0 +1,115 @@ > +/* Check GLIBC_TUNABLES parsing for enable_secure. > + Copyright (C) 2023 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 > + <https://www.gnu.org/licenses/>. */ > + > +#include <array_length.h> > +#include <dl-tunables.h> > +#include <getopt.h> > +#include <intprops.h> > +#include <stdint.h> > +#include <stdlib.h> > +#include <support/capture_subprocess.h> > +#include <support/check.h> > +#include <sys/auxv.h> > +#include <unistd.h> > + > +static int restart; > +#define CMDLINE_OPTIONS \ > + { "restart", no_argument, &restart, 1 }, > + > +static const struct test_t > +{ > + const char *env; > + int32_t expected_malloc_check; > + int32_t expected_enable_secure; > +} tests[] = > +{ > + /* Expected tunable format. */ > + { > + "glibc.malloc.check=2:glibc.rtld.enable_secure=1", > + 2, > + 1, > + }, > + /* Tunables encountered after enable_secure should be ignored. */ > + { > + "glibc.rtld.enable_secure=1:glibc.malloc.check=2", > + 0, > + 1, > + }, > +}; > + > +static int > +handle_restart (int i) > +{ > + TEST_COMPARE (tests[i].expected_malloc_check, > + TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL)); > + TEST_COMPARE (tests[i].expected_enable_secure, > + TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL)); > + if (tests[i].expected_enable_secure == 1) > + { > + TEST_COMPARE (1, __libc_enable_secure); > + } > + return 0; > +} > + > +static int > +do_test (int argc, char *argv[]) > +{ > + /* We must have either: > + - One or four parameters left if called initially: > + + path to ld.so optional > + + "--library-path" optional > + + the library path optional > + + the application name > + + the test to check */ > + > + TEST_VERIFY_EXIT (argc == 2 || argc == 5); > + > + if (restart) > + return handle_restart (atoi (argv[1])); > + > + char nteststr[INT_BUFSIZE_BOUND (int)]; > + > + char *spargv[10]; > + { > + int i = 0; > + for (; i < argc - 1; i++) > + spargv[i] = argv[i + 1]; > + spargv[i++] = (char *) "--direct"; > + spargv[i++] = (char *) "--restart"; > + spargv[i++] = nteststr; > + spargv[i] = NULL; > + } > + > + for (int i = 0; i < array_length (tests); i++) > + { > + snprintf (nteststr, sizeof nteststr, "%d", i); > + > + printf ("[%d] Spawned test for %s\n", i, tests[i].env); > + setenv ("GLIBC_TUNABLES", tests[i].env, 1); > + struct support_capture_subprocess result > + = support_capture_subprogram (spargv[0], spargv); > + support_capture_subprocess_check (&result, "tst-tunables-enable_secure", > + 0, sc_allow_stderr); > + support_capture_subprocess_free (&result); > + } > + > + return 0; > +} > + > +#define TEST_FUNCTION_ARGV do_test > +#include <support/test-driver.c> > diff --git a/sysdeps/unix/sysv/linux/dl-sysdep.c b/sysdeps/unix/sysv/linux/dl-sysdep.c > index 1b3dd869b5..d3a30e8d7a 100644 > --- a/sysdeps/unix/sysv/linux/dl-sysdep.c > +++ b/sysdeps/unix/sysv/linux/dl-sysdep.c > @@ -108,6 +108,9 @@ _dl_sysdep_start (void **start_argptr, > dl_hwcap_check (); > > __tunables_init (_environ); > + int32_t tes = TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL); > + if (tes == 1) > + __libc_enable_secure = 1; OK, but maybe better to just do it in __tunables_init. Any specific reason why you're doing it here? For example, you've missed the hurd dl-sysdep change. > > /* Initialize DSO sorting algorithm after tunables. */ > _dl_sort_maps_init (); Thanks, Sid ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] rtld: Add glibc.rtld.enable_secure tunable. 2023-12-12 19:48 ` Siddhesh Poyarekar @ 2024-01-02 21:16 ` Joe Simmons-Talbott 0 siblings, 0 replies; 3+ messages in thread From: Joe Simmons-Talbott @ 2024-01-02 21:16 UTC (permalink / raw) To: Siddhesh Poyarekar; +Cc: libc-alpha On Tue, Dec 12, 2023 at 02:48:53PM -0500, Siddhesh Poyarekar wrote: > On 2023-12-05 12:45, Joe Simmons-Talbott wrote: > > Add a tunable for setting __libc_enable_secure to 1. Does not set > > __libc_enable_secure to 0 if the tunable is set to 0. Ignores any > > tunables following glib.rtld.enable_secure. One use-case for this > > addition is to enable testing code paths that depend on > > __libc_eanble_secure being set without the need to use setxid binaries. > > --- > > Changes to v1: > > * handle the tunable for the dynamic loader as well. > > NOTE: I'm not certain I've picked the appropriate place to handle > > glibc.rtld.enable_secure. I tried to make it happen as early as > > possible to minimize and places where __libc_enable_secure might be > > checked before the tunable initialization takes place. > > > > NEWS | 4 + > > csu/libc-start.c | 4 + > > elf/Makefile | 2 + > > elf/dl-tunables.c | 8 +- > > elf/dl-tunables.h | 11 +++ > > elf/dl-tunables.list | 6 ++ > > elf/tst-rtld-list-tunables.exp | 1 + > > elf/tst-tunables-enable_secure.c | 115 ++++++++++++++++++++++++++++ > > sysdeps/unix/sysv/linux/dl-sysdep.c | 3 + > > 9 files changed, 153 insertions(+), 1 deletion(-) > > create mode 100644 elf/tst-tunables-enable_secure.c > > > > diff --git a/NEWS b/NEWS > > index 8c1c149f91..b04fb15064 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -43,6 +43,10 @@ Major new features: > > on thread stack created by pthread_create or memory allocated by > > malloc). > > +* A new tunable, glibc.rtld.enable_secure, used to set __libc_enable_secure > > + to 1. Setting the tunable to 0 does *NOT* set __libc_enable_secure to 0. > > + Once the tunable is encountered all following tunables are ignored. > > + > > __libc_enable_secure is an internal detail. How about rephrasing it in > terms of the feature this implements, i.e. "A new tunable, ... used to run a > program as if it were a setuid process, enabling a number of security > features. This is currently a testing tool to allow more extensive > verification tests for AT_SECURE programs and not meant to be a security > feature". Updated. > > > Deprecated and removed features, and other changes affecting compatibility: > > * The ldconfig program now skips file names containing ';' or ending in > > diff --git a/csu/libc-start.c b/csu/libc-start.c > > index c3bb6d09bc..2f546a3677 100644 > > --- a/csu/libc-start.c > > +++ b/csu/libc-start.c > > @@ -266,6 +266,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), > > __tunables_init (__environ); > > + int32_t tes = TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL); > > + if (tes == 1) > > + __libc_enable_secure = 1; > > + > > OK, but I wonder if it's easier to just do this in __tunables_init so that > it's done in a single place. Moved this __tunables_init as you've suggested. > > > ARCH_INIT_CPU_FEATURES (); > > /* Do static pie self relocation after tunables and cpu features > > diff --git a/elf/Makefile b/elf/Makefile > > index afec7be084..6c38a72b0d 100644 > > --- a/elf/Makefile > > +++ b/elf/Makefile > > @@ -285,6 +285,7 @@ tests-static-internal := \ > > tst-tls1-static \ > > tst-tls1-static-non-pie \ > > tst-tunables \ > > + tst-tunables-enable_secure \ > > # tests-static-internal > > CRT-tst-tls1-static-non-pie := $(csu-objpfx)crt1.o > > @@ -2672,6 +2673,7 @@ $(objpfx)tst-glibc-hwcaps-mask.out: \ > > $(objpfx)tst-glibc-hwcaps-cache.out: $(objpfx)tst-glibc-hwcaps > > tst-tunables-ARGS = -- $(host-test-program-cmd) > > +tst-tunables-enable_secure-ARGS = -- $(host-test-program-cmd) > > $(objpfx)list-tunables.out: tst-rtld-list-tunables.sh $(objpfx)ld.so > > $(SHELL) $< $(objpfx)ld.so '$(test-wrapper-env)' \ > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > > index 644d21d1b0..d6c55cfaf2 100644 > > --- a/elf/dl-tunables.c > > +++ b/elf/dl-tunables.c > > @@ -250,7 +250,13 @@ parse_tunables (char *valstring) > > } > > for (int i = 0; i < ntunables; i++) > > - tunable_initialize (tunables[i].t, tunables[i].value); > > + { > > + tunable_initialize (tunables[i].t, tunables[i].value); > > + > > + /* Ignore all tunables after encountering "glibc.rtld.enable_secure" */ > > + if (tunable_strcmp(tunables[i].t->name, "glibc.rtld.enable_secure") == 0) > > + break; > > This still has the problem that Szabolcs pointed out, i.e. one only needs to > reorder tunables to be before enable_secure to avoid being ignored. What > you need to do is when you encounter enable_secure in parse_tunable_string > (look for the tunable_is_name call in there), you return 0. That will > ensure that no tunables are set at all and you've successfully emulated > setxid for tunables. If you set __libc_enable_secure there, all code that > follows will run as if the program were AT_SECURE. > Updated as you suggested and setting enable_secure doesn't set any tunables. > > + } > > } > > /* Initialize the tunables list from the environment. For now we only use the > > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > > index 0df4dde24e..2d4fc84d74 100644 > > --- a/elf/dl-tunables.h > > +++ b/elf/dl-tunables.h > > @@ -164,4 +164,15 @@ tunable_is_name (const char *orig, const char *envname) > > return false; > > } > > +/* Compare two strings. */ > > +static __always_inline int > > +tunable_strcmp (const char *left, const char *right) > > +{ > > + for (;*left != '\0' && *right != '\0'; right++, left++) > > + if (*left != *right) > > + break; > > + > > + return *left - *right; > > +} > > + > > #endif > > diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list > > index 1b23fc9473..cb1b35854e 100644 > > --- a/elf/dl-tunables.list > > +++ b/elf/dl-tunables.list > > @@ -136,6 +136,12 @@ glibc { > > minval: 0 > > default: 512 > > } > > + enable_secure { > > + type: INT_32 > > + minval: 0 > > + maxval: 1 > > + default: 0 > > + } > > } > > mem { > > diff --git a/elf/tst-rtld-list-tunables.exp b/elf/tst-rtld-list-tunables.exp > > index 2233ea9c7c..db0e1c86e9 100644 > > --- a/elf/tst-rtld-list-tunables.exp > > +++ b/elf/tst-rtld-list-tunables.exp > > @@ -12,5 +12,6 @@ glibc.malloc.tcache_unsorted_limit: 0x0 (min: 0x0, max: 0x[f]+) > > glibc.malloc.top_pad: 0x20000 (min: 0x0, max: 0x[f]+) > > glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0x[f]+) > > glibc.rtld.dynamic_sort: 2 (min: 1, max: 2) > > +glibc.rtld.enable_secure: 0 (min: 0, max: 1) > > glibc.rtld.nns: 0x4 (min: 0x1, max: 0x10) > > glibc.rtld.optional_static_tls: 0x200 (min: 0x0, max: 0x[f]+) > > diff --git a/elf/tst-tunables-enable_secure.c b/elf/tst-tunables-enable_secure.c > > new file mode 100644 > > index 0000000000..e749ec9f17 > > --- /dev/null > > +++ b/elf/tst-tunables-enable_secure.c > > @@ -0,0 +1,115 @@ > > +/* Check GLIBC_TUNABLES parsing for enable_secure. > > + Copyright (C) 2023 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 > > + <https://www.gnu.org/licenses/>. */ > > + > > +#include <array_length.h> > > +#include <dl-tunables.h> > > +#include <getopt.h> > > +#include <intprops.h> > > +#include <stdint.h> > > +#include <stdlib.h> > > +#include <support/capture_subprocess.h> > > +#include <support/check.h> > > +#include <sys/auxv.h> > > +#include <unistd.h> > > + > > +static int restart; > > +#define CMDLINE_OPTIONS \ > > + { "restart", no_argument, &restart, 1 }, > > + > > +static const struct test_t > > +{ > > + const char *env; > > + int32_t expected_malloc_check; > > + int32_t expected_enable_secure; > > +} tests[] = > > +{ > > + /* Expected tunable format. */ > > + { > > + "glibc.malloc.check=2:glibc.rtld.enable_secure=1", > > + 2, > > + 1, > > + }, > > + /* Tunables encountered after enable_secure should be ignored. */ > > + { > > + "glibc.rtld.enable_secure=1:glibc.malloc.check=2", > > + 0, > > + 1, > > + }, > > +}; > > + > > +static int > > +handle_restart (int i) > > +{ > > + TEST_COMPARE (tests[i].expected_malloc_check, > > + TUNABLE_GET_FULL (glibc, malloc, check, int32_t, NULL)); > > + TEST_COMPARE (tests[i].expected_enable_secure, > > + TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL)); > > + if (tests[i].expected_enable_secure == 1) > > + { > > + TEST_COMPARE (1, __libc_enable_secure); > > + } > > + return 0; > > +} > > + > > +static int > > +do_test (int argc, char *argv[]) > > +{ > > + /* We must have either: > > + - One or four parameters left if called initially: > > + + path to ld.so optional > > + + "--library-path" optional > > + + the library path optional > > + + the application name > > + + the test to check */ > > + > > + TEST_VERIFY_EXIT (argc == 2 || argc == 5); > > + > > + if (restart) > > + return handle_restart (atoi (argv[1])); > > + > > + char nteststr[INT_BUFSIZE_BOUND (int)]; > > + > > + char *spargv[10]; > > + { > > + int i = 0; > > + for (; i < argc - 1; i++) > > + spargv[i] = argv[i + 1]; > > + spargv[i++] = (char *) "--direct"; > > + spargv[i++] = (char *) "--restart"; > > + spargv[i++] = nteststr; > > + spargv[i] = NULL; > > + } > > + > > + for (int i = 0; i < array_length (tests); i++) > > + { > > + snprintf (nteststr, sizeof nteststr, "%d", i); > > + > > + printf ("[%d] Spawned test for %s\n", i, tests[i].env); > > + setenv ("GLIBC_TUNABLES", tests[i].env, 1); > > + struct support_capture_subprocess result > > + = support_capture_subprogram (spargv[0], spargv); > > + support_capture_subprocess_check (&result, "tst-tunables-enable_secure", > > + 0, sc_allow_stderr); > > + support_capture_subprocess_free (&result); > > + } > > + > > + return 0; > > +} > > + > > +#define TEST_FUNCTION_ARGV do_test > > +#include <support/test-driver.c> > > diff --git a/sysdeps/unix/sysv/linux/dl-sysdep.c b/sysdeps/unix/sysv/linux/dl-sysdep.c > > index 1b3dd869b5..d3a30e8d7a 100644 > > --- a/sysdeps/unix/sysv/linux/dl-sysdep.c > > +++ b/sysdeps/unix/sysv/linux/dl-sysdep.c > > @@ -108,6 +108,9 @@ _dl_sysdep_start (void **start_argptr, > > dl_hwcap_check (); > > __tunables_init (_environ); > > + int32_t tes = TUNABLE_GET_FULL (glibc, rtld, enable_secure, int32_t, NULL); > > + if (tes == 1) > > + __libc_enable_secure = 1; > > OK, but maybe better to just do it in __tunables_init. Any specific reason > why you're doing it here? For example, you've missed the hurd dl-sysdep > change. Updated to do this in __tubables_init as you've suggested. Thanks, Joe ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-02 21:16 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-12-05 17:45 [PATCH v2] rtld: Add glibc.rtld.enable_secure tunable Joe Simmons-Talbott 2023-12-12 19:48 ` Siddhesh Poyarekar 2024-01-02 21:16 ` Joe Simmons-Talbott
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).