Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-11 Thread Alexander Bokovoy

On Wed, 10 Jul 2013, Simo Sorce wrote:

On Wed, 2013-07-10 at 19:15 +0300, Alexander Bokovoy wrote:

On Tue, 09 Jul 2013, Jakub Hrozek wrote:
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 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-10 Thread Alexander Bokovoy

On Tue, 09 Jul 2013, Jakub Hrozek wrote:

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 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-10 Thread Simo Sorce
On Wed, 2013-07-10 at 19:15 +0300, Alexander Bokovoy wrote:
 On Tue, 09 Jul 2013, Jakub Hrozek wrote:
 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
 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-09 Thread Alexander Bokovoy

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.




 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 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-09 Thread Jakub Hrozek
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 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-09 Thread Jakub Hrozek
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 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-08 Thread Alexander Bokovoy

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.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-08 Thread Alexander Bokovoy

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.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-08 Thread Alexander Bokovoy

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);
 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 ) {
+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;
+ 

Re: [Freeipa-devel] [PATCH] 117 extdom: replace winbind calls with POSIX/SSSD calls

2013-07-08 Thread Jakub Hrozek
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