Message-ID: <20200926155502.GA930344@rani.riverdale.lan>
Date:   Sat, 26 Sep 2020 11:55:02 -0400
From:   Arvind Sankar <nivedita@...m.mit.edu>
To:     "Madhavan T. Venkataraman" <madvenka@...ux.microsoft.com>
Cc:     Arvind Sankar <nivedita@...m.mit.edu>,
        Florian Weimer <fw@...eb.enyo.de>,
        kernel-hardening@...ts.openwall.com, linux-api@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-fsdevel@...r.kernel.org, linux-integrity@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org, oleg@...hat.com,
        x86@...nel.org, libffi-discuss@...rceware.org, luto@...nel.org,
        David.Laight@...LAB.COM, mark.rutland@....com, mic@...ikod.net,
        pavel@....cz
Subject: Re: [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor

On Fri, Sep 25, 2020 at 05:44:56PM -0500, Madhavan T. Venkataraman wrote:
> 
> 
> On 9/24/20 6:43 PM, Arvind Sankar wrote:
> > 
> > The source PC will generally not be available if the compiler decided to
> > tail-call optimize the call to the trampoline into a jump.
> > 
> 
> This is still work in progress. But I am thinking that labels can be used.
> So, if the code is:
> 
> 	invoke_tramp:
> 		(*tramp)();
> 
> then, invoke_tramp can be supplied as the calling PC.
> 
> Similarly, labels can be used in assembly functions as well.
> 
> Like I said, I have to think about this more.

What I mean is that the kernel won't have access to the actual source
PC. If I followed your v1 correctly, it works by making any branch to
the trampoline code trigger a page fault. At this point, the PC has
already been updated to the trampoline entry, so the only thing the
fault handler can know is the return address on the top of the stack,
which (a) might not be where the branch actually originated, either
because it was a jump, or you've already been hacked and you got here
using a ret; (b) is available to userspace anyway.

> 
> > What's special about these trampolines anyway? Any indirect function
> > call could have these same problems -- an attacker could have
> > overwritten the pointer the same way, whether it's supposed to point to
> > a normal function or it is the target of this trampoline.
> > 
> > For making them a bit safer, userspace could just map the page holding
> > the data pointers/destination address(es) as read-only after
> > initialization.
> > 
> 
> You need to look at version 1 of trampfd for how to do "allowed pcs".
> As an example, libffi defines ABI handlers for every arch-ABI combo.
> These ABI handler pointers could be placed in an array in .rodata.
> Then, the array can be written into trampfd for setting allowed PCS.
> When the target PC is set for a trampoline, the kernel will check
> it against allowed PCs and reject it if it has been overwritten.

I'm not asking how it's implemented. I'm asking what's the point? On a
typical linux system, at least on x86, every library function call is an
indirect branch. The protection they get is that the dynamic linker can
map the pointer table read-only after initializing it.

For the RO mapping, libffi could be mapping both the entire closure
structure, as well as the structure that describes the arguments and
return types of the function, read-only once they are initialized.

For libffi, there are three indirect branches for every trampoline call
with your suggested trampoline: one to get to the trampoline, one to
jump to the handler, and one to call the actual user function. If we are
particularly concerned about the trampoline to handler branch for some
reason, we could just replace it with a direct branch: if the kernel was
generating the code, there's no reason to allow the data pointer or code
target to be changed after the trampoline was created. It can just
hard-code them in the generated code and be done with it. Even with
user-space trampolines, you can use a direct call. All you need is
libffi-trampoline.so which contains a few thousand trampolines all
jumping to one handler, which then decides what to do based on which
trampoline was called. Sure libffi currently dispatches to one of 2-3
handlers based on the ABI, but there's no technical reason it couldn't
dispatch to just one that handled all the ABIs, and the trampoline could
be boiled down to just:
	endbr
	call handler
	ret

> >>
> >> In order to address the FFI_REGISTER ABI in libffi, we could use the secure
> >> trampoline. In FFI_REGISTER, the data is pushed on the stack and the code
> >> is jumped to without using any registers.
> >>
> >> As outlined in version 1, the kernel can push the data address on the stack
> >> and write the code address into the PC and return to userland.
> >>
> >> For doing all of this, we need trampfd.
> > 
> > We don't need this for FFI_REGISTER. I presented a solution that works
> > in userspace. Even if you want to use a trampoline created by the
> > kernel, there's no reason it needs to trap into the kernel at trampoline
> > execution time. libffi's trampolines already handle this case today.
> > 
> 
> libffi handles this using user level dynamic code which needs to be executed.
> If the security subsystem prevents that, then the dynamic code cannot execute.
> That is the whole point of this RFC.

/If/ you are using a trampoline created by the kernel, it can just
create the one that libffi is using today; which doesn't need trapping
into the kernel at execution time.

And if you aren't, you can use the trampoline I wrote, which has no
dynamic code, and doesn't need to trap into the kernel at execution time
either.

> 
> >>
> >> Permitting the use of trampfd
> >> -----------------------------
> >>
> >> An "exectramp" setting can be implemented in SELinux to selectively allow the
> >> use of trampfd for applications.
> >>
> >> Madhavan
> > 
> > Applications can use their own userspace trampolines regardless of this
> > setting, so it doesn't provide any additional security benefit by
> > preventing usage of trampfd.
> > 
> 
> The background for all of this is that dynamic code such as trampolines
> need to be placed in a page with executable permissions so they can
> execute. If security measures such as W^X are present, this will not
> be possible. Admitted, today some user level tricks exist to get around
> W^X. I have alluded to those. IMO, they are all security holes and will
> get plugged sooner or later. Then, these trampolines cannot execute.
> Currently, there exist security exceptions such as execmem to let them
> execute. But we would like to do it without making security exceptions.
> 
> Madhavan

How can you still say this after this whole discussion? Applications can
get the exact same functionality as your proposed trampfd using static
code, no W^X tricks needed.

This only matters if you have a trampfd that generates _truly_ dynamic
code, not just code that can be trivially made static.

Powered by blists - more mailing lists