a13xp0p0v / kernel-hardening-checker

A tool for checking the security hardening options of the Linux kernel
GNU General Public License v3.0
1.69k stars 156 forks source link

False positive and false negatives #13

Closed Bernhard40 closed 5 years ago

Bernhard40 commented 5 years ago

PAGE_POISONING_NO_SANITY and PAGE_POISONING_ZERO depend on PAGE_POISONING. Checking distro config which doesn't enable PAGE_POISONING (like Fedora) will show OK: not found for the first two even as it's far from ok in this case.

Currently script checks only for MODULE_SIG_SHA512. Some distros (like Fedora) may use SHA256 which I think should be fine as well even if KSPP chose different example.

a13xp0p0v commented 5 years ago

Hello @Bernhard40, Thanks for your report, let's discuss it.

PAGE_POISONING_NO_SANITY and PAGE_POISONING_ZERO depend on PAGE_POISONING. Checking distro config which doesn't enable PAGE_POISONING (like Fedora) will show OK: not found for the first two even as it's far from ok in this case.

Yes, they are dependent on PAGE_POISONING. These options make this feature weaker, so the script is checking that they are disabled. When the PAGE_POISONING is disabled, the error count is incremented anyway. I don't think that checking PAGE_POISONING_NO_SANITY and PAGE_POISONING_ZERO should behave differently in that case.

Currently script checks only for MODULE_SIG_SHA512. Some distros (like Fedora) may use SHA256 which I think should be fine as well even if KSPP chose different example.

The MODULE_SIG_SHA512 option is the KSPP recommendation, it is explicitly indicated by the script. Distros may have various reasons to do it differently. One day the script will support the error annotations (the idea is described here: https://github.com/a13xp0p0v/kconfig-hardened-check/pull/9#issuecomment-453810119)

Bernhard40 commented 5 years ago

Yes, they are dependent on PAGE_POISONING. These options make this feature weaker, so the script is checking that they are disabled. When the PAGE_POISONING is disabled, the error count is incremented anyway. I don't think that checking PAGE_POISONING_NO_SANITY and PAGE_POISONING_ZERO should behave differently in that case.

Consider distro which have PAGE_POISONING=n. In check it gets:

CONFIG_PAGE_POISONING                  |      y      |   kspp   |  self_protection   ||     FAIL: "is not set" 
CONFIG_PAGE_POISONING_NO_SANITY        | is not set  |    my    |  self_protection   ||       OK: not found
CONFIG_PAGE_POISONING_ZERO             | is not set  |    my    |  self_protection   ||       OK: not found

The sum is: 1xFAIL + 2xOK

Now, consider distro which has PAGE_POISONING=y, PAGE_POISONING_NO_SANITY=y, PAGE_POISONING_ZERO=y. In check it gets:

CONFIG_PAGE_POISONING                  |      y      |   kspp   |  self_protection   ||             OK
CONFIG_PAGE_POISONING_NO_SANITY        | is not set  |    my    |  self_protection   ||         FAIL: "y"
CONFIG_PAGE_POISONING_ZERO             | is not set  |    my    |  self_protection   ||         FAIL: "y"

The sum is: 2xFAIL + 1xOK

The check shows that distro which disables PAGE_POISONING completely is better than one which enables its weaker version! Specifically for fedora it's 52 errors with the former (actual config) vs 53 errors with the latter.

The MODULE_SIG_SHA512 option is the KSPP recommendation, it is explicitly indicated by the script.

I read this recommendation as sign your modules rather than sign your modules using SHA512. The KSPP page says But if CONFIG_MODULE=y is needed, at least they must be signed with a per-build key. Below they show an example with SHA512. I highly doubt they meant SHA512 explicitly and nothing else. IMO they just used one example because iterating it for SHA256/SHA384 would be rather redundant. You may ask Kees about what he had in mind when he wrote this.

a13xp0p0v commented 5 years ago

The check shows that distro which disables PAGE_POISONING completely is better than one which enables its weaker version! Specifically for fedora it's 52 errors with the former (actual config) vs 53 errors with the latter.

Right. Please have a look how I've solved this issue.

You may ask Kees about what he had in mind when he wrote this.

Ok, I will remember that. There are several things which can be added to KSPP wiki. I'll work on that later.

Bernhard40 commented 5 years ago

It's now used for PAGE_POISONING_NO_SANITY and PAGE_POISONING_ZERO - they are not checked if PAGE_POISONING is off:

You could also always mark them as failed in that case like FAIL: "dependency missing". That would prevent FAIL count from increasing when enabling only PAGE_POISONING.

a13xp0p0v commented 5 years ago

You could also always mark them as failed in that case like FAIL: "dependency missing"

@Bernhard40, nice idea, thank you. Implemented in d9aca2d28e9f95266bca2da09625d7d2c885a6b2.