[Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls
Jakub Hrozek
jhrozek at redhat.com
Tue Jul 9 09:42:00 UTC 2013
On Tue, Jul 09, 2013 at 10:33:19AM +0300, Alexander Bokovoy wrote:
> On Mon, 08 Jul 2013, Jakub Hrozek wrote:
> >On Mon, Jul 08, 2013 at 07:32:41PM +0300, Alexander Bokovoy wrote:
> >>On Mon, 08 Jul 2013, Jakub Hrozek wrote:
> >>>On Mon, Jul 08, 2013 at 04:15:39PM +0300, Alexander Bokovoy wrote:
> >>>>On Mon, 08 Jul 2013, Alexander Bokovoy wrote:
> >>>>>On Wed, 03 Jul 2013, Sumit Bose wrote:
> >>>>>>Hi,
> >>>>>>
> >>>>>>with this patch the extdom plugin, the LDAP extended operation that
> >>>>>>allows IPA clients with recent SSSD to lookup AD users and groups, will
> >>>>>>not use winbind for the lookup anymore but will use SSSD running in
> >>>>>>ipa_server_mode.
> >>>>>>
> >>>>>>Since now no plugin uses the winbind client libraries anymore, the
> >>>>>>second patch removes the related configures checks.
> >>>>>>
> >>>>>>I think for the time being we cannot remove winbind completely because
> >>>>>>it might be needed for msbd to work properly in a trusted environment.
> >>>>>s/msbd/smbd/
> >>>>>
> >>>>>ACK. I need to add 'ipa_server_mode = True' support to
> >>>>>the installer code and then these patches can go in.
> >>>>Actually, the code still doesn't work due to some bug in sssd which
> >>>>fails to respond properly to getsidbyname() request in libsss_nss_idmap.
> >>>>
> >>>>Additionally I've found one missing treatment of domain_name for
> >>>>INP_NAME requests.
> >>>>
> >>>>We are working with Jakub on tracking down what's wrong on SSSD side.
> >>>
> >>>Indeed, there was a casing issue in sysdb. You can continue testing with
> >>>lowercase user names in the meantime. A patch is already on the SSSD
> >>>list.
> >>In addition, we need to disqualify user name when returning back a
> >>packet from extdom operation as this is what SSSD expects.
> >>
> >>Attached patch does it for all types of requests.
> >>
> >>--
> >>/ Alexander Bokovoy
> >
> >>>From 3659059c646f7b584ee07fb9e780759bcc0bb08e Mon Sep 17 00:00:00 2001
> >>From: Alexander Bokovoy <abokovoy at redhat.com>
> >>Date: Mon, 8 Jul 2013 19:19:56 +0300
> >>Subject: [PATCH] Fix extdom plugin to provide unqualified name in response as
> >> sssd expects
> >>
> >>extdom plugin handles external operation over which SSSD asks IPA server about
> >>trusted domain users not found through normal paths but detected to belong
> >>to the trusted domains associated with IPA realm.
> >>
> >>SSSD expects that user or group name in the response will be unqualified
> >>because domain name for the user or group is also included in the response.
> >>Strip domain name from the name if getgrnam_r/getpwnam_r calls returned fully
> >>qualified name which includes the domain name we are asked to handle.
> >>
> >>The code already expects that fully-qualified names are following user at domain
> >>convention so we are simply tracking whether '@' symbol is present and is followed
> >>by the domain name.
> >>---
> >> .../ipa-extdom-extop/ipa_extdom_common.c | 26 ++++++++++++++++++++++
> >> 1 file changed, 26 insertions(+)
> >>
> >>diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
> >>index 8aa22e1..290da4e 100644
> >>--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
> >>+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
> >>@@ -295,6 +295,7 @@ int handle_request(struct ipa_extdom_ctx *ctx, struct extdom_req *req,
> >> &grp_result);
> >> }
> >> }
> >>+ domain_name = strdup(req->data.name.domain_name);
> >
> >I would prefer if this was a separate patch. But this is a correct
> >change.
> Separated.
>
Ack to this patch.
> >
> >> break;
> >> default:
> >> ret = LDAP_PROTOCOL_ERROR;
> >>@@ -338,6 +339,7 @@ int create_response(struct extdom_req *req, struct pwd_grp *pg_data,
> >> const char *domain_name, struct extdom_res **_res)
> >> {
> >> int ret = EFAULT;
> >>+ char *locat = NULL;
> >> struct extdom_res *res;
> >>
> >> res = calloc(1, sizeof(struct extdom_res));
> >>@@ -354,10 +356,20 @@ int create_response(struct extdom_req *req, struct pwd_grp *pg_data,
> >> switch(id_type) {
> >> case SSS_ID_TYPE_UID:
> >> case SSS_ID_TYPE_BOTH:
> >>+ if ((locat = strchr(pg_data->data.pwd.pw_name, '@')) != NULL) {
> >>+ if (strstr(locat, domain_name) != NULL ) {
> >
> >strstr doesn't work correctly in my case. In SSSD, the domain names are
> >case-insensitive, so you can use strcasestr here. In my case, the
> >condition is never hit as the domain case differs:
> >
> >407 res->data.user.domain_name = strdup(domain_name);
> >(gdb)
> >408 if ((locat = strchr(pg_data->data.pwd.pw_name, '@')) != NULL) {
> >(gdb) n
> >409 if (strstr(locat, domain_name) != NULL) {
> >(gdb)
> >414 strdup(pg_data->data.pwd.pw_name);
> >(gdb) p domain_name
> >$1 = 0x7f2e800f0690 "AD.EXAMPLE.COM"
> >(gdb) p locat
> >$2 = 0x7f2e8006500d "@ad.example.com"
> Replaced by strcasestr.
>
>
> >
> >Also, this is good enough for the beta or when the default values are used, but
> >in theory, the admin could configure the fully qualified name format to not
> >include the @-sign (see full_name_format in sssd.conf) at all.
> >
> >It's not very likely, but I think we should account for that case
> >eventually. I'm not exactly sure how yet as the full_name_format is pretty
> >free-form, maybe we could simply disallow setting it if server_mode was
> >set to True.
> I'd prefer the latter indeed. Given that you can have full_name_format
> varying between servers and clients, this is simply asking for disaster.
>
> I'd preper concentrating our effort on making default configuration
> working so well that you never need to modify these parameters. After
> all, we are talking about SSSD use as FreeIPA client with trusts
> enabled. SSSD configuration is dictated by ipa-client-install in this
> case for all clients.
And Ack to the second patch as well.
I opened https://fedorahosted.org/sssd/ticket/2009 on the SSSD side to
make the behaviour more predictable and robust.
More information about the Freeipa-devel
mailing list