date:20130709

Re: [Freeipa-devel] [PATCH] 116 Add PAC to master host TGTs

2013-07-09 Thread Jakub Hrozek

On Tue, Jul 09, 2013 at 02:12:33PM +0300, Alexander Bokovoy wrote:
> On Tue, 09 Jul 2013, Jakub Hrozek wrote:
> >On Wed, Jul 03, 2013 at 02:53:55PM +0200, Sumit Bose wrote:
> >>On Wed, Jul 03, 2013 at 01:00:43PM +0300, Alexander Bokovoy wrote:
> >>> On Mon, 01 Jul 2013, Sumit Bose wrote:
> >>> >Hi,
> >>> >
> >>> >this patch fixes https://fedorahosted.org/freeipa/ticket/3651 but only
> >>> >to allow SSSD running on a FreeIPA server to access the AD LDAP server.
> >>> >In the ticket a more generic solution is described but since there is no
> >>> >other use case so far I think this patch is sufficient for the time
> >>> >being.
> >>> >
> >>> >bye,
> >>> >Sumit
> >>>
> >>> >From a707d8f9d771dfe4fb8487e051519dba0ef72449 Mon Sep 17 00:00:00 2001
> >>> >From: Sumit Bose 
> >>> >Date: Mon, 1 Jul 2013 13:47:22 +0200
> >>> >Subject: [PATCH] Add PAC to master host TGTs
> >>> >
> >>> >For a proper SALS bind with GSSAPI against an AD LDAP server a PAC is
> >>> >needed. To allow SSSD in ipa_server_mode to access the LDAP or GC server
> >>> >of a trusted domain with the credentials of a FreeIPA server host a
> >>> >PAC must be added to the TGT for the host.
> >>> s/SALS/SASL/
> >>
> >>Thank you for the review, I've fixed the typo and added the numerical
> >>values for the well-known RIDs to the commit message.
> >>
> >>>
> >>>
> >>> >To determine if a host is a FreeIPA server or not it is checked if there
> >>> >is an entry for the host in cn=master,cn=ipa,cn=etc,$base. Unfortunately
> >>> >this requires an additional LDAP lookup. But since TGS-REQs for hosts
> >>> >should be rare I think it is acceptable for the time being.
> >>> I think it is better to change this lookup to
> >>> "cn=ADTRUST,cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX", it would
> >>> explicitly limit us to the IPA masters running AD trusts.
> >>
> >>I'm not sure if this restriction is needed. With SSSD's ipa_server_mode
> >>any IPA master (which networkwise can access an AD server of the trusted
> >>domain) can read AD user and group data, no running smbd or winbind is
> >>required. So it would be possible to run the extdom plugin or the compat
> >>plugin for the legacy clients on any IPA server which would allow a much
> >>better load balancing.
> >>
> >>If there are other concerns I'm happy to add the restriction.
> >>
> >>bye,
> >>Sumit
> >
> >I don't think I know the code good enough to provide a full review, but
> >the patch enables the lookups from an IPA master without any additional
> >hacks. So ack on functionality at least.
> Ok.
> 
> I've extended this functionality to generate MS-PAC also for services
> running on IPA masters. Patch attached.
> 
> This is needed to finally get rid of access to trust auth material for
> IPA python code. HTTP/fqdn@REALM will now be able to authenticate
> against AD LDAP server and look up needed information directly, without
> elevating privileges to trust admins.
> 
> This should also help for AD range discovery Tomas is working on.
> 

Hi,

The patch looks good to me so I'm giving my +1. I would appreciate other
review too before a full ack, though.

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

Re: [Freeipa-devel] [PATCH] 148 Skip cert issuer validation in service and host commands in CA-less install

2013-07-09 Thread Rob Crittenden


Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza


ACK, pushed to master and ipa-3-2

rob

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

Re: [Freeipa-devel] [PATCH] Fix client install exception if /etc/ssh is missing

2013-07-09 Thread Rob Crittenden


Nathaniel McCallum wrote:

