[SSSD] Support for subdomains - stage 1

Jan Zelený jzeleny at redhat.com
Fri Apr 6 07:38:55 UTC 2012


> On Thu, Apr 05, 2012 at 10:46:55AM +0200, Jan Zelený wrote:
> > > On Mon, Apr 02, 2012 at 10:23:37AM +0200, Jan Zeleny wrote:
> > > > Once more, this time rebased on top of current master.
> > > > 
> > > > Thanks
> > > > Jan
> > > 
> > > Patch 0001: Sysdb routines for subdomains
> > > Ack, can you just add a DEBUG message in case
> > > sysdb_transaction_commit() fails?
> > 
> > Done
> > 
> > > Patch 0002: Add some utility functions for subdomains
> > > Ack
> > > 
> > > Patch 0003: Add conn_name to allow different names for domains and
> > > connections
> > > Ack
> > > 
> > > Patch 0004: Responder part of the subdomain retrieval work
> > > "get_domains_timeout" is not mentioned in neither configAPI nor
> > > manpages
> > > 
> > > +        if (ret == EOK) {
> > > +            DEBUG(SSSDBG_TRACE_FUNC,
> > > +                  ("Last call was too recent, nothing to do!\n"));
> > > +            ret = EOK;
> > > 
> > >         ^^ this is redundant
> > > 
> > > +            goto done;
> > > +        } else if (ret != EAGAIN) {
> > 
> > Fixed
> > 
> > > Please stick to either goto or return in get_domains_next()
> > 
> > Done
> > 
> > > Two review items from the original review are not addressed yet:
> > > There is no explanation of the "hint" parameter (and without one, it's
> > > hard to say what to do with the FIXME) and there is no
> > > sss_dp_get_domains_recv()
> > 
> > The hint is actually described right at the point where it is given as an
> > argument to dbus call.
> > 
> > sss_dp_get_domains_recv() added
> > 
> > > Patch 0005: Modified responder_get_domain()
> > > It is a convention to put a TALLOC_CTX we allocate on as the first
> > > parameter (responder_get_domain())
> > 
> > I know, I mentioned this to Sumit when I was reviewing reviewing the code
> > once but I forgot to fix it. Fixed now.
> > 
> > > I think that it might be useful to store
> > > CONFDB_RESPONDER_GET_DOMAINS_TIMEOUT in rctx rather than load it from
> > > confdb on every request. The case where confdb_get_int either errors
> > > out or the timeout is negative should at least trigger a DEBUG
> > > message.
> > 
> > Done and done
> > 
> > > As a general question, why are the domain names case sensitive but
> > > subdomain names seem to be case insensitive? For example
> > > check_last_request() is doing a case-sensitive strcmp but
> > > responder_get_domain is doing a strcasecmp.
> > 
> > I wrote this when the same question was on my mind (you can notice the
> > FIXME right above it). It will be safer to treat all domains and
> > subdomains case insensitively. Is there any other place where you
> > noticed this?
> 
> The domain names are defined in sssd.conf and only used as defined. With
> sub-domains it might be more difficult, because it is not always clear
> if e.g. the DNS domain name or the realm name is used. Typically both
> will be the same except for the case.

Ok, that means case-insensititve comparison is the right approach.

> > > Also please split the long DEBUG() statement in responder_get_domain()
> > > into two lines, it doesn't fit on my screen :-)
> > 
> > Done, sorry for the inconvenience ;-)
> > 
> > > Patch 0006: Retrieve subdomains if there is a request for fully
> > > qualified user
> > > Looks OK, will just need amendment when sss_dp_get_domains_recv() is
> > > implemented. Currently the patch supports users and netgroups, are
> > > there plans to also support services and the SSH responder? I think we
> > > can leave out the sudo responder for now, sudo doesn't support domains
> > > (yet?). Autofs in theory can send a request for a fully-qualified map,
> > > but it doesn't so the work can be done later, although I would prefer
> > > a ticket to show that we are aware of the issue.
> > 
> > The amendment has been done. About support of another responders - I
> > honestly don't know. I wasn't aware that a request for fully qualified
> > service/ssh entity can be given to SSSD. Perhaps Sumit will know more.
> 
> ah, sorry, I think I haven't read patch 0006 to the end. We do not need
> netgroups for sub-domains. Only the user object is need for sub-domains
> (and a way to authenticate the user :-). All other things like access
> control and group memberships will be controlled by the local domain.

Does that mean that I should remove all those parts where we are requesting 
various entities from subdomain?

Thanks
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120406/576ef02b/attachment.sig>


More information about the sssd-devel mailing list