Misterious comment.

[alejandro-colomar]

shadow/lib/utmp.c

Lines 374 to 377 in e78742e

	#if !defined(USE_PAM)
	/* This is done by pam_lastlog */
	updwtmpx(_PATH_WTMP, ut);
	#endif

Who calls pam_lastlog()? I don't see any explicit calls in our source code. This comment should mention the PAM call that we do call that does that.

@ikerexxe , @hallyn Do you know which PAM call does that?

[alejandro-colomar]

@Karlson2k Do you know about this?

[hallyn]

pam_lastlog is not a call our code would make. pam_lastlog.so is a pam modules which applications can call.

So this comment, which is under # !defined( USE_PAM ), is saying that, since we
are running without pam, we will do the updwtmpx() call ourselves in this case, to append an entry to wtmp.

So the comment is explaining why it's under that if!def.

[hallyn]

btw, looking at the pam source in ubuntu, pam_lastlog.c calls logwtmp(), which in turns (per the manpage) calls updwtmp().

So it looks correct.

[alejandro-colomar]

pam_lastlog is not a call our code would make. pam_lastlog.so is a pam modules which applications can call.

My doubt is: are we sure it will be called? Is there a chance that PAM will not do what we think it will do? That is, are we sure that pam_lastlog() is called if USE_PAM?

So this comment, which is under # !defined( USE_PAM ), is saying that, since we are running without pam, we will do the updwtmpx() call ourselves in this case, to append an entry to wtmp.

So the comment is explaining why it's under that if!def.

[hallyn]

pam_lastlog will only be called if the pam config specifies it.

On ubuntu, by default /etc/pam.d/login specifies it

serge@sl25:/etc/pam.d$ grep lastlog -n *
login:82:session    optional   pam_lastlog.so

But what do you really care? It's just a wtmp entry. If an admin doesn't want it, that's the admin's problem.

[alejandro-colomar]

Ahh, so maybe the comment should say something like this?:

wtmp(5) is PAM's responsibility

[Karlson2k]

Ahh, so maybe the comment should say something like this?:

wtmp(5) is PAM's responsibility

However, the problem is that PAM is not managing wtmp.

[hallyn]

Ahh, so maybe the comment should say something like this?:

wtmp(5) is PAM's responsibility

That doesn't really make sense to me. If PAM is installed, then sure. But
you can have a system without PAM. Your comment suggests that on such a
system wtmp would be unsupported, and that's not the case.

Again, the comment is a bit inverted, but what it's saying is that when PAM
is enabled, then we don't have to call that function because PAM is supposed
to do it then, but if we are built without PAM, then we have to do it.

It is not (as I understand it) "PAM's responsibility".

[alejandro-colomar]

Ahh, so maybe the comment should say something like this?:

wtmp(5) is PAM's responsibility

That doesn't really make sense to me. If PAM is installed, then sure. But you can have a system without PAM. Your comment suggests that on such a system wtmp would be unsupported, and that's not the case.

I was assuming that since the comment is inside ifdef USE_PAM, it is understood that it means wtmp(5) is PAM's responsibility if the system is using it.

Again, the comment is a bit inverted, but what it's saying is that when PAM is enabled, then we don't have to call that function because PAM is supposed to do it then, but if we are built without PAM, then we have to do it.

It is not (as I understand it) "PAM's responsibility".

How about this?

On a system using PAM, wtmp(5) is PAM's responsibility

[Karlson2k]

My information was incorrect.
wtmp (or wtmpdb) and lastlog (or lastlog2) is managed by PAM modules (if PAM is used).
While utmp is not managed by PAM modules (a module was implemented but never widely used).

[alejandro-colomar]

My information was incorrect. wtmp (or wtmpdb) and lastlog (or lastlog2) is managed by PAM modules (if PAM is used). While utmp is not managed by PAM modules (a module was implemented by never widely used).

Thanks!