Re: [Freeipa-devel] [PATCH] 0041 Fix bug in adtrustinstance

[Alexander Bokovoy]

On Tue, 25 Jun 2013, Ana Krivokapic wrote:

Hello,

Attached patch fixes https://fedorahosted.org/freeipa/ticket/3746.

--
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.




From 548f1626dbf9edf45bfcab358b28e510d1ec5757 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Tue, 25 Jun 2013 15:52:29 +0200
Subject: [PATCH] Fix bug in adtrustinstance

Incorrect tuple unpacking in adtrustinstance was causing ipa-adtrust-install
to fail when IPA was installed with no DNS.

https://fedorahosted.org/freeipa/ticket/3746
---
ipaserver/install/adtrustinstance.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
d2929801b431625764e5b949349ab63d2caaf696..4eb20d9517906df0b1952fe5033a050f3b55a797
 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -535,9 +535,9 @@ def __add_dns_service_records(self):
self.print_msg(err_msg)
self.print_msg("Add the following service records to your DNS " \
   "server for DNS zone %s: " % zone)
-for (srv, rdata) in ipa_srv_rec:
+for srv in ipa_srv_rec:
for suff in win_srv_suffix:
-self.print_msg(" - %s%s"  % (srv, suff))
+self.print_msg(" - %s%s"  % (srv[0], suff))
return

for (srv, rdata, port) in ipa_srv_rec:

ACK.


--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups

[Alexander Bokovoy]

On Thu, 27 Jun 2013, Jan Cholasta wrote:

On 27.6.2013 17:34, Rich Megginson wrote:

On 06/27/2013 09:31 AM, Jan Cholasta wrote:

The search is hard-coded in the referint plugin, see
.



Not sure if it makes sense to do a wildcard/substr search here - please
file a ticket with 389 to investigate.


https://fedorahosted.org/389/ticket/47411

So, should we merge this patchset or wait until 389-ds analyzes 47411?
To me it looks like we can use this one as an interim solution, once Web
UI performance is checked through.

--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCH] 427 Disable checkboxes and radios for readonly attributes

[Alexander Bokovoy]

On Fri, 28 Jun 2013, Petr Vobornik wrote:

https://fedorahosted.org/freeipa/ticket/3764

ACK

--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] F19 is broken w.r.t. user password change

[Alexander Bokovoy]

On Sat, 29 Jun 2013, Simo Sorce wrote:

On Sat, 2013-06-29 at 07:46 +0300, Alexander Bokovoy wrote:

On Fri, 28 Jun 2013, Alexander Bokovoy wrote:
>Hi!
>
>Found today when preparing my talk at LVEE conference:
>
>When running 'ipa passwd ' or 'kinit ' for the first time
>(i.e. forcing a password change), ipa-pwd-extop causes denial of
>password change:
>
>[28/Jun/2013:22:02:43 +0300] ipa-pwd-extop - Received extended operation 
request with OID 1.3.6.1.4.1.4203.1.11.1
>
>[28/Jun/2013:22:02:43 +0300] ipa-pwd-extop - Pre-Encoded passwords are not 
valid
>[28/Jun/2013:22:02:43 +0300] roles-plugin - --> roles_post_op
>[28/Jun/2013:22:02:43 +0300] roles-plugin - --> roles_cache_change_notify
>[28/Jun/2013:22:02:43 +0300] roles-plugin - <-- roles_post_op
>[28/Jun/2013:22:02:43 +0300] ipa-pwd-extop - Failed to update password
>
>Apparently, we receive password encoded as {SSHA} scheme and it breaks
>any password change. Appropriate code checks are in
>daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c:719-738
>
>I've reproduced it with Fedora 19 RC2 ISO, with git master rpms, and
>with freeipa-devel repo. Basically, this is release blocker for 3.3
>right now.
Thanks to Nathan to point out to this change in 389-ds-base:
http://directory.fedoraproject.org/wiki/Password_Administrator

I added

passwordAdminDn: cn=admins,cn=groups,cn=accounts,$SUFFIX

to cn=config and got it fixed for stock FreeIPA configuration.
However, the change like this would not be enough for delegated roles.

Patch that fixes basic problem is attached, please review.


Although the patch 'fixes' the problem for the admin group it break s
the IPA model.
We need to get a way to disable this behavior in 389DS (we already do
our own checks since long), and work for a long term solution where
policy checks can be delegated to a plugin.

It is a priority to revert the new logic in 389DS immediately, and work
on a plan.

389-ds team (thanks Nathan and Noriko!) released fixed version and it is
now available in Fedora 19. I've checked 1.3.1.3-1.fc19 and it fixes the
issue without additional changes to FreeIPA.

--
/ 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

[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

[Freeipa-devel] [PATCH 0171-0172] Fix potential problems found by Clang static analyzer

[Petr Spacek]

Hello,

several warnings from Clang static analyzer popped up after upgrade to Fedora 
19. Attached patches should fix all problems found by 
clang-analyzer-3.3-0.6.rc3.fc19.x86_64.


--
Petr^2 Spacek
From fe43bd17172c899e86e105e398dfe3a0dd27422d Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 8 Jul 2013 10:27:44 +0200
Subject: [PATCH] Remove false positive NULL dereference reported by Clang
 static analyzer.

The potential problem was reported by clang-analyzer-3.3-0.6.rc3.fc19.x86_64.
http://llvm.org/bugs/show_bug.cgi?id=16541

Signed-off-by: Petr Spacek 
---
 src/ldap_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index c98c9cb955252d1381c514a454e6679b8dc93521..25d86862130ad14a62d716f03a3b3a93118cdf77 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3513,6 +3513,9 @@ ldap_pool_getconnection(ldap_pool_t *pool, ldap_connection_t ** conn)
 	ldap_conn = *conn;
 
 	CHECK(semaphore_wait_timed(&pool->conn_semaphore));
+	/* Following assertion is necessary to convince clang static analyzer
+	 * that the loop is always entered. */
+	REQUIRE(pool->connections > 0);
 	for (i = 0; i < pool->connections; i++) {
 		ldap_conn = pool->conns[i];
 		if (isc_mutex_trylock(&ldap_conn->lock) == ISC_R_SUCCESS)
-- 
1.8.3.1

From b2918086c7bead609dc2a2d7a494322fe5106bb3 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 8 Jul 2013 10:40:51 +0200
Subject: [PATCH] Modify ldap_pool_destroy() to handle partially allocated
 ldap_pool_t.

The potential problem was reported by clang-analyzer-3.3-0.6.rc3.fc19.x86_64.

Signed-off-by: Petr Spacek 
---
 src/ldap_helper.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 25d86862130ad14a62d716f03a3b3a93118cdf77..514b81e8da1539e9402cef8a07f8feafeb13ff4d 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -3473,6 +3473,7 @@ cleanup:
 	ldap_pool_destroy(&pool);
 	return result;
 }
+
 static void
 ldap_pool_destroy(ldap_pool_t **poolp)
 {
@@ -3486,14 +3487,16 @@ ldap_pool_destroy(ldap_pool_t **poolp)
 	if (pool == NULL)
 		return;
 
-	for (i = 0; i < pool->connections; i++) {
-		ldap_conn = pool->conns[i];
-		if (ldap_conn != NULL)
-			destroy_ldap_connection(&ldap_conn);
-	}
+	if (pool->conns != NULL) {
+		for (i = 0; i < pool->connections; i++) {
+			ldap_conn = pool->conns[i];
+			if (ldap_conn != NULL)
+destroy_ldap_connection(&ldap_conn);
+		}
 
-	SAFE_MEM_PUT(pool->mctx, pool->conns,
-		 pool->connections * sizeof(ldap_connection_t *));
+		SAFE_MEM_PUT(pool->mctx, pool->conns,
+			 pool->connections * sizeof(ldap_connection_t *));
+	}
 
 	semaphore_destroy(&pool->conn_semaphore);
 
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCHES] 0039-0040 systemd ipactl fixes

[Simo Sorce]

On Thu, 2013-06-20 at 17:13 +0200, Ana Krivokapic wrote:
> -After=network.target
> +After=network.target dirsrv.target
> pki-tomcatd@pki-tomcat.service pki-cad.target certmonger.service
> httpd.service krb5kdc.service messagebus.service nslcd.service
> nscd.service ntpd.service portmap.service rpcbind.service
> kadmin.service sshd.service autofs.service rpcgssd.service
> rpcidmapd.service chronyd.service
> 
Won't this cause ipa.service to try to restart things twice ?
Also this will unconditionally try to start the CA even if not
installed.

NACK, please let ipa.service handle starting and stopping daemons.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
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

[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] [PATCHES] 0039-0040 systemd ipactl fixes

[Alexander Bokovoy]

On Mon, 08 Jul 2013, Simo Sorce wrote:

On Thu, 2013-06-20 at 17:13 +0200, Ana Krivokapic wrote:

-After=network.target
+After=network.target dirsrv.target
pki-tomcatd@pki-tomcat.service pki-cad.target certmonger.service
httpd.service krb5kdc.service messagebus.service nslcd.service
nscd.service ntpd.service portmap.service rpcbind.service
kadmin.service sshd.service autofs.service rpcgssd.service
rpcidmapd.service chronyd.service


Won't this cause ipa.service to try to restart things twice ?
Also this will unconditionally try to start the CA even if not
installed.

No, this is for dependency ordering only, not for actual start/stop
dependency action.


From systemd.unit(5):


Note that this setting is independent of and orthogonal to the
requirement dependencies as configured by Requires=. It is a common
pattern to include a unit name in both the After= and Requires= option
in which case the unit listed will be started before the unit that is
configured with these options. This option may be specified more than
once, in which case ordering dependencies for all listed names are
created.



--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCHES] 0039-0040 systemd ipactl fixes

[Simo Sorce]

On Mon, 2013-07-08 at 16:18 +0300, Alexander Bokovoy wrote:
> On Mon, 08 Jul 2013, Simo Sorce wrote:
> >On Thu, 2013-06-20 at 17:13 +0200, Ana Krivokapic wrote:
> >> -After=network.target
> >> +After=network.target dirsrv.target
> >> pki-tomcatd@pki-tomcat.service pki-cad.target certmonger.service
> >> httpd.service krb5kdc.service messagebus.service nslcd.service
> >> nscd.service ntpd.service portmap.service rpcbind.service
> >> kadmin.service sshd.service autofs.service rpcgssd.service
> >> rpcidmapd.service chronyd.service
> >>
> >Won't this cause ipa.service to try to restart things twice ?
> >Also this will unconditionally try to start the CA even if not
> >installed.
> No, this is for dependency ordering only, not for actual start/stop
> dependency action.
> 
> From systemd.unit(5):
> 
> Note that this setting is independent of and orthogonal to the
> requirement dependencies as configured by Requires=. It is a common
> pattern to include a unit name in both the After= and Requires= option
> in which case the unit listed will be started before the unit that is
> configured with these options. This option may be specified more than
> once, in which case ordering dependencies for all listed names are
> created.

Yes but what is the point of "starting" ipa.service "after" its own
components ?

I can understand putting there the following:
 * network.target
 * certmonger.service
 * messagebus.service

I do not understand putting there these:
 * dirsrv.target
 * pki-tomcatd@pki-tomcat.service
 * pki-cad.target
 * httpd.service
 * krb5kdc.service
 * kadmin.service
 * ntpd.service
As these are started by ipa.service itself

I am not sure why these are put there either:
 * nslcd.service
 * nscd.service
 * portmap.service
 * rpcbind.service
 * sshd.service
 * autofs.service
 * rpcgssd.service
 * rpcidmapd.service
 * chronyd.service
Why do we need to be started after these ? We have no direct dependency.
Besides we do not use nslcd, nscd, chronyd at all when installing
freeipa as we replace all of them with sssd (actually not listed at all)
and ntpd

Also why nfs and the rpc stuff should be started earlier ?
rpcgssd in particular may try to kinit with the nfs keytab, so starting
it before the kdc wouldn't work well.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
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

[Jakub Hrozek]

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.

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

Re: [Freeipa-devel] [PATCH] 0029 Make sure replication works after DM password is changed

[Ana Krivokapic]

On 06/25/2013 05:28 PM, Ana Krivokapic wrote:
> On 06/24/2013 02:27 PM, Tomas Babej wrote:
>> On 06/11/2013 04:42 PM, Ade Lee wrote:
>> [snip]
>>> Just FYI, we plan to do a new release of pki-core today (pki-core-10.0.3-2)
>>> to address this issue.
 -- 
 Regards,

 Ana Krivokapic
 Associate Software Engineer
 FreeIPA team
 Red Hat Inc.
