Thread (77 messages) 77 messages, 8 authors, 2019-06-14

RE: [RFC PATCH 0/9] security: x86/sgx: SGX vs. LSM

From: Xing, Cedric <hidden>
Date: 2019-06-03 18:03:04
Also in: lkml, selinux

From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
owner@vger.kernel.org] On Behalf Of Stephen Smalley
Sent: Monday, June 03, 2019 10:47 AM

On 6/2/19 3:29 AM, Xing, Cedric wrote:
quoted
Hi Sean,
quoted
From: Christopherson, Sean J
Sent: Friday, May 31, 2019 4:32 PM

This series is the result of a rather absurd amount of discussion
over how to get SGX to play nice with LSM policies, without having to
resort to evil shenanigans or put undue burden on userspace.  The
discussion definitely wandered into completely insane territory at
times, but I think/hope we ended up with something reasonable.
quoted
quoted
The basic gist of the approach is to require userspace to declare
what protections are maximally allowed for any given page, e.g. add a
flags field for loading enclave pages that takes
ALLOW_{READ,WRITE,EXEC}.  LSMs can then adjust the allowed
protections, e.g. clear ALLOW_EXEC to prevent ever mapping the page
with PROT_EXEC.  SGX enforces the allowed perms via a new mprotect()
vm_ops hook, e.g. like regular mprotect() uses MAY_{READ,WRITE,EXEC}.
quoted
quoted
ALLOW_EXEC is used to deny hings like loading an enclave from a
noexec file system or from a file without EXECUTE permissions, e.g.
without the ALLOW_EXEC concept, on SGX2 hardware (regardless of
kernel support) userspace could EADD from a noexec file using read-
only permissions, and later use mprotect() and ENCLU[EMODPE] to gain
execute permissions.
quoted
quoted
ALLOW_WRITE is used in conjuction with ALLOW_EXEC to enforce
SELinux's EXECMOD (or EXECMEM).
quoted
quoted
This is very much an RFC series.  It's only compile tested, likely
has obvious bugs, the SELinux patch could be completely harebrained,
etc...
quoted
quoted
My goal at this point is to get feedback at a macro level, e.g. is
the core concept viable/acceptable, are there objection to hooking
mprotect(), etc...
quoted
quoted
Andy and Cedric, hopefully this aligns with your general expectations
based on our last discussion.
I couldn't understand the real intentions of ALLOW_* flags until I saw
them in code. I have to say C is more expressive than English in that
regard :)

Generally I agree with your direction but think ALLOW_* flags are
completely internal to LSM because they can be both produced and
consumed inside an LSM module. So spilling them into SGX driver and also
user mode code makes the solution ugly and in some cases impractical
because not every enclave host process has a priori knowledge on whether
or not an enclave page would be EMODPE'd at runtime.
quoted
Theoretically speaking, what you really need is a per page flag (let's
name it WRITTEN?) indicating whether a page has ever been written to (or
more precisely, granted PROT_WRITE), which will be used to decide
whether to grant PROT_EXEC when requested in future. Given the fact that
all mprotect() goes through LSM and mmap() is limited to PROT_NONE, it's
easy for LSM to capture that flag by itself instead of asking user mode
code to provide it.
quoted
That said, here is the summary of what I think is a better approach.
* In hook security_file_alloc(), if @file is an enclave, allocate some
data structure to store for every page, the WRITTEN flag as described
above. WRITTEN is cleared initially for all pages.
quoted
   Open: Given a file of type struct file *, how to tell if it is an
enclave (i.e. /dev/sgx/enclave)?
quoted
* In hook security_mmap_file(), if @file is an enclave, make sure
@prot can only be PROT_NONE. This is to force all protection changes to
go through security_file_mprotect().
quoted
* In the newly introduced hook security_enclave_load(), set WRITTEN
for pages that are requested PROT_WRITE.
quoted
* In hook security_file_mprotect(), if @vma->vm_file is an enclave,
look up and use WRITTEN flags for all pages within @vma, along with
other global flags (e.g. PROCESS__EXECMEM/FILE__EXECMOD in the case of
SELinux) to decide on
allowing/rejecting @prot.

At this point we have no knowledge of the source vma/file, right?  So
what do we check FILE__EXECUTE and/or FILE__EXECMOD against?
vma->vm_file at this point is /dev/sgx/enclave, right?
My apology to the confusions here.

Yes, vma->vm_file is always /dev/sgx/enclave, but each open("/dev/sgx/enclave") returns a *new* file struct (let's denote it as @enclave_fd) that uniquely identifies one enclave instance, and the expectation is that @enclave_fd->f_security would be used by LSM to store enclave specific information, including ALLOW_* flags and whatever deemed appropriate by an LSM module.

In the case of SELinux, and if the choice is to use FILE__EXECMOD of .sigstruct file to authorize RW->RX at runtime, then SELinux could cache that flag in @enclave_fd->f_security upon security_enclave_init().
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help