Re: [PATCH 05/25] x86/sgx: Introduce runtime protection bits

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Jarkko,

On 1/20/2022 4:53 AM, Jarkko Sakkinen wrote:
> On Tue, 2022-01-18 at 12:59 -0800, Reinette Chatre wrote:
>> Hi Jarkko,
>>
>> On 1/17/2022 6:22 PM, Jarkko Sakkinen wrote:
>>> On Tue, Jan 18, 2022 at 03:59:29AM +0200, Jarkko Sakkinen wrote:
>>>> On Mon, Jan 17, 2022 at 08:13:32AM -0500, Nathaniel McCallum
>>>> wrote:
>>>>> On Sat, Jan 15, 2022 at 6:57 AM Jarkko Sakkinen
>>>>> <jarkko@kernel.org> wrote:
>>>>>>
>>>>>> On Sat, Jan 15, 2022 at 03:18:04AM +0200, Jarkko Sakkinen
>>>>>> wrote:
>>>>>>> On Fri, Jan 14, 2022 at 04:41:59PM -0800, Reinette Chatre
>>>>>>> wrote:
>>>>>>>> Hi Jarkko,
>>>>>>>>
>>>>>>>> On 1/14/2022 4:27 PM, Jarkko Sakkinen wrote:
>>>>>>>>> On Fri, Jan 14, 2022 at 04:01:33PM -0800, Reinette
>>>>>>>>> Chatre wrote:
>>>>>>>>>> Hi Jarkko,
>>>>>>>>>>
>>>>>>>>>> On 1/14/2022 3:15 PM, Jarkko Sakkinen wrote:
>>>>>>>>>>> On Fri, Jan 14, 2022 at 03:05:21PM -0800, Reinette
>>>>>>>>>>> Chatre wrote:
>>>>>>>>>>>> Hi Jarkko,
>>>>>>>>>>>
>>>>>>>>>>> How enclave can check a page range that EPCM has
>>>>>>>>>>> the expected permissions?
>>>>>>>>>>
>>>>>>>>>> Only way to change EPCM permissions from outside
>>>>>>>>>> enclave is to run ENCLS[EMODPR]
>>>>>>>>>> that needs to be accepted from within the enclave via
>>>>>>>>>> ENCLU[EACCEPT]. At that
>>>>>>>>>> time the enclave provides the expected permissions
>>>>>>>>>> and that will fail
>>>>>>>>>> if there is a mismatch with the EPCM permissions
>>>>>>>>>> (SGX_PAGE_ATTRIBUTES_MISMATCH).
>>>>>>>>>
>>>>>>>>> This is a very valid point but that does make the
>>>>>>>>> introspection possible
>>>>>>>>> only at the time of EACCEPT.
>>>>>>>>>
>>>>>>>>> It does not give tools for enclave to make sure that
>>>>>>>>> EMODPR-ETRACK dance
>>>>>>>>> was ever exercised.
>>>>>>>>
>>>>>>>> Could you please elaborate? EACCEPT is available to the
>>>>>>>> enclave as a tool
>>>>>>>> and it would fail if ETRACK was not completed (error
>>>>>>>> SGX_NOT_TRACKED).
>>>>>>>>
>>>>>>>> Here is the relevant snippet from the SDM from the
>>>>>>>> section where it
>>>>>>>> describes EACCEPT:
>>>>>>>>
>>>>>>>> IF (Tracking not correct)
>>>>>>>>     THEN
>>>>>>>>         RFLAGS.ZF := 1;
>>>>>>>>         RAX := SGX_NOT_TRACKED;
>>>>>>>>         GOTO DONE;
>>>>>>>> FI;
>>>>>>>>
>>>>>>>> Reinette
>>>>>>>
>>>>>>> Yes, if enclave calls EACCEPT it does the necessary
>>>>>>> introspection and makes
>>>>>>> sure that ETRACK is completed. I have trouble understanding
>>>>>>> how enclave
>>>>>>> makes sure that EACCEPT was called.
>>>>>>
>>>>>> I'm not concerned of anything going wrong once EMODPR has
>>>>>> been started.
>>>>>>
>>>>>> The problem nails down to that the whole EMODPR process is
>>>>>> spawned by
>>>>>> the entity that is not trusted so maybe that should further
>>>>>> broke down
>>>>>> to three roles:
>>>>>>
>>>>>> 1. Build process B
>>>>>> 2. Runner process R.
>>>>>> 3. Enclave E.
>>>>>>
>>>>>> And to the costraint that we trust B *more* than R. Once B
>>>>>> has done all the
>>>>>> needed EMODPR calls it would send the file descriptor to R.
>>>>>> Even if R would
>>>>>> have full access to /dev/sgx_enclave, it would not matter,
>>>>>> since B has done
>>>>>> EMODPR-EACCEPT dance with E.
>>>>>>
>>>>>> So what you can achieve with EMODPR is not protection against
>>>>>> mistrusted
>>>>>> *OS*. There's absolutely no chance you could use it for that
>>>>>> purpose
>>>>>> because mistrusted OS controls the whole process.
>>>>>>
>>>>>> EMODPR is to help to protect enclave against mistrusted
>>>>>> *process*, i.e.
>>>>>> in the above scenario R.
>>>>>
>>>>> There are two general cases that I can see. Both are valid.
>>>>>
>>>>> 1. The OS moves from a trusted to an untrusted state. This
>>>>> could be
>>>>> the multi-process system you've described. But it could also be
>>>>> that
>>>>> the kernel becomes compromised after the enclave is fully
>>>>> initialized.
>>>>>
>>>>> 2. The OS is untrustworthy from the start.
>>>>>
>>>>> The second case is the stronger one and if you can solve it,
>>>>> the first
>>>>> one is solved implicitly. And our end goal is that if the OS
>>>>> does
>>>>> anything malicious we will crash in a controlled way.
>>>>>
>>>>> A defensive enclave will always want to have the least number
>>>>> of
>>>>> privileges for the maximum protection. Therefore, the enclave
>>>>> will
>>>>> want the OS to call EMODPR. If that were it, the host could
>>>>> just lie.
>>>>> But the enclave also verifies that the EMODPR operation was, in
>>>>> fact,
>>>>> executed by doing EACCEPT. When the enclave calls EACCEPT, if
>>>>> the
>>>>> kernel hasn't restricted permissions then we get a controlled
>>>>> crash.
>>>>> Therefore, we have solved the second case.
>>>>
>>>> So you're referring to this part of the SDM pseude code in the
>>>> SDM:
>>>>
>>>> (* Check the destination EPC page for concurrency *)
>>>> IF ( EPC page in use )
>>>>     THEN #GP(0); FI;
>>>>
>>>> I wonder does "EPC page in use" unconditionally trigger when
>>>> EACCEPT
>>>> is invoked for a page for which all of these conditions hold:
>>>>
>>>> - .PR := 0 (no EMODPR in progress)
>>>> - .MODIFIED := 0 (no EMODT in progress)
>>>> - .PENDING := 0 (no EMODPR in progress)
>>>>
>>>> I don't know the exact scope and scale of "EPC page in use".
>>>>
>>>> Then, yes, EACCEPT could be at least used to validate that one of
>>>> the
>>>> three operations above was requested. However, enclave thread
>>>> cannot say
>>>> which one was it, so it is guesswork.
>>>
>>> OK, I got it, and this last paragraph is not true. SECINFO given
>>> EACCEPT
>>> will lock in rest of the details and make the operation
>>> deterministic.
>>
>> Indeed - so the SDM pseudo code that is relevant here can be found
>> under
>> the "(* Verify that accept request matches current EPC page settings
>> *)"
>> comment where the enclave can verify that all EPCM values are as they
>> should
>> and would fail with SGX_PAGE_ATTRIBUTES_MISMATCH if there is anything
>> amiss.
>>
>>>
>>> The only question mark then is the condition when no requests are
>>> active.
>>
>> Could you please elaborate what you mean with this question? If no
>> request
>> is active then I understand that to mean that no request has started.
> 
> My issue was that when:
> 
> - .PR := 0 (no EMODPR in progress)
> - .MODIFIED := 0 (no EMODT in progress)
> - .PENDING := 0 (no EMODPR in progress)
> 
> Does this trigger #GP when you call EACCEPT?

