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

[Jordan Abrahams via Libc-alpha <libc-alpha@sourceware.org>]

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