Re: [RFC PATCH v4 09/12] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX

[Stephen Smalley <sds@tycho.nsa.gov>]

On 6/25/19 5:01 PM, Stephen Smalley wrote:
> On 6/21/19 1:05 PM, Xing, Cedric wrote:
>>> From: Christopherson, Sean J
>>> Sent: Wednesday, June 19, 2019 3:24 PM
>>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index 6a1f54ba6794..572ddfc53039 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -1832,11 +1832,18 @@ static inline void 
>>> security_bpf_prog_free(struct bpf_prog_aux *aux)
>>>   #ifdef CONFIG_INTEL_SGX
>>>   #ifdef CONFIG_SECURITY
>>>   int security_enclave_map(unsigned long prot);
>>> +int security_enclave_load(struct vm_area_struct *vma, unsigned long 
>>> prot,
>>> +              bool measured);
>>>   #else
>>>   static inline int security_enclave_map(unsigned long prot)
>>>   {
>>>       return 0;
>>>   }
>>> +static inline int security_enclave_load(struct vm_area_struct *vma,
>>> +                    unsigned long prot, bool measured)
>>> +{
>>> +    return 0;
>>> +}
>>>   #endif /* CONFIG_SECURITY */
>>>   #endif /* CONFIG_INTEL_SGX */
>>
>> Parameters to security_enclave_load() are specific on what's being 
>> loading only, but unspecific on which enclave to be loaded into. That 
>> kills the possibility of an LSM module making enclave dependent 
>> decisions.
>>
>> Btw, if enclave (in the form of struct file) is also passed in as a 
>> parameter, it'd let LSM know that file is an enclave, hence would be 
>> able to make the same decision in security_mmap_file() as in 
>> security_enclave_map(). In other words, you wouldn't need 
>> security_enclave_map().
> 
> Sorry, you want security_enclave_load() to stash a reference to the 
> enclave file in some security module-internal state, then match it upon 
> later security_mmap_file() calls to determine that it is dealing with an 
> enclave, and then adjust its logic accordingly?  When do we release that 
> reference?

I guess you mean set a flag in the enclave file security struct upon 
security_enclave_load() and check that flag in security_mmap_file().

This seems somewhat similar to one of Sean's alternatives in the patch 
description for 06/12, except by pushing the information from sgx to LSM 
upon security_enclave_load() rather than pulling it via a 
is_sgx_enclave() helper.  Not clear if it is still subject to the same 
limitations.