Improve security of sysfs pseudo-filesystem

[dimakuv]

Description of the problem

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

• [Pal/LibOS] Add physical id and fix cpu cores in /proc/cpuinfo #1910
• Add support for /sys pseudo-filesystem #1975
• ...and several fixup PRs to these...

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]

@vijaydhanraj Please comment on this issue (something like "Please assign me" is enough), so I can mark this GitHub issue as assigned to you. (Otherwise GitHub doesn't allow the assignment.)

[vijaydhanraj]

Please assign this issue to me.

[dimakuv]

Closing this to avoid duplication (moved to #2105)