On Thu, May 16, 2019 at 6:06 PM Xing, Cedric [off-list ref] wrote:

quoted

From: Andy Lutomirski [mailto:luto@kernel.org]

On Thu, May 16, 2019 at 3:23 PM Xing, Cedric [off-list ref]
wrote:

quoted

Hi Andy,

quoted

quoted

SIGSTRUCT isn't necessarily stored on disk so may not always have

a fd.

quoted

quoted

How about the following?

quoted

void *ss_pointer = mmap(sigstruct_fd, PROT_READ,...);
ioctl(enclave_fd, SGX_INIT_THE_ENCLAVE, ss_pointer);

The idea here is SIGSTRUCT will still be passed in memory so it
works

the same way when no LSM modules are loaded or basing its decision
on the .sigstruct file. Otherwise, an LSM module can figure out the
backing file (and offset within that file) by looking into the VMA
covering ss_pointer.

I don’t love this approach.  Application authors seem likely to use
read() instead of mmap(), and it’ll still work in many cares. It
would also complicate the kernel implementation, and looking at the
inode backing the vma that backs a pointer is at least rather

unusual.

quoted

quoted

Instead, if the sigstruct isn’t on disk because it’s dynamic or came
from a network, the application can put it in a memfd.

I understand your concern here. But I guess we are making too much

assumption on how enclaves are structured/packaged. My concern is, what
if a SIGSTRUCT really has to be from memory? For example, an enclave
(along with its SIGSTRUCT) could be embedded inside a shared object (or
even the "main" executable) so it shows up in memory to begin with.

Hmm.  That's a fair point, although opening /proc/self/exe could be
somewhat of a workaround.  It does suffer from a bit of an in-band
signaling problem, though, in that it's possible that some other random
bytes in the library resemble a SIGSTRUCT.

quoted

I was not saying enclaves were exempt to good security practices. What

I was trying to say was, EPC pages are *not* subject to the same attacks
as regular pages so I suspect there will be a desire to enforce
different policies on them, especially after new SGX2
features/applications become available. So I think it beneficial to
distinguish between regular vs. enclave virtual ranges. And to do that,
a new VM_SGX flag in VMA is probably a very simple/easy way. And with
that VM_SGX flag, we could add a new security_sgx_mprot() hook so that
LSM modules/policies could act differently.

I'm not opposed to this, but I also don't think this needs to be in the
initial upstream driver.  VM_SGX also isn't strictly necessary -- an LSM
could inspect the VMA to decide whether it's an SGX VMA if it really
wanted to.

VM_SGX is just what I think is the easiest way for any module to tell enclave VMAs from all others. I agree totally with you that doesn't have to be in the initial release!

quoted

That being said, do you have any specific behavior differences in mind
aside from the oddities involved in loading the enclave.

The major thing is dynamically linked enclaves. Say if you want something like dlopen() inside an enclave, the driver would need to EAUG a page as RW initially, and then change to RX after it has been EACCEPTCOPY'ed by the enclave. So it's like a RW->RX transition and an LSM module/policy may want to allow it only if it's within an enclave range (ELRANGE), or deny it otherwise.

I'm not convinced.  Given that the kernel has no way to tell that the
dynamically loaded code wasn't dynamically generated, I don't think it
makes sense to allow this in an enclave but disallow it outside an
enclave.

quoted

quoted

And if you are with me on that bigger picture, the next question is:

what should be the default behavior of security_sgx_mprot() for
existing/non-SGX-aware LSM modules/policies? I'd say a reasonable
default is to allow R, RW and RX, but not anything else. It'd suffice to
get rid of EXECMEM/EXECMOD requirements on enclave applications. For
SGX1, EPCM permissions are immutable so it really doesn't matter what
security_sgx_mprot() does. For SGX2 and beyond, there's still time and
new SGX-aware LSM modules/policies will probably have emerged by then.

I hadn't thought about the SGX1 vs SGX2 difference.  If the driver
initially only wants to support SGX1, then I guess we really could get
away with constraining the EPC flags based on the source page permission
and not restricting mprotect() and mmap() permissions on
/dev/sgx/enclave at all.

This is exactly what I'm going after!

But I have to apologize for this silly question because I don't know much about SELinux: Wouldn't it require changes to existing SELinux policies to *not* restrict mprotect() on /dev/sgx/enclave?

I'm assuming we'd make a small in-kernel change to SELinux to make it
work without policy changes, assuming the SELinux maintainers would be
okay with this.