[SSSD] Support for subdomains - stage 1

Jan Zelený jzeleny at redhat.com
Fri Apr 13 06:24:18 UTC 2012


> On Thu, Apr 12, 2012 at 09:52:14PM +0200, Jan Zeleny wrote:
> > Jakub Hrozek <jhrozek at redhat.com> wrote:
> > > On Tue, Apr 10, 2012 at 12:38:31AM -0400, Jakub Hrozek wrote:
> > > > The mails grow huge. I'm going to trim the response a little and omit
> > > > patches that were already acked elsewhere.
> > > > 
> > > > Patch 0001: Ack
> > > > Patch 0002: Ack
> > > > Patch 0003: Ack
> > > > Patch 0004: Ack
> > > > Patch 0005: Ack
> > > > 
> > > > On Fri, Apr 06, 2012 at 01:52:36PM +0200, Jan Zelený wrote:
> > > > > > > > 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.
> > > > > > 
> > > > > > No, sss_dp_get_domains_recv() is not used anywhere.
> > > > > 
> > > > > Fixed
> > > > > 
> > > > > > > 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.
> > > > > > 
> > > > > > Replied elsewhere.
> > > > > 
> > > > > Fixed
> > > > 
> > > > Please also remove "subreq" from setnetgrent_send() and the
> > > > declaration of setnetgrent_cb().
> > > > 
> > > > Patch 0007: Ack
> > > > Patch 0008: Ack
> > > > Patch 0009: Ack
> > > > Patch 0010: Ack
> > > > Patch 0011: Ack
> > > > 
> > > > > > > > Patch 0012: New config options for subdomains
> > > > > > > > See my reply in the other part of this thread. If you are
> > > > > > > > goig forward with these options, add them to the configAPI.
> > > > > > > 
> > > > > > > Based on previous discussion I removed the part which is
> > > > > > > setting default shell. I intend to keep the homedir for two
> > > > > > > reasons:
> > > > > > > 
> > > > > > > I recalled why I created the config option in the first place.
> > > > > > > It's because override_homedir can't be set per domain. After
> > > > > > > your comment yesterday, I checked to be sure and IMO the man
> > > > > > > page is incorrect and the override_homedir only works for
> > > > > > > responder.
> > > > > > 
> > > > > > What was your testcase and what exactly wasn't working for you?
> > > > > > 
> > > > > > The option works fine for me in a subdomain and the code seems to
> > > > > > be quite clear (get_homedir_override())
> > > > > 
> > > > > I know it is straightforward, but it didn't work for me.
> > > > 
> > > > Can you file a bug if the override is not working in a domain? It
> > > > should and if something is not clear, then it's most probably a docs
> > > > bug..
> > > > 
> > > > > However when looking
> > > > > a bit deeper I discovered a bug in completely different part of the
> > > > > patch set. This is now fixed and the override works as expected.
> > > > > 
> > > > > > > The second reason is that admins might want to separate
> > > > > > > homedirs for native users from homedirs for trusted users on
> > > > > > > some systems.
> > > > > > 
> > > > > > Then let's extend the homedir template by a subdomain name or
> > > > > > "full domain name" option.
> > > > > 
> > > > > I'm not sure about this. For one thing this functionality is
> > > > > already there. But I was considering scenario when the path to
> > > > > subdomain homedirs is completely different. Something like this:
> > > > > 
> > > > > /home/ipa/%u - the main IPA domain
> > > > > /home/ad/%u  - AD subdomains
> > > > > 
> > > > > I there is a consensus that it is not necessary, I can remove it.
> > > > > But frankly I can see us having exactly this RFE several months
> > > > > later.
> > > > 
> > > > If the AD users are to be treated differently than native IPA users
> > > > the new option is OK.
> > > > 
> > > > Patch 0013: Ack
> > > > 
> > > > > > > > Patch 0014: Add s2n extended operation
> > > > > > > > Old DEBUG macros again. There is a typo in
> > > > > > > > s2n_response_to_attrs, one of the debug messages says "go
> > > > > > > > [%s]", should be "got [%s]". Otherwise looks fine.
> > > > > > > 
> > > > > > > Done
> > > > > > 
> > > > > > GCC emits these warnings when compiling with -O2:
> > > > > > src/providers/ipa/ipa_s2n_exop.c: In function
> > > > > > 'ipa_s2n_get_user_done': src/providers/ipa/ipa_s2n_exop.c:408:16:
> > > > > > warning: 'retoid' may be used uninitialized in this function
> > > > > > [-Wuninitialized]
> > > > > > src/providers/ipa/ipa_s2n_exop.c:582:11: note: 'retoid' was
> > > > > > declared here
> > > > > > src/providers/ipa/ipa_s2n_exop.c:408:35: warning: 'retdata' may
> > > > > > be used uninitialized in this function [-Wuninitialized]
> > > > > > src/providers/ipa/ipa_s2n_exop.c:583:20: note: 'retdata' was
> > > > > > declared here
> > > > > > src/providers/ipa/ipa_s2n_exop.c: In function
> > > > > > 'ipa_s2n_get_acct_info_send':
> > > > > > src/providers/ipa/ipa_s2n_exop.c:84:9: warning: 'bv_req' may be
> > > > > > used uninitialized in this function [-Wuninitialized]
> > > > > > src/providers/ipa/ipa_s2n_exop.c:532:20: note: 'bv_req' was
> > > > > > declared here
> > > > > 
> > > > > All fixed
> > > > 
> > > > Ack
> > > > 
> > > > Patch 0015: Ack
> > > > Patch 0016: Ack
> > > > Patch 0017: Ack
> > > > Patch 0018: Ack
> > > > Patch 0019: Ack
> > > > Patch 0020: Ack
> > > > Patch 0021: Ack
> > > > 
> > > > Becuse the only remaining review issue is a trivial one and I know
> > > > that Stephen has been reviewing the patches as well, feel free to
> > > > resend another patch set when/if Stephen sends his comments.
> > > > 
> > > > Great work!
> > > 
> > > There seems to be a startup error with non-IPA back ends:
> > > 
> > > (Tue Apr 10 17:54:11 2012) [sssd[be[ldap]]] [load_backend_module]
> > > (0x0010): Unable to load init fn sssm_ldap_subdomains_init from module
> > > ldap, error: /usr/lib64/sssd/libsss_ldap.so: undefined symbol:
> > > sssm_ldap_subdomains_init
> > 
> > Here is the patch set with both things fixed. In the end I had to add the
> > subdomains_provider option, it was the only solution for the linking
> > issue.
> > 
> > Thanks
> > Jan
> 
> This works fine, but the new option should be added to the configAPI
> (sorry, I hope this would be the last iteration).

My mistake, I knew I'm missing something but late hour was stronger than my 
will to re-check everything.

> The DEBUG message level is set to SSSDBG_CRIT_FAILURE, which seems quite
> high because this DEBUG message is emited for any back end which does
> not support subdomains which is everything but IPA. This should be a
> MINOR failure at best, maybe even lower.

I will change this in separate patch

> Also, instead of defaulting to "not set", would it be a good idea to
> default to "ipa" in the IPA provider and maybe check during the first
> request if subdomains are enabled on the server  -- provided that runtime
> detection  is possible which I honestly don't know. That way the clients
> wouldn't have to be reconfigured if trusts are added on the server.

I'm not completely sure about this since Sumit did the part where SSSD 
communicates with IPA. I will discuss it with him once he is back from 
vacation. I'll send the patch set then.

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/20120413/17f2fa98/attachment.sig>


More information about the sssd-devel mailing list