Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modify requirements for Android configs #91

Closed
wants to merge 2 commits into from
Closed

Modify requirements for Android configs #91

wants to merge 2 commits into from

Conversation

petervanvugt
Copy link

Android configs require various things that are currently disallowed in this tool. We can use CONFIG_ANDROID to detect Android configs and generate reports with fewer positives that cannot/should not be changed.

@a13xp0p0v
Copy link
Owner

Hello @petervanvugt,

Nice idea, thanks.

Let's discuss some details.

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -51,8 +52,9 @@ def add_kconfig_checks(l, arch):
l += [OR(KconfigCheck('self_protection', 'defconfig', 'STRICT_MODULE_RWX', 'y'),
KconfigCheck('self_protection', 'defconfig', 'DEBUG_SET_MODULE_RONX', 'y'),
modules_not_set)] # DEBUG_SET_MODULE_RONX was before v4.11
l += [OR(KconfigCheck('self_protection', 'defconfig', 'REFCOUNT_FULL', 'y'),
VersionCheck((5, 5)))] # REFCOUNT_FULL is enabled by default since v5.5
if arch in ('X86_64'):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you describe the rationale behind this change?

I think REFCOUNT_FULL was available for other architectures, not only for X86_64.

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -118,7 +120,7 @@ def add_kconfig_checks(l, arch):
l += [KconfigCheck('self_protection', 'kspp', 'DEBUG_NOTIFIERS', 'y')]
l += [KconfigCheck('self_protection', 'kspp', 'INIT_ON_ALLOC_DEFAULT_ON', 'y')]
l += [KconfigCheck('self_protection', 'kspp', 'KFENCE', 'y')]
l += [KconfigCheck('self_protection', 'kspp', 'HW_RANDOM_TPM', 'y')]
l += [OR(KconfigCheck('self_protection', 'kspp', 'HW_RANDOM_TPM', 'y'), is_android)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you describe the contradiction between ANDROID and HW_RANDOM_TPM?

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -222,11 +224,13 @@ def add_kconfig_checks(l, arch):
l += [KconfigCheck('security_policy', 'defconfig', 'SECURITY', 'y')]
if arch == 'ARM':
l += [KconfigCheck('security_policy', 'kspp', 'SECURITY', 'y')]
l += [KconfigCheck('security_policy', 'kspp', 'SECURITY_YAMA', 'y')]
l += [OR(KconfigCheck('security_policy', 'kspp', 'SECURITY_YAMA', 'y'),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you describe the contradiction between ANDROID and SECURITY_YAMA?

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -265,7 +269,7 @@ def add_kconfig_checks(l, arch):
l += [KconfigCheck('cut_attack_surface', 'kspp', 'MODIFY_LDT_SYSCALL', 'is not set')]
l += [KconfigCheck('cut_attack_surface', 'kspp', 'OABI_COMPAT', 'is not set')]
l += [KconfigCheck('cut_attack_surface', 'kspp', 'X86_MSR', 'is not set')] # refers to LOCKDOWN
l += [modules_not_set]
l += [OR(modules_not_set, is_android)] # Android requires kernel modules
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've found this in the documentation: https://source.android.com/docs/core/architecture/kernel/loadable-kernel-modules

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
l += [KconfigCheck('security_policy', 'kspp', 'SECURITY_LANDLOCK', 'y')]
l += [KconfigCheck('security_policy', 'kspp', 'SECURITY_SELINUX_DISABLE', 'is not set')]
l += [KconfigCheck('security_policy', 'kspp', 'SECURITY_SELINUX_BOOTPARAM', 'is not set')]
l += [KconfigCheck('security_policy', 'kspp', 'SECURITY_SELINUX_DEVELOP', 'is not set')]
l += [OR(KconfigCheck('security_policy', 'kspp', 'SECURITY_SELINUX_DEVELOP', 'is not set'),
is_android)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting.

Why does ANDROID require this debug option for SELINUX?

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -338,7 +342,8 @@ def add_kconfig_checks(l, arch):
l += [KconfigCheck('cut_attack_surface', 'clipos', 'STAGING', 'is not set')]
l += [KconfigCheck('cut_attack_surface', 'clipos', 'KSM', 'is not set')] # to prevent FLUSH+RELOAD attack
l += [KconfigCheck('cut_attack_surface', 'clipos', 'KALLSYMS', 'is not set')]
l += [KconfigCheck('cut_attack_surface', 'clipos', 'MAGIC_SYSRQ', 'is not set')]
l += [OR(KconfigCheck('cut_attack_surface', 'clipos', 'MAGIC_SYSRQ', 'is not set'),
is_android)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you describe the contradiction between ANDROID and MAGIC_SYSRQ?

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -367,6 +372,7 @@ def add_kconfig_checks(l, arch):
l += [KconfigCheck('cut_attack_surface', 'my', 'CORESIGHT', 'is not set')]
l += [KconfigCheck('cut_attack_surface', 'my', 'XFS_SUPPORT_V4', 'is not set')]
l += [OR(KconfigCheck('cut_attack_surface', 'my', 'TRIM_UNUSED_KSYMS', 'y'),
is_android, # Android frequently uses out-of-tree modules
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, I would ask a reference to the documentation.

a13xp0p0v
a13xp0p0v reviewed Nov 22, 2023
View reviewed changes
kernel_hardening_checker/checks.py
@@ -351,7 +356,7 @@ def add_kconfig_checks(l, arch):
l += [KconfigCheck('cut_attack_surface', 'lockdown', 'EFI_TEST', 'is not set')] # refers to LOCKDOWN
l += [KconfigCheck('cut_attack_surface', 'lockdown', 'MMIOTRACE_TEST', 'is not set')] # refers to LOCKDOWN
l += [KconfigCheck('cut_attack_surface', 'lockdown', 'KPROBES', 'is not set')] # refers to LOCKDOWN
l += [bpf_syscall_not_set] # refers to LOCKDOWN
l += [OR(bpf_syscall_not_set, is_android)] # refers to LOCKDOWN, Android requires BPF
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would ask a reference to the documentation.

@a13xp0p0v a13xp0p0v added the new_feature A new feature of the tool label Nov 22, 2023
jvoisin added a commit to jvoisin/kernel-hardening-checker that referenced this pull request May 17, 2024
@jvoisin
Tweak the checks for android
7dae594 
> Android configs require various things that are currently disallowed in this
tool. We can use CONFIG_ANDROID to detect Android configs and generate reports
with fewer positives that cannot/should not be changed.

Based on a13xp0p0v#91
@jvoisin jvoisin mentioned this pull request May 17, 2024
Closed
@a13xp0p0v a13xp0p0v force-pushed the master branch 2 times, most recently from ea24300 to 78f5595 Compare June 2, 2024 12:49
jvoisin added a commit to jvoisin/kernel-hardening-checker that referenced this pull request Jun 28, 2024
@jvoisin
Tweak the checks for android
1f04c2e 
> Android configs require various things that are currently disallowed in this
> tool. We can use CONFIG_ANDROID to detect Android configs and generate reports with fewer positives that cannot/should not be changed.

Based on a13xp0p0v#91
@jvoisin jvoisin mentioned this pull request Jun 28, 2024
Closed
@a13xp0p0v a13xp0p0v removed the new_feature A new feature of the tool label Jul 3, 2024
@a13xp0p0v
Copy link
Owner

For now, closing this pull request.

See the details in #142.

@a13xp0p0v a13xp0p0v closed this Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a13xp0p0v a13xp0p0v a13xp0p0v left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@petervanvugt @a13xp0p0v