Bug 1874282
| Summary: | dbus-x11 could start per-connect session bus when user does ssh X11 DISPLAY forwarding | |||
|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 8 | Reporter: | Andrew Mike <amike> | |
| Component: | dbus | Assignee: | David King <dking> | |
| Status: | CLOSED ERRATA | QA Contact: | Petr Schindler <pschindl> | |
| Severity: | urgent | Docs Contact: | ||
| Priority: | urgent | |||
| Version: | 8.2 | CC: | alanm, brclark, casantos, chaekim, cschalle, debarshir, djast, dking, jaeshin, jamills, jaredz, jeischma, jinjli, jko, jkoten, joboyer, jwedgeco, jwright, mclasen, mkolbas, mpoole, mvanderw, pandrade, rstrode, sbarcomb, schanzle, simon.matter, tpelka, tpopela | |
| Target Milestone: | rc | Keywords: | ZStream | |
| Target Release: | 8.0 | Flags: | pm-rhel:
mirror+
|
|
| Hardware: | Unspecified | |||
| OS: | Unspecified | |||
| Whiteboard: | ||||
| Fixed In Version: | dbus-1.12.8-12.el8 | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | ||
| Clone Of: | ||||
| : | 1916124 1916125 (view as bug list) | Environment: | ||
| Last Closed: | 2021-05-18 15:05:03 UTC | Type: | Bug | |
| Regression: | --- | Mount Type: | --- | |
| Documentation: | --- | CRM: | ||
| Verified Versions: | Category: | --- | ||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | ||
| Cloudforms Team: | --- | Target Upstream Version: | ||
| Embargoed: | ||||
| Bug Depends On: | ||||
| Bug Blocks: | 1916124, 1916125 | |||
|
Description
Andrew Mike
2020-08-31 21:55:12 UTC
So to fix this, I think we need hooks from ssh. Something like /etc/ssh/sshrc but for distro use (/usr/share/ssh/sshrc.d/* ?). Ideally, we'd be able to: 1. set GDK_BACKEND=x11 environment variable 2. run dbus-launch to start a session bus for just this ssh connection Note we would need some way to know the user is doing X forwarding, since we don't want to do any of the above if the user isn't forwarding the display. But, I think that should be enough to sufficiently disconnect the app from the user session that things work. Of course, we could also potentially use the same mechanism to do the `dbus-update-activation-environment` call mentioned in comment 0 to put the new DISPLAY in the user bus and systemd user manager environment, but I don't think that's quite a nice of a solution. Jakub, is this something we could potentially get in 8.4 ? Basically, we just want to be able run a script when the user connects that let's us tweak the environment the shell is run in, and run some commands. hmm, thinking about this more, we might be able to get away with profile.d snippets. something like:
/etc/profile.d/set-up-ssh-forwarding-session.sh
[ -z "$SSH_CONNECTION" ] && return
[ -z "$DISPLAY" ] && return
[ "${DISPLAY:0:1}" = ":" ] && return
[ "$SHLVL" -ne 1 ] && return
export GDK_BACKEND=x11
eval `dbus-launch --sh-syntax --exit-with-session`
If I remember well, I was filling some bug about handling of X11 forwarding when we planned RHEL8. But I do not remember the outcome, if there was any. What you suggest in the profile.d scripts makes sense for me. I believe it should not be shipped by ssh, but by some gnome application/desktop environment (it is probably up to you to chose where it will be more appropriate) so headless servers do not need to go through these hops. If it will be shipped by something that does not depend on dbus-launch, checking if the executable is in place before calling exec might make sense too. It would deserve some more testing before we would push this. As far as I remember, there is bunch of applications running in client/server mode (gnome-terminal), which might be solved by the above, but might need some other hops to work as expected. *** Bug 1686892 has been marked as a duplicate of this bug. *** The dbus-update-activation-environment invocation updates the variables in the DBUS instance, but note that if the user is already logged into the graphical console in addition to the SSH session, updating those variables may interfere with the existing console session. (E.g., after running dbus-update-activation-environment from the X11-forwarded SSH session, applications launched on the console desktop via DBUS may be started in the X11-forwarded session rather than on the console.) That's true, which is why i've suggested an alternative in comment 1. comment 1 was unintentionally private before, so I've opened it up now. (though, in general running multiple simultaneous "sessions" like this for the same user is iffy, and not well supported) (In reply to Ray Strode [halfline] from comment #3) > hmm, thinking about this more, we might be able to get away with profile.d > snippets. something like: > > /etc/profile.d/set-up-ssh-forwarding-session.sh > > [ -z "$SSH_CONNECTION" ] && return > [ -z "$DISPLAY" ] && return > [ "${DISPLAY:0:1}" = ":" ] && return > [ "$SHLVL" -ne 1 ] && return Should be: [ "$SHLVL" -ne 2 ] && return > export GDK_BACKEND=x11 > eval `dbus-launch --sh-syntax --exit-with-session` (In reply to Debarshi Ray from comment #14) > > [ "$SHLVL" -ne 1 ] && return > > Should be: > [ "$SHLVL" -ne 2 ] && return Not sure I'm following. We only want to run the script if it's an unnested shell. If the user runs "su" or something, we don't want to run the script again. So SHLVL has to be "1" if we want to run the script. If it's not equal to 1 the script should return without doing anything. am I missing something? (In reply to Ray Strode [halfline] from comment #15) > (In reply to Debarshi Ray from comment #14) > > > > [ "$SHLVL" -ne 1 ] && return > > > > Should be: > > [ "$SHLVL" -ne 2 ] && return > Not sure I'm following. I might be guilty of not properly testing the deployed code. I merely played around with the snippet in an interactive terminal and threw it at a customer, who mentioned that it had to be '2' for the snippet to take effect. I then echo:ed SHLVL from a throwaway shell script that I ran from an interactive shell. It came out as '2'. Sorry for the noise. :/ Broadly, I do not mind having this snippet provided in dbus-x11, but I have a few reservations. For RHEL9, switching to dbus-broker is, presumably, desirable, as that would be the logical outcome of an import from recent versions (F31, and onwards, I think) Fedora, where the default was changed there. If that is the case (and if it has not been discussed or considered, now is probably the time to do so), where does that leave dbus-x11, and should it continue to be installed as described in comment #12? This change uses dbus-launch instead of the more modern dbus-run-session, which in its man page specifically describes a use case of login shells over SSH. Should dbus-run-session be used instead? I am not convinced that a new dbus session bus per SSH session is the best way to set this up. Are we planning on using the user bus, as opposed to the session bus, for RHEL9? If that is the plan, then making this addition (or behaviour change, if you prefer) to RHEL8 would seem a little harder to undo during the RHEL9 transition. I am not against a change like this, but I would prefer we push users towards exporting a complete, functional session over the network (such as through Vino or gnome-remote-desktop_, rather than claiming that doing the with X11-forwarding over SSH is well-supported. Our test suite passes for all architectures. Applications over ssh start as expected. I couldn't able to reproduce this bug with dbus-1.12.8-12.el8.x86_64.rpm Responses inline: ##################################################################### > - Can you ensure that /etc/profile.d/ssh-x-forwarding.sh is on the system? yes, it is. > - What shell are you using? /bin/bash > - What is the value of GDK_BACKEND before you explicitly set it to x11? It seems to be unset when using FastX, but set when using "ssh -X ..." . gnome-terminal fails over ssh when GDK_BACKEND=x11, . FastX works with GDK_BACKEND=x11 > - Does it work for any other users on the same system? Same behavior for other users. ##################################################################### Attaching requested output files as well This change in dbus-1.12.8-12 appears to be responsible for Bug 1940067. (In reply to Dan Astoorian from comment #52) > This change in dbus-1.12.8-12 appears to be responsible for Bug 1940067. Agreed. This change breaks ssh for common use cases (including rsync) for users with "ForwardX11 yes" in their ~/.ssh/config. This is clearly a regression. Should it be promptly reverted? Does it meet customer expectations of "Enterprise Linux"? As I understand it, testing and refinement should be done in Fedora and/or CentOS Stream, not RHEL8. Why the ssh-x-forwarding.sh has --exit-with-session, what consumes input and will race with the shell, and ssh-x-forwarding.csh has --exit-with-x11. Maybe it should be better to tell users to run: $ dbus-launch gnome-terminal and so on on ssh -X connections; then it would be required to fix why it hangs at exit. Also, this is an issue for upstream, but --exit-with-session likely should use another approach to detect a SIGHUP in stdin file descriptor. As is now it will race with the shell for input, and users will see only a few of the characters they type actually go to the terminal. Untested, but maybe could do something like this, with a new option
specific for ss-x-forwarding.{c,}sh. Instead of the babysit function,
that races for tty input with the shell, have something like this:
ino_fd = inotify_init();
inotify_add_watch(ino_fd, "/dev/stdin", IN_CLOSE_NOWRITE);
<< proceed as inotify(7) example on a loop, waiting for the close event >>
do proper error checking, exiting at any failure, then just wait for the close
event, or, more simple, and somewhat like what it is already doing, spawn a
new shell, wait for it to exit, then just close stdin itself, so that the
parent shell will also exit.
Okay, I think it's now obvious relying on dbus-launch to kill the dbus-daemon automatically when the session exits isn't going to work.
The ssh connection will keep the X connection alive until it has no more clients, and dbus-launch is a client itself.
This means dbus-launch relies on detecting hang ups on the terminal to know when to exit, but in the case of single command ssh invocations, there's on terminal.
My first thought was add [ -z "$SSH_TTY" ] && return to the top of the profile.d script, but that would obviously cause a regression for the original fix in the single command case.
And the "eating keystrokes" problem Paulo has found makes it clear it's not even a good idea in the case where there is a tty.
Probably rather than relying on the tty, we should rely on the logind session, which is what the dbus-daemon should really be scoped to, since it actually defines the scope of the session.
I think if we're going to fix this with just shell script changes, we could do something like:
╎❯ cat /etc/profile.d/ssh-x-forwarding.sh
[ -z "$SSH_CONNECTION" ] && return
[ -z "$DISPLAY" ] && return
[ "${DISPLAY:0:1}" = ":" ] && return
[ "$SHLVL" -ne 1 ] && return
export GDK_BACKEND=x11
setsid -f /usr/libexec/dbus-1/dbus-start-ssh-session-bus
and then,
╎❯ /usr/libexec/dbus-1/dbus-start-ssh-session-bus
#!/bin/bash
exec >& /dev/null
eval `dbus-launch --sh-syntax`
while inotifywait /run/systemd/sessions/$XDG_SESSION_ID; do
session_state=$(loginctl show-session $XDG_SESSION_ID | sed -n 's/^State=\(.*\)/\1/p')
[ "$session_state" = "active" ] && continue
kill -TERM $DBUS_SESSION_BUS_PID
break
done
But it might be better to fix dbus-launch --exit-with-session to look at a logind session first and foremost if it's getting run in one.
actually this might be a little more fault tolerant:
#!/bin/bash
exec >& /dev/null
eval `dbus-launch --sh-syntax`
while
session_state=$(loginctl show-session $XDG_SESSION_ID | sed -n 's/^State=\(.*\)/\1/p')
[ "$session_state" = "active" ]
do
inotifywait /run/systemd/sessions/$XDG_SESSION_ID
done
kill -TERM $DBUS_SESSION_BUS_PID
(since it's unlikely, though possible, that the logind session could exit as soon as it starts before this script runs)
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (dbus bug fix and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2021:1670 (In reply to Ray Strode [halfline] from comment #57) ... > while > session_state=$(loginctl show-session $XDG_SESSION_ID | sed -n > 's/^State=\(.*\)/\1/p') > [ "$session_state" = "active" ] > do > inotifywait /run/systemd/sessions/$XDG_SESSION_ID > done Could the 'while' expression/list be simplified to: while loginctl show-session $XDG_SESSION_ID | grep -q ^State=active; do ... Relying on the exit value of grep and eliminating the session_state variable (hopefully 'unset' later in the final code). Sorry for the late suggestion. Perhaps for future release. I'm not sure how all the above has been fixed according to https://access.redhat.com/errata/RHBA-2021:1670 when it says it's fixed in dbus-1.12.8-12.el8. I've tested dbus-1.12.8-12.el8 and now also the newer dbus-1.12.8-12.el8_4.2 and it doesn't seem to fix the above because it still has the faulty snippets in /etc/profile.d/ssh-x-forwarding.[c]sh. Am I missing something? Simon, I believe the remaining issues are being tracked by bug 1940067 |