bug#64937: ssh sessions in systemd

[Thorsten Kukuk via GNU coreutils Bug Reports]

On Wed, Aug 02, Bruno Haible wrote:

> Thorsten Kukuk wrote:

> > openssh is really special: it does not need a TTY for all kind of ssh
> > sessions, and thus only opens a TTY if needed after creating the
> > logind session. Thus the logind session does not contain the TTY 
> > informations.
> > systemd-logind v254 provides now an interface for this case, which
> > allows to set the TTY later. For openssh you need this patch:
> > 
> > https://github.com/thkukuk/utmpx/blob/main/patches/openssh/logind-set-tty.patch
> 
> That would make sense, yes. But I wonder:
> - Why is it possible to set the "type" of the session to "tty", without
>   specifying a value for the "tty"?

sshd specifies a tty, but a dummy one: "sshd".
And systemd/logind has a hack to delete this dummy entry, so that a
fallback hack becomes active, which tries to determine the tty in
another way. But this "hack" exists only for the dbus interface, not for
libsystemd...

I know that Lennart Poettering had discussions about systemd and
sshd with the openssh developers to clean up some of the stuff and to
find better solutions, but without much success.

> - While at it: Shouldn't OpenSSH also provide a value for the "remote_user"
>   property?

I think it should, but I don't know why they don't set "PAM_RUSER".

  Thorsten

-- 
Thorsten Kukuk, Distinguished Engineer, Senior Architect, Future Technologies
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nuernberg, 
Germany
Managing Director: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

bug#64937: session leaders pids

[Bruno Haible]

Thorsten Kukuk wrote:
> > * The pids now refer to the session leader, which is a parent (or ancestor)
> >   of the process which has allocated the pty.
> 
> Depends on your application. For the majority of applications, which we
> evaluated, the PIDs reported by logind were identical to the PID from utmp.

