After installing libpam-smbpass to allow other applications to authenticate against the Samba password datase, I found that one of our applications, which authenticates via PAM, hit a file descriptor leak; taking a look in lsof, and it looks like libpam-smbpass is leaking a number of file descriptors for each authentication: editshare 27036 editshare 24r CHR 1,9 0t0 1682 /dev/urandom editshare 27036 editshare 25u REG 8,5 36864 1048724 /var/lib/samba/account_policy.tdb editshare 27036 editshare 26u REG 8,5 425984 262567 /var/cache/samba/gencache.tdb editshare 27036 editshare 27ur REG 0,15 421888 6972 /run/samba/gencache_notrans.tdb editshare 27036 editshare 28u REG 8,5 696 1048666 /var/lib/samba/group_mapping.tdb editshare 27036 editshare 29u REG 8,5 45056 1053656 /var/lib/samba/private/secrets.tdb editshare 27036 editshare 30u REG 8,5 45056 1053656 /var/lib/samba/private/secrets.tdb editshare 27036 editshare 31r CHR 1,9 0t0 1682 /dev/urandom editshare 27036 editshare 32u REG 8,5 36864 1048724 /var/lib/samba/account_policy.tdb editshare 27036 editshare 33u REG 8,5 425984 262567 /var/cache/samba/gencache.tdb editshare 27036 editshare 34ur REG 0,15 421888 6972 /run/samba/gencache_notrans.tdb editshare 27036 editshare 35u REG 8,5 696 1048666 /var/lib/samba/group_mapping.tdb I've boiled this down into a simple test program in Python that reproduces the issue; this uses the pam module v0.1.4 from https://pypi.python.org/pypi/pam (packaged in recent versions of Debian/Ubuntu as "python-pampy", and in Fedora as "python-pam"). In order to reproduce the issue, the username and password should be correct: import pam import os for x in xrange(100): pam.authenticate('test', '1234') for f in os.listdir("/proc/self/fd"): path = os.path.join("/proc/self/fd", f) if os.path.islink(path): print "%s: %s" % (f, os.readlink(path)) I've reproduced this on my own build of Samba 4.1.7 on Ubuntu 12.04, as well as the official Ubuntu packages on Ubuntu 14.04.
I should also point out that for the repro steps, the username and password were authenticated against the Unix password database, not the Samba password database. While libpam-smbpass was installed and configured, the password was actually authenticated against the user's Unix password, no Samba password was ever configured for this user.
I should also point out that the above script must be run as root to reproduce the problem; otherwise you simply get an error about accessing secrets.tdb.
Can you give me more information on how you're driving pam here ? Also your smb.conf ? Are you using threads in your pam authentication calls ? Looking inside the source3/passdb/account_pol.c code (which is one of the places leaking fd's in your lsof trace) we only open the /var/lib/samba/account_policy.tdb file using the following code: static struct db_context *db; bool init_account_policy(void) { const char *vstring = "INFO/version"; uint32_t version = 0; int i; NTSTATUS status; if (db != NULL) { return True; } db = db_open(NULL, state_path("account_policy.tdb"), 0, TDB_DEFAULT, O_RDWR, 0600, DBWRAP_LOCK_ORDER_1); .... } Then all calls into the account policy code do: bool account_policy_get(enum pdb_policy_type type, uint32_t *value) { const char *name; uint32 regval; NTSTATUS status; if (!init_account_policy()) { return False; } ... } which is obviously not thread-safe, but should only open the account_policy.tdb file once per process. I'm not sure how we're hitting this problem... Jeremy.
No threads, nothing special, just bog standard pam_start(), pam_authenticate(), pam_end(). I can reproduce this with an empty smb.conf, so I'm using all default values for everything. The following config in /etc/pam.d/pamtest is sufficient to trigger the bug: #%PAM-1.0 auth sufficient pam_smbpass.so I included a test case to reproduce the issue in the original description, that depends only on Python and one small easily available module. But to make it even easier to reproduce, I'll attach a version translated into C, with no dependencies other than PAM itself. Compile with "gcc --std=gnu99 pamtest.c -lpam -o pamtest", then run as "sudo ./pamtest user pass", with the above configuration. The password can be correct or not, it doesn't really matter, but if it's not correct the test will take longer to run as you have to wait for timeouts.
Created attachment 10045 [details] libpam-smbpass fd leak test program
It's leaking these file descriptors because nothing is closing the files when the libraries are unloaded via dlclose(); each time through the pam_start(), pam_authenticate(), pam_end() cycle, another copy of libpam-smbpass is opened and closed. A similar bug was fixed in libpam-winbind; see bug 7684.
(In reply to comment #6) > It's leaking these file descriptors because nothing is closing the files when > the libraries are unloaded via dlclose(); each time through the pam_start(), > pam_authenticate(), pam_end() cycle, another copy of libpam-smbpass is opened > and closed. A similar bug was fixed in libpam-winbind; see bug 7684. My take on this would be to just remove pam_smbpass as quickly as possible. I don't see how we can make sure over time that we always correctly run down all resources we allocate properly. pam_smbpass links in half of Samba, and linking that into arbitrary processes by today's standards is a very bad idea, at least IMHO. Yes, all these missing proper rundown procedures are bugs, but none of them bite us when running a server. smbd and winbind are long-running processes that open the resources once and then just exit. The alternative is to run pam_winbind with winbind looking at passdb. We have to solve the issue that pam_winbind auth with unqualified users need to look at the local sam, but but with 'NETBIOSNAME\user' it already works today. Any opinions to remove pam_smbpass for 4.2?
Created attachment 10047 [details] Make unqualified users auth against passdb With this patch pam_winbind also seems to authenticate unqualified users against the local passdb and thus pam_winbind might be a valid successor of pam_smbpass
(In reply to comment #6) > It's leaking these file descriptors because nothing is closing the files when > the libraries are unloaded via dlclose(); each time through the pam_start(), > pam_authenticate(), pam_end() cycle, another copy of libpam-smbpass is opened > and closed. A similar bug was fixed in libpam-winbind; see bug 7684. Oh well that makes sense :-). Thanks. Yes, the pam_smbpass code certainly isn't designed to work that way, it's expected to remain resident once opened. I'll take a look at fixing it. Jeremy.
(In reply to comment #7) > (In reply to comment #6) > > It's leaking these file descriptors because nothing is closing the files when > > the libraries are unloaded via dlclose(); each time through the pam_start(), > > pam_authenticate(), pam_end() cycle, another copy of libpam-smbpass is opened > > and closed. A similar bug was fixed in libpam-winbind; see bug 7684. > > My take on this would be to just remove pam_smbpass as quickly as possible. I > don't see how we can make sure over time that we always correctly run down all > resources we allocate properly. pam_smbpass links in half of Samba, and linking > that into arbitrary processes by today's standards is a very bad idea, at least > IMHO. > > Yes, all these missing proper rundown procedures are bugs, but none of them > bite us when running a server. smbd and winbind are long-running processes that > open the resources once and then just exit. > > The alternative is to run pam_winbind with winbind looking at passdb. We have > to solve the issue that pam_winbind auth with unqualified users need to look at > the local sam, but but with 'NETBIOSNAME\user' it already works today. > > Any opinions to remove pam_smbpass for 4.2? + 1 Let's remove it. All the more, since the patch from the next comment should make it possible to use pam_winbindd instead... Michael
+1 to remove pam_smbpass for 4.2 with Volker's patch applied. Brian, does that work for you ? Also, what version of Samba are you using pam_smbpass with ? Jeremy.
I'm using Samba 4.1.7, our own package that's based on the Debian Samba 4.1.7 package plus a few of our own patches. I am perfectly happy dropping libpam-smbpass in favor of libpam-winbind as long as we can still set up Netatalk authenticate against the Samba password database I tried the patch that Volker supplied, and it looks like the underlying winbind authentication succeeds but we then fail to retrieve the account information because winbind returns "DOMAIN\user" instead of just "user": Jun 30 16:16:56 lambda-es7-cluster1 afpd[22048]: pam_winbind(netatalk:auth): getting password (0x00000000) Jun 30 16:16:56 lambda-es7-cluster1 afpd[22048]: pam_winbind(netatalk:auth): user 'advanced' granted access Jun 30 16:16:56 lambda-es7-cluster1 afpd[22048]: pam_unix(netatalk:account): could not identify user (from getpwnam(LAMBDA-ES7-CLUSTER1\advanced))
Much though I hate the parameter, would setting "winbind use default domain = yes" work here ?
No, it appears to have the same behavior if I set "winbind use default domain = yes"; authenticates correctly, but pam_unix fails to find an account because it can't find DOMAIN\user.
I should note that the behavior I mentioned for winbind use default domain was with Volker's patch. Without it, authentication just fails; with it, authentication succeeds but it can't look up the account info because pam_unix can't find DOMAIN\user.
(In reply to comment #12) > I'm using Samba 4.1.7, our own package that's based on the Debian Samba 4.1.7 > package plus a few of our own patches. > > I am perfectly happy dropping libpam-smbpass in favor of libpam-winbind as long > as we can still set up Netatalk authenticate against the Samba password > database > > I tried the patch that Volker supplied, and it looks like the underlying > winbind authentication succeeds but we then fail to retrieve the account > information because winbind returns "DOMAIN\user" instead of just "user": Hmm. Is there any way I can see that live? You know that I could ask Ralph to help set this up, but logging into such a box remotely might be more efficient :-) Thanks, Volker
samba-bugs@samba.org writes: > https://bugzilla.samba.org/show_bug.cgi?id=10669 > > --- Comment #16 from Volker Lendecke <vl@samba.org> 2014-07-01 08:33:45 UTC --- > (In reply to comment #12) >> I'm using Samba 4.1.7, our own package that's based on the Debian Samba 4.1.7 >> package plus a few of our own patches. >> >> I am perfectly happy dropping libpam-smbpass in favor of libpam-winbind as long >> as we can still set up Netatalk authenticate against the Samba password >> database >> >> I tried the patch that Volker supplied, and it looks like the underlying >> winbind authentication succeeds but we then fail to retrieve the account >> information because winbind returns "DOMAIN\user" instead of just "user": > > Hmm. Is there any way I can see that live? You know that I could ask Ralph to > help set this up, but logging into such a box remotely might be more efficient > :-) Sure. I've gotten a server VM and Mac client set up that you can play around with, but waiting on IT to get it accessible to the network without being on our internal network. I'll contact you off Bugzilla when that's set up and accessible.
pam-smbpass will be dropped with samba 4.4. See als the thread "Remove pam_smbpass module from Samba source code" from 2015 on samba-technical on the topic.
I've been using pam-smbpass for years to sync /etc/passwd to smbpasswd on my laptop (running Ubuntu). Now that it's gone in Ubuntu 16.04, I'm not sure how to achieve the same effect. The comments here seem to say that pam-winbind should be a replacement, but it seems you're advocating the idea of replacing authentication against /etc/shadow with authentication against smbpasswd? That's sounds like a pretty radical change and not really a replacement for pam-smbpass.
(In reply to iueoqre from comment #19) We realise that, but the code it was wrapping really wasn't written to be used as a library inside arbitrary processes, and was unmaintained. So while we are sympathetic, we also think unmaintained security code is a really bad idea.
(In reply to Andrew Bartlett from comment #20) Andrew, sorry if my comment sounded like a complaint. My intention was more to make sure that I understand the proposed approach. Thanks to the Samba Team for caring about security!