Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[LibOS/PAL] Sanitize /sysfs pseudo filesystem #2105

Closed
vijaydhanraj opened this issue Jan 20, 2021 · 2 comments · Fixed by gramineproject/gramine#530
Closed

[LibOS/PAL] Sanitize /sysfs pseudo filesystem #2105

vijaydhanraj opened this issue Jan 20, 2021 · 2 comments · Fixed by gramineproject/gramine#530
Assignees
Milestone

Comments

@vijaydhanraj
Copy link
Contributor

Description of the problem

We pass a lot of information from host-level /sys/ files. We sanitize some values in some files, but not everything. Need to audit and add missing sanitizations, if any.

Reference: #1975

@mkow mkow changed the title [LibOS/PAL]: Sanitize /sysfs pseudo filesystem [LibOS/PAL] Sanitize /sysfs pseudo filesystem Feb 25, 2021
@dimakuv dimakuv added this to the release v1.2 milestone Jul 22, 2021
@dimakuv
Copy link

dimakuv commented Aug 4, 2021

(the below text is moved from #2556 to avoid duplication.)

Description of the problem

@vijaydhanraj recently expanded support for /proc/cpuinfo and added support for /sys pseudo-filesystems:

The general approach is to copy all relevant information from host-level /proc and /sys filesystems and store it inside the PAL state (see file https://github.com/oscarlab/graphene/blob/master/Pal/include/pal/pal.h):

typedef struct PAL_CONTROL_ {
    ...
    PAL_CPU_INFO cpu_info; /*!< CPU information (only required ones) */
    PAL_MEM_INFO mem_info; /*!< memory information (only required ones) */
    PAL_TOPO_INFO topo_info; /*!< Topology information (only required ones) */
} PAL_CONTROL;

The PAL_CPU_INFO, PAL_MEM_INFO and PAL_TOPO_INFO structs are defined in https://github.com/oscarlab/graphene/blob/master/Pal/include/arch/x86_64/pal-arch.h. One can note that only PAL_CPU_INFO store numerical values, whereas PAL_MEM_INFO and PAL_TOPO_INFO (and their sub-structs) mostly store strings.

PAL contains a global variable that holds all this procfs and sysfs information (see file https://github.com/oscarlab/graphene/blob/master/Pal/src/db_main.c):

PAL_CONTROL g_pal_control;

PAL initializates all these g_pal_control.cpu_info, g_pal_control.mem_info, g_pal_control.topo_info fields at Graphene (PAL) startup. LibOS simply reads these fields from global g_pal_control on demand, via the wrapper DkGetPalControl() (which returns a pointer to this global variable).

Note that each new Graphene process (e.g., a child spawned by the parent) initializes all these fields anew. In other words, each Graphene enclave populates its g_pal_control fields from the host procfs/sysfs files, and not receives them from parent enclave.

Crux of the Problems

There are three problems with this approach:

  1. PAL_MEM_INFO and PAL_TOPO_INFO copy strings as-is from host-level procfs/sysfs files. This complicates parsing and validation of these strings inside the enclave.
  2. The procfs/sysfs state is stored in the PAL component (in the g_pal_control variable). This contradicts the philosophy of Graphene that PAL is (mostly) stateless and all state must be stored in the LibOS component.
  3. Each Graphene enclave has its own version of procfs/sysfs. Since these are populated from the host OS, a malicious host could feed different information to each Graphene enclave and break security / lead to inconsistencies.

Solutions

  1. Change the format of PAL_MEM_INFO and PAL_TOPO_INFO from raw strings to integers/sets of integers/maps. In untrusted part of PAL, read from host-level procfs and sysfs and transform into this new format. In trusted part of PAL, sanitize (validate) this new format and pass to LibOS.
  2. Trusted part of PAL must hand over these values to LibOS. LibOS must store these values somewhere in https://github.com/oscarlab/graphene/tree/master/LibOS/shim/src/fs/sys. LibOS must construct proper strings from this format whenever application wants to read these files.
  3. Child Graphene enclaves must never populate procfs/sysfs structures from the host OS. Instead, they must receive these structures as part of the checkpoint sent by the parent enclave. This item depends on item 2 -- our checkpoint-and-restore logic is implemented in LibOS, so the procfs/sysfs checkpoint-and-restore must be based on LibOS primitives.

What security issues does it solve?

Below are the security issues with the current approach.

  • The complex information within the files of sysfs/procfs might be malcrafted to trigger a memory corruption bug within the naïve application.
  • The complex information within the files of sysfs/procfs might be malcrafted to trigger a memory corruption bug within Graphene sysfs/procfs parser. To make parser simpler, we must move the complexity of the initial parsing into the untrusted PAL. And leave a very simplistic parsing logic inside the enclave.
  • Inconsistency of results returned by the kernel across calls between forked processes.

@dimakuv
Copy link

dimakuv commented Sep 23, 2021

We hope to finish this effort before the release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants