Ping? Re: [PATCH][v3] Add dynamic linker support for $EXEC_ORIGIN.

[Brooks Moses <bmoses at google dot com>]

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
>