diff mbox series

hardening: Refresh KCFI options, add some more

Message ID 20240426222940.work.884-kees@kernel.org (mailing list archive)
State In Next
Commit 99803fb635e4041c3e4e3f8eae7aceeeb44e3cef
Headers show
Series hardening: Refresh KCFI options, add some more | expand

Commit Message

Kees Cook April 26, 2024, 10:29 p.m. UTC
Add some stuff that got missed along the way:

- CONFIG_UNWIND_PATCH_PAC_INTO_SCS=y so SCS vs PAC is hardware
  selectable.

- CONFIG_X86_KERNEL_IBT=y while a default, just be sure.

- CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
  it isn't as secure as straight KCFI.)

- CONFIG_PAGE_TABLE_CHECK=y for userspace mapping sanity.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Bill Wendling <morbo@google.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: linux-hardening@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: llvm@lists.linux.dev
---
 arch/arm64/configs/hardening.config | 5 +++++
 arch/x86/configs/hardening.config   | 9 +++++++++
 kernel/configs/hardening.config     | 4 ++++
 3 files changed, 18 insertions(+)

Comments

Nathan Chancellor April 29, 2024, 10:16 p.m. UTC | #1
On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> Add some stuff that got missed along the way:
> 
> - CONFIG_UNWIND_PATCH_PAC_INTO_SCS=y so SCS vs PAC is hardware
>   selectable.
> 
> - CONFIG_X86_KERNEL_IBT=y while a default, just be sure.
> 
> - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
>   it isn't as secure as straight KCFI.)
> 
> - CONFIG_PAGE_TABLE_CHECK=y for userspace mapping sanity.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Seems reasonable to me.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

One comment below.

> ---
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: x86@kernel.org
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Bill Wendling <morbo@google.com>
> Cc: Justin Stitt <justinstitt@google.com>
> Cc: linux-hardening@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: llvm@lists.linux.dev
> ---
>  arch/arm64/configs/hardening.config | 5 +++++
>  arch/x86/configs/hardening.config   | 9 +++++++++
>  kernel/configs/hardening.config     | 4 ++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/arch/arm64/configs/hardening.config b/arch/arm64/configs/hardening.config
> index b0e795208998..e8a18fec7a3e 100644
> --- a/arch/arm64/configs/hardening.config
> +++ b/arch/arm64/configs/hardening.config
> @@ -5,6 +5,7 @@ CONFIG_ARM64_SW_TTBR0_PAN=y
>  
>  # Software Shadow Stack or PAC
>  CONFIG_SHADOW_CALL_STACK=y
> +CONFIG_UNWIND_PATCH_PAC_INTO_SCS=y
>  
>  # Pointer authentication (ARMv8.3 and later). If hardware actually supports
>  # it, one can turn off CONFIG_STACKPROTECTOR_STRONG with this enabled.
> @@ -20,3 +21,7 @@ CONFIG_ARM64_E0PD=y
>  
>  # Available in ARMv8.7 and later.
>  CONFIG_ARM64_EPAN=y
> +
> +# Enable Kernel Control Flow Integrity (currently Clang only).
> +CONFIG_CFI_CLANG=y
> +# CONFIG_CFI_PERMISSIVE is not set

Should this be a part of kernel/configs/hardening.config because RISC-V
supports it (and 32-bit ARM will soon too)?

> diff --git a/arch/x86/configs/hardening.config b/arch/x86/configs/hardening.config
> index 7b497f3b7bc3..b47e5f411dd3 100644
> --- a/arch/x86/configs/hardening.config
> +++ b/arch/x86/configs/hardening.config
> @@ -10,5 +10,14 @@ CONFIG_INTEL_IOMMU_DEFAULT_ON=y
>  CONFIG_INTEL_IOMMU_SVM=y
>  CONFIG_AMD_IOMMU=y
>  
> +# Enforce CET Indirect Branch Tracking in the kernel.
> +CONFIG_X86_KERNEL_IBT=y
> +
> +# Enable Kernel Control Flow Integrity (currently Clang only), but disable
> +# weaker FINEIBT landing pads.
> +CONFIG_CFI_CLANG=y
> +# CONFIG_CFI_PERMISSIVE is not set
> +# CONFIG_FINEIBT is not set
> +
>  # Enable CET Shadow Stack for userspace.
>  CONFIG_X86_USER_SHADOW_STACK=y
> diff --git a/kernel/configs/hardening.config b/kernel/configs/hardening.config
> index 7a5bbfc024b7..4be0de1f085c 100644
> --- a/kernel/configs/hardening.config
> +++ b/kernel/configs/hardening.config
> @@ -23,6 +23,10 @@ CONFIG_SLAB_FREELIST_HARDENED=y
>  CONFIG_SHUFFLE_PAGE_ALLOCATOR=y
>  CONFIG_RANDOM_KMALLOC_CACHES=y
>  
> +# Sanity check userspace page table mappings.
> +CONFIG_PAGE_TABLE_CHECK=y
> +CONFIG_PAGE_TABLE_CHECK_ENFORCED=y
> +
>  # Randomize kernel stack offset on syscall entry.
>  CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y
>  
> -- 
> 2.34.1
>
Kees Cook April 30, 2024, 5:35 a.m. UTC | #2
On Mon, Apr 29, 2024 at 03:16:50PM -0700, Nathan Chancellor wrote:
> On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> [...]
> > +# Enable Kernel Control Flow Integrity (currently Clang only).
> > +CONFIG_CFI_CLANG=y
> > +# CONFIG_CFI_PERMISSIVE is not set
> 
> Should this be a part of kernel/configs/hardening.config because RISC-V
> supports it (and 32-bit ARM will soon too)?

Probably yes. I was worried it might be "noisy" for archs that don't
support it, but frankly if someone is using "make hardening.config" they
probably want to know about unsupported options. :)
Peter Zijlstra April 30, 2024, 9:21 a.m. UTC | #3
On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:

> - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
>   it isn't as secure as straight KCFI.)

Oi ?
Nathan Chancellor April 30, 2024, 3:12 p.m. UTC | #4
On Mon, Apr 29, 2024 at 10:35:03PM -0700, Kees Cook wrote:
> On Mon, Apr 29, 2024 at 03:16:50PM -0700, Nathan Chancellor wrote:
> > On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> > [...]
> > > +# Enable Kernel Control Flow Integrity (currently Clang only).
> > > +CONFIG_CFI_CLANG=y
> > > +# CONFIG_CFI_PERMISSIVE is not set
> > 
> > Should this be a part of kernel/configs/hardening.config because RISC-V
> > supports it (and 32-bit ARM will soon too)?
> 
> Probably yes. I was worried it might be "noisy" for archs that don't
> support it, but frankly if someone is using "make hardening.config" they
> probably want to know about unsupported options. :)

It would be potentially noisy as it is currently written since someone
building with GCC for arm64 or x86_64 could merge hardening.config into
their configuration and they would see CONFIG_CFI_CLANG get enabled by
merge_config.sh but on oldconfig or olddefconfig, it would get flipped
off again because the toolchain dependencies are not met. Might as well
make it architecture agnostic at that point :)

Cheers,
Nathan
Kees Cook April 30, 2024, 5:48 p.m. UTC | #5
On Tue, Apr 30, 2024 at 11:21:40AM +0200, Peter Zijlstra wrote:
> On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> 
> > - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
> >   it isn't as secure as straight KCFI.)
> 
> Oi ?

Same objection I always had[1]: moving the check into the destination
means attacks with control over executable memory contents can just omit
the check.