On Thu, 2013-07-04 at 11:20 +0200, Jan Cholasta wrote:

On 3.7.2013 22:11, Nathaniel McCallum wrote:

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



If the directory doesn't exist, update_ssh_keys is no-op, so I would prefer

if not os.path.isdir(ssh_dir):
  return

at the beginning of update_ssh_keys instead.


Attached.

I don't care which version of the patch gets merged...


ACK, pushed to master and ipa-3-2

rob

___
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

2013-07-09 Thread Rob Crittenden


Alexander Bokovoy wrote:

On Fri, 28 Jun 2013, Petr Vobornik wrote:

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

ACK



pushed to master and ipa-3-2

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

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

2013-07-09 Thread Rob Crittenden


Alexander Bokovoy wrote:

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.




pushed to master and ipa-3-2

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

Re: [Freeipa-devel] Web UI integration tests

2013-07-09 Thread Ana Krivokapic

On 06/21/2013 10:56 AM, Petr Vobornik wrote:
> Sending an initial implementation of Web UI integration tests. The effort is
> documented at http://www.freeipa.org/page/Web_UI_Integration_Tests .
>
> The coverage is not complete but rather basic. Sending it now as this is
> ongoing task.
>
> Currently it tries to open all facets and dialogs and perform
> add,mod,find,delete,add/remove associations for every entity except trusts (to
> be implemented).
>
> First two attached patches are changing Web UI to be little more test-friendly
> - they add some names to element and such. Tests are in the last patch.
>
> https://fedorahosted.org/freeipa/ticket/3744
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

I tried setting up and running the new tests, and overall it works really well.
The documentation page is very clear and to the point, and the setup script is
especially handy. Thanks!

Below are some comments (mostly related to the python code in ui_driver.py).

- I got whitespace warnings when applying patch 424
- ui_driver.py:110 - Should be 'except IOError, e', as you use 'e' within the
except clause.
- ui_driver.py:110 - There's too much code in the try block. We should have as
little code as possible in the try block (ideally only the code that could
actually raise the exception).
- ui_driver.py:200 - It says 'single' in the docstring, but the argument name is
'all'.
- ui_driver.py:205 - Instead of raising ValueError, I would prefer something
like: assert expression, 'expression is missing'
- ui_driver.py:643 - There's a print statement, I assume a leftover from some
debugging. :)



-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

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

[Freeipa-devel] [PATCHES] 149-151 Ask for PKCS#12 password interactively

2013-07-09 Thread Jan Cholasta


Hi,

the attached patches fix .

Also added a small patch to fix a formatting issue with 
installutils.read_password.


Honza

--
Jan Cholasta
>From 6a1eedeb478dce9acced03cf3ee2a502384428a9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 9 Jul 2013 10:23:47 +
Subject: [PATCH 1/3] Ask for PKCS#12 password interactively in
 ipa-server-install.

https://fedorahosted.org/freeipa/ticket/3717
---
 install/tools/ipa-server-install | 66 ++--
 1 file changed, 43 insertions(+), 23 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index cc88a0b..e477dee 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -276,14 +276,20 @@ def parse_options():
 if not options.forwarders and not options.no_forwarders:
 parser.error("You must specify at least one --forwarder option or --no-forwarders option")
 
-# If any of the PKCS#12 options are selected, all are required. Create a
-# list of the options and count it to enforce that all are required without
-# having a huge set of it blocks.
-pkcs12 = [options.dirsrv_pkcs12, options.http_pkcs12, options.dirsrv_pin, options.http_pin]
-cnt = pkcs12.count(None)
-if cnt > 0 and cnt < 4:
+# If any of the PKCS#12 options are selected, all are required.
+pkcs12_req = (options.dirsrv_pkcs12, options.http_pkcs12)
+pkcs12_opt = (options.pkinit_pkcs12,)
+if any(pkcs12_req + pkcs12_opt) and not all(pkcs12_req):
 parser.error("All PKCS#12 options are required if any are used.")
 
