Merge lp:~lotheac/lightdm/utmpx into lp:lightdm

Proposed by Lauri Tirkkonen on 2012-05-29
Status: Merged
Merged at revision: 1508
Proposed branch: lp:~lotheac/lightdm/utmpx
Merge into: lp:lightdm
Diff against target: 98 lines (+42/-8) 1 file modified
To merge this branch: bzr merge lp:~lotheac/lightdm/utmpx
Reviewer Review Type Date Requested Status
Robert Ancell 2012-05-29 Pending
LightDM Development Team 2012-05-29 Pending
Review via email: mp+107739@code.launchpad.net

This proposal supersedes a proposal from 2012-05-23.

Description of the Change

Write utmpx records for user sessions. This fixes bug #870297 in Ubuntu.

To post a comment you must log in.
Robert Ancell (robert-ancell) wrote : Posted in a previous version of this proposal

Hi, thanks for that!

40 + is_greeter = (class && 0 == strncmp(class, XDG_SESSION_CLASS_GREETER,
41 + sizeof XDG_SESSION_CLASS_GREETER));
This can be more simply done with:
is_greeter = g_strcmp0 (class, XDG_SESSION_CLASS_GREETER) == 0;

58 + if (!is_greeter) {
59 + /* Open the session */
60 + result = pam_open_session (pam_handle, 0);
You should still open a PAM session for the greeter, why not?

73 - /* Wait for the command to complete (blocks) */
You seem to have accidentally removed a comment :(

76 + if (!is_greeter) {
Braces start on a new line

86 + if (remote_host_name) {
87 + strncpy (ut.ut_host, remote_host_name, sizeof (ut.ut_host));
88 + } else {
89 + memset (ut.ut_host, 0, sizeof (ut.ut_host));
90 + }
Don't need braces for single line branches

review: Needs Fixing
Lauri Tirkkonen (lotheac) wrote : Posted in a previous version of this proposal

> 58 + if (!is_greeter) {
> 59 + /* Open the session */
> 60 + result = pam_open_session (pam_handle, 0);
> You should still open a PAM session for the greeter, why not?

Because if we don't, we can stack pam_lastlog as a session module in order to record user sessions to wtmp and lastlog. I could comment that, though.

> 73 - /* Wait for the command to complete (blocks) */
> You seem to have accidentally removed a comment :(

It's actually just been moved closer to the wait call :) line 101 in the diff

I'll try to get around to fixing the rest later today.

Robert Ancell (robert-ancell) wrote :

I've committed the core of this patch with the following changes:
- Don't stop opening PAM sessions for the greeter - the PAM configuration might require to set something up in the session hooks for the greeter so we need to run those (and it is a session, just not a user session). The correct way to solve this is to make the greeter use a different PAM service so it can have different configuration.
- Remove the record when the session exits
- Minor layout changes

Preview Diff

1=== modified file 'src/session-child.c'
2--- src/session-child.c 2012-05-15 23:56:33 +0000
3+++ src/session-child.c 2012-05-29 09:00:34 +0000
4@@ -11,6 +11,7 @@
5 #include <grp.h>
6 #include <glib.h>
7 #include <security/pam_appl.h>
8+#include <utmpx.h>
9
10 #include "session-child.h"
11 #include "session.h"
12@@ -182,6 +183,7 @@
13 gchar *console_kit_cookie;
14 const gchar *path;
15 GError *error = NULL;
16+ gboolean is_greeter = FALSE;
17
18 g_type_init ();
19
20@@ -334,6 +336,8 @@
21 return EXIT_FAILURE;
22 }
23
24+ is_greeter = g_strcmp0 (class, XDG_SESSION_CLASS_GREETER) == 0;
25+
26 /* Stop if we didn't authenticated */
27 if (authentication_result != PAM_SUCCESS)
28 return EXIT_FAILURE;
29@@ -384,13 +388,18 @@
30 g_printerr ("Failed to establish PAM credentials: %s\n", pam_strerror (pam_handle, result));
31 return EXIT_FAILURE;
32 }
33-
34- /* Open the session */
35- result = pam_open_session (pam_handle, 0);
36- if (result != PAM_SUCCESS)
37+
38+ /* Don't open a PAM session for greeters. This way we can stack pam_lastlog
39+ * as a session module to record wtmp and lastlog for user sessions. */
40+ if (!is_greeter)
41 {
42- g_printerr ("Failed to open PAM session: %s\n", pam_strerror (pam_handle, result));
43- return EXIT_FAILURE;
44+ /* Open the session */
45+ result = pam_open_session (pam_handle, 0);
46+ if (result != PAM_SUCCESS)
47+ {
48+ g_printerr ("Failed to open PAM session: %s\n", pam_strerror (pam_handle, result));
49+ return EXIT_FAILURE;
50+ }
51 }
52
53 /* Open a connection to the system bus for ConsoleKit - we must keep it open or CK will close the session */
54@@ -524,9 +533,33 @@
55 return_code = EXIT_FAILURE;
56 }
57
58- /* Wait for the command to complete (blocks) */
59 if (child_pid > 0)
60 {
61+ if (!is_greeter)
62+ {
63+ /* This is a user session */
64+ struct utmpx ut;
65+ struct timeval tv;
66+ /* Log to utmp */
67+ ut.ut_type = USER_PROCESS;
68+ ut.ut_pid = child_pid;
69+ strncpy (ut.ut_line, tty + strlen ("/dev/"), sizeof (ut.ut_line));
70+ strncpy (ut.ut_user, username, sizeof (ut.ut_user));
71+ strncpy (ut.ut_id, xdisplay, sizeof (ut.ut_id));
72+ if (remote_host_name)
73+ strncpy (ut.ut_host, remote_host_name, sizeof (ut.ut_host));
74+ else
75+ memset (ut.ut_host, 0, sizeof (ut.ut_host));
76+ gettimeofday (&tv, NULL);
77+ ut.ut_tv.tv_sec = tv.tv_sec;
78+ ut.ut_tv.tv_usec = tv.tv_usec;
79+ setutxent ();
80+ if (NULL == pututxline (&ut))
81+ g_printerr ("Failed to write utmpx: %s\n", strerror (errno));
82+ endutxent ();
83+ }
84+
85+ /* Wait for the command to complete (blocks) */
86 waitpid (child_pid, &return_code, 0);
87 child_pid = 0;
88 }
89@@ -560,7 +593,8 @@
90 ck_close_session (console_kit_cookie);
91
92 /* Close the session */
93- pam_close_session (pam_handle, 0);
94+ if (!is_greeter)
95+ pam_close_session (pam_handle, 0);
96
97 /* Remove credentials */
98 result = pam_setcred (pam_handle, PAM_DELETE_CRED);

Subscribers

People subscribed via source and target branches