Kicksecure / security-misc

Kernel Hardening; Protect Linux User Accounts against Brute Force Attacks; Improve Entropy Collection; Strong Linux User Account Separation; Enhances Misc Security Settings - https://www.kicksecure.com/wiki/Security-misc
https://www.kicksecure.com/wiki/Impressum
Other
517 stars 51 forks source link

Add KSPP compliance notices to corresponding parameters and `sysctls` #264

Closed raja-grewal closed 3 months ago

raja-grewal commented 3 months ago

This PR adds KSPP compliance notices to corresponding parameters and sysctls as per https://github.com/Kicksecure/security-misc/issues/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 https://github.com/Kicksecure/security-misc/pull/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 https://github.com/Kicksecure/security-misc/pull/242.

Changes

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

Mandatory Checklist

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.

raja-grewal commented 3 months ago

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 commented 3 months ago

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
raja-grewal commented 3 months ago

@therealmate thank you for the review!

These suggestions had already been applied in https://github.com/Kicksecure/security-misc/pull/264/commits/56b28e38264fe742b8d694176f1057c15574fc08.

therealmate commented 3 months ago

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

adrelanos commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.

raja-grewal commented 2 months ago

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.