RE: nss_ldap feature broken by changes in tls.c (ITS#1555)

[hyc@highlandsun.com]

> -----Original Message-----
> From: owner-openldap-bugs@OpenLDAP.org
> [mailto:owner-openldap-bugs@OpenLDAP.org]On Behalf Of
> andrew.findlay@skills-1st.co.uk

> Full_Name: Andrew Findlay
> Version: 2.0.11
> OS: Red Hat 7.2
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (217.206.98.194)
>
>
> A change introduced into libraries/libldap/tls.c at revision 1.8.2.23 has
> broken
> the tls_checkpeer functionality in nss_ldap. This is the set of
> config options
> that allows a client machine to verify that an LDAP server is using
> the correct
> X.509 certificate before trusting its answers.
>
> Among the affected config options in ldap.conf are:
>
> 	tls_cacertfile
> 	tls_cacertdir
>
> In terms of OpenLDAP these set the options LDAP_OPT_X_TLS_CACERTFILE and
> LDAP_OPT_X_TLS_CACERTDIR, via the function ldap_pvt_tls_set_option in tls.c
>
> As called from recent versions of nss_ldap, the first parameter is filled in
> with
> a pointer to the LDAP association structure. This suggests that these are
> per-association parameters which seems reasonable to me, though in fact they
> are
> implemented as static globals.
>
> However, ldap_pvt_tls_set_option contains code to reject calls that
> try to set
> the CACERT options if an association pointer is provided. Thus,
> nss_ldap cannot
> check certificates.
>
> One workaround is to modify do_ssl_options in ldap-nss.c in nss_ldap
> so that it
> passes NULL as the first parameter to ldap_set_option. I have tested this and
> it
> works. However, I feel that this is the wrong solution.
>
> It seems right to move the tls_opt_cacertfile and tls_opt_cacertdir (etc)
> variables
> into the per-association structure in tls.c though this may break other
> programs
> that assume them to be global.
>
> An easy workaround which preserves some backwards-compatibility would be to
> simply
> remove the test for (ld != NULL) in ldap_pvt_tls_set_option. This would allow
> calling programs to supply the association structure, but would not
> require it
> until/unless the variables are made non-global.

It doesn't make sense to make these variables non-global, since they can only
affect the library's default TLS context. Also, they can only take effect if
the options are set before the library initializes its default context. As
such, it really only makes sense to call this with a NULL association; by the
time you've created a valid association with an active connection, it's too
late to set these options.

If you really want to set options on a per-association basis, you do it by
calling the SSL libraries yourself to create your own SSL context, set whatever
parameters you need on it, and then use set_option with LDAP_OPT_X_TLS_CTX to
store your context on a specific association.

Your suggested workaround seems harmless enough, but I think the correct fix is
to change nss_ldap to use a NULL association on its call.

  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support