gistfile1.txt

--- session-child.c.orig	2018-09-05 11:03:31.000000000 +0930
+++ session-child.c	2022-09-23 18:09:16.132448000 +0930
@@ -13,9 +13,11 @@
 #include <grp.h>
 #include <glib.h>
 #include <security/pam_appl.h>
-#include <utmp.h>
 #include <utmpx.h>
 #include <sys/mman.h>
+#if HAVE_SETUSERCONTEXT
+#include <login_cap.h>
+#endif

 #if HAVE_LIBAUDIT
 #include <libaudit.h>
@@ -192,28 +194,6 @@
     return x_authority_new (x_authority_family, x_authority_address, x_authority_address_length, x_authority_number, x_authority_name, x_authority_data, x_authority_data_length);
 }

-/* GNU provides this but we can't rely on that so let's make our own version */
-static void
-updwtmpx (const gchar *wtmp_file, struct utmpx *ut)
-{
-    struct utmp u;
-    memset (&u, 0, sizeof (u));
-    u.ut_type = ut->ut_type;
-    u.ut_pid = ut->ut_pid;
-    if (ut->ut_line)
-        strncpy (u.ut_line, ut->ut_line, sizeof (u.ut_line));
-    if (ut->ut_id)
-        strncpy (u.ut_id, ut->ut_id, sizeof (u.ut_id));
-    if (ut->ut_user)
-        strncpy (u.ut_user, ut->ut_user, sizeof (u.ut_user));
-    if (ut->ut_host)
-        strncpy (u.ut_host, ut->ut_host, sizeof (u.ut_host));
-    u.ut_tv.tv_sec = ut->ut_tv.tv_sec;
-    u.ut_tv.tv_usec = ut->ut_tv.tv_usec;
-
-    updwtmp (wtmp_file, &u);
-}
-
 #if HAVE_LIBAUDIT
 static void
 audit_event (int type, const gchar *username, uid_t uid, const gchar *remote_host_name, const gchar *tty, gboolean success)
@@ -363,7 +343,6 @@
             ut.ut_tv.tv_sec = tv.tv_sec;
             ut.ut_tv.tv_usec = tv.tv_usec;

-            updwtmpx ("/var/log/btmp", &ut);

 #if HAVE_LIBAUDIT
             audit_event (AUDIT_USER_LOGIN, username, -1, remote_host_name, tty, FALSE);
@@ -393,7 +372,7 @@
         else
         {
             /* Set POSIX variables */
-            pam_putenv (pam_handle, "PATH=/usr/local/bin:/usr/bin:/bin");
+            pam_putenv (pam_handle, "PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:~/bin");
             pam_putenv (pam_handle, g_strdup_printf ("USER=%s", username));
             pam_putenv (pam_handle, g_strdup_printf ("LOGNAME=%s", username));
             pam_putenv (pam_handle, g_strdup_printf ("HOME=%s", user_get_home_directory (user)));
@@ -636,6 +615,29 @@
         if (setsid () < 0)
             _exit (errno);

+#if HAVE_SETUSERCONTEXT
+    /* Setup user context
+     * Reset the current environment to what is in the PAM context,
+     * then setusercontext will add to it as necessary as there is no
+     * option for setusercontext to add to a PAM context.
+     */
+    extern char **environ;
+    environ = pam_getenvlist (pam_handle);
+    struct passwd* pwd = getpwnam (username);
+    if (pwd) {
+	if (setusercontext (NULL, pwd, pwd->pw_uid, LOGIN_SETALL) < 0) {
+	    int _errno = errno;
+	    fprintf(stderr, "setusercontext for \"%s\" (%d) failed: %s\n",
+		    username, user_get_uid (user), strerror (errno));
+	    _exit (_errno);
+	}
+	endpwent();
+    } else {
+	fprintf (stderr, "getpwname for \"%s\" failed: %s\n",
+		    username, strerror (errno));
+	_exit (ENOENT);
+    }
+#else
         /* Change to this user */
         if (getuid () == 0)
         {
@@ -645,7 +647,7 @@
             if (setuid (uid) != 0)
                 _exit (errno);
         }
-
+#endif
         /* Change working directory */
         /* NOTE: This must be done after the permissions are changed because NFS filesystems can
          * be setup so the local root user accesses the NFS files as 'nobody'.  If the home directories
@@ -658,6 +660,7 @@
             int fd = log_file_open (log_filename, log_mode);
             if (fd >= 0)
             {
+                dup2 (fd, STDOUT_FILENO);
                 dup2 (fd, STDERR_FILENO);
                 close (fd);
             }
@@ -667,7 +670,13 @@
         signal (SIGPIPE, SIG_DFL);

         /* Run the command */
-        execve (command_argv[0], command_argv, pam_getenvlist (pam_handle));
+	execve (command_argv[0], command_argv,
+#if HAVE_SETUSERCONTEXT
+		environ
+#else
+		pam_getenvlist (pam_handle)
+#endif
+		);
         _exit (EXIT_FAILURE);
     }

@@ -708,7 +717,6 @@
             if (!pututxline (&ut))
                 g_printerr ("Failed to write utmpx: %s\n", strerror (errno));
             endutxent ();
-            updwtmpx ("/var/log/wtmp", &ut);

 #if HAVE_LIBAUDIT
             audit_event (AUDIT_USER_LOGIN, username, uid, remote_host_name, tty, TRUE);
@@ -749,7 +757,6 @@
             if (!pututxline (&ut))
                 g_printerr ("Failed to write utmpx: %s\n", strerror (errno));
             endutxent ();
-            updwtmpx ("/var/log/wtmp", &ut);

 #if HAVE_LIBAUDIT
             audit_event (AUDIT_USER_LOGOUT, username, uid, remote_host_name, tty, TRUE);
[cain2 18:10] ~/lightdm/work/lightdm-1.30.0/src >diff -u session-child.c.orig session-child.c
--- session-child.c.orig	2018-09-05 11:03:31.000000000 +0930
+++ session-child.c	2022-09-23 18:09:16.132448000 +0930
@@ -13,9 +13,11 @@
 #include <grp.h>
 #include <glib.h>
 #include <security/pam_appl.h>
-#include <utmp.h>
 #include <utmpx.h>
 #include <sys/mman.h>
+#if HAVE_SETUSERCONTEXT
+#include <login_cap.h>
+#endif

 #if HAVE_LIBAUDIT
 #include <libaudit.h>
@@ -192,28 +194,6 @@
     return x_authority_new (x_authority_family, x_authority_address, x_authority_address_length, x_authority_number, x_authority_name, x_authority_data, x_authority_data_length);
 }

-/* GNU provides this but we can't rely on that so let's make our own version */
-static void
-updwtmpx (const gchar *wtmp_file, struct utmpx *ut)
-{
-    struct utmp u;
-    memset (&u, 0, sizeof (u));
-    u.ut_type = ut->ut_type;
-    u.ut_pid = ut->ut_pid;
-    if (ut->ut_line)
-        strncpy (u.ut_line, ut->ut_line, sizeof (u.ut_line));
-    if (ut->ut_id)
-        strncpy (u.ut_id, ut->ut_id, sizeof (u.ut_id));
-    if (ut->ut_user)
-        strncpy (u.ut_user, ut->ut_user, sizeof (u.ut_user));
-    if (ut->ut_host)
-        strncpy (u.ut_host, ut->ut_host, sizeof (u.ut_host));
-    u.ut_tv.tv_sec = ut->ut_tv.tv_sec;
-    u.ut_tv.tv_usec = ut->ut_tv.tv_usec;
-
-    updwtmp (wtmp_file, &u);
-}
-
 #if HAVE_LIBAUDIT
 static void
 audit_event (int type, const gchar *username, uid_t uid, const gchar *remote_host_name, const gchar *tty, gboolean success)
@@ -363,7 +343,6 @@
             ut.ut_tv.tv_sec = tv.tv_sec;
             ut.ut_tv.tv_usec = tv.tv_usec;

-            updwtmpx ("/var/log/btmp", &ut);

 #if HAVE_LIBAUDIT
             audit_event (AUDIT_USER_LOGIN, username, -1, remote_host_name, tty, FALSE);
@@ -393,7 +372,7 @@
         else
         {
             /* Set POSIX variables */
-            pam_putenv (pam_handle, "PATH=/usr/local/bin:/usr/bin:/bin");
+            pam_putenv (pam_handle, "PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:~/bin");
             pam_putenv (pam_handle, g_strdup_printf ("USER=%s", username));
             pam_putenv (pam_handle, g_strdup_printf ("LOGNAME=%s", username));
             pam_putenv (pam_handle, g_strdup_printf ("HOME=%s", user_get_home_directory (user)));
@@ -636,6 +615,29 @@
         if (setsid () < 0)
             _exit (errno);

+#if HAVE_SETUSERCONTEXT
+    /* Setup user context
+     * Reset the current environment to what is in the PAM context,
+     * then setusercontext will add to it as necessary as there is no
+     * option for setusercontext to add to a PAM context.
+     */
+    extern char **environ;
+    environ = pam_getenvlist (pam_handle);
+    struct passwd* pwd = getpwnam (username);
+    if (pwd) {
+	if (setusercontext (NULL, pwd, pwd->pw_uid, LOGIN_SETALL) < 0) {
+	    int _errno = errno;
+	    fprintf(stderr, "setusercontext for \"%s\" (%d) failed: %s\n",
+		    username, user_get_uid (user), strerror (errno));
+	    _exit (_errno);
+	}
+	endpwent();
+    } else {
+	fprintf (stderr, "getpwname for \"%s\" failed: %s\n",
+		    username, strerror (errno));
+	_exit (ENOENT);
+    }
+#else
         /* Change to this user */
         if (getuid () == 0)
         {
@@ -645,7 +647,7 @@
             if (setuid (uid) != 0)
                 _exit (errno);
         }
-
+#endif
         /* Change working directory */
         /* NOTE: This must be done after the permissions are changed because NFS filesystems can
          * be setup so the local root user accesses the NFS files as 'nobody'.  If the home directories
@@ -658,6 +660,7 @@
             int fd = log_file_open (log_filename, log_mode);
             if (fd >= 0)
             {
+                dup2 (fd, STDOUT_FILENO);
                 dup2 (fd, STDERR_FILENO);
                 close (fd);
             }
@@ -667,7 +670,13 @@
         signal (SIGPIPE, SIG_DFL);

         /* Run the command */
-        execve (command_argv[0], command_argv, pam_getenvlist (pam_handle));
+	execve (command_argv[0], command_argv,
+#if HAVE_SETUSERCONTEXT
+		environ
+#else
+		pam_getenvlist (pam_handle)
+#endif
+		);
         _exit (EXIT_FAILURE);
     }

@@ -708,7 +717,6 @@
             if (!pututxline (&ut))
                 g_printerr ("Failed to write utmpx: %s\n", strerror (errno));
             endutxent ();
-            updwtmpx ("/var/log/wtmp", &ut);

 #if HAVE_LIBAUDIT
             audit_event (AUDIT_USER_LOGIN, username, uid, remote_host_name, tty, TRUE);
@@ -749,7 +757,6 @@
             if (!pututxline (&ut))
                 g_printerr ("Failed to write utmpx: %s\n", strerror (errno));
             endutxent ();
-            updwtmpx ("/var/log/wtmp", &ut);

 #if HAVE_LIBAUDIT
             audit_event (AUDIT_USER_LOGOUT, username, uid, remote_host_name, tty, TRUE);

Hi,

I'm trying to understand some of the changes you have here that are unclear to me.

• Line 49 and others, you are removing these calls to the wtmpx registering function, and remove the whole function and replace it with nothing. Why are you doing this? I think this is useful functionality (registering the session) and should be retained. We can replace the code here with something else but I would be against completely removing the functionality.

• Line 198, I can understand putting a better PATH here, and maybe we can make the patch in the port substitute LOCALBASE here, but I think adding ~/bin is not well advised. Do you have a logic why you choose this exact PATH string?

• Lines 206-228, Did you copy this code from somewhere? I have not gone through it still, but knowing if it has an origin would help evaluating it.

• Line 245, I'm not sure I get why you add this. Can you elaborate?

Apart from these it looks relatively right. I could rework it to something that can be added to the port.

Oh, now, scrub the first two questions, this is already included in the port and now I understand why that's so.

Sorry for the confusion.

Hi again, I have one more question about this patch:

Who is defining HAVE_SETUSERCONTEXT ?

configure script is not checking for it. If we want this to work, and be acceptable by upstream, we also need to adapt that.

I wrote this a year ago and have forgotten most of the context, I had a quick go at testing things now but all my systems are out of date so building it is a pain.

@DanielO

I imported your changes, and the required configure.ac changes, in a GH fork, you can look at the diff here: canonical/lightdm@main...madpilot78:lightdm:use_setusercontext

I'm testing this to integrate it in the port, but I stumbled on further issues.

The FreeBSD lightdm port does need some attention, I'm trying to integrate a bunch of fixes in it.