[PATCH] Deny preload of files on NOEXEC mounts

[PATCH] Deny preload of files on NOEXEC mounts

[Jordan R Abrahams via Libc-alpha]

I'm from Google's ChromeOS Toolchain team, and I have a security
hardening patch to port upstream. I've confirmed with our
security team that this patch can be public, and I'm looking for
review and feedback.

I'm a new contributor, but Google already has a CLA on file for
the FSF, so it should not be an issue. If I'm missing any
contribution info, please do let me know.

Best regards,
~Jordan R Abrahams

-- >8 --
This commit hardens a security flaw in dl-load.c.

At present, one could dynamically load any shared object files even if
they resided on a NOEXEC mount partition. This introduces an exploit
where an attacker may get around this NOEXEC requirement by preloading
a malicious shared library.

For example, from shell this can look like:

  $ LD_PRELOAD='/tmp/malicious_lib.so' id
  Hello world from malicious_lib!

Which will work even if /tmp/ is a noexec mount.

A proof of concept of this exploit can be found here at
https://bugs.chromium.org/p/chromium/issues/detail?id=1182687
The bug thread is restricted, but we are willing to add
interested parties with the approval of our security team.

This patch hardens against the exploit by checking the file descriptor
if the file lies in a NOEXEC mount partition via an fstatvfs call.
The error string is set, and we jump to the `lose` cleanup code.

With this patch, the above example instead becomes:

  $ LD_PRELOAD='/tmp/malicious_lib.so' id
  /usr/bin/id: error while loading shared libraries: /tmp/malicious_lib.so: file not located on exec mount
  # <... normal stdout for id ...>

Signed-off-by: Jordan R Abrahams <ajordanr@google.com>
---
 elf/dl-load.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 650e4edc35..81ca08c38f 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -29,6 +29,7 @@
 #include <sys/mman.h>
 #include <sys/param.h>
 #include <sys/stat.h>
+#include <sys/statvfs.h>
 #include <sys/types.h>
 #include <gnu/lib-names.h>
 
@@ -1572,6 +1573,20 @@ print_search_path (struct r_search_path_elem **list,
     _dl_debug_printf_c ("\t\t(%s)\n", what);
 }
 \f
