* RE: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Xing, Cedric @ 2019-06-14 0:31 UTC (permalink / raw)
To: Christopherson, Sean J
Cc: Stephen Smalley, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-sgx@vger.kernel.org, jarkko.sakkinen@linux.intel.com,
luto@kernel.org, jmorris@namei.org, serge@hallyn.com,
paul@paul-moore.com, eparis@parisplace.org, jethro@fortanix.com,
Hansen, Dave, tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <20190613231755.GD18385@linux.intel.com>
> From: Christopherson, Sean J
> Sent: Thursday, June 13, 2019 4:18 PM
>
> On Thu, Jun 13, 2019 at 04:03:24PM -0700, Xing, Cedric wrote:
> > > From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> > > Sent: Thursday, June 13, 2019 10:02 AM
> > >
> > > > My RFC series[1] implements #1. My understanding is that Andy
> > > > (Lutomirski) prefers #2. Cedric's RFC series implements #3.
> > > >
> > > > Perhaps the easiest way to make forward progress is to rule out
> the
> > > > options we absolutely *don't* want by focusing on the potentially
> > > > blocking issue with each option:
> > > >
> > > > #1 - SGX UAPI funkiness
> > > >
> > > > #2 - Auditing complexity, potential enclave lock contention
> > > >
> > > > #3 - Pushing SGX details into LSMs and complexity of kernel
> > > > implementation
> > > >
> > > >
> > > > [1]
> > > > https://lkml.kernel.org/r/20190606021145.12604-1-
> sean.j.christopherson
> > > > @intel.com
> > >
> > > Given the complexity tradeoff, what is the clear motivating example
> for
> > > why #1 isn't the obvious choice? That the enclave loader has no way
> of
> > > knowing a priori whether the enclave will require W->X or WX? But
> > > aren't we better off requiring enclaves to be explicitly marked as
> > > needing such so that we can make a more informed decision about
> whether
> > > to load them in the first place?
> >
> > Are you asking this question at a) page granularity, b) file
> granularity or
> > c) enclave (potentially comprised of multiple executable files)
> granularity?
> >
> > #b is what we have on regular executable files and shared objects (i.e.
> > FILE__EXECMOD). We all know how to do that.
> >
> > #c is kind of new but could be done via some proxy file (e.g.
> sigstruct file)
> > hence reduced to #b.
> >
> > #a is problematic. It'd require compilers/linkers to generate such
> > information, and proper executable image file format to carry that
> > information, to be eventually picked up the loader. SELinux doesn't
> have
> > PAGE__EXECMOD I guess is because it is generally considered
> impractical.
> >
> > Option #1 however requires #a because the driver doesn't track which
> page was
> > loaded from which file, otherwise it can no longer be qualified
> "simple". Or
> > we could just implement #c, which will make all options simpler. But I
> guess
> > #b is still preferred, to be aligned with what SELinux is enforcing
> today on
> > regular memory pages.o
>
> Option #1 doesn't require (a). The checks will happen for every page,
> but in the RFCs I sent, the policies are still attached to files and
> processes, i.e. (b).
I was talking at the UAPI level - i.e. your ioctl requires ALLOW_* at page granularity, hence #a.
^ permalink raw reply
* Re: [RFC 0/7] Introduce TEE based Trusted Keys support
From: Mimi Zohar @ 2019-06-14 0:03 UTC (permalink / raw)
To: Casey Schaufler, Sumit Garg, keyrings, linux-integrity,
linux-security-module
Cc: jens.wiklander, corbet, dhowells, jejb, jarkko.sakkinen, jmorris,
serge, ard.biesheuvel, daniel.thompson, linux-doc, linux-kernel,
tee-dev
In-Reply-To: <d803283e-5e69-5deb-fe94-3f2e45fb95af@schaufler-ca.com>
On Thu, 2019-06-13 at 09:40 -0700, Casey Schaufler wrote:
> On 6/13/2019 3:30 AM, Sumit Garg wrote:
> > Add support for TEE based trusted keys where TEE provides the functionality
> > to seal and unseal trusted keys using hardware unique key. Also, this is
> > an alternative in case platform doesn't possess a TPM device.
> >
> > This series also adds some TEE features like:
>
> Please expand the acronym TEE on first use. That will
> help people who don't work with it on a daily basis
> understand what you're going on about.
Thanks, Casey.
"[6/7] doc: keys: Document usage of TEE based Trusted Keys" refers to
the kernel tee documentation, but that documentation is limited to
userspace interaction with the tee.
A trusted key is a random number generated and sealed(encrypted) by
the TPM, so that only the TPM may unseal it. The sealing key never
leaves the TPM. The sealed, trusted key may be exported to userspace.
In the tee case, can the "sealing" key ever leave the tee? Can the
sealed, trusted key, exported to userspace, be unsealed by the tee?
Are the tee security protections similar to those of the TPM? How do
they compare?
Mimi
>
> >
> > Patch #1, #2 enables support for registered kernel shared memory with TEE.
> >
> > Patch #3 enables support for private kernel login method required for
> > cases like trusted keys where we don't wan't user-space to directly access
> > TEE service to retrieve trusted key contents.
> >
> > Rest of the patches from #4 to #7 adds support for TEE based trusted keys.
> >
> > This patch-set has been tested with OP-TEE based pseudo TA which can be
> > found here [1].
> >
> > Looking forward to your valuable feedback/suggestions.
^ permalink raw reply
* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Sean Christopherson @ 2019-06-13 23:17 UTC (permalink / raw)
To: Xing, Cedric
Cc: Stephen Smalley, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-sgx@vger.kernel.org, jarkko.sakkinen@linux.intel.com,
luto@kernel.org, jmorris@namei.org, serge@hallyn.com,
paul@paul-moore.com, eparis@parisplace.org, jethro@fortanix.com,
Hansen, Dave, tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F65503EDD@ORSMSX116.amr.corp.intel.com>
On Thu, Jun 13, 2019 at 04:03:24PM -0700, Xing, Cedric wrote:
> > From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> > Sent: Thursday, June 13, 2019 10:02 AM
> >
> > > My RFC series[1] implements #1. My understanding is that Andy
> > > (Lutomirski) prefers #2. Cedric's RFC series implements #3.
> > >
> > > Perhaps the easiest way to make forward progress is to rule out the
> > > options we absolutely *don't* want by focusing on the potentially
> > > blocking issue with each option:
> > >
> > > #1 - SGX UAPI funkiness
> > >
> > > #2 - Auditing complexity, potential enclave lock contention
> > >
> > > #3 - Pushing SGX details into LSMs and complexity of kernel
> > > implementation
> > >
> > >
> > > [1]
> > > https://lkml.kernel.org/r/20190606021145.12604-1-sean.j.christopherson
> > > @intel.com
> >
> > Given the complexity tradeoff, what is the clear motivating example for
> > why #1 isn't the obvious choice? That the enclave loader has no way of
> > knowing a priori whether the enclave will require W->X or WX? But
> > aren't we better off requiring enclaves to be explicitly marked as
> > needing such so that we can make a more informed decision about whether
> > to load them in the first place?
>
> Are you asking this question at a) page granularity, b) file granularity or
> c) enclave (potentially comprised of multiple executable files) granularity?
>
> #b is what we have on regular executable files and shared objects (i.e.
> FILE__EXECMOD). We all know how to do that.
>
> #c is kind of new but could be done via some proxy file (e.g. sigstruct file)
> hence reduced to #b.
>
> #a is problematic. It'd require compilers/linkers to generate such
> information, and proper executable image file format to carry that
> information, to be eventually picked up the loader. SELinux doesn't have
> PAGE__EXECMOD I guess is because it is generally considered impractical.
>
> Option #1 however requires #a because the driver doesn't track which page was
> loaded from which file, otherwise it can no longer be qualified "simple". Or
> we could just implement #c, which will make all options simpler. But I guess
> #b is still preferred, to be aligned with what SELinux is enforcing today on
> regular memory pages.o
Option #1 doesn't require (a). The checks will happen for every page,
but in the RFCs I sent, the policies are still attached to files and
processes, i.e. (b).
^ permalink raw reply
* RE: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Xing, Cedric @ 2019-06-13 23:03 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J
Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-sgx@vger.kernel.org,
jarkko.sakkinen@linux.intel.com, luto@kernel.org,
jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com,
eparis@parisplace.org, jethro@fortanix.com, Hansen, Dave,
tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <8d99d8fb-a921-286a-8cf0-cd522e09b37c@tycho.nsa.gov>
> From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> Sent: Thursday, June 13, 2019 10:02 AM
>
> On 6/11/19 6:02 PM, Sean Christopherson wrote:
> > On Tue, Jun 11, 2019 at 09:40:25AM -0400, Stephen Smalley wrote:
> >> I haven't looked at this code closely, but it feels like a lot of
> >> SGX-specific logic embedded into SELinux that will have to be
> >> repeated or reused for every security module. Does SGX not track
> this state itself?
> >
> > SGX does track equivalent state.
> >
> > There are three proposals on the table (I think):
> >
> > 1. Require userspace to explicitly specificy (maximal) enclave page
> > permissions at build time. The enclave page permissions are
> provided
> > to, and checked by, LSMs at enclave build time.
> >
> > Pros: Low-complexity kernel implementation, straightforward
> auditing
> > Cons: Sullies the SGX UAPI to some extent, may increase
> complexity of
> > SGX2 enclave loaders.
> >
> > 2. Pre-check LSM permissions and dynamically track mappings to
> enclave
> > pages, e.g. add an SGX mprotect() hook to restrict W->X and WX
> > based on the pre-checked permissions.
> >
> > Pros: Does not impact SGX UAPI, medium kernel complexity
> > Cons: Auditing is complex/weird, requires taking enclave-
> specific
> > lock during mprotect() to query/update tracking.
> >
> > 3. Implement LSM hooks in SGX to allow LSMs to track enclave
> regions
> > from cradle to grave, but otherwise defer everything to LSMs.
> >
> > Pros: Does not impact SGX UAPI, maximum flexibility, precise
> auditing
> > Cons: Most complex and "heaviest" kernel implementation of the
> three,
> > pushes more SGX details into LSMs.
> >
> > My RFC series[1] implements #1. My understanding is that Andy
> > (Lutomirski) prefers #2. Cedric's RFC series implements #3.
> >
> > Perhaps the easiest way to make forward progress is to rule out the
> > options we absolutely *don't* want by focusing on the potentially
> > blocking issue with each option:
> >
> > #1 - SGX UAPI funkiness
> >
> > #2 - Auditing complexity, potential enclave lock contention
> >
> > #3 - Pushing SGX details into LSMs and complexity of kernel
> > implementation
> >
> >
> > [1]
> > https://lkml.kernel.org/r/20190606021145.12604-1-sean.j.christopherson
> > @intel.com
>
> Given the complexity tradeoff, what is the clear motivating example for
> why #1 isn't the obvious choice? That the enclave loader has no way of
> knowing a priori whether the enclave will require W->X or WX? But
> aren't we better off requiring enclaves to be explicitly marked as
> needing such so that we can make a more informed decision about whether
> to load them in the first place?
Are you asking this question at a) page granularity, b) file granularity or c) enclave (potentially comprised of multiple executable files) granularity?
#b is what we have on regular executable files and shared objects (i.e. FILE__EXECMOD). We all know how to do that.
#c is kind of new but could be done via some proxy file (e.g. sigstruct file) hence reduced to #b.
#a is problematic. It'd require compilers/linkers to generate such information, and proper executable image file format to carry that information, to be eventually picked up the loader. SELinux doesn't have PAGE__EXECMOD I guess is because it is generally considered impractical.
Option #1 however requires #a because the driver doesn't track which page was loaded from which file, otherwise it can no longer be qualified "simple". Or we could just implement #c, which will make all options simpler. But I guess #b is still preferred, to be aligned with what SELinux is enforcing today on regular memory pages.
^ permalink raw reply
* RE: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Xing, Cedric @ 2019-06-13 21:09 UTC (permalink / raw)
To: Christopherson, Sean J, Stephen Smalley
Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-sgx@vger.kernel.org,
jarkko.sakkinen@linux.intel.com, luto@kernel.org,
jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com,
eparis@parisplace.org, jethro@fortanix.com, Hansen, Dave,
tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <20190613194833.GB18385@linux.intel.com>
> From: Christopherson, Sean J
> Sent: Thursday, June 13, 2019 12:49 PM
>
> > >By "SGX", did you mean the SGX subsystem being upstreamed? It doesn’t
> > >track that state. In practice, there's no way for SGX to track it
> > >because there's no vm_ops->may_mprotect() callback. It doesn't follow
> > >the philosophy of Linux either, as mprotect() doesn't track it for
> > >regular memory. And it doesn't have a use without LSM, so I believe
> > >it makes more sense to track it inside LSM.
> >
> > Yes, the SGX driver/subsystem. I had the impression from Sean that it
> > does track this kind of per-page state already in some manner, but
> > possibly he means it does under a given proposal and not in the
> current driver.
>
> Yeah, under a given proposal. SGX has per-page tracking, adding new
> flags is fairly easy. Philosophical objections aside,
> adding .may_mprotect() is trivial.
As I pointed out in an earlier email, protection flags are associated with ranges. They could of course be duplicated to every page but that will hurt performance because every page within the range would have to be tested individually.
Furthermore, though .may_protect()is able to make the decision, I don't think it can do the audit log as well, unless it is coded in an SELinux specific way. Then I wonder how it could work with LSM modules other than SELinux.
>
> > Even the #b remembering might end up being SELinux-specific if we also
> > have to remember the original inputs used to compute the answer so
> > that we can audit that information when access is denied later upon
> > mprotect(). At the least we'd need it to save some opaque data and
> > pass it to a callback into SELinux to perform that auditing.
^ permalink raw reply
* RE: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Xing, Cedric @ 2019-06-13 21:02 UTC (permalink / raw)
To: Stephen Smalley, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-sgx@vger.kernel.org
Cc: jarkko.sakkinen@linux.intel.com, luto@kernel.org,
jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com,
eparis@parisplace.org, jethro@fortanix.com, Hansen, Dave,
tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <b7f3decf-b1d2-01b1-6294-ccf9ba2d5df4@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
>
> On 6/11/19 6:55 PM, Xing, Cedric wrote:
> >> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> >> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> >> Sent: Tuesday, June 11, 2019 6:40 AM
> >>
> >>>
> >>> +#ifdef CONFIG_INTEL_SGX
> >>> + rc = sgxsec_mprotect(vma, prot);
> >>> + if (rc <= 0)
> >>> + return rc;
> >>
> >> Why are you skipping the file_map_prot_check() call when rc == 0?
> >> What would SELinux check if you didn't do so -
> >> FILE__READ|FILE__WRITE|FILE__EXECUTE to /dev/sgx/enclave? Is it a
> >> problem to let SELinux proceed with that check?
> >
> > We can continue the check. But in practice, all
> FILE__{READ|WRITE|EXECUTE} are needed for every enclave, then what's the
> point of checking them? FILE__EXECMOD may be the only flag that has a
> meaning, but it's kind of redundant because sigstruct file was checked
> against that already.
>
> I don't believe FILE__EXECMOD will be checked since it is a shared file
> mapping. We'll check at least FILE__READ and FILE__WRITE anyway upon
> open(), and possibly FILE__EXECUTE upon mmap() unless that is never
> PROT_EXEC. We want the policy to accurately reflect the operations of
> the system, even when an operation "must" be allowed, and even here this
> only needs to be allowed to processes authorized as enclave loaders, not
> to all processes.
>
> I don't think there are other examples where we skip a SELinux check
> like this. If we were to do so here, we would at least need a comment
> explaining that it was intentional and why. The risk would be that
> future checking added into file_map_prot_check() would be unwittingly
> bypassed for these mappings. A warning there would also be advisable if
> we skip it for these mappings.
You are right! The code was written assuming file_map_prot_check() wouldn't object if sgxsec_mprotect() approves it, but that may not always be the case if new checks are added in future. I'll add the check back.
>
> >
> >>> +static int selinux_enclave_load(struct file *encl, unsigned long
> addr,
> >>> + unsigned long size, unsigned long prot,
> >>> + struct vm_area_struct *source)
> >>> +{
> >>> + if (source) {
> >>> + /**
> >>> + * Adding page from source => EADD request
> >>> + */
> >>> + int rc = selinux_file_mprotect(source, prot, prot);
> >>> + if (rc)
> >>> + return rc;
> >>> +
> >>> + if (!(prot & VM_EXEC) &&
> >>> + selinux_file_mprotect(source, VM_EXEC, VM_EXEC))
> >>
> >> I wouldn't conflate VM_EXEC with PROT_EXEC even if they happen to be
> >> defined with the same values currently. Elsewhere the kernel appears
> >> to explicitly translate them ala calc_vm_prot_bits().
> >
> > Thanks! I'd change them to PROT_EXEC in the next version.
> >
> >>
> >> Also, this will mean that we will always perform an execute check on
> >> all sources, thereby triggering audit denial messages for any EADD
> >> sources that are only intended to be data. Depending on the source,
> >> this could trigger PROCESS__EXECMEM or FILE__EXECMOD or
> >> FILE__EXECUTE. In a world where users often just run any denials
> >> they see through audit2allow, they'll end up always allowing them
> >> all. How can they tell whether it was needed? It would be preferable
> >> if we could only trigger execute checks when there is some
> >> probability that execute will be requested in the future.
> >> Alternatives would be to silence the audit of these permission checks
> >> always via use of _noaudit() interfaces or to silence audit of these
> >> permissions via dontaudit rules in policy, but the latter would hide
> >> all denials of the permission by the process, not just those
> >> triggered from security_enclave_load(). And if we silence them, then
> we won't see them even if they were needed.
> >
> > *_noaudit() is exactly what I wanted. But I couldn't find
> selinux_file_mprotect_noaudit()/file_has_perm_noaudit(), and I'm
> reluctant to duplicate code. Any suggestions?
>
> I would have no objection to adding _noaudit() variants of these, either
> duplicating code (if sufficiently small/simple) or creating a common
> helper with a bool audit flag that gets used for both. But the larger
> issue would be to resolve how to ultimately ensure that a denial is
> audited later if the denied permission is actually requested and blocked
> via sgxsec_mprotect().
The idea here is to precompute the answers as if a certain request were received, so that we don't have to store all inputs to the precomputation. sgxsec_mprotect(), if coded correctly, would make the same decision regardless it was precomputed or computed at the time of the real request. Auditing requires more information than making the decision itself, such as the file path and when the request was made. I'm reluctant to keep the source files open just for audit logs. I'll need a closer look at the auditing code to figure out an appropriate way.
>
> >
> >>
> >>> + prot = 0;
> >>> + else {
> >>> + prot = SGX__EXECUTE;
> >>> + if (source->vm_file &&
> >>> + !file_has_perm(current_cred(), source->vm_file,
> >>> + FILE__EXECMOD))
> >>> + prot |= SGX__EXECMOD;
> >>
> >> Similarly, this means that we will always perform a FILE__EXECMOD
> check
> >> on all executable sources, triggering audit denial messages for any
> EADD
> >> source that is executable but to which EXECMOD is not allowed, and
> again
> >> the most common pattern will be that users will add EXECMOD to all
> >> executable sources to avoid this.
> >>
> >>> + }
> >>> + return sgxsec_eadd(encl, addr, size, prot);
> >>> + } else {
> >>> + /**
> >>> + * Adding page from NULL => EAUG request
> >>> + */
> >>> + return sgxsec_eaug(encl, addr, size, prot);
> >>> + }
> >>> +}
> >>> +
> >>> +static int selinux_enclave_init(struct file *encl,
> >>> + const struct sgx_sigstruct *sigstruct,
> >>> + struct vm_area_struct *vma)
> >>> +{
> >>> + int rc = 0;
> >>> +
> >>> + if (!vma)
> >>> + rc = -EINVAL;
> >>
> >> Is it ever valid to call this hook with a NULL vma? If not, this
> should
> >> be handled/prevented by the caller. If so, I'd just return -EINVAL
> >> immediately here.
> >
> > vma shall never be NULL. I'll update it in the next version.
> >
> >>
> >>> +
> >>> + if (!rc && !(vma->vm_flags & VM_EXEC))
> >>> + rc = selinux_file_mprotect(vma, VM_EXEC, VM_EXEC);
> >>
> >> I had thought we were trying to avoid overloading FILE__EXECUTE (or
> >> whatever gets checked here, e.g. could be PROCESS__EXECMEM or
> >> FILE__EXECMOD) on the sigstruct file, since the caller isn't truly
> >> executing code from it.
> >
> > Agreed. Another problem with FILE__EXECMOD on the sigstruct file is
> that user code would then be allowed to modify SIGSTRUCT at will, which
> effectively wipes out the protection provided by FILE__EXECUTE.
> >
> >>
> >> I'd define new ENCLAVE__* permissions, including an up-front
> >> ENCLAVE__INIT permission that governs whether the sigstruct file can
> be
> >> used at all irrespective of memory protections.
> >
> > Agreed.
> >
> >>
> >> Then you can also have ENCLAVE__EXECUTE, ENCLAVE__EXECMEM,
> >> ENCLAVE__EXECMOD for the execute-related checks. Or you can use the
> >> /dev/sgx/enclave inode as the target for the execute checks and just
> >> reuse the file permissions there.
> >
> > Now we've got 2 options - 1) New ENCLAVE__* flags on sigstruct file or
> 2) FILE__* on /dev/sgx/enclave. Which one do you think makes more sense?
> >
> > ENCLAVE__EXECMEM seems to offer finer granularity (than
> PROCESS__EXECMEM) but I wonder if it'd have any real use in practice.
>
> Defining a separate ENCLAVE__EXECUTE and using it here for the sigstruct
> file would avoid any ambiguity with the FILE__EXECUTE check to the
> /dev/sgx/enclave inode that might occur upon mmap() or mprotect(). A
> separate ENCLAVE__EXECMEM would enable allowing WX within the enclave
> while denying it in the host application or vice versa, which could be a
> good thing for security, particularly if SGX2 largely ends up always
> wanting WX.
Agreed. I'll include those new flags in my next version.
>
> >
> >>> +int sgxsec_mprotect(struct vm_area_struct *vma, size_t prot) {
> >>> + struct enclave_sec *esec;
> >>> + int rc;
> >>> +
> >>> + if (!vma->vm_file || !(esec = __esec(selinux_file(vma->vm_file))))
> >> {
> >>> + /* Positive return value indicates non-enclave VMA */
> >>> + return 1;
> >>> + }
> >>> +
> >>> + down_read(&esec->sem);
> >>> + rc = enclave_mprotect(&esec->regions, vma->vm_start, vma->vm_end,
> >>> +prot);
> >>
> >> Why is it safe for this to only use down_read()? enclave_mprotect()
> can
> >> call enclave_prot_set_cb() which modifies the list?
> >
> > Probably because it was too late at night when I wrote this line:-
> ( Good catch!
> >
> >>
> >> I haven't looked at this code closely, but it feels like a lot of
> SGX-
> >> specific logic embedded into SELinux that will have to be repeated or
> >> reused for every security module. Does SGX not track this state
> itself?
> >
> > I can tell you have looked quite closely, and I truly think you for
> your time!
> >
> > You are right that there are SGX specific stuff. More precisely, SGX
> enclaves don't have access to anything except memory, so there are only
> 3 questions that need to be answered for each enclave page: 1) whether X
> is allowed; 2) whether W->X is allowed and 3 whether WX is allowed. This
> proposal tries to cache the answers to those questions upon creation of
> each enclave page, meaning it involves a) figuring out the answers and b)
> "remember" them for every page. #b is generic, mostly captured in
> intel_sgx.c, and could be shared among all LSM modules; while #a is
> SELinux specific. I could move intel_sgx.c up one level in the directory
> hierarchy if that's what you'd suggest.
> >
> > By "SGX", did you mean the SGX subsystem being upstreamed? It doesn’t
> track that state. In practice, there's no way for SGX to track it
> because there's no vm_ops->may_mprotect() callback. It doesn't follow
> the philosophy of Linux either, as mprotect() doesn't track it for
> regular memory. And it doesn't have a use without LSM, so I believe it
> makes more sense to track it inside LSM.
>
> Yes, the SGX driver/subsystem. I had the impression from Sean that it
> does track this kind of per-page state already in some manner, but
> possibly he means it does under a given proposal and not in the current
> driver.
Yes, SGX subsystem does track per-page states. But this page protection flags apply to *ranges*.
In practice, those per-page states are *not* checked at mmap/mprotect. They are used mainly by vm_ops->fault() and the page swapper thread.
That said, merging protection flags into per-page states will require page-by-page checks, which will definitely hurt performance. Unless the driver also maintains some range oriented structures just like what you see here.
>
> Even the #b remembering might end up being SELinux-specific if we also
> have to remember the original inputs used to compute the answer so that
> we can audit that information when access is denied later upon
> mprotect(). At the least we'd need it to save some opaque data and pass
> it to a callback into SELinux to perform that auditing.
Agreed. What's commonly needed here is a data structure that supports setting/querying value on ranges. It's close to what xarray supports, but xarray doesn't support range querying.
^ permalink raw reply
* Re: [PATCH V8 0/3] Add support for measuring the boot command line during kexec_file_load
From: Mimi Zohar @ 2019-06-13 20:48 UTC (permalink / raw)
To: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel
Cc: roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-1-prsriva02@gmail.com>
On Wed, 2019-06-12 at 15:15 -0700, Prakhar Srivastava wrote:
> The kexec cmdline hash is stored in the "d-ng" field of the template data.
> and can be verified using
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
> grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
This information should also be included in one of the patches.
Mimi
^ permalink raw reply
* Re: [PATCH V8 3/3] Call ima_kexec_cmdline to measure the cmdline args
From: Mimi Zohar @ 2019-06-13 20:20 UTC (permalink / raw)
To: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel
Cc: roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-4-prsriva02@gmail.com>
On Wed, 2019-06-12 at 15:15 -0700, Prakhar Srivastava wrote:
> During soft reboot(kexec_file_load) boot cmdline args
Any reason for not spelling it out and using the "boot command line"?
> are not measured.Thus the new kernel on load boots with
> an assumption of cold reboot.
Double spaces after a period.
What does "boots with an assumption of cold reboot" mean? Not all
systems are booted the same way. Is this comment relevant?
>
> This patch makes a call to the ima hook ima_kexec_cmdline,
> added in "Define a new IMA hook to measure the boot command
> line arguments"
"added in ..." is unnecessry.
> to measure the boot cmdline args into the ima log.
^IMA measurement list.
Mimi
^ permalink raw reply
* Re: [PATCH V8 3/3] Call ima_kexec_cmdline to measure the cmdline args
From: Mimi Zohar @ 2019-06-13 20:07 UTC (permalink / raw)
To: Dave Young
Cc: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel, roberto.sassu, Eric W. Biederman, vgoyal, kexec
In-Reply-To: <20190613082627.GA30288@dhcp-128-65.nay.redhat.com>
On Thu, 2019-06-13 at 16:26 +0800, Dave Young wrote:
> On 06/12/19 at 06:31pm, Mimi Zohar wrote:
> > [Cc: kexec mailing list]
> >
> > Hi Eric, Dave,
> >
> > On Wed, 2019-06-12 at 15:15 -0700, Prakhar Srivastava wrote:
> > > During soft reboot(kexec_file_load) boot cmdline args
> > > are not measured.Thus the new kernel on load boots with
> > > an assumption of cold reboot.
> > >
> > > This patch makes a call to the ima hook ima_kexec_cmdline,
> > > added in "Define a new IMA hook to measure the boot command
> > > line arguments"
> > > to measure the boot cmdline args into the ima log.
> > >
> > > - call ima_kexec_cmdline from kexec_file_load.
> > > - move the call ima_add_kexec_buffer after the cmdline
> > > args have been measured.
> > >
> > > Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dave Young <dyoung@redhat.com>
> >
> > Any chance we could get some Acks?
>
> The ima_* is blackbox functions to me, looks like this patch is trying
> to measure kexec cmdline buffer and save in some ima logs and then add all the
> measure results including those for kernel/initrd to a kexec_buf and pass to 2nd
Right, including the new boot command line measurement.
> kernel.
>
> It should be good and only take effect when IMA enabled. If all the
> assumptions are right:
>
> Acked-by: Dave Young <dyoung@redhat.com>
Thanks, Dave.
^ permalink raw reply
* Re: [PATCH V8 2/3] Define a new ima template field buf
From: Mimi Zohar @ 2019-06-13 19:59 UTC (permalink / raw)
To: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel
Cc: roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-3-prsriva02@gmail.com>
On Wed, 2019-06-12 at 15:15 -0700, Prakhar Srivastava wrote:
As before, the patch title needs to be prefixed with "ima: ".
> /* IMA template field data definition */
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index ea7d8cbf712f..83ca99d65e4b 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -140,7 +140,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
> struct ima_template_entry *entry;
> struct inode *inode = file_inode(file);
> struct ima_event_data event_data = {iint, file, filename, NULL, 0,
> - cause};
> + cause, NULL, 0};
This change here and
> int violation = 1;
> int result;
>
> @@ -296,7 +296,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
> struct inode *inode = file_inode(file);
> struct ima_template_entry *entry;
> struct ima_event_data event_data = {iint, file, filename, xattr_value,
> - xattr_len, NULL};
> + xattr_len, NULL, NULL, 0};
here and
> int violation = 0;
>
> if (iint->measured_pcrs & (0x1 << pcr))
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 993d0f1915ff..c8591406c0e2 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -50,7 +50,7 @@ static int __init ima_add_boot_aggregate(void)
> struct ima_template_entry *entry;
> struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
> struct ima_event_data event_data = {iint, NULL, boot_aggregate_name,
> - NULL, 0, NULL};
> + NULL, 0, NULL, NULL, 0};
here, don't belong in this patch. It belongs in "IMA: support for per
policy rule template formats", in case it should ever be backported.
Please post this as a separate patch, that will be squashed with
"IMA: support for per policy rule template formats".
> int result = -ENOMEM;
> int violation = 0;
> struct {
> diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
> index 6a3d8b831deb..f0178bc60c55 100644
> --- a/security/integrity/ima/ima_template_lib.h
> +++ b/security/integrity/ima/ima_template_lib.h
> @@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
> struct ima_field_data *field_data);
> void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
> struct ima_field_data *field_data);
> +void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
> + struct ima_field_data *field_data);
Formatting ...
> int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
> int maxfields, struct ima_field_data *fields, int *curfields,
> unsigned long *len_mask, int enforce_mask, char *bufname);
> @@ -42,4 +44,6 @@ int ima_eventname_ng_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data);
> int ima_eventsig_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data);
> +int ima_eventbuf_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data);
Formatting ...
> #endif /* __LINUX_IMA_TEMPLATE_LIB_H */
^ permalink raw reply
* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Sean Christopherson @ 2019-06-13 19:48 UTC (permalink / raw)
To: Stephen Smalley
Cc: Xing, Cedric, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-sgx@vger.kernel.org, jarkko.sakkinen@linux.intel.com,
luto@kernel.org, jmorris@namei.org, serge@hallyn.com,
paul@paul-moore.com, eparis@parisplace.org, jethro@fortanix.com,
Hansen, Dave, tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <b7f3decf-b1d2-01b1-6294-ccf9ba2d5df4@tycho.nsa.gov>
On Thu, Jun 13, 2019 at 02:00:29PM -0400, Stephen Smalley wrote:
> On 6/11/19 6:55 PM, Xing, Cedric wrote:
> >You are right that there are SGX specific stuff. More precisely, SGX
> >enclaves don't have access to anything except memory, so there are only 3
> >questions that need to be answered for each enclave page: 1) whether X is
> >allowed; 2) whether W->X is allowed and 3 whether WX is allowed. This
> >proposal tries to cache the answers to those questions upon creation of each
> >enclave page, meaning it involves a) figuring out the answers and b)
> >"remember" them for every page. #b is generic, mostly captured in
> >intel_sgx.c, and could be shared among all LSM modules; while #a is SELinux
> >specific. I could move intel_sgx.c up one level in the directory hierarchy
> >if that's what you'd suggest.
> >
> >By "SGX", did you mean the SGX subsystem being upstreamed? It doesn’t track
> >that state. In practice, there's no way for SGX to track it because there's
> >no vm_ops->may_mprotect() callback. It doesn't follow the philosophy of
> >Linux either, as mprotect() doesn't track it for regular memory. And it
> >doesn't have a use without LSM, so I believe it makes more sense to track it
> >inside LSM.
>
> Yes, the SGX driver/subsystem. I had the impression from Sean that it does
> track this kind of per-page state already in some manner, but possibly he
> means it does under a given proposal and not in the current driver.
Yeah, under a given proposal. SGX has per-page tracking, adding new flags
is fairly easy. Philosophical objections aside, adding .may_mprotect() is
trivial.
> Even the #b remembering might end up being SELinux-specific if we also have
> to remember the original inputs used to compute the answer so that we can
> audit that information when access is denied later upon mprotect(). At the
> least we'd need it to save some opaque data and pass it to a callback into
> SELinux to perform that auditing.
^ permalink raw reply
* Re: [PATCH V8 1/3] Define a new IMA hook to measure the boot command line arguments
From: Mimi Zohar @ 2019-06-13 19:22 UTC (permalink / raw)
To: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel
Cc: roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-2-prsriva02@gmail.com>
Hi Prakhar,
Patches titles in the subject line need to be prefixed with the
subsystem, in this case "ima: ".
On Wed, 2019-06-12 at 15:15 -0700, Prakhar Srivastava wrote:
> This patch adds support in ima to measure kexec cmdline args
> during soft reboot(kexec_file_load).
Based on the patch title, the word "ima" is redundant. Patch
descriptions are suppose to be written in the third person. "This
patch adds" is unnecessary. Please review section 3 "Describe your
changes" in Documentation/process/submitting-patches.rst.
>
> - A new ima hook ima_kexec_cmdline is defined to be called by the
> kexec code.
> - A new function process_buffer_measurement is defined to measure
> the buffer hash into the ima log.
> - A new func policy KEXEC_CMDLINE is defined to control the
> measurement.[Suggested by Mimi]
>
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index fd9b01881d17..98e351e13557 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -292,6 +292,13 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> {
> int i;
>
> + /* only incase of KEXEC_CMDLINE, inode is NULL */
> + if (func == KEXEC_CMDLINE) {
> + if ((rule->flags & IMA_FUNC) &&
> + (rule->func == func) && (!inode))
Thank you for fixing the other formatting issues. Here's another one.
Is checking !inode needed?
Mimi
> + return true;
> + return false;
> + }
> if ((rule->flags & IMA_FUNC) &&
> (rule->func != func && func != POST_SETATTR))
> return false;
>
^ permalink raw reply
* Re: [PATCH V8 3/3] Call ima_kexec_cmdline to measure the cmdline args
From: James Morris @ 2019-06-13 19:16 UTC (permalink / raw)
To: Prakhar Srivastava
Cc: linux-integrity, linux-security-module, linux-kernel, zohar,
roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-4-prsriva02@gmail.com>
On Wed, 12 Jun 2019, Prakhar Srivastava wrote:
> During soft reboot(kexec_file_load) boot cmdline args
> are not measured.Thus the new kernel on load boots with
> an assumption of cold reboot.
>
> This patch makes a call to the ima hook ima_kexec_cmdline,
> added in "Define a new IMA hook to measure the boot command
> line arguments"
> to measure the boot cmdline args into the ima log.
>
> - call ima_kexec_cmdline from kexec_file_load.
> - move the call ima_add_kexec_buffer after the cmdline
> args have been measured.
>
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> ---
> kernel/kexec_file.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH V8 2/3] Define a new ima template field buf
From: James Morris @ 2019-06-13 19:15 UTC (permalink / raw)
To: Prakhar Srivastava
Cc: linux-integrity, linux-security-module, linux-kernel, zohar,
roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-3-prsriva02@gmail.com>
On Wed, 12 Jun 2019, Prakhar Srivastava wrote:
> A buffer(kexec cmdline args) measured into ima cannot be
> appraised without already being aware of the buffer contents.
> Since hashes are non-reversible, raw buffer is needed for
> validation or regenerating hash for appraisal/attestation.
>
> This patch adds support to ima to allow store/read the
> buffer contents in HEX.
>
> - Add two new fields to ima_event_data to hold the buf and
> buf_len [Suggested by Roberto]
> - Add a new temaplte field 'buf' to be used to store/read
> the buffer data.[Suggested by Mimi]
> - Updated process_buffer_meaurement to add the buffer to
> ima_event_data. process_buffer_measurement added in
> "Define a new IMA hook to measure the boot command line
> arguments"
> - Add a new template policy name ima-buf to represent
> 'd-ng|n-ng|buf'
>
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH V8 1/3] Define a new IMA hook to measure the boot command line arguments
From: James Morris @ 2019-06-13 19:10 UTC (permalink / raw)
To: Prakhar Srivastava
Cc: linux-integrity, linux-security-module, linux-kernel, zohar,
roberto.sassu, vgoyal
In-Reply-To: <20190612221549.28399-2-prsriva02@gmail.com>
On Wed, 12 Jun 2019, Prakhar Srivastava wrote:
> This patch adds support in ima to measure kexec cmdline args
> during soft reboot(kexec_file_load).
>
> - A new ima hook ima_kexec_cmdline is defined to be called by the
> kexec code.
> - A new function process_buffer_measurement is defined to measure
> the buffer hash into the ima log.
> - A new func policy KEXEC_CMDLINE is defined to control the
> measurement.[Suggested by Mimi]
>
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
> + struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
> + struct ima_event_data event_data = {.iint = iint };
Minor nit: looks like this could be simplified to:
struct integrity_iint_cache iint = {};
struct ima_event_data event_data = {.iint = &iint };
which also saves the later memset. 'hash' can also be initialized with '=
{}'.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: What do LSMs *actually* need for checks on notifications?
From: Stephen Smalley @ 2019-06-13 18:46 UTC (permalink / raw)
To: David Howells
Cc: Casey Schaufler, Andy Lutomirski, viro, linux-usb,
linux-security-module, linux-fsdevel, linux-kernel, Paul Moore
In-Reply-To: <25045.1560339786@warthog.procyon.org.uk>
On 6/12/19 7:43 AM, David Howells wrote:
> Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
>>> (6) The security attributes of all the objects between the object in (5)
>>> and the object in (4), assuming we work from (5) towards (4) if the
>>> two aren't coincident (WATCH_INFO_RECURSIVE).
>>
>> Does this apply to anything other than mount notifications?
>
> Not at the moment. I'm considering making it such that you can make a watch
> on a keyring get automatically propagated to keys that get added to the
> keyring (and removed upon unlink) - the idea being that there is no 'single
> parent path' concept for a keyring as there is for a directory.
>
> I'm also pondering the idea of making it possible to have superblock watches
> automatically propagated to superblocks created by automount points on the
> watched superblock.
So at the point where you can set a watch on one object O1 with label X,
and receive notifications triggered upon operations on another object O2
with label Y, we have to consider whether the relationship between X and
Y is controlled in any way (possibly transitively through a series of
checks performed earlier) and whether we can reasonably infer that the
authorization to watch X implies the ability to be notified of
operations on Y. Not a problem for the mount notifications AFAICS
because there is only truly one object - the mount namespace itself, and
it is always our own.
>
>> And for mount notifications, isn't the notification actually for a change to
>> the mount namespace, not a change to any file?
>
> Yes.
>
>> Hence, the real "object" for events that trigger mount notifications is the
>> mount namespace, right?
>
> Um... arguably. Would that mean that that would need a label from somewhere?
That takes us into the whole question of whether namespaces should be
labeled (presumably from their creator), and the association between
processes and their namespaces should be controlled. I think when we
originally looked at them, it wasn't much of a concern since the only
means of creating a new namespace and associating with it was via
clone() and then later also via unshare(). /proc/pid/ns and setns()
changed that picture, but still requires ptrace read mode access, which
at least provides some control over entering namespaces created by
others. I suspect that ultimately we want namespaces to be labeled and
controlled but that isn't your problem to solve here.
For your purposes, a process is setting a watch on its own namespace,
and it already inherently can observe changes to that namespace without
needing watches/notifications, and it can modify that namespace iff
privileged wrt to the namespace. One might argue that no check is
required at all for setting the watch, and at most, it would be a check
between the process and its own label to match the checking when
accessing /proc/self/mounts. That presumes that no additional
information is conveyed via the notification that isn't already
available from /proc/self/mounts, particularly any information specific
to the process that triggered the notification. Does that make sense?
>
>> The watched path is just a way of identifying a subtree of the mount
>> namespace for notifications - it isn't the real object being watched.
>
> I like that argument.
>
> Thanks,
> David
>
^ permalink raw reply
* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Stephen Smalley @ 2019-06-13 18:00 UTC (permalink / raw)
To: Xing, Cedric, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-sgx@vger.kernel.org
Cc: jarkko.sakkinen@linux.intel.com, luto@kernel.org,
jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com,
eparis@parisplace.org, jethro@fortanix.com, Hansen, Dave,
tglx@linutronix.de, torvalds@linux-foundation.org,
akpm@linux-foundation.org, nhorman@redhat.com,
pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F65502A85@ORSMSX116.amr.corp.intel.com>
On 6/11/19 6:55 PM, Xing, Cedric wrote:
>> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
>> owner@vger.kernel.org] On Behalf Of Stephen Smalley
>> Sent: Tuesday, June 11, 2019 6:40 AM
>>
>>>
>>> +#ifdef CONFIG_INTEL_SGX
>>> + rc = sgxsec_mprotect(vma, prot);
>>> + if (rc <= 0)
>>> + return rc;
>>
>> Why are you skipping the file_map_prot_check() call when rc == 0?
>> What would SELinux check if you didn't do so -
>> FILE__READ|FILE__WRITE|FILE__EXECUTE to /dev/sgx/enclave? Is it a
>> problem to let SELinux proceed with that check?
>
> We can continue the check. But in practice, all FILE__{READ|WRITE|EXECUTE} are needed for every enclave, then what's the point of checking them? FILE__EXECMOD may be the only flag that has a meaning, but it's kind of redundant because sigstruct file was checked against that already.
I don't believe FILE__EXECMOD will be checked since it is a shared file
mapping. We'll check at least FILE__READ and FILE__WRITE anyway upon
open(), and possibly FILE__EXECUTE upon mmap() unless that is never
PROT_EXEC. We want the policy to accurately reflect the operations of
the system, even when an operation "must" be allowed, and even here this
only needs to be allowed to processes authorized as enclave loaders, not
to all processes.
I don't think there are other examples where we skip a SELinux check
like this. If we were to do so here, we would at least need a comment
explaining that it was intentional and why. The risk would be that
future checking added into file_map_prot_check() would be unwittingly
bypassed for these mappings. A warning there would also be advisable if
we skip it for these mappings.
>
>>> +static int selinux_enclave_load(struct file *encl, unsigned long addr,
>>> + unsigned long size, unsigned long prot,
>>> + struct vm_area_struct *source)
>>> +{
>>> + if (source) {
>>> + /**
>>> + * Adding page from source => EADD request
>>> + */
>>> + int rc = selinux_file_mprotect(source, prot, prot);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + if (!(prot & VM_EXEC) &&
>>> + selinux_file_mprotect(source, VM_EXEC, VM_EXEC))
>>
>> I wouldn't conflate VM_EXEC with PROT_EXEC even if they happen to be
>> defined with the same values currently. Elsewhere the kernel appears to
>> explicitly translate them ala calc_vm_prot_bits().
>
> Thanks! I'd change them to PROT_EXEC in the next version.
>
>>
>> Also, this will mean that we will always perform an execute check on all
>> sources, thereby triggering audit denial messages for any EADD sources
>> that are only intended to be data. Depending on the source, this could
>> trigger PROCESS__EXECMEM or FILE__EXECMOD or FILE__EXECUTE. In a world
>> where users often just run any denials they see through audit2allow,
>> they'll end up always allowing them all. How can they tell whether it
>> was needed? It would be preferable if we could only trigger execute
>> checks when there is some probability that execute will be requested in
>> the future. Alternatives would be to silence the audit of these
>> permission checks always via use of _noaudit() interfaces or to silence
>> audit of these permissions via dontaudit rules in policy, but the latter
>> would hide all denials of the permission by the process, not just those
>> triggered from security_enclave_load(). And if we silence them, then we
>> won't see them even if they were needed.
>
> *_noaudit() is exactly what I wanted. But I couldn't find selinux_file_mprotect_noaudit()/file_has_perm_noaudit(), and I'm reluctant to duplicate code. Any suggestions?
I would have no objection to adding _noaudit() variants of these, either
duplicating code (if sufficiently small/simple) or creating a common
helper with a bool audit flag that gets used for both. But the larger
issue would be to resolve how to ultimately ensure that a denial is
audited later if the denied permission is actually requested and blocked
via sgxsec_mprotect().
>
>>
>>> + prot = 0;
>>> + else {
>>> + prot = SGX__EXECUTE;
>>> + if (source->vm_file &&
>>> + !file_has_perm(current_cred(), source->vm_file,
>>> + FILE__EXECMOD))
>>> + prot |= SGX__EXECMOD;
>>
>> Similarly, this means that we will always perform a FILE__EXECMOD check
>> on all executable sources, triggering audit denial messages for any EADD
>> source that is executable but to which EXECMOD is not allowed, and again
>> the most common pattern will be that users will add EXECMOD to all
>> executable sources to avoid this.
>>
>>> + }
>>> + return sgxsec_eadd(encl, addr, size, prot);
>>> + } else {
>>> + /**
>>> + * Adding page from NULL => EAUG request
>>> + */
>>> + return sgxsec_eaug(encl, addr, size, prot);
>>> + }
>>> +}
>>> +
>>> +static int selinux_enclave_init(struct file *encl,
>>> + const struct sgx_sigstruct *sigstruct,
>>> + struct vm_area_struct *vma)
>>> +{
>>> + int rc = 0;
>>> +
>>> + if (!vma)
>>> + rc = -EINVAL;
>>
>> Is it ever valid to call this hook with a NULL vma? If not, this should
>> be handled/prevented by the caller. If so, I'd just return -EINVAL
>> immediately here.
>
> vma shall never be NULL. I'll update it in the next version.
>
>>
>>> +
>>> + if (!rc && !(vma->vm_flags & VM_EXEC))
>>> + rc = selinux_file_mprotect(vma, VM_EXEC, VM_EXEC);
>>
>> I had thought we were trying to avoid overloading FILE__EXECUTE (or
>> whatever gets checked here, e.g. could be PROCESS__EXECMEM or
>> FILE__EXECMOD) on the sigstruct file, since the caller isn't truly
>> executing code from it.
>
> Agreed. Another problem with FILE__EXECMOD on the sigstruct file is that user code would then be allowed to modify SIGSTRUCT at will, which effectively wipes out the protection provided by FILE__EXECUTE.
>
>>
>> I'd define new ENCLAVE__* permissions, including an up-front
>> ENCLAVE__INIT permission that governs whether the sigstruct file can be
>> used at all irrespective of memory protections.
>
> Agreed.
>
>>
>> Then you can also have ENCLAVE__EXECUTE, ENCLAVE__EXECMEM,
>> ENCLAVE__EXECMOD for the execute-related checks. Or you can use the
>> /dev/sgx/enclave inode as the target for the execute checks and just
>> reuse the file permissions there.
>
> Now we've got 2 options - 1) New ENCLAVE__* flags on sigstruct file or 2) FILE__* on /dev/sgx/enclave. Which one do you think makes more sense?
>
> ENCLAVE__EXECMEM seems to offer finer granularity (than PROCESS__EXECMEM) but I wonder if it'd have any real use in practice.
Defining a separate ENCLAVE__EXECUTE and using it here for the sigstruct
file would avoid any ambiguity with the FILE__EXECUTE check to the
/dev/sgx/enclave inode that might occur upon mmap() or mprotect(). A
separate ENCLAVE__EXECMEM would enable allowing WX within the enclave
while denying it in the host application or vice versa, which could be a
good thing for security, particularly if SGX2 largely ends up always
wanting WX.
>
>>> +int sgxsec_mprotect(struct vm_area_struct *vma, size_t prot) {
>>> + struct enclave_sec *esec;
>>> + int rc;
>>> +
>>> + if (!vma->vm_file || !(esec = __esec(selinux_file(vma->vm_file))))
>> {
>>> + /* Positive return value indicates non-enclave VMA */
>>> + return 1;
>>> + }
>>> +
>>> + down_read(&esec->sem);
>>> + rc = enclave_mprotect(&esec->regions, vma->vm_start, vma->vm_end,
>>> +prot);
>>
>> Why is it safe for this to only use down_read()? enclave_mprotect() can
>> call enclave_prot_set_cb() which modifies the list?
>
> Probably because it was too late at night when I wrote this line:-( Good catch!
>
>>
>> I haven't looked at this code closely, but it feels like a lot of SGX-
>> specific logic embedded into SELinux that will have to be repeated or
>> reused for every security module. Does SGX not track this state itself?
>
> I can tell you have looked quite closely, and I truly think you for your time!
>
> You are right that there are SGX specific stuff. More precisely, SGX enclaves don't have access to anything except memory, so there are only 3 questions that need to be answered for each enclave page: 1) whether X is allowed; 2) whether W->X is allowed and 3 whether WX is allowed. This proposal tries to cache the answers to those questions upon creation of each enclave page, meaning it involves a) figuring out the answers and b) "remember" them for every page. #b is generic, mostly captured in intel_sgx.c, and could be shared among all LSM modules; while #a is SELinux specific. I could move intel_sgx.c up one level in the directory hierarchy if that's what you'd suggest.
>
> By "SGX", did you mean the SGX subsystem being upstreamed? It doesn’t track that state. In practice, there's no way for SGX to track it because there's no vm_ops->may_mprotect() callback. It doesn't follow the philosophy of Linux either, as mprotect() doesn't track it for regular memory. And it doesn't have a use without LSM, so I believe it makes more sense to track it inside LSM.
Yes, the SGX driver/subsystem. I had the impression from Sean that it
does track this kind of per-page state already in some manner, but
possibly he means it does under a given proposal and not in the current
driver.
Even the #b remembering might end up being SELinux-specific if we also
have to remember the original inputs used to compute the answer so that
we can audit that information when access is denied later upon
mprotect(). At the least we'd need it to save some opaque data and pass
it to a callback into SELinux to perform that auditing.
^ permalink raw reply
* Re: [RFC PATCH 2/9] x86/sgx: Do not naturally align MAP_FIXED address
From: Sean Christopherson @ 2019-06-13 17:14 UTC (permalink / raw)
To: Xing, Cedric
Cc: Jarkko Sakkinen, Andy Lutomirski, Andy Lutomirski,
Stephen Smalley, James Morris, Serge E . Hallyn, LSM List,
Paul Moore, Eric Paris, selinux@vger.kernel.org, Jethro Beekman,
Hansen, Dave, Thomas Gleixner, Linus Torvalds, LKML, X86 ML,
linux-sgx@vger.kernel.org, Andrew Morton, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
Huang, Haitao, Andy Shevchenko, Svahn, Kai, Borislav Petkov,
Josh Triplett, Huang, Kai, David Rientjes, Roberts, William C,
Tricca, Philip B
In-Reply-To: <960B34DE67B9E140824F1DCDEC400C0F65503A93@ORSMSX116.amr.corp.intel.com>
On Thu, Jun 13, 2019 at 09:47:06AM -0700, Xing, Cedric wrote:
> > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com]
> > Sent: Thursday, June 13, 2019 6:48 AM
> >
> > On Thu, Jun 06, 2019 at 06:37:10PM +0300, Jarkko Sakkinen wrote:
> > > On Wed, Jun 05, 2019 at 01:14:04PM -0700, Andy Lutomirski wrote:
> > > >
> > > >
> > > > > On Jun 5, 2019, at 8:17 AM, Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > >
> > > > >> On Tue, Jun 04, 2019 at 10:10:22PM +0000, Xing, Cedric wrote:
> > > > >> A bit off topic here. This mmap()/mprotect() discussion reminds
> > > > >> me a question (guess for Jarkko): Now that
> > > > >> vma->vm_file->private_data keeps a pointer to the enclave, why do
> > we store it again in vma->vm_private?
> > > > >> It isn't a big deal but non-NULL vm_private does prevent
> > > > >> mprotect() from merging adjacent VMAs.
> > > > >
> > > > > Same semantics as with a regular mmap i.e. you can close the file
> > > > > and still use the mapping.
> > > > >
> > > > >
> > > >
> > > > The file should be properly refcounted — vm_file should not go away
> > while it’s mapped.
> >
> > mm already takes care of that so vm_file does not go away. Still we need
> > an internal refcount for enclaves to synchronize with the swapper. In
> > summary nothing needs to be done.
>
> I don't get this. The swapper takes a read lock on mm->mmap_sem, which locks
> the vma, which in turn reference counts vma->vm_file. Why is the internal
> refcount still needed?
mmap_sem is only held when reclaim is touching PTEs, e.g. to test/clear
its accessed bit and to zap the PTE. The liveliness of the enclave needs
to be guaranteed for the entire duration of reclaim, e.g. we can't have
the enclave disappearing when we go to do EWB. It's also worth nothing
that a single reclaim may operate on more than one mmap_sem, as enclaves
can be shared across processes (mm_structs).
^ permalink raw reply
* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Sean Christopherson @ 2019-06-12 22:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Smalley, Cedric Xing, LSM List, selinux, LKML, linux-sgx,
Jarkko Sakkinen, James Morris, Serge E. Hallyn, Paul Moore,
Eric Paris, Jethro Beekman, Dave Hansen, Thomas Gleixner,
Linus Torvalds, Andrew Morton, nhorman, pmccallum, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Svahn, Kai,
Borislav Petkov, Josh Triplett, Huang, Kai, David Rientjes,
Roberts, William C, Philip Tricca
In-Reply-To: <CALCETrWQT3AG+-OKBOzuw-a6VPApkNYsKqZiBmS56-b-72bfYQ@mail.gmail.com>
On Wed, Jun 12, 2019 at 12:30:20PM -0700, Andy Lutomirski wrote:
> On Tue, Jun 11, 2019 at 3:02 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > 1. Require userspace to explicitly specificy (maximal) enclave page
> > permissions at build time. The enclave page permissions are provided
> > to, and checked by, LSMs at enclave build time.
> >
> > Pros: Low-complexity kernel implementation, straightforward auditing
> > Cons: Sullies the SGX UAPI to some extent, may increase complexity of
> > SGX2 enclave loaders.
>
> In my notes, this works like this. This is similar, but not
> identical, to what Sean has been sending out.
...
> mmap() and mprotect() enforce the following rules:
>
> - Deny if a PROT_ flag is requested but the corresponding ALLOW_ flag
> is not set for all pages in question.
>
> - Deny if PROT_WRITE, PROT_EXEC, and DENY_WX are all set.
>
> - Deny if PROT_EXEC, ALLOW_WRITE, and DENY_X_IF_ALLOW_WRITE are all set.
>
> mprotect() and mmap() do *not* call SGX-specific LSM hooks to ask for
> permission, although they can optionally call an LSM hook if they hit one of
> the -EPERM cases for auditing purposes.
IMO, #1 only makes sense if it's stripped down to avoid auditing and
locking complications, i.e. gets a pass/fail at security_enclave_load()
and clears VM_MAY* flags during mmap(). If we want WX and W->X to be
differentiated by security_enclave_init() as opposed to
security_enclave_load(), then we should just scrap #1.
> I think this model works quite well in an SGX1 world. The main thing
> that makes me uneasy about this model is that, in SGX2, it requires
> that an SGX2-compatible enclave loader must pre-declare to the kernel
> whether it intends for its dynamically allocated memory to be
> ALLOW_EXEC. If ALLOW_EXEC is set but not actually needed, it will
> still fail if DENY_X_IF_ALLOW_WRITE ends up being set. The other
> version below does not have this limitation.
I'm not convinced this will be a meaningful limitation in practice, though
that's probably obvious from my RFCs :-). That being said, the UAPI quirk
is essentially a dealbreaker for multiple people, so let's drop #1.
I discussed the options with Cedric offline, and he is ok with option #2
*if* the idea actually translates to acceptable code and doesn't present
problems for userspace and/or future SGX features.
So, I'll work on an RFC series to implement #2 as described below. If it
works out, yay! If not, i.e. option #2 is fundamentally broken, I'll
shift my focus to Cedric's code (option #3).
> > 2. Pre-check LSM permissions and dynamically track mappings to enclave
> > pages, e.g. add an SGX mprotect() hook to restrict W->X and WX
> > based on the pre-checked permissions.
> >
> > Pros: Does not impact SGX UAPI, medium kernel complexity
> > Cons: Auditing is complex/weird, requires taking enclave-specific
> > lock during mprotect() to query/update tracking.
>
> Here's how this looks in my mind. It's quite similar, except that
> ALLOW_READ, ALLOW_WRITE, and ALLOW_EXEC are replaced with a little
> state machine.
>
> EADD does not take any special flags. It calls this LSM hook:
>
> int security_enclave_load(struct vm_area_struct *source);
>
> This hook can return -EPERM. Otherwise it 0 or ALLOC_EXEC_IF_UNMODIFIED
> (i.e. 1). This hook enforces permissions (a) and (b).
>
> The driver tracks a state for each page, and the possible states are:
>
> - CLEAN_MAYEXEC /* no W or X VMAs have existed, but X is okay */
> - CLEAN_NOEXEC /* no W or X VMAs have existed, and X is not okay */
> - CLEAN_EXEC /* no W VMA has existed, but an X VMA has existed */
> - DIRTY /* a W VMA has existed */
>
> The initial state for a page is CLEAN_MAYEXEC if the hook said
> ALLOW_EXEC_IF_UNMODIFIED and CLEAN_NOEXEC otherwise.
>
> The future EAUG does not call a hook at all and puts pages into the state
> CLEAN_NOEXEC. If SGX3 or later ever adds EAUG-but-don't-clear, it can
> call security_enclave_load() and add CLEAN_MAYEXEC pages if appropriate.
>
> EINIT takes a sigstruct pointer. SGX calls a new hook:
>
> unsigned int security_enclave_init(struct sigstruct *sigstruct,
> struct vm_area_struct *source, unsigned int flags);
>
> This hook can return -EPERM. Otherwise it returns 0 or a combination of
> flags DENY_WX and DENY_X_DIRTY. The driver saves this value.
> These represent permissions (c) and (d).
>
> If we want to have a permission for "execute code supplied from outside the
> enclave that was not measured", we could have a flag like
> HAS_UNMEASURED_CLEAN_EXEC_PAGE that the LSM could consider.
>
> mmap() and mprotect() enforce the following rules:
>
> - If VM_EXEC is requested and (either the page is DIRTY or VM_WRITE is
> requested) and DENY_X_DIRTY, then deny.
>
> - If VM_WRITE and VM_EXEC are both requested and DENY_WX, then deny.
>
> - If VM_WRITE is requested, we need to update the state. If it was
> CLEAN_EXEC, then we reject if DENY_X_DIRTY. Otherwise we change the
> state to DIRTY.
>
> - If VM_EXEC is requested and the page is CLEAN_NOEXEC, then deny.
>
> mprotect() and mmap() do *not* call SGX-specific LSM hooks to ask for
> permission, although they can optionally call an LSM hook if they hit one of
> the -EPERM cases for auditing purposes.
>
> Before the SIGSTRUCT is provided to the driver, the driver acts as though
> DENY_X_DIRTY and DENY_WX are both set.
^ permalink raw reply
* [PATCH V8 0/3] Add support for measuring the boot command line during kexec_file_load
From: Prakhar Srivastava @ 2019-06-12 22:15 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
The kexec boot command line arguments are not currently being
measured.
Currently during soft reboot(kexec)
- the PCRS are not reset
- the command line arguments used for the next kernel are not measured.
This gives the impression to the secure boot attestation that a cold boot took
place.
For secure boot attestation, it is necessary to measure the kernel
command line. For cold boot, the boot loader can be enhanced to measure
these parameters.
(https://mjg59.dreamwidth.org/48897.html)
This patch set aims to address measuring the boot command line during
soft reboot(kexec_file_load).
To achive the above the patch series does the following
-Add a new ima hook: ima_kexec_cmdline which measures the cmdline args
into the ima log, behind a new ima policy entry KEXEC_CMDLINE.
The kexec cmdline hash is stored in the "d-ng" field of the template data.
-Since the cmldine args cannot be appraised, a new template field(buf) is
added. The template field contains the buffer passed(cmldine args), which
can be used to appraise/attest at a later stage.
The kexec cmdline buffer is stored as HEX in the buf field of the event_data.
-Call the ima_kexec_cmdline(...) hook from kexec_file_load call.
The ima logs need to be carried over to the next kernel, which will be followed
up by other patchsets for x86_64 and arm64.
The kexec cmdline hash is stored in the "d-ng" field of the template data.
and can be verified using
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements |
grep kexec-cmdline | cut -d' ' -f 6 | xxd -r -p | sha256sum
Changelog:
V8(since V7):
- added a new ima template name "ima-buf"
- code cleanup
V7:
- rebased to next-queued-testing
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-queued-testing
V6:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
add new fields to ima_event_data to store/read buffer data.
[suggested by Roberto]
-call ima_kexec_cmdline from kexec_file_load path
v5:
-add a new ima hook and policy to measure the cmdline
args(ima_kexec_cmdline)
-add a new template field buf to contain the buffer measured.
[suggested by Mimi Zohar]
-call ima_kexec_cmdline from kexec_file_load path
v4:
- per feedback from LSM community, removed the LSM hook and renamed the
IMA policy to KEXEC_CMDLINE
v3: (rebase changes to next-general)
- Add policy checks for buffer[suggested by Mimi Zohar]
- use the IMA_XATTR to add buffer
- Add kexec_cmdline used for kexec file load
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
v2:
- Add policy checks for buffer[suggested by Mimi Zohar]
- Add an LSM hook to allow usage by other LSM.[suggestd by Mimi Zohar]
- use the IMA_XATTR to add buffer instead of sig template
v1:
-Add kconfigs to control the ima_buffer_check
-measure the cmdline args suffixed with the kernel file name
-add the buffer to the template sig field.
Prakhar Srivastava (3):
Add a new ima hook ima_kexec_cmdline to measure cmdline args
add a new ima template field buf
call ima_kexec_cmdline to measure the cmdline args
Documentation/ABI/testing/ima_policy | 1 +
Documentation/security/IMA-templates.rst | 2 +-
include/linux/ima.h | 2 +
kernel/kexec_file.c | 8 ++-
security/integrity/ima/ima.h | 3 +
security/integrity/ima/ima_api.c | 5 +-
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 80 +++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 9 +++
security/integrity/ima/ima_template.c | 2 +
security/integrity/ima/ima_template_lib.c | 20 ++++++
security/integrity/ima/ima_template_lib.h | 4 ++
12 files changed, 131 insertions(+), 7 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH V8 2/3] Define a new ima template field buf
From: Prakhar Srivastava @ 2019-06-12 22:15 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190612221549.28399-1-prsriva02@gmail.com>
A buffer(kexec cmdline args) measured into ima cannot be
appraised without already being aware of the buffer contents.
Since hashes are non-reversible, raw buffer is needed for
validation or regenerating hash for appraisal/attestation.
This patch adds support to ima to allow store/read the
buffer contents in HEX.
- Add two new fields to ima_event_data to hold the buf and
buf_len [Suggested by Roberto]
- Add a new temaplte field 'buf' to be used to store/read
the buffer data.[Suggested by Mimi]
- Updated process_buffer_meaurement to add the buffer to
ima_event_data. process_buffer_measurement added in
"Define a new IMA hook to measure the boot command line
arguments"
- Add a new template policy name ima-buf to represent
'd-ng|n-ng|buf'
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
---
Documentation/security/IMA-templates.rst | 7 ++++---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_api.c | 4 ++--
security/integrity/ima/ima_init.c | 2 +-
security/integrity/ima/ima_main.c | 2 ++
security/integrity/ima/ima_template.c | 3 +++
security/integrity/ima/ima_template_lib.c | 21 +++++++++++++++++++++
security/integrity/ima/ima_template_lib.h | 4 ++++
8 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..fccdbbc984f5 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -69,14 +69,15 @@ descriptors by adding their identifier to the format string
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
- 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature.
-
+ - 'sig': the file signature;
+ - 'buf': the buffer data that was used to generate the hash without size limitations;
Below, there is the list of defined template descriptors:
- "ima": its format is ``d|n``;
- "ima-ng" (default): its format is ``d-ng|n-ng``;
- - "ima-sig": its format is ``d-ng|n-ng|sig``.
+ - "ima-sig": its format is ``d-ng|n-ng|sig``;
+ - "ima-buf": its format is ``d-ng|n-ng|buf``;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a4ad1270bffa..16110180545c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -65,6 +65,8 @@ struct ima_event_data {
struct evm_ima_xattr_data *xattr_value;
int xattr_len;
const char *violation;
+ const void *buf;
+ int buf_len;
};
/* IMA template field data definition */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index ea7d8cbf712f..83ca99d65e4b 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -140,7 +140,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
struct ima_template_entry *entry;
struct inode *inode = file_inode(file);
struct ima_event_data event_data = {iint, file, filename, NULL, 0,
- cause};
+ cause, NULL, 0};
int violation = 1;
int result;
@@ -296,7 +296,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
struct inode *inode = file_inode(file);
struct ima_template_entry *entry;
struct ima_event_data event_data = {iint, file, filename, xattr_value,
- xattr_len, NULL};
+ xattr_len, NULL, NULL, 0};
int violation = 0;
if (iint->measured_pcrs & (0x1 << pcr))
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 993d0f1915ff..c8591406c0e2 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -50,7 +50,7 @@ static int __init ima_add_boot_aggregate(void)
struct ima_template_entry *entry;
struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
struct ima_event_data event_data = {iint, NULL, boot_aggregate_name,
- NULL, 0, NULL};
+ NULL, 0, NULL, NULL, 0};
int result = -ENOMEM;
int violation = 0;
struct {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d78b15a0bd44..720151b766fa 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -641,6 +641,8 @@ static void process_buffer_measurement(const void *buf, int size,
memset(&hash, 0, sizeof(hash));
event_data.filename = eventname;
+ event_data.buf = buf;
+ event_data.buf_len = size;
iint->ima_hash = &hash.hdr;
iint->ima_hash->algo = ima_hash_algo;
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e6e892f31cbd..632f314c0e5a 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -26,6 +26,7 @@ static struct ima_template_desc builtin_templates[] = {
{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
{.name = "ima-ng", .fmt = "d-ng|n-ng"},
{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
+ {.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
{.name = "", .fmt = ""}, /* placeholder for a custom format */
};
@@ -43,6 +44,8 @@ static const struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "buf", .field_init = ima_eventbuf_init,
+ .field_show = ima_show_template_buf},
};
#define MAX_TEMPLATE_NAME_LEN 15
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 513b457ae900..baf4de45c5aa 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -162,6 +162,12 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
}
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data)
+{
+ ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data);
+}
+
/**
* ima_parse_buf() - Parses lengths and data from an input buffer
* @bufstartp: Buffer start address.
@@ -389,3 +395,18 @@ int ima_eventsig_init(struct ima_event_data *event_data,
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
DATA_FMT_HEX, field_data);
}
+
+/*
+ * ima_eventbuf_init - include the buffer(kexec-cmldine) as part of the
+ * template data.
+ */
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ if ((!event_data->buf) || (event_data->buf_len == 0))
+ return 0;
+
+ return ima_write_template_field_data(event_data->buf,
+ event_data->buf_len, DATA_FMT_HEX,
+ field_data);
+}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..f0178bc60c55 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -29,6 +29,8 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
struct ima_field_data *field_data);
+void ima_show_template_buf(struct seq_file *m, enum ima_show_type show,
+ struct ima_field_data *field_data);
int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
int maxfields, struct ima_field_data *fields, int *curfields,
unsigned long *len_mask, int enforce_mask, char *bufname);
@@ -42,4 +44,6 @@ int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventbuf_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
#endif /* __LINUX_IMA_TEMPLATE_LIB_H */
--
2.19.1
^ permalink raw reply related
* [PATCH V8 1/3] Define a new IMA hook to measure the boot command line arguments
From: Prakhar Srivastava @ 2019-06-12 22:15 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190612221549.28399-1-prsriva02@gmail.com>
This patch adds support in ima to measure kexec cmdline args
during soft reboot(kexec_file_load).
- A new ima hook ima_kexec_cmdline is defined to be called by the
kexec code.
- A new function process_buffer_measurement is defined to measure
the buffer hash into the ima log.
- A new func policy KEXEC_CMDLINE is defined to control the
measurement.[Suggested by Mimi]
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
Documentation/ABI/testing/ima_policy | 1 +
include/linux/ima.h | 2 +
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 1 +
security/integrity/ima/ima_main.c | 77 ++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 9 ++++
6 files changed, 91 insertions(+)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index b383c1763610..fc376a323908 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -28,6 +28,7 @@ Description:
base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
+ [KEXEC_CMDLINE]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
diff --git a/include/linux/ima.h b/include/linux/ima.h
index fd9f7cf4cdf5..b42f5a006042 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -26,6 +26,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
enum kernel_read_file_id id);
extern void ima_post_path_mknod(struct dentry *dentry);
+extern void ima_kexec_cmdline(const void *buf, int size);
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
@@ -92,6 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
return;
}
+static inline void ima_kexec_cmdline(const void *buf, int size) {}
#endif /* CONFIG_IMA */
#ifndef CONFIG_IMA_KEXEC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 18b48a6d0b80..a4ad1270bffa 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -185,6 +185,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
hook(KEXEC_KERNEL_CHECK) \
hook(KEXEC_INITRAMFS_CHECK) \
hook(POLICY_CHECK) \
+ hook(KEXEC_CMDLINE) \
hook(MAX_CHECK)
#define __ima_hook_enumify(ENUM) ENUM,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 78eb11c7ac07..ea7d8cbf712f 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -176,6 +176,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
* func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
+ * | KEXEC_CMDLINE
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index af341a80118f..d78b15a0bd44 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -605,6 +605,83 @@ int ima_load_data(enum kernel_load_data_id id)
return 0;
}
+/*
+ * process_buffer_measurement - Measure the buffer to ima log.
+ * @buf: pointer to the buffer that needs to be added to the log.
+ * @size: size of buffer(in bytes).
+ * @eventname: event name to be used for the buffer entry.
+ * @cred: a pointer to a credentials structure for user validation.
+ * @secid: the secid of the task to be validated.
+ *
+ * Based on policy, the buffer is measured into the ima log.
+ */
+static void process_buffer_measurement(const void *buf, int size,
+ const char *eventname,
+ const struct cred *cred, u32 secid)
+{
+ int ret = 0;
+ struct ima_template_entry *entry = NULL;
+ struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
+ struct ima_event_data event_data = {.iint = iint };
+ struct ima_template_desc *template_desc = NULL;
+ struct {
+ struct ima_digest_data hdr;
+ char digest[IMA_MAX_DIGEST_SIZE];
+ } hash;
+ int violation = 0;
+ int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+ int action = 0;
+
+ action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr,
+ &template_desc);
+ if (!(action & IMA_MEASURE))
+ goto out;
+
+ memset(iint, 0, sizeof(*iint));
+ memset(&hash, 0, sizeof(hash));
+
+ event_data.filename = eventname;
+
+ iint->ima_hash = &hash.hdr;
+ iint->ima_hash->algo = ima_hash_algo;
+ iint->ima_hash->length = hash_digest_size[ima_hash_algo];
+
+ ret = ima_calc_buffer_hash(buf, size, iint->ima_hash);
+ if (ret < 0)
+ goto out;
+
+ ret = ima_alloc_init_template(&event_data, &entry, template_desc);
+ if (ret < 0)
+ goto out;
+
+ if (action & IMA_MEASURE)
+ ret = ima_store_template(entry, violation, NULL, buf, pcr);
+
+ if (ret < 0)
+ ima_free_template_entry(entry);
+
+out:
+ return;
+}
+
+/**
+ * ima_kexec_cmdline - measure kexec cmdline boot args
+ * @buf: pointer to buffer
+ * @size: size of buffer
+ *
+ * Buffers can only be measured, not appraised.
+ */
+void ima_kexec_cmdline(const void *buf, int size)
+{
+ u32 secid;
+
+ if (buf && size != 0) {
+ security_task_getsecid(current, &secid);
+ process_buffer_measurement(buf, size, "kexec-cmdline",
+ current_cred(), secid);
+ }
+}
+
static int __init init_ima(void)
{
int error;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fd9b01881d17..98e351e13557 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -292,6 +292,13 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
+ /* only incase of KEXEC_CMDLINE, inode is NULL */
+ if (func == KEXEC_CMDLINE) {
+ if ((rule->flags & IMA_FUNC) &&
+ (rule->func == func) && (!inode))
+ return true;
+ return false;
+ }
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
return false;
@@ -880,6 +887,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = KEXEC_INITRAMFS_CHECK;
else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
entry->func = POLICY_CHECK;
+ else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
+ entry->func = KEXEC_CMDLINE;
else
result = -EINVAL;
if (!result)
--
2.19.1
^ permalink raw reply related
* [PATCH V8 3/3] Call ima_kexec_cmdline to measure the cmdline args
From: Prakhar Srivastava @ 2019-06-12 22:15 UTC (permalink / raw)
To: linux-integrity, linux-security-module, linux-kernel
Cc: zohar, roberto.sassu, vgoyal, Prakhar Srivastava
In-Reply-To: <20190612221549.28399-1-prsriva02@gmail.com>
During soft reboot(kexec_file_load) boot cmdline args
are not measured.Thus the new kernel on load boots with
an assumption of cold reboot.
This patch makes a call to the ima hook ima_kexec_cmdline,
added in "Define a new IMA hook to measure the boot command
line arguments"
to measure the boot cmdline args into the ima log.
- call ima_kexec_cmdline from kexec_file_load.
- move the call ima_add_kexec_buffer after the cmdline
args have been measured.
Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
---
kernel/kexec_file.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 072b6ee55e3f..b0c724e5d86c 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -198,9 +198,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
return ret;
image->kernel_buf_len = size;
- /* IMA needs to pass the measurement list to the next kernel. */
- ima_add_kexec_buffer(image);
-
/* Call arch image probe handlers */
ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
image->kernel_buf_len);
@@ -241,8 +238,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
ret = -EINVAL;
goto out;
}
+
+ ima_kexec_cmdline(image->cmdline_buf,
+ image->cmdline_buf_len - 1);
}
+ /* IMA needs to pass the measurement list to the next kernel. */
+ ima_add_kexec_buffer(image);
+
/* Call arch image load handlers */
ldata = arch_kexec_kernel_image_load(image);
--
2.19.1
^ permalink raw reply related
* Re: [PATCH V8 3/3] Call ima_kexec_cmdline to measure the cmdline args
From: Mimi Zohar @ 2019-06-12 22:31 UTC (permalink / raw)
To: Prakhar Srivastava, linux-integrity, linux-security-module,
linux-kernel
Cc: roberto.sassu, vgoyal, Eric W. Biederman, Dave Young, kexec
In-Reply-To: <20190612221549.28399-4-prsriva02@gmail.com>
[Cc: kexec mailing list]
Hi Eric, Dave,
On Wed, 2019-06-12 at 15:15 -0700, Prakhar Srivastava wrote:
> During soft reboot(kexec_file_load) boot cmdline args
> are not measured.Thus the new kernel on load boots with
> an assumption of cold reboot.
>
> This patch makes a call to the ima hook ima_kexec_cmdline,
> added in "Define a new IMA hook to measure the boot command
> line arguments"
> to measure the boot cmdline args into the ima log.
>
> - call ima_kexec_cmdline from kexec_file_load.
> - move the call ima_add_kexec_buffer after the cmdline
> args have been measured.
>
> Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dave Young <dyoung@redhat.com>
Any chance we could get some Acks?
thanks,
Mimi
> ---
> kernel/kexec_file.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 072b6ee55e3f..b0c724e5d86c 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -198,9 +198,6 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> return ret;
> image->kernel_buf_len = size;
>
> - /* IMA needs to pass the measurement list to the next kernel. */
> - ima_add_kexec_buffer(image);
> -
> /* Call arch image probe handlers */
> ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
> image->kernel_buf_len);
> @@ -241,8 +238,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> ret = -EINVAL;
> goto out;
> }
> +
> + ima_kexec_cmdline(image->cmdline_buf,
> + image->cmdline_buf_len - 1);
> }
>
> + /* IMA needs to pass the measurement list to the next kernel. */
> + ima_add_kexec_buffer(image);
> +
> /* Call arch image load handlers */
> ldata = arch_kexec_kernel_image_load(image);
>
^ permalink raw reply
* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Stephen Smalley @ 2019-06-13 17:02 UTC (permalink / raw)
To: Sean Christopherson
Cc: Cedric Xing, linux-security-module, selinux, linux-kernel,
linux-sgx, jarkko.sakkinen, luto, jmorris, serge, paul, eparis,
jethro, dave.hansen, tglx, torvalds, akpm, nhorman, pmccallum,
serge.ayoun, shay.katz-zamir, haitao.huang, andriy.shevchenko,
kai.svahn, bp, josh, kai.huang, rientjes, william.c.roberts,
philip.b.tricca
In-Reply-To: <20190611220243.GB3416@linux.intel.com>
On 6/11/19 6:02 PM, Sean Christopherson wrote:
> On Tue, Jun 11, 2019 at 09:40:25AM -0400, Stephen Smalley wrote:
>> I haven't looked at this code closely, but it feels like a lot of
>> SGX-specific logic embedded into SELinux that will have to be repeated or
>> reused for every security module. Does SGX not track this state itself?
>
> SGX does track equivalent state.
>
> There are three proposals on the table (I think):
>
> 1. Require userspace to explicitly specificy (maximal) enclave page
> permissions at build time. The enclave page permissions are provided
> to, and checked by, LSMs at enclave build time.
>
> Pros: Low-complexity kernel implementation, straightforward auditing
> Cons: Sullies the SGX UAPI to some extent, may increase complexity of
> SGX2 enclave loaders.
>
> 2. Pre-check LSM permissions and dynamically track mappings to enclave
> pages, e.g. add an SGX mprotect() hook to restrict W->X and WX
> based on the pre-checked permissions.
>
> Pros: Does not impact SGX UAPI, medium kernel complexity
> Cons: Auditing is complex/weird, requires taking enclave-specific
> lock during mprotect() to query/update tracking.
>
> 3. Implement LSM hooks in SGX to allow LSMs to track enclave regions
> from cradle to grave, but otherwise defer everything to LSMs.
>
> Pros: Does not impact SGX UAPI, maximum flexibility, precise auditing
> Cons: Most complex and "heaviest" kernel implementation of the three,
> pushes more SGX details into LSMs.
>
> My RFC series[1] implements #1. My understanding is that Andy (Lutomirski)
> prefers #2. Cedric's RFC series implements #3.
>
> Perhaps the easiest way to make forward progress is to rule out the
> options we absolutely *don't* want by focusing on the potentially blocking
> issue with each option:
>
> #1 - SGX UAPI funkiness
>
> #2 - Auditing complexity, potential enclave lock contention
>
> #3 - Pushing SGX details into LSMs and complexity of kernel implementation
>
>
> [1] https://lkml.kernel.org/r/20190606021145.12604-1-sean.j.christopherson@intel.com
Given the complexity tradeoff, what is the clear motivating example for
why #1 isn't the obvious choice? That the enclave loader has no way of
knowing a priori whether the enclave will require W->X or WX? But
aren't we better off requiring enclaves to be explicitly marked as
needing such so that we can make a more informed decision about whether
to load them in the first place?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox