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