[RFC,v3,0/4] security/x86/sgx: SGX specific LSM hooks
mbox series

Message ID cover.1562542383.git.cedric.xing@intel.com
Headers show
Series
  • security/x86/sgx: SGX specific LSM hooks
Related show

Message

Xing, Cedric July 7, 2019, 11:41 p.m. UTC
This series intends to make the new SGX subsystem to work with the existing LSM
architecture smoothly so that, say, SGX cannot be abused to work around
restrictions set forth by LSM modules/policies. 

This patch is based on and could be applied cleanly on top of Jarkko Sakkinen’s
SGX patch series v20 (https://patchwork.kernel.org/cover/10905141/).

For those who haven’t followed closely, the whole discussion started from the
primary question of how to prevent creating an executable enclave page from a
regular memory page that is NOT executable as prohibited by LSM
modules/policies. And that can be translated into 2 relating questions in
practice, i.e. 1) how to determine the allowed initial protections of enclave
pages when they are being loaded and 2) how to determine the allowed
protections of enclave pages at runtime. Those who are familiar with LSM may
notice that, for regular files, #1 is determined by security_mmap_file() while
#2 is covered by security_file_mprotect(). Those 2 hooks however are
insufficient for enclaves due to the distinct composition and lifespan of
enclave pages. Specifically, security_mmap_file() only passes in the file but
is not specific on which portion of the file being mmap()’ed, with the
assumption that all pages of the same file shall have the same set of
allowed/disallowed protections. But that assumption is no longer true for
enclaves for 2 reasons: a) pages of an enclave may be loaded from different
image files with different attributes and b) enclave pages retain contents
across munmap()/mmap(), therefore, say, if a policy prohibits execution of
modified pages, then pages flagged modified have to stay modified across
munmap()/mmap() so that the policy cannot be circumvented by remapping (i.e.
munmap() followed by mmap() on the same range). But the lack of range
information in security_mmap_file()’s arguments simply blocks LSM modules from
tracking enclave pages properly.

A rational solution would always involve tracking the correspondence between
enclave pages and their origin (e.g. files from which they were loaded), which
is similar to tracking regular memory pages and their origin via vm_file of
struct vm_area_struct. But given the longer lifespan of enclave pages (than
VMAs they are mapped into), such correspondence has to be stored in a separate
data structure outside of VMAs. In theory, the correspondence could be stored
either in LSM or in the SGX subsystem. This series has picked the former
because firstly, such information is useful only within LSM so it makes more
sense to keep it as “LSM internal” and secondly, keeping the data structure
inside LSM would allow additional information to be cached in LSM modules
without affecting the rest of the kernel, while lastly, those data structures
would be gone when LSM is disabled hence would not impose any unnecessary
overhead. 

Those who are familiar with this topic and related discussions may also notice
that, Sean Christopherson has sent out an RFC patch recently to address the
same problem as this series. He adopted the other approach of tracking
page/origin correspondence inside the SGX subsystem. However, to reduce memory
overhead in practice, he cached the FSM (Finite State Machine) instead of
page/origin correspondences. By “FSM”, I mean policy FSM defined as sets of
states and events that may trigger state transitions. Generally speaking, any
LSM module has its own definition of FSM and usually uses attributes attached
to files to argument the FSM, then it advances the FSM as events are observed
and gives out decision based on the current FSM state. Sean’s implementation
attempts to move the FSM into the SGX subsystem, and by caching the arguments
returned by LSM it tries to monitor events and reach the same decisions by
itself. So from architecture perspective, that model has to face tough
challenges in reality, such as how to support multiple LSM modules that employ
different FSMs to govern page protection transitions. Implementation wise, his
model also imposes unwanted restrictions specifically to SGX2, such as:
  · Complicated/Restricted UAPI – Enclave loaders are required to provide
    “maximal protection” at page load time, but such information is NOT always
    available. For example, Graphene containers may run different applications
    comprised of different set of executables and/or shared objects. Some of
    them may contain self-modifying code (or text relocation) while others
    don’t. The generic enclave loader usually doesn’t have such information so
    wouldn’t be able to provide it ahead of time.
  · Inefficient Auditing – Audit logs are supposed to help system
    administrators to determine the set of minimally needed permissions and to
    detect abnormal behaviors. But consider the “maximal protection” model, if
    “maximal protection” is set to be too permissive, then audit log wouldn’t
    be able to detect anomalies; or if “maximal protection” is too restrictive,
    then audit log cannot identify the file violating the policy. In either
    case the audit log cannot fulfill its purposes.
  · Inability to support #PF driven EPC allocation in SGX2 – For those
    unfamiliar with SGX2 software flows, an SGX2 enclave requests a page by
    issuing EACCEPT on the address that a new page is wanted, and the resulted
    #PF is expected to be handled by the kernel by EAUG’ing an EPC page at the
    fault address, and then the enclave would be resumed and the faulting
    EACCEPT retried, and succeed. The key requirement is to allow mmap()’ing
    non-existing enclave pages so that the SGX module/subsystem could respond
    to #PFs by EAUG’ing new pages. Sean’s implementation doesn’t allow
    mmap()’ing non-existing pages for variety of reasons and therefore blocks
    this major SGX2 usage.

Please note that this series has only been compile-tested.

History:
  · This is version 3 of this patch series, with the following changes over
    version 2 per comments/requests from the community.
    - Per Casey Schaufler, moved EMA from the LSM infrastructure into a new LSM
      module, named “ema”, which is responsible for maintaining EMA maps for
      enclaves and offers APIs for other LSM modules to query/update EMA nodes.
    - Per Stephen Smalley, removed kernel command line option
      “lsm.ema.cache_decisions”. Enclave page origins will always be tracked
      and audit logs will always be accurate.
    - Per Andy Lutomirski, a new PROCESS2__ENCLAVE_EXECANON permission has been
      added to SELinux to allow EADD’ing executable pages from non-executable
      anonymous source pages.
    - Revised permission checks for enclave pages in SELinux. See
      selinux_enclave_load() and enclave_mprotect() functions in
      security/selinux/hooks.c for details.
  · v2 – https://patchwork.kernel.org/cover/11020301/
  · v1 – https://patchwork.kernel.org/cover/10984127/

Cedric Xing (4):
  x86/sgx: Add SGX specific LSM hooks
  x86/64: Call LSM hooks from SGX subsystem/module
  X86/sgx: Introduce EMA as a new LSM module
  x86/sgx: Implement SGX specific hooks in SELinux

 arch/x86/kernel/cpu/sgx/driver/ioctl.c |  80 ++++++-
 arch/x86/kernel/cpu/sgx/driver/main.c  |  16 +-
 include/linux/lsm_ema.h                |  97 +++++++++
 include/linux/lsm_hooks.h              |  27 +++
 include/linux/security.h               |  23 ++
 security/Makefile                      |   1 +
 security/commonema.c                   | 277 +++++++++++++++++++++++++
 security/security.c                    |  17 ++
 security/selinux/hooks.c               | 236 ++++++++++++++++++++-
 security/selinux/include/classmap.h    |   3 +-
 security/selinux/include/objsec.h      |   7 +
 11 files changed, 770 insertions(+), 14 deletions(-)
 create mode 100644 include/linux/lsm_ema.h
 create mode 100644 security/commonema.c

Comments

Sean Christopherson July 8, 2019, 3:55 p.m. UTC | #1
On Sun, Jul 07, 2019 at 04:41:30PM -0700, Cedric Xing wrote:
...

> different FSMs to govern page protection transitions. Implementation wise, his
> model also imposes unwanted restrictions specifically to SGX2, such as:
>   · Complicated/Restricted UAPI – Enclave loaders are required to provide

I don't think "complicated" is a fair assessment.  For SGX1 enclaves it's
literally a direct propagation of the SECINFO RWX flags.

>     “maximal protection” at page load time, but such information is NOT always
>     available. For example, Graphene containers may run different applications
>     comprised of different set of executables and/or shared objects. Some of
>     them may contain self-modifying code (or text relocation) while others
>     don’t. The generic enclave loader usually doesn’t have such information so
>     wouldn’t be able to provide it ahead of time.

I'm unconvinced that it would be remotely difficult to teach an enclave
loader that an enclave or hosted application employs SMC, relocation or
any other behavior that would require declaring RWX on all pages.

>   · Inefficient Auditing – Audit logs are supposed to help system
>     administrators to determine the set of minimally needed permissions and to
>     detect abnormal behaviors. But consider the “maximal protection” model, if
>     “maximal protection” is set to be too permissive, then audit log wouldn’t
>     be able to detect anomalies;

Huh?  Declaring overly permissive protections is only problematic if an
LSM denies the permission, in which case it will generate an accurate
audit log.

If the enclave/loader "requires" a permission it doesn't actually need,
e.g. EXECDIRTY, then it's a software bug that should be fixed.  I don't
see how this scenario is any different than an application that uses
assembly code without 'noexecstack' and inadvertantly "requires"
EXECSTACK due to triggering "read implies exec".  In both cases the
denied permission is unnecessary due to a userspace application bug.

>     or if “maximal protection” is too restrictive,
>     then audit log cannot identify the file violating the policy.

Maximal protections that are too restrictive are completely orthogonal to
LSMs as the enclave would fail to run irrespective of LSMs.  This is no
different than specifying the wrong RWX flags in SECINFO, or opening a
file as RO instead of RW.

> In either case the audit log cannot fulfill its purposes.
>   · Inability to support #PF driven EPC allocation in SGX2 – For those
>     unfamiliar with SGX2 software flows, an SGX2 enclave requests a page by
>     issuing EACCEPT on the address that a new page is wanted, and the resulted
>     #PF is expected to be handled by the kernel by EAUG’ing an EPC page at the
>     fault address, and then the enclave would be resumed and the faulting
>     EACCEPT retried, and succeed. The key requirement is to allow mmap()’ing
>     non-existing enclave pages so that the SGX module/subsystem could respond
>     to #PFs by EAUG’ing new pages. Sean’s implementation doesn’t allow
>     mmap()’ing non-existing pages for variety of reasons and therefore blocks
>     this major SGX2 usage.

This is simply wrong.  The key requirement in the theoretical EAUG scheme
is to mmap() pages that have not been added to the _hardware_ maintained
enclave.  The pages (or some optimized representation of a range of pages)
would exist in the kernel's software mode of the enclave.
Xing, Cedric July 8, 2019, 5:49 p.m. UTC | #2
On 7/8/2019 8:55 AM, Sean Christopherson wrote:
> On Sun, Jul 07, 2019 at 04:41:30PM -0700, Cedric Xing wrote:
> ...
> 
>> different FSMs to govern page protection transitions. Implementation wise, his
>> model also imposes unwanted restrictions specifically to SGX2, such as:
>>    · Complicated/Restricted UAPI – Enclave loaders are required to provide
> 
> I don't think "complicated" is a fair assessment.  For SGX1 enclaves it's
> literally a direct propagation of the SECINFO RWX flags.

True only for SGX1.
>>      “maximal protection” at page load time, but such information is NOT always
>>      available. For example, Graphene containers may run different applications
>>      comprised of different set of executables and/or shared objects. Some of
>>      them may contain self-modifying code (or text relocation) while others
>>      don’t. The generic enclave loader usually doesn’t have such information so
>>      wouldn’t be able to provide it ahead of time.
> 
> I'm unconvinced that it would be remotely difficult to teach an enclave
> loader that an enclave or hosted application employs SMC, relocation or
> any other behavior that would require declaring RWX on all pages.

