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

Allow mounting -v /run:/run without leaking .containerenv file to the host #14577

Closed
jistr opened this issue Jun 13, 2022 · 17 comments · Fixed by #14582
Closed

Allow mounting -v /run:/run without leaking .containerenv file to the host #14577

jistr opened this issue Jun 13, 2022 · 17 comments · Fixed by #14582
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@jistr
Copy link

jistr commented Jun 13, 2022

/kind feature

Description

Not sure if this is a bug or RFE, i guess it depends on the point of view 😃 .

Some of our containers are tightly coupled to the host, and they mount -v /run:/run. This results in creation of /run/.containerenv on the host machine, which makes programs running on the host machine (not in a container) think that they are running in a container.

I understand that "don't mount /run" is one way to solve this, but it may not be workable for all our containers. So i'd like us to have some option to mount -v /run:/run without breaking the host. Possible solutions that come to my mind are:

  • Make podman bind mount the /run/.containerenv file into the container, rather than creating it via a mechanic like touch /run/.containerenv. (I don't know how Podman creates the file, but given that the file exists on the host after container exits, i preseume Podman creates the file on filesystem inside the container, rather than bind mounting the file into the container namespace?)

  • Allow us to explicitly disable the creation of containerenv file for a given container. I guess that would mean adding something like --no-containerenv option to podman create and podman run commands.

Steps to reproduce the issue:

[root@dendrit ~]# systemd-detect-virt
none
[root@dendrit ~]# ls /run/.containerenv
ls: cannot access '/run/.containerenv': No such file or directory
[root@dendrit ~]# podman run -v /run:/run quay.io/fedora/fedora:35-x86_64 true
[root@dendrit ~]# systemd-detect-virt 
podman
[root@dendrit ~]# ls /run/.containerenv 
/run/.containerenv

Output of podman version:

Tested on 2 systems, one with:

podman version 4.1.0

and another with:

podman version 3.4.7

(I don't think the other debug info from the template is helpful here, but can provide it on request.)

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/main/troubleshooting.md)

Yes

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 13, 2022
@rhatdan
Copy link
Member

rhatdan commented Jun 13, 2022

If you volume mount /run on /run in a container then Podman must create a inode mount point to mount the .containerenv onto the directory. This is where tools look. Our choice would be to not create /run/.containerenv in the /run directory at all, which might break container processes looking for this file. Or what we do now. I don't think we have a great solution to this problem.

@cjeanner
Copy link

Allowing to pass a "--no-containerenv" or the like would report the "issues" on the user, not podman; and would allow anyone sharing /run on the host to not break their host. We're facing some really not-nice issues with that lately, and we can't remove the /run:/run yet - maybe later. But not even sure...

More over, reading how that file is managed, there may be other issues since we mount /run in multiple containers, some being privileged, some not; apparently, the content of that file may differ in such cases, but would be the content of the latest started container.... Sooo... yeah. Really, being able to NOT get that file with an option would be good (and probably the easiest thing to do).

Cheers,

C.

@mheon
Copy link
Member

mheon commented Jun 13, 2022

I think not creating a .containerenv if /run is a bind mount seems reasonable. Adding a flag is also possible but I'm starting to get a little wary - we're up to 120+ flags on podman run and I'd rather our manpages not turn into an unreadable novel.

@rhatdan
Copy link
Member

rhatdan commented Jun 13, 2022

I think we already do this elsewhere. but not sure we are cosistent? What do we do if user mounts on /run/secrets? /dev?
Would this be a breaking change that we need to wait for podman 5.0? We could just add a flag to containers.conf to indicate what we should do.

@cjeanner
Copy link

cjeanner commented Jun 14, 2022

@mheon one can make an "easy" if /run not in volumes, then create /run/.containerenv - IIRC there's something related to /dev, or maybe /dev/ptmx (or something like that) doing this kind of "tweaking".

Regarding version/breaking change : at least for OSP case, especially osp-17 on el9, we will need something in order to not get that file; we already had to hack our way around[1] and I'd rather avoid doing more of that, and get something more predictable and stable, like "podman doesn't create that file" :).

If there's a need to validate either solution with some scratch build, I'm available, ready to fire an osp-17 env on el9 for that.

Cheers,

C.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2075080, leading to this ugly thing: https://review.opendev.org/#/q/215d918651eef3eaef0e612fca2d35742a78e4fa

@giuseppe
Copy link
Member

if you don't need to modify /run on the host, you could set up an overlay for run: -v /run:/run:O. That will prevent the creation of /run/.containerenv

giuseppe added a commit to giuseppe/libpod that referenced this issue Jun 14, 2022
if /run is on a volume do not create the file /run/.containerenv as it
would leak outside of the container.

Closes: containers#14577

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
Copy link
Member

in any case, I think we should not create the file when /run is a volume, I've opened a PR: #14582

@cjeanner
Copy link

the content is modified from both sides (hosts and container). Basically, some containers are talking to services on the host via their socket, usually located in /run. Sooo... yeah. it's probably a terrible idea anyway ;). But "it's working" - until now at least. Thanks for that change proposal - shall we create an associated bugzilla at Red Hat in order to see this feature backported to el9?

@giuseppe
Copy link
Member

let's get it upstream first and see if other maintainers agree with the proposed fix.

If it is not an urgent feature to have we could just wait for the next rebase

@cjeanner
Copy link

It kind of is «urgent» for OSP actually :/. But we still have a couple of weeks before things get (really) nasty for us. We also have a workaround in the pipe already - I'll be happy to remove it once we get that proposal in (or anything that would allow to remove the workaround ;)).

Let's see how your proposal goes through the reviews!

@rhatdan
Copy link
Member

rhatdan commented Jun 14, 2022

@cjeanner does -v /run:/run:O work?

@cjeanner
Copy link

@rhatdan I'll have to run a test with that - I'll come back to you before the end of the week. If it works, that would be really nice - I didn't know about that option (and still have to read its meaning, actually). Sorry, I'm multiplexing tasks :(.

@giuseppe
Copy link
Member

-v /run:/run:O won't work if you want changes from the container to be persisted on the host. It mounts an overlay on top of /run inside the container

@cjeanner
Copy link

@giuseppe basically, container can access host (and other containers?) but host can't access container content - correct? If so, it MAY work. At least I could deploy the undercloud, but I'll need to work a bit further to get other nodes... Though that would require extensive testing imho.

@cjeanner
Copy link

@rhatdan just tested. While deploy is apparently OK, it breaks some services (compute) because multiple containers are talking via sockets created within the container, located in /run - so the :O param stop the propagation of those sockets :(. Worth a try, but it's not applicable for OSP. Thanks for the hint though, I'll try to remember about it if some matching cases happen :).

@rhatdan
Copy link
Member

rhatdan commented Jun 15, 2022

Yes I was wondering what would happen with sockets. @rhgoyal, is this what you would understand to happen?

@jistr
Copy link
Author

jistr commented Jun 15, 2022

@giuseppe Thanks for the fix!

TomSweeneyRedHat pushed a commit to TomSweeneyRedHat/podman that referenced this issue May 25, 2023
if /run is on a volume do not create the file /run/.containerenv as it
would leak outside of the container.

Closes: containers#14577

Backporting containers#14582 to v4.0-rhel to complete the backports to https://issues.redhat.com/browse/OCPBUGS-7522

Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Tom Sweeney <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants