kconfig-hardened-check icon indicating copy to clipboard operation
kconfig-hardened-check copied to clipboard

add check for CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE=0x0 too

Open thestinger opened this issue 1 year ago • 10 comments

Disabling SYSRQ support entirely is nice, but not always possible. For example, Android uses /proc/sysrq-trigger from userspace processes for multiple purposes from privileged core system processes and controls access via SELinux. Android still sets the kernel.sysrq sysctl to 0 in early boot via init to disable using it via a keyboard, but it makes a lot more sense for that to happen via the kernel to close any opportunity to use it before init disables it. It would make sense to check for CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE=0x0 when SYSRQ isn't disabled to at least disable doing it via the keyboard by default.

thestinger avatar Jan 19 '24 07:01 thestinger

Hello @thestinger ,

Thanks for the idea.

Collecting all pieces together, we can have the following rules:

l += [OR(KconfigCheck('cut_attack_surface', 'clipos', 'MAGIC_SYSRQ', 'is not set'),
         KconfigCheck('cut_attack_surface', 'my', 'MAGIC_SYSRQ_DEFAULT_ENABLE', '0x0'))]
...
l += [CmdlineCheck('cut_attack_surface', 'my', 'sysrq_always_enabled', 'is not set')]
...
l += [SysctlCheck('cut_attack_surface', 'my', 'kernel.sysrq', '0')]

Do you agree?

a13xp0p0v avatar Feb 18 '24 18:02 a13xp0p0v

MAGIC_SYSRQ_DEFAULT_ENABLE being set to 0x0 without being enabled via sysrq_always_enabled or kernel.sysrq should provide similar benefits. It's probably still best to fully disable the functionality.

Disabling it via the sysctl alone leaves a gap in early boot where it's enabled if MAGIC_SYSRQ_DEFAULT_ENABLE is 0x1 which seemed like a problem.

There's also MAGIC_SYSRQ_SERIAL for controlling whether sysrq can be enabled via the serial port. Having that enabled is a potential hole although it depends on having something implementing it.

I think either having MAGIC_SYSRQ disabled or having MAGIC_SYSRQ_DEFAULT_ENABLE set to 0x0 + MAGIC_SYSRQ_SERIAL disabled + not overriding it via kernel command line or kernel.sysrq is fine.

Android sets kernel.sysrq in early boot but yet lots of devices enable it via sysrq_always_enabled on the kernel command line and disabling in early boot also doesn't really seem right since there's a gap between the kernel being ready and init disabling it.

thestinger avatar Feb 19 '24 14:02 thestinger

Hello @thestinger,

Thanks again for your explanation.

I added:

  • the MAGIC_SYSRQ_DEFAULT_ENABLE check: https://github.com/a13xp0p0v/kernel-hardening-checker/commit/48ff85596d7c1ed707a74844cfac72d736d0c71c
  • the kernel.sysrq check: https://github.com/a13xp0p0v/kernel-hardening-checker/commit/538af12944c3a16f5707db51f49b1f4d053300d0
  • the MAGIC_SYSRQ_SERIAL check: https://github.com/a13xp0p0v/kernel-hardening-checker/commit/d995dd6eab4d14d8400abe16bbf14c3364f99fb6

Do you like it?

a13xp0p0v avatar Jun 16 '24 04:06 a13xp0p0v

By the way, the KSPP added this recommendation:

CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE=176

It allows sync, remount read-only and reboot/poweroff.

@thestinger, what do you think about it?

a13xp0p0v avatar Jun 16 '24 04:06 a13xp0p0v

Hi @thestinger!

Currently, these sysrq checks in kernel-hardening-checker are marked as my recommendations. But it would be nice to mark them as GrapheneOS recommendations. Could you give a link to the GrapheneOS documentation or code enforcing this configuration? I would put it to the references.

And what do you think about CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE=176 recommended by KSPP?

a13xp0p0v avatar Jul 07 '24 13:07 a13xp0p0v

We don't really need credit for it particularly since many of the KSPP recommendations came from us anyway.

thestinger avatar Jul 07 '24 15:07 thestinger

I mean the sysrq checks that I developed according to this issue #104:

l += [OR(KconfigCheck('cut_attack_surface', 'clipos', 'MAGIC_SYSRQ', 'is not set'),
         KconfigCheck('cut_attack_surface', 'a13xp0p0v', 'MAGIC_SYSRQ_DEFAULT_ENABLE', '0x0'))]
l += [OR(KconfigCheck('cut_attack_surface', 'a13xp0p0v', 'MAGIC_SYSRQ_SERIAL', 'is not set'),
         KconfigCheck('cut_attack_surface', 'a13xp0p0v', 'MAGIC_SYSRQ_DEFAULT_ENABLE', '0x0'))]

l += [CmdlineCheck('cut_attack_surface', 'a13xp0p0v', 'sysrq_always_enabled', 'is not set')]

l += [OR(SysctlCheck('cut_attack_surface', 'a13xp0p0v', 'kernel.sysrq', '0'),
         AND(KconfigCheck('cut_attack_surface', 'clipos', 'MAGIC_SYSRQ', 'is not set'),
             have_kconfig))]

They are currently marked as clipos or a13xp0p0v. They are more restrictive than the KSPP recommendation.

Is it possible to refer to the GrapheneOS documentation or code for these checks?

a13xp0p0v avatar Jul 07 '24 17:07 a13xp0p0v

There's just https://github.com/GrapheneOS/kernel_common-6.6/commit/af734ccc119eb324e99f55da9883617ab0bc6304.

thestinger avatar Jul 07 '24 18:07 thestinger

You can see our overall core kernel changes here:

https://github.com/GrapheneOS/kernel_common-6.6/commits/15/

Bear in mind we're starting from the Android Generic Kernel Image configuration with Clang CFI, etc. already enabled and some downstream hardening features. Android uses strict full system SELinux policies and that is what gets used to restrict eBPF, io_uring, userfaultfd, ioctl commands, dmesg, perf events, etc. rather than very coarse kernel features that are close to all or nothing. This means most things related to restricting userspace access aren't relevant since SELinux policy is used. We also introduce our own extensions to SELinux.

Some our hardening is hardware-specific so it's outside of this GKI repository.

As an example of how a feature can be spread out across a bunch of areas, we have this USB-C / pogo pins port control feature for reducing attack surface while locked at both a software (kernel) and hardware (USB-C / pogo pins controller) level:

https://grapheneos.org/features#usb-c-port-and-pogo-pins-control

Due to this feature, disabling SYSRQ support isn't very relevant since we disable USB at a hardware and software level anyway.

Here's our software-level USB protection infrastructure in the kernel:

https://github.com/GrapheneOS/kernel_common-6.6/commit/777f92add12737c27bdf21a4314f88096055525d https://github.com/GrapheneOS/kernel_common-6.6/commit/7da1fe795c7d4770b2e6cc48e231db0ebaa96950 https://github.com/GrapheneOS/kernel_common-6.6/commit/57b0d1cd602249e442142482a15e8de61c9dbb0d https://github.com/GrapheneOS/kernel_common-6.6/commit/1b31f2c37ee57134f49f48a6bf9cbc86a79e8999

Hardware-level USB-C and pogo pins protection infrastructure:

https://github.com/GrapheneOS/kernel_google-modules_soc_gs/commits/14/ and https://github.com/GrapheneOS/kernel_gs/commits/14/ (USB-C driver) https://github.com/GrapheneOS/kernel_devices_google_tangorpro/commits/14/ (pogo pins driver)

The userspace part is spread across several repositories, at least these:

https://github.com/GrapheneOS/platform_frameworks_base/commits/14/ (tiny portion of our changes there are relevant) https://github.com/GrapheneOS/platform_system_sepolicy/commits/14/ (small portion of our changes there are relevant) https://github.com/GrapheneOS/platform_packages_apps_Settings/commits/14/ (tiny portion of our changes there are relevant) https://github.com/GrapheneOS/device_google_gs101/commits/14/ (small portion of our changes there are relevant) https://github.com/GrapheneOS/device_google_gs201/commits/14/ (small portion of our changes there are relevant) https://github.com/GrapheneOS/device_google_zuma/commits/14/ (small portion of our changes there are relevant)

Just as an example of one of our major features which involves kernel changes. There are others, and we try to put as much as possible in SELinux, etc. as possible rather than changing the kernel so there's not really that much there especially since we try to upstream stuff.

thestinger avatar Jul 07 '24 18:07 thestinger

There are GKI repositories for 5.10, 5.15, 6.1 and 6.6 but soon only the 6.1 and 6.6 ones will be relevant once Pixels all move to 6.1 which hopefully happens sooner rather than later. 6th/7th gen Pixels have out-of-tree drivers built from kernel/gs (kernel_gs) but 8th gen onwards have moved to having small repositories used with the GKI common kernel repository instead rather than both the common kernel repository for the kernel image / generic modules and another device kernel repository. Will go away for 6th/7th gen Pixels when they move to a newer kernel. Anyway, it's spread out more than you'd probably think. We've also been trying to do as much as we can via SELinux, seccomp-bpf and userspace code rather than patching the kernel when not necessary. When things get a bit cleaner when Pixels move to newer GKI branches and the last device kernel repository is gone (kernel_gs), we'll probably start making more kernel changes again such as making our hardened allocator with a best in class MTE implementation as we did in userspace.

thestinger avatar Jul 07 '24 18:07 thestinger