Wtmp #1077

moobyfr · 2018-03-23T12:30:15Z

Add the utmp/wtmp support.

metalefty · 2018-03-23T12:50:32Z

I'll rebase the wtmp branch to current devel head after March release.

speidy · 2018-03-27T04:58:28Z

can you squash some commits?
there's a WIP commits too.

speidy

what about other pam files like archlinux pam file ?

speidy · 2018-03-27T05:00:01Z

configure.ac

@@ -5,6 +5,7 @@ AC_INIT([xrdp], [0.9.6], [[email protected]])
 AC_CONFIG_HEADERS(config_ac.h:config_ac-h.in)
 AM_INIT_AUTOMAKE([1.7.2 foreign])
 AC_CONFIG_MACRO_DIR([m4])
+AC_GNU_SOURCE


Why this is needed?

Needed to get UTMPX_FILE and other variables on linux, because of __USE_GNU in utmpx.h

speidy · 2018-03-27T05:02:47Z

sesman/Makefile.am

@@ -61,6 +61,8 @@ xrdp_sesman_SOURCES = \
  sesman.h \
  session.c \
  session.h \
+  sessionrecord.c \


I don't like the name sessionrecord.c , its misleading.
one might think of graphical session recording.
can you think of a better name? maybe logontrack.c or just utmp.c?

First work used utmp.c and utmp.h but the #include directive was not able to get utmp.h from the sources and from /usr/include (with <> or "")

speidy · 2018-03-27T05:04:06Z

sesman/sessionrecord.c

+ * this can handle login and logout at once with the 'state' parameter
+ */
+
+int


make it static

speidy · 2018-03-27T05:04:53Z

sesman/sessionrecord.h

+
+#define XRDP_LINE_FORMAT "xrdp:%d"
+
+int add_xtmp_entry(int pid, const char *line, const char *user, const char *rhostname, short state);


why you export this function?
I dont see its used outside of sessionrecord.c.

speidy · 2018-03-27T05:08:51Z

sesman/sessionrecord.h

+typedef struct utmp _utmp;
+#endif
+
+#define XRDP_LINE_FORMAT "xrdp:%d"


put it in the .c file, its used only there.

speidy · 2018-03-27T05:14:31Z

sesman/sessionrecord.c

+ */
+
+int
+add_xtmp_entry(int pid, const char *line, const char *user, const char *rhostname, short state)


you never pass state argument when calling this function.
also, its unused here.

consider removing it.

They are used in utmp_login and utmp_logout, state is set to USER_PROCESS or DEAD_PROCESS

oh, right. my fault.
so just make it const short state

speidy · 2018-03-27T05:16:59Z

sesman/sessionrecord.c

+    g_strncpy(ut.ut_host, hostname, sizeof(ut.ut_host));
+
+    /* utmp */
+    setutxent();


can you add a comment explaining those calls ?

speidy · 2018-03-27T05:18:05Z

sesman/sessionrecord.c

+
+    ut.ut_type=state;
+    ut.ut_pid = pid;
+    gettimeofday(&tv, NULL);


can we use g_time1 , g_time2 or g_time3 from os_calls.c ?
otherwise, consider adding another wrapper there (or add support to the current wrappers)

speidy · 2018-03-27T05:21:55Z

sesman/sessionrecord.c

+    endutxent();
+
+    /* wtmp : update on linux, FreeBSD uses utx */
+#ifdef HAVE_UTMPX_H


i prefer to avoid ifdefs inside the function body.
lets add some wrapper to this call outside the function and call it here.
for example:

#if defined(__FreeBSD__) static inline void updwtmp(...) { ... } #elif defined(HAVE_UTMP_H) static inline void updwtmp(...) { ... } #endif

speidy · 2018-03-27T05:24:26Z

sesman/sessionrecord.c

+ * this can handle login and logout at once with the 'state' parameter
+ */
+
+int


either add error checks and return error in such cases or make this function return void

