Bug 43997 - reload_passwd uses fgetpwent rather than getpwent, ignoring /etc/nsswitch.conf
Summary: reload_passwd uses fgetpwent rather than getpwent, ignoring /etc/nsswitch.conf
Status: RESOLVED FIXED
Alias: None
Product: accountsservice
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium enhancement
Assignee: Matthias Clasen
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-20 22:03 UTC by Caleb Callaway
Modified: 2012-03-15 16:47 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Use getpwent() instead of fgetpwent() (2.22 KB, patch)
2011-12-20 22:03 UTC, Caleb Callaway
Details | Splinter Review
fix-cached-remote-users.patch (511 bytes, patch)
2011-12-22 06:56 UTC, Cédric Schieli
Details | Splinter Review

Description Caleb Callaway 2011-12-20 22:03:48 UTC
Created attachment 54623 [details] [review]
Use getpwent() instead of fgetpwent()

I'm one of the affected users from https://bugs.launchpad.net/ubuntu/+source/accountsservice/+bug/873784 with some additional comments.

I notice a recent commit ((http://cgit.freedesktop.org/accountsservice/commit/?id=dfa1a6239b01c823ce0fec781c6c9541c988f56e) enables getpwent() for systems that don't have fgetpwent(). I think it makes sense to do the same for every OS, and remove all references to fgetpwent(). The code changes to use getpwent() are fairly trivial--a patch is attached. I've tested these changes on a VM running Ubuntu 11.10 (Oneiric), local passwd entries seem to work fine, so as far as I can tell, this change doesn't break anything.

Using the general getpwent() command doesn't completely address the use of network pw entries, though. If the accounts daemon starts before a network connection is available, it won't be able to obtain information from the network pw entry database. 

I think the best way to resolve the connectivity issue is to modify the accounts daemon to regularly poll the pw entries for new users and notify interested services, in much the same way the daemon currently monitors the passwd file. It doesn't seem like a good idea to delay the start of the accounts daemon until a network connection is available (particularly for laptops). Running the accounts daemon as a service and restarting it when a network connection comes up would probably work, but seems over-weight and clunky.

Does anyone have objections or know that the polling scheme is impractical? If not, I'll get to work on a patch.
Comment 1 Ray Strode [halfline] 2011-12-21 07:16:19 UTC
Hi,

accountsservice is supposed to list a merged list of two types of users:

1) local users
2) remote users that have logged in to this specific machine before

we approximate 1) by reading /etc/passwd and we approximate 2) by looking at wtmp.
I say approximate, because there are configurations where both 1 and 2 fall down.

getpwent() is always wrong since it's blocking and potentially very slow, and gives users that aren't in class 1 or 2 above.  The commit you referenced is wrong and probably shouldn't have gone in.
Comment 2 Cédric Schieli 2011-12-22 06:56:29 UTC
Created attachment 54700 [details] [review]
fix-cached-remote-users.patch

"accountsservice is supposed to list a merged list of two types of users"

but it does not...

The problem is that when /etc/passwd users are (re)loaded, wtmp users missing from /etc/passwd are removed from the cached users hashtable, leading to the situation reported in the above mentioned Ubuntu bug.

With the attached patch applied, the loading order is inverted, so remote wtmp users are (re)added after the (re)loading of /etc/passwd users, and the Ubuntu greeter works as expected.
Comment 3 Ray Strode [halfline] 2011-12-22 07:11:06 UTC
Hi,

Thanks for the patch!  We actually fixed that a while back, though:

http://cgit.freedesktop.org/accountsservice/commit/?id=b73f539602f74271650a102d90253dc697488478
Author: Ray Strode <rstrode@redhat.com>
Date:   Wed Sep 21 17:20:24 2011 -0400

    daemon: load wtmp after passwd file
    
    The passwd file handling code clears the list of loaded
    users so we need to always do wtmp on top of it, instead of
    before it.

You probably just need to upgrade your accountsservice to a newer version.
Comment 4 Cédric Schieli 2011-12-22 12:04:50 UTC
Indeed. I didn't look carefully enough in git before posting...
Ubuntu Oneiric is at 0.6.14, just one commit behind this one...
Comment 5 Caleb Callaway 2011-12-23 19:39:11 UTC
Hi Ray, thanks for the clarification.

Do you have any more details on the situations where the wtmp lookup "falls down"? In my experimentation related to this bug report, I've found the presence and absence of remote users to be quite erratic--sometimes remote users are listed, and sometimes not. I've yet to detect any pattern.

I'm querying the DBus interface using qdbusviewer, so it seems unrelated to the logon manager (gdm). Remote logins are very definitely present at the top of the output from the 'last' command.
Comment 6 Ray Strode [halfline] 2012-01-04 06:42:16 UTC
By falls down I meant the wtmp file is often log rotated so users will "drop off the map" if they log in one day and the next day the file is rotated.

The display manager does wtmp registration for the user, so if some logins aren't counting it could be a GDM (or LightDM if that's what you're using) bug.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.