Message-Id: <569B4719-6283-4575-A16E-D0A78D280F4E@amacapital.net>
Date:   Tue, 19 Jun 2018 07:50:43 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andy Lutomirski <luto@...nel.org>,
        "H. J. Lu" <hjl.tools@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        LKML <linux-kernel@...r.kernel.org>, linux-doc@...r.kernel.org,
        Linux-MM <linux-mm@...ck.org>,
        linux-arch <linux-arch@...r.kernel.org>, X86 ML <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        "Shanbhogue, Vedvyas" <vedvyas.shanbhogue@...el.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Jonathan Corbet <corbet@....net>,
        Oleg Nesterov <oleg@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        mike.kravetz@...cle.com, Florian Weimer <fweimer@...hat.com>
Subject: Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack



> On Jun 18, 2018, at 5:52 PM, Kees Cook <keescook@...omium.org> wrote:
> 
>> On Mon, Jun 18, 2018 at 3:03 PM, Andy Lutomirski <luto@...nel.org> wrote:
>>> On Tue, Jun 12, 2018 at 12:34 PM H.J. Lu <hjl.tools@...il.com> wrote:
>>> 
>>>> On Tue, Jun 12, 2018 at 11:59 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
>>>>> On Tue, 12 Jun 2018, H.J. Lu wrote:
>>>>>> On Tue, Jun 12, 2018 at 9:34 AM, Andy Lutomirski <luto@...nel.org> wrote:
>>>>>>> On Tue, Jun 12, 2018 at 9:05 AM H.J. Lu <hjl.tools@...il.com> wrote:
>>>>>>>> On Tue, Jun 12, 2018 at 9:01 AM, Andy Lutomirski <luto@...nel.org> wrote:
>>>>>>>>> On Tue, Jun 12, 2018 at 4:43 AM H.J. Lu <hjl.tools@...il.com> wrote:
>>>>>>>>>> On Tue, Jun 12, 2018 at 3:03 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
>>>>>>>>>> That works for stuff which loads all libraries at start time, but what
>>>>>>>>>> happens if the program uses dlopen() later on? If CET is force locked and
>>>>>>>>>> the library is not CET enabled, it will fail.
>>>>>>>>> 
>>>>>>>>> That is to prevent disabling CET by dlopening a legacy shared library.
>>>>>>>>> 
>>>>>>>>>> I don't see the point of trying to support CET by magic. It adds complexity
>>>>>>>>>> and you'll never be able to handle all corner cases correctly. dlopen() is
>>>>>>>>>> not even a corner case.
>>>>>>>>> 
>>>>>>>>> That is a price we pay for security.  To enable CET, especially shadow
>>>>>>>>> shack, the program and all of shared libraries it uses should be CET
>>>>>>>>> enabled.  Most of programs can be enabled with CET by compiling them
>>>>>>>>> with -fcf-protection.
>>>>>>>> 
>>>>>>>> If you charge too high a price for security, people may turn it off.
>>>>>>>> I think we're going to need a mode where a program says "I want to use
>>>>>>>> the CET, but turn it off if I dlopen an unsupported library".  There
>>>>>>>> are programs that load binary-only plugins.
>>>>>>> 
>>>>>>> You can do
>>>>>>> 
>>>>>>> # export GLIBC_TUNABLES=glibc.tune.hwcaps=-SHSTK
>>>>>>> 
>>>>>>> which turns off shadow stack.
>>>>>>> 
>>>>>> 
>>>>>> Which exactly illustrates my point.  By making your security story too
>>>>>> absolute, you'll force people to turn it off when they don't need to.
>>>>>> If I'm using a fully CET-ified distro and I'm using a CET-aware
>>>>>> program that loads binary plugins, and I may or may not have an old
>>>>>> (binary-only, perhaps) plugin that doesn't support CET, then the
>>>>>> behavior I want is for CET to be on until I dlopen() a program that
>>>>>> doesn't support it.  Unless there's some ABI reason why that can't be
>>>>>> done, but I don't think there is.
>>>>> 
>>>>> We can make it opt-in via GLIBC_TUNABLES.  But by default, the legacy
>>>>> shared object is disallowed when CET is enabled.
>>>> 
>>>> That's a bad idea. Stuff has launchers which people might not be able to
>>>> change. So they will simply turn of CET completely or it makes them hack
>>>> horrible crap into init, e.g. the above export.
>>>> 
>>>> Give them sane kernel options:
>>>> 
>>>>     cet = off, relaxed, forced
>>>> 
>>>> where relaxed allows to run binary plugins. Then let dlopen() call into the
>>>> kernel with the filepath of the library to check for CET and that will tell
>>>> you whether its ok or or not and do the necessary magic in the kernel when
>>>> CET has to be disabled due to a !CET library/application.
>>>> 
>>>> That's also making the whole thing independent of magic glibc environment
>>>> options and allows it to be used all over the place in the same way.
>>> 
>>> This is very similar to our ARCH_CET_EXEC proposal which controls how
>>> CET should be enforced.   But Andy thinks it is a bad idea.
>> 
>> I do think it's a bad idea to have a new piece of state that survives
>> across exec().  It's going to have nasty usability problems and nasty
>> security problems.
>> 
>> We may need a mode by which glibc can turn CET *back off* even after a
>> program had it on if it dlopens() an old binary.  Or maybe there won't
>> be demand.  I can certainly understand why the CET_LOCK feature is
>> there, although I think we need a way to override it using something
>> like ptrace().  I'm not convinced that CET_LOCK is really needed, but
>> someone who understand the thread model should chime in.
>> 
>> Kees, do you know anyone who has a good enough understanding of
>> usermode exploits and how they'll interact with CET?
> 
> Adding Florian to CC, but if something gets CET enabled, it really
> shouldn't have a way to turn it off. If there's a way to turn it off,
> all the ROP research will suddenly turn to exactly one gadget before
> doing the rest of the ROP: turning off CET. Right now ROP is: use
> stack-pivot gadget, do everything else. Allowed CET to turn off will
> just add one step: use CET-off gadget, use stack-pivot gadget, do
> everything else. :P

Fair enough 

> 
> Following Linus's request for "slow introduction" of new security
> features, likely the best approach is to default to "relaxed" (with a
> warning about down-grades), and allow distros/end-users to pick
> "forced" if they know their libraries are all CET-enabled.

I still don’t get what “relaxed” is for.  I think the right design is:

Processes start with CET on or off depending on the ELF note, but they start with CET unlocked no matter what. They can freely switch CET on and off (subject to being clever enough not to crash if they turn it on and then return right off the end of the shadow stack) until they call ARCH_CET_LOCK.

Ptrace gets new APIs to turn CET on and off and to lock and unlock it.  If an attacker finds a “ptrace me and turn off CET” gadget, then they might as well just do “ptrace me and write shell code” instead. It’s basically the same gadget. Keep in mind that the actual sequence of syscalls to do this is incredibly complicated.

It’s unclear to me that forcing CET on belongs in the kernel at all.  By the time an attacker can find a non-CET ELF binary and can exec it in a context where it does their bidding, the attacker is far beyond what CET can even try to help. At this point we’re talking about an attacker who can effectively invoke system(3) with arbitrary parameters, and attackers with *that* power don’t need ROP and the like.

There is a new feature I’d like to see, though: add an ELF note to bless a binary as being an ELF interpreter. And add an LSM callback to validate an ELF interpreter.  Let’s minimize the shenanigans that people who control containers can get up to. (Obviously the ELF note part would need to be opt-in.)

Powered by blists - more mailing lists