From: Jarkko Sakkinen <jarkko@kernel.org>
To: Haitao Huang <haitao.huang@linux.intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>,
Andy Lutomirski <luto@kernel.org>,
dave.hansen@linux.intel.com, tglx@linutronix.de, bp@alien8.de,
mingo@redhat.com, linux-sgx@vger.kernel.org, x86@kernel.org,
seanjc@google.com, kai.huang@intel.com, cathy.zhang@intel.com,
cedric.xing@intel.com, haitao.huang@intel.com,
mark.shanahan@intel.com, hpa@zytor.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/25] x86/sgx: Introduce runtime protection bits
Date: Thu, 13 Jan 2022 01:48:48 +0200 [thread overview]
Message-ID: <Yd9o4BJWNtK5AxoB@iki.fi> (raw)
In-Reply-To: <op.1ftbip0cwjvjmi@hhuan26-mobl1.mshome.net>
On Mon, Jan 10, 2022 at 09:48:15PM -0600, Haitao Huang wrote:
> On Mon, 10 Jan 2022 20:15:28 -0600, Jarkko Sakkinen <jarkko@kernel.org>
> wrote:
>
> > On Tue, Jan 11, 2022 at 04:03:32AM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Jan 11, 2022 at 03:55:59AM +0200, Jarkko Sakkinen wrote:
> > > > On Tue, Jan 11, 2022 at 03:53:26AM +0200, Jarkko Sakkinen wrote:
> > > > > On Mon, Jan 10, 2022 at 04:05:21PM -0600, Haitao Huang wrote:
> > > > > > On Sat, 08 Jan 2022 10:22:30 -0600, Jarkko Sakkinen
> > > <jarkko@kernel.org>
> > > > > > wrote:
> > > > > >
> > > > > > > On Sat, Jan 08, 2022 at 05:51:46PM +0200, Jarkko Sakkinen wrote:
> > > > > > > > On Sat, Jan 08, 2022 at 05:45:44PM +0200, Jarkko Sakkinen
> > > wrote:
> > > > > > > > > On Fri, Jan 07, 2022 at 10:14:29AM -0600, Haitao Huang
> > > wrote:
> > > > > > > > > > > > > OK, so the question is: do we need both or would a
> > > > > > > > mechanism just
> > > > > > > > > > > > to extend
> > > > > > > > > > > > > permissions be sufficient?
> > > > > > > > > > > >
> > > > > > > > > > > > I do believe that we need both in order to support
> > > pages
> > > > > > > > having only
> > > > > > > > > > > > the permissions required to support their intended use
> > > > > > > > during the
> > > > > > > > > > > > time the
> > > > > > > > > > > > particular access is required. While technically it is
> > > > > > > > possible to grant
> > > > > > > > > > > > pages all permissions they may need during their
> > > lifetime it
> > > > > > > > is safer to
> > > > > > > > > > > > remove permissions when no longer required.
> > > > > > > > > > >
> > > > > > > > > > > So if we imagine a run-time: how EMODPR would be
> > > useful, and
> > > > > > > > how using it
> > > > > > > > > > > would make things safer?
> > > > > > > > > > >
> > > > > > > > > > In scenarios of JIT compilers, once code is generated
> > > into RW pages,
> > > > > > > > > > modifying both PTE and EPCM permissions to RX would be
> > > a good
> > > > > > > > defensive
> > > > > > > > > > measure. In that case, EMODPR is useful.
> > > > > > > > >
> > > > > > > > > What is the exact threat we are talking about?
> > > > > > > >
> > > > > > > > To add: it should be *significantly* critical thread,
> > > given that not
> > > > > > > > supporting only EAUG would leave us only one complex call
> > > pattern with
> > > > > > > > EACCEPT involvement.
> > > > > > > >
> > > > > > > > I'd even go to suggest to leave EMODPR out of the patch
> > > set, and
> > > > > > > > introduce
> > > > > > > > it when there is PoC code for any of the existing run-time
> > > that
> > > > > > > > demonstrates the demand for it. Right now this way too
> > > speculative.
> > > > > > > >
> > > > > > > > Supporting EMODPE is IMHO by factors more critical.
> > > > > > >
> > > > > > > At least it does not protected against enclave code because
> > > an enclave
> > > > > > > can
> > > > > > > always choose not to EACCEPT any of the EMODPR requests. I'm
> > > not only
> > > > > > > confused here about the actual threat but also the potential
> > > adversary
> > > > > > > and
> > > > > > > target.
> > > > > > >
> > > > > > I'm not sure I follow your thoughts here. The sequence should
> > > be for enclave
> > > > > > to request EMODPR in the first place through runtime to
> > > kernel, then to
> > > > > > verify with EACCEPT that the OS indeed has done EMODPR.
> > > > > > If enclave does not verify with EACCEPT, then its own code has
> > > > > > vulnerability. But this does not justify OS not providing the
> > > mechanism to
> > > > > > request EMODPR.
> > > > >
> > > > > The question is really simple: what is the threat scenario? In
> > > order to use
> > > > > the word "vulnerability", you would need one.
> > > > >
> > > > > Given the complexity of the whole dance with EMODPR it is
> > > mandatory to have
> > > > > one, in order to ack it to the mainline.
> > > > >
> > > > > > Similar to how we don't want have RWX code pages for normal Linux
> > > > > > application, when an enclave loads code pages (either directly
> > > or JIT
> > > > > > compiled from high level code ) into EAUG'd page (which has
> > > RW), we do not
> > > > > > want leave pages to be RWX for code to be executable, hence
> > > the need of
> > > > > > EMODPR request OS to reduce the permissions to RX once the
> > > code is ready to
> > > > > > execute.
> > > > >
> > > > > You cannot compare *enforced* permissions outside the enclave,
> > > and claim that
> > > > > they would be equivalent to the permissions of the already
> > > sandboxed code
> > > > > inside the enclave, with permissions that are not enforced but
> > > are based
> > > > > on good will of the enclave code.
> > > >
> > > > To add, you can already do "EMODPR" by simply adjusting VMA
> > > permissions to be
> > > > more restrictive. How this would be worse than this collaboration
> > > based
> > > > thing?
> > >
> > > ... or you could even make soft version of EMODPR without using that
> > > opcode
> > > by writing an ioctl to update our xarray to allow lower permissions.
> > > That
> > > ties the hands of the process who is doing the mmap() already.
> >
> > E.g. why not just
> >
> > #define SGX_IOC_ENCLAVE_RESTRICT_PAGE_PERMISSIONS \
> > _IOW(SGX_MAGIC, 0x05, struct sgx_enclave_modify_page_permissions)
> > #define SGX_IOC_ENCLAVE_EXTEND_PAGE_PERMISSIONS \
> > _IOW(SGX_MAGIC, 0x06, struct sgx_enclave_modify_page_permissions)
> >
> > struct sgx_enclave_restrict_page_permissions {
> > __u64 src;
> > __u64 offset;
> > __u64 length;
> > __u64 secinfo;
> > __u64 count;
> > };
> > struct sgx_enclave_extend_page_permissions {
> > __u64 src;
> > __u64 offset;
> > __u64 length;
> > __u64 secinfo;
> > __u64 count;
> > };
> >
> > These would simply update the xarray and nothing else. I'd go with two
> > ioctls (with the necessary checks for secinfo) in order to provide hook
> > up points in the future for LSMs.
> >
> > This leaves only EAUG and EMODT requiring the EACCEPT handshake.
> >
> > /Jarkko
> The trusted code base here is the enclave. It can't trust any code outside
> for enforcement. There is also need for TLB shootdown.
>
> To answer your earlier question about threat, the threat is
> malicious/compromised code inside enclave. Yes, you can say the whole thing
> is sand-boxed, but the runtime inside enclave could load complex upper layer
> code. Therefore the runtime needs to have a trusted mechanism to ensure
> code pages not writable so that there is less/no chance for compromised
> malicious enclave to modify existing code pages. I still consider it to be
> similar to normal Linux elf-loader/dynamic linker relying on mmap/mprotect
> and trusting OS to enforce permissions, but here the enclave runtime only
> trust the HW provided mechanism: EMODPR to change EPCM records and EACCEPT
> to verify.
So what if:
1. User space does EMODPR ioctl.
2. Enclave does EACCEPT.
3. Enclave does EMODPE.
The problem here is the asymmetry of these operations. If EMODPE also
required EACCEPT from the run-time, EMODPR would also make sense.
Please give a code example on how EMODPR improves trust.
/Jarkko
next prev parent reply other threads:[~2022-01-12 23:50 UTC|newest]
Thread overview: 155+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-01 19:22 [PATCH 00/25] x86/sgx and selftests/sgx: Support SGX2 Reinette Chatre
2021-12-01 19:22 ` [PATCH 01/25] x86/sgx: Add shortlog descriptions to ENCLS wrappers Reinette Chatre
2021-12-04 18:30 ` Jarkko Sakkinen
2021-12-06 21:13 ` Reinette Chatre
2021-12-11 5:28 ` Jarkko Sakkinen
2021-12-13 22:06 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 02/25] x86/sgx: Add wrappers for SGX2 functions Reinette Chatre
2021-12-04 22:04 ` Jarkko Sakkinen
2021-12-06 21:15 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 03/25] x86/sgx: Support VMA permissions exceeding enclave permissions Reinette Chatre
2021-12-04 22:25 ` Jarkko Sakkinen
2021-12-04 22:27 ` Jarkko Sakkinen
2021-12-06 21:16 ` Reinette Chatre
2021-12-11 5:39 ` Jarkko Sakkinen
2021-12-13 22:08 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 04/25] x86/sgx: Add pfn_mkwrite() handler for present PTEs Reinette Chatre
2021-12-04 22:43 ` Jarkko Sakkinen
2021-12-06 21:18 ` Reinette Chatre
2021-12-11 7:37 ` Jarkko Sakkinen
2021-12-13 22:09 ` Reinette Chatre
2021-12-28 14:51 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 05/25] x86/sgx: Introduce runtime protection bits Reinette Chatre
2021-12-03 19:28 ` Andy Lutomirski
2021-12-03 22:12 ` Reinette Chatre
2021-12-04 0:38 ` Andy Lutomirski
2021-12-04 1:14 ` Reinette Chatre
2021-12-04 17:56 ` Andy Lutomirski
2021-12-04 23:55 ` Reinette Chatre
2021-12-13 22:34 ` Reinette Chatre
2021-12-04 23:57 ` Jarkko Sakkinen
2021-12-06 21:20 ` Reinette Chatre
2021-12-11 7:42 ` Jarkko Sakkinen
2021-12-13 22:10 ` Reinette Chatre
2021-12-28 14:52 ` Jarkko Sakkinen
2022-01-06 17:46 ` Reinette Chatre
2022-01-07 12:16 ` Jarkko Sakkinen
2022-01-07 16:14 ` Haitao Huang
2022-01-08 15:45 ` Jarkko Sakkinen
2022-01-08 15:51 ` Jarkko Sakkinen
2022-01-08 16:22 ` Jarkko Sakkinen
2022-01-10 22:05 ` Haitao Huang
2022-01-11 1:53 ` Jarkko Sakkinen
2022-01-11 1:55 ` Jarkko Sakkinen
2022-01-11 2:03 ` Jarkko Sakkinen
2022-01-11 2:15 ` Jarkko Sakkinen
2022-01-11 3:48 ` Haitao Huang
2022-01-12 23:48 ` Jarkko Sakkinen [this message]
2022-01-13 2:41 ` Haitao Huang
2022-01-14 21:36 ` Jarkko Sakkinen
2022-01-11 17:13 ` Reinette Chatre
2022-01-12 23:50 ` Jarkko Sakkinen
2022-01-12 23:56 ` Jarkko Sakkinen
2022-01-13 20:09 ` Nathaniel McCallum
2022-01-13 21:42 ` Reinette Chatre
2022-01-14 21:53 ` Jarkko Sakkinen
2022-01-14 21:57 ` Jarkko Sakkinen
2022-01-14 22:00 ` Jarkko Sakkinen
2022-01-14 22:17 ` Jarkko Sakkinen
2022-01-14 22:23 ` Jarkko Sakkinen
2022-01-14 22:34 ` Jarkko Sakkinen
2022-01-14 23:05 ` Reinette Chatre
2022-01-14 23:15 ` Jarkko Sakkinen
2022-01-15 0:01 ` Reinette Chatre
2022-01-15 0:27 ` Jarkko Sakkinen
2022-01-15 0:41 ` Reinette Chatre
2022-01-15 1:18 ` Jarkko Sakkinen
2022-01-15 11:56 ` Jarkko Sakkinen
2022-01-15 11:59 ` Jarkko Sakkinen
2022-01-17 13:13 ` Nathaniel McCallum
2022-01-18 1:59 ` Jarkko Sakkinen
2022-01-18 2:22 ` Jarkko Sakkinen
2022-01-18 3:31 ` Jarkko Sakkinen
2022-01-18 20:59 ` Reinette Chatre
2022-01-20 12:53 ` Jarkko Sakkinen
2022-01-20 16:52 ` Reinette Chatre
2022-01-26 14:41 ` Jarkko Sakkinen
2022-01-15 16:49 ` Jarkko Sakkinen
2022-01-18 21:18 ` Reinette Chatre
2022-01-17 13:27 ` Nathaniel McCallum
2022-01-18 21:11 ` Reinette Chatre
2021-12-04 22:50 ` Jarkko Sakkinen
2021-12-06 21:28 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 06/25] x86/sgx: Use more generic name for enclave cpumask function Reinette Chatre
2021-12-04 22:56 ` Jarkko Sakkinen
2021-12-06 21:29 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 07/25] x86/sgx: Move PTE zap code to separate function Reinette Chatre
2021-12-04 22:59 ` Jarkko Sakkinen
2021-12-06 21:30 ` Reinette Chatre
2021-12-11 7:52 ` Jarkko Sakkinen
2021-12-13 22:11 ` Reinette Chatre
2021-12-28 14:55 ` Jarkko Sakkinen
2022-01-06 17:46 ` Reinette Chatre
2022-01-07 12:26 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 08/25] x86/sgx: Make SGX IPI callback available internally Reinette Chatre
2021-12-04 23:00 ` Jarkko Sakkinen
2021-12-06 21:36 ` Reinette Chatre
2021-12-11 7:53 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 09/25] x86/sgx: Keep record of SGX page type Reinette Chatre
2021-12-04 23:03 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 10/25] x86/sgx: Support enclave page permission changes Reinette Chatre
2021-12-02 23:48 ` Dave Hansen
2021-12-03 18:18 ` Reinette Chatre
2021-12-03 0:32 ` Dave Hansen
2021-12-03 18:18 ` Reinette Chatre
2021-12-03 18:14 ` Dave Hansen
2021-12-03 18:49 ` Reinette Chatre
2021-12-03 19:38 ` Andy Lutomirski
2021-12-03 22:34 ` Reinette Chatre
2021-12-04 0:42 ` Andy Lutomirski
2021-12-04 1:35 ` Reinette Chatre
2021-12-04 23:08 ` Jarkko Sakkinen
2021-12-06 20:19 ` Dave Hansen
2021-12-11 5:17 ` Jarkko Sakkinen
2021-12-06 21:42 ` Reinette Chatre
2021-12-11 7:57 ` Jarkko Sakkinen
2021-12-13 22:12 ` Reinette Chatre
2021-12-28 14:56 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 11/25] selftests/sgx: Add test for EPCM " Reinette Chatre
2021-12-01 19:23 ` [PATCH 12/25] selftests/sgx: Add test for TCS page " Reinette Chatre
2021-12-01 19:23 ` [PATCH 13/25] x86/sgx: Support adding of pages to initialized enclave Reinette Chatre
2021-12-03 0:38 ` Dave Hansen
2021-12-03 18:47 ` Reinette Chatre
2021-12-04 23:13 ` Jarkko Sakkinen
2021-12-06 21:44 ` Reinette Chatre
2021-12-11 8:00 ` Jarkko Sakkinen
2021-12-13 22:12 ` Reinette Chatre
2021-12-28 14:57 ` Jarkko Sakkinen
2022-03-01 15:13 ` Jarkko Sakkinen
2022-03-01 17:08 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 14/25] x86/sgx: Tighten accessible memory range after enclave initialization Reinette Chatre
2021-12-04 23:14 ` Jarkko Sakkinen
2021-12-06 21:45 ` Reinette Chatre
2021-12-11 8:01 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 15/25] selftests/sgx: Test two different SGX2 EAUG flows Reinette Chatre
2021-12-01 19:23 ` [PATCH 16/25] x86/sgx: Support modifying SGX page type Reinette Chatre
2021-12-04 23:45 ` Jarkko Sakkinen
2021-12-06 21:48 ` Reinette Chatre
2021-12-11 8:02 ` Jarkko Sakkinen
2021-12-13 17:43 ` Dave Hansen
2021-12-21 8:52 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 17/25] x86/sgx: Support complete page removal Reinette Chatre
2021-12-04 23:45 ` Jarkko Sakkinen
2021-12-06 21:49 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 18/25] selftests/sgx: Introduce dynamic entry point Reinette Chatre
2021-12-01 19:23 ` [PATCH 19/25] selftests/sgx: Introduce TCS initialization enclave operation Reinette Chatre
2021-12-01 19:23 ` [PATCH 20/25] selftests/sgx: Test complete changing of page type flow Reinette Chatre
2021-12-01 19:23 ` [PATCH 21/25] selftests/sgx: Test faulty enclave behavior Reinette Chatre
2021-12-01 19:23 ` [PATCH 22/25] selftests/sgx: Test invalid access to removed enclave page Reinette Chatre
2021-12-01 19:23 ` [PATCH 23/25] selftests/sgx: Test reclaiming of untouched page Reinette Chatre
2021-12-01 19:23 ` [PATCH 24/25] x86/sgx: Free up EPC pages directly to support large page ranges Reinette Chatre
2021-12-04 23:47 ` Jarkko Sakkinen
2021-12-06 22:07 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 25/25] selftests/sgx: Page removal stress test Reinette Chatre
2021-12-02 18:30 ` [PATCH 00/25] x86/sgx and selftests/sgx: Support SGX2 Dave Hansen
2021-12-02 20:38 ` Nathaniel McCallum
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yd9o4BJWNtK5AxoB@iki.fi \
--to=jarkko@kernel.org \
--cc=bp@alien8.de \
--cc=cathy.zhang@intel.com \
--cc=cedric.xing@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=haitao.huang@intel.com \
--cc=haitao.huang@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kai.huang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mark.shanahan@intel.com \
--cc=mingo@redhat.com \
--cc=reinette.chatre@intel.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).