rootless-containers / rootlesskit

Linux-native "fake root" for implementing rootless containers
Apache License 2.0
991 stars 98 forks source link

Add execution branch for socket activation to correct LISTEN_PID #449

Closed alopukhov closed 4 months ago

alopukhov commented 4 months ago

Proposal on https://github.com/rootless-containers/rootlesskit/issues/448 Fixing LISTEN_PID env variable for target process if it equals to PID of [rootlless:parent] process.

Tested with dockerd. Extra $$ script is not required anymore.

systemd-socket-activate -E "XDG_RUNTIME_DIR=${XDG_RUNTIME_DIR}" -l 12312 \
./rootlesskit/rootlesskit --net=slirp4netns --slirp4netns-sandbox=true \
--slirp4netns-seccomp=true \
--disable-host-loopback \
--port-driver=slirp4netns \
--copy-up=/etc \
--copy-up=/run \
sh -c 'rm /run/docker ; exec dockerd -H fd://'

Can be demonstated.

> (timeout 10 systemd-socket-activate -l 12345 ./rootlesskit --pidns sh -c 'echo "\$\$ is $$; LISTEN_PID is $LISTEN_PID"' &); sleep 1; netcat localhost 12345
Listening on [::]:12345 as 3.
Communication attempt on fd 3.
Execing ./rootlesskit (./rootlesskit --pidns sh -c echo "\$\$ is $$; LISTEN_PID is $LISTEN_PID")
$$ is 11; LISTEN_PID is 11

Example output for v2.1.0:

(timeout 10 systemd-socket-activate -l 12345 ./rootlesskit --pidns sh -c 'echo "\$\$ is $$; LISTEN_PID is $LISTEN_PID"' &); sleep 1; netcat localhost 12345
Listening on [::]:12345 as 3.
Communication attempt on fd 3.
Execing ./rootlesskit (./rootlesskit --pidns sh -c echo "\$\$ is $$; LISTEN_PID is $LISTEN_PID")
$$ is 11; LISTEN_PID is 3868868
AkihiroSuda commented 4 months ago

cc @charliemirabile

charliemirabile commented 4 months ago

Looks good. Thanks for picking up where I left off. I was hesitant to introduce another whole process into the already somewhat complex way that rootlesskit launches the app as part of #429 especially for a first contribution and given my lack of experience with Go.

It is unfortunate that fork and exec are so tightly coupled in Go (though this of course helps with making the language suitable for cross platform development for other projects), since in C, it would be as easy as inserting a call to setenv in the code between fork and exec whereas here we need to introduce a whole other program as a shim and successfully plumb all the relevant info through it, but your implementation looks sane.

AkihiroSuda commented 4 months ago

Could you squash the commits?