nssldap-update-ignoreusers needs to be configurable to ignore users

[Joshua Kugler]

Binary package hint: libnss-ldap

# lsb_release -rd
Description: Ubuntu 10.04.1 LTS
Release: 10.04

# apt-cache policy libnss-ldap
libnss-ldap:
  Installed: 264-2ubuntu2
  Candidate: 264-2ubuntu2
  Version table:
 *** 264-2ubuntu2 0
        500 http://us.archive.ubuntu.com/ubuntu/ lucid/main Packages
        100 /var/lib/dpkg/status
     261-2.1ubuntu1 0
        500 http://us.archive.ubuntu.com/ubuntu/ jaunty/main Packages

Currently, nssldap-update-ignoreusers can only be configured to ignore users over a certain numeric UID. It blindly includes all users less than the configured UID. However, this breaks our setup. We have some system users (namely www-data and www-priv) that are in groups in LDAP. Thus, when you query the 'Subversion' group, you get back a list that includes www-priv. However, if you try to query the groups to which www-priv belongs, it fails to return the correct groups because it ignores www-priv, thus breaking privileges because the system then thinks www-priv is not in the Subversion group.

The only work around for now is to disable the run of nssldap-update-ignoreusers.

I would work on a patch to facilitate configuring users to *not* include in the ignore list if someone will commit to getting the patch accepted: we don't really want to maintain our own branch of one file in a package. :)

[Joshua Kugler (jkugler)]

Mathias -

OK, I know not every bug can be "High," but can you explain why a bug that breaks permissions has been classified as "Low?"

Not trolling, just curious.

[Joshua Kugler (jkugler)]

OK, so rolling around some ideas on how to handle this. One simple way, one somewhat complex, but kind of neat, way.

Method 1:
Use a config file for nssldap-update-ignoreusers. Probably /etc/default/nssldap-update-ignoreusers. Have a single line something like:

nss_initgroups_okusers=user1,user2,user3,etc.

If a user was in that list, nssldap-update-ignoreusers would not include the user in the nss_initgroups_ignoreuses line when it updates the ldap.conf file.

Method 2:
Create a new system-level group named something like nss_okusers. Then, if a user was a member of that group, nssldap-update-ignoreusers would not include the user in the nss_initgroups_ignoreuses line when it updates the ldap.conf file. But that's abusing groups for system configuration, which I'm not sure is the best idea.

Comments? Questions?

We're talking about five lines at most of shell code. I believe it would be a worthwhile addition to nssldap-update-ignoreusers.

I might just work it up and attach a patch. :)

[Joshua Kugler (jkugler)]

So, here is the patch.

You add a line in /etc/ldap.conf of the form:

nss_initgroups_okusers user1,user2

Thanks to pgas on #awk for help with the awk syntax and best practices.

Comments/critique welcome.

[Scott Moser (smoser)]

Joshua,
  I've looked at your patch. It took a while for me to fully understand what we have working in libnss-ldap to implement 'nss_initgroups_minimum_uid'.

  I have the following comments:
* The concept and implementation seem like it is fine to me (I've only reviewed, not tested, though).
* OKUSERS=`grep "^nss_initgroups_okusers " $CONF | tail -n 1 | awk '{print $2}')`
is better (faster) written as:
   $1 == "nss_initgroups_okusers" { v=$2 }; END { print v }'
  I realize you just copied the format of 'MIN=', but both could be improved.
* You should update the man page nssldap-update-ignoreusers.8 as you're adding function there. we'd like it to be documented.
* It would be best if you created a debdiff (or bzr merge proposal),. that would reduce the work for someone to pick this up.

Note:
it seems that upstream at least still has an interest in nss_initgroups_minimum_uid (http://bugzilla.padl.com/show_bug.cgi?id=341). If we add 'nss_initgroups_okusers', via the same mechanism we're using for minimum_uid, we would need to then address merging the native support for that with our workaround. That would possibly be a bit more hairy if we also had to address nss_initgroups_okusers.

[Scott Moser (smoser)]

bah. 'is better (faster) written as: should have said:
   OKUSERS=`awk -v vname=nss_initgroups_okusers '$1 == vname { v=$2 }; END { print v }'`
rather than
  $1 == "nss_initgroups_okusers" { v=$2 }; END { print v }'

[Jamie Strandboge (jdstrand)]

TEST CASE (based on comment #84 from bug #155947):
1. apt-get install ldap-auth-client # pulls in libnss-ldap

2. configure ldap to use (via debconf):
ldap://127.0.0.1/
root requires a password: 'no'
everything else defaults

3. verify on fresh install nss_initgroups_ignoreusers is not present:
$ cat /etc/ldap.conf |grep "^nss" || echo "ok"
ok

4. run /etc/init.d/libnss-ldap stop and verify it populated ldap.conf
$ sudo /etc/init.d/libnss-ldap stop
 * Running nssldap-update-ignoreusers... [ OK ]
$ cat /etc/ldap.conf |grep "^nss"
nss_initgroups_ignoreusers avahi,avahi-autoipd,backup,bin,couchdb,daemon,games,gdm,gnats,haldaemon,hplip,irc,kernoops,libuuid,list,lp,mail,man,messagebus,news,proxy,pulse,root,rtkit,saned,speech-dispatcher,sshd,sync,sys,syslog,usbmux,uucp,www-data

5. add to /etc/ldap.conf the following:
nss_initgroups_okusers uucp,www-data

6. run /etc/init.d/libnss-ldap stop and verify it updated ldap.conf correctly:
$ sudo /etc/init.d/libnss-ldap stop
 * Running nssldap-update-ignoreusers... [ OK ]
$ cat /etc/ldap.conf |grep "^nss"
nss_initgroups_ignoreusers avahi,avahi-autoipd,backup,bin,couchdb,daemon,games,gdm,gnats,haldaemon,hplip,irc,kernoops,libuuid,list,lp,mail,man,messagebus,news,proxy,pulse,root,rtkit,saned,speech-dispatcher,sshd,sync,sys,syslog,usbmux

The groups listed were in a VM with ubuntu-desktop installed on Lucid. To properly test this, the groups listed in step '5' should be compared with the old libnss-ldap and the proposed libnss-ldap to make sure that the groups are the same.

[Jamie Strandboge (jdstrand)]

Based on the above test case, NAK on the current patch. It has a trailing ')' in the OKUSERS line, does not output the correct 'nss_initgroups_ignoreusers' line (ie, uucp and www-data are still listed) and the man page has not been updated.

[Jamie Strandboge (jdstrand)]

@Scott: NAK on your suggestion as it is currently incomplete. I'd be much more comfortable using the current syntax right before optimizing for speed anyway.

[Jamie Strandboge (jdstrand)]

The problem with the patch as written is that while 'users' gets updated correctly with this line:
users=`awk -v min="$MIN" -v okuser="$OKUSERS" -F: 'BEGIN{split(okuser,a,/,/);for (i in a) b[a[i]]} ($3 < min) && !($1 in b){printf "%s%s",s,$1;s=","}' /etc/passwd`

'users' is immediately updated after this however to merge what was in nss_initgroups_ignoreusers before and what is currently in 'users', so because 'uucp,www-data' was in nss_initgroups_ignoreusers before running nssldap-update-ignoreusers, it gets merged back in.

[Scott Moser (smoser)]

sorry for mis-pasting.
OKUSERS=`awk -v vname=nss_initgroups_okusers '$1 == vname { v=$2 }; END { print v }' "${CONF}"`

[Jamie Strandboge (jdstrand)]

So in talking with Scott on irc, he brought up an important point-- it is undesirable to add the additional nss_initgroups_okusers option if upstream actually implements nss_initgroups_ignoreusers/nss_initgroups_minimum_uid in nss-ldap proper (see upstream bug http://bugzilla.padl.com/show_bug.cgi?id=341). I think the best course of action is for people interested in fixing this bug to comment in the upstream bug about how nss_initgroups_ignoreusers/nss_initgroups_minimum_uid isn't always enough, and there should be some sort of whitelist. At that point we can evaluate the best way to move forward (and have a blessed config option).

If they NAK it, we could theoretically still implement this feature in nssldap-update-ignoreusers, with the understanding that nssldap-update-ignoreusers would have to be updated when upstream implements nss_initgroups_ignoreusers/nss_initgroups_minimum_uid and only remove users in nss_initgroups_okusers from nss_initgroups_ignoreusers rather than trying to generate nss_initgroups_ignoreusers on the fly each time.

[Jamie Strandboge (jdstrand)]

Unsubscribing ubuntu-sponsors for now. Feel free to resubscribe when there is feedback from upstream and a debdiff (or patch for nssldap-update-ignoreusers and nssldap-update-ignoreusers.8) is attached.

[Joshua Kugler (jkugler)]

Filed.

http://bugzilla.padl.com/show_bug.cgi?id=432