login(1) "Last login" hostname is too short
The hostname field for the "Last login" line is only 16 characters long, way
too short to be useful for IPv6 addresses, for example. This limitation seems
to come from
char ll_host; /* same as in utmp */
However, utmpx.h defines ut_host as 257 bytes long, so perhaps lastlog.h should
be changed to match. This might break existing lastlog files though?
Updated by Lauri Tirkkonen over 7 years ago
In addition to growing the size of struct lastlog, my solution:
- changes the lastlog file path to "/var/adm/lastlog.v2", and exposes that as _PATH_LASTLOG in lastlog.h. This makes sure existing binaries don't corrupt the file with writes differently sized than expected, and new binaries don't interfere with the old file.
- removes all lastlog writers from gate, except pam_unix_session
- makes pam_unix_session also report last login via a PAM_TEXT_INFO message, and therefore removes the need for most readers in gate (finger(1) is one exception because it doesn't do PAM). pam_unix_session will fall back to the old "/var/adm/lastlog" file using the old struct definition to report the last login if there is no record in the new file; it does not write to the old file, however.
The backward compatibility is thus such that:
- existing binaries will continue to write and read correct-sized records from the old file, which is, however, not updated on login by pam_unix_session any more. I don't believe this to be an issue because third-party consumers are few and far between (OpenSSH is one, but I know of no others)
- newly built third-party binaries which directly interpret lastlog will pick up the new size and _PATH_LASTLOG from the header, making them consume the correct file.
- existing last login date will be displayed to users logging in after upgrade, from the old file -- however, pam_unix_account does not read the old file; the only reason it even needs to look at lastlog is the 'sp_inact' shadow attribute, to deny logins if too much time has passed since the user's last login. This means that it is possible for such a user to be allowed a login after upgrading to the new lastlog, which they would not have been allowed otherwise, because pam_unix_account will think they have never logged in (which is apparently not a problem for this feature...) There is still ongoing discussion about this on the mailing list so this may or may not change.
It might bear further discussion whether we should remove the lastlog header entirely to prevent third party consumers from touching this private implementation detail, but I chose not to do so yet because this particular bug has already grown way larger than I anticipated.
Updated by Electric Monk about 7 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 6249f9725f411468c70516176806c553ac983270
commit 6249f9725f411468c70516176806c553ac983270 Author: Lauri Tirkkonen <firstname.lastname@example.org> Date: 2015-12-28T22:05:10.000Z 6057 login(1) "Last login" hostname is too short Reviewed by: Gary Mills <email@example.com> Reviewed by: Albert Lee <firstname.lastname@example.org> Reviewed by: Jason King <email@example.com> Reviewed by: Alex Wilson <firstname.lastname@example.org> Approved by: Dan McDonald <email@example.com>
Updated by Gordon Ross about 7 years ago
- Status changed from Closed to Feedback
Updated by Marcel Telka about 3 years ago
commit 2de0a7d66c00b4cb047dc93352fe8b77707d2838 Author: Dan McDonald <firstname.lastname@example.org> AuthorDate: Mon Jan 25 00:07:05 2016 -0500 Commit: Dan McDonald <email@example.com> CommitDate: Mon Jan 25 00:07:05 2016 -0500 Revert "6057 login(1) "Last login" hostname is too short" (Needs further testing.) This reverts commit 6249f9725f411468c70516176806c553ac983270.