-
Notifications
You must be signed in to change notification settings - Fork 2k
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 noswap to secretdir tmpfs #24645
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @chuckyz, nice to hear from you!
This is a great idea, but the noswap
option was only introduced in Linux 6.4 and so we can't apply it blindly (many users have older kernels). For example, on Ubuntu 22.04:
$ uname -r
5.15.0-126-generic
$ mkdir /home/tim/secrets
$ sudo mount -t tmpfs -o size=100M,noswap tmpfs /home/tim/secrets
mount: /home/tim/secrets: wrong fs type, bad option, bad superblock on tmpfs, missing codepage or helper program, or other error.
$ sudo mount -t tmpfs -o size=100M tmpfs /home/tim/secrets
$ mount | grep secrets
tmpfs on /home/tim/secrets type tmpfs (rw,relatime,size=102400k,inode64)
This presents as follows in a build from this PR:
Recent Events:
Time Type Description
2024-12-11T08:49:39-05:00 Not Restarting Error was unrecoverable
2024-12-11T08:49:39-05:00 Task hook failed task_dir: mount: invalid argument
But I'd love to find a way to get this in because it's a good incremental improvement to security. We just need a way to test for this option at runtime (maybe try with swap and then fallback to not using it? I'd like to avoid checking the kernel version directly because the various Frankenkernels out there make that less useful).
Also, once we've got that figured out can you run make cl
to add a changelog item? Thanks!
Hey @tgross that sounds great. Another option would potentially be I personally strongly prefer the idea of just sticking with tmpfs. I was going to call |
If we do that, we won't be applying it on RHEL9, where the kernel version is 5.14 but they've apparently backported the |
Edit: I'm calling Gave this a shot, got tests mostly working locally. Would you consider maybe accepting Also I got fancy with this solution and then realized KISS and tried to make this way simpler (catch error once, then try again before returning a real error). For reference I'm running tests on a Macbook M3 with the following VM setup managed by Orbstack using go 1.23.4:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @chuckyz! One last item: can you run make cl
to add a changelog entry. Something like "client: add noswap directive to secrets directory where supported on Linux"
Should be good to go! |
Description
This adds
noswap
mount option to theSecretDir
mount.Testing & Reproduction steps
(having a hell of a time trying to get tests to run locally so I'm going to rely on GHA to run unit tests (fingers crossed))
Links
todo issue
Contributor Checklist
changelog entry using the
make cl
command.and job configuration, please update the Nomad website documentation to reflect this. Refer to
the website README for docs guidelines. Please also consider whether the
change requires notes within the upgrade guide.
Reviewer Checklist
backporting document.
in the majority of situations. The main exceptions are long-lived feature branches or merges where
history should be preserved.