But now that I went to go look I see 0c3e806ec0f9 ("x86/cfi: Add boot
time hash randomization") is only enabled under FINEIBT... seems better
if that were always enabled...

-Kees

[1] https://lore.kernel.org/all/202210181020.79AF7F7@keescook/
Kees Cook April 30, 2024, 9:15 p.m. UTC | #6
On Tue, Apr 30, 2024 at 10:48:36AM -0700, Kees Cook wrote:
> On Tue, Apr 30, 2024 at 11:21:40AM +0200, Peter Zijlstra wrote:
> > On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> > 
> > > - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
> > >   it isn't as secure as straight KCFI.)
> > 
> > Oi ?
> 
> Same objection I always had[1]: moving the check into the destination
> means attacks with control over executable memory contents can just omit
> the check.
> 
> But now that I went to go look I see 0c3e806ec0f9 ("x86/cfi: Add boot
> time hash randomization") is only enabled under FINEIBT... seems better
> if that were always enabled...

And FINEIBT actually can't be disabled... :|

And as it turns out CFI_CLANG doesn't work at all on v6.9...

[    0.587220] no CFI hash found at: __call_sites+0x339a8/0x34450 ffffffffac20cef8 00 00 00 00 00
[    0.588226] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/alternative.c:1182 __apply_fineibt+0x7a9/0x820
...
[    0.619220] SMP alternatives: Something went horribly wrong trying to rewrite the CFI implementation.
*hang*
Kees Cook April 30, 2024, 9:25 p.m. UTC | #7
On Tue, Apr 30, 2024 at 02:15:53PM -0700, Kees Cook wrote:
> On Tue, Apr 30, 2024 at 10:48:36AM -0700, Kees Cook wrote:
> > On Tue, Apr 30, 2024 at 11:21:40AM +0200, Peter Zijlstra wrote:
> > > On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> > > 
> > > > - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
> > > >   it isn't as secure as straight KCFI.)
> > > 
> > > Oi ?
> > 
> > Same objection I always had[1]: moving the check into the destination
> > means attacks with control over executable memory contents can just omit
> > the check.
> > 
> > But now that I went to go look I see 0c3e806ec0f9 ("x86/cfi: Add boot
> > time hash randomization") is only enabled under FINEIBT... seems better
> > if that were always enabled...
> 
> And FINEIBT actually can't be disabled... :|
> 
> And as it turns out CFI_CLANG doesn't work at all on v6.9...
> 
> [    0.587220] no CFI hash found at: __call_sites+0x339a8/0x34450 ffffffffac20cef8 00 00 00 00 00
> [    0.588226] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/alternative.c:1182 __apply_fineibt+0x7a9/0x820
> ...
> [    0.619220] SMP alternatives: Something went horribly wrong trying to rewrite the CFI implementation.
> *hang*

Welcome to my TED talk.

I'll keep debugging this: boots fine on v6.8, but booting v6.9 with
defconfig+CFI_CLANG _does_ work, so something in my .config broke it...
Peter Zijlstra May 1, 2024, 11:06 a.m. UTC | #8
On Tue, Apr 30, 2024 at 10:48:36AM -0700, Kees Cook wrote:
> On Tue, Apr 30, 2024 at 11:21:40AM +0200, Peter Zijlstra wrote:
> > On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> > 
> > > - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
> > >   it isn't as secure as straight KCFI.)
> > 
> > Oi ?
> 
> Same objection I always had[1]: moving the check into the destination
> means attacks with control over executable memory contents can just omit
> the check.

I thought it was game over if you could write arbitrary test anyway?

The whole CFI thing was about clobbering data (function pointers to be
more specific), and both are robust against that.
Kees Cook May 1, 2024, 7:27 p.m. UTC | #9
On Wed, May 01, 2024 at 01:06:14PM +0200, Peter Zijlstra wrote:
> On Tue, Apr 30, 2024 at 10:48:36AM -0700, Kees Cook wrote:
> > On Tue, Apr 30, 2024 at 11:21:40AM +0200, Peter Zijlstra wrote:
> > > On Fri, Apr 26, 2024 at 03:29:44PM -0700, Kees Cook wrote:
> > > 
> > > > - CONFIG_CFI_CLANG=y for x86 and arm64. (And disable FINEIBT since
> > > >   it isn't as secure as straight KCFI.)
> > > 
> > > Oi ?
> > 
> > Same objection I always had[1]: moving the check into the destination
> > means attacks with control over executable memory contents can just omit
> > the check.
> 
> I thought it was game over if you could write arbitrary test anyway?

Yes, an attack having arbitrary write control over an executable memory
area is the place where CFI does get much weaker. But presently, there's
no reason to make it easier. With the hash randomization we have, an
attack needs to locate and read the kcfi_seed, otherwise everything else
is a fixed value.

In the future, when we have viable X^R kernel text, then the trade-off
becomes much more clear: we want FineIBT very very much. :)

> The whole CFI thing was about clobbering data (function pointers to be
> more specific), and both are robust against that.

Yes, if the threat model is limited to arbitrary writes to function
pointers, we're totally good with FineIBT.

-Kees
diff mbox series

Patch

diff --git a/arch/arm64/configs/hardening.config b/arch/arm64/configs/hardening.config
index b0e795208998..e8a18fec7a3e 100644
--- a/arch/arm64/configs/hardening.config
+++ b/arch/arm64/configs/hardening.config
@@ -5,6 +5,7 @@  CONFIG_ARM64_SW_TTBR0_PAN=y
 
 # Software Shadow Stack or PAC
 CONFIG_SHADOW_CALL_STACK=y
+CONFIG_UNWIND_PATCH_PAC_INTO_SCS=y
 
 # Pointer authentication (ARMv8.3 and later). If hardware actually supports
 # it, one can turn off CONFIG_STACKPROTECTOR_STRONG with this enabled.
@@ -20,3 +21,7 @@  CONFIG_ARM64_E0PD=y
 
 # Available in ARMv8.7 and later.
 CONFIG_ARM64_EPAN=y
+
+# Enable Kernel Control Flow Integrity (currently Clang only).
+CONFIG_CFI_CLANG=y
+# CONFIG_CFI_PERMISSIVE is not set
diff --git a/arch/x86/configs/hardening.config b/arch/x86/configs/hardening.config
index 7b497f3b7bc3..b47e5f411dd3 100644
--- a/arch/x86/configs/hardening.config
+++ b/arch/x86/configs/hardening.config
@@ -10,5 +10,14 @@  CONFIG_INTEL_IOMMU_DEFAULT_ON=y
 CONFIG_INTEL_IOMMU_SVM=y
 CONFIG_AMD_IOMMU=y
 
+# Enforce CET Indirect Branch Tracking in the kernel.
+CONFIG_X86_KERNEL_IBT=y
+
+# Enable Kernel Control Flow Integrity (currently Clang only), but disable
+# weaker FINEIBT landing pads.
+CONFIG_CFI_CLANG=y
+# CONFIG_CFI_PERMISSIVE is not set
+# CONFIG_FINEIBT is not set
+
 # Enable CET Shadow Stack for userspace.
 CONFIG_X86_USER_SHADOW_STACK=y
diff --git a/kernel/configs/hardening.config b/kernel/configs/hardening.config
index 7a5bbfc024b7..4be0de1f085c 100644
--- a/kernel/configs/hardening.config
+++ b/kernel/configs/hardening.config
@@ -23,6 +23,10 @@  CONFIG_SLAB_FREELIST_HARDENED=y
 CONFIG_SHUFFLE_PAGE_ALLOCATOR=y
 CONFIG_RANDOM_KMALLOC_CACHES=y
 
+# Sanity check userspace page table mappings.
+CONFIG_PAGE_TABLE_CHECK=y
+CONFIG_PAGE_TABLE_CHECK_ENFORCED=y
+
 # Randomize kernel stack offset on syscall entry.
 CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y