You've been talking as if "enclave loader" is tailored to the enclave it 
is loading. But in reality "enclave loader" is usually a library knowing 
usually nothing about the enclave. How could it know if an enclave 
contains self-modifying code?

>>    · Inefficient Auditing – Audit logs are supposed to help system
>>      administrators to determine the set of minimally needed permissions and to
>>      detect abnormal behaviors. But consider the “maximal protection” model, if
>>      “maximal protection” is set to be too permissive, then audit log wouldn’t
>>      be able to detect anomalies;
> 
> Huh?  Declaring overly permissive protections is only problematic if an
> LSM denies the permission, in which case it will generate an accurate
> audit log.
> 
> If the enclave/loader "requires" a permission it doesn't actually need,
> e.g. EXECDIRTY, then it's a software bug that should be fixed.  I don't
> see how this scenario is any different than an application that uses
> assembly code without 'noexecstack' and inadvertantly "requires"
> EXECSTACK due to triggering "read implies exec".  In both cases the
> denied permission is unnecessary due to a userspace application bug.

You see, you've been assuming "enclave loader" knows everything and 
tailored to what it loads in a particular application. But the reality 
is the loader is generic and probably shared by multiple applications. 
It needs some generic way to figure out the "maximal protection". An 
implementation could use information embedded in the enclave file, or 
could just be "configurable". In the former case, you put extra burdens 
on the build tools, while in the latter case, your audit logs cannot 
help generating an appropriate configuration.

>>      or if “maximal protection” is too restrictive,
>>      then audit log cannot identify the file violating the policy.
> 
> Maximal protections that are too restrictive are completely orthogonal to
> LSMs as the enclave would fail to run irrespective of LSMs.  This is no
> different than specifying the wrong RWX flags in SECINFO, or opening a
> file as RO instead of RW.

Say loader is configurable. By looking at the log, can an administrator 
tell which file has too restrictive "maximal protection"?

>> In either case the audit log cannot fulfill its purposes.
>>    · Inability to support #PF driven EPC allocation in SGX2 – For those
>>      unfamiliar with SGX2 software flows, an SGX2 enclave requests a page by
>>      issuing EACCEPT on the address that a new page is wanted, and the resulted
>>      #PF is expected to be handled by the kernel by EAUG’ing an EPC page at the
>>      fault address, and then the enclave would be resumed and the faulting
>>      EACCEPT retried, and succeed. The key requirement is to allow mmap()’ing
>>      non-existing enclave pages so that the SGX module/subsystem could respond
>>      to #PFs by EAUG’ing new pages. Sean’s implementation doesn’t allow
>>      mmap()’ing non-existing pages for variety of reasons and therefore blocks
>>      this major SGX2 usage.
> 
> This is simply wrong.  The key requirement in the theoretical EAUG scheme
> is to mmap() pages that have not been added to the _hardware_ maintained
> enclave.  The pages (or some optimized representation of a range of pages)
> would exist in the kernel's software mode of the enclave.

You are right. Code can always change. My assessment was made on what's 
in your patch.

The key point here is your patch is more complicated than it seems 
because you've been hand-waving on imminent requirements.
Sean Christopherson July 8, 2019, 6:49 p.m. UTC | #3
On Mon, Jul 08, 2019 at 10:49:59AM -0700, Xing, Cedric wrote:
> On 7/8/2019 8:55 AM, Sean Christopherson wrote:
> >On Sun, Jul 07, 2019 at 04:41:30PM -0700, Cedric Xing wrote:
> True only for SGX1.
> >>     “maximal protection” at page load time, but such information is NOT always
> >>     available. For example, Graphene containers may run different applications
> >>     comprised of different set of executables and/or shared objects. Some of
> >>     them may contain self-modifying code (or text relocation) while others
> >>     don’t. The generic enclave loader usually doesn’t have such information so
> >>     wouldn’t be able to provide it ahead of time.
> >
> >I'm unconvinced that it would be remotely difficult to teach an enclave
> >loader that an enclave or hosted application employs SMC, relocation or
> >any other behavior that would require declaring RWX on all pages.
> 
> You've been talking as if "enclave loader" is tailored to the enclave it is
> loading. But in reality "enclave loader" is usually a library knowing
> usually nothing about the enclave. How could it know if an enclave contains
> self-modifying code?

