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.

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

Add KSPP compliance notices to corresponding parameters and sysctls #264

Merged
merged 11 commits into from
Aug 28, 2024

Conversation

raja-grewal
Copy link
Contributor

@raja-grewal raja-grewal commented Aug 18, 2024

This PR adds KSPP compliance notices to corresponding parameters and sysctls as per #256.

I have tried to be comprehensive and thorough but please re-check my work.

For the time being I have decided not to add "KSPP=no" notices as I am not sure that these would be helpful? Is there any point in showing non-compliance if we already show what is consistent with the KSPP.

Also there are two areas of of partial compliance:

## KSPP sets the stricter sysctl user.max_user_namespaces=0.
##
kernel.unprivileged_userns_clone=0

## KSPP sets the stricter sysctl kernel.yama.ptrace_scope=3.
##
kernel.yama.ptrace_scope=2

Do we want to adhere to the KSPP on these?

The first is discussed in #263 and has the potential to cause breakages but I am not that familiar with its real world usage.

The second was discussed earlier in #242.

Changes

There are currently no changes to the functionality of the code.

Mandatory Checklist

  • Legal agreements accepted. By contributing to this organisation, you acknowledge you have read, understood, and agree to be bound by these these agreements:

Terms of Service, Privacy Policy, Cookie Policy, E-Sign Consent, DMCA, Imprint

Optional Checklist

The following items are optional but might be requested in certain cases.

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

@raja-grewal
Copy link
Contributor Author

Another area of partial compliance is regarding forcing and immediate system reboot upon a "oops":

## KSPP sets CONFIG_PANIC_ON_OOPS=y, but also requires CONFIG_PANIC_TIMEOUT=-1.

We currently only have:

kernel.panic_on_oops=1
#kernel.panic=-1

Personally I think making the system reboot by default is a good from a security stand-point but I am not sure what the scale of impact will be on end users.

@raja-grewal
Copy link
Contributor Author

Note, we are also non-compliant when it comes to kernel warnings causing panics.

This requires setting sysctl kernel.panic_on_warn=1.

KSPP recommends immediate reboots at any oops or warn:

# Reboot after even 1 WARN or BUG/Oops. Adjust for your tolerances. (Since v6.2)
# If you want to set oops_limit greater than one, you will need to disable CONFIG_PANIC_ON_OOPS.
kernel.warn_limit = 1
kernel.oops_limit = 1

These two sysctls were introduced kernel 6.2 so are not usable. Instead, to be compliant we must currently set:

kernel.panic=-1
kernel.panic_on_oops=1
kernel.panic_on_warn=1

usr/lib/sysctl.d/990-security-misc.conf Outdated Show resolved Hide resolved
etc/default/grub.d/40_kernel_hardening.cfg Outdated Show resolved Hide resolved
@raja-grewal
Copy link
Contributor Author

@therealmate thank you for the review!

These suggestions had already been applied in 56b28e3.

@therealmate
Copy link

😅 yeah you're right, i must have been looking at an older diff

@adrelanos adrelanos marked this pull request as ready for review August 25, 2024 15:05
@adrelanos
Copy link
Member

Great work!

Could use a definition somewhere what KSPP=yes means.

## definitions:
## `KSPP=yes: recommended by KSPP

Suggestions welcome.

I'd like to merge this rather sooner than later. Ready?

Another (perhaps future work, separate PR?) could be to express security-misc KSPP compliance or non-compliance. Not sure that would make sense or any other metadata?

I guess any non-compliance can be seen from the context of additional comments, is however not standardized like KSPP= yes?

Not sure about KSPP=no either. Let's find consensus before proceeding. My idea was to express, that some settings came from sources other than KSPP.

@raja-grewal
Copy link
Contributor Author

express security-misc KSPP compliance or non-compliance

I am not exactly sure what you mean by this?

Regarding other metadata, there is no limit to the amount of fidelity that we could add but I am not sure if any of it will be that helpful to many people.

We could add an equivalent 'Madaidan=yes', but given the over two years of inactivity, I do not think this is necessary despite them being a large early contributor of the settings. The only other sizable sources I can think of at the moment are you and me.

I think our configs have already greatly expanded in size/lines from all the refactoring I have submitted over the last 6 weeks. Adding more metadata would certainly be helpful, but I we need a solid reason for their inclusion.

On top of this there are 2 outstanding issues that I can currently conceive:

  1. Should we disable ptrace() as per the KSPP recommendation or leave it as it is?
  2. Should we enable kernel.panic_on_warn=1 and kernel.panic=-1 to be compliant with KSPP?

@adrelanos
Copy link
Member

express security-misc KSPP compliance or non-compliance

I am not exactly sure what you mean by this?

I mean to add some easily readable metadata comment that expresses when we deviate from KSPP recommendations. For example in human understandable terms, "KSPP recommended, yes, but not implemented by security-misc, because ...".

On top of this there are 2 outstanding issues that I can currently conceive:

1. Should we disable `ptrace()` as per the KSPP recommendation or leave it as it is?
2. Should we enable `kernel.panic_on_warn=1` and `kernel.panic=-1` to be compliant with KSPP?

Best discussed in separate issues to not mix it with this big comment changes pull request.

@adrelanos adrelanos merged commit 328840c into Kicksecure:master Aug 28, 2024
@raja-grewal raja-grewal deleted the kspp_compliance branch August 28, 2024 15:44
@raja-grewal
Copy link
Contributor Author

I mean to add some easily readable metadata comment that expresses when we deviate from KSPP recommendations. For example in human understandable terms, "KSPP recommended, yes, but not implemented by security-misc, because ...".

This is a good idea! It will definitely decrease future maintenance burden. I will think about what would be the best way to implement this.

However, note that in most situations the comments surrounding a setting almost always generally explain why we diverge from the KSPP. However, it would certainly be efficient to convert this into metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants