Avoiding further (LDAP) stack proliferation in Samba

Christof Schmitt cs at samba.org
Mon Jun 8 23:17:34 UTC 2020


Hi Andrew,

as Metze wrote, there is still work to be done across tldap and ldb, and
probably more questions will come up, which libraries to re-use, how to
handle dependencies to those, etc.

For winbindd, the long-tem goal is to improve failover to different
domain controllers, in case of network problems or DC outages. The
current problem today is that winbindd uses libads, which encapsulates
DC selection, LDAP queries and retries in a way that is completely
outside of the control of winbindd. So the idea is to move winbindd to
tldap first, and then later on move winbindd to be fully async, avoid
the child processes and keep the connection state in one place.

The goals to unify the LDAP stacks are worthwhile, but i do not see
those as necessary for the winbindd changes. I suspect that would also
trigger a wider discussion, e.g. which ASN.1 library to use, how to
reprent common data structures, where to put these to handle
dependencies.

Christof

On Mon, May 25, 2020 at 09:44:07PM +1200, Andrew Bartlett via samba-technical wrote:
> Thanks metze for your reply,
> 
> There is much we agree on, and other things that we have different
> perspectives on here.  
> 
> My opening thought is that in this, I recognise that I need to be
> careful what I wish for.  Broad scale lift-and-shift operations have
> been popular in Samba of late, and this is another of them in a way.
> 
> At the start, they appear very simple, but as noted here there is much
> complexity in the end result, and a very risky transition that needs to
> be done by experienced developers and reviewed incredibly carefully.
> 
> Even if we didn't have my concerns about the API and implementation of
> tldap, as you note in (3) below the "ldap ssl ads" parameter (which is
> used enough that we have a WIP patch set to fix it for channel
> bindings) and "client ldap sasl wrapping" parameter are not
> implemented.
> 
> And I do have concerns about tldap.  My view since the inception, which
> I think I expressed - I've not checked the archives - is that tldap
> should:
>  * Use the struct ldb_request, struct ldb_parse_tree, struct ldb_dn,
> struct ldb_message and the related structures.  Not because they are
> divinely inspired but because they enable a suite of utility functions
> that will, as the library becomes mature, be duplicated (like dumping
> to LDIF, parsing extended DNs, extracting a GUID etc).
> 
> I've re-checked the code more recently, and I'm sorry to see so much
> inline ASN.1 parsing.  It really should re-use ldap_message.c for the
> ASN.1 parsing, so the ASN.1 encode and decode can be tested (see the
> recent tests added in test_ldap_message) and fuzzed (fuzz_ldap_decode).
> 
> Why do I raise this again?  Because as noted, lift and shift is risky. 
> So if it is ever to be done, it should be done while tldap supports
> only one small, optional winbind module.
> 
> That is why I appreciate but don't accept the "but this is not
> something new" (i.e. it is already in use) argument.  This is is a
> significant new use that makes it harder and much less likely that
> tldap will ever be overhauled and suggests that tldap is already the
> "future ideal" API and implementation.
> 
> Of course I now wish I had raised these objections again in 2015, but
> we both know that would not have been a good idea. 
> 
> But here in 2020 I would be much less opposed to winbindd using tldap
> directly if these matters were addressed, which is why I tried to leave
> this as a possible way forward in my original mail.  It would mean that
> tldap would just be a different API to a common LDAP client lib, not an
> standalone creature itself. 
> 
> But whatever we do, it must not become an independent, fully featured,
> LDAP client stack, it must complement and integrate with what we have. 
> 
> Swen and Christof,
> 
> All this does however really bring into question if for the nominal
> purpose, there is some way to get the timeout handling we need out of
> the OpenLDAP libraries under libads in the meantime.
> 
> I'm sorry this has become a can of worms.  That happens in Samba
> sometimes.  Perhaps start by proposing just your tests, and someone can
> look at them in isolation?
> 
> Andrew Bartlett
> 
> On Fri, 2020-05-22 at 15:02 +0200, Stefan Metzmacher wrote:
> 
> > As you know I like the idea of having things implemented just once!
> > But as found out in the past this is a lot of work and
> > replacing everything at once is often not possible.
> > We learnt that we sometimes have to do small steps with intermediate
> > states, which we sometimes not like, but at the same time require
> > in order to make progress at all.
> > 
> > There're a lot of things I'd like to see:
> > 1. The ldb api should not be used for pure LDAP users,
> >    it's bad enough that the strange async model exists at all!
> >    We should hope that it's only used for LDAP for command line
> >    tools in a sync fashion.
> > 2. source3/lib/tldap_gensec_bind.c should use gensec_update_send/recv
> > 3. tldap makes use of the "client ldap sasl wrapping" and other
> >    options, which are use by source4/libcli/ldap/ and
> >    source3/libads/
> > 4. We can add some helpers to get/pass 'struct ldb_message' from/to
> >    tldap c.
> > 5. users of source4/libcli/ldap/ should move to the tldap api
> >    - lib/ldb-samba/ldb_ildap.c can become lib/ldb-samba/ldb_tldap.c
> > 6. libads should go away it, at least it's low level internals
> >    maybe it can be build on top of tldap as a first step in
> >    order to avoid a rewrite of all non-winbind users.
> > 7. winbindd should avoid libads and only use tldap.
> > 
> > But the goal of
> > https://gitlab.com/samba-team/samba/-/merge_requests/1351
> > is moving along with 7.
> > 
> > And I'm not seeing why we would require 4, 5, 6 before doing 7.
> > They would be nice to have, but they tasks for another day.
> > 
> > BTW: I'm not saying that I'm happy with the current patchset of merge
> > request 1351. But that's a different discussion.
> > 
> > metze
> > 
> -- 
> Andrew Bartlett                       https://samba.org/~abartlet/
> Authentication Developer, Samba Team  https://samba.org
> Samba Developer, Catalyst IT          
> https://catalyst.net.nz/services/samba
> 
> 
> 
> 



More information about the samba-technical mailing list