> From: linux-sgx-owner@xxxxxxxxxxxxxxx [mailto:linux-sgx- > owner@xxxxxxxxxxxxxxx] On Behalf Of Stephen Smalley > Sent: Monday, June 03, 2019 10:47 AM > > On 6/2/19 3:29 AM, Xing, Cedric wrote: > > Hi Sean, > > > >> 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. > >> > >> 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}. > >> > >> 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. > >> > >> ALLOW_WRITE is used in conjuction with ALLOW_EXEC to enforce > SELinux's EXECMOD (or EXECMEM). > >> > >> This is very much an RFC series. It's only compile tested, likely > >> has obvious bugs, the SELinux patch could be completely harebrained, > etc... > >> 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... > >> > >> 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. > > > > 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. > > > > 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. > > Open: Given a file of type struct file *, how to tell if it is an > enclave (i.e. /dev/sgx/enclave)? > > * 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(). > > * In the newly introduced hook security_enclave_load(), set WRITTEN > for pages that are requested PROT_WRITE. > > * 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().