Given the rarity of SMC, require enclaves to declare "I do SMC"...  The
Intel SDK already requires the enclave developer to declare heap size,
stack size, thread affinity, etc...  I have a very hard time believing
that it can't support SMC and relocation flags.
 
> >>   · Inefficient Auditing – Audit logs are supposed to help system
> >>     administrators to determine the set of minimally needed permissions and to
> >>     detect abnormal behaviors. But consider the “maximal protection” model, if
> >>     “maximal protection” is set to be too permissive, then audit log wouldn’t
> >>     be able to detect anomalies;
> >
> >Huh?  Declaring overly permissive protections is only problematic if an
> >LSM denies the permission, in which case it will generate an accurate
> >audit log.
> >
> >If the enclave/loader "requires" a permission it doesn't actually need,
> >e.g. EXECDIRTY, then it's a software bug that should be fixed.  I don't
> >see how this scenario is any different than an application that uses
> >assembly code without 'noexecstack' and inadvertantly "requires"
> >EXECSTACK due to triggering "read implies exec".  In both cases the
> >denied permission is unnecessary due to a userspace application bug.
> 
> You see, you've been assuming "enclave loader" knows everything and tailored
> to what it loads in a particular application. But the reality is the loader
> is generic and probably shared by multiple applications. 

No, I'm assuming that an enclave can communicate its basic needs without
undue pain.

> It needs some generic way to figure out the "maximal protection". An
> implementation could use information embedded in the enclave file, or could
> just be "configurable". In the former case, you put extra burdens on the build
> tools, while in the latter case, your audit logs cannot help generating an
> appropriate configuration.

I'm contending the "extra burdens" is minimal.

  if (do_smc || do_relocation)
          max_prot = RWX;
  else
          max_prot = SECINFO.FLAGS;

> >>     or if “maximal protection” is too restrictive,
> >>     then audit log cannot identify the file violating the policy.
> >
> >Maximal protections that are too restrictive are completely orthogonal to
> >LSMs as the enclave would fail to run irrespective of LSMs.  This is no
> >different than specifying the wrong RWX flags in SECINFO, or opening a
> >file as RO instead of RW.
> 
> Say loader is configurable. By looking at the log, can an administrator tell
> which file has too restrictive "maximal protection"?

Again, this fails irrespective of LSMs.  So the answer is "no", because
there is no log.  But the admin will never have to deal with this issue
because the enclave will *never* run, i.e. would unconditionally fail to
run during initial development.  And the developer has bigger problems if
they can't debug their own code.

> >>In either case the audit log cannot fulfill its purposes.
Xing, Cedric July 8, 2019, 10:26 p.m. UTC | #4
Hi Sean,

What's in my cover letter is my assessment on what's in your series. You 
may disagree. But I don't think it productive until you can prove your 
points in code.

The key points I'm making are:
(1) The impact to user mode code due to UAPI change is more significant 
than you have envisioned.
(2) Your series has implemented less than required in practice.

For #1, regular shared objects don't carry info like whether it contains 
self-modifying code or generates code on the fly. So your requirement of 
"maximal protection" is new, and you should at least put together a 
story to show everyone how it could be met, especially without changing 
build tools.

For #2, SGX2 is imminent, and the upcoming ICX server will support 512GB 
of EPC. So the problems in mprotect() performance and EAUG-on-#PF must 
be solved, let alone other problems. I guess you have to code them up so 
everyone will be able to evaluate whether your approach is really as 
simple as you have claimed.

-Cedric

