All of lore.kernel.org
 help / color / mirror / Atom feed
From: Don Porter <porter@cs.unc.edu>
To: Thomas Gleixner <tglx@linutronix.de>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>, Sasha Levin <sashal@kernel.org>,
	linux-kernel@vger.kernel.org, bp@alien8.de, luto@kernel.org,
	hpa@zytor.com, dave.hansen@intel.com, tony.luck@intel.com,
	ravi.v.shankar@intel.com, chang.seok.bae@intel.com
Subject: Re: [PATCH v12 00/18] Enable FSGSBASE instructions
Date: Tue, 26 May 2020 08:42:09 -0400	[thread overview]
Message-ID: <c5fffcd1-c262-7046-a047-67de2bbccd78@cs.unc.edu> (raw)
In-Reply-To: <87h7w7qy18.fsf@nanos.tec.linutronix.de>

Hi Thomas,

On 5/22/20 8:45 PM, Thomas Gleixner wrote:
> Don,
> 
> Don Porter <porter@cs.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.

I think there is a significant misunderstanding here.  This line of 
research assumes the kernel is already compromised and behaving 
adversarially toward a more trusted application.  Thus, the attack 
surface under scrutiny in these projects is between the enclave and the 
rest of the system.  Not that we want kernels to be rooted, or make this 
easier, but exploits happen in practice.

The threat model for Graphene, and most SGX papers, is quite explicit: 
we assume that Intel’s CPU package, the software in the enclave, and 
possibly Intel’s Attestation Service (IAS) are the only trusted 
components.  Any other software should be assumed compromised, and one 
can even assume memory is physically tampered or that one has plugged in 
an adversarial device. It is not a question of the limitations of the 
kernel, the threat model assumes that the kernel is already rooted.

For the community these papers are typically written to, this assumption 
would be well understood.  And thus it is common to see code artifacts 
that might emulate or even undermine security of untrusted components. 
Not appropriate for production use, but for the typical audience, this 
risk would be understood.  And, initially, when people started using 
Graphene, I checked who they were - almost exclusively SGX researchers 
who would have this context.  It has only been recently that the 
interest has grown to a level that these sorts of warnings need to be 
revised for a more general audience.  But the point that we should 
revise our readme and warnings for a more general audience is well taken.

> 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.

Yes!  I think we are in agreement that we expected Intel to upstream 
this support - it is their product. I don’t see why I am personally 
responsible to come to the aid of a multi-billion dollar corporation in 
my free time, or that it is wrong to at least let them try first and see 
how far they get.

Until recently, we were doing proof-of-concept research, not product 
development, and there are limited hours in the day.  I also hasten to 
say that the product of research is an article, the software artifact 
serves as documentation of the experiment.  In contrast, the product of 
software development is software.  It takes significant time and effort 
to convert one to the other.  Upstreaming code is of little scientific 
interest.  But things have changed for our project; we had no users in 
2015 and we are now un-cutting corners that are appropriate for research 
but inappropriate for production.  For a research artifact with an 
audience that knew the risks, we shipped a module because it was easier 
to maintain and install than a kernel patch.

Also, there is a chicken-and-egg problem here: AFAIU a kernel patch 
needs a userspace demonstration to motivate merging.  We can’t do a 
userspace demonstration without this feature.  My main interest in 
showing up for this discussion was to try to make the case that, 
compared to 2015, there is a more convincing userspace demonstration and 
larger population of interested users.

> 
> 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?

As far as the patch and pull request, I personally think the right thing 
to do is add the warnings you suggest, help test this or another kernel 
patch, and advise users that patching their kernel is more secure than 
this module.  I am not in favor of fully deleting the module, in the 
interest of transparency and reproducibility.

> 
> 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.

Finally, I must rebut the claim that my research abuses public funds to 
advertise for Intel.  I have been working on this problem since before I 
knew SGX existed, and have been completely transparent regarding 
subsequent collaborations with Intel.  I believe that understanding the 
pros and cons of different techniques to harden an application against a 
compromised kernel is in the public interest, and my research projects 
have been reviewed and overseen according to standard practices at both 
the university and US government funding agencies.  The expectations of 
agencies in the US funding research are the paper, the insights, and 
proof-of-concept software; converting proof-of-concept software into 
production quality is generally considered a “nice to have”.

