Revert "pam_systemd: set $DBUS_SESSION_BUS_ADDRESS unconditionally"

This reverts commit 69bd76f2b90cd00c1596b2e2c05845a4d9596fd2.

$DBUS_SESSION_BUS_ADDRESS is again set only if the socket exists.

Quoting https://github.com/systemd/systemd/pull/11327#issuecomment-452019027:
> [setting $DBUS_SESSION_BUS_ADDRESS unconditionally] makes pam_systemd
> incompatible with installations and distributions where dbus was not
> configured with --enable-user-session, and the session dbus-daemon is started
> by autolaunching or dbus-launch (as opposed to dbus.socket). I don't think
> that's wise: using autolaunching or dbus-launch, and disabling or not
> installing dbus.socket and dbus.service on the systemd user instance, is our
> compatibility story for people who still need a D-Bus session bus per X11
> session for whatever reason.
>
> For example, Debian can currently do either way, with a dbus-user-session
> package strongly recommended but not actually mandatory. dbus-user-session
> requires libpam-systemd; if pam_systemd now requires dbus.socket (which is in
> the dbus-user-session package), that's a circular dependency, which we
> normally try hard to avoid.

For systems that use dbus.socket this doesn't matter much, because the
user session is ordered after the user managaer, which pulls in dbus.socket
very early. For example, when logging over ssh:

sshd[20796]: pam_systemd(sshd:session): pam-systemd initializing
sshd[20796]: pam_systemd(sshd:session): Asking logind to create session: uid=1001 pid=20796 service=sshd type=tty class=user desktop= seat= vtnr=0 tty= display= remote=yes remote_user= remote_host=::1
sshd[20796]: pam_systemd(sshd:session): Session limits: memory_max=n/a tasks_max=n/a cpu_weight=n/a io_weight=n/a
systemd[1]: Created slice User Slice of UID 1001.
systemd[1]: Starting User Runtime Directory /run/user/1001...
systemd-logind[1210]: New session 3796 of user guest.
systemd[1]: Started User Runtime Directory /run/user/1001.
systemd[1]: Starting User Manager for UID 1001...
systemd[20805]: pam_systemd(systemd-user:session): pam-systemd initializing
systemd[20805]: Starting D-Bus User Message Bus Socket.
...
systemd[20805]: Reached target Sockets.
systemd[20805]: Reached target Basic System.
systemd[1]: Started User Manager for UID 1001.
systemd[1]: Started Session 3796 of user guest.
sshd[20796]: pam_systemd(sshd:session): Reply from logind: id=3796 object_path=/org/freedesktop/login1/session/_33796 runtime_path=/run/user/1001 session_fd=13 seat= vtnr=0 original_uid=1001
sshd[20796]: pam_unix(sshd:session): session opened for user guest by (uid=0)

Hence, everything in the ssh session is ordered after the user instance.
And in the user instance, services should be orderd after dbus.socket using
inter-unit dependencies. dbus.socket in turns does
systemctl --user set-environment DBUS_SESSION_BUS_ADDRESS=unix:path=%t/bus.

So there should be no race between starting of the dbus socket and our check
if it exists.

The alternative would be to set the "DBUS_SESSION_BUS_ADDRESS=unix:path=%s/bus;autolaunch:".
AFAICT, this would work as well. But I don't see any case where it actually works
better. Since this is an area with many compatiblity concerns, let's stick to
the previous setup which seems to work well.

diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c
index a33d8f05c50fee73960af5dbed0680a15daa97f7..2fd930ec7d17d4a3c56d5ebb1d68540693f67222 100644 (file)
--- a/src/login/pam_systemd.c
+++ b/src/login/pam_systemd.c
@@ -196,13 +196,30 @@ static int export_legacy_dbus_address(
                 uid_t uid,
                 const char *runtime) {
 
-        _cleanup_free_ char *s = NULL;
+        const char *s;
+        _cleanup_free_ char *t = NULL;
         int r = PAM_BUF_ERR;
 
-        if (asprintf(&s, DEFAULT_USER_BUS_ADDRESS_FMT, runtime) < 0)
+        /* We need to export $DBUS_SESSION_BUS_ADDRESS because various applications will not connect
+         * correctly to the bus without it. This setting matches what dbus.socket does for the user
+         * session using 'systemctl --user set-environment'. We want to have the same configuration
+         * in processes started from the PAM session.
+         *
+         * The setting of the address is guarded by the access() check because it is also possible to compile
+         * dbus without --enable-user-session, in which case this socket is not used, and
+         * $DBUS_SESSION_BUS_ADDRESS should not be set. An alternative approach would to not do the access()
+         * check here, and let applications try on their own, by using "unix:path=%s/bus;autolaunch:". But we
+         * expect the socket to be present by the time we do this check, so we can just as well check once
+         * here. */
+
+        s = strjoina(runtime, "/bus");
+        if (access(s, F_OK) < 0)
+                return PAM_SUCCESS;
+
+        if (asprintf(&t, DEFAULT_USER_BUS_ADDRESS_FMT, runtime) < 0)
                 goto error;
 
-        r = pam_misc_setenv(handle, "DBUS_SESSION_BUS_ADDRESS", s, 0);
+        r = pam_misc_setenv(handle, "DBUS_SESSION_BUS_ADDRESS", t, 0);
         if (r != PAM_SUCCESS)
                 goto error;