lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Mar 2022 11:09:36 -0800 From: Reinette Chatre <reinette.chatre@...el.com> To: Jarkko Sakkinen <jarkko@...nel.org>, <linux-sgx@...r.kernel.org> CC: Nathaniel McCallum <nathaniel@...fian.com>, Dave Hansen <dave.hansen@...ux.intel.com>, Thomas Gleixner <tglx@...utronix.de>, "Ingo Molnar" <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>, "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v3] x86/sgx: Do not limit EAUG'd pages by pre-initialization policy Hi Jarkko, On 3/3/2022 7:39 PM, Jarkko Sakkinen wrote: > Pre-initialization policy is meant for EADD'd pages because they are > part of the enclave identity. It's a good practice to not let touch the > permissions after initialization, and does provide guarantees to e.g. > LSM's about the enclave. I disagree. There are scenarios where it is indeed good practice to modify the permissions after initialization. For example, pages that may be used for relocatable code can start with RWX permissions but once the pages have been populated with the code they should be able to restrict permissions to RX only. It is not good practice to require RWX permission over their entire lifetime. Ideally pages should only have the lowest permissions possible. Supporting the modification of permissions after initialization enables the security conscious enclave owner to support the security principle of least privilege. > > For EAUG'd pages it should be sufficient to let mmap(), mprotect() and > SGX opcodes to control the permissions. Thus effectively disable > pre-initialization policy by setting vm_max_prot_bits to RWX. Setting vm_max_prot_bits of EAUG pages to RWX results in dynamically added (RW) pages to allow RWX PTEs! Please correct me if I am wrong but I do not see how mmap() and mprotect() will control permissions here. Will these flows be able to prevent a RWX mapping of dynamically added RW EPCM pages? From what I understand SGX controls the permissions here with sgx_encl_may_map() called during mmap() and mprotect() and it will allow such mappings after this patch. RWX mapping (VMA) with RWX PTEs of RW EPCM memory will be allowed, correct? That means, an enclave using EAUG to expand its heap will have enclave pages with RW EPCM but yet could have RWX PTEs pointing to them after the user creates a RWX mmap(). >From what I understand this circumvents the kernel's security mechanisms. This also breaks one of the original rules of SGX as per: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/x86/sgx.rst#n74 "EPCM permissions are separate from the normal page tables. This prevents the kernel from, for instance, allowing writes to data which an enclave wishes to remain read-only." In these changes the kernel allows all dynamically added pages to be executable - even when the enclave wishes them not to be. > Then, remove vm_run_prot_bits. For EADD'd pages the roof is where > it was during construction, for EAUG'd we don't simply care. This > hard to keep in-sync variable adds only a layer of complexity and > nothing else. > > Without vm_run_prot_bits existing, SGX_IOC_ENCLAVE_RELAX_PERMISSIONS > does absolutely nothing. Therefore, it can be safely removed. Removing vm_run_prot_bits cripples SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS. With this removal SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS will support the modification of EPCM permissions but PTE and VMA permissions will continue to allow the maximum access possible for the page, whether the enclave page supports the permission or not. I find it risky to circumvent the kernel's security mechanisms and I am not comfortable signing off on this. Or am I just not understanding it correctly? Reinette
Powered by blists - more mailing lists