[Bug 491767] Review Request: nss-ldapd - An nsswitch module which uses directory servers

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=491767





--- Comment #13 from Nalin Dahyabhai <nalin at redhat.com>  2009-04-17 09:09:53 EDT ---
(In reply to comment #12)
> First, the rpmlint complaints:
>   nss-ldapd.x86_64: W: no-version-in-last-changelog
> Indeed, the most recent changelog is missing a version.  

Fixed.

> Any reason for not just using a release of "1%{?dist}"?  This doesn't seem to
> be a prerelease package so your release number should be a positive integer. 
> If you're trying to make the initial import of the package have release 1 then
> it should be OK although I don't really understand why it would matter.

Good point.  Fixed.

> I'll admit to not comprehending the dependency on pam_ldap.so, but only because
> I don't really know what you intend to do with nss_ldap in the future.  I'm
> guessing that this package really doesn't have any need for pam_ldap, and
> you're just trying to make sure that it stays around in the case that nss-ldapd
> starts obsoleting nss_ldap.  I'm curious as to whether that's really necessary
> or if you're just doing some CYA.

Nope, that's it --  the F12 branch splits nss_ldap and pam_ldap into separate
binary packages, but keeps them in the same source package (for now, at least).

> The scriptlets are pretty complex and somewhat scary.  I note that installing
> this prints 'USELDAP=yes' to the console, which it probably shouldn't.  I
> suppose the egrep call needs -q or a redirect.  Other than that, though, they
> seem to work well enough.  However, there are some issues with things that are
> supported with nss_ldap that are deprecated or not supported with nss-ldapd and
> I wonder if it's worth worrying about migrating them?

The logic's copying any options that start with tls_ or ssl wholesale, and not
doing anything smart about converting or dropping specific settings.  I think
that the settings which don't port over cleanly are important enough that
triggering the error is, possibly perversely, a behavior worth keeping.

> Starting nslcd:
> nslcd: /etc/nss-ldapd.conf:130: option tls_checkpeer is deprecated (and will be
> removed in an upcoming release), use tls_reqcert instead
> nslcd: /etc/nss-ldapd.conf:131: option tls_cacertfile is currently untested
> (please report any successes)
> 
> I think something's off with the groupadd stuff:
>   getent group nslcd > /dev/null || /usr/sbin/groupadd -r -g 55 ldap
> Shouldn't it try to add "nslcd" instead of "ldap"?  Also, why does this need to
> have a specific numbered account?  Shouldn't any low UID suffice?

It would, but I started the ball rolling on getting a new one (see bug
#491899), and while that netted a UID, we end up sharing the 'ldap' group with
the openldap package.  The inconsistency was me missing half of correcting it
when that happened, thanks for catching it!  Fixed.

> ?  /lib64/security/pam_ldap.so

Requiring pam_ldap by package name (F12) won't guarantee that you get one that
matches your arch on multilib boxes, so it's being required as
/%{_lib}/security/pam_ldap.so, to ensure thatwe get the pam_ldap.so that
matches the arch of the libnss_ldap.so.2 that this binary package is supplying.
 We've had weird problems both composing trees and with things not always
working with multilib and plugins (I'm thinking of cyrus-sasl, but PAM modules
have similar problems), and I'm trying to avoid that.

Spec updated, new source package built using it.

Thanks!

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.