Message-ID: <A9483B8B-C0DD-46CB-AD5D-D12EC61BB331@zytor.com>
Date:   Sun, 24 May 2020 12:45:18 -0700
From:   hpa@...or.com
To:     Thomas Gleixner <tglx@...utronix.de>,
        Don Porter <porter@...unc.edu>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
CC:     Andi Kleen <ak@...ux.intel.com>, Sasha Levin <sashal@...nel.org>,
        linux-kernel@...r.kernel.org, bp@...en8.de, luto@...nel.org,
        dave.hansen@...el.com, tony.luck@...el.com,
        ravi.v.shankar@...el.com, chang.seok.bae@...el.com
Subject: Re: Re: [PATCH v12 00/18] Enable FSGSBASE instructions

On May 22, 2020 5:45:39 PM PDT, Thomas Gleixner <tglx@...utronix.de> wrote:
>Don,
>
>Don Porter <porter@...unc.edu> writes:
>> On 5/19/20 12:48 PM, Jarkko Sakkinen wrote:
>>> On Tue, May 19, 2020 at 01:03:25AM +0200, Thomas Gleixner wrote:
>>>>
>>>> That justifies to write books which recommend to load a kernel
>module
>>>> which creates a full unpriviledged root hole. I bet none of these
>papers
>>>> ever mentioned that.
>>
>> I wanted to clarify that we never intended the Graphene kernel module
>
>> you mention for production use, as well as to comment in support of
>this 
>> patch.
>
>let me clarify, that despite your intentions:
>
>   - there is not a single word in any paper, slide deck, documentation
>     etc. which mentions that loading this module and enabling FSGSBASE
>      behind the kernels back is a fully unpriviledged root hole.
>
>    - the module lacks a big fat warning emitted to dmesg, that this
>      turns the host kernel into a complete security disaster.
>
>    - the module fails to set the TAINT_CRAP flag when initialized.
>
>This shows a pretty obvious discrepancy between intention and action.
>
>> Setting the fs register in userspace is an essential feature for
>running 
>> legacy code in SGX.  We have been following LKML discussions on this 
>> instruction for years, and hoping this feature would be supported by 
>> Linux, so that we can retire this module.
>
>The way to get things done in the kernel is to actively work on the
>problem. Hoping that someone else will fix that for you is naive at
>best. Wilful ignorance might be a less polite but nevertheless accurate
>term.
>
>> To our knowledge, every SGX library OS has a similar module, waiting
>> for this or a similar patch to be merged into Linux.  This indicates
>a
>> growing user base that needs this instruction.
>
>I'm failing to understand that a whole industry which is so confident
>about their ultimate solution to the security problem puts possible
>users and customers into the situation to decide between:
>
> 1) Secure host kernel (with known limitations)
>
> 2) SGX enclaves
>
>I would not mind if this would be a choice between fire and frying pan,
>but this is a choice between a well understood reality and a very
>dangerous illusion.
>
>> Nonetheless, Graphene is moving towards adoption in production
>systems, 
>> and we are actively working to make the code base secure and robust. 
>> This issue has been on our to-do list before a production release. 
>It 
>> would certainly make our lives easier to deprecate our module and
>just 
>> use a robust, in-kernel implementation.
>
>Would make your life easier?
>
>Having proper in kernel FSGSBASE support is the only solution to that
>problem and this has been true since the whole SGX frenzy started.
>Intel
>failed to upstream FSGSBASE since 2015 (sic!). See
>
>https://lore.kernel.org/lkml/alpine.DEB.2.21.1903261010380.1789@nanos.tec.linutronix.de/
>
>for a detailed time line. And that mail is more than a year old.
>
>Since then there happened even more trainwrecks including the revert of
>already queued patches a few days before the 5.3 merge window opened.
>
>After that we saw yet more broken variants of that patch set including
>the fail to provide information which is required to re-merge that.
>
>Instead of providing that information the next version re-introduced
>the
>wreckage which was carefully sorted out during earlier review cycles up
>to the revert.
>
>So you (and everybody else who has interrest in SGX) just sat there,
>watched and hoped that this will solve itself magically. And with that
>"hope" argument you really want to make me believe that all of this was
>against your intentions?
>
>It's beyond hillarious that the renewed attempt to get FSGSBASE support
>merged does not come from the company which has the main interest to
>get
>this solved, i.e Intel.
>
>Based on your argumentation that all of this is uninteded, I assume
>that
>the pull request on github which removes this security hole from
>graphene:
>
>        https://github.com/oscarlab/graphene/pull/1529
>
>is perfectly fine, right?
>
>Quite the contrary, it's completely usesless and at the same time
>perfectly fitting into this picture:
>
>  The changelog is SGX marketing compliant: Zero technical content. Not
>  a single word about the real implications of that blantant violation
>  of any principle of sane (security) engineering.
>
>Not that I'm surprised about this. That change originates from Intel
>and
>the poor sod who had to place the pull request - coincidentally a few
>days after this insanity became public - was not allowed to spell out
>the real reasons why this removal is necessary.
>
>Please read security relevant changelogs in the kernel git tree and
>then
>explain to me the utter void in this one.
>
>Looking at the advertising which all involved parties including the
>Confidential Computing Consortium are conducting, plus the fact that
>Intel has major investments in SGX supporting companies and projects,
>this is one of the worst marketing scams I've seen in decades.
>
>This all violates the fundamental engineering principle of "correctnes
>first" and I'm flabbergasted that academic research has degraded into
>the "features first" advocating domain.
>
>What's worse it that public funded research is failing to serve the
>public interest and instead is acting as an advertsiing machine for
>their
>corporate sponsors.
>
>Thanks,
>
>        Thomas
> 

On a related topic (needless to say, this should never have happened and is being raised at the highest levels inside Intel):

There are legitimate reasons to write a root-hole module, the main one being able to test security features like SMAP. I have requested before a TAINT flag specifically for this purpose, because TAINT_CRAP is nowhere near explicit enough, and is also used for staging drivers. Call it TAINT_TOXIC or TAINT_ROOTHOLE; it should always be accompanied with a CRIT level alert.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Powered by blists - more mailing lists