Re: [RFC PATCH v3 3/4] X86/sgx: Introduce EMA as a new LSM module

["Xing, Cedric" <cedric.xing@intel.com>]

On 7/11/2019 9:32 AM, Stephen Smalley wrote:
> On 7/11/19 12:25 PM, Sean Christopherson wrote:
>> On Thu, Jul 11, 2019 at 12:11:06PM -0400, Stephen Smalley wrote:
>>> On 7/11/19 11:12 AM, Sean Christopherson wrote:
>>>> On Thu, Jul 11, 2019 at 09:51:19AM -0400, Stephen Smalley wrote:
>>>>> I'd also feel better if there was clear consensus among all of the
>>>>> @intel.com participants that this is the right approach. To date 
>>>>> that has
>>>>> seemed elusive.
>>>>
>>>> That's a very kind way to phrase things :-)
>>>>
>>>> For initial upstreaming, we've agreed that there is no need to 
>>>> extend the
>>>> uapi, i.e. we can punt on deciding between on-the-fly tracking and 
>>>> having
>>>> userspace specify maximal permissions until we add SGX2 support.
>>>>
>>>> The last open (knock on wood) for initial upstreaming is whether 
>>>> SELinux
>>>> would prefer to have new enclave specific permissions or reuse the
>>>> existing PROCESS__EXECMEM, FILE__EXECUTE and FILE__EXECMOD permissions.
>>>> My understanding is that enclave specific permissions are preferred.
>>>
>>> I was left unclear on this topic after the email exchanges with Cedric.
>>> There are at least three options:
>>>
>>> 1) Reuse the existing EXECMEM, EXECUTE, and EXECMOD permissions.  Pros:
>>> Existing distro policies will be applied in the expected manner with 
>>> respect
>>> to the introduction of executable code into the system, consistent 
>>> control
>>> will be provided over the enclave and the host process, no change for
>>> users/documentation wrt policy.  Cons: Existing permissions don't map
>>> exactly to SGX semantics, no ability to distinguish executable content
>>> within the enclave versus the host process at the LSM level (argued 
>>> earlier
>>> by Cedric to be unnecessary and perhaps meaningless), need to allow
>>> FILE__EXECUTE or other checks on sigstruct files that may not actually
>>> contain code.
>>>
>>> 2) Define new permissions within existing security classes (e.g. 
>>> process2,
>>> file). Pros: Can tailor permission names and definitions to SGX 
>>> semantics,
>>> ability to distinguish enclave versus host process execute access, no 
>>> need
>>> to grant FILE__EXECUTE to sigstruct files, class matches the target 
>>> object,
>>> permissions computed and cached upon existing checks (i.e. when a 
>>> process
>>> accesses a file, all of the permissions to that file are computed and 
>>> then
>>> cached at once, including the enclave-related ones).  Cons: Typical 
>>> distro
>>> policies (unlike Android) allow unknown permissions by default for 
>>> forward
>>> kernel compatibility reasons, so existing policies will permit these new
>>> permissions by default and enforcement will only truly take effect once
>>> policies are updated, adding new permissions to existing classes 
>>> requires an
>>> update to the base policy (so they can't be shipped as a third party 
>>> policy
>>> module alongside the SGX driver or installed as a local module by an 
>>> admin,
>>> for example), documentation/user education required for new permissions.
>>>
>>> 3) Define new permissions in new security classes (e.g. enclave). Pros
>>> relative to #2: New classes and permissions can be defined and 
>>> installed in
>>> third party or local policy module without requiring a change to the 
>>> base
>>> policy.  Cons relative to #2: Class won't correspond to the target 
>>> object,
>>> permissions won't be computed and cached upon existing checks (only when
>>> performing the checks against the new classes).
>>>
>>> Combinations are also possible, of course.
>>
>> What's the impact on distros/ecosystems if we go with #1 for now and 
>> later
>> decide to switch to #2 after upstreaming?  I.e. can we take a minimal-ish
>> approach now without painting ourselves into a corner?
> 
> Yes, I think that's fine.

I can't agree more on this. It's easier to add new things than to take 
existing things out. We can just wait until usages come up that really 
require new permissions.