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

disable uncommon filesystems and network protocols #2255

Merged
merged 2 commits into from
Jul 7, 2022

Conversation

bcressey
Copy link
Contributor

@bcressey bcressey commented Jun 29, 2022

Issue number:
#2235
#2236

Description of changes:
Disable uncommon filesystem and network protocols. See the individual commit messages for the reasoning behind these choices.

Fix some unrelated errors in the existing config that triggered warnings at build time.

Testing done:
Verified that the expected filesystem and network features were removed from the kernel configurations.

diffconfig output:
https://gist.github.com/bcressey/f77d2807549dfb36acc19ba446a23510

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@bcressey
Copy link
Contributor Author

bcressey commented Jun 29, 2022

The risk here is that one of these features ends up being a breaking change for someone using one of the current variants across the supported platforms - AWS, VMware, and bare metal. The benefit is that by disabling all of these, we help to "solve" compliance with typical audits by removing the protocols and filesystem types that (almost) no one uses.

Typically we'd err on the side of making these changes in newer variants only, but that ends up deferring the potential compliance benefit for the next three to six months as the new 5.15 kernel finds its way into variants, which isn't awesome if the risk is low.

The filesystems that worry me most are UDF and HFS / HFS+ (for the "prepare images" use case), and SCTP (for telecom use cases). However, most popular solutions for creating UDF and HFS+ images don't appear to require mounting those filesystems. Also, SCTP isn't especially well-supported on either AWS or VMware today, and bare metal support is quite new and likely to be early in the qualification stage.

@markusboehme
Copy link
Member

The changes seem very reasonable to me.

Thinking a bit ahead here: How might we provide rarely used functionality to users in the same image without complicating the audit process for all those who don't use it? Would it be sufficient to prevent unprivileged users from triggering module auto-loads by the kernel (i.e. request_module code paths), e.g. via (a combination of) SELinux policy, modprobe block list, modprobe wrapper (kernel.modprobe sysctl)?

@markusboehme
Copy link
Member

Btw, disabling support for UDF reminded me of the CD-ROM approach to providing user-data for VMware variants, but after checking it seems we only ever supported ISO 9660 for that anyway.

Copy link
Contributor

@foersleo foersleo left a comment

Choose a reason for hiding this comment

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

Looks good. I had something like this in the pipeline, but not with as detailed reasoning on why we are fine dropping these things. Sorry, it took me so land that you had to step up yourself Ben.

@bcressey
Copy link
Contributor Author

Sorry, it took me so land that you had to step up yourself Ben.

@foersleo - Please don't worry! That's on me for not communicating better. I'm sorry. This particular PR corresponds to a decent chunk of the benchmark, percentage-wise, and I wanted to feel OK about trimming that content.

Would it be sufficient to prevent unprivileged users from triggering module auto-loads by the kernel (i.e. request_module code paths), e.g. via (a combination of) SELinux policy, modprobe block list, modprobe wrapper (kernel.modprobe sysctl)?

@markusboehme - I agree that we need something like this. UDF support is relevant for Azure and SCTP is likely to be relevant for telecom use cases. Variant-specific kernel configs might cover a little of that, but it's kind of unsatisfying to be told that just because you're running on Azure or bare metal that you must have these modules enabled.

Using SELinux policy is complicated by the fact that some CNI plugins, notably amazon-vpc-cni-k8s used for EKS, do not run as privileged. There's an inherent ambiguity there - generally containers shouldn't run as privileged, and this particular container doesn't need that flag, and therefore by the principle of least privilege it shouldn't have it. However, from the host's perspective, what that container does ought to be privileged, or at least not available to unprivileged containers.

Specifying the SELinux label for the container - which is a subset of "privileged" - would help, but it also means chasing various plugins and drivers to adopt that practice, with a lot of uncertainty around when fixes are deployed widely enough that the policy can actually block the access without breaking everything.

I've tried to avoid one possible solution - opt-in SELinux modules to further restrict access - since it complicates testing as well as documentation.

In this case I think something along the lines of what the CIS benchmark suggests might be best. For Bottlerocket, that would mean a settings API to allow certain modules to be silently blocked from loading:

[settings.kernel.modules.sctp]
allowed = false

Where we would render /etc/modprobe.conf with lines like this:

install sctp /bin/true

I'm planning to implement that here, and also to bring back UDF and SCTP support - because they're known to be useful, and so they can serve as examples in the benchmark in case there are other modules that someone wants to disable.

