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 <aboko...@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@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. > 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" 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. > + locat[0] = 0; > + } > + } > res->data.name.object_name = > > strdup(pg_data->data.pwd.pw_name); > break; > case SSS_ID_TYPE_GID: > + if ((locat = strchr(pg_data->data.grp.gr_name, '@')) > != NULL) { > + if (strstr(locat, domain_name) != NULL) { > + locat[0] = 0; > + } > + } > res->data.name.object_name = > > strdup(pg_data->data.grp.gr_name); > break; > @@ -393,6 +405,11 @@ int create_response(struct extdom_req *req, struct > pwd_grp *pg_data, > case SSS_ID_TYPE_BOTH: > res->response_type = RESP_USER; > res->data.user.domain_name = strdup(domain_name); > + if ((locat = strchr(pg_data->data.pwd.pw_name, '@')) != > NULL) { > + if (strstr(locat, domain_name) != NULL) { > + locat[0] = 0; > + } > + } > res->data.user.user_name = > > strdup(pg_data->data.pwd.pw_name); > > @@ -408,6 +425,11 @@ int create_response(struct extdom_req *req, struct > pwd_grp *pg_data, > case SSS_ID_TYPE_GID: > res->response_type = RESP_GROUP; > res->data.group.domain_name = strdup(domain_name); > + if ((locat = strchr(pg_data->data.grp.gr_name, '@')) != > NULL) { > + if (strstr(locat, domain_name) != NULL) { > + locat[0] = 0; > + } > + } > res->data.group.group_name = > > strdup(pg_data->data.grp.gr_name); > > @@ -438,6 +460,10 @@ done: > free_resp_data(res); > } > > + if (locat != NULL) { > + locat[0] = '@'; > + } > + > return ret; > } > > -- > 1.8.3.1 > > _______________________________________________ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel