| Message ID | 20190617222438.2080-10-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | security: x86/sgx: SGX vs. LSM, round 3 | expand |
On Mon, 2019-06-17 at 15:24 -0700, Sean Christopherson wrote: > enclave_load() is roughly analogous to the existing file_mprotect(). > > Due to the nature of SGX and its Enclave Page Cache (EPC), all enclave > VMAs are backed by a single file, i.e. /dev/sgx/enclave, that must be > MAP_SHARED. Furthermore, all enclaves need read, write and execute > VMAs. As a result, the existing/standard call to file_mprotect() does > not provide any meaningful security for enclaves since an LSM can only > deny/grant access to the EPC as a whole. > > security_enclave_load() is called when SGX is first loading an enclave > page, i.e. copying a page from normal memory into the EPC. Although > the prototype for enclave_load() is similar to file_mprotect(), e.g. > SGX could theoretically use file_mprotect() and set reqprot=prot, a > separate hook is desirable as the semantics of an enclave's protection > bits are different than those of vmas, e.g. an enclave page tracks the > maximal set of protections, whereas file_mprotect() operates on the > actual protections being provided. Enclaves also have unique security > properties, e.g. measured code, that LSMs may want to consider. In > other words, LSMs will likely want to implement different policies for > enclave page protections. > > Note, extensive discussion yielded no sane alternative to some form of > SGX specific LSM hook[1]. > > [1] > https://lkml.kernel.org/r/CALCETrXf8mSK45h7sTK5Wf+pXLVn=Bjsc_RLpgO-h-qdzBRo5Q@mail.gmail.com > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Can LSM callbacks ever non-generic when it comes to hardware? This is the very first time I ever see such callbacks being introduced. I suspect that from maintainers perspective, accepting such changes for Intel hardware, could open a pandoras box. /Jarkko
On Wed, 19 Jun 2019, Jarkko Sakkinen wrote: > Can LSM callbacks ever non-generic when it comes to hardware? This is > the very first time I ever see such callbacks being introduced. > > I suspect that from maintainers perspective, accepting such changes for > Intel hardware, could open a pandoras box. If there's a major distro/userbase committing to ship with these hooks enabled via a supported in-tree LSM, the case for inclusion is clear. If the hooks could be generalized beyond just SGX, that would be ideal, but it's not clear if that's feasible.
On Thu, Jun 20, 2019 at 07:13:50AM +1000, James Morris wrote: Good morning, I hope the week is going well for everyone. > On Wed, 19 Jun 2019, Jarkko Sakkinen wrote: > > > Can LSM callbacks ever non-generic when it comes to hardware? This is > > the very first time I ever see such callbacks being introduced. > > > > I suspect that from maintainers perspective, accepting such changes for > > Intel hardware, could open a pandoras box. > If there's a major distro/userbase committing to ship with these > hooks enabled via a supported in-tree LSM, the case for inclusion is > clear. We've been waiting for this concern over SGX specific LSM functionality to eventuate for the last month or so of this discussion. It would seem that mainstream acceptance of SGX specific LSM modifications is complicated by the fact, as we noted in a previous e-mail, that a 1400+ machine SAAS implementation we have experience with will only ever be run with selinux=0. Hence our concerns and continued comments regarding the need to strike the proper balance between implementation complexity and the actual effective security guarantees that are being achieved. > If the hooks could be generalized beyond just SGX, that would be > ideal, but it's not clear if that's feasible. We have been working to develop some thoughts on this issue. We will forward those thoughts along after I get somewhere different from where I am right now. > James Morris Have a good day. Dr. Greg As always, Dr. Greg Wettstein, Ph.D, Worker IDfusion, LLC 4206 N. 19th Ave. Implementing measured information privacy Fargo, ND 58102 and integrity architectures. PH: 701-281-1686 FAX: 701-281-3949 EMAIL: gw@idfusion.org ------------------------------------------------------------------------------ "Can't they? A 64bit number incremented every millisecond can grow for half a billion years. As far as I'm concerned, that is forever." -- Neil Brown linux-raid
On Thu, Jun 20, 2019 at 07:13:50AM +1000, James Morris wrote: > On Wed, 19 Jun 2019, Jarkko Sakkinen wrote: > > > Can LSM callbacks ever non-generic when it comes to hardware? This is > > the very first time I ever see such callbacks being introduced. > > > > I suspect that from maintainers perspective, accepting such changes for > > Intel hardware, could open a pandoras box. > > If there's a major distro/userbase committing to ship with these hooks > enabled via a supported in-tree LSM, the case for inclusion is clear. I think there is. > If the hooks could be generalized beyond just SGX, that would be ideal, > but it's not clear if that's feasible. OK, thanks for responding. This was really important to know what to focus on (and what not). /Jarkko
On Thu, Jun 20, 2019 at 07:13:50AM +1000, James Morris wrote: Good morning, I hope the weekend has been going well for everyone. > On Wed, 19 Jun 2019, Jarkko Sakkinen wrote: > > > Can LSM callbacks ever non-generic when it comes to hardware? This is > > the very first time I ever see such callbacks being introduced. > > > > I suspect that from maintainers perspective, accepting such changes for > > Intel hardware, could open a pandoras box. > If there's a major distro/userbase committing to ship with these > hooks enabled via a supported in-tree LSM, the case for inclusion is > clear. I see that Jarkko responded down thread that there may be a major distribution already committed to SGX specific LSM hooks. My apologies for providing these reflections if that is the case and there is some type of 'deal' in place with respect to all of this. > If the hooks could be generalized beyond just SGX, that would be > ideal, but it's not clear if that's feasible. We believe there is some degree of commonality that can be addressed with respect to implementing LSM enforcement over SGX enclaves. However, big picture, here is the challenge that we see with respect to these conversations surrounding the integration of the SGX driver with the LSM: As a technology, SGX was designed to enable software to protect itself from an adversarial operating system and hardware platform. Given that, if we are intellectually honest, how effective can the LSM/OS be with respect to controlling the actions of an enclave? Without question, being able to regulate and control which identities can intersect to load executable content into an enclave is important. All of the infrastructure appears to be already there to accomplish that, given the default model of a shared library implementation of an enclave and requiring the loader to mmap file backed TEXT pages RX. The most relevant and important control with respect to whether or not an enclave should be allowed to execute is evaluation of the SIGSTRUCT. Given the trajectory that platform security is on, SGX is not going to be the last technology of its type nor the only technology that makes use of cryptographically based code provenance. As a result, if we are content with handing an opaque pointer of a descriptive struture to an LSM routine, a generic hook that is tasked with verifying code or execution environment provenance doesn't seem like it would need to be technology specific nor controversial. That leaves as the last thorny issue the question of dynamic allocation of memory for executable content. As we have stated before, and at the outset of this note, from a security perspective this is only, effectively, a binary question for the platform owner as to whether or not the concept should be allowed. A generic LSM hook, appropriately named, could execute that decision without being SGX specific. Arguably, the hook should be named to indicate that it is seeking approval for allocating memory to be used for anonymous executable content, since that is what it would be effectively requesting approval for, in the case of SGX. For completeness a third generic hook may be useful. The purpose of that hook would be to verify a block of memory as being measured or signed for consideration as executable content. Arguably that will have utility far beyond SGX. In the case of SGX it would address the issue as to whether or not a block of executable content in untrusted space is eligible for anonymous execution. That may be a useful security measure in order to provide some control over an enclave being used as a random execution oracle. It obviously has no security utility against the enclave author since, as we have noted before, it is possible for the enclave author to simply pull whatever code is desired over an encrypted network connection. > James Morris Hopefully these comments are a useful basis for further discussion. Best wishes for a productive week to everyone. Dr. Greg As always, Dr. Greg Wettstein, Ph.D, Worker IDfusion, LLC 4206 N. 19th Ave. Implementing measured information privacy Fargo, ND 58102 and integrity architectures. PH: 701-281-1686 FAX: 701-281-3949 EMAIL: gw@idfusion.org ------------------------------------------------------------------------------ "My thoughts on trusting Open-Source? A quote I once saw said it best: 'Remember, Amateurs built the ark. Professionals built the Titanic.' Perhaps most significantly the ark was one guy, there were no doubt committees involved with the Titanic project." -- Dr. G.W. Wettstein Resurrection
On Sun, 23 Jun 2019, Dr. Greg wrote: > The most relevant and important control with respect to whether or not > an enclave should be allowed to execute is evaluation of the > SIGSTRUCT. Given the trajectory that platform security is on, SGX is > not going to be the last technology of its type nor the only > technology that makes use of cryptographically based code provenance. > > As a result, if we are content with handing an opaque pointer of a > descriptive struture to an LSM routine, a generic hook that is tasked > with verifying code or execution environment provenance doesn't seem > like it would need to be technology specific nor controversial. > > That leaves as the last thorny issue the question of dynamic > allocation of memory for executable content. As we have stated > before, and at the outset of this note, from a security perspective > this is only, effectively, a binary question for the platform owner as > to whether or not the concept should be allowed. > > A generic LSM hook, appropriately named, could execute that decision > without being SGX specific. Arguably, the hook should be named to > indicate that it is seeking approval for allocating memory to be used > for anonymous executable content, since that is what it would be > effectively requesting approval for, in the case of SGX. > > For completeness a third generic hook may be useful. The purpose of > that hook would be to verify a block of memory as being > measured or signed for consideration as executable content. Arguably > that will have utility far beyond SGX. > > In the case of SGX it would address the issue as to whether or not a > block of executable content in untrusted space is eligible for > anonymous execution. That may be a useful security measure in order > to provide some control over an enclave being used as a random > execution oracle. > > It obviously has no security utility against the enclave author since, > as we have noted before, it is possible for the enclave author to > simply pull whatever code is desired over an encrypted network > connection. > > > James Morris > > Hopefully these comments are a useful basis for further discussion. Thanks, this is helpful.
diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c index f239300e0fc2..9b554d698388 100644 --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c @@ -9,6 +9,7 @@ #include <linux/highmem.h> #include <linux/ratelimit.h> #include <linux/sched/signal.h> +#include <linux/security.h> #include <linux/shmem_fs.h> #include <linux/slab.h> #include <linux/suspend.h> @@ -564,7 +565,8 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, return ret; } -static int sgx_encl_page_copy(void *dst, unsigned long src, unsigned long prot) +static int sgx_encl_page_copy(void *dst, unsigned long src, unsigned long prot, + u16 mrmask) { struct vm_area_struct *vma; int ret; @@ -572,24 +574,24 @@ static int sgx_encl_page_copy(void *dst, unsigned long src, unsigned long prot) /* Hold mmap_sem across copy_from_user() to avoid a TOCTOU race. */ down_read(¤t->mm->mmap_sem); + vma = find_vma(current->mm, src); + if (!vma) { + ret = -EFAULT; + goto out; + } + /* Query vma's VM_MAYEXEC as an indirect path_noexec() check. */ - if (prot & PROT_EXEC) { - vma = find_vma(current->mm, src); - if (!vma) { - ret = -EFAULT; - goto out; - } - - if (!(vma->vm_flags & VM_MAYEXEC)) { - ret = -EACCES; - goto out; - } + if ((prot & PROT_EXEC) && !(vma->vm_flags & VM_MAYEXEC)) { + ret = -EACCES; + goto out; } + ret = security_enclave_load(vma, prot, mrmask == 0xffff); + if (ret) + goto out; + if (copy_from_user(dst, (void __user *)src, PAGE_SIZE)) ret = -EFAULT; - else - ret = 0; out: up_read(¤t->mm->mmap_sem); @@ -639,7 +641,7 @@ static long sgx_ioc_enclave_add_page(struct file *filep, void __user *arg) prot = addp.flags & (PROT_READ | PROT_WRITE | PROT_EXEC); - ret = sgx_encl_page_copy(data, addp.src, prot); + ret = sgx_encl_page_copy(data, addp.src, prot, addp.mrmask); if (ret) goto out; diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7c1357105e61..3bc92c65f287 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1451,6 +1451,11 @@ * @enclave_map: * @prot contains the protection that will be applied by the kernel. * Return 0 if permission is granted. + * + * @enclave_load: + * @vma: the source memory region of the enclave page being loaded. + * @prot: the (maximal) protections of the enclave page. + * Return 0 if permission is granted. */ union security_list_options { int (*binder_set_context_mgr)(struct task_struct *mgr); @@ -1815,6 +1820,8 @@ union security_list_options { #ifdef CONFIG_INTEL_SGX int (*enclave_map)(unsigned long prot); + int (*enclave_load)(struct vm_area_struct *vma, unsigned long prot, + bool measured); #endif /* CONFIG_INTEL_SGX */ }; @@ -2057,6 +2064,7 @@ struct security_hook_heads { #endif /* CONFIG_BPF_SYSCALL */ #ifdef CONFIG_INTEL_SGX struct hlist_head enclave_map; + struct hlist_head enclave_load; #endif /* CONFIG_INTEL_SGX */ } __randomize_layout; diff --git a/include/linux/security.h b/include/linux/security.h index 6a1f54ba6794..572ddfc53039 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1832,11 +1832,18 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux) #ifdef CONFIG_INTEL_SGX #ifdef CONFIG_SECURITY int security_enclave_map(unsigned long prot); +int security_enclave_load(struct vm_area_struct *vma, unsigned long prot, + bool measured); #else static inline int security_enclave_map(unsigned long prot) { return 0; } +static inline int security_enclave_load(struct vm_area_struct *vma, + unsigned long prot, bool measured) +{ + return 0; +} #endif /* CONFIG_SECURITY */ #endif /* CONFIG_INTEL_SGX */ diff --git a/security/security.c b/security/security.c index 03951e08bdfc..00f483beb1cc 100644 --- a/security/security.c +++ b/security/security.c @@ -2365,4 +2365,9 @@ int security_enclave_map(unsigned long prot) { return call_int_hook(enclave_map, 0, prot); } +int security_enclave_load(struct vm_area_struct *vma, unsigned long prot, + bool measured) +{ + return call_int_hook(enclave_load, 0, vma, prot, measured); +} #endif /* CONFIG_INTEL_SGX */
enclave_load() is roughly analogous to the existing file_mprotect(). Due to the nature of SGX and its Enclave Page Cache (EPC), all enclave VMAs are backed by a single file, i.e. /dev/sgx/enclave, that must be MAP_SHARED. Furthermore, all enclaves need read, write and execute VMAs. As a result, the existing/standard call to file_mprotect() does not provide any meaningful security for enclaves since an LSM can only deny/grant access to the EPC as a whole. security_enclave_load() is called when SGX is first loading an enclave page, i.e. copying a page from normal memory into the EPC. Although the prototype for enclave_load() is similar to file_mprotect(), e.g. SGX could theoretically use file_mprotect() and set reqprot=prot, a separate hook is desirable as the semantics of an enclave's protection bits are different than those of vmas, e.g. an enclave page tracks the maximal set of protections, whereas file_mprotect() operates on the actual protections being provided. Enclaves also have unique security properties, e.g. measured code, that LSMs may want to consider. In other words, LSMs will likely want to implement different policies for enclave page protections. Note, extensive discussion yielded no sane alternative to some form of SGX specific LSM hook[1]. [1] https://lkml.kernel.org/r/CALCETrXf8mSK45h7sTK5Wf+pXLVn=Bjsc_RLpgO-h-qdzBRo5Q@mail.gmail.com Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kernel/cpu/sgx/driver/ioctl.c | 32 ++++++++++++++------------ include/linux/lsm_hooks.h | 8 +++++++ include/linux/security.h | 7 ++++++ security/security.c | 5 ++++ 4 files changed, 37 insertions(+), 15 deletions(-)