@bcressey
Copy link
Contributor Author

bcressey commented Jul 6, 2022

The above force push restores:

  • CONFIG_IP_SCTP=m
  • CONFIG_DLM=m (needed SCTP)
  • CONFIG_UDF_FS=m

I've also updated the diffconfig output linked from the test results.

I'll follow up with a separate PR to add settings.kernel.modules to deal with SCTP and UDF.

@bcressey
Copy link
Contributor Author

bcressey commented Jul 6, 2022

@foersleo do you want me to drop the two fix commits here in lieu of #2268? Otherwise I can rebase once #2264 lands to put them into the metal config.

@foersleo
Copy link
Contributor

foersleo commented Jul 7, 2022

@foersleo do you want me to drop the two fix commits here in lieu of #2268? Otherwise I can rebase once #2264 lands to put them into the metal config.

I think you will have to touch the commits either way in order to merge it. I'd say whatever is easier for you. As #2268 and other config related work will touch and hopefully tidy up quite a bit of the configs I am fine with you dropping the fixups here if it makes merging easier.

@markusboehme
Copy link
Member

do you want me to drop the two fix commits here in lieu of #2268?

I knew I had seen them fixed somewhere, but couldn't find it yesterday. Thought it best to create a ticket lest we miss it in the various config-related PRs going around right now.

@bcressey
Copy link
Contributor Author

bcressey commented Jul 7, 2022

Moved the fix commits to #2269.

It's not possible to rule out the existence of workloads using these
filesystems, but it is possible to make a series of educated guesses.

For Kubernetes variants, a CSI driver that supports the filesystem
would be required to use it for container storage. This is especially
true for network-based fileystems, because Bottlerocket does not ship
any of the userspace tools required.

Disabled network filesystems:
* afs - network-based, no CSI driver
* gfs2 - network-based, no CSI driver
* nfs v2 - obsoleted by later versions of NFS

Another use case would be containers that run with CAP_SYS_ADMIN and
mount full disk or filesystem images. Disabling these filesystems is
more of a judgment call, and comes down to whether the format is
obsolete, whether it's in common use, whether it's useful on current
platforms, and if it's consistently enabled across architectures.

Obsolete local filesystems:
* cramfs - read-only format, obsoleted by squashfs
* ecryptfs - obsoleted by native filesystem encryption
* ext2 - obsolete, handled by the ext4 driver
* ext3 - obsolete, handled by the ext4 driver
* romfs - obsoleted by initramfs

Uncommon local filesystems:
* hfs, hfsplus - not enabled on aarch64
* jfs - not enabled on aarch64
* jffs2 - not supported by current platforms
* nilfs2 - not enabled on aarch64
* ntfs - not enabled on 5.10 kernels
* ufs - not enabled on aarch64
* zonefs - not supported by current platforms

Note that a potential use case for hfsplus could be to generate DMG
files for OS X software installs. However, the more common approach
appears to be using `genisoimage` on Linux.

Signed-off-by: Ben Cressey <[email protected]>
These protocols are unlikely to be used. They might require special
hardware; they might just not be supported on the platforms where
Bottlerocket runs today; they might raise security concerns; or some
other reasoning might apply.

Requires special hardware or platform support:
* atm - an alternative to IP
* can - used in automative and industrial applications
* hsr - redundancy protocol for wired networks
* rfkill - controls RF switches on WiFi and Bluetooth cards

Raises security concerns:
* dccp - CVE-2020-16119, CVE-2018-1130
* rds - CVE-2021-45480, CVE-2019-11815
* tipc - CVE-2022-0435, CVE-2021-29646

Other reasons:
* af-rxrpc - only used by AFS, which is disabled
* l2tp - not enabled in 5.10 for x86_64

Signed-off-by: Ben Cressey <[email protected]>
@bcressey
Copy link
Contributor Author

bcressey commented Jul 7, 2022

The above force push removes the fix commits with no other changes.

Conveniently that also fixes the conflict with "develop" so I don't need to rebase.

@bcressey bcressey merged commit 9ccbf6a into bottlerocket-os:develop Jul 7, 2022
@bcressey bcressey deleted the disable-unused branch July 7, 2022 18:03
@arnaldo2792 arnaldo2792 mentioned this pull request Jul 7, 2022
1 task
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.

5 participants