-
Notifications
You must be signed in to change notification settings - Fork 22
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
(#1795555) Provide a way for privileged users to interact with systemd user instances #258
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
systemd-rhel-bot
added
pr/needs-ci
Formerly needs-ci
pr/needs-review
Formerly needs-review
tracker/unapproved
Formerly needs-acks
and removed
pr/needs-ci
Formerly needs-ci
labels
Jan 25, 2022
systemd-rhel-bot
added
tracker/missing
Formerly needs-bz
tracker/unapproved
Formerly needs-acks
and removed
tracker/unapproved
Formerly needs-acks
tracker/missing
Formerly needs-bz
labels
Aug 8, 2022
systemd-rhel-bot
added
tracker/missing
Formerly needs-bz
tracker/unapproved
Formerly needs-acks
and removed
tracker/unapproved
Formerly needs-acks
tracker/missing
Formerly needs-bz
labels
Aug 27, 2022
systemd-rhel-bot
added
tracker/unapproved
Formerly needs-acks
and removed
tracker/unapproved
Formerly needs-acks
labels
Jan 24, 2023
@jacekmigacz I see this is still marked as Draft. Please double-check what is still missing and drop "Draft" label if appropriate. Btw, also please rebase this work on top of main and force push to rerun all CIs. |
--machine has been missing for a while in systemd-stdio-bridge this syntax can be switched to be more standard. v2: Support the old syntax too. timedatectl -H server1.myhostingcompany.com:5555/container1 Closes: #8071
The "x-machine-kernel" dbus address has been removed a long time ago, hence don't generate it either.
The immediately following container_get_leader() call validate the name anyway, no need to twice exactly the same way twice immediately after each other.
…id() Let's clean up hostname_is_valid() a bit: let's turn the second boolean argument into a more explanatory flags field, and add a flag that accepts the special name ".host" as valid. This is useful for the container logic, where the special hostname ".host" refers to the "root container", i.e. the host system itself, and can be specified at various places. let's also get rid of machine_name_is_valid(). It was just an alias, which is confusing and even more so now that we have the flags param.
So far when asked for augmented bus credentials and the process was already gone we'd fail fatally. Let's make this graceful instead, and never allow augmenting fail due to PID having vanished — unless the augmenting is the explicit and only purpose of the requested operation. This should be safe as clients have to explicitly query the acquired creds anyway and handle if they couldn't be acquired. Moreover we already handle permission problems gracefully, thus clients must be ready to deal with missing creds. This is useful to make selinux authorization work for short-lived client proceses. PReviously we'd augment creds to have more info to log about (the selinux decision would not be based on augmented data however, because that'd be unsafe), and would fail if we couldn't get it. Now, we'll try to acquire the data, but if we cannot acquire it, we'll still do the selinux check, except that logging will be more limited.
…cific container This is unfortunately harder to implement than it sounds. The user's bus is bound a to the user's lifecycle after all (i.e. only exists as long as the user has at least one PAM session), and the path dynamically (at least theoretically, in practice it's going to be the same always) generated via $XDG_RUNTIME_DIR in /run/. To fix this properly, we'll thus go through PAM before connecting to a user bus. Which is hard since we cannot just link against libpam in the container, since the container might have been compiled entirely differently. So our way out is to use systemd-run from outside, which invokes a transient unit that does PAM from outside, doing so via D-Bus. Inside the transient unit we then invoke systemd-stdio-bridge which forwards D-Bus from the user bus to us. The systemd-stdio-bridge makes up the PAM session and thus we can sure tht the bus exists at least as long as the bus connection is kept. Or so say this differently: if you use "systemctl -M lennart@foobar" now, the bus connection works like this: 1. sd-bus on the host forks off: systemd-run -M foobar -PGq --wait -pUser=lennart -pPAMName=login systemd-stdio-bridge 2. systemd-run gets a connection to the "foobar" container's system bus, and invokes the "systemd-stdio-bridge" binary as transient service inside a PAM session for the user "lennart" 3. The systemd-stdio-bridge then proxies our D-Bus traffic to the user bus. sd-bus (on host) → systemd-run (on host) → systemd-stdio-bridge (in container) Complicated? Well, to some point yes, but otoh it's actually nice in various other ways, primarily as it makes the -H and -M codepaths more alike. In the -H case (i.e. connect to remote host via SSH) a very similar three steps are used. The only difference is that instead of "systemd-run" the "ssh" binary is used to invoke the stdio bridge in a PAM session of some other system. Thus we get similar implementation and isolation for similar operations. Fixes: #14580
--machine hasn't been supported since 798c486 Closes: #8116
So far, the bridge always acted as if "--system" was used, i.e. would unconditionally connect to the system bus. Let's add "--user" too, to connect to the users session bus. This is mostly for completeness' sake. I wanted to use this when making sd-bus's ability to connect to other user's D-Bus busses work, but it didn't exist so far. In the interest of keeping things compatible the implementation in sd-bus will not use the new "--user" switch, and instead manually construct the right bus path via "--path=", but we still should add the proper switches, as preparation for a brighter future, one day.
…r of hostname_is_valid()
Commit validationTracker - Missing issue tracker ✋ The following commits need an inspection
Tracker validation🔴 Missing tracker or Unknown tracker type; type: 'unknown' Pull Request validationFailed🔴 Failed or pending checks - |
systemd-rhel-bot
added
tracker/missing
Formerly needs-bz
tracker/unapproved
Formerly needs-acks
and removed
tracker/unapproved
Formerly needs-acks
tracker/missing
Formerly needs-bz
labels
Aug 2, 2023
This comment was marked as duplicate.
This comment was marked as duplicate.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
dont-merge
needs-rebase
pr/needs-ci
Formerly needs-ci
pr/needs-review
Formerly needs-review
tracker/missing
Formerly needs-bz
tracker/unapproved
Formerly needs-acks
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.