On Tue, Jul 09, 2013 at 11:42:00AM +0200, Jakub Hrozek wrote:
> 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 <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.
> > 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.
btw of course I tested with Sumit's patches applied before these two.
They are quite large and I don't think I can review them, but from
purely functional point of view, ack.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