speidy · 2018-03-28T18:51:30Z

sesman/sessionrecord.c

+
+    g_memset(&ut, 0, sizeof(ut));
+
+    ut.ut_type=state;


space between operators (=)

speidy · 2018-03-28T18:59:16Z

sesman/sessionrecord.c

+     * So the IP is the string up the two last colons
+     */
+    int i = g_strlen(rhostname) - 1;
+    while ((i>0) && (rhostname[i] != ':'))


This logic is very specific to the way the given string is built right now.
The string might be changed at the future.
I'm not sure what is the best way to get the IP, but maybe we can fetch it from other, more reliable, source?

Yes, this is very ad-hoc workaround right now and not the best.

moobyfr · 2018-03-29T19:26:23Z

I've made the changes requested (minus the gettimeofday and the pam file because he doesn't exist in this branch), but I'm unable to push it in the current branch. the change can be chery-picked on moobyfr@9b915fc

metalefty · 2018-03-29T23:53:57Z

I'll cherry-pick.

metalefty · 2018-03-29T23:54:26Z

and squash some commits.

boppbo · 2018-04-18T14:34:58Z

Regarding the arch pam file, system-login already contains pam_loginuid.so (system-remote-login includes system-login). But it is added as optional not as required, will this cause issues?

- renamed the two files, including the header was conflicting with official headers - configure look for utmp/utmpx headers, wo we know which struct to use - reworked the usage for linux, works mostly (last still showing 'gone' for loggued users)

like updwtmpx

So /proc/<uid>/loginuid will be filled by the uid of the user. This will fix entries in 'last' with "gone - no logout" instead of 'still logged in'

use g_*

Follow coding standard

NZJourneyMan · 2018-08-16T11:05:16Z

This branch now looks to be good to go? I'm interested in when it is likely to be merged as my organisation needs the wtmp changes before we can onboard xrdp in to production. We have been using a patch from this branch for testing and the results have been good.

shaneforsythe · 2019-03-13T03:04:12Z

What is the status of this Pull request?

NZJourneyMan · 2019-09-09T13:47:17Z

I see JonesB has copied this PR and added the missing reconnect functionality. The new PR can be found here: #1408. Is it possible to get this reviewed?

PaulJohnson · 2020-10-29T11:41:37Z

Can we get this accepted please.

metalefty · 2021-09-22T01:46:37Z

This should be redone after #1961. There's very few chance to get merged with current authentication architecture.

matt335672 · 2021-09-23T09:33:06Z

Anyone following this may find this comment useful, particularly on Red Hat and derivatives.

matt335672 · 2023-07-03T10:27:57Z

Can anyone following this thread please comment on #2745?

metalefty force-pushed the wtmp branch from d2e2d62 to 8dbda1a Compare March 26, 2018 06:12

speidy requested changes Mar 27, 2018

View reviewed changes

speidy requested changes Mar 28, 2018

View reviewed changes

sesman/sessionrecord.c Outdated

g_memset(&ut, 0, sizeof(ut));

ut.ut_type=state;

Copy link

Member

speidy Mar 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space between operators (=)

speidy reviewed Mar 28, 2018

View reviewed changes

moobyfr and others added 16 commits August 2, 2018 09:02

Initial support for utmp/wtmp on linux

9d6537a

remove trailing space

42c0ff4

sesman: add utmp files to makefile

477852a

don't forget include utmp.h

db86911

os_calls.h needs to be included to use g_snprintf()

ae31066

WIP utmp/wtmp

38786d4

- renamed the two files, including the header was conflicting with official headers - configure look for utmp/utmpx headers, wo we know which struct to use - reworked the usage for linux, works mostly (last still showing 'gone' for loggued users)

Add GNU_SOURCE to configure.ac: it allow usage f more GNU functions,

016037a

like updwtmpx

Remove utmp.{c,h}

8085d89

Include sys/time.h to use gettimeofday()

8447990

cosmetic changes

