Re: [Freeipa-devel] [PATCHES] 98-101 Preserve case of LDAP attribute names
On 5.2.2013 15:45, Petr Viktorin wrote:
On 02/05/2013 01:38 PM, Jan Cholasta wrote:
On 4.2.2013 15:49, Petr Viktorin wrote:
[...]
I see one of the changes is using has_key instead of `in` for a CIDict.
Given that dict.has_key() is deprecated, I think a better solution would
be to add __contains__ to CIDict. Is there a reason against that?
I think a separate patch with this and other changes to make CIDict more
like dict would be better.
OK, I'll make a patch.
[...]
Updated patches attached.
The changes look good but I can no longer apply the patches. Can you
please rebase them?
Sure.
--
Jan Cholasta
From 5ca15b97a54ae5afc85bfbed36a386b79862c6e0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 31 Jan 2013 11:19:13 +0100
Subject: [PATCH 1/4] Use the dn attribute of LDAPEntry to set/get DNs of
entries.
Convert all code that uses the 'dn' key of LDAPEntry for this to use the dn
attribute instead.
---
install/tools/ipa-compliance | 10 +++
install/tools/ipa-replica-install | 2 +-
ipalib/plugins/automember.py | 9 --
ipalib/plugins/baseldap.py| 58 +++
ipalib/plugins/krbtpolicy.py | 6 ++--
ipalib/plugins/permission.py | 6 ++--
ipalib/plugins/sudorule.py| 8 --
ipalib/plugins/trust.py | 2 +-
ipalib/plugins/user.py| 9 ++
ipaserver/ipaldap.py | 4 +--
ipaserver/plugins/ldap2.py| 2 --
11 files changed, 73 insertions(+), 43 deletions(-)
diff --git a/install/tools/ipa-compliance b/install/tools/ipa-compliance
index c82e415..9b34350 100644
--- a/install/tools/ipa-compliance
+++ b/install/tools/ipa-compliance
@@ -116,7 +116,7 @@ def check_compliance(tmpdir, debug=False):
hostcount = 0
# Get the hosts first
try:
-(entries, truncated) = conn.find_entries('(krblastpwdchange=*)', ['dn'],
+(entries, truncated) = conn.find_entries('(krblastpwdchange=*)', [],
DN(api.env.container_host, api.env.basedn),
conn.SCOPE_ONELEVEL,
size_limit = -1)
@@ -136,10 +136,10 @@ def check_compliance(tmpdir, debug=False):
available = 0
try:
(entries, truncated) = conn.find_entries('(objectclass=ipaentitlement)',
-['dn', 'userCertificate'],
-DN(api.env.container_entitlements, api.env.basedn),
-conn.SCOPE_ONELEVEL,
-size_limit = -1)
+['userCertificate'],
+DN(api.env.container_entitlements, api.env.basedn),
+conn.SCOPE_ONELEVEL,
+size_limit = -1)
for entry in entries:
(dn, attrs) = entry
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 13c3260..846122d 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -572,7 +572,7 @@ def main():
config.dirman_password)
found = False
try:
-entry = conn.find_entries(u'fqdn=%s' % host, ['dn', 'fqdn'], DN(api.env.container_host, api.env.basedn))
+entry = conn.find_entries(u'fqdn=%s' % host, ['fqdn'], DN(api.env.container_host, api.env.basedn))
print The host %s already exists on the master server.\nYou should remove it before proceeding: % host
print %% ipa host-del %s % host
found = True
diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index af39f6a..520f8a0 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -316,10 +316,12 @@ class automember_add_condition(LDAPUpdate):
except errors.NotFound:
failed['failed'][attr].append(regex)
+entry_attrs = entry_to_dict(entry_attrs, **options)
+
# Set failed and completed to they can be harvested in the execute super
setattr(context, 'failed', failed)
setattr(context, 'completed', completed)
-setattr(context, 'entry_attrs', dict(entry_attrs))
+setattr(context, 'entry_attrs', entry_attrs)
# Make sure to returned the failed results if there is nothing to remove
if completed == 0:
@@ -406,10 +408,13 @@ class automember_remove_condition(LDAPUpdate):
else:
failed['failed'][attr].append(regex)
entry_attrs[attr] = old_entry
+
+entry_attrs = entry_to_dict(entry_attrs, **options)
+
# Set failed and completed to they can be harvested in the execute super
setattr(context, 'failed', failed)
setattr(context, 'completed', completed)
-setattr(context, 'entry_attrs', dict(entry_attrs))
+setattr(context, 'entry_attrs', entry_attrs)
# Make sure to returned the failed results if there is nothing to remove
if completed == 0:
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 44751e1..74e2384 100644
Re: [Freeipa-devel] [PATCH] 0176 Don't add another nsDS5ReplicaId on updates if one already exists
On 02/05/2013 02:38 PM, Martin Kosek wrote: On 02/05/2013 01:10 PM, Petr Viktorin wrote: Hello, In the LDAP refactor I made getting single-valued attributes[1] ensure that there is really only one value. Previously we just used the first one. This exposed a bug in replica installation: see https://fedorahosted.org/freeipa/ticket/3394. Fix attached. It seems that 389 happened to return the larger value first and the 3 second, so we didn't notice the problem. Still, should be fixed in all branches. Hopefully all regressions caused by the refactor are of this kind :) [1] For those who are following: getEntry() in old code, single_value() after my patches ACK, thanks for catching this. Martin Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 362 Add LDAP server fallback to client installer
On 02/06/2013 04:12 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/05/2013 05:57 PM, Rob Crittenden wrote: Martin Kosek wrote: On 02/04/2013 05:59 PM, Rob Crittenden wrote: Martin Kosek wrote: When ipa-client-install is run without --server option, it tries to search SRV records for IPA/LDAP server hostname, but it returns only the first record found and when the LDAP server on that hostname is not available, the whole client installation fails. Get all LDAP SRV records instead and fallback to next hostname when the current one is not available. https://fedorahosted.org/freeipa/ticket/3388 I worked on the same ticket, unfortunately, but I didn't mark it as assigned which caused some duplicate effort. Sorry about that. I came up with a very similar solution but took it a bit further. This expands the treatment of the discovered servers as a list of servers rather than a single value. I do a bit more aggressive testing of all servers returned and remove any from the list that are not IPA servers. A server not responding is left in the configured list. rob 1) (minor) If I connected IPA host to other IPA server before next enrollment, there will be outdated /etc/ipa/ca.crt. This will cause all tries to connect to IPA server to fail with TLS error, but without giving any clue to user... Yeah, this can be a problem but I'm going to leave things as-is for now. I believe we have a separate ticket to clean this up. I don't want to combine too many things into this patch, it is disruptive enough. # ipa-client-install Provide your IPA server name (ex: ipa.example.com): He would need to reach out to the log to find this line: LDAP Error: Connect error: TLS error -8054:You are attempting to import a cert with the same issuer/serial as an existing cert, but that is not the same cert. I am thinking if we should not expose some LDAP errors after all? To give some clue to user... Yeah, I switched the LDAP error unhandled exception back from debug to error. 2) (minor) While we are touching these errors I would also fix a typo there :-) ... if isinstance(err, ldap.INAPPROPRIATE_AUTH): root_logger.debug(LDAP Error: Anonymous acces not allowed) return [NO_ACCESS_TO_LDAP] ^ ... Heh, ok. 3) (Regression) You neither set ds.server nor add server to valid_servers when anonymous access is not enabled. The installer then does not try to connect to this server even though the installation would succeed: ... elif ldapret[0] == NO_ACCESS_TO_LDAP or ldapret[0] == NO_TLS_LDAP: ldapaccess = False ... Good point. The handling for this is done a bit later so I need to defer a little processing but it should work now. 4) (Regression) Client will now report success in discovery even when the server is down: # ipa-client-install Unable to verify that vm-037.idm.lab.bos.redhat.com (realm IDM.LAB.BOS.REDHAT.COM) is an IPA server Discovery was successful! Hostname: vm-148.idm.lab.bos.redhat.com Realm: IDM.LAB.BOS.REDHAT.COM DNS Domain: idm.lab.bos.redhat.com IPA Server: vm-037.idm.lab.bos.redhat.com BaseDN: dc=idm,dc=lab,dc=bos,dc=redhat,dc=com Continue to configure the system with these values? [no]: y User authorized to enroll computers: admin Synchronizing time with KDC... Password for ad...@idm.lab.bos.redhat.com: Kerberos authentication failed kinit: Generic error (see e-text) while getting initial credentials Please make sure the following ports are opened in the firewall settings: TCP: 80, 88, 389 UDP: 88 (at least one of TCP/UDP ports 88 has to be open) Also note that following ports are necessary for ipa-client working properly after enrollment: TCP: 464 UDP: 464, 123 (if NTP enabled) Installation failed. Rolling back changes. IPA client is not configured on this system. LDAP on vm-037 in this case is down. I think this would cause a regression too, because previously we simply reported that no discovered server is available and let user enter the server manually. Fixed. IMO we are trying to be too smart when processing the (discovered) servers. Why do we need to process and verify _all_ discovered servers even when the list is not written anywhere in case of SRV lookup? I think it causes unnecessary traffic on network - we should connect to the first server available. That's a good point. Now we except on the first discovered server. I think for user-provided servers we still want to verify them all since they will be hardcoded in a config file. 5) In ipa-client-automount: +# Now confirm that our server is an IPA server. Stop checking on the first +# success so we know we have at least one good one. +for server in servers: +root_logger.debug(Verifying that %s is an IPA server % server) +ldapret =
Re: [Freeipa-devel] [RFE] Configurable SID Blacklists
On 02/05/2013 05:25 PM, Martin Kosek wrote: Hello, below is a design page for ticket https://fedorahosted.org/freeipa/ticket/3289: http://www.freeipa.org/page/V3/Configurable_SID_Blacklists There is one question in the text. Martin I had to refactor ipa-kdb Changes section as I realized that ipadb_mspac structure is global and we want these list per-trust. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES 0031-0032] Improve HBAC rule handling in selinuxusermap-add/mod/find
Hi,
this pair of patches improves HBAC rule handling in selinuxusermap commands.
Patch 0031 deals with:
https://fedorahosted.org/freeipa/ticket/3349
Patch 0032 takes care of:
https://fedorahosted.org/freeipa/ticket/3348
and is to be applied on top of Patch 0031.
See commit messages for detailed info.
Tomas
From aa171a4e3bc5295cdf332215e1b2477c7512180a Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 6 Feb 2013 07:04:03 -0500
Subject: [PATCH 31/32] Improve HBAC rule handling in
selinuxusermap-add/mod/find
Pre-patch handling of HBAC rules in selinuxusermap commands tried
to allow the user to specify the HBAC rule using the rule's name.
However, this approach created obstacles to modifying the HBAC
rule's DN directly in LDAP (attribute 'seealso') using --setattr
or --addattr options.
This patchs removes the option for setting 'seealso' via CLI
option(named 'hbacrule') and creates a new virtual attribute
(named 'hbacrule'). If 'hbacrule' is set, rule's DN is resolved
and set to 'seealso' attribute. This allows the user to specify
the rule comfortably using its name as well as modify the entry
directly using --setattr/--addattr.
https://fedorahosted.org/freeipa/ticket/3349
---
API.txt | 15 +-
ipalib/plugins/hbacrule.py | 2 +-
ipalib/plugins/selinuxusermap.py| 81 +++---
tests/test_xmlrpc/test_selinuxusermap_plugin.py | 204 +++-
4 files changed, 240 insertions(+), 62 deletions(-)
diff --git a/API.txt b/API.txt
index 8fbfe6f5d8da44e991b8d1a36725fc6ace1f0616..ee76871d15ba30bff18e5d16858ede615b046a96 100644
--- a/API.txt
+++ b/API.txt
@@ -2607,16 +2607,17 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
command: selinuxusermap_add
-args: 1,11,3
+args: 1,12,3
arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
+option: Str('hbacrule', attribute=False, cli_name='hbacrule', multivalue=False, required=False)
option: StrEnum('hostcategory', attribute=True, cli_name='hostcat', multivalue=False, required=False, values=(u'all',))
option: Bool('ipaenabledflag', attribute=True, cli_name='ipaenabledflag', multivalue=False, required=False)
option: Str('ipaselinuxuser', attribute=True, cli_name='selinuxuser', multivalue=False, required=True)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Str('seealso', attribute=True, cli_name='hbacrule', multivalue=False, required=False)
+option: DNParam('seealso', attribute=True, cli_name='seealso', multivalue=False, required=False)
option: Str('setattr*', cli_name='setattr', exclude='webui')
option: StrEnum('usercategory', attribute=True, cli_name='usercat', multivalue=False, required=False, values=(u'all',))
option: Str('version?', exclude='webui')
@@ -2665,17 +2666,18 @@ output: Output('result', type 'bool', None)
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: Output('value', type 'unicode', None)
command: selinuxusermap_find
-args: 1,13,4
+args: 1,14,4
arg: Str('criteria?', noextrawhitespace=False)
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False)
option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
+option: Str('hbacrule', attribute=False, autofill=False, cli_name='hbacrule', multivalue=False, query=True, required=False)
option: StrEnum('hostcategory', attribute=True, autofill=False, cli_name='hostcat', multivalue=False, query=True, required=False, values=(u'all',))
option: Bool('ipaenabledflag', attribute=True, autofill=False, cli_name='ipaenabledflag', multivalue=False, query=True, required=False)
option: Str('ipaselinuxuser', attribute=True, autofill=False, cli_name='selinuxuser', multivalue=False, query=True, required=False)
option: Flag('pkey_only?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Str('seealso', attribute=True, autofill=False, cli_name='hbacrule', multivalue=False, query=True, required=False)
+option: DNParam('seealso', attribute=True, autofill=False, cli_name='seealso', multivalue=False, query=True, required=False)
option: Int('sizelimit?', autofill=False, minvalue=0)
option: Int('timelimit?', autofill=False, minvalue=0)
option: StrEnum('usercategory', attribute=True, autofill=False,
Re: [Freeipa-devel] [PATCHES 0031-0032] Improve HBAC rule handling in selinuxusermap-add/mod/find
Tomas Babej wrote: Hi, this pair of patches improves HBAC rule handling in selinuxusermap commands. Patch 0031 deals with: https://fedorahosted.org/freeipa/ticket/3349 Patch 0032 takes care of: https://fedorahosted.org/freeipa/ticket/3348 and is to be applied on top of Patch 0031. See commit messages for detailed info. Tomas ACK for patch 0032. For patch 0031 we can't change the data type of an existing attribute. It will break backwards compatibility. Can you test with an older client to see if it cares (it may not care about the name of the type). If older clients will work then this is probably ok. I gather that seealso detected as a DN attribute and converted into a DN class and this is blowing up the Str validator? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0004 Take into consideration services when deleting replicas
Ana Krivokapic wrote: On 01/24/2013 04:17 PM, Rob Crittenden wrote: Ana Krivokapic wrote: When deleting a replica from IPA domain, warn if the installation is left without CA and/or DNS. Ticket: https://fedorahosted.org/freeipa/ticket/2879 IMHO we should not give an option to delete the last CA. DNS can be more easily re-added if needed. Should we not ask this if the --force flag is set? rob Makes sense, thanks Rob. I have changed the behaviour to: * abort the deletion if the last CA is about to be deleted * warn if the last DNS is about to be deleted (but only require user confirmation if the --force flag is not set) Updated patch is attached. Looks good. I removed the comma in the sentence about the CA then pushed to master and ipa-3-1. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0107-0114 Fix Confusing ipa tool online help organization
Petr Viktorin wrote: On 02/01/2013 06:06 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 01/31/2013 07:35 PM, Rob Crittenden wrote: Petr Viktorin wrote: On 12/14/2012 01:46 AM, Dmitri Pal wrote: On 12/13/2012 10:21 AM, Petr Viktorin wrote: https://fedorahosted.org/freeipa/ticket/3060 Here is a collection of smallish fixes to `ipa help` and `ipa something --help`. This should address most of Nikolai's proposal. Additionally, it's now possible to run `ipa command --help` without a Kerberos ticket. And there are some new tests. I've not included the Often used commands in `ipa help`; I think that is material for a manual/tutorial, not a help command. Selecting a topic from `ipa topics` and then choosing a command from `ipa help TOPIC` is a better way to use the help than the verbose `ipa help commands` or proposed incomplete Often used commands. Since the ticket has a bit of discussion and you indicate that you did not to address everything can you please extract what have been addressed and put it into a design page. I know it is not RFE but it would help to validate the changes by testers. Please put the wiki link into the ticket. http://freeipa.org/page/V3/Help What is the purpose of the no-option outfile? Do you anticipate at some point opening this up as a real option or making it easier to log while using the api directly? On incorrect invocation (`ipa`, `ipa TOPIC`), the help command is called internally with outfile=sys.stderr. That's true, but this is a lot of code to abstract writing to sys.stderr. I'm just trying to understand the reasoning. Do you imagine that some time in the future we'd want to control where the output goes? I don't think that would be useful, I can't imagine why someone would want to log help texts, or use them to script something. Do you have another idea of how this should be done? The help for help is a little confusing: - Purpose: Display help for a command or topic. Usage: ipa [global-options] help [TOPIC] [options] Positional arguments: TOPIC The topic or command name. Options: -h, --help show this help message and exit - Should [TOPIC] be [TOPIC | COMMAND] or something else? I'm trying to discourage `ipa help COMMAND` in favor of `ipa COMMAND --help`, that way you get the proper help for ambiguous words like ping (https://fedorahosted.org/freeipa/ticket/3247) I also wanted to keep the help simple and not explain this unimportant detail here. If you have better wording I can of course change it. Your reasoning is sound. Im ok with gently pushing users in that direction but leaving undocumented the old behavior. Should we create a ticket to eventually return an error in that case? Users will expect `ipa help COMMAND` to get them command help, it's pretty standard in tools with subcommands. If it was a part of the API I'd be all for enforcing proper usage, but I think a help command deserves some slack -- it's not that big a deal if you get topic help for ping instead of command help... Hm. Now I see that I should add a notice to the topic help text, to lead users to the right path. Patch attached. We will want to remove `ipa help COMMAND` from the docs, and note that ipa help favors topics. We can turn https://fedorahosted.org/freeipa/ticket/3247 into a doc ticket. On my fresh F-18 install one of the new unit tests fails: == FAIL: Test that `help user-add` `user-add -h` are equivalent and contain doc -- 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/rcrit/redhat/freeipa/tests/test_cmdline/test_help.py, line 111, in test_command_help assert h_ctx.stdout == help_ctx.stdout AssertionError This is weird, it works for me. Could you send me the two outputs so I can get some idea what's wrong? Can you check you didn't leave out patch 111 (Add command summary…)? Yeah, I missed 110 and 111. Tests are passing now. I still don't understand the purpose of outfile. What do we gain by making this configurable and who or what would ever change it? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
