Date:   Fri, 15 May 2020 15:54:40 -0400
From:   Nathaniel McCallum <npmccallum@...hat.com>
To:     "Dr. Greg" <greg@...ellic.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
        x86@...nel.org, linux-sgx@...r.kernel.org,
        akpm@...ux-foundation.org, dave.hansen@...el.com,
        Neil Horman <nhorman@...hat.com>,
        "Huang, Haitao" <haitao.huang@...el.com>,
        andriy.shevchenko@...ux.intel.com,
        "Svahn, Kai" <kai.svahn@...el.com>, bp@...en8.de,
        Josh Triplett <josh@...htriplett.org>, luto@...nel.org,
        kai.huang@...el.com, David Rientjes <rientjes@...gle.com>,
        "Xing, Cedric" <cedric.xing@...el.com>,
        Patrick Uiterwijk <puiterwijk@...hat.com>
Subject: Re: [PATCH v29 00/20] Intel SGX foundations

On Thu, May 14, 2020 at 5:17 AM Dr. Greg <greg@...ellic.com> wrote:
>
> On Fri, May 08, 2020 at 12:56:35PM -0700, Sean Christopherson wrote:
>
> Good morning, I hope the week is proceeding well for everyone.
>
> > On Fri, May 08, 2020 at 02:02:26PM -0500, Dr. Greg wrote:
> > > On Thu, May 07, 2020 at 02:41:30AM +0200, Thomas Gleixner wrote:
> > > > The diffstat of your patch is irrelevant. What's relevant is the
> > > > fact that it is completely unreviewed and that it creates yet
> > > > another user space visible ABI with a noticable lack of
> > > > documentation.
> >
> > ...
> >
> > > As to lack of review, we would certainly welcome technical and API
> > > comments but we cannot force them.
>
> > Thomas' point isn't that your code needs to be reviewed, it's that
> > trying to squeeze it into the initial merge will, not might, _will_
> > push out the acceptance of SGX.  The same is true for other features
> > we have lined up, e.g. KVM and cgroup support.  It's not a slight on
> > your code, it's just reality at this point.
>
> For the record and for whatever it is worth at this point.  The patch
> has been available in its present form and function for around 14
> months.
>
> So there was plenty of time for its consideration and integration into
> what is being prepared as the final merge candidate.
>
> > > In fact, anyone who reviews the patch will see that the current driver
> > > creates a pointer in the ioctl call to pass downward into the hardware
> > > initialization routines.  Our code simply replaces that pointer with
> > > one supplied by userspace.
>
> > Personally, I'm in favor of adding a reserved placeholder for a
> > token so as to avoid adding a second ioctl() in the event that
> > something gets upstreamed that wants the token.  Ditto for a file
> > descriptor for the backing store in sgx_enclave_create.
>
> That would be a very low cost and forward looking addition.
>
> > > At the very least a modular form of the driver should be
> > > considered that would allow alternate implementations.  Sean
> > > indicated that there was a 'kludgy' approach that would allow an
> > > alternate modular implementation alongside the in-kernel driver.
> > > I believe that Andy has already indicated that may not be an
> > > advisable approach.
>
> > Modularizing the the driver does nothing for your use case.  The
> > "driver" is a relatively lightweight wrapper, removing that is akin
> > to making /dev/sgx/enclave inaccessible, i.e. it doesn't make the
> > EPC tracking and core code go away.  Modularizing the whole thing is
> > flat out not an option, as doing so is not compatible with an EPC
> > cgroup and adds unnecessary complexity to KVM enabling, both of
> > which are highly desired features by pretty much everyone that plans
> > on using SGX.
>
> All well taken technical points, but they don't speak directly to the
> issue at hand.  The potential security issue in all of this is access
> to /dev/sgx/enclave, not the EPC tracking and core code.
>
> Here in a nutshell is the paradox the kernel faces:
>
> No one seems to be disputing the fact that the primary focus of this
> driver will be to support the notion of 'runtime encryption' and
> Confidential Computing.  The whole premise of the concept and economic
> predicate of the initiative is that the owner/manager of the platform
> has no visibility into what is being done on the platform.
>
> If the Linux community believes that standard platform security
> controls can make qualitatively good judgements on what enclave based
> execution is doing, it is effectively making the statement that the
> very concept of runtime encryption and by extension Confidential
> Computing is invalid.
>
> If we accept the concept that Confidential Computing is valid then we
> admit the technology provides the opportunity for unsupervised code
> execution and data manipulation.
>
> Our premise in all of this is that one page of code implementing three
> linked lists is a small price to pay to provide platform owners with
> the opportunity to optionally implement what is effectively 2-factor
> authentication over this process.
>
> Going forward, if we are intellectually honest, all of this suggests
> that adding complexity to the driver with LSM controls makes little
> sense technically.  Since, by definition and economic intent, the
> technology provides tools and infrastructure that allows these
> controls to be evaded.
>
> The notion that entities with adversarial intent would not try and
> take advantage of this flies in the face of everything we know about
> security.
>
> > Andy is right to caution against playing games to squish in-kernel
> > things, but the virtualization snafu is a completely different
> > beast.  E.g. SGX doesn't require fiddling with CR4, won't corrupt
> > random memory across kexec(), doesn't block INIT, etc...  And I'm
> > not advocating long-term shenanigans, I just wanted to point out
> > that there are options for running out-of-tree drivers in the short
> > term, e.g. until proper policy controls land upstream.
>
> It appears that the distributions, at least IBM/RHEL, are going to
> compile this driver in and backport it as well.

The (Red Hat sponsored) Enarx project will continue building an
unofficial, unsupported version of the Fedora kernel with the SGX
patches[0] until such time as the patches are upstream. Once upstream,
I intend to propose that the feature be enabled in the stock Fedora
kernel.

Enarx requires EDMM support as a prerequisite to being production
ready. Therefore, we are likely to continue building this custom
Fedora kernel with the latest patches until such point as EDMM support
lands upstream. This also implies that I have no current plan to
request an SGX backport to a RHEL kernel until such time as it
supports our full needs.

Disclaimer: I do not control RHEL or Fedora kernel features. None of
the above constitutes a commitment to deliver anything.

[0]: https://copr.fedorainfracloud.org/coprs/npmccallum/enarx/package/kernel/

> What we would recommend at this point is that you and Jarkko do the
> Linux community and beyond a favor and wire up a simple kernel
> command-line parameter that controls the ability of the driver to be
> used, ie. enables/disables access to /dev/sgx/enclave.
>
> Distributions or others can set this command-line parameter by default
> to 'disable' and avoid any possibility of the technology being used
> for nefarious purposes.  Since the technology now appears to be
> focused only on the cloud providers they will certainly be capable of
> configuring their implementations to change the default.
>
> In essence, make the kernel's behavior secure by default.
>
> Best wishes for a pleasant weekend to everyone.
>
> Dr. Greg
>
> As always,
> Dr. Greg Wettstein, Ph.D, Worker      Artisans in autonomously
> Enjellic Systems Development, LLC     self-defensive IOT platforms
> 4206 N. 19th Ave.                     and edge devices.
> Fargo, ND  58102
> PH: 701-281-1686                      EMAIL: greg@...ellic.com
> ------------------------------------------------------------------------------
> "Intellectuals solve problems; geniuses prevent them."
>                                 -- Albert Einstein
>

Powered by blists - more mailing lists