+if options.unattended:
+if options.dirsrv_pkcs12 and not options.dirsrv_pin:
+parser.error("You must specify --dirsrv_pin with --dirsrv_pkcs12")
+if options.http_pkcs12 and not options.http_pin:
+parser.error("You must specify --http_pin with --http_pkcs12")
+if options.pkinit_pkcs12 and not options.pkinit_pin:
+parser.error("You must specify --pkinit_pin with --pkinit_pkcs12")
+
 if options.dirsrv_pkcs12 and not options.root_ca_file:
 parser.error(
 "--root-ca-file must be given with the PKCS#12 options.")
@@ -704,18 +710,6 @@ def main():
 sys.exit(1)
 cert = certdict[certissuer]
 
-if options.http_pkcs12:
-http_pin_file = ipautil.write_tmp_file(options.http_pin)
-http_pkcs12_info = (options.http_pkcs12, http_pin_file.name)
-
-if options.dirsrv_pkcs12:
-dirsrv_pin_file = ipautil.write_tmp_file(options.dirsrv_pin)
-dirsrv_pkcs12_info = (options.dirsrv_pkcs12, dirsrv_pin_file.name)
-
-if options.pkinit_pkcs12:
-pkinit_pin_file = ipautil.write_tmp_file(options.pkinit_pin)
-pkinit_pkcs12_info = (options.pkinit_pkcs12, pkinit_pin_file.name)
-
 # We only set up the CA if the PKCS#12 options are not given.
 if options.dirsrv_pkcs12:
 setup_ca = False
@@ -834,12 +828,38 @@ def main():
 else:
 domain_name = options.domain_name
 
+ca_file = options.root_ca_file
+
 if options.http_pkcs12:
