Re: [Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page
On 03/06/2013 08:26 PM, Ana Krivokapic wrote: On 03/06/2013 10:40 AM, Petr Vobornik wrote: On 03/05/2013 05:52 PM, Ana Krivokapic wrote: On 02/27/2013 05:10 PM, Petr Vobornik wrote: On 02/27/2013 04:20 PM, Ana Krivokapic wrote: Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 The patch looks good, but there is a issue we don't have a precedence for. The mod command is doing dns check for new domains. Currently we can't specify --force option to bypass the check. I see two possible implementations: 1) On update, when user adds or modifies the values, a dialog would pop up and ask user whether he wants to force it. 2) Another option is to disable edit on the list(deletion would be still allowed) and move the add operation to separate action in action list. I prefer the former. Latter might have issues with two modifications (delete and add) at the same time at two different places (facet and add dialog). Added force option to the error dialog. Updated patch is attached. 1) I think the dialog with the force should be shown before executing the operation. Sometimes, DNS check can take several seconds. There is no point for waiting for the error if you know that it will fail. 2) Regardless of #1. I don't think that just adding 'force' button without explaining the user what it means is the way to go. Previously (solution #1) I had in mind to show following dialog after clicking on 'update': --- [Check DNS] --- Do you also want to perform DNS check? [Check DNS] [Force Update] --- Default button (confirm button) will be [Check DNS] Thanks, fixed, and I also added a Cancel button, in case the user wants to back out. Almost there, as discussed in person: 1. following strings should be add to and obtained from internal.py plugin: title: 'Check DNS', message: 'Do you also want to perform DNS check?', ok_label: 'Check DNS', 2. the server plugin should report all dns resolution failures, not just the first one. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1088 Recover DNA ranges when deleting a master
On 03/06/2013 09:52 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
[...]
On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric
Assignment Plugin,cn=plugins,cn=config is added before the entry itself.
I didn't test everything as I didn't get the access.
It shouldn't make a difference. What isn't working?
I get a CRITICAL message in the log:
add aci:
(targetattr=dnaNextRange || dnaNextValue ||
dnaMaxValue)(version 3.0;acl permission:Modify DNA Range;allow (write)
groupdn = ldap:///cn=Modify DNA
Range,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com;)
modifying entry cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config
2013-03-07T11:01:48Z DEBUG stderr=ldap_initialize(
ldap://vm-081.idm.lab.eng.brq.redhat.com:389/??base )
ldap_modify: No such object (32)
2013-03-07T11:01:48Z CRITICAL Failed to load replica-acis.ldif: Command
'/usr/bin/ldapmodify -v -f /tmp/tmpT55upM -H
ldap://vm-081.idm.lab.eng.brq.redhat.com:389 -x -D cn=Directory Manager
-y /tmp/tmplFeere' returned non-zero exit status 32
This particular ACI is added later by the updater. But, after failing
here the rest of the file is skipped, and the subsequent ACIs aren't
added in updates. Not being able to run CLEANALLRUV prevents cleanly
deleting a replica.
[...]
[...]
+try:
+repl = replication.ReplicationManager(realm, hostname,
dirman_passwd)
+except Exception, e:
+sys.exit(Connection failed: %s %
ipautil.convert_ldap_error(e))
ipaldap should convert LDAP errors to IPA ones, there's no need to call
convert_ldap_error. Same in other places.
It does in some but it isn't consistent. I removed my calls though.
$ ipa-replica-manage dnarange-show -p badpassword
Connection failed: {'desc': 'Invalid credentials'}
That's a bug. My patch 0194 should fix this, I'll check after it's
merged. Anyway it's not a show stopper now.
[...]
+failed = 0
+for ent in entries:
This loops more than necessary and is somewhat hard to follow. Consider
using for-else here:
for ...:
...
if okay:
break
else:
raise error
I simplified things a bit but a for/else won't work here as we need to
check all ranges all the time. It is perfectly fine to not fit into a
range, as long as it fits into SOME range.
Well, that's how for's (not if's) else clause works -- it's executed
after all the looping's done if you didn't `break` out.
http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops
Maybe I'm just used to it and it's too esoteric to the average reader,
though.
[...]
Ok, I'll drop this since it doesn't affect things with the new LDAP
backend.
Please do, you left it in by mistake.
I also added one change related to the LDAP core changes. In the past if
you did not have a ticket it would prompt for DM password. This was
broken after the updates. I added an additional except in
test_connection().
This should also be fixed soon in ipaldap. Thanks for putting up with
the changes.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 116-119 Make LDAP schema retrieval optional
Hi, these patches add flags to LDAPClient and IPAdmin constructors which can be used to disable schema retrieval and decoding of attributes. This should make interacting with AD easier (see http://www.redhat.com/archives/freeipa-devel/2013-March/msg00076.html). Honza -- Jan Cholasta From 33142d7e0a8508a783e1a1b4a7a22525337ce54d Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 7 Mar 2013 10:50:57 +0100 Subject: [PATCH 1/4] Do not fail if schema cannot be retrieved from LDAP server. --- ipaserver/ipaldap.py | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py index 4a46532..e6f82dc 100644 --- a/ipaserver/ipaldap.py +++ b/ipaserver/ipaldap.py @@ -270,13 +270,19 @@ class IPASimpleLDAPObject(object): self.log = log_mgr.get_logger(self) self.uri = uri self.conn = SimpleLDAPObject(uri) +self._has_schema = False self._schema = None self._force_schema_updates = force_schema_updates def _get_schema(self): -if self._schema is None: -self._schema = schema_cache.get_schema( -self.uri, self.conn, force_update=self._force_schema_updates) +if not self._has_schema: +try: +self._schema = schema_cache.get_schema( +self.uri, self.conn, +force_update=self._force_schema_updates) +except: +pass +self._has_schema = True return self._schema schema = property(_get_schema, None, None, 'schema associated with this LDAP server') @@ -307,6 +313,7 @@ class IPASimpleLDAPObject(object): # logical operations that have the potential to cause a schema # change. +self._has_schema = False self._schema = None def get_syntax(self, attr): @@ -315,6 +322,9 @@ class IPASimpleLDAPObject(object): if syntax is not None: return syntax +if self.schema is None: +return None + # Try to lookup the syntax in the schema returned by the server obj = self.schema.get_obj(ldap.schema.AttributeType, attr) if obj is not None: -- 1.8.1 From d4fccb0c9fac71f0c0be90cd19ec737fbff8c428 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 7 Mar 2013 10:52:57 +0100 Subject: [PATCH 2/4] Allow disabling LDAP schema retrieval in LDAPClient and IPAdmin. --- ipaserver/ipaldap.py | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py index e6f82dc..9d350f7 100644 --- a/ipaserver/ipaldap.py +++ b/ipaserver/ipaldap.py @@ -255,7 +255,7 @@ class IPASimpleLDAPObject(object): 'originscope': DN_SYNTAX_OID, # DN }) -def __init__(self, uri, force_schema_updates): +def __init__(self, uri, force_schema_updates, no_schema=False): An internal LDAP connection object :param uri: The LDAP URI to connect to @@ -266,15 +266,19 @@ class IPASimpleLDAPObject(object): Generally, it should be true if the API context is 'installer' or 'updates', but it must be given explicitly since the API object is not always available +:param no_schema: If true, schema is never requested from the server. self.log = log_mgr.get_logger(self) self.uri = uri self.conn = SimpleLDAPObject(uri) +self._no_schema = no_schema self._has_schema = False self._schema = None self._force_schema_updates = force_schema_updates def _get_schema(self): +if self._no_schema: +return None if not self._has_schema: try: self._schema = schema_cache.get_schema( @@ -1649,7 +1653,7 @@ class IPAdmin(LDAPClient): def __init__(self, host='', port=389, cacert=None, debug=None, ldapi=False, realm=None, protocol=None, force_schema_updates=True, - start_tls=False, ldap_uri=None): + start_tls=False, ldap_uri=None, no_schema=False): self.conn = None log_mgr.get_logger(self, True) if debug and debug.lower() == on: @@ -1669,7 +1673,8 @@ class IPAdmin(LDAPClient): LDAPClient.__init__(self, ldap_uri) -self.conn = IPASimpleLDAPObject(ldap_uri, force_schema_updates=True) +self.conn = IPASimpleLDAPObject(ldap_uri, force_schema_updates=True, +no_schema=no_schema) if start_tls: self.conn.start_tls_s() -- 1.8.1 From 8c89c1058ba1fa9798b284c9ed3efa5cd8d5a844 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 7 Mar 2013 10:56:03 +0100 Subject: [PATCH 3/4] Allow disabling attribute decoding in LDAPClient and IPAdmin. --- ipaserver/ipaldap.py | 16 +--- 1 file changed, 13
[Freeipa-devel] Custom SSL certificates
Hi, I'm investigating https://fedorahosted.org/freeipa/ticket/3363 (fix --http_pkcs12 friends). I can't find documentation on these options, and from the code I can't figure out enough about how they are/were supposed to work. Is it the case that they were last used/tested before IPA started using Dogtag, and have rotted since then? Custom certs don't make sense to use if Dogtag is installed, right? So when they're provided we should not install the CA and Certmonger? If that's the case it would be easier (development- and testing-wise) to just remove self-signed CA in the same set of patches. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 381 Preserve order of servers in ipa-client-install
When multiple servers are passed via --server option, ipadiscovery module changed its order. Make sure that we preserve it. Also make sure that user is always warned when a tested server is not available as then the server will be excluded from the fixed server list. https://fedorahosted.org/freeipa/ticket/3418 -- When working on this ticket I was thinking - do we make the right thing we deliberately remove a server from user-provided server list just because we cannot connect to it at the moment if discovery? It may just be temporarily down or something. Maybe we should preserve the original --server list in this case and use this list when writing krb5.conf or sssd.conf. Of course, for ipa-join or other active configuration commands we would have to use only the valid servers so that the we do not hit the server that is currently down. Martin From 1ba31219970ca32cfc27850a0655fa5b0c0a9da8 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Thu, 7 Mar 2013 13:54:11 +0100 Subject: [PATCH] Preserve order of servers in ipa-client-install When multiple servers are passed via --server option, ipadiscovery module changed its order. Make sure that we preserve it. Also make sure that user is always warned when a tested server is not available as then the server will be excluded from the fixed server list. https://fedorahosted.org/freeipa/ticket/3418 --- ipa-client/ipaclient/ipadiscovery.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py index 7fc6aae88672e15a6bf3068ef8769e4cc80184a4..f58937e7b9e14712ca025af3236416560af4beaa 100644 --- a/ipa-client/ipaclient/ipadiscovery.py +++ b/ipa-client/ipaclient/ipadiscovery.py @@ -248,7 +248,7 @@ class IPADiscovery(object): self.realm = ldapret[2] self.server_source = self.realm_source = ( 'Discovered from LDAP DNS records in %s' % self.server) -valid_servers.insert(0, server) +valid_servers.append(server) # verified, we actually talked to the remote server and it # is definetely an IPA server verified_servers = True @@ -258,7 +258,7 @@ class IPADiscovery(object): break elif ldapret[0] == NO_ACCESS_TO_LDAP or ldapret[0] == NO_TLS_LDAP: ldapaccess = False -valid_servers.insert(0, server) +valid_servers.append(server) # we may set verified_servers below, we don't have it yet if autodiscovered: # No need to keep verifying servers if we discovered them @@ -268,7 +268,7 @@ class IPADiscovery(object): root_logger.warn( '%s (realm %s) is not an IPA server', server, self.realm) elif ldapret[0] == NO_LDAP_SERVER: -root_logger.debug( +root_logger.warn( 'Unable to verify that %s (realm %s) is an IPA server', server, self.realm) -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page
On 03/07/2013 12:41 PM, Petr Vobornik wrote: On 03/06/2013 08:26 PM, Ana Krivokapic wrote: On 03/06/2013 10:40 AM, Petr Vobornik wrote: On 03/05/2013 05:52 PM, Ana Krivokapic wrote: On 02/27/2013 05:10 PM, Petr Vobornik wrote: On 02/27/2013 04:20 PM, Ana Krivokapic wrote: Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 The patch looks good, but there is a issue we don't have a precedence for. The mod command is doing dns check for new domains. Currently we can't specify --force option to bypass the check. I see two possible implementations: 1) On update, when user adds or modifies the values, a dialog would pop up and ask user whether he wants to force it. 2) Another option is to disable edit on the list(deletion would be still allowed) and move the add operation to separate action in action list. I prefer the former. Latter might have issues with two modifications (delete and add) at the same time at two different places (facet and add dialog). Added force option to the error dialog. Updated patch is attached. 1) I think the dialog with the force should be shown before executing the operation. Sometimes, DNS check can take several seconds. There is no point for waiting for the error if you know that it will fail. 2) Regardless of #1. I don't think that just adding 'force' button without explaining the user what it means is the way to go. Previously (solution #1) I had in mind to show following dialog after clicking on 'update': --- [Check DNS] --- Do you also want to perform DNS check? [Check DNS] [Force Update] --- Default button (confirm button) will be [Check DNS] Thanks, fixed, and I also added a Cancel button, in case the user wants to back out. Almost there, as discussed in person: 1. following strings should be add to and obtained from internal.py plugin: title: 'Check DNS', message: 'Do you also want to perform DNS check?', ok_label: 'Check DNS', 2. the server plugin should report all dns resolution failures, not just the first one. Fixed, updated patch is attached. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From 965fd9dfd19bf3539ce050a4490001716d1e1e6b Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Thu, 7 Mar 2013 14:12:49 +0100 Subject: [PATCH] Realm Domains page Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 --- install/ui/src/freeipa/app.js | 1 + install/ui/src/freeipa/realmdomains.js| 103 ++ install/ui/src/freeipa/webui.js | 3 +- install/ui/test/data/ipa_init.json| 6 ++ install/ui/test/data/ipa_init_objects.json| 42 +++ install/ui/test/data/realmdomains_show.json | 24 ++ ipalib/plugins/internal.py| 6 ++ ipalib/plugins/realmdomains.py| 7 +- tests/test_xmlrpc/test_realmdomains_plugin.py | 3 +- 9 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 install/ui/src/freeipa/realmdomains.js create mode 100644 install/ui/test/data/realmdomains_show.json diff --git a/install/ui/src/freeipa/app.js b/install/ui/src/freeipa/app.js index 9d89c1aede857ddfc27ebffa306c41172ed56bca..3dcb10f493824923254636c06b715164e419cce5 100644 --- a/install/ui/src/freeipa/app.js +++ b/install/ui/src/freeipa/app.js @@ -41,6 +41,7 @@ define([ './idrange', './netgroup', './policy', +'./realmdomains', './rule', './selinux', './serverconfig', diff --git a/install/ui/src/freeipa/realmdomains.js b/install/ui/src/freeipa/realmdomains.js new file mode 100644 index ..ea3997a151b8038276ec9cf3cf8c093dc10b468c --- /dev/null +++ b/install/ui/src/freeipa/realmdomains.js @@ -0,0 +1,103 @@ +/* Authors: + *Ana Krivokapic akriv...@redhat.com + * + * Copyright (C) 2013 Red Hat + * see file 'COPYING' for use and warranty information + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ + +define(['./ipa', './jquery', './details', './entity'], function
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/14/2013 04:56 PM, Endi Sukma Dewata wrote:
On 2/14/2013 6:30 AM, Petr Vobornik wrote:
If they are mutually exclusive, they probably should be separated using
radio buttons like this:
PAC: ( ) None
(o) Type:
[x] MS-PAC
[ ] PAD
You missed one option: nothing selected. It can be solved by adding '(
) Inherited' radio.
I wouldn't have guessed that :) I agree we should add the 'Inherited'
option.
Anyway, this design seems more user friendly for more general audience
than mine so I will implement it. The only problem with it is that one
have to come with new label for each group and empty value - can't be
inferred from metadata.
Is there any issue adding new labels at this point? Worst case we could
hard code the label now and add a translation later.
It might be better to use a composite widget of radio buttons and
checkboxes so we can reuse the code. Probably the definition will look
something like this:
{
name: 'ipakrbauthzdata',
type: 'radio',
Not sure if it should be radio, more like something new.
Right, probably the current radio widget can't do this. So either we
improve the radio widget or create something new.
label: ...,
options: [
{
label: ...,
value: 'NONE'
},
{
label: ...,
type: 'checkboxes',
Do you expect to be there something different than checkboxes, or do you
want it to do it this way for possible future customization.
Ideally it should be generic enough to combine any widgets. This might
be a common scenario somewhere else:
Something: ( ) Option 1
( ) Option 2
(o) Other: [something else ]
This design has a flaw:
https://fedorahosted.org/freeipa/ticket/3404#comment:5
I implemented following design:
https://fedorahosted.org/freeipa/ticket/3404#comment:7
Patch attached (255-1).
I have a dilemma. I practically implemented the previous design (and
then I've found the flaw..). Patches attached as wip-fre... I wonder if
we can use it somehow or we should ditch it.
--
Petr Vobornik
From 854fb91a1fdc45f4d1ea45b9dc18f82b52a83e6d Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 7 Mar 2013 14:16:21 +0100
Subject: [PATCH] Added Web UI support for service PAC type option: NONE
ipakrbauthzdata accepts [null, 'NONE', 'MS-PAC, 'PAD']. 'MS-PAC' and 'PAD' can be combined together but they are mutually exclusive with 'NONE'. Hence, we can't use normal radio buttons nor checkboxes.
New multivalued radio widget was developed to solve the problem. One can define option with multiple values. Thus following layout is possible:
PAC type: ( ) Inherited
( ) None
(o) MS-PAC
( ) PAD
( ) MS-PAC + PAD
https://fedorahosted.org/freeipa/ticket/3404
---
install/ui/src/freeipa/field.js| 1 +
install/ui/src/freeipa/service.js | 20 +++--
install/ui/src/freeipa/widget.js | 44 ++
install/ui/test/data/ipa_init.json | 5 +
ipalib/plugins/internal.py | 5 +
5 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/install/ui/src/freeipa/field.js b/install/ui/src/freeipa/field.js
index f705ef7b899f502b717daa384c01763c96aea664..efa9568124c0ebc59fdfc93c55a2bb888a616f90 100644
--- a/install/ui/src/freeipa/field.js
+++ b/install/ui/src/freeipa/field.js
@@ -921,6 +921,7 @@ IPA.field_factories['entity_select'] = IPA.field;
IPA.field_factories['field'] = IPA.field;
IPA.field_factories['link'] = IPA.link_field;
IPA.field_factories['multivalued'] = IPA.multivalued_field;
+IPA.field_factories['multivalued_radio'] = IPA.radio_field;
IPA.field_factories['password'] = IPA.field;
IPA.field_factories['radio'] = IPA.radio_field;
IPA.field_factories['select'] = IPA.select_field;
diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js
index ecb8ce9b30518bd620dbd6ffab05342cff7a946b..8e21a9a225e2fcf5b34512c85bd471dd2cf71704 100644
--- a/install/ui/src/freeipa/service.js
+++ b/install/ui/src/freeipa/service.js
@@ -54,8 +54,24 @@ IPA.service.entity = function(spec) {
},
{
name: 'ipakrbauthzdata',
-type: 'checkboxes',
-options: IPA.create_options(['MS-PAC', 'PAD'])
+type: 'multivalued_radio',
+direction: 'vertical',
+options: IPA.create_options([
+{
+label: IPA.messages.krbauthzdata.inherited,
+value: ''
+},
+{
+label: IPA.messages.krbauthzdata.none,
+value: 'NONE'
+},
+
Re: [Freeipa-devel] [PATCHES] 116-119 Make LDAP schema retrieval optional
On 03/07/2013 01:43 PM, Jan Cholasta wrote: Hi, these patches add flags to LDAPClient and IPAdmin constructors which can be used to disable schema retrieval and decoding of attributes. This should make interacting with AD easier (see http://www.redhat.com/archives/freeipa-devel/2013-March/msg00076.html). Honza The first three patches look good, except a nitpick below. In the last patch, I don't see why you added back search_s. Is get_entries inadequate in some way? From 33142d7e0a8508a783e1a1b4a7a22525337ce54d Mon Sep 17 00:00:00 2001 From: Jan Cholastajchol...@redhat.com Date: Thu, 7 Mar 2013 10:50:57 +0100 Subject: [PATCH 1/4] Do not fail if schema cannot be retrieved from LDAP server. --- ipaserver/ipaldap.py | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py [...] +try: +self._schema = schema_cache.get_schema( +self.uri, self.conn, +force_update=self._force_schema_updates) +except: Don't use bare except. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 381 Preserve order of servers in ipa-client-install
On 03/07/2013 02:00 PM, Martin Kosek wrote: When multiple servers are passed via --server option, ipadiscovery module changed its order. Make sure that we preserve it. Also make sure that user is always warned when a tested server is not available as then the server will be excluded from the fixed server list. The message doesn't actually say that the server will be removed. It would be nice if it did. Otherwise, ACK. -- When working on this ticket I was thinking - do we make the right thing we deliberately remove a server from user-provided server list just because we cannot connect to it at the moment if discovery? It may just be temporarily down or something. Maybe we should preserve the original --server list in this case and use this list when writing krb5.conf or sssd.conf. Of course, for ipa-join or other active configuration commands we would have to use only the valid servers so that the we do not hit the server that is currently down. Martin Good point, this deserves a ticket. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab
On 03/06/2013 01:30 PM, Petr Spacek wrote: On 6.3.2013 13:04, Tomas Babej wrote: On 03/05/2013 02:10 PM, Petr Viktorin wrote: Thanks! The mechanism works, but see below. This is a RFE so it needs a design document. http://freeipa.org/page/V3/Client_install_using_keytab I added Security Considerations section with couple questions inside. Please add more details about un-enrolling process, pre-requirements and so on. I improved the design and added additional explanations to Security Considerations and elsewhere. Please have a look if anything needs more clarification. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Custom SSL certificates
Petr Viktorin wrote: Hi, I'm investigating https://fedorahosted.org/freeipa/ticket/3363 (fix --http_pkcs12 friends). I can't find documentation on these options, and from the code I can't figure out enough about how they are/were supposed to work. Is it the case that they were last used/tested before IPA started using Dogtag, and have rotted since then? Custom certs don't make sense to use if Dogtag is installed, right? So when they're provided we should not install the CA and Certmonger? If that's the case it would be easier (development- and testing-wise) to just remove self-signed CA in the same set of patches. I lack a little context about what was discussed on Tuesday but yeah, that sounds about right. We probably only want this if we don't have a real CA (including selfsign) behind IPA and I'm fine with dropping selfsign if we provide a no-CA option. These options come from V1 where many people didn't like our extremely limited selfsign CA and wanted to be able to provide their own certs. The bit-rot is a bit limited I think because a bunch of this code is still used, when installing replicas for example. The problem is that there are a lot of corner cases that we don't exercise because the certs we use are all of one predictable form. John added some PKCS#12 support to python-nss which should make some of this handling a lot cleaner. IIRC right now we handle everything using certutil which is very limiting. We also still tend to stomp on everything we see, and in V1 it was even more so, hence our overwriting NSS database files at will. As for how they should work... We need to stand up two SSL services, HTTP and LDAP. So we provide options for the use to present two PKCS#12 files (they can be the same) and the pins for those files. We need to: - load the PKCS#12 files into an NSS database - determine what the nickname of the server cert is so we can configure things - determine what the nickname of the CA is so we can publish *something* in /etc/ipa/ca.crt, and get the trust right I believe the assumption is that the same CA is used for both SSL and HTTP, though from a technical perspective this doesn't need to be true. And you're right, no certmonger here. There also needs to be some way to enforce that the user provide certs when running ipa-replica-prepare. What I'd propose is a new value for ra_plugin to indicate user-provided certs. I don't know if a new subclass would be desired or not, but it might be nice to provide a specific message rather than 'not implemented' for all cert commands. Be aware that a little bit of selfsign-specific code made it into the parent class in rabase.py. I suspect that pretty much all the NSS database code in __init__ can go away when selfsign is dropped. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 382 Do not hide idrange-add errors when adding trust
We catched all errors that could be raised by idrange-add command and
just raised an uncomprehensible ValidationError. This could hide
a real underlying problem and make the debugging harder.
We should rather just let the command raise the real error (which
will be already a PublicError).
https://fedorahosted.org/freeipa/ticket/3288
---
NOTE: I discussed this issue with tsunamie (ticket logger). He hit this error
in some early FreeIPA 3.0 release and he does not hit it any more. I also did
not reproduce it with current master so I am just sending a patch to allow
us/user to see the reason of the range error in case it occurs again.
Martin
From a944a83193869412207ba688626ca2a991b47b91 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 7 Mar 2013 15:34:25 +0100
Subject: [PATCH] Do not hide idrange-add errors when adding trust
We catched all errors that could be raised by idrange-add command and
just raised an uncomprehensible ValidationError. This could hide
a real underlying problem and make the debugging harder.
We should rather just let the command raise the real error (which
will be already a PublicError).
https://fedorahosted.org/freeipa/ticket/3288
---
ipalib/plugins/trust.py | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 2d772130e3853475f133b38ea96ccd757c796a74..7733d9b15c15aff361c63e829f4d9bfa9af98676 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -346,15 +346,12 @@ sides.
else:
base_id = 20 + (pysss_murmur.murmurhash3(dom_sid, len(dom_sid), 0xdeadbeef) % 1) * 20
-try:
-new_range = api.Command['idrange_add'](range_name,
- ipabaseid=base_id,
- ipaidrangesize=options['range_size'],
- ipabaserid=0,
- ipanttrusteddomainsid=dom_sid)
-except Exception, e:
-raise errors.ValidationError(name=_('ID range exists'),
- error = _('ID range already exists, must be added manually'))
+# Add new ID range
+api.Command['idrange_add'](range_name,
+ ipabaseid=base_id,
+ ipaidrangesize=options['range_size'],
+ ipabaserid=0,
+ ipanttrusteddomainsid=dom_sid)
def execute_ad(self, *keys, **options):
# Join domain using full credentials and with random trustdom
--
1.8.1.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab
Thanks! I just have two more very minor nitpicks. On 03/06/2013 01:04 PM, Tomas Babej wrote: On 03/05/2013 02:10 PM, Petr Viktorin wrote: Thanks! The mechanism works, but see below. This is a RFE so it needs a design document. http://freeipa.org/page/V3/Client_install_using_keytab Please also add the link to the commit message. I think you answered Petr²'s security questions adequately. Petr, note that this is a client-side change; if the keytab is compromised the attacker can do all this manually anyway. diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..a16a6b2d7cddbf7085b27c3835a4676919a8a15b 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -104,6 +104,8 @@ def parse_options(): [...] @@ -1691,8 +1693,12 @@ def install(options, env, fstore, statestore): except ipaclient.ntpconf.NTPConfigurationError: pass -if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master: -root_logger.error(One of password and principal are required.) +if options.unattended and ((options.password is None and +options.principal is None and +options.keytab is None and +options.prompt_password is False)\ +and not options.on_master): Please also remove the inner parentheses and the backslash. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab
On 03/07/2013 04:12 PM, Petr Viktorin wrote: Thanks! I just have two more very minor nitpicks. On 03/06/2013 01:04 PM, Tomas Babej wrote: On 03/05/2013 02:10 PM, Petr Viktorin wrote: Thanks! The mechanism works, but see below. This is a RFE so it needs a design document. http://freeipa.org/page/V3/Client_install_using_keytab Please also add the link to the commit message. I think you answered Petr²'s security questions adequately. Petr, note that this is a client-side change; if the keytab is compromised the attacker can do all this manually anyway. diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..a16a6b2d7cddbf7085b27c3835a4676919a8a15b 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -104,6 +104,8 @@ def parse_options(): [...] @@ -1691,8 +1693,12 @@ def install(options, env, fstore, statestore): except ipaclient.ntpconf.NTPConfigurationError: pass -if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master: -root_logger.error(One of password and principal are required.) +if options.unattended and ((options.password is None and +options.principal is None and +options.keytab is None and +options.prompt_password is False)\ +and not options.on_master): Please also remove the inner parentheses and the backslash. Both fixed, updated patch attached. Tomas From 1833de2b4c55f6342a80c0ca1c8e103c8bf3189e Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Tue, 26 Feb 2013 13:20:13 +0100 Subject: [PATCH] Add support for re-enrolling hosts using keytab A host that has been recreated and does not have its host entry disabled or removed, can be re-enrolled using a previously backed up keytab file. A new option --keytab has been added to ipa-client-install. This can be used to specify path to the keytab and can be used instead of -p or -w options. A new option -f has been added to ipa-join. It forces client to join even if the host entry already exits. A new certificate, ssh keys are generated, ipaUniqueID stays the same. Design page: http://freeipa.org/page/V3/Client_install_using_keytab https://fedorahosted.org/freeipa/ticket/3374 --- ipa-client/ipa-install/ipa-client-install | 40 +++ ipa-client/ipa-join.c | 14 +++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..bd458ed09856dfccd161b1dc96f4b1e0ec7f7e40 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -104,6 +104,8 @@ def parse_options(): help=principal to use to join the IPA realm), basic_group.add_option(-w, --password, dest=password, sensitive=True, help=password to join the IPA realm (assumes bulk password unless principal is also set)), +basic_group.add_option(-k, --keytab, dest=keytab, + help=path to backed up keytab from previous enrollment), basic_group.add_option(-W, dest=prompt_password, action=store_true, default=False, help=Prompt for a password to join the IPA realm), @@ -1691,8 +1693,12 @@ def install(options, env, fstore, statestore): except ipaclient.ntpconf.NTPConfigurationError: pass -if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master: -root_logger.error(One of password and principal are required.) +if options.unattended and (options.password is None and + options.principal is None and + options.keytab is None and + options.prompt_password is False and + not options.on_master): +root_logger.error(One of password / principal / keytab is required.) return CLIENT_INSTALL_ERROR if options.hostname: @@ -1910,8 +1916,10 @@ def install(options, env, fstore, statestore): ipaservices.backup_and_replace_hostname(fstore, statestore, options.hostname) if not options.unattended: -if options.principal is None and options.password is None and options.prompt_password is False: -options.principal = user_input(User authorized to enroll computers, allow_empty=False) +if (options.principal is None and options.password is None and +options.prompt_password is False and options.keytab is None): +
Re: [Freeipa-devel] [PATCHES] 116-119 Make LDAP schema retrieval optional
On 7.3.2013 14:53, Petr Viktorin wrote: On 03/07/2013 01:43 PM, Jan Cholasta wrote: Hi, these patches add flags to LDAPClient and IPAdmin constructors which can be used to disable schema retrieval and decoding of attributes. This should make interacting with AD easier (see http://www.redhat.com/archives/freeipa-devel/2013-March/msg00076.html). Honza The first three patches look good, except a nitpick below. In the last patch, I don't see why you added back search_s. Is get_entries inadequate in some way? Nope, it's just that __search_in_gc uses it. Fixed. From 33142d7e0a8508a783e1a1b4a7a22525337ce54d Mon Sep 17 00:00:00 2001 From: Jan Cholastajchol...@redhat.com Date: Thu, 7 Mar 2013 10:50:57 +0100 Subject: [PATCH 1/4] Do not fail if schema cannot be retrieved from LDAP server. --- ipaserver/ipaldap.py | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py [...] +try: +self._schema = schema_cache.get_schema( +self.uri, self.conn, +force_update=self._force_schema_updates) +except: Don't use bare except. Fixed. Updated patches attached. Honza -- Jan Cholasta From 598dc757e1cb65499a2e46f2be6a09f65a9d6d7f Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 7 Mar 2013 10:50:57 +0100 Subject: [PATCH 1/4] Do not fail if schema cannot be retrieved from LDAP server. --- ipaserver/ipaldap.py | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py index 4a46532..2928a67 100644 --- a/ipaserver/ipaldap.py +++ b/ipaserver/ipaldap.py @@ -270,13 +270,19 @@ class IPASimpleLDAPObject(object): self.log = log_mgr.get_logger(self) self.uri = uri self.conn = SimpleLDAPObject(uri) +self._has_schema = False self._schema = None self._force_schema_updates = force_schema_updates def _get_schema(self): -if self._schema is None: -self._schema = schema_cache.get_schema( -self.uri, self.conn, force_update=self._force_schema_updates) +if not self._has_schema: +try: +self._schema = schema_cache.get_schema( +self.uri, self.conn, +force_update=self._force_schema_updates) +except (errors.ExecutionError, IndexError): +pass +self._has_schema = True return self._schema schema = property(_get_schema, None, None, 'schema associated with this LDAP server') @@ -307,6 +313,7 @@ class IPASimpleLDAPObject(object): # logical operations that have the potential to cause a schema # change. +self._has_schema = False self._schema = None def get_syntax(self, attr): @@ -315,6 +322,9 @@ class IPASimpleLDAPObject(object): if syntax is not None: return syntax +if self.schema is None: +return None + # Try to lookup the syntax in the schema returned by the server obj = self.schema.get_obj(ldap.schema.AttributeType, attr) if obj is not None: -- 1.8.1 From 798abe2b369656f4ce99ea9f47a5c84fbe98775d Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 7 Mar 2013 10:52:57 +0100 Subject: [PATCH 2/4] Allow disabling LDAP schema retrieval in LDAPClient and IPAdmin. --- ipaserver/ipaldap.py | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py index 2928a67..0f153a5 100644 --- a/ipaserver/ipaldap.py +++ b/ipaserver/ipaldap.py @@ -255,7 +255,7 @@ class IPASimpleLDAPObject(object): 'originscope': DN_SYNTAX_OID, # DN }) -def __init__(self, uri, force_schema_updates): +def __init__(self, uri, force_schema_updates, no_schema=False): An internal LDAP connection object :param uri: The LDAP URI to connect to @@ -266,15 +266,19 @@ class IPASimpleLDAPObject(object): Generally, it should be true if the API context is 'installer' or 'updates', but it must be given explicitly since the API object is not always available +:param no_schema: If true, schema is never requested from the server. self.log = log_mgr.get_logger(self) self.uri = uri self.conn = SimpleLDAPObject(uri) +self._no_schema = no_schema self._has_schema = False self._schema = None self._force_schema_updates = force_schema_updates def _get_schema(self): +if self._no_schema: +return None if not self._has_schema: try: self._schema = schema_cache.get_schema( @@ -1649,7 +1653,7 @@ class IPAdmin(LDAPClient): def __init__(self, host='', port=389, cacert=None, debug=None,
Re: [Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab
On 03/07/2013 04:27 PM, Tomas Babej wrote: On 03/07/2013 04:12 PM, Petr Viktorin wrote: Thanks! I just have two more very minor nitpicks. On 03/06/2013 01:04 PM, Tomas Babej wrote: On 03/05/2013 02:10 PM, Petr Viktorin wrote: Thanks! The mechanism works, but see below. This is a RFE so it needs a design document. http://freeipa.org/page/V3/Client_install_using_keytab Please also add the link to the commit message. I think you answered Petr²'s security questions adequately. Petr, note that this is a client-side change; if the keytab is compromised the attacker can do all this manually anyway. diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..a16a6b2d7cddbf7085b27c3835a4676919a8a15b 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -104,6 +104,8 @@ def parse_options(): [...] @@ -1691,8 +1693,12 @@ def install(options, env, fstore, statestore): except ipaclient.ntpconf.NTPConfigurationError: pass -if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master: -root_logger.error(One of password and principal are required.) +if options.unattended and ((options.password is None and +options.principal is None and +options.keytab is None and +options.prompt_password is False)\ +and not options.on_master): Please also remove the inner parentheses and the backslash. Both fixed, updated patch attached. Tomas ACK, thanks! -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab
On Thu 07 Mar 2013 04:54:02 PM CET, Petr Viktorin wrote: On 03/07/2013 04:27 PM, Tomas Babej wrote: On 03/07/2013 04:12 PM, Petr Viktorin wrote: Thanks! I just have two more very minor nitpicks. On 03/06/2013 01:04 PM, Tomas Babej wrote: On 03/05/2013 02:10 PM, Petr Viktorin wrote: Thanks! The mechanism works, but see below. This is a RFE so it needs a design document. http://freeipa.org/page/V3/Client_install_using_keytab Please also add the link to the commit message. I think you answered Petr²'s security questions adequately. Petr, note that this is a client-side change; if the keytab is compromised the attacker can do all this manually anyway. diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..a16a6b2d7cddbf7085b27c3835a4676919a8a15b 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -104,6 +104,8 @@ def parse_options(): [...] @@ -1691,8 +1693,12 @@ def install(options, env, fstore, statestore): except ipaclient.ntpconf.NTPConfigurationError: pass -if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master: -root_logger.error(One of password and principal are required.) +if options.unattended and ((options.password is None and +options.principal is None and +options.keytab is None and +options.prompt_password is False)\ +and not options.on_master): Please also remove the inner parentheses and the backslash. Both fixed, updated patch attached. Tomas ACK, thanks! With your blessing, I moved the link to the design page from V3 proposals to V3 designs. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page
On 03/07/2013 02:19 PM, Ana Krivokapic wrote: On 03/07/2013 12:41 PM, Petr Vobornik wrote: On 03/06/2013 08:26 PM, Ana Krivokapic wrote: On 03/06/2013 10:40 AM, Petr Vobornik wrote: On 03/05/2013 05:52 PM, Ana Krivokapic wrote: On 02/27/2013 05:10 PM, Petr Vobornik wrote: On 02/27/2013 04:20 PM, Ana Krivokapic wrote: Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 8--- Almost there, as discussed in person: 1. following strings should be add to and obtained from internal.py plugin: title: 'Check DNS', message: 'Do you also want to perform DNS check?', ok_label: 'Check DNS', 2. the server plugin should report all dns resolution failures, not just the first one. Fixed, updated patch is attached. Works fine, but you forgot to update all related tests (s/domain/domains/): == FAIL: test_realmdomains[8]: realmdomains_mod: Try to replace list of realm domains with a list with an invalid domain doesnotexist.test -- Traceback (most recent call last): File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in runTest self.test(*self.arg) File /home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py, line 264, in lambda func = lambda: self.check(nice, **test) File /home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py, line 278, in check self.check_exception(nice, cmd, args, options, expected) File /home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py, line 304, in check_exception assert_deepequal(expected.strerror, e.strerror) File /home/pvoborni/dev/freeipa/tests/util.py, line 343, in assert_deepequal VALUE % (doc, expected, got, stack) AssertionError: assert_deepequal: expected != got. expected = uinvalid 'domain': no SOA or NS records found for domains: doesnotexist.test got = uinvalid 'domain': no SOA or NS records found for domain doesnotexist.test path = () -- -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer password migration
On 6.3.2013 16:29, Petr Viktorin wrote: Hello, These patches move ipaldap to ipapython, and make the client installer use it. Also password migration web-app is made to use ipaldap; they both called a shared a utility function that is converted to use ipaldap. This should fix https://fedorahosted.org/freeipa/ticket/3446 (freeipa-client-install KeyError in 'namingcontexts') and similar errors. https://fedorahosted.org/freeipa/ticket/3487 Patch 191: The patch is missing the ipapython/ipaldap.py file. I think it should go into ipalib instead of ipapython. rant It doesn't make sense to keep ipapython and ipalib separate if they depend on each other. We should either merge them or clean up the mess by removing ipalib imports from ipapython. I'm not saying we should do it now, just please don't add new modules to ipapython which import from ipalib. /rant Also I am not very fond of the ipa prefix in ipaldap. The module lives in the namespace of our own package, so there's no need for it to have such a prefix, is there? Patch 193: +scope=conn.SCOPE_BASE, +filter='objectclass=pkiCA', +attrs_list=[ca_cert_attr], Can we use a proper filter here please? +:param conn: Bound LDAPConnection that will be used for searching LDAPClient Patch 194: -ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, True) and -lh.set_option(ldap.OPT_X_TLS_DEMAND, True) Is removing these options safe? Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 116-119 Make LDAP schema retrieval optional
On 03/07/2013 04:33 PM, Jan Cholasta wrote: On 7.3.2013 14:53, Petr Viktorin wrote: On 03/07/2013 01:43 PM, Jan Cholasta wrote: Hi, these patches add flags to LDAPClient and IPAdmin constructors which can be used to disable schema retrieval and decoding of attributes. This should make interacting with AD easier (see http://www.redhat.com/archives/freeipa-devel/2013-March/msg00076.html). Honza [...] Updated patches attached. Honza In LDAPEntry.__setitem__, schema.get_obj is used without checking if the schema is None. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer password migration
Jan Cholasta wrote: On 6.3.2013 16:29, Petr Viktorin wrote: Hello, These patches move ipaldap to ipapython, and make the client installer use it. Also password migration web-app is made to use ipaldap; they both called a shared a utility function that is converted to use ipaldap. This should fix https://fedorahosted.org/freeipa/ticket/3446 (freeipa-client-install KeyError in 'namingcontexts') and similar errors. https://fedorahosted.org/freeipa/ticket/3487 Patch 191: The patch is missing the ipapython/ipaldap.py file. I think it should go into ipalib instead of ipapython. rant It doesn't make sense to keep ipapython and ipalib separate if they depend on each other. We should either merge them or clean up the mess by removing ipalib imports from ipapython. I'm not saying we should do it now, just please don't add new modules to ipapython which import from ipalib. /rant There have been platforms in the past that we do not ship the framework on (e.g. RHEL 5). ipapython is supposed to be platform agnostic, though it clearly isn't, to allow a bridge for those systems that can't or won't have the framework. So I think removing the imports from ipapython is the way to go. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 116-119 Make LDAP schema retrieval optional
On 7.3.2013 17:59, Petr Viktorin wrote:
On 03/07/2013 04:33 PM, Jan Cholasta wrote:
On 7.3.2013 14:53, Petr Viktorin wrote:
On 03/07/2013 01:43 PM, Jan Cholasta wrote:
Hi,
these patches add flags to LDAPClient and IPAdmin constructors which
can
be used to disable schema retrieval and decoding of attributes. This
should make interacting with AD easier (see
http://www.redhat.com/archives/freeipa-devel/2013-March/msg00076.html).
Honza
[...]
Updated patches attached.
Honza
In LDAPEntry.__setitem__, schema.get_obj is used without checking if the
schema is None.
I knew I forgot something! Thanks. Fixed.
Updated patches attached.
Honza
--
Jan Cholasta
From 411ba6d422b39e27deab63acfc127573e7db1fb2 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 7 Mar 2013 10:50:57 +0100
Subject: [PATCH 1/4] Do not fail if schema cannot be retrieved from LDAP
server.
---
ipaserver/ipaldap.py | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 4a46532..d9f91d5 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -270,13 +270,19 @@ class IPASimpleLDAPObject(object):
self.log = log_mgr.get_logger(self)
self.uri = uri
self.conn = SimpleLDAPObject(uri)
+self._has_schema = False
self._schema = None
self._force_schema_updates = force_schema_updates
def _get_schema(self):
-if self._schema is None:
-self._schema = schema_cache.get_schema(
-self.uri, self.conn, force_update=self._force_schema_updates)
+if not self._has_schema:
+try:
+self._schema = schema_cache.get_schema(
+self.uri, self.conn,
+force_update=self._force_schema_updates)
+except (errors.ExecutionError, IndexError):
+pass
+self._has_schema = True
return self._schema
schema = property(_get_schema, None, None, 'schema associated with this LDAP server')
@@ -307,6 +313,7 @@ class IPASimpleLDAPObject(object):
# logical operations that have the potential to cause a schema
# change.
+self._has_schema = False
self._schema = None
def get_syntax(self, attr):
@@ -315,6 +322,9 @@ class IPASimpleLDAPObject(object):
if syntax is not None:
return syntax
+if self.schema is None:
+return None
+
# Try to lookup the syntax in the schema returned by the server
obj = self.schema.get_obj(ldap.schema.AttributeType, attr)
if obj is not None:
@@ -708,12 +718,8 @@ class LDAPEntry(dict):
else:
self._names[name] = name
-try:
-schema = self._conn.schema
-except:
-pass
-else:
-attrtype = schema.get_obj(ldap.schema.AttributeType,
+if self._conn.schema is not None:
+attrtype = self._conn.schema.get_obj(ldap.schema.AttributeType,
name.encode('utf-8'))
if attrtype is not None:
for altname in attrtype.names:
--
1.8.1
From f6c25d9d8abe318745ddb5726a14f4aa9956b63a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 7 Mar 2013 10:52:57 +0100
Subject: [PATCH 2/4] Allow disabling LDAP schema retrieval in LDAPClient and
IPAdmin.
---
ipaserver/ipaldap.py | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index d9f91d5..c814f57 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -255,7 +255,7 @@ class IPASimpleLDAPObject(object):
'originscope': DN_SYNTAX_OID, # DN
})
-def __init__(self, uri, force_schema_updates):
+def __init__(self, uri, force_schema_updates, no_schema=False):
An internal LDAP connection object
:param uri: The LDAP URI to connect to
@@ -266,15 +266,19 @@ class IPASimpleLDAPObject(object):
Generally, it should be true if the API context is 'installer' or
'updates', but it must be given explicitly since the API object
is not always available
+:param no_schema: If true, schema is never requested from the server.
self.log = log_mgr.get_logger(self)
self.uri = uri
self.conn = SimpleLDAPObject(uri)
+self._no_schema = no_schema
self._has_schema = False
self._schema = None
self._force_schema_updates = force_schema_updates
def _get_schema(self):
+if self._no_schema:
+return None
if not self._has_schema:
try:
self._schema = schema_cache.get_schema(
@@ -1645,7 +1649,7 @@ class IPAdmin(LDAPClient):
def __init__(self, host='', port=389, cacert=None, debug=None, ldapi=False,
Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer password migration
On 03/07/2013 06:01 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 6.3.2013 16:29, Petr Viktorin wrote: Hello, These patches move ipaldap to ipapython, and make the client installer use it. Also password migration web-app is made to use ipaldap; they both called a shared a utility function that is converted to use ipaldap. This should fix https://fedorahosted.org/freeipa/ticket/3446 (freeipa-client-install KeyError in 'namingcontexts') and similar errors. https://fedorahosted.org/freeipa/ticket/3487 Patch 191: The patch is missing the ipapython/ipaldap.py file. It's there, it's just copied from ipaserver/ipaldap.py with a small change at the bottom. I'll respond to the other comments tomorrow. I think it should go into ipalib instead of ipapython. rant It doesn't make sense to keep ipapython and ipalib separate if they depend on each other. We should either merge them or clean up the mess by removing ipalib imports from ipapython. I'm not saying we should do it now, just please don't add new modules to ipapython which import from ipalib. /rant There have been platforms in the past that we do not ship the framework on (e.g. RHEL 5). ipapython is supposed to be platform agnostic, though it clearly isn't, to allow a bridge for those systems that can't or won't have the framework. So I think removing the imports from ipapython is the way to go. Then we need to move errors and text to ipapython, those need to be imported pretty much everywhere. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, Mar 06, 2013 at 05:33:43PM +0100, Sumit Bose wrote: On Wed, Mar 06, 2013 at 08:51:47AM -0500, Simo Sorce wrote: On Wed, 2013-03-06 at 14:49 +0100, Martin Kosek wrote: On 03/06/2013 10:41 AM, Sumit Bose wrote: On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: On 03/04/2013 04:22 PM, Sumit Bose wrote: On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: On 03/01/2013 09:20 AM, Sumit Bose wrote: On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: On 02/28/2013 03:28 PM, Simo Sorce wrote: On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: On 02/28/2013 12:42 PM, Sumit Bose wrote: On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: On 02/27/2013 06:48 PM, Sumit Bose wrote: Hi Sumit, This looks like a good idea and would prevent the magic default PAC type, yes. Though I would not add this service-specific setting to global IPA config object. I would rather like to see that in the service tree, for example as a configuration option of the service root which could be controlled with serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD # ipa serviceconfig-show Default PAC Map: nfs:NONE, cifs:PAD Are you thinking of having this in addition to the for-all-services default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't like the first case because then three different objects needs to be consulted to find out which is the right type. This wouldn't be an issue for the plugin, but I think it is hard for the user/admin to follow. Hm, you are right. If the current defaults shall be dropped I think this is a major change because it will require changes in the current CLI and WebUI which will be visible to the users. I'm not against this change, I'm just wondering if it is worth the effort for the next release? Maybe an argument to keep this is in global default is that the settings are used for the host/*.* services as well which are in a different sub-tree of the cn=accounts container. Additionally in future we might want apply those setting to the user TGTs as well? Yeah, that was actually my point. That we are mixing service-specific PAC rules to the global setting. Which may be shared with host/*.* principals and user principals. This automatic PAC rules may require some designing so that is is generally usable. I think putting everything in the general config is more understandable and discoverable. These per-service defaults are basically exceptions to the general rule so it make sense to keep everything together. Simo. Ok, if these are really just an exceptions to the general rule (and there will not be too many of them), I think we can leave it in config entry. But if we expect to have exceptions for other types of entries (hosts, users), I think we should rather use something like service:nfs:NONE do distinguish this exception. Question is, do we want to implement the interface and processing for that in current Sumit's patches or do we use that is they are? I would like to update the patches so that they can handle the service:TYPE style entry and replace the current update code with just adding nfs:NONE to the global options. I will update the design page accordingly, too. Ok. If the update procedure shrinks just to adding service:nfs:NONE then it'd be great. If we need to distinguish between service principals and user principals I would prefer rather use a special keyword for upns service: is redundant and I do not want here to be able to say upn:martin:NONE because per principal options are available on the principal object. I actually really do not see the need for changing the default just for user principals. If we are worried that one day we might want to really have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we might add upn:NONE and the lack of / will tell us this is not a service named upn/foo.bar.baz but rather it means user principal names. However I do not see us ever really needing upn:NONE I would prefer if the enhancements needed for the CLI and WebUI can be covered by other/new tickets, but I'm happy to add the needed information to the design page too. bye, Sumit I am OK with adding the interface for this special exception later. In that case, a
Re: [Freeipa-devel] [PATCH] 0007 Web UI: Realm Domains page
On 03/07/2013 05:32 PM, Petr Vobornik wrote: On 03/07/2013 02:19 PM, Ana Krivokapic wrote: On 03/07/2013 12:41 PM, Petr Vobornik wrote: On 03/06/2013 08:26 PM, Ana Krivokapic wrote: On 03/06/2013 10:40 AM, Petr Vobornik wrote: On 03/05/2013 05:52 PM, Ana Krivokapic wrote: On 02/27/2013 05:10 PM, Petr Vobornik wrote: On 02/27/2013 04:20 PM, Ana Krivokapic wrote: Add support for Realm Domains to web UI. https://fedorahosted.org/freeipa/ticket/3407 8--- Almost there, as discussed in person: 1. following strings should be add to and obtained from internal.py plugin: title: 'Check DNS', message: 'Do you also want to perform DNS check?', ok_label: 'Check DNS', 2. the server plugin should report all dns resolution failures, not just the first one. Fixed, updated patch is attached. Works fine, but you forgot to update all related tests (s/domain/domains/): == FAIL: test_realmdomains[8]: realmdomains_mod: Try to replace list of realm domains with a list with an invalid domain doesnotexist.test -- Traceback (most recent call last): File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in runTest self.test(*self.arg) File /home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py, line 264, in lambda func = lambda: self.check(nice, **test) File /home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py, line 278, in check self.check_exception(nice, cmd, args, options, expected) File /home/pvoborni/dev/freeipa/tests/test_xmlrpc/xmlrpc_test.py, line 304, in check_exception assert_deepequal(expected.strerror, e.strerror) File /home/pvoborni/dev/freeipa/tests/util.py, line 343, in assert_deepequal VALUE % (doc, expected, got, stack) AssertionError: assert_deepequal: expected != got. expected = uinvalid 'domain': no SOA or NS records found for domains: doesnotexist.test got = uinvalid 'domain': no SOA or NS records found for domain doesnotexist.test path = () -- False alarm. It was an error on my side. ACK -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer password migration
Petr Viktorin wrote: On 03/07/2013 06:01 PM, Rob Crittenden wrote: Jan Cholasta wrote: On 6.3.2013 16:29, Petr Viktorin wrote: Hello, These patches move ipaldap to ipapython, and make the client installer use it. Also password migration web-app is made to use ipaldap; they both called a shared a utility function that is converted to use ipaldap. This should fix https://fedorahosted.org/freeipa/ticket/3446 (freeipa-client-install KeyError in 'namingcontexts') and similar errors. https://fedorahosted.org/freeipa/ticket/3487 Patch 191: The patch is missing the ipapython/ipaldap.py file. It's there, it's just copied from ipaserver/ipaldap.py with a small change at the bottom. I'll respond to the other comments tomorrow. I think it should go into ipalib instead of ipapython. rant It doesn't make sense to keep ipapython and ipalib separate if they depend on each other. We should either merge them or clean up the mess by removing ipalib imports from ipapython. I'm not saying we should do it now, just please don't add new modules to ipapython which import from ipalib. /rant There have been platforms in the past that we do not ship the framework on (e.g. RHEL 5). ipapython is supposed to be platform agnostic, though it clearly isn't, to allow a bridge for those systems that can't or won't have the framework. So I think removing the imports from ipapython is the way to go. Then we need to move errors and text to ipapython, those need to be imported pretty much everywhere. Yup, that too is a bit of a violation, at least for errors as we really want to publish those as part of the framework. This is why I didn't raise a fuss when we starting doing minor mixing of things (and I really felt the pain when trying to do certain things in RHEL 5). The client is now using more and more from the framework (SSH uploads, for example). So maybe it is time to revisit how we package things. If we decide to go whole hog and merge ipapython and ipalib I think there'd be no going back. We'd have to carefully consider all the consequences. I tend to be rather conservative in this regard, but perhaps I'm just living in the past. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1088 Recover DNA ranges when deleting a master
Petr Viktorin wrote:
On 03/06/2013 09:52 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
[...]
On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric
Assignment Plugin,cn=plugins,cn=config is added before the entry itself.
I didn't test everything as I didn't get the access.
It shouldn't make a difference. What isn't working?
I get a CRITICAL message in the log:
add aci:
(targetattr=dnaNextRange || dnaNextValue ||
dnaMaxValue)(version 3.0;acl permission:Modify DNA Range;allow (write)
groupdn = ldap:///cn=Modify DNA
Range,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com;)
modifying entry cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config
2013-03-07T11:01:48Z DEBUG stderr=ldap_initialize(
ldap://vm-081.idm.lab.eng.brq.redhat.com:389/??base )
ldap_modify: No such object (32)
2013-03-07T11:01:48Z CRITICAL Failed to load replica-acis.ldif: Command
'/usr/bin/ldapmodify -v -f /tmp/tmpT55upM -H
ldap://vm-081.idm.lab.eng.brq.redhat.com:389 -x -D cn=Directory Manager
-y /tmp/tmplFeere' returned non-zero exit status 32
Gotcha. I moved where the replica acis are loaded.
This particular ACI is added later by the updater. But, after failing
here the rest of the file is skipped, and the subsequent ACIs aren't
added in updates. Not being able to run CLEANALLRUV prevents cleanly
deleting a replica.
[...]
[...]
+try:
+repl = replication.ReplicationManager(realm, hostname,
dirman_passwd)
+except Exception, e:
+sys.exit(Connection failed: %s %
ipautil.convert_ldap_error(e))
ipaldap should convert LDAP errors to IPA ones, there's no need to call
convert_ldap_error. Same in other places.
It does in some but it isn't consistent. I removed my calls though.
$ ipa-replica-manage dnarange-show -p badpassword
Connection failed: {'desc': 'Invalid credentials'}
That's a bug. My patch 0194 should fix this, I'll check after it's
merged. Anyway it's not a show stopper now.
[...]
+failed = 0
+for ent in entries:
This loops more than necessary and is somewhat hard to follow. Consider
using for-else here:
for ...:
...
if okay:
break
else:
raise error
I simplified things a bit but a for/else won't work here as we need to
check all ranges all the time. It is perfectly fine to not fit into a
range, as long as it fits into SOME range.
Well, that's how for's (not if's) else clause works -- it's executed
after all the looping's done if you didn't `break` out.
http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops
Maybe I'm just used to it and it's too esoteric to the average reader,
though.
Thanks for the vote of confidence. Like I said, I wanted it to check all
the ranges. A for/else quits on the break, which I guess is really
probably ok assuming we trust that nothing else is going to stuff bad
ranges in. I can go ahead and make the change.
[...]
Ok, I'll drop this since it doesn't affect things with the new LDAP
backend.
Please do, you left it in by mistake.
Yeah, there it is sitting unsquashed in my tree :-(
I also added one change related to the LDAP core changes. In the past if
you did not have a ticket it would prompt for DM password. This was
broken after the updates. I added an additional except in
test_connection().
This should also be fixed soon in ipaldap. Thanks for putting up with
the changes.
So should I drop this in my patch then? I don't really like having to
import ldap.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PROPOSAL] Kerberos flags
Based on a comment from Sumit in ticket https://fedorahosted.org/freeipa/ticket/3329 here is a bare outline of how one might do it: http://freeipa.org/page/V3/Kerberos_Flags There is a bit of hand waving going on around how the flags are actually set inside the KDB plugin since I'm not at all familiar with that code but I don't expect it to be too big a deal. I'm not necessarily volunteering to do this work, just trying to keep the ball moving forward. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab
Petr Viktorin wrote: On 03/07/2013 04:27 PM, Tomas Babej wrote: On 03/07/2013 04:12 PM, Petr Viktorin wrote: Thanks! I just have two more very minor nitpicks. On 03/06/2013 01:04 PM, Tomas Babej wrote: On 03/05/2013 02:10 PM, Petr Viktorin wrote: Thanks! The mechanism works, but see below. This is a RFE so it needs a design document. http://freeipa.org/page/V3/Client_install_using_keytab Please also add the link to the commit message. I think you answered Petr²'s security questions adequately. Petr, note that this is a client-side change; if the keytab is compromised the attacker can do all this manually anyway. diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..a16a6b2d7cddbf7085b27c3835a4676919a8a15b 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -104,6 +104,8 @@ def parse_options(): [...] @@ -1691,8 +1693,12 @@ def install(options, env, fstore, statestore): except ipaclient.ntpconf.NTPConfigurationError: pass -if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master: -root_logger.error(One of password and principal are required.) +if options.unattended and ((options.password is None and +options.principal is None and +options.keytab is None and +options.prompt_password is False)\ +and not options.on_master): Please also remove the inner parentheses and the backslash. Both fixed, updated patch attached. Tomas ACK, thanks! This needs related man page updates before we can push it. Can you update the design to specifically include that the old certificate needs to be revoked, not just that a new certificate be issued (sort of implied, and it worked in my testing)? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0038] Perform secondary rid range overlap check for local ranges
Tomas Babej wrote: Hi, Any of the following checks: - overlap between primary RID range and secondary RID range - overlap between secondary RID range and secondary RID range is performed now only if both of the ranges involved are local domain ranges. https://fedorahosted.org/freeipa/ticket/3391 Tomas So I assume in your reproduction steps the secondary range for both is 0 hence the overlap? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 376-377 Use tkey-gssapi-keytab in named.conf
Martin Kosek wrote: Remove obsolete BIND GSSAPI configuration options tkey-gssapi-credential and tkey-domain and replace them with tkey-gssapi-keytab which avoids unnecessary Kerberos checks on BIND startup and can cause issues when KDC is not available. Both new and current IPA installations are updated. https://fedorahosted.org/freeipa/ticket/3429 Still reviewing this but I noticed that after upgrading my 3.1.99 server pre-patch to with with-patch version the connections argument in named.conf got set to 4 (courtesy of ipa-upgradeconfig). Should we be setting that to 4 during the initial install too? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