- Don


  parent reply	other threads:[~2020-05-26 12:42 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-11  4:52 [PATCH v12 00/18] Enable FSGSBASE instructions Sasha Levin
2020-05-11  4:52 ` [PATCH v12 01/18] x86/ptrace: Prevent ptrace from clearing the FS/GS selector Sasha Levin
2020-05-11  4:52 ` [PATCH v12 02/18] selftests/x86/fsgsbase: Test GS selector on ptracer-induced GS base write Sasha Levin
2020-05-11  4:52 ` [PATCH v12 03/18] x86/cpu: Add 'unsafe_fsgsbase' to enable CR4.FSGSBASE Sasha Levin
2020-05-11  4:52 ` [PATCH v12 04/18] x86/entry/64: Clean up paranoid exit Sasha Levin
2020-05-11  4:52 ` [PATCH v12 05/18] x86/entry/64: Switch CR3 before SWAPGS in paranoid entry Sasha Levin
2020-05-11  4:52 ` [PATCH v12 06/18] x86/entry/64: Introduce the FIND_PERCPU_BASE macro Sasha Levin
2020-05-11  4:53 ` [PATCH v12 07/18] x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit Sasha Levin
2020-05-11  4:53 ` [PATCH v12 08/18] x86/entry/64: Document GSBASE handling in the paranoid path Sasha Levin
2020-05-11  4:53 ` [PATCH v12 09/18] x86/fsgsbase/64: Add intrinsics for FSGSBASE instructions Sasha Levin
2020-05-11  4:53 ` [PATCH v12 10/18] x86/fsgsbase/64: Enable FSGSBASE instructions in helper functions Sasha Levin
2020-05-18 18:20   ` Thomas Gleixner
2020-05-18 20:24     ` Sasha Levin
2020-05-18 22:59       ` Thomas Gleixner
2020-05-19 12:20       ` David Laight
2020-05-19 14:48         ` Thomas Gleixner
2020-05-20  9:13           ` David Laight
2020-05-11  4:53 ` [PATCH v12 11/18] x86/fsgsbase/64: Use FSGSBASE in switch_to() if available Sasha Levin
2020-05-11  4:53 ` [PATCH v12 12/18] x86/fsgsbase/64: move save_fsgs to header file Sasha Levin
2020-05-11  4:53 ` [PATCH v12 13/18] x86/fsgsbase/64: Use FSGSBASE instructions on thread copy and ptrace Sasha Levin
2020-05-11  4:53 ` [PATCH v12 14/18] x86/speculation/swapgs: Check FSGSBASE in enabling SWAPGS mitigation Sasha Levin
2020-05-11  4:53 ` [PATCH v12 15/18] selftests/x86/fsgsbase: Test ptracer-induced GS base write with FSGSBASE Sasha Levin
2020-05-11  4:53 ` [PATCH v12 16/18] x86/fsgsbase/64: Enable FSGSBASE on 64bit by default and add a chicken bit Sasha Levin
2020-05-11  4:53 ` [PATCH v12 17/18] x86/elf: Enumerate kernel FSGSBASE capability in AT_HWCAP2 Sasha Levin
2020-05-11  4:53 ` [PATCH v12 18/18] Documentation/x86/64: Add documentation for GS/FS addressing mode Sasha Levin
2020-05-15  9:24 ` [PATCH v12 00/18] Enable FSGSBASE instructions Jarkko Sakkinen
2020-05-15 16:40   ` Sasha Levin
2020-05-15 17:55     ` Andi Kleen
2020-05-15 23:07       ` Sasha Levin
2020-05-16 12:21       ` Jarkko Sakkinen
2020-05-16  9:50     ` Jarkko Sakkinen
2020-05-18 15:34       ` Andi Kleen
2020-05-18 20:01         ` Jarkko Sakkinen
2020-05-18 23:03           ` Thomas Gleixner
2020-05-19 16:48             ` Jarkko Sakkinen
2020-05-22 20:14               ` Don Porter
2020-05-22 20:55                 ` Dave Hansen
2020-05-23  0:45                 ` Thomas Gleixner
2020-05-24 19:45                   ` hpa
2020-05-24 21:19                     ` Sasha Levin
2020-05-24 23:44                       ` hpa
2020-05-25  7:54                       ` Richard Weinberger
2020-05-25 21:56                         ` Tony Luck
2020-05-26  8:12                         ` David Laight
2020-05-26  8:23                           ` Richard Weinberger
2020-05-27  8:31                     ` Jarkko Sakkinen
2020-05-26 12:42                   ` Don Porter [this message]
2020-05-26 20:27                     ` Sasha Levin
2020-05-26 22:03                       ` Don Porter
2020-05-26 22:51                         ` Sasha Levin
2020-05-28 17:37                           ` Don Porter
2020-05-28 10:29                     ` Thomas Gleixner
2020-05-28 17:40                       ` Don Porter
2020-05-28 18:38                         ` Andy Lutomirski
2020-05-29 15:27                           ` Wojtek Porczyk
2020-06-25 15:27                             ` Don Porter
2020-06-25 21:37                               ` Jarkko Sakkinen
2020-07-18 18:19                                 ` Don Porter
2020-07-23  3:23                                   ` Jarkko Sakkinen
2020-05-28 19:19                         ` Jarkko Sakkinen
2020-05-28 19:41                           ` Sasha Levin
2020-05-29  3:07                             ` Jarkko Sakkinen
2020-05-29  3:10                               ` Jarkko Sakkinen
2020-06-25 15:30                                 ` Don Porter
2020-06-25 21:40                                   ` Jarkko Sakkinen
2020-05-23  4:19                 ` Andi Kleen
2020-05-28 10:36                   ` Thomas Gleixner
2020-05-27  8:20                 ` Jarkko Sakkinen
2020-05-27 12:42                   ` Wojtek Porczyk
2020-05-18  9:51     ` Thomas Gleixner
2020-05-18 15:16       ` Sasha Levin
2020-05-18 18:28         ` Thomas Gleixner
2020-05-18 19:36       ` Jarkko Sakkinen
2020-05-18  6:18 ` Christoph Hellwig
2020-05-18 12:33   ` Sasha Levin
2020-05-18 14:53 ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c5fffcd1-c262-7046-a047-67de2bbccd78@cs.unc.edu \
    --to=porter@cs.unc.edu \
    --cc=ak@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=chang.seok.bae@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=sashal@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.