>> Ok, so I tested the patch, since pki-core has the PkiExport command fixed 
>> now.
>>
>> I'm getting a little bit further now.
>>
>> [tbabej@vm-127 ~]$ sudo ipa-replica-prepare --ip-address 10.34.47.129
>> vm-129.idm.lab.eng.brq.redhat.com
>> Directory Manager (existing master) password:
>>
>> Preparing replica for vm-129.idm.lab.eng.brq.redhat.com from
>> vm-127.idm.lab.eng.brq.redhat.com
>> Constraint violation: Failed to update password
>>
>> With debug output, I get (snipped out irrelevant parts):
>>
>> Directory Manager (existing master) password:
>>
>> ipa.ipaserver.plugins.ldap2.ldap2: DEBUG: Created connection
>> context.ldap2_57668944
>> ipa.ipapython.ipaldap.SchemaCache: DEBUG: retrieving schema for SchemaCache
>> url=ldapi://%2fvar%2frun%2fslapd-IDM-LAB-ENG-BRQ-REDHAT-COM.socket
>> conn=
>> ipa.ipaserver.plugins.ldap2.ldap2: DEBUG: Destroyed connection
>> context.ldap2_57668944
>> ipa: DEBUG: Search DNS for vm-129.idm.lab.eng.brq.redhat.com
>> ipa: DEBUG: Search failed: [Errno -2] Name or service not known
>> ipa.ipapython.ipaldap.SchemaCache: DEBUG: flushing
>> ldapi://%2fvar%2frun%2fslapd-IDM-LAB-ENG-BRQ-REDHAT-COM.socket from 
>> SchemaCache
>> ipa.ipapython.ipaldap.SchemaCache: DEBUG: retrieving schema for SchemaCache
>> url=ldapi://%2fvar%2frun%2fslapd-IDM-LAB-ENG-BRQ-REDHAT-COM.socket
>> conn=
>> ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: DEBUG: Not logging
>> to a file
>> ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: DEBUG:
>> ipa-replica-prepare was invoked with arguments
>> ['vm-129.idm.lab.eng.brq.redhat.com'] and options: {'log_file': None,
>> 'verbose': True, 'reverse_zone': None, 'setup_pkinit': True, 'http_pin': 
>> None,
>> 'quiet': False, 'http_pkcs12': None, 'pkinit_pkcs12': None, 'ca_file':
>> '/root/cacert.p12', 'no_reverse': False, 'dirsrv_pkcs12': None, 'password':
>> None, 'ip_address': CheckedIPAddress('10.34.47.129'), 'dirsrv_pin': None,
>> 'pkinit_pin': None}
>> ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: INFO: Preparing
>> replica for vm-129.idm.lab.eng.brq.redhat.com from
>> vm-127.idm.lab.eng.brq.redhat.com
>> ipa.ipapython.ipaldap.SchemaCache: DEBUG: flushing
>> ldapi://%2fvar%2frun%2fslapd-IDM-LAB-ENG-BRQ-REDHAT-COM.socket from 
>> SchemaCache
>> ipa.ipapython.ipaldap.SchemaCache: DEBUG: retrieving schema for SchemaCache
>> url=ldapi://%2fvar%2frun%2fslapd-IDM-LAB-ENG-BRQ-REDHAT-COM.socket
>> conn=
>> ipa: DEBUG: Starting external process
>> ipa: DEBUG: args=/usr/bin/PKCS12Export -d /etc/pki/pki-tomcat/alias/ -p
>> /tmp/tmprgUrso -w /tmp/tmp6SBBXF -o /root/cacert.p12
>> ipa: DEBUG: Process finished, return code=0
>> ipa: DEBUG: stdout=
>> ipa: DEBUG: stderr=
>> ipa.ipaserver.plugins.ldap2.ldap2: DEBUG: Created connection
>> context.ldap2_139884970376144
>> ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: DEBUG: File
>> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in 
>> execute
>> return_value = self.run()
>>   File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_replica_prepare.py",
>> line 245, in run
>> self.copy_ds_certificate()
>>   File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_replica_prepare.py",
>> line 281, in copy_ds_certificate
>> self.update_pki_admin_password()
>>   File
>> "/usr/lib/python2.7/site-packages/ipaserver/install/ipa_replica_prepare.py",
>> line 520, in update_pki_admin_password
>> ldap.modify_password(dn, self.dirman_password)
>>   File "/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py", line
>> 332, in modify_password
>> self.conn.passwd_s(dn, old_pass, new_pass)
>>   File "/usr/lib64/python2.7/contextlib.py", line 35, in __exit__
>> self.gen.throw(type, value, traceback)
>>   File "/usr/lib/python2.7/site-packages/ipapython/ipaldap.py", line 919, in
>> error_handler
>> raise errors.DatabaseError(desc=desc, info=info)
>>
>> ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: DEBUG: The
>> ipa-replica-prepare command failed, exception: DatabaseError: Constraint
>> violation: Failed to update password
>> ipa.ipaserver.install.ipa_replica_prepare.ReplicaPrepare: ERROR: Constraint
>> violation: Failed to update password
>>
>> Tomas
> It seems that this time the culprit is 389-ds-base packages. The password 
> change
> is rejected when using the latest version of 389-ds-base
> (389-ds-base-1.3.1.2-1.fc19.x86_64). I tried testing it with a previous 
> version
> (389-ds-base-1.3.0.5-1.fc19.x86_64) and it works.
>
> I open an upstream ticket for the 389 DS project:
> https://fedorahosted.org/389/ticket/47406.
>

The password change rejecti

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

[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 
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

[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 
> 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