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?
We can map quite closely to the existing intent of EXECUTE, EXECMOD and
EXECMEM via a combination of checking protections at enclave load time and
again at mmap()/mprotect(), e.g.:
#ifdef CONFIG_INTEL_SGX
static inline int enclave_has_perm(u32 sid, u32 requested)
{
return avc_has_perm(&selinux_state, sid, sid,
SECCLASS_PROCESS, requested, NULL);
}
static int selinux_enclave_map(unsigned long prot)
{
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
if ((prot & PROT_EXEC) && (prot & PROT_WRITE))
return enclave_has_perm(sid, PROCESS__EXECMEM);
return 0;
}
static int selinux_enclave_load(struct vm_area_struct *vma, unsigned long prot)
{
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
int ret;
/* Only executable enclave pages are restricted in any way. */
if (!(prot & PROT_EXEC))
return 0;
if (!vma->vm_file || IS_PRIVATE(file_inode(vma->vm_file))) {
ret = enclave_has_perm(sid, PROCESS__EXECMEM);
} else {
ret = file_has_perm(cred, vma->vm_file, FILE__EXECUTE);
if (ret)
goto out;
/*
* Load code from a modified private mapping or from a file
* with the ability to do W->X within the enclave.
*/
if (vma->anon_vma || (prot & PROT_WRITE))
ret = file_has_perm(cred, vma->vm_file,
FILE__EXECMOD);
}
out:
return ret;
}
#endif