8493e08

Add the loginuid pam module, as we are starting a session.

7d8f7c1

So /proc/<uid>/loginuid will be filled by the uid of the user. This will fix entries in 'last' with "gone - no logout" instead of 'still logged in'

Add support for FreeBSD

8fc741e

Extract the exact ip from the client_ip

c61ea8d

don't forget a free

3ef2e5f

use g_*

More g_* functions usage

0041228

Follow coding standard

Move some definitions, and small fixes

4ff49d0

metalefty force-pushed the wtmp branch from 1160fe8 to 4ff49d0 Compare August 2, 2018 00:14

shaneforsythe mentioned this pull request Mar 14, 2019

XRDP not updating lastlog info. #1302

Closed

Jonesb mentioned this pull request Sep 6, 2019

Merged WTMP changes back in and fixed reconnect IP logging #1408

Closed

evanlinde mentioned this pull request Sep 7, 2019

pam tally failure counter not reset #262

Closed

NZJourneyMan mentioned this pull request Sep 10, 2019

Who is logged in? #870

Closed

metalefty closed this Sep 22, 2021

matt335672 mentioned this pull request Jun 30, 2023

Add utmp/wtmp support #2745

Merged

matt335672 deleted the wtmp branch February 21, 2024 14:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wtmp #1077

Wtmp #1077

moobyfr commented Mar 23, 2018

metalefty commented Mar 23, 2018

speidy commented Mar 27, 2018

speidy left a comment

speidy Mar 27, 2018

moobyfr Mar 27, 2018

speidy Mar 28, 2018

speidy Mar 27, 2018 •

edited

Loading

moobyfr Mar 27, 2018 •

edited

Loading

speidy Mar 27, 2018

speidy Mar 27, 2018

speidy Mar 27, 2018

speidy Mar 27, 2018

moobyfr Mar 27, 2018

speidy Mar 28, 2018 •

edited

Loading

speidy Mar 27, 2018

speidy Mar 27, 2018 •

edited

Loading

speidy Mar 27, 2018 •

edited

Loading

speidy Mar 27, 2018

speidy Mar 28, 2018

speidy Mar 28, 2018 •

edited

Loading

metalefty Mar 29, 2018

moobyfr commented Mar 29, 2018

metalefty commented Mar 29, 2018

metalefty commented Mar 29, 2018

boppbo commented Apr 18, 2018

NZJourneyMan commented Aug 16, 2018

shaneforsythe commented Mar 13, 2019

NZJourneyMan commented Sep 9, 2019

PaulJohnson commented Oct 29, 2020

metalefty commented Sep 22, 2021

matt335672 commented Sep 23, 2021

matt335672 commented Jul 3, 2023


		#define XRDP_LINE_FORMAT "xrdp:%d"

		int add_xtmp_entry(int pid, const char line, const char user, const char *rhostname, short state);

Wtmp #1077

Wtmp #1077

Conversation

moobyfr commented Mar 23, 2018

metalefty commented Mar 23, 2018

speidy commented Mar 27, 2018

speidy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speidy Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

moobyfr Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speidy Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speidy Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

speidy Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speidy Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moobyfr commented Mar 29, 2018

metalefty commented Mar 29, 2018

metalefty commented Mar 29, 2018

boppbo commented Apr 18, 2018

NZJourneyMan commented Aug 16, 2018

shaneforsythe commented Mar 13, 2019

NZJourneyMan commented Sep 9, 2019

PaulJohnson commented Oct 29, 2020

metalefty commented Sep 22, 2021

matt335672 commented Sep 23, 2021

matt335672 commented Jul 3, 2023

speidy Mar 27, 2018 •

edited

Loading

moobyfr Mar 27, 2018 •

edited

Loading

speidy Mar 28, 2018 •

edited

Loading

speidy Mar 27, 2018 •

edited

Loading

speidy Mar 27, 2018 •

edited

Loading

speidy Mar 28, 2018 •

edited

Loading