[SSSD] [PATCHES] Add support for multiple servers in the Kerberos locator plugin

Stephen Gallagher sgallagh at redhat.com
Wed Jul 18 12:05:32 UTC 2012


On Tue, 2012-07-17 at 17:46 -0400, Simo Sorce wrote:
> On Tue, 2012-07-17 at 13:36 -0400, Stephen Gallagher wrote:
> > This is the first half of the work necessary to resolve
> > https://fedorahosted.org/sssd/ticket/941
> > 
> > This patch largely rewrites the Kerberos locator plugin so that it can
> > parse kdcinfo files containing multiple servers (one per line, with
> > optional port). These patches are testable by manually
> > editing /var/lib/sss/pubconf/kdcinfo.REALM
> > (or /var/lib/sss/pubconf/kpasswdinfo.REALM, as appropriate). They are
> > also fully backwards-compatible.
> > 
> > I will be working on and sending the KRB5 provider side of things after
> > this, but since these are ready, manually testable and
> > backwards-compatible, it seemed prudent to send them for review in
> > parallel.
> 
> Is it forward compatible ?
> Meaning, if I have an application that already has the old plugin loaded
> and you upgrade sssd to start generating multiple lines in the file, how
> will the old app behave ?
> 

No, it's not forward-compatible. The old code was VERY finicky. It just
reads in the entire file, reverse-searches for a colon and assumes that
the address is everything before that last colon. So it's not going to
be able to read the new file.

FWIW, the old version couldn't even handle a trailing newline in the
file.

I suppose we could switch to using a new filename (kdcinfo2.REALM for
instance) and have the Kerberos provider produce both files for a while.

I don't love that answer, though.

> > Patch 0001: Convert memory allocation functions to use talloc. There's
> > no reason to avoid this dependency, and it will make subsequent patches
> > much simpler.
> 
> Nack,
> this code is loaded by libkrb5 and imported into other applications.
> Talloc is not thread safe and we do not want to drag this dependency in
> random applications unless it is fully thread safe.
> 

I don't understand. Where do you see a threading issue (that wouldn't be
present with malloc alone)? We allocate one top-level sssd_ctx as the
private data of the plugin and everything else as a child of that.

> > Patch 0002: Create a structure for holding address and port information
> > (with linked-list semantics)
> 
> Do we really need a complex linked list ?
> How many entries do we expect to have ?
> Wouldn't a simple array be easier to manage ?
> 

Eh, I wanted to be able to handle an arbitrary-length kdcinfo file, but
I suppose we could get away with a fixed limit of addresses. (I was
planning to make the number of addresses configurable in the Kerberos
provider). But picking a reasonable limit (10) and making it an array is
probably fine.

> > Patch 0003: Rewrite the kdcinfo parser so that it supports multiple
> > entries, one per line. It ignores blank lines.
> > Patch 0004: Modify the sssd_krb5_locator_lookup() function to send
> > repeated calls to the cbfunc() callback, providing all of the
> > kdcinfo-specified servers as values. Clients using libkrb5 will
> > transparently try all of the provided servers until finding one that
> > works.
> 
> Haven't really looked at the rest as the first nack unfortunately means
> a lot needs to be changed now.


Please help me understand where you see a thread-safety issue. I'm also
more inclined to fix the synchronization in the locator than I am to
back out of using talloc if I can help it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120718/fcf8dddc/attachment.sig>


More information about the sssd-devel mailing list