Re: [Freeipa-devel] [PATCH 0069] Adds 389DS plugin to enforce UUID token IDs
On 09/22/2014 09:28 PM, Simo Sorce wrote: On Mon, 22 Sep 2014 21:21:04 +0200 Martin Kosek mko...@redhat.com wrote: On 09/22/2014 04:55 PM, Simo Sorce wrote: On Mon, 22 Sep 2014 10:02:01 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Mon, 2014-09-22 at 09:50 -0400, Simo Sorce wrote: On Mon, 22 Sep 2014 10:34:54 +0200 Martin Kosek mko...@redhat.com wrote: On 09/22/2014 09:33 AM, thierry bordaz wrote: Hello Nathaniel, Just a remark, in is_token if the entry is objectclass=ipaToken it returns without freeing the 'objectclass' char array. thanks thierry On 09/21/2014 09:07 PM, Nathaniel McCallum wrote: Users that can rename the token (such as admins) can also create non-UUID token names. https://fedorahosted.org/freeipa/ticket/4456 NOTE: this patch is an alternate approach to my patch 0065. This version has two main advantages compared to 0065: 1. Permissions are more flexible (not tied to the admin group). 2. Enforcement occurs at the DS-level It should also be noted that this patch does not enforce UUID randomness, only syntax. Users can still specify a token ID so long as it is in UUID format. Nathaniel I am still thinking we may be overcomplicating it. Why cannot we use the similar UUID generation mechanism as we do for SUDO commands for example: # ipa sudocmd-add barbar --all --raw --- Added Sudo Command barbar --- dn: ipaUniqueID=3a96de14-4232-11e4-9d66-001a4a104ec9,cn=sudocmds,cn=sudo,dc=mkosek-fedora20,dc=test sudocmd: barbar ipaUniqueID: 3a96de14-4232-11e4-9d66-001a4a104ec9 objectClass: ipasudocmd objectClass: ipaobject It lets DS generaterename the object's DN when it finds out that the ipaUniqueID is set to autogenerate (in baseldap.py). We could let DS generate the UUID and only add the autogenerate keyword in otptoken-add command. For authorization, we can simply allow users to only add tokens with autogenerate ID, see my example here: http://www.redhat.com/archives/freeipa-devel/2014-September/msg00438.html Admin's or special privilege-owners would have more generous ACI allowing other values than just autogenerate. IMO, then the whole ipatoken-add mechanism would be a lot simpler and we would not need a special DS plugin (unless we want regular users to generate their own UUIDs instead of letting IPA DS to generate it - which I do not think is the case). Good point Martin. This is the avenue I first pursued. The problem is that the client has no way to look up the DN after the entry is added. In the case of sudocmd-add, the lookup is performed using the sudocmd attribute (see sudocmd.get_dn()). We have no similar attribute in this case and the lookup cannot be performed. Well in theory we could search with creatorName and createTimestamp set to the user's own DN and a range a few seconds in the past ... It is not robust if you add multiple tokens at the same time, but would this be a concern for user created tokens ? Simo. Ugh, no offense, but this approach sounds really ugly :-) I will wait for Thierry or Ludwig's assessment, but I still think there is either existing control for the modify operation to return an updated entry or we could create one as you suggest down the thread - as this may be useful for other use cases too. See following mails, the right approach is to use the POST READ control. Simo. Hello, Yes that is correct. Mark implemented it with https://fedorahosted.org/389/ticket/589 and is present since 1.3.2.18 (F20) [root@vm-046 ~]# ldapsearch -LLL -h localhost -p 389 -x -b -s base supportedcontrol | grep 1.3.6.1.1.13.2 supportedcontrol: 1.3.6.1.1.13.2 [root@vm-046 ~]# uname -a Linux vm-046.idm.lab.bos.redhat.com 3.13.10-200.fc20.x86_64 #1 SMP Mon Apr 14 20:34:16 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 247-259] ID views - management part
On 09/16/2014 01:21 PM, Tomas Babej wrote: Petr, thanks for the review, your input is, as always, much to the point. My responses are inline. Also, I'm attaching a new patchset, rebased on master, please, have a look at that. Most of the patches have at least minor changes, since I rebased another ~15 patches into this patchset, and there are additional 9 patches on top. Patch 254 was deprecated, as we decided to go with more explicit way of having two separate commands for user and group ID overrides. Also, the test suite (around 80 tests) for ID views is attached - patch 270. It depends on patch 269, sent in separate thread. Tomas On 09/10/2014 03:10 PM, Petr Viktorin wrote: On 08/01/2014 12:30 PM, Tomas Babej wrote: Hi, the following set of patches implements the ID view creation and management of views and ID overrides in IPA. Pending questions: 1.) The patch 253 implements basic managed permissions for ID views and ID overrides. Do we want to have a separate permission for assigning ID views? 2.) Performance: idview-apply command takes ~100 seconds for hostgroups with 1000 member hosts. I am able to lower that by 20-30% using raw ldap calls, is bypassing the framework worth the performance gain? We'll lose the possiblity to hook on exception callbacks. The commands also need more helpful documentation, I am working on that. Not tested yet, but here are my comments on the patches: 0247 looks good In 0248 you deleted a newline at EOF in 71-idviews.update 0249 looks good 0250: With so many names imported from one module, I find it more readable to `from ipalib.plugins import baseldap`, and use qualified names later. Same in the 2 other patches. This is personal preference/tip, feel free to disagree :) I was just trying to be consistent here, rest of the IPA plugins seem to use non-qualified names for plugins. Even tough the main reason is probably that thay use evil star imports :) Let's keep it this way for now. Yes, the rest of the plugins are using star imports; no consistency to be kept here. I think that after these patches nobody will touch it again, so for now is basically forever. 0521 looks good 0252: The pattern_errmsg in idoverride's uid should be expanded to fully describe the pattern. If we want to expand it, then it should be corrected in user.py plugin too (I have taken it from there). Still, it's a tradeoff between conciseness and and exactness, do we really want to specify that user login may be at most 253 letters long or the $ character may only be used as terminal one? Or the fact that - can't be used at the beggining? This is probably the way it was since forever in user plugin. I think the message should at be free of false positives. Using e.e. -test-name- and getting back may only include letters, numbers, _, -, . and $ would be extremely frustrating. Should I file a ticket? 0253 looks good 0255-0256 looks good 0258: idview_apply: typo in hostgroup doc, should be hosts to apply the ID view to and after running the idview-apply command. Typos in output params, should be e.g. this ID view Fixed the second and third complaint ( IMHO this is clear from context, but I added this to be explicit), I'm not so sure about the first one though. Can we get a native speaker input on that? Actually both just need an article, doesn't matter much if it's the or this. By subclassing idview_unapply from idview_apply you're violating Liskov's substitution principle. Make a common base class for them. Done. In the wrong patch, it seems :) 0259: show_id_overrides: cn is a MUST attribute in ipa*Override; use view.single_value['cn'] instead of get(). In enumerate_hosts, is None okay if cn is missing? This was actually an error, fixed in updated version of patches. New code uses 'ipaanchoruuid' (which is a MUST) and converts it (with your objections addressed.) If it's MUST, use simply `single_value['ipaanchoruuid']` instead of get(). 0260: The get() method on dict-like objects returns None (or a given default) if the item is missing. If sAMAccountName is MUST, use regular dict item access. If sAMAccountName is MAY, then you're not handling the None you might get. Also look at the other uses of get() to see if you're doing the right thing. I've looked at help(pysss_nss_idmap.getnamebysid). Shouldn't you use pysss_nss_idmap.NAME_KEY and TYPE_KEY? 0261: Again you're using get() on MUST attributes. 0262: ipalib/constants.py - No newline at end of file Is IPA_ANCHOR_PREFIX and SID_ANCHOR_PREFIX to be used outside idviews.py? Do we want it in constants? +if anchor.startswith(IPA_ANCHOR_PREFIX): +uuid = anchor.split(IPA_ANCHOR_PREFIX)[1].strip() This is a dangerous idiom to use for removing a prefix, since it will silently give wrong results if the string contains the prefix. Prefer: anchor[len(IPA_ANCHOR_PREFIX):] Same with SID_ANCHOR_PREFIX below. In get_dn, why do you resolve only the last key?
Re: [Freeipa-devel] [PATCH 0298-0302] Implement handling of inactive master zones
On 22.9.2014 15:10, Tomas Hozza wrote: On 09/19/2014 03:46 PM, Petr Spacek wrote: Hello, This patch set fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/127 https://bugzilla.redhat.com/show_bug.cgi?id=1138317 Please review it ASAP, it targets IPA 4.1/Fedora 21. Tomas and Martin, please communicate who is going to review what:-) Thank you for your time! The code seems to be fine. ACK. Thank you for review! I'm branching v5 at this point (before the push). In other words v5 branch will not contain this patch set and master branch will contain the patch set. Pushed to master (to-be-v6): 23f2ddb317ab46419ea83725f458caae1b432fb5 27a911cae9de911938d90c94c61cecd494136fc1 3dd95594e664b7b59a9f8c1d15cb1280eebeb3e7 169f5e5f2a551253bc489edf109eab71a8331c3f 5ef943a39cfdfbadeb2a41cc3efd707caeca36bd -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0303-0305] Preparation for 6.0 release
Hello, Pushed to master: Bump NVR to 6.0. aeba2eacd5bc85cd4b2872e87a6db5f6d680c266 Update NEWS for upcoming 6.0 release. 92164653b0b8a44d19dea547ddb4917069398e82 Update idnsZoneActive description in README. 8863b85b509984259052d47cbaadf2e7b84a881b -- Petr^2 Spacek From 8863b85b509984259052d47cbaadf2e7b84a881b Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 23 Sep 2014 10:34:59 +0200 Subject: [PATCH] Update idnsZoneActive description in README. Signed-off-by: Petr Spacek pspa...@redhat.com --- README | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/README b/README index fe452029965357f15915afcf87af4ea41c45cfd1..1b8f2a713ad901d3aab5ed799cbb9c5e9c746c44 100644 --- a/README +++ b/README @@ -156,9 +156,20 @@ Attributes: transfers from multiple masters to single slave. * idnsZoneActive -Boolean which speicifies if particular DNS zone should be loaded -or not. This option is not supported in versions = 4.0. -https://fedorahosted.org/bind-dyndb-ldap/ticket/127 +Boolean which speicifies if particular DNS zone should be visible +to clients or not. This attribute can be changed at run-time. + +Inactive zones are loaded into memory in the same way as active zones. +The only difference is that inactive zones are not added to DNS view +used by bind-dyndb-ldap. + +Zone will be re-added to DNS view if idnsActiveZone attribute is +changed to TRUE so the change should be almost immediate. + +Usual zone maintenance (serial number maintenance, DNSSEC in-line +signing etc.) is done for all zones, no matter if the zone +is active or not. This allows us to maintain zone journal so IXFR +works correctly even after zone re-activation. * nSEC3PARAMRecord NSEC3PARAM resource record definition according to RFC5155. -- 1.9.3 From 92164653b0b8a44d19dea547ddb4917069398e82 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 23 Sep 2014 10:36:34 +0200 Subject: [PATCH] Update NEWS for upcoming 6.0 release. Signed-off-by: Petr Spacek pspa...@redhat.com --- NEWS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS b/NEWS index f9869ba447ff34bed0d42d514846f8a47e5be496..f88d8552d34a774f684081f14340208cdf00a43b 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,9 @@ +6.0 + +[1] idnsZoneActive attribute is supported again and zones can be activated +and deactivated at run-time. +https://fedorahosted.org/bind-dyndb-ldap/ticket/127 + 5.3 [1] Internal locking was reworked to prevent crashes and deadlocks. -- 1.9.3 From aeba2eacd5bc85cd4b2872e87a6db5f6d680c266 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 23 Sep 2014 10:38:38 +0200 Subject: [PATCH] Bump NVR to 6.0. Signed-off-by: Petr Spacek pspa...@redhat.com --- configure.ac | 2 +- contrib/bind-dyndb-ldap.spec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index adb3b549e13f243d3f18adc91ddf766d036c4686..d471038ada54c07dcfc211c8a2572850e3b28205 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ([2.59]) -AC_INIT([bind-dyndb-ldap], [5.3], [freeipa-devel@redhat.com]) +AC_INIT([bind-dyndb-ldap], [6.0], [freeipa-devel@redhat.com]) AM_INIT_AUTOMAKE([-Wall foreign dist-bzip2]) diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec index 70796e62e349a94fb97047d00e4f2b145576412d..572076597a7597c8495ac7a977f26578e840603e 100644 --- a/contrib/bind-dyndb-ldap.spec +++ b/contrib/bind-dyndb-ldap.spec @@ -1,7 +1,7 @@ %define VERSION %{version} Name: bind-dyndb-ldap -Version:5.3 +Version:6.0 Release:0%{?dist} Summary:LDAP back-end plug-in for BIND -- 1.9.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 314 Allow specifying key algorithm of the IPA CA cert in ipa-server-install
Dne 6.8.2014 v 18:17 Jan Cholasta napsal(a): Dne 6.8.2014 v 14:43 Rob Crittenden napsal(a): Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4447. +cert_group.add_option(--ca-key-algorithm, dest=ca_key_algorithm, + help=Key algorithm of the IPA CA certificate (default SHA256withRSA)) Why not set the default here rather than later? CA-related defaults should be internalized in CA-related code IMHO. Should the list of options be added to the man page as well? Sure, why not. Do we want to support the MD*-based signing algorithms? I'd think not. Since the reason this patch exists is to support old and/or broken external CAs, I would think yes, but I don't have a strong opinion on this. Turns out Dogtag does not like them, so I removed them. Seeing the context makes me wonder if we should eventually add options for CA key size and signing alg as well. rob Updated patch attached. -- Jan Cholasta From 528587b8024055a8cd783cc9ebd493684b6dcc62 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Wed, 6 Aug 2014 09:43:19 +0200 Subject: [PATCH] Allow specifying key algorithm of the IPA CA cert in ipa-server-install. This is especially useful for external CA install, as the algorithm is also used for the CSR signature. https://fedorahosted.org/freeipa/ticket/4447 --- install/tools/ipa-server-install | 13 ++--- install/tools/man/ipa-server-install.1 | 3 +++ ipaserver/install/cainstance.py| 12 ++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index 207dabd..e0cc165 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -226,6 +226,10 @@ def parse_options(): cert_group.add_option(--subject, action=callback, callback=subject_callback, type=string, help=The certificate subject base (default O=realm-name)) +cert_group.add_option(--ca-key-algorithm, dest=ca_key_algorithm, + type=choice, + choices=('SHA1withRSA', 'SHA256withRSA', 'SHA512withRSA'), + help=Key algorithm of the IPA CA certificate) parser.add_option_group(cert_group) dns_group = OptionGroup(parser, DNS options) @@ -1082,7 +1086,8 @@ def main(): dogtag_constants=dogtag.install_constants) if external == 0: ca.configure_instance(host_name, domain_name, dm_password, - dm_password, subject_base=options.subject) + dm_password, subject_base=options.subject, + ca_key_algorithm=options.ca_key_algorithm) elif external == 1: # stage 1 of external CA installation options.realm_name = realm_name @@ -1097,14 +1102,16 @@ def main(): write_cache(vars(options)) ca.configure_instance(host_name, domain_name, dm_password, dm_password, csr_file=paths.ROOT_IPA_CSR, - subject_base=options.subject) + subject_base=options.subject, + ca_key_algorithm=options.ca_key_algorithm) else: # stage 2 of external CA installation ca.configure_instance(host_name, domain_name, dm_password, dm_password, cert_file=options.external_cert_file, cert_chain_file=options.external_ca_file, - subject_base=options.subject) + subject_base=options.subject, + ca_key_algorithm=options.ca_key_algorithm) # Now put the CA cert where other instances exepct it ca.publish_ca_cert(CACERT) diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1 index 8cc2ffa..957bd6d 100644 --- a/install/tools/man/ipa-server-install.1 +++ b/install/tools/man/ipa-server-install.1 @@ -123,6 +123,9 @@ PEM file containing the CA certificate of the CA which issued the Directory Serv .TP \fB\-\-subject\fR=\fISUBJECT\fR The certificate subject base (default O=REALM.NAME) +.TP +\fB\-\-ca\-key\-algorithm\fR=\fIALGORITHM\fR +Key algorithm of the IPA CA certificate. Possible values are SHA1withRSA, SHA256withRSA, SHA512withRSA. Default value is SHA256withRSA. .SS DNS OPTIONS .TP diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index 8c1b139..6534010 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -410,7 +410,7 @@ class CAInstance(service.Service): pkcs12_info=None, master_host=None, csr_file=None,
Re: [Freeipa-devel] [PATCHES] 0264-0267 backup, restore: Don't overwrite /etc/{passwd, group}
On 08/26/2014 01:16 PM, Petr Viktorin wrote: On 07/30/2014 04:26 PM, Petr Viktorin wrote: On 07/29/2014 06:03 PM, Petr Viktorin wrote: On 07/29/2014 05:02 PM, Petr Viktorin wrote: Hello, The first patch here consolidates our system user creation code a bit. The second patch fixes an oversight in the restore script. The third changes the backup script to not include /etc/{passwd,group}, and the restore script to create the PKI user if a CA is being restored. Note that the DS user is already created early in the restore process. (In the future we may want a nice generic framework for restoring users, but I'd like to extrapolate from more than one data point when designing it.) Another note: tar uses owner user/group names by default, so no additional chowning is required even if the IDs change between backup restore. The fourth patch adds a log entry I find very useful in testing backup/restore. Rebased onto current master. Rebased again. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I like the approach, solves the issue quite neatly. I didn't find any issues in the changes themselves, or during the testing, so ACK from me for the whole patcheset (which means patches 624-627, despite the subject). -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0631-0632 Integration tests for backup restore
On 08/06/2014 04:52 PM, Petr Viktorin wrote: On 08/06/2014 04:36 PM, Petr Viktorin wrote: Hello, These patches add integration tests for backup restore. They depend on my earlier backup/restore patches, 0624-0627. I'm also attaching a patch for the job definitions at https://github.com/encukou/freeipa-ci I hit Send too soon, sorry for that Thank you for these patches, the tests themselves seem all right to me. My only objection is that I don't think that the coverage for the basic backup-restore test is exhaustive enough. Right now we only check the presence of the admin entry in the LDAP via raw ldap calls and CLI, and the output of the cert-find command. In the future, I'd like this to be extended with basic functionality tests for each of the services, i.e. does DNS work after the restore? Are all the services up and running? However, this does not block the patches, I think they can be pushed now (this means a ACK from me) in their current form and extended later. If you agree, I can file a ticket. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0264-0267 backup, restore: Don't overwrite /etc/{passwd, group}
On 09/23/2014 12:10 PM, Tomas Babej wrote: On 08/26/2014 01:16 PM, Petr Viktorin wrote: On 07/30/2014 04:26 PM, Petr Viktorin wrote: On 07/29/2014 06:03 PM, Petr Viktorin wrote: On 07/29/2014 05:02 PM, Petr Viktorin wrote: Hello, The first patch here consolidates our system user creation code a bit. The second patch fixes an oversight in the restore script. The third changes the backup script to not include /etc/{passwd,group}, and the restore script to create the PKI user if a CA is being restored. Note that the DS user is already created early in the restore process. I like the approach, solves the issue quite neatly. I didn't find any issues in the changes themselves, or during the testing, so ACK from me for the whole patcheset (which means patches 624-627, despite the subject). Thanks! Pushed to: master: abba25c8269f4928d659cfcaf8363ad2491bd736 ipa-4-1: 127e7a1dcc2ed0624f65597292ac58535ccc0602 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/18/2014 06:34 PM, Martin Basti wrote: ... 1) +if options.unattended: +for ip in ip_addresses: +if search_reverse_zones and find_reverse_zone(str(ip)): +# reverse zone is already in LDAP +continue +for rz in ret_reverse_zones: +if verify_reverse_zone(rz, ip): +# reverse zone was entered by user +break +else: +rz = get_reverse_zone_default(str(ip)) +ret_reverse_zones.append(rz) +elif options.reverse_zones or create_reverse(): +for ip in ip_addresses: +if search_reverse_zones and find_reverse_zone(str(ip)): +# reverse zone is already in LDAP +continue +for rz in ret_reverse_zones: +if verify_reverse_zone(rz, ip): +# reverse zone was entered by user +break +else: +rz = get_reverse_zone_default(str(ip)) +rz = read_reverse_zone(rz, str(ip)) +ret_reverse_zones.append(rz) +else: +options.no_reverse = True +ret_reverse_zones = [] You can make it shorter without duplications: # this ifs can be in one line if not options.unatended: if not options.reverse_zones if not create_reverse(): options.no_reverse=True return [] for ip in ip_addresses: if search_reverse_zones and find_reverse_zone(str(ip)): # reverse zone is already in LDAP continue for rz in ret_reverse_zones: if verify_reverse_zone(rz, ip): # reverse zone was entered by user break else: rz = get_reverse_zone_default(str(ip)) if not options.unattended: rz = read_reverse_zone(rz, str(ip)) ret_reverse_zones.append(rz) Thanks, I modified it bit different way to alse address recommendation 3). 2) Typo? There is no IP address matching reverze_zone %s. -^^ Thanks, fixed. 3) Would be nice to ask user to create new zones only if new zones are required. (If all required zones exist in LDAP, you ask user anyway) I added one more variable and ask only once. 4) Ask framework gurus, if installutils module is better place for function above Petr^3 said that it's ok to have it in bindinstance.py. -- David Kupka From cea7c3e8eb41798d7f2bba916a95a78c034ee052 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 27 Aug 2014 13:50:21 +0200 Subject: [PATCH] Detect and configure all usable IP addresses. Find, verify and configure all IP addresses that can be used to reach the server FreeIPA is being installed on. Ignore some IP address only if user specifies subset of detected addresses using --ip-address option. This change simplyfies FreeIPA installation on multihomed and dual-stacked servers. https://fedorahosted.org/freeipa/ticket/3575 --- install/tools/ipa-replica-install| 48 ++--- install/tools/ipa-server-install | 50 ++ ipaserver/install/bindinstance.py| 114 ++- ipaserver/install/installutils.py| 96 +- ipaserver/install/ipa_replica_prepare.py | 65 ++ 5 files changed, 207 insertions(+), 166 deletions(-) diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install index e3b65b0963d92e4d102a1166aa001fe8cb164562..bdc1a86fd339718c91a67b6c32daa69905025e90 100755 --- a/install/tools/ipa-replica-install +++ b/install/tools/ipa-replica-install @@ -67,8 +67,8 @@ def parse_options(): default=False, help=configure a dogtag CA) basic_group.add_option(--setup-kra, dest=setup_kra, action=store_true, default=False, help=configure a dogtag KRA) -basic_group.add_option(--ip-address, dest=ip_address, - type=ip, ip_local=True, +basic_group.add_option(--ip-address, dest=ip_addresses, + type=ip, ip_local=True, action=append, default=[], help=Replica server IP Address) basic_group.add_option(-p, --password, dest=password, sensitive=True, help=Directory Manager (existing master) password) @@ -112,7 +112,8 @@ def parse_options(): type=ip, help=Add a DNS forwarder) dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true, default=False, help=Do not add any DNS forwarders, use root servers instead) -dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use) +dns_group.add_option(--reverse-zone, dest=reverse_zones, default=[], +
[Freeipa-devel] Announcing bind-dyndb-ldap version 6.0
The FreeIPA team is proud to announce bind-dyndb-ldap version 6.0. It can be downloaded from https://fedorahosted.org/released/bind-dyndb-ldap/ The new version has also been built for Fedora 21+ and and is on its way to updates-testing: https://admin.fedoraproject.org/updates/bind-dyndb-ldap-6.0-1.fc21 This version is also available from FreeIPA COPR repo: http://copr.fedoraproject.org/coprs/mkosek/freeipa/ 6.0 [1] idnsZoneActive attribute is supported again and zones can be activated and deactivated at run-time. https://fedorahosted.org/bind-dyndb-ldap/ticket/127 == Upgrading == A server can be upgraded by installing updated RPM. BIND has to be restarted manually after the RPM installation. Downgrading back to any 5.x version is supported if idnsZoneActive is always set to TRUE. == Feedback == Please provide comments, report bugs and send any other feedback via the freeipa-users mailing list: http://www.redhat.com/mailman/listinfo/freeipa-users -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 Fix certmonger code causing the ca_renewal_master update plugin to fail
On 09/17/2014 03:57 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4547. Honza Works for me, thanks for patch. ACK. -- David Kupka ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0105 FIX: LDAP_updater
On 22/09/14 14:04, Petr Viktorin wrote: On 09/01/2014 04:31 PM, Martin Basti wrote: On 24/07/14 09:06, Martin Basti wrote: On 23/07/14 15:17, Martin Basti wrote: This patch fixes ordering problem of schema updates Martin should it be in IPA 4.0.x ? It requires rebased ldap_python (will be in Fedora 21) Patch attached I found a bug there, but before I send updated version, I need to decide how print modlist: 1. Print modlist before each LDAP update (if changes exist), show modlist per incremental update 2. Print modlist only at once with all updates 3. Use [1] for live_run, [2] for test Print modlis before each LDAP update Updated patch attached. Thanks! This works nicely, just some comments: The required python-ldap is quite new, not even built in Koji yet. We need to get it to the IPA COPR repo before this is merged. The _get_oid_dependency_order function is not indented well. It would be nice if it was a bit more obvious that the loop in _get_oid_dependency_order is guaranteed to terminate. Could you assert that len(unordered_oids) decreases in each iteration? +SCHEMA_ELEMENT_CLASSES_KEYS = (x[0] for x in SCHEMA_ELEMENT_CLASSES) You're creating a generator here, which is only good for one use. Use a list instead. Also, this is used just once so it doesn't need to be a module-level constant. +# FIXME: We should have a better way to display the modlist, +# for now display raw output of our internal routine Not entirely related to your change, but you can drop this comment. Since 61887ac we don't use an internal routine. Thank you! updated patch attached. -- Martin Basti From 996992efff178b8bd400dcbfe6ab6203b14beefc Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 23 Jul 2014 14:42:33 +0200 Subject: [PATCH] FIX: ldap schmema updater needs correct ordering of the updates Required bugfix in python-ldap 2.4.15 Updates must respect SUP objectclasses/attributes and update dependencies first --- freeipa.spec.in | 2 +- ipaserver/install/schemaupdate.py | 125 ++ 2 files changed, 87 insertions(+), 40 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index c92c245776c83bb86c78575e0f88bc62b849a831..d692e8c3d054be144298696a9f1a383a1f5d60d0 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -99,7 +99,7 @@ Requires: httpd = 2.4.6-6 Requires: mod_wsgi Requires: mod_auth_kerb = 5.4-16 Requires: mod_nss = 1.0.8-26 -Requires: python-ldap +Requires: python-ldap = 2.4.15 Requires: python-krbV Requires: acl Requires: python-pyasn1 diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py index bb2f0f161499c0358452d97f27f5c39b9b64a5ad..eaa45d5e947803bb339b9462df666f58f06e0a2d 100644 --- a/ipaserver/install/schemaupdate.py +++ b/ipaserver/install/schemaupdate.py @@ -29,17 +29,60 @@ from ipaserver.install.ldapupdate import connect from ipaserver.install import installutils -SCHEMA_ELEMENT_CLASSES = { +SCHEMA_ELEMENT_CLASSES = ( # All schema model classes this tool can modify -'objectclasses': ldap.schema.models.ObjectClass, -'attributetypes': ldap.schema.models.AttributeType, -} +# Depends on order, attributes first, then objectclasses +('attributetypes', ldap.schema.models.AttributeType), +('objectclasses', ldap.schema.models.ObjectClass), +) ORIGIN = 'IPA v%s' % ipapython.version.VERSION log = log_mgr.get_logger(__name__) +def _get_oid_dependency_order(schema, cls): + +Returns a ordered list of OIDs sets, in order which respects inheritance in LDAP +OIDs in second set, depend on first set, etc. + +:return [set(1st-tree-level), set(2nd-tree-level), ...] + +top_node = '_' +ordered_oid_groups = [] + +tree = schema.tree(cls) # tree structure of schema + +# remove top_node from tree, it breaks ordering +# we don't need this, tree from file is not consistent +del tree[top_node] +unordered_oids = tree.keys() + +# split into two groups, parents and child nodes, and iterate until +# child nodes are not empty +while unordered_oids: +parent_nodes = set() +child_nodes = set() + +for node in unordered_oids: +if node not in child_nodes: +# if node was child once, must remain as child +parent_nodes.add(node) + +for child_oid in tree[node]: +# iterate over all child nodes stored in tree[node] per node +# child node must be removed from parents +parent_nodes.discard(child_oid) +child_nodes.add(child_oid) + +ordered_oid_groups.append(parent_nodes) # parents nodes are not dependent + +assert len(child_nodes) len(unordered_oids) # while iteration must be finite +unordered_oids = child_nodes # extract new parent nodes in next iteration + +return ordered_oid_groups
[Freeipa-devel] [PATCH] JSON client: Log pretty-printed request and response with -vvv or above
https://fedorahosted.org/freeipa/ticket/4233 $ ipa -vvv user-show admin ipa: INFO: trying https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json ipa: INFO: Forwarding 'user_show' to json server 'https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json' ipa: INFO: Request: { id: 0, method: user_show, params: [ [ admin ], { all: false, no_members: false, raw: false, rights: false, version: 2.102 } ] } [...bunch of output omitted...] ipa: INFO: Response: { error: null, id: 0, principal: ad...@idm.lab.eng.brq.redhat.com, result: { result: { dn: uid=admin,cn=users,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com, gidnumber: [ 36480 ], has_keytab: true, has_password: true, homedirectory: [ /home/admin ], loginshell: [ /bin/bash ], memberof_group: [ admins, trust admins ], nsaccountlock: false, sn: [ Administrator ], uid: [ admin ], uidnumber: [ 36480 ] }, summary: null, value: admin }, version: 4.0.0GIT88bea65 } User login: admin Last name: Administrator Home directory: /home/admin Login shell: /bin/bash UID: 36480 GID: 36480 Account disabled: False Password: True Member of groups: admins, trust admins Kerberos keys available: True -- Petr³ From 88bea6539b0f23c9a92427ae3036178b67bcbcdb Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 23 Sep 2014 12:10:56 +0200 Subject: [PATCH] JSON client: Log pretty-printed request and response with -vvv or above Changes `verbose` in the connection to be the level from api.env, rather than a boolean value. https://fedorahosted.org/freeipa/ticket/4233 --- ipalib/backend.py | 2 +- ipalib/rpc.py | 11 +-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ipalib/backend.py b/ipalib/backend.py index b94264236795b65905ba425ef15e452e7a66625b..210058981a6ecc5884040648e9d7929abf473af7 100644 --- a/ipalib/backend.py +++ b/ipalib/backend.py @@ -113,7 +113,7 @@ def create_context(self, ccache=None, client_ip=None): if self.env.in_server: self.Backend.ldap2.connect(ccache=ccache) else: -self.Backend.rpcclient.connect(verbose=(self.env.verbose = 2), +self.Backend.rpcclient.connect(verbose=self.env.verbose, fallback=self.env.fallback, delegate=self.env.delegate) if client_ip is not None: setattr(context, client_ip, client_ip) diff --git a/ipalib/rpc.py b/ipalib/rpc.py index 1bfc4c3d3f8ac36c9dbb135561707eaa9f6ac7f3..88bed16807b3fb3255225f2aa1942933062b0f48 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -761,7 +761,7 @@ def apply_session_cookie(self, url): return session_url -def create_connection(self, ccache=None, verbose=False, fallback=True, +def create_connection(self, ccache=None, verbose=0, fallback=True, delegate=False, nss_dir=None): try: rpc_uri = self.env[self.env_rpc_uri_key] @@ -965,11 +965,15 @@ def __request(self, name, args): payload = {'method': unicode(name), 'params': args, 'id': 0} version = args[1].get('version', VERSION_WITHOUT_CAPABILITIES) +if self.__verbose = 3: +root_logger.info('Request: %s', + json.dumps(payload, sort_keys=True, indent=4)) + response = self.__transport.request( self.__host, self.__handler, json.dumps(json_encode_binary(payload, version)), -verbose=self.__verbose, +verbose=self.__verbose = 2, ) try: @@ -977,6 +981,9 @@ def __request(self, name, args): except ValueError, e: raise JSONError(str(e)) +if self.__verbose = 3: +root_logger.info('Response: %s', + json.dumps(response, sort_keys=True, indent=4)) error = response.get('error') if error: try: -- 1.9.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] JSON client: Log pretty-printed request and response with -vv or above
On 09/23/2014 03:13 PM, Petr Viktorin wrote: https://fedorahosted.org/freeipa/ticket/4233 After talking to Rob, I've changed what the -v means a bit more: A single -v just turns on INFO logging, as before: $ ipa -v ping ipa: INFO: trying https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json ipa: INFO: Forwarding 'ping' to json server 'https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json' - IPA server version 4.0.0GIT8543d4c. API version 2.102 - Two -v's pretty-print the request response: $ ipa -vv ping ipa: INFO: trying https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json ipa: INFO: Forwarding 'ping' to json server 'https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json' ipa: INFO: Request: { id: 0, method: ping, params: [ [], { version: 2.102 } ] } ipa: INFO: Response: { error: null, id: 0, principal: ad...@idm.lab.eng.brq.redhat.com, result: { summary: IPA server version 4.0.0GIT8543d4c. API version 2.102 }, version: 4.0.0GIT8543d4c } - IPA server version 4.0.0GIT8543d4c. API version 2.102 - And three -v's print everything -- the pretty-printed JSON and all of the HTTP communication. Also, when using XML-RPC, a single -v will now also print all the HTTP stuff. It could respond to two -v's as before I don't think it's worth complicating the code (keep in mind this is client only, XML-RPC is not used unless requested in the env). This patch also updates the man page. -- Petr³ From 721fdbc50ffd951eba2e5427233c909d0245c788 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 23 Sep 2014 12:10:56 +0200 Subject: [PATCH] JSON client: Log pretty-printed request and response with -vv or above The whole HTTP request is now printed with -vvv or above. Changes `verbose` in the connection to be the level from api.env, rather than a boolean value. For XML-RPC, the whole request will be shown already with -v. https://fedorahosted.org/freeipa/ticket/4233 --- ipa.1 | 3 ++- ipalib/backend.py | 2 +- ipalib/rpc.py | 11 +-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ipa.1 b/ipa.1 index fc39fceaae5aa4c614ccaaa7e608f2326d926755..cadd9aa98d7d976f5a683b5fe83ab30ee357dd99 100644 --- a/ipa.1 +++ b/ipa.1 @@ -56,7 +56,8 @@ Prompt for all parameters of \fICOMMAND\fR, even if they are optional. Don't fall back to other IPA servers if the default doesn't work. .TP \fB\-v\fR, \fB\-\-verbose\fR -Produce verbose output. A second \-v displays the XML\-RPC request. +Produce verbose output. A second -v pretty-prints the JSON request and response. A third \-v displays the HTTP request and response. +.TP \fB\-\-version\fR Display the IPA version and API version. .SH COMMANDS diff --git a/ipalib/backend.py b/ipalib/backend.py index b94264236795b65905ba425ef15e452e7a66625b..210058981a6ecc5884040648e9d7929abf473af7 100644 --- a/ipalib/backend.py +++ b/ipalib/backend.py @@ -113,7 +113,7 @@ def create_context(self, ccache=None, client_ip=None): if self.env.in_server: self.Backend.ldap2.connect(ccache=ccache) else: -self.Backend.rpcclient.connect(verbose=(self.env.verbose = 2), +self.Backend.rpcclient.connect(verbose=self.env.verbose, fallback=self.env.fallback, delegate=self.env.delegate) if client_ip is not None: setattr(context, client_ip, client_ip) diff --git a/ipalib/rpc.py b/ipalib/rpc.py index 1bfc4c3d3f8ac36c9dbb135561707eaa9f6ac7f3..e7e60f414b9a519fc98efdb0cee2205ed60b331c 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -761,7 +761,7 @@ def apply_session_cookie(self, url): return session_url -def create_connection(self, ccache=None, verbose=False, fallback=True, +def create_connection(self, ccache=None, verbose=0, fallback=True, delegate=False, nss_dir=None): try: rpc_uri = self.env[self.env_rpc_uri_key] @@ -965,11 +965,15 @@ def __request(self, name, args): payload = {'method': unicode(name), 'params': args, 'id': 0} version = args[1].get('version', VERSION_WITHOUT_CAPABILITIES) +if self.__verbose = 2: +root_logger.info('Request: %s', + json.dumps(payload, sort_keys=True, indent=4)) + response = self.__transport.request( self.__host, self.__handler, json.dumps(json_encode_binary(payload, version)), -verbose=self.__verbose, +verbose=self.__verbose = 3, ) try: @@ -977,6 +981,9 @@ def __request(self, name, args): except ValueError, e: raise JSONError(str(e)) +if self.__verbose = 2: +
Re: [Freeipa-devel] [PATCH] 323 Fix certmonger code causing the ca_renewal_master update plugin to fail
On 09/23/2014 02:34 PM, David Kupka wrote: On 09/17/2014 03:57 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4547. Honza Works for me, thanks for patch. ACK. Pushed to: master: f680a63158d172042c91537a1cb7f6f53766e2ad ipa-4-1: 1a327cf42929919219c2f0bfa9b48eb2d0b039f4 ipa-4-0: 26188d7610170ff2fb89b12cd63a0c698a2381cb -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 130] extdom: add support for new version
Hi, this patch should fix https://fedorahosted.org/freeipa/ticket/4031 and with the corresponding SSSD part it would be possible to get the full list of group memberships with the id command even for user who didn't log in before. bye, Sumit From 23ff38cdea85995b211e73f474bcb4b0d7fb8039 Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Tue, 23 Sep 2014 15:55:43 +0200 Subject: [PATCH] extdom: add support for new version Currently the extdom plugin is basically used to translate SIDs of AD users and groups to names and POSIX IDs. With this patch a new version is added which will return the full member list for groups and the full list of group memberships for a user. Additionally the gecos field, the home directory and the login shell of a user are returned and an optional list of key-value pairs which currently will contain the SID of the requested object if available. https://fedorahosted.org/freeipa/ticket/4031 --- .../ipa-extdom-extop/ipa_extdom.h | 29 +- .../ipa-extdom-extop/ipa_extdom_common.c | 850 +++-- .../ipa-extdom-extop/ipa_extdom_extop.c| 28 +- 3 files changed, 640 insertions(+), 267 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h index 5f834a047a579104cd2589ce417c580c1c5388d3..548ee74f561c474854c049726c4c3e71da5cbbea 100644 --- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h +++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h @@ -64,6 +64,7 @@ #include sss_nss_idmap.h #define EXOP_EXTDOM_OID 2.16.840.1.113730.3.8.10.4 +#define EXOP_EXTDOM_V2_OID 2.16.840.1.113730.3.8.10.4.1 #define IPA_EXTDOM_PLUGIN_NAME ipa-extdom-extop #define IPA_EXTDOM_FEATURE_DESC IPA trusted domain ID mapper @@ -71,6 +72,11 @@ #define IPA_PLUGIN_NAME IPA_EXTDOM_PLUGIN_NAME +enum extdom_version { +EXTDOM_V1 = 1, +EXTDOM_V2 +}; + enum input_types { INP_SID = 1, INP_NAME, @@ -80,14 +86,17 @@ enum input_types { enum request_types { REQ_SIMPLE = 1, -REQ_FULL +REQ_FULL, +REQ_FULL_WITH_GROUPS }; enum response_types { RESP_SID = 1, RESP_NAME, RESP_USER, -RESP_GROUP +RESP_GROUP, +RESP_USER_GROUPLIST, +RESP_GROUP_MEMBERS }; struct extdom_req { @@ -123,11 +132,18 @@ struct extdom_res { char *user_name; uid_t uid; gid_t gid; +char *gecos; +char *home; +char *shell; +size_t ngroups; +char **groups; } user; struct { char *domain_name; char *group_name; gid_t gid; +size_t nmembers; +char **members; } group; } data; }; @@ -150,15 +166,14 @@ struct pwd_grp { struct passwd pwd; struct group grp; } data; +int ngroups; +gid_t *groups; }; int parse_request_data(struct berval *req_val, struct extdom_req **_req); void free_req_data(struct extdom_req *req); +int check_request(struct extdom_req *req, enum extdom_version version); int handle_request(struct ipa_extdom_ctx *ctx, struct extdom_req *req, - struct extdom_res **res); -int create_response(struct extdom_req *req, struct pwd_grp *pg_data, -const char *sid_str, enum sss_id_type id_type, -const char *domain_name, struct extdom_res **_res); -void free_resp_data(struct extdom_res *res); + struct berval **berval); int pack_response(struct extdom_res *res, struct berval **ret_val); #endif /* _IPA_EXTDOM_H_ */ diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c index 025d37dc5eda05c8db43d4e8176fd7898ed32fe7..5c1ae79c818676c3660d5cd5b8ca5515a4f0f18d 100644 --- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c +++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c @@ -70,6 +70,7 @@ int parse_request_data(struct berval *req_val, struct extdom_req **_req) *requestType ENUMERATED { *simple (1), *full (2) + *full_with_groups (3) *}, *data InputData * } @@ -179,23 +180,23 @@ void free_req_data(struct extdom_req *req) free(req); } -int handle_request(struct ipa_extdom_ctx *ctx, struct extdom_req *req, - struct extdom_res **res) +int check_request(struct extdom_req *req, enum extdom_version version) +{ +if (version == EXTDOM_V1) { +if (req-request_type == REQ_FULL_WITH_GROUPS) { +return LDAP_PROTOCOL_ERROR; +} +} + +return LDAP_SUCCESS; +} + +static int get_buffer(size_t *_buf_len, char **_buf) { -int ret; -char *domain_name = NULL; -char *sid_str = NULL; -size_t buf_len; -char *buf = NULL; long pw_max; long gr_max; -struct pwd_grp pg_data; -
Re: [Freeipa-devel] Krb service delegation rules in CLI
On 09/22/2014 09:48 PM, Alexander Bokovoy wrote: On Mon, 22 Sep 2014, Martin Basti wrote: Hello, Related ticket: https://fedorahosted.org/freeipa/ticket/3644 1) API The ipaKrb5DelegationACL objectclass requires targets which are stored in extra objectclass. A) we allow users to create groups of principals and then associate them as targets -- user can use same group for multiple delegation ACL B) users specify only list of target principals (no groups) B seems better to me. No. You have three clearly separate object types here: 1. Principals -- any object in the tree that has krbPrincipalAux supplemental auxiliary object class. These objects can be anywhere in the tree, we can decide on limiting subtrees to search. 2. Delegation ACLs -- any object that has ipaKrb5DelegationACL supplemental structural object class. These objects can be anywhere under cn=s4u2proxy,$SUFFIX 3. Groups of principals -- any object that has groupOfPrincipals _and_ doesn't have ipaKrb5DelegationACL object classes. These objects can be anywhere in the tree, we can decide on limiting subtrees to search and can decide on a specific subtree to put new groups of principals. For (1) you just need to be able to reference principals, this only requires a search filter to define DN of the principal based on krbPrincipalName or krbCanonicalName, with support for 'shortcuts' (specifying principal without realm which is our realm). (1) is only used to resolve principals for (2) and (3). For (2) you need to implement a proper object handling, like any other IPA object that supports: -- multiple values for ipaAllowToImpersonate -- multiple values for ipaAllowedTarget CLI would look something like this: ipa krbdelegation-add -show -find -mod -add-source -add-target -del-source -del-target -del For (3) you need to implement a proper object handling where creating the object would happen in the predefined subtree but search or modify would be possible for any object matched by a proper filter. CLI would look something like this: ipa krbprincipalgroup-add -show -find -mod -add-member -del-member The way we use right now groupOfPrincipals, the objects referenced are placed in different subtrees, according to their primary role. That's right, as krbPrincipalAux is an auxiliary class, and is added on top of other object classes to turn other objects into principals. We also use groupOfPrincipals not only for Kerberos delegation ACLs, that's why support for multiple subtrees is needed. Creating new groupOfPrincipals objects can be allowed in a single designated place, cn=s4u2proxy,$SUFFIX, because this is primary use of this API. Alexander's proposal makes sense to me. I would just personally name the commands differently to share the same prefix. I.e. ipa krbdelegation-* and ipa krbdelegationgroup-* as we are not creating general purpose principal groups but only groups for S4U2Proxy usage. I would also reconsider your request to search for the objects in the whole s4u2proxy tree. Across FreeIPA framework, we handle objects in static and defined location of our tree, not in the entire subtree. I think we should although indeed consider adding group of principals to own nsContainer (like cn=targets) as otherwise we will have a clash between krbdelegation rule names and krbdelegationgroup names. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0114-0115] DNS: allow to add root zone '.'
On 25.8.2014 14:52, Martin Basti wrote: Patches attached. Ticket: https://fedorahosted.org/freeipa/ticket/4149 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the named service is stopped after deleting zone. Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138 Review of: http://www.redhat.com/archives/freeipa-devel/2014-September/msg00484.html 1. Please follow pep8 for the new code. # git diff HEAD~7 -U0 | pep8 --diff --ignore=E501 Produces 25 erros. Only E124 and E128 could be ignored if they are part of old code. Patch 114: 2. Also should mention reverse zones: -# except for our own domain +# except for our own domain, and root zone Patch 115: looks ok (except #1) Patch 120: 3. This patch uses term 'deprecated' in a different meaning than a DeprecatedParam. It creates inconsistency - future confusion. IMHO this usage is correct since the usual understanding of deprecation is that the param is still usable but user should be prepared that it will be removed in a future. IMHO DeprecatedParam is badly designed but that's not an issue of this patch. I think we can leave this as is and create a ticket to rename DeprecatedParam e.g. to RemovedParam. What do you think? That said, I don't see a reason for keeping 'doc', 'label', _validate_ipaddr in 'ip_address' param definition since the param is not used anywhere, and is not present in CLI nor Web UI. 4. Convention for defining optional param is to use '?' at the end of name not: `required=False,` (idnssoamname) 5. You've removed 'idnssoamname' and 'force' from Web UI but dnszone-add precallback still uses these params. What is the intended purpose? Patch 121: 6. Could this be evaluated as 'None' string? idnssoamname=unicode(ns_hostname), the method doesn't guarantee that 'hostname is not None' and ipa_replica_prepare calls it without args: `add_zone(domain)` 7. This line is too long: +self.step(adding NS record to the zones, self.__add_self_ns) # all zones must be created before this step 8. Forgotten debugging lines: print no ldap2 connection #TODO remove and maybe: print entry, repr(entry) 9. Is the LDAP connection check in remove_server_ns_records required? Other RR removal methods are called before this one and they don't have it. IMO if it has to be done, it should be done by a caller. Note: I have not tested replica installation and removal in this round of review process. Patch 123: 10. In `normalize_zonemgr(zonemgr)`, if zonemgr contains '@' shouldn't it be normalized to contain '.' at the end? Or is it handled by bind-dyndb-ldap? 11. You can remove `validate_zonemgr` import from dns.py Patch 124: 12. 'idnssoamname' field in dnszone details page should be hidden or marked as required since the LDAP attribute is a MUST 13. You can completely remove `IPA.dnszone_adder_dialog` class since it existed only to contain the logic for idnssamname and ip_address fields (depends on #5). Then remove `$factory: IPA.dnszone_adder_dialog,` from the dialog spec. Patch 125: 14. Got: File /home/pvoborni/dev/freeipa/ipatests/test_xmlrpc/test_dns_plugin.py, line 332, in module class test_dns(Declarative): File /home/pvoborni/dev/freeipa/ipatests/test_xmlrpc/test_dns_plugin.py, line 422, in test_dns 'nsrecord': nameservers, NameError: name 'nameservers' is not defined Did not investigate much.. but this line is weird: `e = No DNS servers found in LDAP` shouldn't 'e' be 'get_nameservers_error'? Unrelated to this patch set: a. One is able to run: # ipa dnszone-remove-permission $zone multiple times and it always returns success Is it intentional? b. Web UI doesn't have means to call dnszone-mod with --force option -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0645 ipa-replica-prepare: Wait for the DNS entry to be resolvable
On 22.9.2014 14:09, Petr Viktorin wrote: On 09/22/2014 01:48 PM, Petr Spacek wrote: On 22.9.2014 10:38, Martin Kosek wrote: On 09/22/2014 10:31 AM, Petr Spacek wrote: On 22.9.2014 10:14, Martin Kosek wrote: On 09/19/2014 07:29 PM, Petr Viktorin wrote: https://fedorahosted.org/freeipa/ticket/4551 See ticket commit message for details. Shouldn't we add a 1 sec sleep between tries? Wouldn't current version just hammer DNS server with as many DNS queries as it can send? Oh yes, please add some time.sleep() call :-) Wow, no idea how that slipped out. Thanks for the catch. Also I would like to see more detailed message: +self.log.info('Waiting for hostname %s to be resolvable', + self.replica_fqdn) = 'Waiting for hostname %s to be resolvable to A or record' bikeshed Really? Shouldn't term resolvable already have that covered? A good software should work on all network types, whether it is IPv4, IPv6 or IPv8. So I personally do not think we need to be that specific and can stick to original proposal. I will agree with you if you post magic code which will work with DNS records for IPv8 :-) The code is not going to work with IPv8 just because we didn't mention 'A/' in the error message, A and RRtypes are hardcoded in the code. +1; we're checking A and so that's what we should say we're doing. Is this wording OK? Little NACK. (However, the wording is fine.) Tcpdump revealed this: IP vm-117.test.34067 vm-133.test.domain: 38467+ A? vm-092.test. (51) IP vm-133.test.domain vm-117.test.34067: 38467 NXDomain* 0/1/0 (116) IP vm-117.test.36006 vm-133.test.domain: 20194+ A? vm-092.test.ipa.example. (63) IP vm-133.test.domain vm-117.test.36006: 20194 NXDomain* 0/1/0 (143) IP vm-117.test.51333 vm-133.test.domain: 34027+ ? vm-092.test. (51) IP vm-133.test.domain vm-117.test.51333: 34027 NXDomain* 0/1/0 (116) IP vm-117.test.60373 vm-133.test.domain: 45679+ ? vm-092.test.ipa.example. (63) You can see that the query for each A/ type is repeated twice, the second time with 'ipa.example.' suffix. This is caused by search list processing (search directive in /etc/resolv.conf) and is highly undesirable. (Read this [1] e-mail if you want to hear it from Paul Vixie.) The fix is simple: You have to be sure that self.replica_fqdn is actually absolute FQDN - with the trailing period. Naive solution would be to use dns_answer = resolver.query(self.replica_fqdn + '.', 'A', 'IN') but I don't know if self.replica_fqdn variable can contain trailing period or not. Mbasti can show you more advanced code snippets using 'dns.name'. [1] https://lists.dns-oarc.net/pipermail/dns-operations/2014-September/012157.html -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0126 - 0127] DNS: remove --class option
On 22.9.2014 19:21, Martin Basti wrote: On 22/09/14 13:17, Petr Vobornik wrote: On 19.9.2014 16:15, Martin Basti wrote: Ticket: https://fedorahosted.org/freeipa/ticket/3414 Patch attached. Patch 126: 1. I think that just DeprecatedParam('dnsclass?'), should be enough. Also 2. You forgot to update API.txt and VERSION Patch 127: ACK Updated patchset attached ACK, it works for me. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0126 - 0127] DNS: remove --class option
On 23/09/14 18:35, Petr Spacek wrote: On 22.9.2014 19:21, Martin Basti wrote: On 22/09/14 13:17, Petr Vobornik wrote: On 19.9.2014 16:15, Martin Basti wrote: Ticket: https://fedorahosted.org/freeipa/ticket/3414 Patch attached. Patch 126: 1. I think that just DeprecatedParam('dnsclass?'), should be enough. Also 2. You forgot to update API.txt and VERSION Patch 127: ACK Updated patchset attached ACK, it works for me. Please don't push, we discuss this and we will nit use the DeprecatedParam. -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 23/09/14 13:23, David Kupka wrote: On 09/18/2014 06:34 PM, Martin Basti wrote: ... 1) +if options.unattended: +for ip in ip_addresses: +if search_reverse_zones and find_reverse_zone(str(ip)): +# reverse zone is already in LDAP +continue +for rz in ret_reverse_zones: +if verify_reverse_zone(rz, ip): +# reverse zone was entered by user +break +else: +rz = get_reverse_zone_default(str(ip)) +ret_reverse_zones.append(rz) +elif options.reverse_zones or create_reverse(): +for ip in ip_addresses: +if search_reverse_zones and find_reverse_zone(str(ip)): +# reverse zone is already in LDAP +continue +for rz in ret_reverse_zones: +if verify_reverse_zone(rz, ip): +# reverse zone was entered by user +break +else: +rz = get_reverse_zone_default(str(ip)) +rz = read_reverse_zone(rz, str(ip)) +ret_reverse_zones.append(rz) +else: +options.no_reverse = True +ret_reverse_zones = [] You can make it shorter without duplications: # this ifs can be in one line if not options.unatended: if not options.reverse_zones if not create_reverse(): options.no_reverse=True return [] for ip in ip_addresses: if search_reverse_zones and find_reverse_zone(str(ip)): # reverse zone is already in LDAP continue for rz in ret_reverse_zones: if verify_reverse_zone(rz, ip): # reverse zone was entered by user break else: rz = get_reverse_zone_default(str(ip)) if not options.unattended: rz = read_reverse_zone(rz, str(ip)) ret_reverse_zones.append(rz) Thanks, I modified it bit different way to alse address recommendation 3). 2) Typo? There is no IP address matching reverze_zone %s. -^^ Thanks, fixed. 3) Would be nice to ask user to create new zones only if new zones are required. (If all required zones exist in LDAP, you ask user anyway) I added one more variable and ask only once. 4) Ask framework gurus, if installutils module is better place for function above Petr^3 said that it's ok to have it in bindinstance.py. NACK, most important point is 7 1) I'm not sure if this is bug, but interactively is allowed to add only one ip address Unable to resolve IP address for host name Please provide the IP address to be used for this host name: 2001:db8::2 The kerberos protocol requires a Realm name to be defined. 2) I'm getting error message Invalid reverse zone 10.in-addr.arpa. for IP address 2001:db8::dead:beef Invalid reverse zone 10.in-addr.arpa. for IP address fed0:babe:baab:0:21a:4aff:fe10:4e18 - or - Do you want to configure the reverse zone? [yes]: Please specify the reverse zone name [0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.]: Invalid reverse zone 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa. for IP address fed0:babe:baab:0:21a:4aff:fe10:4e18 Please specify the reverse zone name [0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.]: Using reverse zone(s) 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa., 0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa. This shouldn't be there Could be better to ask user to specific zone for ip address a.b.c.d. 4) just nitpick The IPA Master Server will be configured with: ... IP address(es): 2001:db8::dead:beef, fed0:babe:baab:0:21a:4aff:fe10:4e18 ... Reverse zone: 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa., 0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa. You have label IP address(es), so you should use label Reverse zone(s) 5) ipa-server-install --ip-address=10.16.78.105 --reverse-zone=10.in-addr.arpa. --reverse-zone=16.10.in-addr.arpa. --setup-dns Creates both reverse zones, but 10.in-addr.arpa. is empty. I'm not sure if this is wrong, but we prevents user to add zone without address in it, so we should prevents, to add empty zone. 6) ipa-replica-prepare --ip-address 10.16.78.105 --ip-address 2001:db8::dead:beef --reverse-zone 1.0.0.2.ip6.arpa. vm-105.example.com Directory Manager (existing master) password: Invalid reverse zone 1.0.0.2.ip6.arpa. for IP address 10.16.78.105 Invalid reverse zone 1.0.0.2.ip6.arpa. IMO This should work, right? +sys.exit(There is no IP address matching reverse zone %s. % rz) I expected at least this error to be shown. 7) ipa-replica-prepare --ip-address 10.16.78.105 --ip-address 2001:db8::dead:beef vm-105.example.com Directory Manager (existing master) password: ... Adding DNS records for vm-105.example.com Values instance has no
Re: [Freeipa-devel] [PATCHES] 319-321 Build and packaging fixes
On 17.9.2014 13:58, Jan Cholasta wrote: Dne 17.9.2014 v 13:07 Alexander Bokovoy napsal(a): On Wed, 17 Sep 2014, Martin Kosek wrote: On 09/17/2014 12:31 PM, Jan Cholasta wrote: +Conflicts: %{alt_name}-server-trust-ad +Obsoletes: %{alt_name}-server-trust-ad %{version}-%{release} Just one question - should we check also for %{release}? Generally, release number does not have much value, we could rebuild our version of the package 10 times, but it does not mean it is newer. I.e. freeipa-server-4.0.0-10 should probably not obsolete ipa-server-4.0.0-1 but only ipa-server-3.3.3-10, right? I agree with the %{release} idea. We definitely don't need to be so picky with it. OK, fixed. I used wrong numbers for the patches, they should be 320-322. Fixed as well. Updated patches attached. Patch 320: ACK Patch 321: ACK Patch 322: ACK All pushed: master: * 0e2dc70d8ec4d96914190a38dac9a240605cab1d Allow RPM upgrade from ipa-* packages * 9fa8cff6da6b6b85aa5b03028386f159fc816124 Include ipaplatform in client-only build * 449d10b85cf2fe6a5b41bf167419f2432e752ad1 Include the ipa command in client-only build ipa-4-1: * 9486f3dc8d549b83f5a64b3dc8e57bf4896588d8 Allow RPM upgrade from ipa-* packages * 72a82b855b77e04a0e672eaffc59e33256a66721 Include ipaplatform in client-only build * fbc6345153391213d59e1c7c267e6425fdf7b77e Include the ipa command in client-only build ipa-4-0: * faba3f060e7513790d584bde3a9901793031fcf1 Allow RPM upgrade from ipa-* packages * df187a4b4a9a2eeac8b11152de0cdc250e55cfb1 Include ipaplatform in client-only build * 22ce015913c482308d499572bd24389bca98a0e5 Include the ipa command in client-only build -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes
Jan Cholasta wrote: Hi, the attached patches fix various bugs and shortcomings in the CA management and renewal code. Related tickets: https://fedorahosted.org/freeipa/ticket/4416, https://fedorahosted.org/freeipa/ticket/4460. (Patch 319 was originally posted at http://www.redhat.com/archives/freeipa-devel/2014-September/msg00132.html.) Only two of the patches includes what ticket(s) they address. Most have the tersest of commit messages. If got more and more difficult to see why the changes were needed at all, as you'll see. As a side note, it was rather difficult to review parts of this as different patches modify exactly the same code in different ways. General: These patches don't replace /etc/pki/nssdb, they just add a new location. Do we need to keep using the shared db because of p11-kit? 319: The post-install script tries to get a cert with nickname 'IPA CA'. It should also look for '$REALM IPA CA'. I'd use ipapythhon.certdb.py:CA_NICKNAME_FMT. Having a similar option for External CA is probably not a bad idea. You don't need the password file to import a cert into an NSS database. It may be too complex in a spec file but would it be better to try to fetch things out of LDAP? This nearly screams for a separate script to do the work. Are these operations important enough to be logged to /var/log/ipaupgrade.log? I suspect it's the kind of thing that might fail and never be noticed. At some point we'll upgrade to sql NSS databases. Is this going to complicate things? There are some places you do str(e[2]) that aren't necessary. Might be worthwhile to clean up the comments regarding XML-RPC while you're at it and just refer to RPC. I'd rename ca_certs_nss to ca_certs_trust for clarity. Is the separate helper create_ipa_nssdb() necessary? Should create_db() be extended to do this extra work? 324: ACK 325: The exception from get_cert() is very rough so wouldn't cover the case of file permissions, missing database, etc, but since the overall behavior isn't changing, ACK. Might be worth a comment though indicating a potential source of oddness for the uninitiated. 326: ACK 327: @@ -133,6 +116,7 @@ class CertUpdate(admintool.AdminTool): criteria = { 'cert-database': dogtag_constants.ALIAS_DIR, 'cert-nickname': nickname, +'ca-name': 'dogtag-ipa-ca-renew-agent', } request_id = certmonger.get_request_id(criteria) if request_id is not None: Doesn't seem related to this change. 328: ACK 329: ACK 330: I don't understand the reference to ipa.p11-kit (an unowned file, btw). Are you removing any cert that may be left over from some previous install? If the file were to exist it could cause update-ca-trust to be executed twice. Is that needed? I think more clarity in the commit message will clear things up. 331: ACK 332: What is the reason for this refactoring? 333: Seems reasonable but why would a CA profile change in the middle of a request? 334: Same boat, why? 335: I kinda get the picture, along with previous patches, but need more info. Some tips on testing would be helpful. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Continuous Integration dependency tree testing
Hello, The recent problem with Tomcat forced me to think how we can detect that some other package broke IPA before it hit users. After all, it seems pretty easy. 0) Get a VM for testing purposes (preferably with minimal set of IPA dependencies). The VM has to be reverted to snapshot after every run. (This is pretty simple with RHEV-M/libvirt.) 1) Detect all changes in the minimal set of packages installed on the test VM. It is easy because we can simply hash output of YUM/DNF upgrade command: # echo n | dnf upgrade | md5sum changeset.id changeset.id will change every time when a package which is present on test system changes but stays the same if there were no updates from last run. (There could be false positives but it doesn't matter - the test will be simply run twice.) 2) if (changeset.id != previous_changeset.id) run_tests() 3) This machinery could be triggered by cron or even by a message on fedmsg about a new package pushed to updates-testing... Tests could use released versions of IPA (present in fedora-updates) to get better signal/noise ratio. Okay, it is time to stop dreaming and go to bed. Good night from Europe :-) -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Krb service delegation rules in CLI
On Tue, 23 Sep 2014, Martin Kosek wrote: On 09/22/2014 09:48 PM, Alexander Bokovoy wrote: On Mon, 22 Sep 2014, Martin Basti wrote: Hello, Related ticket: https://fedorahosted.org/freeipa/ticket/3644 1) API The ipaKrb5DelegationACL objectclass requires targets which are stored in extra objectclass. A) we allow users to create groups of principals and then associate them as targets -- user can use same group for multiple delegation ACL B) users specify only list of target principals (no groups) B seems better to me. No. You have three clearly separate object types here: 1. Principals -- any object in the tree that has krbPrincipalAux supplemental auxiliary object class. These objects can be anywhere in the tree, we can decide on limiting subtrees to search. 2. Delegation ACLs -- any object that has ipaKrb5DelegationACL supplemental structural object class. These objects can be anywhere under cn=s4u2proxy,$SUFFIX 3. Groups of principals -- any object that has groupOfPrincipals _and_ doesn't have ipaKrb5DelegationACL object classes. These objects can be anywhere in the tree, we can decide on limiting subtrees to search and can decide on a specific subtree to put new groups of principals. For (1) you just need to be able to reference principals, this only requires a search filter to define DN of the principal based on krbPrincipalName or krbCanonicalName, with support for 'shortcuts' (specifying principal without realm which is our realm). (1) is only used to resolve principals for (2) and (3). For (2) you need to implement a proper object handling, like any other IPA object that supports: -- multiple values for ipaAllowToImpersonate -- multiple values for ipaAllowedTarget CLI would look something like this: ipa krbdelegation-add -show -find -mod -add-source -add-target -del-source -del-target -del For (3) you need to implement a proper object handling where creating the object would happen in the predefined subtree but search or modify would be possible for any object matched by a proper filter. CLI would look something like this: ipa krbprincipalgroup-add -show -find -mod -add-member -del-member The way we use right now groupOfPrincipals, the objects referenced are placed in different subtrees, according to their primary role. That's right, as krbPrincipalAux is an auxiliary class, and is added on top of other object classes to turn other objects into principals. We also use groupOfPrincipals not only for Kerberos delegation ACLs, that's why support for multiple subtrees is needed. Creating new groupOfPrincipals objects can be allowed in a single designated place, cn=s4u2proxy,$SUFFIX, because this is primary use of this API. Alexander's proposal makes sense to me. I would just personally name the commands differently to share the same prefix. I.e. ipa krbdelegation-* and ipa krbdelegationgroup-* as we are not creating general purpose principal groups but only groups for S4U2Proxy usage. I would also reconsider your request to search for the objects in the whole s4u2proxy tree. Across FreeIPA framework, we handle objects in static and defined location of our tree, not in the entire subtree. You have to deal with the fact that we already have established uses and need to support them. New entities can be created where they are but existing ones cannot be moved for the sake of moving because DNs of some objects are used in external config files. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Krb service delegation rules in CLI
On Tue, 23 Sep 2014 17:18:38 +0200 Martin Kosek mko...@redhat.com wrote: On 09/22/2014 09:48 PM, Alexander Bokovoy wrote: On Mon, 22 Sep 2014, Martin Basti wrote: Hello, Related ticket: https://fedorahosted.org/freeipa/ticket/3644 1) API The ipaKrb5DelegationACL objectclass requires targets which are stored in extra objectclass. A) we allow users to create groups of principals and then associate them as targets -- user can use same group for multiple delegation ACL B) users specify only list of target principals (no groups) B seems better to me. No. You have three clearly separate object types here: 1. Principals -- any object in the tree that has krbPrincipalAux supplemental auxiliary object class. These objects can be anywhere in the tree, we can decide on limiting subtrees to search. 2. Delegation ACLs -- any object that has ipaKrb5DelegationACL supplemental structural object class. These objects can be anywhere under cn=s4u2proxy,$SUFFIX 3. Groups of principals -- any object that has groupOfPrincipals _and_ doesn't have ipaKrb5DelegationACL object classes. These objects can be anywhere in the tree, we can decide on limiting subtrees to search and can decide on a specific subtree to put new groups of principals. For (1) you just need to be able to reference principals, this only requires a search filter to define DN of the principal based on krbPrincipalName or krbCanonicalName, with support for 'shortcuts' (specifying principal without realm which is our realm). (1) is only used to resolve principals for (2) and (3). For (2) you need to implement a proper object handling, like any other IPA object that supports: -- multiple values for ipaAllowToImpersonate -- multiple values for ipaAllowedTarget CLI would look something like this: ipa krbdelegation-add -show -find -mod -add-source -add-target -del-source -del-target -del For (3) you need to implement a proper object handling where creating the object would happen in the predefined subtree but search or modify would be possible for any object matched by a proper filter. CLI would look something like this: ipa krbprincipalgroup-add -show -find -mod -add-member -del-member The way we use right now groupOfPrincipals, the objects referenced are placed in different subtrees, according to their primary role. That's right, as krbPrincipalAux is an auxiliary class, and is added on top of other object classes to turn other objects into principals. We also use groupOfPrincipals not only for Kerberos delegation ACLs, that's why support for multiple subtrees is needed. Creating new groupOfPrincipals objects can be allowed in a single designated place, cn=s4u2proxy,$SUFFIX, because this is primary use of this API. Alexander's proposal makes sense to me. I would just personally name the commands differently to share the same prefix. I.e. ipa krbdelegation-* and ipa krbdelegationgroup-* as we are not creating general purpose principal groups but only groups for S4U2Proxy usage. I would also reconsider your request to search for the objects in the whole s4u2proxy tree. Across FreeIPA framework, we handle objects in static and defined location of our tree, not in the entire subtree. I think we should although indeed consider adding group of principals to own nsContainer (like cn=targets) as otherwise we will have a clash between krbdelegation rule names and krbdelegationgroup names. Keep in mind that we already have a few rules in there and the groups are in the same container, the names are different as the groups are named rule-name-targets, so we'll have to keep using a subtree search, but that is fine cn=s4u2proxy has no subtree atm. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 755 webui-ci: case-insensitive record check
On 9/22/2014 9:49 AM, Petr Vobornik wrote: [PATCH] webui-ci: case-insensitive record check Indirect association are no longer lower cased, which caused a issue in CI. Is the use of |= operator intentional? I don't see the has variable defined anywhere else in this method. has |= self.has_record(pkey.lower(), parent, table_name) If this has been tested to work, then ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 756 webui: fix regression in association facet preop
On 9/22/2014 9:50 AM, Petr Vobornik wrote: Association facet specs use 'add_method' instead of 'add_command' origin: https://fedorahosted.org/freeipa/ticket/4507 ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel