| Message ID | 1613470672-3069-1-git-send-email-pnagar@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | RTIC: selinux: ARM64: Move selinux_state to a separate page | expand |
On Tue, Feb 16, 2021 at 2:19 AM Preeti Nagar <pnagar@codeaurora.org> 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. > > Currently, the change moves selinux_state structure to a separate page. > The page is 2MB aligned not 4K to avoid TLB related performance impact as, > for some CPU core designs, the TLB does not cache 4K stage 2 (IPA to PA) > mappings if the IPA comes from a stage 1 mapping. In future, we plan to > move more security-related kernel assets to this page to enhance > protection. > > Signed-off-by: Preeti Nagar <pnagar@codeaurora.org> This addresses my feedback from the RFC regarding the section symbols. No comment on whether there is a better approach, or the 2MB vs page alignment, but perhaps other folks cc'ed can please take a look. Acked-by: Nick Desaulniers <ndesaulniers@google.com> > --- > The RFC patch reviewed available at: > https://lore.kernel.org/linux-security-module/1610099389-28329-1-git-send-email-pnagar@codeaurora.org/ > --- > include/asm-generic/vmlinux.lds.h | 10 ++++++++++ > include/linux/init.h | 6 ++++++ > security/Kconfig | 11 +++++++++++ > security/selinux/hooks.c | 2 +- > 4 files changed, 28 insertions(+), 1 deletion(-) > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index b97c628..d1a5434 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -770,6 +770,15 @@ > *(.scommon) \ > } > > +#ifdef CONFIG_SECURITY_RTIC > +#define RTIC_BSS \ > + . = ALIGN(SZ_2M); \ > + KEEP(*(.bss.rtic)) \ > + . = ALIGN(SZ_2M); > +#else > +#define RTIC_BSS > +#endif > + > /* > * Allow archectures to redefine BSS_FIRST_SECTIONS to add extra > * sections to the front of bss. > @@ -782,6 +791,7 @@ > . = ALIGN(bss_align); \ > .bss : AT(ADDR(.bss) - LOAD_OFFSET) { \ > BSS_FIRST_SECTIONS \ > + RTIC_BSS \ > . = ALIGN(PAGE_SIZE); \ > *(.bss..page_aligned) \ > . = ALIGN(PAGE_SIZE); \ > diff --git a/include/linux/init.h b/include/linux/init.h > index e668832..e6d452a 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -300,6 +300,12 @@ void __init parse_early_options(char *cmdline); > /* Data marked not to be saved by software suspend */ > #define __nosavedata __section(".data..nosave") > > +#ifdef CONFIG_SECURITY_RTIC > +#define __rticdata __section(".bss.rtic") > +#else > +#define __rticdata > +#endif > + > #ifdef MODULE > #define __exit_p(x) x > #else > diff --git a/security/Kconfig b/security/Kconfig > index 7561f6f..1af913a 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -291,5 +291,16 @@ config LSM > > source "security/Kconfig.hardening" > > +config SECURITY_RTIC > + bool "RunTime Integrity Check feature" > + depends on ARM64 > + help > + RTIC(RunTime Integrity Check) feature is to protect Linux kernel > + at runtime. This relocates some of the security sensitive kernel > + structures to a separate RTIC specific page. > + > + This is to enable monitoring and protection of these kernel assets > + from a higher exception level(EL) against any unauthorized changes. > + > endmenu > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 644b17e..59d7eee 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -104,7 +104,7 @@ > #include "audit.h" > #include "avc_ss.h" > > -struct selinux_state selinux_state; > +struct selinux_state selinux_state __rticdata; > > /* SECMARK reference count */ > static atomic_t selinux_secmark_refcount = ATOMIC_INIT(0); > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation >
On Tue, Feb 16, 2021 at 11:22 AM Preeti Nagar <pnagar@codeaurora.org> 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. > > Currently, the change moves selinux_state structure to a separate page. > The page is 2MB aligned not 4K to avoid TLB related performance impact as, > for some CPU core designs, the TLB does not cache 4K stage 2 (IPA to PA) > mappings if the IPA comes from a stage 1 mapping. In future, we plan to > move more security-related kernel assets to this page to enhance > protection. Part of this commit message should likely be added as a new file under Documentation/ somewhere. > diff --git a/security/Kconfig b/security/Kconfig > index 7561f6f..1af913a 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -291,5 +291,16 @@ config LSM > > source "security/Kconfig.hardening" > > +config SECURITY_RTIC > + bool "RunTime Integrity Check feature" > + depends on ARM64 > + help > + RTIC(RunTime Integrity Check) feature is to protect Linux kernel > + at runtime. This relocates some of the security sensitive kernel > + structures to a separate RTIC specific page. > + > + This is to enable monitoring and protection of these kernel assets > + from a higher exception level(EL) against any unauthorized changes. Rewording suggestion: The RTIC (RunTime Integrity Check) feature protects the kernel at runtime by relocating some of its security-sensitive structures to a separate RTIC-specific page. This enables monitoring and and protecting them from a higher exception level against unauthorized changes. Cheers, Miguel
[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
On Wed, 17 Feb 2021 at 10:42, Will Deacon <will@kernel.org> wrote: > > [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. > 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. > 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
On 2021-02-17 09:42, Will Deacon wrote: > [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. +1 on that. Even if, as I suspect, this is targeting some unspecified hypervisor that is not KVM, the first course of action should be for this to be implemented in the kernel's own hypervisor first so that anyone can review understand what is at play. Thanks, M.
On 2021-02-17 15:45, Marc Zyngier wrote: > On 2021-02-17 09:42, Will Deacon wrote: >> [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. > > +1 on that. Even if, as I suspect, this is targeting some unspecified > hypervisor that is not KVM, the first course of action should be for > this to be implemented in the kernel's own hypervisor first so that > anyone can review understand what is at play. > > Thanks, > > M. Thank you for your comments. The key value add of the feature is a third party independent entity keeping a watch on crucial kernel assets, such that in case the kernel itself is compromised, still, the protection can remain intact. Can this be achieved if the implementation is done in KVM? I've limited knowledge of KVM currently, can surely look into more details for a better understanding. Agree that the mechanism for triaging what constitutes valid access needs a clear approach. We will discuss your suggestions internally if we can use them to improve the overall feature design and share updated patches. Thank you!
On 2021-02-17 02:02, Miguel Ojeda wrote: > On Tue, Feb 16, 2021 at 11:22 AM Preeti Nagar <pnagar@codeaurora.org> > 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. >> >> Currently, the change moves selinux_state structure to a separate >> page. >> The page is 2MB aligned not 4K to avoid TLB related performance impact >> as, >> for some CPU core designs, the TLB does not cache 4K stage 2 (IPA to >> PA) >> mappings if the IPA comes from a stage 1 mapping. In future, we plan >> to >> move more security-related kernel assets to this page to enhance >> protection. > > Part of this commit message should likely be added as a new file under > Documentation/ somewhere. > Yes, that will be helpful, will put it in Documentation/security in the next update. Thank you! >> diff --git a/security/Kconfig b/security/Kconfig >> index 7561f6f..1af913a 100644 >> --- a/security/Kconfig >> +++ b/security/Kconfig >> @@ -291,5 +291,16 @@ config LSM >> >> source "security/Kconfig.hardening" >> >> +config SECURITY_RTIC >> + bool "RunTime Integrity Check feature" >> + depends on ARM64 >> + help >> + RTIC(RunTime Integrity Check) feature is to protect Linux >> kernel >> + at runtime. This relocates some of the security sensitive >> kernel >> + structures to a separate RTIC specific page. >> + >> + This is to enable monitoring and protection of these kernel >> assets >> + from a higher exception level(EL) against any unauthorized >> changes. > > Rewording suggestion: > > The RTIC (RunTime Integrity Check) feature protects the kernel > at runtime by relocating some of its security-sensitive > structures > to a separate RTIC-specific page. This enables monitoring and > and protecting them from a higher exception level against > unauthorized changes. > Thanks :) > Cheers, > Miguel
On 2021-02-16 23:39, Nick Desaulniers wrote: > On Tue, Feb 16, 2021 at 2:19 AM Preeti Nagar <pnagar@codeaurora.org> > 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. >> >> Currently, the change moves selinux_state structure to a separate >> page. >> The page is 2MB aligned not 4K to avoid TLB related performance impact >> as, >> for some CPU core designs, the TLB does not cache 4K stage 2 (IPA to >> PA) >> mappings if the IPA comes from a stage 1 mapping. In future, we plan >> to >> move more security-related kernel assets to this page to enhance >> protection. >> >> Signed-off-by: Preeti Nagar <pnagar@codeaurora.org> > > This addresses my feedback from the RFC regarding the section symbols. > No comment on whether there is a better approach, or the 2MB vs page > alignment, but perhaps other folks cc'ed can please take a look. > > Acked-by: Nick Desaulniers <ndesaulniers@google.com> > Thank you! I look forward to reviews and suggestions from the added folks. >> --- >> The RFC patch reviewed available at: >> https://lore.kernel.org/linux-security-module/1610099389-28329-1-git-send-email-pnagar@codeaurora.org/ >> --- >> include/asm-generic/vmlinux.lds.h | 10 ++++++++++ >> include/linux/init.h | 6 ++++++ >> security/Kconfig | 11 +++++++++++ >> security/selinux/hooks.c | 2 +- >> 4 files changed, 28 insertions(+), 1 deletion(-) >> >> diff --git a/include/asm-generic/vmlinux.lds.h >> b/include/asm-generic/vmlinux.lds.h >> index b97c628..d1a5434 100644 >> --- a/include/asm-generic/vmlinux.lds.h >> +++ b/include/asm-generic/vmlinux.lds.h >> @@ -770,6 +770,15 @@ >> *(.scommon) >> \ >> } >> >> +#ifdef CONFIG_SECURITY_RTIC >> +#define RTIC_BSS >> \ >> + . = ALIGN(SZ_2M); >> \ >> + KEEP(*(.bss.rtic)) >> \ >> + . = ALIGN(SZ_2M); >> +#else >> +#define RTIC_BSS >> +#endif >> + >> /* >> * Allow archectures to redefine BSS_FIRST_SECTIONS to add extra >> * sections to the front of bss. >> @@ -782,6 +791,7 @@ >> . = ALIGN(bss_align); >> \ >> .bss : AT(ADDR(.bss) - LOAD_OFFSET) { >> \ >> BSS_FIRST_SECTIONS >> \ >> + RTIC_BSS >> \ >> . = ALIGN(PAGE_SIZE); >> \ >> *(.bss..page_aligned) >> \ >> . = ALIGN(PAGE_SIZE); >> \ >> diff --git a/include/linux/init.h b/include/linux/init.h >> index e668832..e6d452a 100644 >> --- a/include/linux/init.h >> +++ b/include/linux/init.h >> @@ -300,6 +300,12 @@ void __init parse_early_options(char *cmdline); >> /* Data marked not to be saved by software suspend */ >> #define __nosavedata __section(".data..nosave") >> >> +#ifdef CONFIG_SECURITY_RTIC >> +#define __rticdata __section(".bss.rtic") >> +#else >> +#define __rticdata >> +#endif >> + >> #ifdef MODULE >> #define __exit_p(x) x >> #else >> diff --git a/security/Kconfig b/security/Kconfig >> index 7561f6f..1af913a 100644 >> --- a/security/Kconfig >> +++ b/security/Kconfig >> @@ -291,5 +291,16 @@ config LSM >> >> source "security/Kconfig.hardening" >> >> +config SECURITY_RTIC >> + bool "RunTime Integrity Check feature" >> + depends on ARM64 >> + help >> + RTIC(RunTime Integrity Check) feature is to protect Linux >> kernel >> + at runtime. This relocates some of the security sensitive >> kernel >> + structures to a separate RTIC specific page. >> + >> + This is to enable monitoring and protection of these kernel >> assets >> + from a higher exception level(EL) against any unauthorized >> changes. >> + >> endmenu >> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 644b17e..59d7eee 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -104,7 +104,7 @@ >> #include "audit.h" >> #include "avc_ss.h" >> >> -struct selinux_state selinux_state; >> +struct selinux_state selinux_state __rticdata; >> >> /* SECMARK reference count */ >> static atomic_t selinux_secmark_refcount = ATOMIC_INIT(0); >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a >> member >> of Code Aurora Forum, hosted by The Linux Foundation >>
On 2021-02-17 15:23, Ard Biesheuvel wrote: > On Wed, 17 Feb 2021 at 10:42, Will Deacon <will@kernel.org> wrote: >> >> [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. >> > > 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. >> 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
On 2/17/21 10:42 AM, Will Deacon wrote: > [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) Will that really solve the problem? There is a lot of caches that are used to resolve policy data in selinux, and this caches will not be protected. If you can manipulate kernel data you can do cache poisoning. > 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
On Mon, 22 Feb 2021 04:58:41 +0000, pnagar@codeaurora.org wrote: > > On 2021-02-17 15:45, Marc Zyngier wrote: [...] > > +1 on that. Even if, as I suspect, this is targeting some unspecified > > hypervisor that is not KVM, the first course of action should be for > > this to be implemented in the kernel's own hypervisor first so that > > anyone can review understand what is at play. > > > > Thanks, > > > > M. > > Thank you for your comments. The key value add of the feature is a > third party independent entity keeping a watch on crucial kernel > assets, such that in case the kernel itself is compromised, still, > the protection can remain intact. Can this be achieved if the > implementation is done in KVM? I've limited knowledge of KVM > currently, can surely look into more details for a better > understanding. [+Quentin] KVM/arm64 doesn't currently support Stage-2 mappings on the host side, but there are patches[1] on the list that implement this functionality, and that I'm hoping to get in 5.13 (no pressure, Quentin... ;-). This could also be implemented with the current KVM code though, as a PV service to guests, and I'd suggest looking into that as an initial approach. Thanks, M. [1] https://lore.kernel.org/r/20210108121524.656872-1-qperret@google.com
On Tue, Feb 16, 2021 at 5:19 AM Preeti Nagar <pnagar@codeaurora.org> 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. > > Currently, the change moves selinux_state structure to a separate page. > The page is 2MB aligned not 4K to avoid TLB related performance impact as, > for some CPU core designs, the TLB does not cache 4K stage 2 (IPA to PA) > mappings if the IPA comes from a stage 1 mapping. In future, we plan to > move more security-related kernel assets to this page to enhance > protection. > > Signed-off-by: Preeti Nagar <pnagar@codeaurora.org> > --- > The RFC patch reviewed available at: > https://lore.kernel.org/linux-security-module/1610099389-28329-1-git-send-email-pnagar@codeaurora.org/ > --- > include/asm-generic/vmlinux.lds.h | 10 ++++++++++ > include/linux/init.h | 6 ++++++ > security/Kconfig | 11 +++++++++++ > security/selinux/hooks.c | 2 +- > 4 files changed, 28 insertions(+), 1 deletion(-) As long as we are only talking about moving the selinux_state struct itself and none of the pointers inside I think we should be okay (the access decision cache pointed to by selinux_state->avc could change frequently). Have you done any performance measurements of this change? Assuming they are not terrible, I have no objections to this patch from a SELinux perspective.
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index b97c628..d1a5434 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -770,6 +770,15 @@ *(.scommon) \ } +#ifdef CONFIG_SECURITY_RTIC +#define RTIC_BSS \ + . = ALIGN(SZ_2M); \ + KEEP(*(.bss.rtic)) \ + . = ALIGN(SZ_2M); +#else +#define RTIC_BSS +#endif + /* * Allow archectures to redefine BSS_FIRST_SECTIONS to add extra * sections to the front of bss. @@ -782,6 +791,7 @@ . = ALIGN(bss_align); \ .bss : AT(ADDR(.bss) - LOAD_OFFSET) { \ BSS_FIRST_SECTIONS \ + RTIC_BSS \ . = ALIGN(PAGE_SIZE); \ *(.bss..page_aligned) \ . = ALIGN(PAGE_SIZE); \ diff --git a/include/linux/init.h b/include/linux/init.h index e668832..e6d452a 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -300,6 +300,12 @@ void __init parse_early_options(char *cmdline); /* Data marked not to be saved by software suspend */ #define __nosavedata __section(".data..nosave") +#ifdef CONFIG_SECURITY_RTIC +#define __rticdata __section(".bss.rtic") +#else +#define __rticdata +#endif + #ifdef MODULE #define __exit_p(x) x #else diff --git a/security/Kconfig b/security/Kconfig index 7561f6f..1af913a 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -291,5 +291,16 @@ config LSM source "security/Kconfig.hardening" +config SECURITY_RTIC + bool "RunTime Integrity Check feature" + depends on ARM64 + help + RTIC(RunTime Integrity Check) feature is to protect Linux kernel + at runtime. This relocates some of the security sensitive kernel + structures to a separate RTIC specific page. + + This is to enable monitoring and protection of these kernel assets + from a higher exception level(EL) against any unauthorized changes. + endmenu diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 644b17e..59d7eee 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -104,7 +104,7 @@ #include "audit.h" #include "avc_ss.h" -struct selinux_state selinux_state; +struct selinux_state selinux_state __rticdata; /* SECMARK reference count */ static atomic_t selinux_secmark_refcount = ATOMIC_INIT(0);
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. Currently, the change moves selinux_state structure to a separate page. The page is 2MB aligned not 4K to avoid TLB related performance impact as, for some CPU core designs, the TLB does not cache 4K stage 2 (IPA to PA) mappings if the IPA comes from a stage 1 mapping. In future, we plan to move more security-related kernel assets to this page to enhance protection. Signed-off-by: Preeti Nagar <pnagar@codeaurora.org> --- The RFC patch reviewed available at: https://lore.kernel.org/linux-security-module/1610099389-28329-1-git-send-email-pnagar@codeaurora.org/ --- include/asm-generic/vmlinux.lds.h | 10 ++++++++++ include/linux/init.h | 6 ++++++ security/Kconfig | 11 +++++++++++ security/selinux/hooks.c | 2 +- 4 files changed, 28 insertions(+), 1 deletion(-)