From what I understand a #GP would be triggered if the EACCEPT does not
specify at least one of these. That would be a problem with the EACCEPT
instruction as opposed to the EPCM contents or OS flow though. This
can be found under the following comment in the SDM pseudo code:

(* Check that the combination of requested PT, PENDING and MODIFIED is legal *)

As far as the actual checking of EPCM values goes, it would not result
in a #GP but for an unexpected value of MODIFIED or PENDING the EACCEPT
will fail with SGX_PAGE_ATTRIBUTES_MISMATCH. EACCEPT does not enforce the PR
bit but it _does_ enforce the individual permission bits.

> I don't think the answer matters that much tho sice if e.g. EMODPR was never
> done, and enclave expected a change, #GP would trigger eventually in SECINFO
> validation.

Similar here as I understand it will not be a #GP but EACCEPT failure with
error SGX_PAGE_ATTRIBUTES_MISMATCH. The relevant pseudo-code in the SDM is
below and you can see how MODIFIED and PENDING are matched but PR not (while
the individual permission bits are):

(* Verify that accept request matches current EPC page settings *)
IF ( (EPCM(DS:RCX).ENCLAVEADDRESS ≠ DS:RCX) or (EPCM(DS:RCX).PENDING ≠ SCRATCH_SECINFO.FLAGS.PENDING) or
(EPCM(DS:RCX).MODIFIED ≠ SCRATCH_SECINFO.FLAGS.MODIFIED) or (EPCM(DS:RCX).R ≠ SCRATCH_SECINFO.FLAGS.R) or
(EPCM(DS:RCX).W ≠ SCRATCH_SECINFO.FLAGS.W) or (EPCM(DS:RCX).X ≠ SCRATCH_SECINFO.FLAGS.X) or
(EPCM(DS:RCX).PT ≠ SCRATCH_SECINFO.FLAGS.PT) )
THEN
     RFLAGS.ZF := 1;
     RAX := SGX_PAGE_ATTRIBUTES_MISMATCH;
     GOTO DONE;
FI;


> 
> The way I look at EACCEPT is a memory verification tool it does the same at
> run-time as EINIT does before run-time.

Indeed.

Reinette