Re: [PATCH] RTIC: selinux: ARM64: Move selinux_state to a separate page

On 2021-02-17 15:23, Ard Biesheuvel wrote:

On Wed, 17 Feb 2021 at 10:42, Will Deacon [off-list ref] wrote:

quoted

[Please include arm64 and kvm folks for threads involving the stage-2 
MMU]

On Tue, Feb 16, 2021 at 03:47:52PM +0530, Preeti Nagar wrote:

quoted

The changes introduce a new security feature, RunTime Integrity Check
(RTIC), designed to protect Linux Kernel at runtime. The motivation
behind these changes is:
1. The system protection offered by Security Enhancements(SE) for
Android relies on the assumption of kernel integrity. If the kernel
itself is compromised (by a perhaps as yet unknown future vulnerability),
SE for Android security mechanisms could potentially be disabled and
rendered ineffective.
2. Qualcomm Snapdragon devices use Secure Boot, which adds cryptographic
checks to each stage of the boot-up process, to assert the authenticity
of all secure software images that the device executes.  However, due to
various vulnerabilities in SW modules, the integrity of the system can be
compromised at any time after device boot-up, leading to un-authorized
SW executing.

The feature's idea is to move some sensitive kernel structures to a
separate page and monitor further any unauthorized changes to these,
from higher Exception Levels using stage 2 MMU. Moving these to a
different page will help avoid getting page faults from un-related data.
The mechanism we have been working on removes the write permissions for
HLOS in the stage 2 page tables for the regions to be monitored, such
that any modification attempts to these will lead to faults being
generated and handled by handlers. If the protected assets are moved to
a separate page, faults will be generated corresponding to change attempts
to these assets only. If not moved to a separate page, write attempts to
un-related data present on the monitored pages will also be generated.

Using this feature, some sensitive variables of the kernel which are
initialized after init or are updated rarely can also be protected from
simple overwrites and attacks trying to modify these.

Although I really like the idea of using stage-2 to protect the 
kernel, I
think the approach you outline here is deeply flawed. Identifying 
"sensitive
variables" of the kernel to protect is subjective and doesn't scale.
Furthermore, the triaging of what constitues a valid access is notably
absent from your description and is assumedly implemented in an opaque 
blob
at EL2.

I think a better approach would be along the lines of:

  1. Introduce the protection at stage-1 (like we already have for 
mapping
     e.g. the kernel text R/O)

  2. Implement the handlers in the kernel, so the heuristics are 
clear.

  3. Extend this to involve KVM, so that the host can manage its own
     stage-2 to firm-up the stage-1 protections.

Agree here. Making an arbitrary set of data structures r/o behind the
OS's back doesn't seem like an easy thing to maintain or reason about,
especially if this r/o-ness is only enforced on a tiny subset of
devices. If something needs to be writable only at boot, we have
__ro_after_init, and having hypervisor assisted enforcement of /that/
might be a worthwhile thing to consider, including perhaps ways to do
controlled patching of this region at runtime.

Thank you for the suggestions. We will look into the possibility of 
protection
of __ro_after_init and controlled updates to these. I understand, if 
this can be
made generic as Will also suggested, it might be more useful and easy to 
scale
and maintain.

quoted

I also think we should avoid tying this to specific data structures.
Rather, we should introduce a mechanism to make arbitrary data 
read-only.

I've CC'd Ard and Marc, as I think they've both been thinking about 
this
sort of thing recently as well.

Will