Message-ID: <20210217094205.GA3570@willie-the-truck>
Date:   Wed, 17 Feb 2021 09:42:06 +0000
From:   Will Deacon <will@...nel.org>
To:     Preeti Nagar <pnagar@...eaurora.org>, maz@...nel.org,
        ardb@...nel.org
Cc:     arnd@...db.de, jmorris@...ei.org, serge@...lyn.com,
        paul@...l-moore.com, stephen.smalley.work@...il.com,
        eparis@...isplace.org, linux-security-module@...r.kernel.org,
        selinux@...r.kernel.org, linux-arch@...r.kernel.org,
        casey@...aufler-ca.com, ndesaulniers@...gle.com,
        dhowells@...hat.com, ojeda@...nel.org, psodagud@...eaurora.org,
        nmardana@...eaurora.org, rkavati@...eaurora.org,
        vsekhar@...eaurora.org, mreichar@...eaurora.org, johan@...nel.org,
        joe@...ches.com, jeyu@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] RTIC: selinux: ARM64: Move selinux_state to a separate
 page

[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:
> 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.

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

Powered by blists - more mailing lists