On 7/8/2019 11:49 AM, Sean Christopherson wrote:
> On Mon, Jul 08, 2019 at 10:49:59AM -0700, Xing, Cedric wrote:
>> On 7/8/2019 8:55 AM, Sean Christopherson wrote:
>>> On Sun, Jul 07, 2019 at 04:41:30PM -0700, Cedric Xing wrote:
>> True only for SGX1.
>>>>      “maximal protection” at page load time, but such information is NOT always
>>>>      available. For example, Graphene containers may run different applications
>>>>      comprised of different set of executables and/or shared objects. Some of
>>>>      them may contain self-modifying code (or text relocation) while others
>>>>      don’t. The generic enclave loader usually doesn’t have such information so
>>>>      wouldn’t be able to provide it ahead of time.
>>>
>>> I'm unconvinced that it would be remotely difficult to teach an enclave
>>> loader that an enclave or hosted application employs SMC, relocation or
>>> any other behavior that would require declaring RWX on all pages.
>>
>> You've been talking as if "enclave loader" is tailored to the enclave it is
>> loading. But in reality "enclave loader" is usually a library knowing
>> usually nothing about the enclave. How could it know if an enclave contains
>> self-modifying code?
> 
> Given the rarity of SMC, require enclaves to declare "I do SMC"...  The
> Intel SDK already requires the enclave developer to declare heap size,
> stack size, thread affinity, etc...  I have a very hard time believing
> that it can't support SMC and relocation flags.
>   
>>>>    · Inefficient Auditing – Audit logs are supposed to help system
>>>>      administrators to determine the set of minimally needed permissions and to
>>>>      detect abnormal behaviors. But consider the “maximal protection” model, if
>>>>      “maximal protection” is set to be too permissive, then audit log wouldn’t
>>>>      be able to detect anomalies;
>>>
>>> Huh?  Declaring overly permissive protections is only problematic if an
>>> LSM denies the permission, in which case it will generate an accurate
>>> audit log.
>>>
>>> If the enclave/loader "requires" a permission it doesn't actually need,
>>> e.g. EXECDIRTY, then it's a software bug that should be fixed.  I don't
>>> see how this scenario is any different than an application that uses
>>> assembly code without 'noexecstack' and inadvertantly "requires"
>>> EXECSTACK due to triggering "read implies exec".  In both cases the
>>> denied permission is unnecessary due to a userspace application bug.
>>
>> You see, you've been assuming "enclave loader" knows everything and tailored
>> to what it loads in a particular application. But the reality is the loader
>> is generic and probably shared by multiple applications.
> 
> No, I'm assuming that an enclave can communicate its basic needs without
> undue pain.
> 
>> It needs some generic way to figure out the "maximal protection". An
>> implementation could use information embedded in the enclave file, or could
>> just be "configurable". In the former case, you put extra burdens on the build
>> tools, while in the latter case, your audit logs cannot help generating an
>> appropriate configuration.
> 
> I'm contending the "extra burdens" is minimal.
> 
>    if (do_smc || do_relocation)
>            max_prot = RWX;
>    else
>            max_prot = SECINFO.FLAGS;
> 
>>>>      or if “maximal protection” is too restrictive,
>>>>      then audit log cannot identify the file violating the policy.
>>>
>>> Maximal protections that are too restrictive are completely orthogonal to
>>> LSMs as the enclave would fail to run irrespective of LSMs.  This is no
>>> different than specifying the wrong RWX flags in SECINFO, or opening a
>>> file as RO instead of RW.
>>
>> Say loader is configurable. By looking at the log, can an administrator tell
>> which file has too restrictive "maximal protection"?
> 
> Again, this fails irrespective of LSMs.  So the answer is "no", because
> there is no log.  But the admin will never have to deal with this issue
> because the enclave will *never* run, i.e. would unconditionally fail to
> run during initial development.  And the developer has bigger problems if
> they can't debug their own code.
> 
>>>> In either case the audit log cannot fulfill its purposes.