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

Disable getty@service for containers #24743

Closed
wants to merge 1 commit into from

Conversation

dcermak
Copy link

@dcermak dcermak commented Sep 19, 2022

getty is not really useful in containers and can actually cause harm when the container is launched as privileged (i.e. it has access to the host's /dev/), as it will take over the hosts tty (and can even crash the host)

getty is not really useful in containers and can actually cause harm when the
container is launched as privileged (i.e. it has access to the host's /dev/),
as it will take over the hosts tty (and can even crash the host)
@github-actions github-actions bot added the units label Sep 19, 2022
@poettering
Copy link
Member

Hmm? Why would it have access to the host's /dev/? What container manager are you using that sets things up like that?

we already carry a condition ConditionPathExists=/dev/tty0, as an indicator if the VT subsystem is around. That should suffice, as container managers generally don't make the VT subsystem available.

Anyway, this PR appears wrong, it's a bug in your container manager to allow access to the host's /dev/.

Generally: we want to minimize explicit checks for container environments, and instead focus on checks that check for the availability of specific subsystems. Here we do it by checking for /dev/tty0 which will exactly in the case where VT is available, and otherwise won't. or to say this differently: if you decided to patch through the VT subsystem, then we should use it.

Hence closing.

@poettering poettering closed this Sep 19, 2022
@dcermak
Copy link
Author

dcermak commented Sep 20, 2022

podman with --privileged passes /dev/tty0 into the container and with getty@tty0 running inside the image leads to interesting crashes.

@poettering
Copy link
Member

poettering commented Sep 20, 2022

That's a bug. They really shouldn't do that. Passing access to the VT subsystem to a container is madness.

At the very least if you intend to run systemd inside of it.

Sorry, but this is something we expressly don't support with systemd...

@poettering
Copy link
Member

We document our expectations here:

https://systemd.io/CONTAINER_INTERFACE

If you run a container manager that doesn't follow that, then that's out of scope for us. Sorry!

@dcermak
Copy link
Author

dcermak commented Sep 21, 2022

That's a bug. They really shouldn't do that. Passing access to the VT subsystem to a container is madness.

At the very least if you intend to run systemd inside of it.

Sorry, but this is something we expressly don't support with systemd...

Fair enough, I have sent this upstream instead: containers/podman#15878

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants