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