+/* Check if a the passed in file descriptor points to file on an executable mount.  */
+static int
+check_exec (int fd)
+{
+    struct statvfs buf;
+    int stated = fstatvfs (fd, &buf);
+    if (stated == 0)
+      {
+	return !(buf.f_flag & ST_NOEXEC);
+      }
+    /* Could not fstat the file.  */
+    return false;
+}
+\f
 /* Open a file and verify it is an ELF file for this architecture.  We
    ignore only ELF files for other architectures.  Non-ELF files and
    ELF files with different header information cause fatal errors since
@@ -1650,8 +1665,8 @@ open_verify (const char *name, int fd,
 #endif
 
   if (fd == -1)
-    /* Open the file.  We always open files read-only.  */
-    fd = __open64_nocancel (name, O_RDONLY | O_CLOEXEC);
+      /* Open the file.  We always open files read-only.  */
+      fd = __open64_nocancel (name, O_RDONLY | O_CLOEXEC);
 
   if (fd != -1)
     {
@@ -1667,6 +1682,14 @@ open_verify (const char *name, int fd,
       __set_errno (0);
       fbp->len = 0;
       assert (sizeof (fbp->buf) > sizeof (ElfW(Ehdr)));
+
+      /* Before we read in the file, check if the file is in an exec mount */
+      if (__glibc_unlikely (!check_exec(fd)))
+	{
+	  errstring = N_("file not located on exec mount");
+	  goto call_lose;
+	}
+
       /* Read in the header.  */
       do
 	{
-- 
2.32.0.402.g57bb445576-goog

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Adhemerval Zanella via Libc-alpha]

On 19/07/2021 16:33, Jordan R Abrahams via Libc-alpha wrote:
> I'm from Google's ChromeOS Toolchain team, and I have a security
> hardening patch to port upstream. I've confirmed with our
> security team that this patch can be public, and I'm looking for
> review and feedback.
> 
> I'm a new contributor, but Google already has a CLA on file for
> the FSF, so it should not be an issue. If I'm missing any
> contribution info, please do let me know.
> 
> Best regards,
> ~Jordan R Abrahams
> 
> -- >8 --
> This commit hardens a security flaw in dl-load.c.
> 
> At present, one could dynamically load any shared object files even if
> they resided on a NOEXEC mount partition. This introduces an exploit
> where an attacker may get around this NOEXEC requirement by preloading
> a malicious shared library.
> 
> For example, from shell this can look like:
> 
>   $ LD_PRELOAD='/tmp/malicious_lib.so' id
>   Hello world from malicious_lib!
> 
> Which will work even if /tmp/ is a noexec mount.
> 
> A proof of concept of this exploit can be found here at
> https://bugs.chromium.org/p/chromium/issues/detail?id=1182687
> The bug thread is restricted, but we are willing to add
> interested parties with the approval of our security team.
> 
> This patch hardens against the exploit by checking the file descriptor
> if the file lies in a NOEXEC mount partition via an fstatvfs call.
> The error string is set, and we jump to the `lose` cleanup code.
> 
> With this patch, the above example instead becomes:
> 
>   $ LD_PRELOAD='/tmp/malicious_lib.so' id
>   /usr/bin/id: error while loading shared libraries: /tmp/malicious_lib.so: file not located on exec mount
>   # <... normal stdout for id ...>
> 
> Signed-off-by: Jordan R Abrahams <ajordanr@google.com>
Don't noexec already prevents that? An example below with the glibc
2.33 on 5.11.0-25.

$ cat test.c 
int main (int argc, char *argv[])
{
  return 0;
}
$ cat libpreload.c 
#include <stdio.h>

static void
__attribute__ ((constructor))
lib_init (void)
{
  printf ("%s\n", __func__);
}
$ gcc -Wall test.c -o test
$ gcc -Wall -shared libpreload.c -o libpreload.so
$ LD_PRELOAD=./libpreload.so ./test 
lib_init
$ dd if=/dev/zero of=loopbackfile.img bs=1M count=1
$ sudo losetup -fP loopbackfile.img 
$ losetup -a | grep loopbackfile.img
/dev/loop16: []: (/tmp/test/loopbackfile.img)
$ mkfs.ext4 loopbackfile.img 
$ mkdir loopfs
$ sudo mount -o loop,noexec /dev/loop16 loopfs
$ mount | grep loopfs
/dev/loop16 on /tmp/test/loopfs type ext4 (rw,noexec,relatime)
$ mv libpreload.so loopfs/
$ LD_PRELOAD=loopfs/libpreload.so ./test 
ERROR: ld.so: object 'loopfs/libpreload.so' from LD_PRELOAD cannot be preloaded (failed to map segment from shared object): ignored.

I am not sure if this is an Ubuntu hardnening, if this is only enabled
on recent kernels, or only enabled for some filesystems.  It seems that
with CONFIG_SECURITY the code on security/security.c does filter out
the PROT_EXEC on mmap_prot() (but you will need to check how all the
pieces does work).

This kind of hardening does sound it would be better implemented by the
kernel itself, although I don't have a strong opinion if the idea is
to hardened against possible kernel version or FS that does not fully 
support it.

Some comments below regarding the code. Did you actually ran the make
check? I am asking because first the code does not build (there is
a wrong label usage) and you would see a lot of linknamespace issues
(I explained better below).

> ---
>  elf/dl-load.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 650e4edc35..81ca08c38f 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -29,6 +29,7 @@
>  #include <sys/mman.h>
>  #include <sys/param.h>
>  #include <sys/stat.h>
> +#include <sys/statvfs.h>
>  #include <sys/types.h>
>  #include <gnu/lib-names.h>
>  
> @@ -1572,6 +1573,20 @@ print_search_path (struct r_search_path_elem **list,
>      _dl_debug_printf_c ("\t\t(%s)\n", what);
>  }
>  \f
> +/* Check if a the passed in file descriptor points to file on an executable mount.  */
> +static int
> +check_exec (int fd)
> +{
> +    struct statvfs buf;
> +    int stated = fstatvfs (fd, &buf);
> +    if (stated == 0)
> +      {
> +	return !(buf.f_flag & ST_NOEXEC);
> +      }
> +    /* Could not fstat the file.  */
> +    return false;
> +}
> +\f

Wrong indentation and besides using fstatvfs that creates a linknamespace
polution, it might defeats the hardening (since one can interpose fstatvfs).
You will need to add a hidden_def (__fstatvfs) and use __fstatvfs() instead.

>  /* Open a file and verify it is an ELF file for this architecture.  We
>     ignore only ELF files for other architectures.  Non-ELF files and
>     ELF files with different header information cause fatal errors since
> @@ -1650,8 +1665,8 @@ open_verify (const char *name, int fd,
>  #endif
>  
>    if (fd == -1)
> -    /* Open the file.  We always open files read-only.  */
> -    fd = __open64_nocancel (name, O_RDONLY | O_CLOEXEC);
> +      /* Open the file.  We always open files read-only.  */
> +      fd = __open64_nocancel (name, O_RDONLY | O_CLOEXEC);
>  
>    if (fd != -1)
>      {

Superfluous change here.

> @@ -1667,6 +1682,14 @@ open_verify (const char *name, int fd,
>        __set_errno (0);
>        fbp->len = 0;
>        assert (sizeof (fbp->buf) > sizeof (ElfW(Ehdr)));
> +
> +      /* Before we read in the file, check if the file is in an exec mount */
> +      if (__glibc_unlikely (!check_exec(fd)))

Space before '(': "check_exext (fd)".

> +	{
> +	  errstring = N_("file not located on exec mount");
> +	  goto call_lose;
> +	}
> +

This should 'lose' (there is no label call_lose).

>        /* Read in the header.  */
>        do
>  	{
> 

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Florian Weimer via Libc-alpha]

* Adhemerval Zanella via Libc-alpha:

> I am not sure if this is an Ubuntu hardnening, if this is only enabled
> on recent kernels, or only enabled for some filesystems.  It seems that
> with CONFIG_SECURITY the code on security/security.c does filter out
> the PROT_EXEC on mmap_prot() (but you will need to check how all the
> pieces does work).
>
> This kind of hardening does sound it would be better implemented by the
> kernel itself, although I don't have a strong opinion if the idea is
> to hardened against possible kernel version or FS that does not fully 
> support it.

Yes, this looks like policy that the kernel should implement.  We could
help the kernel by specifying an O_NEEDEXEC or O_MAYEXEC flag, but the
kernel patches in this area do not seem to have made much progress I
think.

Trying to implement a noexec policy in userspace looks wrong to me.

Thanks,
Florian

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Adhemerval Zanella via Libc-alpha]

On 23/07/2021 16:22, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> I am not sure if this is an Ubuntu hardnening, if this is only enabled
>> on recent kernels, or only enabled for some filesystems.  It seems that
>> with CONFIG_SECURITY the code on security/security.c does filter out
>> the PROT_EXEC on mmap_prot() (but you will need to check how all the
>> pieces does work).
>>
>> This kind of hardening does sound it would be better implemented by the
>> kernel itself, although I don't have a strong opinion if the idea is
>> to hardened against possible kernel version or FS that does not fully 
>> support it.
> 
> Yes, this looks like policy that the kernel should implement.  We could
> help the kernel by specifying an O_NEEDEXEC or O_MAYEXEC flag, but the
> kernel patches in this area do not seem to have made much progress I
> think.

So the noexec not allowing PROT_EXEC is not really upstream? 

> 
> Trying to implement a noexec policy in userspace looks wrong to me.

I am not sure exactly what kind of issue they are trying to prevent,
since LD_PRELOAD won't work on setuid/seguid. My guess is on chromeos
all user-accessible mount point are set with noexec, so kernel will
prevent the direct execution of static binaries and a possible loader.

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Florian Weimer via Libc-alpha]

* Adhemerval Zanella:

> On 23/07/2021 16:22, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> I am not sure if this is an Ubuntu hardnening, if this is only enabled
>>> on recent kernels, or only enabled for some filesystems.  It seems that
>>> with CONFIG_SECURITY the code on security/security.c does filter out
>>> the PROT_EXEC on mmap_prot() (but you will need to check how all the
>>> pieces does work).
>>>
>>> This kind of hardening does sound it would be better implemented by the
>>> kernel itself, although I don't have a strong opinion if the idea is
>>> to hardened against possible kernel version or FS that does not fully 
>>> support it.
>> 
>> Yes, this looks like policy that the kernel should implement.  We could
>> help the kernel by specifying an O_NEEDEXEC or O_MAYEXEC flag, but the
>> kernel patches in this area do not seem to have made much progress I
>> think.
>
> So the noexec not allowing PROT_EXEC is not really upstream?

$ chmod 644 libc.so
$ bash testrun.sh nss/getent
nss/getent: wrong number of arguments
Try `getent --help' or `getent --usage' for more information.

So I would say: no, it's not the default.

>> Trying to implement a noexec policy in userspace looks wrong to me.
>
> I am not sure exactly what kind of issue they are trying to prevent,
> since LD_PRELOAD won't work on setuid/seguid. My guess is on chromeos
> all user-accessible mount point are set with noexec, so kernel will
> prevent the direct execution of static binaries and a possible loader.

It's a frequently requested feature.  Some security policies require
that all writable mounts must also be noexec.  That doesn't do much good
if the bypass is trivial.  Of course other bypasses remain (script
interpreters and suchlike, hence the need for O_MAYEXEC).

Thanks,
Florian

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Adhemerval Zanella via Libc-alpha]

On 23/07/2021 17:50, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 23/07/2021 16:22, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> I am not sure if this is an Ubuntu hardnening, if this is only enabled
>>>> on recent kernels, or only enabled for some filesystems.  It seems that
>>>> with CONFIG_SECURITY the code on security/security.c does filter out
>>>> the PROT_EXEC on mmap_prot() (but you will need to check how all the
>>>> pieces does work).
>>>>
>>>> This kind of hardening does sound it would be better implemented by the
>>>> kernel itself, although I don't have a strong opinion if the idea is
>>>> to hardened against possible kernel version or FS that does not fully 
>>>> support it.
>>>
>>> Yes, this looks like policy that the kernel should implement.  We could
>>> help the kernel by specifying an O_NEEDEXEC or O_MAYEXEC flag, but the
>>> kernel patches in this area do not seem to have made much progress I
>>> think.
>>
>> So the noexec not allowing PROT_EXEC is not really upstream?
> 
> $ chmod 644 libc.so
> $ bash testrun.sh nss/getent
> nss/getent: wrong number of arguments
> Try `getent --help' or `getent --usage' for more information.
> 
> So I would say: no, it's not the default.

I am getting the same error on a centos7 with kernel 3.10.0-1160.31.1.el7.x86_64
and glibc 2.17:

$ mount | grep loopfs
/dev/loop0 on /home/azanella/loopfs type ext4 (rw,*noexec*,relatime,seclabel)
$ LD_PRELOAD=loopfs/libpreload.so ./test 
ERROR: ld.so: object 'loopfs/libpreload.so' from LD_PRELOAD cannot be preloaded: ignored.
$ strace env LD_PRELOAD=loopfs/libpreload.so ./test
[...]
mmap(NULL, 2101304, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = -1 EPERM (Operation not permitted)
[...]

I haven't tested a vanilla kernel yet.

> 
>>> Trying to implement a noexec policy in userspace looks wrong to me.
>>
>> I am not sure exactly what kind of issue they are trying to prevent,
>> since LD_PRELOAD won't work on setuid/seguid. My guess is on chromeos
>> all user-accessible mount point are set with noexec, so kernel will
>> prevent the direct execution of static binaries and a possible loader.
> 
> It's a frequently requested feature.  Some security policies require
> that all writable mounts must also be noexec.  That doesn't do much good
> if the bypass is trivial.  Of course other bypasses remain (script
> interpreters and suchlike, hence the need for O_MAYEXEC).

Interesting, I was not aware of the O_MAYEXEC proposal.  I think we might
try to use on loader/dlfcn interfaces, but since we already mmap PROT_EXEC
I think noexec that proper fails should be suffice.

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Alexander Monakov via Libc-alpha]

On Fri, 23 Jul 2021, Adhemerval Zanella via Libc-alpha wrote:

> 
> 
> $ mount | grep loopfs
> /dev/loop0 on /home/azanella/loopfs type ext4 (rw,*noexec*,relatime,seclabel)
> $ LD_PRELOAD=loopfs/libpreload.so ./test 
> ERROR: ld.so: object 'loopfs/libpreload.so' from LD_PRELOAD cannot be preloaded: ignored.
> $ strace env LD_PRELOAD=loopfs/libpreload.so ./test
> [...]
> mmap(NULL, 2101304, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = -1 EPERM (Operation not permitted)
> [...]
> 
> I haven't tested a vanilla kernel yet.

Vanilla kernel also does that, but I think you might be missing Jordan's point:
this is not about refusing mmap from a noexec mount, this is a counter-measure
against specially crafted ELF files taking over the dynamic loader even before
it attempts a PROT_EXEC mmap, as demonstrated in glibc bug #21718:
https://sourceware.org/bugzilla/show_bug.cgi?id=21718

Alexander

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Jordan Abrahams via Libc-alpha]

Hi all,

Thanks for the wonderful feedback and discussion!

I did want to hop in to clarify a few things and give an update on the
code quality.

>>>> Trying to implement a noexec policy in userspace looks wrong to me.

This is a fair point, and I've forwarded this concern to our security
team. A kernel-level patch already exists for similar behaviour here:
https://lore.kernel.org/kernel-hardening/20201203173118.379271-1-mic@digikod.net/T/
This proposal is for the trusted_for syscall, though it's stalled for
a bit it seems. I'll have to discuss with my team on our priorities if
we want to move forward with that.

Ultimately, a change will need to be done on the glibc side of things
even with a kernel patch. Perhaps not with an fstatfs check, but
rather with a new syscall with more fine grained control. Our security
folk seem to believe it could be cleaner and more robust with a
kernel+glibc change. That being said, according to them, a change
solely in glibc is not out-of-line.

To quote Jann Horn from our security team on this:

> Either way, enforcing this kind of policy will require _some_ kind of
> change to glibc, the kernel can't do it on its own. But regarding
> whether it just should be a glibc change or a glibc+kernel change, I
> think you could argue both ways: You could argue that this is a
> system-wide policy decision that would ideally be directly steered by
> the kernel somehow, or you could argue that this is already a fixed
> policy documented in manpages (see
> https://man7.org/linux/man-pages/man2/mmap.2.html: "EPERM  The prot
> argument asks for PROT_EXEC but the mapped area belongs to a file on a
> filesystem that was mounted no-exec") and therefore it's fine for
> glibc to mirror that policy in cases where the kernel doesn't have
> enough information to do it on its own.

The intent of the patch was meant as a way for glibc to mirror the
safety policy when the kernel cannot know. Perhaps the solution should
be to instead fix the kernel (i.e. trusted_for()).

In response to Alexander,

> this is not about refusing mmap from a noexec mount, this is a counter-measure
> against specially crafted ELF files taking over the dynamic loader even before
> it attempts a PROT_EXEC mmap, as demonstrated in glibc bug #21718

Indeed, it's not about refusing an mmap from a noexec mount. These are
handcrafted ELF files which take over the loader. If you would like to
see the proof of concept of the attack, we can add you to the allowed
listings for the linked bug thread
(https://bugs.chromium.org/p/chromium/issues/detail?id=1182687). We
have an ELF for x86_64 devices which shows the above behaviour on our
current kernel of ChromeOS. I'm uncertain of the status of other OSes
(I would need to reach out again to our security team), but I doubt it
would be restricted to just ChromeOS.

On the actual code:
Oops! It looks like a rebase messed up with the patch. It was originally
written for glibc 2.32, and porting it to ToT glibc has led to
mistakes. Doesn't look like I ran through testing with the rebase!
Apologies for this. I'll work on correcting and testing the code once
I've gotten confirmation that this is the correct approach on your
end. Thankfully it doesn't seem like there's much mechanically wrong.
It's just a question as to whether this is a sane approach to the
problem at all.

Thanks,
~Jordan

On Fri, Jul 23, 2021 at 2:27 PM Alexander Monakov <amonakov@ispras.ru> wrote:
>
> On Fri, 23 Jul 2021, Adhemerval Zanella via Libc-alpha wrote:
>
> >
> >
> > $ mount | grep loopfs
> > /dev/loop0 on /home/azanella/loopfs type ext4 (rw,*noexec*,relatime,seclabel)
> > $ LD_PRELOAD=loopfs/libpreload.so ./test
> > ERROR: ld.so: object 'loopfs/libpreload.so' from LD_PRELOAD cannot be preloaded: ignored.
> > $ strace env LD_PRELOAD=loopfs/libpreload.so ./test
> > [...]
> > mmap(NULL, 2101304, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = -1 EPERM (Operation not permitted)
> > [...]
> >
> > I haven't tested a vanilla kernel yet.
>
> Vanilla kernel also does that, but I think you might be missing Jordan's point:
> this is not about refusing mmap from a noexec mount, this is a counter-measure
> against specially crafted ELF files taking over the dynamic loader even before
> it attempts a PROT_EXEC mmap, as demonstrated in glibc bug #21718:
> https://sourceware.org/bugzilla/show_bug.cgi?id=21718
>
> Alexander

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Florian Weimer via Libc-alpha]

* Jordan Abrahams:

> In response to Alexander,
>
>> this is not about refusing mmap from a noexec mount, this is a counter-measure
>> against specially crafted ELF files taking over the dynamic loader even before
>> it attempts a PROT_EXEC mmap, as demonstrated in glibc bug #21718
>
> Indeed, it's not about refusing an mmap from a noexec mount. These are
> handcrafted ELF files which take over the loader. If you would like to
> see the proof of concept of the attack, we can add you to the allowed
> listings for the linked bug thread
> (https://bugs.chromium.org/p/chromium/issues/detail?id=1182687). We
> have an ELF for x86_64 devices which shows the above behaviour on our
> current kernel of ChromeOS. I'm uncertain of the status of other OSes
> (I would need to reach out again to our security team), but I doubt it
> would be restricted to just ChromeOS.

There have been public discussions about such bugs before, not sure why
you are restricting it.

I expect that it will always be possible to take over the dynamic loader
until we are more restrictive when it comes to use of MAP_FIXED in the
loader, probably by using MAP_FIXED_NOREPLACE.  The challenge is to make
the change so conservative that existing (slightly broken, or ia64)
binaries are not impacted.

Thanks,
Florian

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Jordan Abrahams via Libc-alpha]

Hi Florian,

> There have been public discussions about such bugs before, not sure why
> you are restricting it.

I apologise. We can add any interested individuals to view the bug
thread (including you! Just let me know). However it's not in my
ability to make that thread public. Our security team originally
wished access to it to be restricted, and I'm obligated to abide by
that. I assume it's restricted because it has an example program which
bypasses noexec on ChromeOS devices, and Google doesn't want to risk
anything. I _can_ forward a request along to make it public, but I'm
afraid it's not my decision or in my power to do so.

> I expect that it will always be possible to take over the dynamic loader
> until we are more restrictive when it comes to use of MAP_FIXED in the
> loader, probably by using MAP_FIXED_NOREPLACE.  The challenge is to make
> the change so conservative that existing (slightly broken, or ia64)
> binaries are not impacted.

Given this, what are our options on getting dynamic loader attack
preventions upstream? For example, we don't have ia64 testing
ourselves, so we can't verify no impact even if we attempted to patch
the loader on our side.

We'd obviously like to have a more permanent upstream fix. O_NEEDEXEC
/ O_MAYEXEC seems like a fine long term solution in the kernel, and
we're trying to weigh the benefits of that versus maintaining this
glibc noexec patch locally. We don't know how long that would take
however, and our toolchain team certainly doesn't have the needed
expertise or cycles at this time. Plus we'd still also need an interim
solution while that is being worked on.

Thanks,
Jordan

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Jann Horn via Libc-alpha]

On Mon, Aug 23, 2021 at 1:30 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Jordan Abrahams:
>
> > In response to Alexander,
> >
> >> this is not about refusing mmap from a noexec mount, this is a counter-measure
> >> against specially crafted ELF files taking over the dynamic loader even before
> >> it attempts a PROT_EXEC mmap, as demonstrated in glibc bug #21718
> >
> > Indeed, it's not about refusing an mmap from a noexec mount. These are
> > handcrafted ELF files which take over the loader. If you would like to
> > see the proof of concept of the attack, we can add you to the allowed
> > listings for the linked bug thread
> > (https://bugs.chromium.org/p/chromium/issues/detail?id=1182687). We
> > have an ELF for x86_64 devices which shows the above behaviour on our
> > current kernel of ChromeOS. I'm uncertain of the status of other OSes
> > (I would need to reach out again to our security team), but I doubt it
> > would be restricted to just ChromeOS.
>
> There have been public discussions about such bugs before, not sure why
> you are restricting it.
>
> I expect that it will always be possible to take over the dynamic loader
> until we are more restrictive when it comes to use of MAP_FIXED in the
> loader, probably by using MAP_FIXED_NOREPLACE.  The challenge is to make
> the change so conservative that existing (slightly broken, or ia64)
> binaries are not impacted.

My understanding is that the ChromeOS security folks (I'm not on that
team) would like to ensure that, as a security hardening measure,
attempting to load an attacker-controlled library from a noexec mount
doesn't give the attacker the ability to run code in the context of
the loader.

As far as I understand, the kind of ELF file I used in the linked bug
(sorry about the access restriction) looks fairly legitimate: It
doesn't clobber any existing mappings, instead it uses a ".init_array"
section containing a relocation into glibc, like this:

        .section        .init_array,"aw"
        .align 8
        .quad   rmdir+0x774

By picking an appropriate gadget in glibc, and making use of the
register contents at the indirect call to the fake constructor
function, it is possible to use that to e.g. start a ROP chain.


I think there are also more fundamental things that a library without
any executable segments could do, by design, to compromise the
integrity of the process - e.g. depending on when the evil library is
loaded, it might be able to override a pointer symbol that was
supposed to be located in a ".bss" section to point at the evil
library's ".data" section instead and use that to effectively clobber
a NULL-initialized pointer variable with an arbitrary value, or
something like that?

This is why I think that as long as you accept the premise that NOEXEC
should robustly prevent library loading, the loader (possibly in
collaboration with the kernel) has to ensure that even libraries
without executable segments cannot be loaded from non-executable
files.

(Btw, this also means that IMA-based auditing of library loading based
on executable mappings is currently bypassable. I don't think anyone
really cares about that though - I brought it up with the IMA
maintainers back in 2018 and IIRC they didn't see it as a problem.)

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Florian Weimer via Libc-alpha]

* Jann Horn:

> My understanding is that the ChromeOS security folks (I'm not on that
> team) would like to ensure that, as a security hardening measure,
> attempting to load an attacker-controlled library from a noexec mount
> doesn't give the attacker the ability to run code in the context of
> the loader.
>
> As far as I understand, the kind of ELF file I used in the linked bug
> (sorry about the access restriction) looks fairly legitimate: It
> doesn't clobber any existing mappings, instead it uses a ".init_array"
> section containing a relocation into glibc, like this:
>
>         .section        .init_array,"aw"
>         .align 8
>         .quad   rmdir+0x774
>
> By picking an appropriate gadget in glibc, and making use of the
> register contents at the indirect call to the fake constructor
> function, it is possible to use that to e.g. start a ROP chain.

Ah, that really needs trusted_for to fix.

Many distributions do not even mark shared objects as executable, so
executable permission is not something we can enforce in the dynamic
loader.

> I think there are also more fundamental things that a library without
> any executable segments could do, by design, to compromise the
> integrity of the process - e.g. depending on when the evil library is
> loaded, it might be able to override a pointer symbol that was
> supposed to be located in a ".bss" section to point at the evil
> library's ".data" section instead and use that to effectively clobber
> a NULL-initialized pointer variable with an arbitrary value, or
> something like that?

A symbol could have a value that is not within the current shared
object, pointing into some other shared object's executable segment.

I don't think ELF has a generic redirection mechanism that isn't based
on code execution, though.  There is something limited for canonical
function addresses, but I'm not sure if it can be used this way.

> (Btw, this also means that IMA-based auditing of library loading based
> on executable mappings is currently bypassable. I don't think anyone
> really cares about that though - I brought it up with the IMA
> maintainers back in 2018 and IIRC they didn't see it as a problem.)

IMA only covers the contents, I think.  Neither paths nor even the most
basic file attributes are covered by the digest.

Thanks,
Florian

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Mike Frysinger via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On 06 Sep 2021 17:10, Florian Weimer via Libc-alpha wrote:
> * Jann Horn:
> > My understanding is that the ChromeOS security folks (I'm not on that
> > team) would like to ensure that, as a security hardening measure,
> > attempting to load an attacker-controlled library from a noexec mount
> > doesn't give the attacker the ability to run code in the context of
> > the loader.
> >
> > As far as I understand, the kind of ELF file I used in the linked bug
> > (sorry about the access restriction) looks fairly legitimate: It
> > doesn't clobber any existing mappings, instead it uses a ".init_array"
> > section containing a relocation into glibc, like this:
> >
> >         .section        .init_array,"aw"
> >         .align 8
> >         .quad   rmdir+0x774
> >
> > By picking an appropriate gadget in glibc, and making use of the
> > register contents at the indirect call to the fake constructor
> > function, it is possible to use that to e.g. start a ROP chain.
> 
> Ah, that really needs trusted_for to fix.
> 
> Many distributions do not even mark shared objects as executable, so
> executable permission is not something we can enforce in the dynamic
> loader.

we shouldn't let bad distro decisions constrain possibilities.  we have the
ability to move the ecosystem.  we can add new features that are disabled by
default via configure options with warnings (including in NEWS) that these
will change in the future.  distros will migrate if they want newer glibc.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Cristian Rodríguez]

On Mon, Aug 23, 2021 at 7:30 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
> The challenge is to make
> the change so conservative that existing (slightly broken, or ia64)
> binaries are not impacted.

I don't see why a deceased arch has to block any other work.. that
would be very weird.. something that is no longer shipping and its
manufacturer has given up on shouldn't be stopping free software....

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Adhemerval Zanella via Libc-alpha]

On 23/08/2021 20:40, Cristian Rodríguez wrote:
> On Mon, Aug 23, 2021 at 7:30 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>> The challenge is to make
>> the change so conservative that existing (slightly broken, or ia64)
>> binaries are not impacted.
> 
> I don't see why a deceased arch has to block any other work.. that
> would be very weird.. something that is no longer shipping and its
> manufacturer has given up on shouldn't be stopping free software....
> 

It should not indeed, if an architecture does not support the best way
forward is to just disable the feature on the affected ones.  I think
what Florian is saying it how to properly enable it without breaking
other architectures.

Re: [PATCH] Deny preload of files on NOEXEC mounts

[Adhemerval Zanella via Libc-alpha]

On 23/07/2021 18:27, Alexander Monakov wrote:
> On Fri, 23 Jul 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>>
>>
>> $ mount | grep loopfs
>> /dev/loop0 on /home/azanella/loopfs type ext4 (rw,*noexec*,relatime,seclabel)
>> $ LD_PRELOAD=loopfs/libpreload.so ./test 
>> ERROR: ld.so: object 'loopfs/libpreload.so' from LD_PRELOAD cannot be preloaded: ignored.
>> $ strace env LD_PRELOAD=loopfs/libpreload.so ./test
>> [...]
>> mmap(NULL, 2101304, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = -1 EPERM (Operation not permitted)
>> [...]
>>
>> I haven't tested a vanilla kernel yet.
> 
> Vanilla kernel also does that, but I think you might be missing Jordan's point:
> this is not about refusing mmap from a noexec mount, this is a counter-measure
> against specially crafted ELF files taking over the dynamic loader even before
> it attempts a PROT_EXEC mmap, as demonstrated in glibc bug #21718:
> https://sourceware.org/bugzilla/show_bug.cgi?id=21718
> 
> Alexander
> 

Yes I am fully aware of that, but this is not what was the idea for the loader
laid out some years ago [1]. So maybe we should discuss again the design goal 
and reevaluate some ideas. For instance, should we discuss to remove the multiple 
features that are really a security hazards that we still support due compatibility? 
What about somewhat useful features that contains multiple issue and possible 
security issue like rtld-audit?

I tend to agree with Siddhesh, but at same time trying to parse ill-formed ELF
files are not easy tasks and might clash with current idea of a somewhat simple
and fast loader.  We already ave multiple layers or security and filtering in
current modern OS, will adding another complexity on the loader really pay
off?

If we really want to focus on loader hardening, I think we really should use
Carlos's approach along with the idea of starting *deprecate* and remove some
features, even if it will result in incompatibilities (yes, some may frown
upon it, but it past time to really start to stop support things like
exec-stack, text-rels, and ldd.bash.in).

[1] https://sourceware.org/pipermail/libc-alpha/2015-July/062464.html