Well, for me it's the opposite: For all desktops that I evaluated, the
PIDs reported by sd_session_get_leader — which is the replacement that you
recommend in 
(and I don't see any other one) — are different from the PID in utmp.
Only for inbound ssh do I see the same PID.

* Wayland-based desktops:

  - GNOME:
systemd returns the PID of the gdm-session-worker process.
utmp contains the PID of the gdm-wayland-session process
(a child process of the former).

  - KDE:
systemd returns the PID of the sddm-helper process.
utmp contains the PID of the startplasma-wayland process
(a child process of the former).

* X11-based desktops:

  - Budgie:
systemd returns the PID of a lightdm process.
utmp contains the PID of the gnome-session-binary process
(a child process of the former).

  - Cinnamon:
systemd returns the PID of a lightdm process.
utmp contains the PID of the cinnamon-session process
(a child process of the former).

  - MATE:
systemd returns the PID of a lightdm process.
utmp contains the PID of the mate-session process
(a child process of the former).

  - Xfce:
systemd returns the PID of a lightdm process.
utmp contains the PID of the xfce4-session process
(a child process of the former).

  - SoaS:
systemd returns the PID of a lightdm process.
utmp contains the PID of a python3 process
(a child process of the former).

  - LXQt:
systemd returns the PID of the sddm-helper process.
utmp contains the PID of the lxqt-session process
(a child process of the former).

  - Sway:
systemd returns the PID of the sddm-helper process.
utmp contains the PID of the sway process
(a child process of the former).

  - LXDE:
systemd returns the PID of the lxdm-session process.
utmp contains no PID at all! (Oh oh, looks like a bug in lxsession.)


Find attached the experiments' outputs.

Bruno


outputs.tar.gz
Description: application/compressed-tar

bug#64937: ssh sessions in systemd

[Bruno Haible]

Thorsten Kukuk wrote:
> And systemd/logind has a hack to delete this dummy entry, so that a
> fallback hack becomes active, which tries to determine the tty in
> another way. But this "hack" exists only for the dbus interface, not for
> libsystemd...

I see some hack in systemd/src/basic/terminal-util.c, indeed...

Bruno

bug#64937: ssh sessions in systemd

[Bruno Haible]

Thorsten Kukuk wrote:
> > The only problem with this mapping is for the ut_line row, which
> > used to contain, for inbound ssh, the pty device name. It is not easy
> > to find the pty name in this situation; at least, none of the systemd
> > APIs and /run/systemd/** files that I looked at helped.
> 
> You mean you don't see a TTY on ssh sessions?

Yes, I see a session
  uid=1001 user=other state=active active=Y remote=Y \
  starttime=2023-08-01T15:19:25 seat=(null) leaderpid=40513 uiclass=user \
  pamservice=sshd vt=-1 type=tty tty=(null) \
  display=(null) desktop=(null) remotehost=::1 remoteuser=(null)

"uitype=tty" and "tty=(null)" don't make much sense together.

> openssh is really special: it does not need a TTY for all kind of ssh
> sessions, and thus only opens a TTY if needed after creating the
> logind session. Thus the logind session does not contain the TTY 
> informations.
> systemd-logind v254 provides now an interface for this case, which
> allows to set the TTY later. For openssh you need this patch:
> 
> https://github.com/thkukuk/utmpx/blob/main/patches/openssh/logind-set-tty.patch

That would make sense, yes. But I wonder:
- Why is it possible to set the "type" of the session to "tty", without
  specifying a value for the "tty"?
- While at it: Shouldn't OpenSSH also provide a value for the "remote_user"
  property?

Bruno

bug#65003: pinky's "Idle" column lost the unit

[Paul Eggert]

Thanks for reporting that; I installed the attached.From 93e30466ff6eec8a2cd66374e199017763821478 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 2 Aug 2023 06:47:13 -0700
Subject: [PATCH] pinky: fix "d" typo

Problem reported by Bruno Haible (bug#65003).
* src/pinky.c (idle_string): Fix recently-introduced typo:
missing "d" for "days".
---
 src/pinky.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pinky.c b/src/pinky.c
index 381e753b6..b6d411c85 100644
--- a/src/pinky.c
+++ b/src/pinky.c
@@ -147,7 +147,7 @@ static char const *
 idle_string (time_t when)
 {
   static time_t now = 0;
-  static char buf[INT_STRLEN_BOUND (intmax_t) + 2];
+  static char buf[INT_STRLEN_BOUND (intmax_t) + sizeof "d"];
   time_t seconds_idle;
 
   if (now == 0)
@@ -165,7 +165,7 @@ idle_string (time_t when)
   else
 {
   intmax_t days = seconds_idle / (24 * 60 * 60);
-  sprintf (buf, "%"PRIdMAX, days);
+  sprintf (buf, "%"PRIdMAX"d", days);
 }
   return buf;
 }
-- 
2.39.2

bug#64937: "who" reports funny dates

[Bruno Haible]

I wrote:
> The proposed patch is attached.

Oops, I missed a sizeof of the ut_id field. A corrected patch is attached.

>From 97be578b107e7b87e32e0c3c2d49dc550489415b Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Wed, 2 Aug 2023 01:32:55 +0200
Subject: [PATCH] maint: Update after gnulib module 'readutmp' changed

For year-2038 safety on Linux/{x86,arm}, use systemd APIs.

* configure.ac: Don't test whether 'struct utmp' and 'struct utmpx' have
the ut_host field; this is now done in gnulib's readutmp module.
* src/system.h (STREQ_LEN): Allow passing a third argument with value
-1.
* src/pinky.c: Test HAVE_STRUCT_XTMP_UT_HOST instead of HAVE_UT_HOST.
(print_entry): Support the situation where ut_line is a 'char *' rather
than a 'char[]' of fixed size. Likewise for ut_user and ut_host.
* src/who.c: Test HAVE_STRUCT_XTMP_UT_HOST instead of HAVE_UT_HOST.
(print_user): Support the situation where ut_line is a 'char *' rather
than a 'char[]' of fixed size. Likewise for ut_user and ut_host.
(print_deadprocs, print_login, print_initspawn, scan_entries): Likewise.
(make_id_equals_comment): Likewise for ut_id.
(who): Free resources before returning.
* src/users.c (users): Free resources before returning.
* src/local.mk: Link the programs 'pinky', 'uptime', 'users', 'who' with
$(READUTMP_LIB).
---
 configure.ac | 30 --
 src/local.mk |  6 ++
 src/pinky.c  | 41 ++--
 src/system.h |  6 +-
 src/users.c  |  1 +
 src/who.c| 59 
 6 files changed, 83 insertions(+), 60 deletions(-)

diff --git a/configure.ac b/configure.ac
index 33441a82f..afc1098f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -406,36 +406,6 @@ AC_DEFUN([coreutils_DUMMY_1],
 ])
 coreutils_DUMMY_1
 
-AC_MSG_CHECKING([ut_host in struct utmp])
-AC_CACHE_VAL([su_cv_func_ut_host_in_utmp],
-[AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include 
-   #include 
-   struct utmp ut;
-   int s = sizeof ut.ut_host;]])],
-  [su_cv_func_ut_host_in_utmp=yes],
-  [su_cv_func_ut_host_in_utmp=no])])
-AC_MSG_RESULT([$su_cv_func_ut_host_in_utmp])
-if test $su_cv_func_ut_host_in_utmp = yes; then
-  have_ut_host=1
-  AC_DEFINE([HAVE_UT_HOST], [1], [FIXME])
-fi
-
-if test -z "$have_ut_host"; then
-  AC_MSG_CHECKING([ut_host in struct utmpx])
-  AC_CACHE_VAL([su_cv_func_ut_host_in_utmpx],
-  [AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include 
- #include 
- struct utmpx ut;
- int s = sizeof ut.ut_host;]])],
-[su_cv_func_ut_host_in_utmpx=yes],
-[su_cv_func_ut_host_in_utmpx=no])])
-  AC_MSG_RESULT([$su_cv_func_ut_host_in_utmpx])
-  if test $su_cv_func_ut_host_in_utmpx = yes; then
-AC_DEFINE([HAVE_UTMPX_H], [1], [FIXME])
-AC_DEFINE([HAVE_UT_HOST], [1], [FIXME])
-  fi
-fi
-
 GNULIB_BOOT_TIME([gl_ADD_PROG([optional_bin_progs], [uptime])])
 
 AC_SYS_POSIX_TERMIOS()
diff --git a/src/local.mk b/src/local.mk
index cb9b39274..4d7df2789 100644
--- a/src/local.mk
+++ b/src/local.mk
@@ -317,6 +317,12 @@ src_who_LDADD += $(GETADDRINFO_LIB)
 src_hostname_LDADD += $(GETHOSTNAME_LIB)
 src_uname_LDADD += $(GETHOSTNAME_LIB)
 
+# for read_utmp
+src_pinky_LDADD += $(READUTMP_LIB)
+src_uptime_LDADD += $(READUTMP_LIB)
+src_users_LDADD += $(READUTMP_LIB)
+src_who_LDADD += $(READUTMP_LIB)
+
 # for strsignal
 src_kill_LDADD += $(LIBTHREAD)
 
diff --git a/src/pinky.c b/src/pinky.c
index 381e753b6..47abd7758 100644
--- a/src/pinky.c
+++ b/src/pinky.c
@@ -62,7 +62,7 @@ static bool include_home_and_shell = true;
 static bool do_short_format = true;
 
 /* if true, display the ut_host field. */
-#ifdef HAVE_UT_HOST
+#if HAVE_STRUCT_XTMP_UT_HOST
 static bool include_where = true;
 #endif
 
@@ -206,7 +206,8 @@ print_entry (const STRUCT_UTMP *utmp_ent)
 #define DEV_DIR_WITH_TRAILING_SLASH "/dev/"
 #define DEV_DIR_LEN (sizeof (DEV_DIR_WITH_TRAILING_SLASH) - 1)
 
-  char line[sizeof (utmp_ent->ut_line) + DEV_DIR_LEN + 1];
+#ifdef UT_LINE_SIZE
+  char line[DEV_DIR_LEN + UT_LINE_SIZE + 1];
   char *p = line;
 
   /* Copy ut_line into LINE, prepending '/dev/' if ut_line is not
@@ -214,7 +215,18 @@ print_entry (const STRUCT_UTMP *utmp_ent)
  absolute file name in ut_line.  */
   if ( ! IS_ABSOLUTE_FILE_NAME (utmp_ent->ut_line))
 p = stpcpy (p, DEV_DIR_WITH_TRAILING_SLASH);
-  stzncpy (p, utmp_ent->ut_line, sizeof (utmp_ent->ut_line));
+  stzncpy (p, utmp_ent->ut_line, UT_LINE_SIZE);
+#else
+  /* If ut_line contains a space, the device name starts after the space,
+ else at the beginning.  */
+  char *line = xmalloc (DEV_DIR_LEN + strlen (utmp_ent->ut_line) + 1);
+  char *space = strchr (utmp_ent->ut_line, ' ');
+  char *device = (space != NULL ? space + 1 : utmp_ent->ut_line);
+  if ( ! IS_ABSOLUTE_FILE_NAME (device))
+stpcpy (stpcpy (line, DEV_DIR_WITH_TRAILING_SLASH), device);
+  

bug#64937: "who" reports funny dates

[Thorsten Kukuk via GNU coreutils Bug Reports]

Hi,

On Tue, Aug 01, Bruno Haible wrote:

> Thorsten Kukuk wrote:
> > If you haven't seen yet, I made some time ago a mapping
> > between utmp struct entries and libsystemd functions: 
> > https://github.com/thkukuk/utmpx/blob/main/utmp-to-logind.md
> 
> Thanks. This is helpful.
> 
> The only problem with this mapping is for the ut_line row, which
> used to contain, for inbound ssh, the pty device name. It is not easy
> to find the pty name in this situation; at least, none of the systemd
> APIs and /run/systemd/** files that I looked at helped.

You mean you don't see a TTY on ssh sessions?

openssh is really special: it does not need a TTY for all kind of ssh
sessions, and thus only opens a TTY if needed after creating the
logind session. Thus the logind session does not contain the TTY 
informations.
systemd-logind v254 provides now an interface for this case, which
allows to set the TTY later. For openssh you need this patch:

https://github.com/thkukuk/utmpx/blob/main/patches/openssh/logind-set-tty.patch

> The major changes are:
> * Entries without BOOT_TIME or USER_PROCESS flag are gone. This includes
>   - runlevel and possibly "old time" / "new time" entries, which I don't know
> how to get from systemd,

systemd has no "runlevel", this entries don't exists.
I haven't found an application which creates "old time" / "new time"
entries, thus I think they can be ignored.

>   - entries for pseudo-terminals (xterm etc.) that are not login shells;
> I don't know how to get them from systemd either, and (IMO) they were
> uninteresting anyway,

Yes, this is a problem or not, because on the other side,  it solves a
problem. E.g.
* xterm creates entries for pseudo-terminals
* gnome-terminal does not create entries for pseudo-terminals

That was confusing in the past and is now consistent.

> * The pids now refer to the session leader, which is a parent (or ancestor)
>   of the process which has allocated the pty.

Depends on your application. For the majority of applications, which we
evaluated, the PIDs reported by logind were identical to the PID from utmp.

> * In the ut_line field ("Device" column) I added extra info about the
>   service name, in this case "sshd". I imagine that is at least as useful as
>   knowing the pty name ("pts/4").

See above, this is a special openssh problem, we need to teach openssh
to report the TTY to logind, too.

  Thorsten

-- 
Thorsten Kukuk, Distinguished Engineer, Senior Architect, Future Technologies
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nuernberg, 
Germany
Managing Director: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)