kured icon indicating copy to clipboard operation
kured copied to clipboard

Make nsenter in reboot command optional

Open bh185140 opened this issue 1 year ago • 8 comments

Following https://github.com/kubereboot/kured/pull/814 which added signal based reboot we could reduce the privilege of the kured pods. With this we could also look into reducing the privileges for the reboot command method which currently is limited by the need to enter the host namespace.

If nsenter was made optional or configurable, this allows a lot more flexibility in setting up reboot methods that are more secure. We can imitate signal reboot by sending signals as a command through /bin/kill. This can also indirectly solve https://github.com/kubereboot/kured/issues/868 as we could also make the reboot command touch a reboot file.

bh185140 avatar Feb 08 '24 12:02 bh185140

But this only would work, if you can use binaries which are already inside the kured-image right? So maybe you use a custom-image?

ckotzbauer avatar Feb 09 '24 16:02 ckotzbauer

But this only would work, if you can use binaries which are already inside the kured-image right? So maybe you use a custom-image?

Yes, for the use cases I've listed

  • systemd shutdown through signal can be done with /bin/kill already in the image
  • https://github.com/kubereboot/kured/issues/868 can be done with /bin/touch also already in the image on a mounted volume

I think it should be fairly small change that won't break any thing. Mainly need it for the reduced privileges as I can't use the new reboot signal methods since I need to run a small shell script in the command before hand.

bh185140 avatar Feb 12 '24 11:02 bh185140

Main driver for this is mostly I needed a mechanism to trigger shutdowns as well as reboot. I could open up a separate issue for it I suppose, not sure if there would be enough interest in supporting shutdown on a reboot daemon. Though this could be a smaller change to go in.

bh185140 avatar Feb 12 '24 15:02 bh185140

Thanks for the explanations. Yes, both binaries are already present. For the path-based method we could just create a file with go-code at a configurable, mounted location. Shutdowns are not directly in scope of kured, I think.

ckotzbauer avatar Feb 12 '24 18:02 ckotzbauer

Happy to open up a PR for this if you think this change could go into Kured. I see currently nsenter is done with /usr/bin/nsenter", fmt.Sprintf("-m/proc/%d/ns/mnt", pid), "-- where the pid is a constant 1. Can possibly replace it with a default argument --reboot-command-prefix="/usr/bin/nsenter -m/proc/1/ns/mnt --". Alternatively just make it boolean parameter on where or not we add nsenter. What do you think?

bh185140 avatar Feb 13 '24 09:02 bh185140

Raised a PR here if interested https://github.com/kubereboot/kured/pull/899/files

// PID set to 1, until we have a better discovery mechanism.

Saw this comment while making this. Could be another use case to support changing the PID as a configuration for rancher.

bh185140 avatar Feb 15 '24 15:02 bh185140

@ckotzbauer Gentle bump for this thread

bh185140 avatar Apr 04 '24 09:04 bh185140

Thanks for the hint, I did a review.

ckotzbauer avatar Apr 04 '24 11:04 ckotzbauer

This issue was automatically considered stale due to lack of activity. Please update it and/or join our slack channels to promote it, before it automatically closes (in 7 days).

github-actions[bot] avatar Jun 04 '24 01:06 github-actions[bot]