now with patches ...
On Tue, Jul 26, 2016 at 04:55:37PM +0200, Sumit Bose wrote:
On Tue, Jul 26, 2016 at 11:12:34AM +0200, Jakub Hrozek wrote:
> On Fri, Jul 22, 2016 at 09:44:33PM +0200, Sumit Bose wrote:
> > Hi,
> >
> > this patch set should fix
https://fedorahosted.org/sssd/ticket/2958
> > "Support multiple principals for IPA users" so the IPA users can log
in
> > with their Kerberos alias as well.
>
> First, thank you very much for splitting the patchset into small
> self-contained changes. The review was much easier this way.
>
> >
> > The overall code-path was already added for the UPN feature but had to
> > be extended at various places. The main difference is that the realm
> > part of the AD UPNs with an alternative domain suffix do not related to
> > a known domain name. The realm part from the Kerberos aliases can come
> > from any domain. The same is true for email addresses which are supported
> > by this patch set as well.
>
> I wonder if we will immediatelly get a request to provide a list of
> e-mail domain suffixes by the users that could be used to map the e-mail
> address to a domain. Do you think it would make sense (maybe in a future
> patch) to let the admin configure the suffixes for direct integration in
> sssd.conf and maybe on the server for IPA-AD trusts?
It might help in some cases, but I think in general it will not help
much. Email domain suffixes as e.g. AD's alternative domain suffixes are
typically used for a whole organization/forest so I would expect that
the cases where you have multiple domains and a single email domain
suffix relates only to a specific domain and is not equal to the domain
name is rare.
What might help more (and would help lookup by id as well) is to reorder
the lookups in the responders to check the cache for all domains first
before iterating over the backends. I think we already have a ticket for
this.
>
> >
> > I know that Jakub had some concerns adding email addresses now and the
> > another attribute in the next version and so on. I would like to make
> > this scheme more generic so that the attributes which should be used for
> > login names can be configured. Unfortunately I didn't had the time do
> > already do it in this patch-set.
>
> np, I think the patches are well structured and make the extension
> possible.
>
> >
> > Adding a larger number of sources for the login name increases the
> > chance for collisions. But since each of the name types, Kerberos
> > principals and email addresses, are expected to be unique in their
> > domain, I hope the chances are still low enough.
>
> Yes.
>
> >
> > bye,
> > Sumit
>
> > From b2012e5b0cc8af65f18a6e48774b80246c68c04a Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Fri, 22 Jul 2016 17:35:43 +0200
> > Subject: [PATCH 01/14] IPA: fix lookup by UPN for subdomains
> >
> > Currently the user name used in the extdom exop request is
> > unconditionally set to the short name. While this is correct for the
> > general name based lookups it breaks UPN/email based lookups where the
> > name part after the @-sign might not match to domain name. I guess this
> > was introduce during the sysdb refactoring.
>
> Yes, it was. Sorry about that. I tested UPN lookups on an IPA-AD trust
> client and they work fine with this patch.
>
> ACK
>
> > From 762cd84cc5851666607c871d53f4e309b8760f6c Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Fri, 22 Jul 2016 12:19:26 +0200
> > Subject: [PATCH 02/14] LDAP: allow multiple user principals
> >
> > In general a user can have multiple principals and recent IPA version
> > added support to defined multiple principals. With this patch SSSD does
> > not only store the first but all principals read by LDAP from a server.
> >
> > Resolves
https://fedorahosted.org/sssd/ticket/2958
>
> ACK
>
>
> > From 5c1a50ef35d816438880e4e9951d0355b283980f Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Sat, 18 Jun 2016 18:24:50 +0200
> > Subject: [PATCH 03/14] LDAP: new attribute option ldap_user_email
>
> ACK
>
> > From 3f09cfc6598d34ff42be1c85f448b5c0635410e0 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Mon, 20 Jun 2016 12:57:43 +0200
> > Subject: [PATCH 04/14] sysdb: include email in UPN searches
>
> ACK just one question..
>
> >
> > Email addresses and Kerberos user principals names (UPNs) do not only
> > look similar they also can be used to identify a user uniquely.
> >
> > In future this approach should be replace by a more generic one where
> > the attributes which can uniquely identifies a user can be configured to
> > support even a wider range of login names.
>
> ..I think we want to track this with a ticket (even if it was deferred
> now). If you agree I can file one.
yes, please.
>
> > From 6ac7dacf122da2933ce6fe9b2ceff356c1b65721 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Mon, 20 Jun 2016 12:58:16 +0200
> > Subject: [PATCH 05/14] LDAP: include email in UPN searches
>
> ACK
>
> > From b3c9c37baa49ade82f8cf873a2e0d2e0e7e16fad Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Mon, 20 Jun 2016 13:37:56 +0200
> > Subject: [PATCH 06/14] NSS: add user email to fill_orig()
> >
> > The IPA server must send the email address of a user to the clients to
> > allow login by email.
>
> ACK
>
> > From 88118645466c50c52a81e5ff38d66ecb2dfda2a7 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Mon, 20 Jun 2016 16:11:11 +0200
> > Subject: [PATCH 07/14] utils: add is_email_from_domain()
>
> ACK
>
> > From 72fb323490acc4155227b56be6470965830b7ade Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Mon, 20 Jun 2016 16:30:03 +0200
> > Subject: [PATCH 08/14] LDAP/IPA: add local email address to aliases
> >
> > Adding email-addresses from the local domain to the alias names is
> > strictly not needed by might help to speed up lookups in the NSS
> > responder.
>
> ACK, this makes sense, the aliases are indexed.
>
> > From 0f2863a0fb1620bf86ffb828e0c51ead4f807b2a Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Tue, 21 Jun 2016 11:06:19 +0200
> > Subject: [PATCH 09/14] NSS: continue with UPN/email search if name was not
> > found
> >
> > Currently we only search for UPNs if the domain part of the name was not
> > know, with Kerberos aliases and email addresses we have to do this even
> > if the domain name is a know domain.
>
> ACK
>
> > From 84813abfb71dc23ad054c42af0e3d2eba1e74e7e Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Wed, 22 Jun 2016 18:21:11 +0200
> > Subject: [PATCH 10/14] PAM: continue with UPN/email search if name was not
> > found
> >
> > Currently we only search for UPNs if the domain part of the name was not
> > know, with Kerberos aliases and email addresses we have to do this even
> > if the domain name is a know domain.
>
> ACK
>
> > From bccc06776316459ec10c8103465664df2433f260 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Fri, 22 Jul 2016 17:34:20 +0200
> > Subject: [PATCH 12/14] PAM: Fix domain for UPN based lookups
> >
> > Since sysdb_search_user_by_upn() searches the whole cache we have to set
> > the domain so that it matches the result.
>
> ACK, we already have a matching commit in NSS
>
> > From c132fa210341b30ec4586c0bb951ffb1dc6feb94 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Fri, 22 Jul 2016 12:20:33 +0200
> > Subject: [PATCH 13/14] SDAP: add special handling for IPA Kerberos enterprise
> > principal strings
> >
> > Unfortunately principal aliases with an alternative realm are stored in
> > IPA as the string representation of an enterprise principal, i.e.
> > name\@alt.realm(a)IPA.REALM. To allow searches with the plain alias
> > 'name(a)alt.realm' the returned value is converted before it is saved to
> > the cache.
>
> ACK
>
> > From 3b4deb7deb64136094aa5e70705b467a836dd05b Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sbose(a)redhat.com>
> > Date: Fri, 22 Jul 2016 20:10:42 +0200
> > Subject: [PATCH 14/14] SDAP: add enterprise principal strings for user
> > searches
> >
> > Unfortunately principal aliases with an alternative realm are stored in
> > IPA as the string representation of an enterprise principal, i.e.
> > name\@alt.realm(a)IPA.REALM. To be able to lookup the alternative
> > principal in LDAP properly the UPN search filter is extended to search
> > for this type of name as well.
>
> There is a warning:
> CC src/tests/cmocka/nestedgroups_tests-common_mock_sdap.o
> /home/remote/jhrozek/devel/sssd/src/providers/ldap/sdap_utils.c: In function
‘get_enterprise_principal_string_filter’:
> /home/remote/jhrozek/devel/sssd/src/providers/ldap/sdap_utils.c:254:37: warning:
field precision specifier ‘.*’ expects argument of type ‘int’, but argument 4 has type
‘long int’ [-Wformat=]
> return talloc_asprintf(mem_ctx, "(%s=%.*s\\\\@%s@%s)", attr_name,
> ^
>
> But the intent of the code looks good to me.
I fixed this with a cast, I hope this is sufficient to silence the
warning.
New version which fixes the Coverity warning as well is attached.
bye,
Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org