linux-kernel - Re: [PATCH 05/25] x86/sgx: Introduce runtime protection bits

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a1b14f33-5142-8cab-3b5f-4cc79b62091e@intel.com>
Date:   Mon, 6 Dec 2021 13:20:36 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        Andy Lutomirski <luto@...nel.org>
CC:     <dave.hansen@...ux.intel.com>, <tglx@...utronix.de>,
        <bp@...en8.de>, <mingo@...hat.com>, <linux-sgx@...r.kernel.org>,
        <x86@...nel.org>, <seanjc@...gle.com>, <kai.huang@...el.com>,
        <cathy.zhang@...el.com>, <cedric.xing@...el.com>,
        <haitao.huang@...el.com>, <mark.shanahan@...el.com>,
        <hpa@...or.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/25] x86/sgx: Introduce runtime protection bits

Hi Jarkko,

On 12/4/2021 3:57 PM, Jarkko Sakkinen wrote:
> On Fri, Dec 03, 2021 at 11:28:04AM -0800, Andy Lutomirski wrote:
>> On 12/1/21 11:23, Reinette Chatre wrote:
>>> Enclave creators declare their paging permission intent at the time
>>> the pages are added to the enclave. These paging permissions are
>>> vetted when pages are added to the enclave and stashed off
>>> (in sgx_encl_page->vm_max_prot_bits) for later comparison with
>>> enclave PTEs.
>>>
>>
>> I'm a bit confused here. ENCLU[EMODPE] allows the enclave to change the EPCM
>> permission bits however it likes with no oversight from the kernel.  So we
>> end up with a whole bunch of permission masks:
>>
>> The PTE: controlled by complex kernel policy
>>
>> The VMA: with your series, this is entirely controlled by userspace.  I
>> think I'm fine with that.
>>
>> vm_max_prot_bits: populated from secinfo at setup time, unless I missed
>> something that changes it later.  Maybe I'm confused or missed something in
>> one of the patches,
>>
>> vm_run_prot_bits: populated from some combination of ioctls.  I'm entirely
>> lost as to what this is for.
>>
>> EPCM bits: controlled by the guest.  basically useless for any host purpose
>> on SGX2 hardware (with or without kernel support -- the enclave can do
>> ENCLU[EMODPE] whether we like it or not, even on old kernels)
>>
>> So I guess I don't understand the purpose of this patch	or of the rules in
>> the later patches, and I feel like this is getting more complicated than
>> makes sense.
>>
>>
>> Could we perhaps make vm_max_prot_bits dynamic or at least controllable in
>> some useful way?  My initial proposal (years ago) was for vm_max_prot_bits
>> to be *separately* configured at initial load time instead of being inferred
>> from secinfo with the intent being that the user untrusted runtime would set
>> it appropriately.  I have no problem with allowing runtime changes as long
>> as the security policy makes sense and it's kept consistent with PTEs.
> 
> This is a valid question. Since EMODPE exists why not just make things for
> EMODPE, and ignore EMODPR altogether?
> 

I believe that we should support the best practice of principle of least 
privilege - once a page no longer needs a particular permission there 
should be a way to remove it (the unneeded permission).

Reinette