-# Check the given PKCS#12 files
-ca_file = options.root_ca_file
-check_pkcs12 = installutils.check_pkcs12
-http_cert_name = check_pkcs12(http_pkcs12_info, ca_file, host_name)
-dirsrv_cert_name = check_pkcs12(dirsrv_pkcs12_info, ca_file, host_name)
+if not options.http_pin:
+options.http_pin = installutils.read_password(
+options.http_pkcs12, confirm=False, validate=False)
+if options.http_pin is None:
+sys.exit("%s password required" % options.http_pkcs12)
+http_pin_file = ipautil.write_tmp_file(options.http_pin)
+http_pkcs12_info = (options.http_pkcs12, http_pin_file.name)
+http_cert_name = installutils.check_pkcs12(
+http_pkcs12_info, ca_file, host_name)
+
+if options.dirsrv_pkcs12:
+if not options.dirsrv_pin:
+options.dirsrv_pin = installutils.read_password(
+options.dirsrv_pkcs12, confirm=False, validate=False)
+if options.dirsrv_pin is None:
+sys.exit("%s password required" % options.dirsrv_pkcs12)
+dirsrv_pin_file = ipautil.write_tmp_file(options.dirsrv_pin)
+dirsrv_pkcs12_info = (options.dirsrv_pkcs12, dirsrv_pin_file.name)
+dirsrv_cert_name = installutils.check_pkcs12(
+dirsrv_pkcs12_info, ca_file, host_name)
+
+if options.pkinit_pkcs12:
+if not options.pkinit_pin:
+options.pkinit_pin = installutils.read_password(
+options.pkinit_pkcs12, confirm=False, validate=False)
+if options.pkinit_pin is None:
+sys.exit("%s password required" % options.pkinit_pkcs12)
+pkinit_pin_file = ipautil.write_tmp_file(options.pkinit_pin)
+pkinit_pkcs12_info = (options.pkinit_pkcs1

Re: [Freeipa-devel] [PATCH] 116 Add PAC to master host TGTs

2013-07-09 Thread Alexander Bokovoy

On Tue, 09 Jul 2013, Jakub Hrozek wrote:

On Wed, Jul 03, 2013 at 02:53:55PM +0200, Sumit Bose wrote:

On Wed, Jul 03, 2013 at 01:00:43PM +0300, Alexander Bokovoy wrote:
> On Mon, 01 Jul 2013, Sumit Bose wrote:
> >Hi,
> >
> >this patch fixes https://fedorahosted.org/freeipa/ticket/3651 but only
> >to allow SSSD running on a FreeIPA server to access the AD LDAP server.
> >In the ticket a more generic solution is described but since there is no
> >other use case so far I think this patch is sufficient for the time
> >being.
> >
> >bye,
> >Sumit
>
> >From a707d8f9d771dfe4fb8487e051519dba0ef72449 Mon Sep 17 00:00:00 2001
> >From: Sumit Bose 
> >Date: Mon, 1 Jul 2013 13:47:22 +0200
> >Subject: [PATCH] Add PAC to master host TGTs
> >
> >For a proper SALS bind with GSSAPI against an AD LDAP server a PAC is
> >needed. To allow SSSD in ipa_server_mode to access the LDAP or GC server
> >of a trusted domain with the credentials of a FreeIPA server host a
> >PAC must be added to the TGT for the host.
> s/SALS/SASL/

Thank you for the review, I've fixed the typo and added the numerical
values for the well-known RIDs to the commit message.

>
>
> >To determine if a host is a FreeIPA server or not it is checked if there
> >is an entry for the host in cn=master,cn=ipa,cn=etc,$base. Unfortunately
> >this requires an additional LDAP lookup. But since TGS-REQs for hosts
> >should be rare I think it is acceptable for the time being.
> I think it is better to change this lookup to
> "cn=ADTRUST,cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX", it would
> explicitly limit us to the IPA masters running AD trusts.

I'm not sure if this restriction is needed. With SSSD's ipa_server_mode
any IPA master (which networkwise can access an AD server of the trusted
domain) can read AD user and group data, no running smbd or winbind is
required. So it would be possible to run the extdom plugin or the compat
plugin for the legacy clients on any IPA server which would allow a much
better load balancing.

If there are other concerns I'm happy to add the restriction.

bye,
Sumit

I don't think I know the code good enough to provide a full review, but
the patch enables the lookups from an IPA master without any additional
hacks. So ack on functionality at least.

Ok.

I've extended this functionality to generate MS-PAC also for services
running on IPA masters. Patch attached.

This is needed to finally get rid of access to trust auth material for
IPA python code. HTTP/fqdn@REALM will now be able to authenticate
against AD LDAP server and look up needed information directly, without
elevating privileges to trust admins.

This should also help for AD range discovery Tomas is working on.

--
/ Alexander Bokovoy
>From 0d377b563edbc31342f1a026deac660b55322f86 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Tue, 9 Jul 2013 14:05:02 +0300
Subject: [PATCH 17/17] Generate syntethic MS-PAC for all services running on
 IPA master

MS-PAC is required to be present in TGT if one wants to connect to
AD services using this TGT. Users get MS-PAC by default, SSSD in
ipa_server_mode uses host/fqdn@REALM principal to talk to AD LDAP.

This patch enables other services running on IPA master to connect
to AD services. This is required for IPA python code doing discovery
of remote AD domain settings shortly after IPA-AD trust has been
established.
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 56 +
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 92dc8dd..f1df022 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -60,6 +60,7 @@ static char *user_pac_attrs[] = {
 "cn",
 "fqdn",
 "gidNumber",
+"managedBy",
 "krbPrincipalName",
 "krbCanonicalName",
 "krbTicketPolicyReference",
@@ -359,15 +360,33 @@ static int sid_split_rid(struct dom_sid *sid, uint32_t 
*rid)
 return 0;
 }

-static bool is_master_host(struct ipadb_context *ipactx, const char *fqdn)
+static bool is_master_host(struct ipadb_context *ipactx, const char *fqdn, 
bool managedby)
 {
 int ret;
 char *master_host_base = NULL;
+char *master_fqdn = NULL;
 LDAPMessage *result = NULL;
 krb5_error_code err;

-ret = asprintf(&master_host_base, "cn=%s,cn=masters,cn=ipa,cn=etc,%s",
-  fqdn, ipactx->base);
+if (managedby) {
+err = ipadb_simple_search(ipactx, (char*) fqdn, LDAP_SCOPE_BASE,
+  NULL, NULL, &result);
+if (err == 0) {
+ret = ipadb_ldap_attr_to_str(ipactx->lcontext, result, "fqdn", 
&master_fqdn);
+if (ret == 0) {
+ret = asprintf(&master_host_base, 
"cn=%s,cn=masters,cn=ipa,cn=etc,%s",
+  master_fqdn, ipactx->base);
+}
+free(master_fqdn);
+ldap_msgfree(result);
+} else {
+

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

Re: [Freeipa-devel] [PATCH] 116 Add PAC to master host TGTs

2013-07-09 Thread Jakub Hrozek

On Wed, Jul 03, 2013 at 02:53:55PM +0200, Sumit Bose wrote:
> On Wed, Jul 03, 2013 at 01:00:43PM +0300, Alexander Bokovoy wrote:
> > On Mon, 01 Jul 2013, Sumit Bose wrote:
> > >Hi,
> > >
> > >this patch fixes https://fedorahosted.org/freeipa/ticket/3651 but only
> > >to allow SSSD running on a FreeIPA server to access the AD LDAP server.
> > >In the ticket a more generic solution is described but since there is no
> > >other use case so far I think this patch is sufficient for the time
> > >being.
> > >
> > >bye,
> > >Sumit
> > 
> > >From a707d8f9d771dfe4fb8487e051519dba0ef72449 Mon Sep 17 00:00:00 2001
> > >From: Sumit Bose 
> > >Date: Mon, 1 Jul 2013 13:47:22 +0200
> > >Subject: [PATCH] Add PAC to master host TGTs
> > >
> > >For a proper SALS bind with GSSAPI against an AD LDAP server a PAC is
> > >needed. To allow SSSD in ipa_server_mode to access the LDAP or GC server
> > >of a trusted domain with the credentials of a FreeIPA server host a
> > >PAC must be added to the TGT for the host.
> > s/SALS/SASL/
> 
> Thank you for the review, I've fixed the typo and added the numerical
> values for the well-known RIDs to the commit message.
> 
> > 
> > 
> > >To determine if a host is a FreeIPA server or not it is checked if there
> > >is an entry for the host in cn=master,cn=ipa,cn=etc,$base. Unfortunately
> > >this requires an additional LDAP lookup. But since TGS-REQs for hosts
> > >should be rare I think it is acceptable for the time being.
> > I think it is better to change this lookup to
> > "cn=ADTRUST,cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX", it would
> > explicitly limit us to the IPA masters running AD trusts.
> 
> I'm not sure if this restriction is needed. With SSSD's ipa_server_mode
> any IPA master (which networkwise can access an AD server of the trusted
> domain) can read AD user and group data, no running smbd or winbind is
> required. So it would be possible to run the extdom plugin or the compat
> plugin for the legacy clients on any IPA server which would allow a much
> better load balancing.
> 
> If there are other concerns I'm happy to add the restriction.
> 
> bye,
> Sumit

I don't think I know the code good enough to provide a full review, but
the patch enables the lookups from an IPA master without any additional
hacks. So ack on functionality at least.

___
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-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 
> >>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(

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

Re: [Freeipa-devel] [PATCH] 116 Add PAC to master host TGTs

Re: [Freeipa-devel] [PATCH] 148 Skip cert issuer validation in service and host commands in CA-less install

Re: [Freeipa-devel] [PATCH] Fix client install exception if /etc/ssh is missing

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

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

Re: [Freeipa-devel] Web UI integration tests

[Freeipa-devel] [PATCHES] 149-151 Ask for PKCS#12 password interactively

Re: [Freeipa-devel] [PATCH] 116 Add PAC to master host TGTs

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

Re: [Freeipa-devel] [PATCH] 116 Add PAC to master host TGTs

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

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

12 matches

Site Navigation

Mail list logo

Footer information