Re: [PATCH v4] rtld: Add glibc.rtld.enable_secure tunable.

[Joe Simmons-Talbott <josimmon@redhat.com>]

On Fri, Jan 12, 2024 at 05:53:23AM -0800, H.J. Lu wrote:
> On Fri, Jan 12, 2024 at 5:44 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote:
> >
> > Add a tunable for setting __libc_enable_secure to 1.  Do not set
> > __libc_enable_secure to 0 if the tunable is set to 0.  Ignore all
> > tunables if glib.rtld.enable_secure is set.  One use-case for this
> > addition is to enable testing code paths that depend on
> > __libc_enable_secure being set without the need to use setxid binaries.
> > ---
> > Changes to v3:
> >   * Rebase and fix merge conflict in NEWS.
> >
> > Changes to v2:
> >   * handle the tunable in __tunables_init so that it's done in a single
> >     place.
> >   * ignore all tunables if the tunable is set.
> >   * update the testcase to only check the tunables if the enable_secure
> >     tunable is not set.
> >   * don't add tunables_strcmp as there is now already a version.
> >
> > Changes to v1:
> >   * handle the tunable for the dynamic loader as well.
> >
> >  NEWS                             |   5 ++
> >  elf/Makefile                     |   2 +
> >  elf/dl-tunables.c                |  11 +++
> >  elf/dl-tunables.list             |   6 ++
> >  elf/tst-rtld-list-tunables.exp   |   1 +
> >  elf/tst-tunables-enable_secure.c | 126 +++++++++++++++++++++++++++++++
> >  6 files changed, 151 insertions(+)
> >  create mode 100644 elf/tst-tunables-enable_secure.c
> >
> > diff --git a/NEWS b/NEWS
> > index 83ae627f47..aff44f6d7f 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -55,6 +55,11 @@ Major new features:
> >    unsigned char, unsigned short, unsigned int, unsigned long int and
> >    unsigned long long int, and a type-generic macro.
> >
> > +* A new tunable, glibc.rtld.enable_secure, 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/elf/Makefile b/elf/Makefile
> > index 5d78b659ce..45a6aa7a8d 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
> > @@ -2676,6 +2677,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 03e1a68675..d3ccd2ecd4 100644
> > --- a/elf/dl-tunables.c
> > +++ b/elf/dl-tunables.c
> > @@ -223,6 +223,17 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
> >             {
> >               tunables[ntunables++] =
> >                 (struct tunable_toset_t) { cur, value, p - value };
> > +
> > +             /* Ignore tunables if enable_secure is set */
> > +             if (tunable_is_name ("glibc.rtld.enable_secure", name))
> > +               {
> > +                  tunable_num_t val = (tunable_num_t) _dl_strtoul (value, NULL);
> > +                 if (val == 1)
> > +                   {
> > +                     __libc_enable_secure = 1;
> > +                     return 0;
> > +                   }
> > +               }
> >               break;
> >             }
> >         }
> > diff --git a/elf/dl-tunables.list b/elf/dl-tunables.list
> > index 1b40407814..1186272c81 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..790d14237e
> > --- /dev/null
> > +++ b/elf/tst-tunables-enable_secure.c
> > @@ -0,0 +1,126 @@
> > +/* Check GLIBC_TUNABLES parsing for enable_secure.
> > +   Copyright (C) 2023 Free Software Foundation, Inc.
> 
> 2024
> 

Thanks for catching that.  Updated in v5.

Thanks,
Joe