Re: [PATCH v20 00/28] Intel SGX1 support

"Haitao Huang" <haitao.huang@xxxxxxxxxxxxxxx> · Tue, 14 May 2019 16:08:52 -0500

On Tue, 14 May 2019 15:45:54 -0500, Andy Lutomirski <luto@xxxxxxxxxx>  
wrote:

On May 14, 2019, at 8:30 AM, Haitao Huang  
<haitao.huang@xxxxxxxxxxxxxxx> wrote:

On Tue, 14 May 2019 10:17:29 -0500, Andy Lutomirski <luto@xxxxxxxxxx>  
wrote:

On Tue, May 14, 2019 at 7:33 AM Haitao Huang
<haitao.huang@xxxxxxxxxxxxxxx> wrote:

On Fri, 10 May 2019 14:22:34 -0500, Andy Lutomirski <luto@xxxxxxxxxx>
wrote:

> On Fri, May 10, 2019 at 12:04 PM Jethro Beekman  
<jethro@xxxxxxxxxxxx>
> wrote:
>>
>> On 2019-05-10 11:56, Xing, Cedric wrote:
>> > Hi Jethro,
>> >
>> >> ELF files are explicitly designed such that you can map them  
(with
>> mmap)
>> >> in 4096-byte chunks. However, sometimes there's overlap and you  
will
>> >> sometimes see that a particular offset is mapped twice because  
the
>> first
>> >> half of the page in the file belongs to an RX range and the  
second
>> half
>> >> to an R-only range. Also, ELF files don't (normally) describe  
stack,
>> >> heap, etc. which you do need for enclaves.
>> >
>> > You have probably misread my email. By mmap(), I meant the  
enclave
>> file would be mapped via *multiple* mmap() calls, in the same way  
as
>> what dlopen() would do in loading regular shared object. The  
intention
>> here is to make the enclave file subject to the same checks as  
regular
>> shared objects.
>>
>> No, I didn't misread your email. My original point still stands:
>> requiring that an enclave's memory is created from one or more mmap
>> calls of a file puts significant restrictions on the enclave's  
on-disk
>> representation.
>>
>
> For a tiny bit of background, Linux (AFAIK*) makes no effort to  
ensure
> the complete integrity of DSOs.  What Linux *does* do (if so
> configured) is to make sure that only approved data is mapped
> executable.  So, if you want to have some bytes be executable, those
> bytes have to come from a file that passes the relevant LSM and IMA
> checks.

Given this, I just want to step back a little to understand the exact
issue that SGX is causing here for LSM/IMA. Sorry if I missed points
discussed earlier.

By the time of EADD, enclave file is opened and should have passed  
IMA and
SELinux policy enforcement gates if any. We really don't need extra  
mmaps
on the enclave files to be IMA and SELinux compliant.

The problem, as i see it, is that they passed the *wrong* checks,
because, as you noticed:

We are loading
enclave files as RO and copying those into EPC.

Which is, semantically, a lot like loading a normal file as RO and
then mprotecting() it to RX, which is disallowed under quite a few LSM
policies.

An IMA policy can enforce
RO files (or any file). And SELinux policy can say which processes can
open the file for what permissions. No extra needed here.

If SELinux says a process may open a file as RO, that does *not* mean
that it can be opened as RX.

But in this case, file itself is mapped as RO treated like data and it  
is not for execution. SGX enclave pages have EPCM enforced permissions.  
So from SELinux point of view I would think it can treat it as RO and  
that's fine.

As an example, SELinux has an “execute” permission (via
security_mmap_file — see file_map_prot_check()) that controls whether
you can execute code from that file.  If you lack this permission on a
file, you may still be able to map it PROT_READ, but you may not map
it PROT_EXEC.  Similarly, if you want to malloc() some memory, write
*code* to it, and execute it, you need a specific permission.

So, unless we somehow think it’s okay for SGX to break the existing
model, we need to respect these restrictions in the SGX driver. In
other words, we either need to respect execmem, etc or require
PROT_EXEC or the equivalent. I like the latter a lot more.

What puzzles me is that this restriction does not add real value to  
security.
When enclave files are mapped with PROT_READ, without SE execute  
permission. No breakage to LSM model in normal process address space as no  
one can execute code directly from the file in normal memory. When enclave  
is built and loaded into EPC by EADDs, if the SIGSTRUCT is trusted (either  
signer or MRENCLAVE), EINIT will guarantee security (both integrity and  
permissions). LSM may not like the fact the a piece of code got loaded  
into EPC page without specifically giving SE execute permission. However,  
LSM can be used to control what SIGSTRUCTs can be trusted as you suggested  
and indirectly enforce what code got executed inside EPC.

So to me, only SIGSTRUCT verification would add value.