This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Ping^4? Re: [PATCH][v3] Add dynamic linker support for $EXEC_ORIGIN.
- From: Brooks Moses <bmoses at google dot com>
- To: libc-alpha <libc-alpha at sourceware dot org>, "Carlos O'Donnell" <carlos at redhat dot com>, Roland McGrath <roland at hack dot frob dot com>, Andreas Schwab <schwab at linux-m68k dot org>
- Date: Wed, 5 Mar 2014 15:44:58 -0800
- Subject: Ping^4? Re: [PATCH][v3] Add dynamic linker support for $EXEC_ORIGIN.
- Authentication-results: sourceware.org; auth=none
Ping^4?
Original here: https://sourceware.org/ml/libc-alpha/2013-12/msg00816.html
On Wed, Feb 26, 2014 at 10:06 AM, Brooks Moses <bmoses@google.com> wrote:
> Ping^3?
>
> On Sun, Feb 9, 2014 at 8:30 AM, Brooks Moses <bmoses@google.com> wrote:
>> Ping^2?
>>
>> On Sat, Dec 28, 2013 at 10:50 AM, Brooks Moses <bmoses@google.com> wrote:
>>>
>>> Andreas, Carlos, Roland -
>>>
>>> As best I can tell from git history, you three are the people most
>>> familiar with the ld.so code. Any chance one of you could review this
>>> before the code freeze? If so, I'd appreciate it, and if not, I'll
>>> hold off on re-pinging until after the release.
>>>
>>> Thanks,
>>> - Brooks
>>>
>>> On Mon, Dec 23, 2013 at 11:46 AM, Brooks Moses <bmoses@google.com> wrote:
>>> > Updated to include Mike Frysinger's comments, as below. Mike suggests
>>> > that this still needs signoff from someone more familiar with the ld.so
>>> > code; any volunteers?
>>> >
>>> > On Sun, Dec 22, 2013 at 11:51 PM, Mike Frysinger <vapier@gentoo.org> wrote:
>>> >> On Thursday 12 December 2013 17:13:39 Brooks Moses wrote:
>>> >>> --- a/elf/dl-dst.h
>>> >>> +++ b/elf/dl-dst.h
>>> >>>
>>> >>> + char *exec_origin = GLRO(dl_exec_origin_path); \
>>> >>
>>> >> shouldn't that be const ?
>>> >
>>> > Yup, it should. Fixed.
>>> >
>>> >>> --- a/elf/rtld.c
>>> >>> +++ b/elf/rtld.c
>>> >>>
>>> >>> +static void set_exec_origin_path(const char *exe_path);
>>> >>
>>> >> space before the (
>>> >
>>> > Also fixed.
>>> >
>>> >> otherwise, looks OK, but probably should get sign off from someone more
>>> >> familiar with the ldso code
>>> >
>>> > Thanks!
>>> > - Brooks
>>> >
>>> > ---
>>> > The $ORIGIN rpath token expands to the directory containing the true
>>> > copy of the application executable -- which is to say, if the executable
>>> > is invoked through a symlink, that symlink will be resolved in the
>>> > $ORIGIN substitution.
>>> >
>>> > In some cases, such as symlink farms backed by cloud storage filesystems
>>> > where the resolved path encodes effectively-arbitrary storage data, it
>>> > is much more useful to have a rpath token that points to the
>>> > non-expanded, as-called version of the executable path. This patch adds
>>> > the $EXEC_ORIGIN token for this purpose, resolving to the executable
>>> > path as passed to execve().
>>> >
>>> > One potential security risk of such a token expansion is that, although
>>> > copying a setuid/setgid executable (to change its $ORIGIN) loses the
>>> > setuid/setgid bit, creating a symlink does not -- and so a user could
>>> > inject arbitrary code into a setuid/setgid executable that references
>>> > $EXEC_ORIGIN by creating an appropriate symlink. In order to slam the
>>> > door shut on this security risk, this patch simply prohibits the use of
>>> > $EXEC_ORIGIN in setuid/setgid binaries.
>>> >
>>> > ---
>>> > 2013-12-08 Brooks Moses <bmoses@google.com>
>>> >
>>> > * elf/dl-support.c (_dl_exec_origin_path): New global variable.
>>> > * elf/dl-dst.h (DL_DST_REQUIRED): Include _dl_exec_origin_path
>>> > in computing dst_len.
>>> > * elf/dl-load.c (_dl_dst_count): Also handle "EXEC_ORIGIN"
>>> > token.
>>> > (_dl_dst_substitute): Likewise.
>>> > * elf/rtld.c (get_at_execfn): New static function.
>>> > (get_directory): New static function.
>>> > (set_exec_origin_path): New static function.
>>> > (dl_main): Call set_exec_origin_path.
>>> > * sysdeps/generic/ldsodefs.h (rtld_global_ro): Add
>>> > _dl_exec_origin_path.
>>> >
>>> > elf/dl-dst.h | 9 ++++--
>>> > elf/dl-load.c | 10 ++++++-
>>> > elf/dl-support.c | 3 ++
>>> > elf/rtld.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>>> > sysdeps/generic/ldsodefs.h | 3 ++
>>> > 5 files changed, 94 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/elf/dl-dst.h b/elf/dl-dst.h
>>> > index 3ed95d0..13cd0e7 100644
>>> > --- a/elf/dl-dst.h
>>> > +++ b/elf/dl-dst.h
>>> > @@ -65,8 +65,13 @@
>>> > else \
>>> > dst_len = (l)->l_origin == (char *) -1 \
>>> > ? 0 : strlen ((l)->l_origin); \
>>> > - dst_len = MAX (MAX (dst_len, GLRO(dl_platformlen)), \
>>> > - strlen (DL_DST_LIB)); \
>>> > + \
>>> > + const char *exec_origin = GLRO(dl_exec_origin_path); \
>>> > + size_t exec_origin_len = \
>>> > + (exec_origin == NULL) ? 0 : strlen (exec_origin); \
>>> > + \
>>> > + dst_len = MAX (MAX (MAX (dst_len, GLRO(dl_platformlen)), \
>>> > + strlen (DL_DST_LIB)), exec_origin_len); \
>>> > if (dst_len > 4) \
>>> > __len += __cnt * (dst_len - 4); \
>>> > } \
>>> > diff --git a/elf/dl-load.c b/elf/dl-load.c
>>> > index d3e1cf8..04956dc 100644
>>> > --- a/elf/dl-load.c
>>> > +++ b/elf/dl-load.c
>>> > @@ -306,7 +306,8 @@ _dl_dst_count (const char *name, int is_path)
>>> > if ((len = is_dst (start, name, "ORIGIN", is_path,
>>> > INTUSE(__libc_enable_secure))) != 0
>>> > || (len = is_dst (start, name, "PLATFORM", is_path, 0)) != 0
>>> > - || (len = is_dst (start, name, "LIB", is_path, 0)) != 0)
>>> > + || (len = is_dst (start, name, "LIB", is_path, 0)) != 0
>>> > + || (len = is_dst (start, name, "EXEC_ORIGIN", is_path, 0)) != 0)
>>> > ++cnt;
>>> >
>>> > name = strchr (name + len, '$');
>>> > @@ -350,6 +351,13 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result,
>>> > repl = GLRO(dl_platform);
>>> > else if ((len = is_dst (start, name, "LIB", is_path, 0)) != 0)
>>> > repl = DL_DST_LIB;
>>> > + else if ((len = is_dst (start, name, "EXEC_ORIGIN", is_path, 0)) != 0)
>>> > + {
>>> > + if (INTUSE(__libc_enable_secure) != 0)
>>> > + _dl_fatal_printf ("$EXEC_ORIGIN rpath entry not allowed in setuid/setgid executables.\n");
>>> > +
>>> > + repl = GLRO(dl_exec_origin_path);
>>> > + }
>>> >
>>> > if (repl != NULL && repl != (const char *) -1)
>>> > {
>>> > diff --git a/elf/dl-support.c b/elf/dl-support.c
>>> > index 2023bd0..d2d427a 100644
>>> > --- a/elf/dl-support.c
>>> > +++ b/elf/dl-support.c
>>> > @@ -67,6 +67,9 @@ void *__libc_stack_end;
>>> > /* Path where the binary is found. */
>>> > const char *_dl_origin_path;
>>> >
>>> > +/* Directory where the AT_EXECFN is found. */
>>> > +const char *_dl_exec_origin_path;
>>> > +
>>> > /* Nonzero if runtime lookup should not update the .got/.plt. */
>>> > int _dl_bind_not;
>>> >
>>> > diff --git a/elf/rtld.c b/elf/rtld.c
>>> > index 3d207a3..0b3dd1f 100644
>>> > --- a/elf/rtld.c
>>> > +++ b/elf/rtld.c
>>> > @@ -51,6 +51,9 @@ extern __typeof (__mempcpy) __mempcpy attribute_hidden;
>>> > extern __typeof (_exit) exit_internal asm ("_exit") attribute_hidden;
>>> > #define _exit exit_internal
>>> >
>>> > +/* Given file path, return absolute directory path. */
>>> > +static char * get_directory (const char *file_path);
>>> > +
>>> > /* Helper function to handle errors while resolving symbols. */
>>> > static void print_unresolved (int errcode, const char *objname,
>>> > const char *errsting);
>>> > @@ -73,6 +76,9 @@ enum mode { normal, list, verify, trace };
>>> > all the entries. */
>>> > static void process_envvars (enum mode *modep);
>>> >
>>> > +/* Set GLRO(dl_exec_origin_path). */
>>> > +static void set_exec_origin_path (const char *exe_path);
>>> > +
>>> > #ifdef DL_ARGV_NOT_RELRO
>>> > int _dl_argc attribute_hidden;
>>> > char **_dl_argv = NULL;
>>> > @@ -1032,6 +1038,8 @@ of this helper program; chances are you did not intend to run this program.\n\
>>> > in LIST\n\
>>> > --audit LIST use objects named in LIST as auditors\n");
>>> >
>>> > + set_exec_origin_path (INTUSE(_dl_argv)[1]);
>>> > +
>>> > ++_dl_skip_args;
>>> > --_dl_argc;
>>> > ++INTUSE(_dl_argv);
>>> > @@ -1126,6 +1134,8 @@ of this helper program; chances are you did not intend to run this program.\n\
>>> > }
>>> > else
>>> > {
>>> > + set_exec_origin_path ((const char *) getauxval (AT_EXECFN));
>>> > +
>>> > /* Create a link_map for the executable itself.
>>> > This will be what dlopen on "" returns. */
>>> > main_map = _dl_new_object ((char *) "", "", lt_executable, NULL,
>>> > @@ -2807,3 +2817,65 @@ print_statistics (hp_timing_t *rtld_total_timep)
>>> > }
>>> > #endif
>>> > }
>>> > +
>>> > +/* Given file path, return an absolute directory path.
>>> > + Examples: in: "/foo/bar/a.out", out: "/foo/bar/";
>>> > + in: "./a.out", out: "/dot/resolved/to/full/path/./". */
>>> > +static char *
>>> > +get_directory (const char *file_path)
>>> > +{
>>> > + assert (file_path != NULL);
>>> > +
>>> > + /* Find the end of the directory substring in file_path. */
>>> > + size_t path_len = strlen (file_path);
>>> > + while (path_len > 0 && file_path[path_len - 1] != '/')
>>> > + --path_len;
>>> > +
>>> > + /* Allocate space and set the path prefix according to whether or not
>>> > + this is an absolute path. */
>>> > + char *dest;
>>> > + char *full_dir_path;
>>> > + if (file_path[0] == '/')
>>> > + {
>>> > + full_dir_path = malloc (path_len + 1);
>>> > + assert (full_dir_path != NULL);
>>> > + dest = full_dir_path;
>>> > + }
>>> > + else
>>> > + {
>>> > + /* For a relative path, we need to include space for the largest
>>> > + possible current path, a joining '/', the relevant part of
>>> > + file_path, and a trailing '\0'. */
>>> > + full_dir_path = malloc (PATH_MAX + path_len + 2);
>>> > + assert (full_dir_path != NULL);
>>> > +
>>> > + char *status = __getcwd (full_dir_path, PATH_MAX);
>>> > + assert (status != NULL);
>>> > +
>>> > + char *dest = __rawmemchr (full_dir_path, '\0');
>>> > + if (dest[-1] != '/')
>>> > + *dest++ = '/';
>>> > + }
>>> > +
>>> > + if (path_len > 0)
>>> > + dest = __mempcpy (dest, file_path, path_len);
>>> > + *dest = '\0';
>>> > +
>>> > + /* Confirm that the constructed path is valid. */
>>> > + struct stat64 st;
>>> > + assert (__xstat64 (_STAT_VER, full_dir_path, &st) == 0);
>>> > +
>>> > + return full_dir_path;
>>> > +}
>>> > +
>>> > +/* Set GLRO(dl_exec_origin_path). */
>>> > +static void
>>> > +set_exec_origin_path (const char *exe_path)
>>> > +{
>>> > + assert (GLRO(dl_exec_origin_path) == NULL);
>>> > +
>>> > + if (GLRO(dl_origin_path) != NULL)
>>> > + GLRO(dl_exec_origin_path) = strdup (GLRO(dl_origin_path));
>>> > + else if (exe_path != NULL)
>>> > + GLRO(dl_exec_origin_path) = get_directory (exe_path);
>>> > +}
>>> > diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
>>> > index 146aca4..561a763 100644
>>> > --- a/sysdeps/generic/ldsodefs.h
>>> > +++ b/sysdeps/generic/ldsodefs.h
>>> > @@ -518,6 +518,9 @@ struct rtld_global_ro
>>> > /* Location of the binary. */
>>> > EXTERN const char *_dl_origin_path;
>>> >
>>> > + /* Directory where the AT_EXECFN is found. */
>>> > + EXTERN const char *_dl_exec_origin_path;
>>> > +
>>> > /* -1 if the dynamic linker should honor library load bias,
>>> > 0 if not, -2 use the default (honor biases for normal
>>> > binaries, don't honor for PIEs). */
>>> > --
>>> > 1.8.5.1
>>> >