pass LISTEN_* environment into container

[vrothberg]

Make sure that Podman passes the LISTEN_* environment into containers.
Similar to runc, LISTEN_PID is set to 1.

Also remove conditionally passing the LISTEN_FDS as extra files.
The condition was wrong (inverted) and introduced to fix #3572 which
related to running under varlink which has been dropped entirely
with Podman 3.0. Note that the NOTIFY_SOCKET and LISTEN_* variables
are cleared when running system service.

Fixes: #10443
Signed-off-by: Daniel J Walsh [email protected]
Signed-off-by: Valentin Rothberg [email protected]

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vrothberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

LGTM
@giuseppe @flouthoc @Luap99 @mheon PTAL

[rhatdan]

Does socket activation work properly after this?

[vrothberg]

/hold Will run more tests tomorrow

…

On Tue 24 Aug 2021 at 19:09, Daniel J Walsh ***@***.***> wrote: Does socket activation work properly after this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11316 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACZDRA67I3VY7E6DFDSUIU3T6PG4DANCNFSM5CXDSUAA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

[TomSweeneyRedHat]

@vrothberg you have a merge conflict.

[flouthoc]

LGTM
One doubt @vrothberg @giuseppe crun and runc both preserves FDs but runc explictly sets LISTEN_PID=1 when not specified should we do same in crun implementation instead of podman.

[giuseppe]

One doubt @vrothberg @giuseppe crun and runc both preserves FDs but runc explictly sets LISTEN_PID=1 when not specified should we do same in crun implementation instead of podman.

we can do that in crun as well, but I think it is still safer to set in Podman as this PR does

[flouthoc]

@vrothberg @giuseppe sure having this at podman is much safer also added a PR at crun to make sure we maintain behavior parity with runc.

[vrothberg]

@rhatdan, I finally got it working but only without selinux:

time->Wed Aug 25 19:52:35 2021
type=AVC msg=audit(1629913955.973:39584): avc: denied { accept } for pid=583176 comm="systemd-socket-" path="/run/user/1000/testing.sock" scontext=system_u:system_r:container_t:s0:c191,c843tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_stream_socket permissive=1

Do I need to perform some labeling on the FDs?

[rhatdan]

Are you creating the FDs? If yes, then you need to call label.SetSocketOpt(ProcessLabel) on them before leaking them.

[vrothberg]

/hold
Still facing SELinux issues in local tests.

[vrothberg]

selinux issue fixed by @rhatdan (https://github.com/containers/container-selinux/releases/tag/v2.167.0)

[vrothberg]

@containers/podman-maintainers PTAL

Will work on an improved test once selinux changes made it into Fedora.

/hold cancel

[TomSweeneyRedHat]

LGTM once @mheon's concern is addressed

[TomSweeneyRedHat]

Another F34 time out, restarted.

[mheon]

/lgtm

[eriksjolund]

When I read this quote from man sd_notify (regarding FDSTORE=1):
Any open sockets and other file descriptors which should not be closed during the restart may be stored this way.
I get the impression that it's not just sockets that could be passed into Podman via LISTEN_FDS because the file descriptors could have been previously stored with FDSTORE=1.

I'm a bit curious what any other file descriptors means. Maybe container-selinux needs to allow all kinds of file descriptors? (I don't know so much about SELINUX so sorry for any confusion on my part)

Taking a look at
containers/container-selinux@v2.165.1...v2.167.0
it seems the change was mostly about unix_stream_socket.

When I find some time for it, I will experiment and see how the socket activation works together with FDSTORE=1 and see whether running with Podman versus running without Podman makes any difference.
(But I have some other projects I will need to attend to first so it will take some time)

[vrothberg]

@eriksjolund, indeed while testing I ran into SELinux issues which @rhatdan has fixed in the meantime.

Feel free to drop a message when you find time to get back.

[rhatdan]

@vrothberg I think it is time for us to write a long blog on the integration between Podman and Systemd.

[vrothberg]

@vrothberg I think it is time for us to write a long blog on the integration between Podman and Systemd.

Started one this morning :-) I'll add you after PTO :^)

[rhatdan]

Sadly, I am not on PTO this afternoon (Customer meeting) or tomorrow for devconf.US.

[alexlarsson]

What if the user passed --init, then the main pid will be 2. Who is supposed to rewrite that. For sure catatonit (currently used on e.g. fedora) doesn't do that.

[flouthoc]

@alexlarsson This is been fixed in underlying runtime via containers/crun#721 if catatonit sets LISTEN_PID=2 crun will adhere it. We can remove this explicit LISTEN_PID=1 once latest crun is released and vendor-ed into the Podman CI.

However following edge case still remains if OCI runtime is runc this is only fixed for crun. cc @giuseppe

[vrothberg]

Fair point, @ alexlarsson. I think Podman should set it since we should not (and to a certain degree cannot) rely on the runtimes or inits to do this work.

Mind opening a PR against Podman?

[alexlarsson]

ftr, @giuseppe is fixing catatonit here: openSUSE/catatonit#15