[RFC PATCH v4 00/12] security: x86/sgx: SGX vs. LSM

[Sean Christopherson]

On Tue, Jul 09, 2019 at 04:11:08PM -0700, Xing, Cedric wrote:
> On 7/9/2019 3:25 PM, Sean Christopherson wrote:
> >On Tue, Jul 09, 2019 at 01:41:28PM -0700, Xing, Cedric wrote:
> >>On 7/9/2019 10:09 AM, Sean Christopherson wrote:
> >>>Translating those to SGX, with a lot of input from Stephen, I ended up
> >>>with the following:
> >>>
> >>>   - FILE__ENCLAVE_EXECUTE: equivalent to FILE__EXECUTE, required to gain X
> >>>                            on an enclave page loaded from a regular file
> >>>
> >>>   - PROCESS2__ENCLAVE_EXECDIRTY: hybrid of EXECMOD and EXECUTE+WRITE,
> >>>                                  required to gain W->X on an enclave page
> >>
> >>EXECMOD basically indicates a file containing self-modifying code. Your
> >>ENCLAVE_EXECDIRTY is however a process permission, which is illogical.
> >
> >How is it illogical?  If a PROCESS wants to EXECute a DIRTY ENCLAVE page,
> >then it needs PROCESS2__ENCLAVE_EXECDIRTY
> Just think of the purpose of FILE__EXECMOD. It indicates to LSM the file has
> self-modifying code, hence W->X transition should be considered "normal" and
> allowed, regardless which process that file is loaded into.
> 
> The same thing for enclaves here. Whether an enclave contains self-modifying
> code is specific to that enclave, regardless which process it is loaded
> into.
> 
> But what are you doing is quite the opposite, and that's I mean by
> "illogical".

Ah.  My intent was to minimize the number of new labels, and because W->X
scenarios are not guaranteed to be backed by a file, I went with a per
process permission.  Ditto for EXECANON.  I'm not opposed to also having a
per file permission that can be used when possible.

Something like this?  And maybe merge EXECANON and EXECDIRTY into a single
permission?

Depending on whether sigstruct is required to be in a pinned file, EAUG
pages would need either EXECDIRTY or EXECMOD.

static int selinux_enclave_load(struct vm_area_struct *vma, unsigned long prot,
				bool measured)
{
	const struct cred *cred = current_cred();
	u32 sid = cred_sid(cred);
	int ret;

	/* Currently supported only in noexec kernels. */
	WARN_ON_ONCE(!default_noexec);

	/* Only executable enclave pages are restricted in any way. */
	if (!(prot & PROT_EXEC))
		return 0;

	if (!measured) {
		ret = enclave_has_perm(sid, PROCESS2__ENCLAVE_EXECUNMR);
		if (ret)
			goto out;
	}

	if (!vma->vm_file || IS_PRIVATE(file_inode(vma->vm_file))) {
		ret = enclave_has_perm(sid, PROCESS2__ENCLAVE_EXECANON);
		if (ret)
			goto out;

		/* Ability to do W->X within the enclave. */
		if (prot & PROT_WRITE)
			ret = enclave_has_perm(sid,
					       PROCESS2__ENCLAVE_EXECDIRTY);
	} else {
		ret = file_has_perm(cred, vma->vm_file, FILE__ENCLAVE_EXECUTE);
		if (ret)
			goto out;

		/*
		 * Load code from a modified private mapping, or from any file
		 * mapping 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__ENCLAVE_EXECMOD);
	}

out:
	return ret;
}