[SSSD] Support for subdomains - stage 1

Sumit Bose sbose at redhat.com
Thu Apr 5 14:22:38 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.

> > 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.

bye,
Sumit



More information about the sssd-devel mailing list