lkml.org 
[lkml]   [2019]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: SGX vs LSM (Re: [PATCH v20 00/28] Intel SGX1 support)
From
Date
On 5/17/19 4:14 PM, Andy Lutomirski wrote:
>
>> On May 17, 2019, at 1:09 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>
>>> On 5/17/19 3:28 PM, Sean Christopherson wrote:
>>>> On Fri, May 17, 2019 at 02:05:39PM -0400, Stephen Smalley wrote:
>>>>> On 5/17/19 1:12 PM, Andy Lutomirski wrote:
>>>>>
>>>>> How can that work? Unless the API changes fairly radically, users
>>>>> fundamentally need to both write and execute the enclave. Some of it will
>>>>> be written only from already executable pages, and some privilege should be
>>>>> needed to execute any enclave page that was not loaded like this.
>>>>
>>>> I'm not sure what the API is. Let's say they do something like this:
>>>>
>>>> fd = open("/dev/sgx/enclave", O_RDONLY);
>>>> addr = mmap(NULL, size, PROT_READ | PROT_EXEC, MAP_SHARED, fd, 0);
>>>> stuff addr into ioctl args
>>>> ioctl(fd, ENCLAVE_CREATE, &ioctlargs);
>>>> ioctl(fd, ENCLAVE_ADD_PAGE, &ioctlargs);
>>>> ioctl(fd, ENCLAVE_INIT, &ioctlargs);
>>> That's rougly the flow, except that that all enclaves need to have RW and
>>> X EPC pages.
>>>> The important points are that they do not open /dev/sgx/enclave with write
>>>> access (otherwise they will trigger FILE__WRITE at open time, and later
>>>> encounter FILE__EXECUTE as well during mmap, thereby requiring both to be
>>>> allowed to /dev/sgx/enclave), and that they do not request PROT_WRITE to the
>>>> resulting mapping (otherwise they will trigger FILE__WRITE at mmap time).
>>>> Then only FILE__READ and FILE__EXECUTE are required to /dev/sgx/enclave in
>>>> policy.
>>>>
>>>> If they switch to an anon inode, then any mmap PROT_EXEC of the opened file
>>>> will trigger an EXECMEM check, at least as currently implemented, as we have
>>>> no useful backing inode information.
>>> Yep, and that's by design in the overall proposal. The trick is that
>>> ENCLAVE_ADD takes a source VMA and copies the contents *and* the
>>> permissions from the source VMA. The source VMA points at regular memory
>>> that was mapped and populated using existing mechanisms for loading DSOs.
>>> E.g. at a high level:
>>> source_fd = open("/home/sean/path/to/my/enclave", O_RDONLY);
>>> for_each_chunk {
>>> <hand waving - mmap()/mprotect() the enclave file into regular memory>
>>> }
>>> enclave_fd = open("/dev/sgx/enclave", O_RDWR); /* allocs anon inode */
>>> enclave_addr = mmap(NULL, size, PROT_READ, MAP_SHARED, enclave_fd, 0);
>>> ioctl(enclave_fd, ENCLAVE_CREATE, {enclave_addr});
>>> for_each_chunk {
>>> struct sgx_enclave_add ioctlargs = {
>>> .offset = chunk.offset,
>>> .source = chunk.addr,
>>> .size = chunk.size,
>>> .type = chunk.type, /* SGX specific metadata */
>>> }
>>> ioctl(fd, ENCLAVE_ADD, &ioctlargs); /* modifies enclave's VMAs */
>>> }
>>> ioctl(fd, ENCLAVE_INIT, ...);
>>> Userspace never explicitly requests PROT_EXEC on enclave_fd, but SGX also
>>> ensures userspace isn't bypassing LSM policies by virtue of copying the
>>> permissions for EPC VMAs from regular VMAs that have already gone through
>>> LSM checks.
>>
>> Is O_RDWR required for /dev/sgx/enclave or would O_RDONLY suffice? Do you do anything other than ioctl() calls on it?
>>
>> What's the advantage of allocating an anon inode in the above? At present anon inodes are exempted from inode-based checking, thereby losing the ability to perform SELinux ioctl whitelisting, unlike the file-backed /dev/sgx/enclave inode.
>>
>> How would SELinux (or other security modules) restrict the authorized enclaves that can be loaded via this interface? Would the sgx driver invoke a new LSM hook with the regular/source VMAs as parameters and allow the security module to reject the ENCLAVE_ADD operation? That could be just based on the vm_file (e.g. whitelist what enclave files are permitted in general) or it could be based on both the process and the vm_file (e.g. only allow specific enclaves to be loaded into specific processes).
>
> This is the idea behind the .sigstruct file. The driver could call a new hook to approve or reject the .sigstruct. The sigstruct contains a hash of the whole enclave and a signature by the author.

Ok, so same idea but moved to ENCLAVE_INIT and passing the vma or file
for the sigstruct instead of the enclave.

\
 
 \ /
  Last update: 2019-05-17 22:48    [W:0.162 / U:5.848 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site