On Fri, May 24, 2019 at 01:42:13PM -0700, Xing, Cedric wrote:

quoted

From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
owner@vger.kernel.org] On Behalf Of Sean Christopherson
Sent: Friday, May 24, 2019 12:14 PM

My point is that enclaves have different properties than shared objects.

Normal LSM behavior with regard to executing files is to label files
with e.g. FILE__EXECUTE.  Because an enclave must be built to the exact
specifications of .sigstruct, requring FILE__EXECUTE on the .sigstruct
is effectively the same as requiring FILE__EXECUTE on the enclave itself.

Addressing your scenario of loading an executable page in EPC, doing so
would require one of the following:

  - Ability to install a .sigstruct with FILE__EXECUTE

  - PROCESS__EXECMEM

  - FILE__EXECMOD and SGX2 support

Now I got your point. It sounds a great idea to me!

But instead of using .sigstruct file, I'd still recommend using file mapping
(i.e. SIGSTRUCT needs to reside in executable memory). But then there'll be a

Why?  Even in the Graphene case the final .sigstruct can be known ahead of
time.  Userspace can always use memfd() if it's generating SIGSTRUCT on
the fly.

hole - a process having FILE__EXECMOD on any file could use that file as a
SIGSTRUCT. Probably we'll need a new type in SELinux to label
enclave/sigstruct files.

quoted

I don't see a fundamental difference between having RWX in an enclave
and RWX in normal memory, either way the process can execute arbitrary
code, i.e. PROCESS__EXECMEM is appropriate.  Yes, an enclave will #UD on
certain instructions, but that's easily sidestepped by having a
trampoline in the host (marked RX) and piping arbitrary code into the
enclave.  Or using EEXIT to do a bit of ROP.

I'm with you.

With your proposal only FILE__EXECMOD is needed on /dev/sgx/enclave to launch
Graphene enclaves or the like.

It wouldn't even need FILE__EXECMOD, assuming Graphene does all of its
libc rewriting before building the enclave, i.e. doesn't EADD RWX pages.

quoted

quoted

quoted

quoted

No changes are required to LSMs, SGX1 has a single LSM touchpoint
in

its

quoted

mmap(), and I *think* the only required userspace change is to
mmap() PROT_NONE when allocating the enclave's virtual address

range.

quoted

I'm not sure I understand the motivation behind this proposal to
decouple initial EPC permissions from source pages.

Pulling permissions from source pages means userspace needs to fully map
the in normal memory, including marking pages executable.  That exposes
the loader to having executable pages in its address space that it has
no intention of executing (outside of the enclave).  And for Graphene,
it means having to actively avoid PROCESS__EXECMEM, e.g. by using a
dummy backing file to build the enclave instead of anon memory.

Agreed.

quoted

quoted

I don't think it a big deal to fully mmap() enclave files, which have
to be parsed by user mode anyway to determine various things including
but not limited to the size of heap(s), size and number of
TCSs/stacks/TLS areas, and the overall enclave size. So with PHDRs
parsed, it's trivial to mmap() each segment with permissions from its

PHDR.

quoted

quoted

quoted

As for Graphene, it doesn't need extra permissions to run its
enclaves, it just needs a way to install .sigstruct, which is a
generic permissions problem and not SGX specific.


For SGX2 maybe:

  - No additional requirements to map an EAUG'd page as RW page.

Not

quoted

quoted

quoted

    aligned with standard MAP_SHARED behavior, but we really don't

want

quoted

quoted

quoted

    to require FILE__WRITE, and thus allow writes to .sigstruct.

  - Require FILE__EXECMOD on the .sigstruct to map previously

writable

quoted

quoted

quoted

    page as executable (which indirectly includes all EAUG'd

pages).

quoted

quoted

quoted

    Wiring this up will be a little funky, but we again we don't

want

quoted

quoted

quoted

    to require FILE__WRITE on .sigstruct.

I'm lost. Why is EAUG tied to permissions on .sigstruct?

Because for the purposes of LSM checks, .sigstruct is the enclave's
backing file, and mapping a previously writable enclave page as
exectuable is roughly equivalent to mapping a CoW'd page as exectuable.

I think I've got your idea. You are trying to use permissions on .sigstruct
to determine whether EAUG will be available to that specific enclave. Am I
right?

Yep.

I'd tie EAUG to the permissions of /dev/sgx/enclave instead. But why? There
are couple of reasons. For one, a SIGSTRUCT identifies the behavior of the
enclave, hence the SGX features needed by that enclave. So if an enclave
requires EAUG, the .sigstruct has to allow EAUG or the enclave wouldn't work.
That means the system admin wouldn't have a choice but to match up what's
needed by the enclave. For two, whether to allow, say loading code
dynamically into an enclave, depends on whether the host process can tolerate
the inherent risk. And that decision is seldom made on individual enclaves
but to the host process as a whole. And /dev/sgx/enclave serves that purpose.

I think I'd be ok either way?  What I really care about is having line of
sight to a sane way to support for SGX2, and both seem sane.  I.e. we can
hash this detail out when SGX2 goes in.