On Thu, May 16, 2019 at 6:06 PM Xing, Cedric <cedric.x...@intel.com> wrote: > > > From: Andy Lutomirski [mailto:l...@kernel.org] > > > > On Thu, May 16, 2019 at 3:23 PM Xing, Cedric <cedric.x...@intel.com> > > wrote: > > > > > > Hi Andy, > > > > > > > > SIGSTRUCT isn't necessarily stored on disk so may not always have > > a fd. > > > > How about the following? > > > > > 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. > > > > 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. > > > > > 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! > > > > > 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. > > > > > > > > > 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.
