Re: [Freeipa-devel] [PATCH 0141] Generalize attribute_name-rdata_type conversions
On 05/10/2013 04:57 PM, Petr Spacek wrote: On 6.5.2013 17:40, Tomas Hozza wrote: On 04/08/2013 07:45 PM, Petr Spacek wrote: Generalize attribute_name-rdata_type conversions. Attribute names are generated on-the-fly: String Record is appended to textual representation of DNS RDATA type. String Record is cut down from the attribute name during attribute name to rdata type conversion. From now, the plugin doesn't add artificial limitation to supported record types. ACK. The patch looks good. (I didn't do functional test) Cosmetic issue: I think it would be good to dynamically allocate mod_type in LDAPMod in every case and include the mod_type memory freeing in free_ldapmod() function. Now one has to be be careful when it is statically or dynamically allocated. Before it was static in every case. It is good idea. This version of the patch contains ldap_mod_create() function which allocates the whole structure including mod_type of fixed size. All writes to mod_type checks the array length, so it should not cause any harm. The function modify_soa_record() still uses statically allocated LDAPMod structure with statically allocated strings for mod_type, but the LDAPMod structure never leave this function. There are no calls to ldap_mod_create() and ldap_mod_free(), so I think it is obvious. Tbabej, please try to dynamically update some A records with sync_ptr enabled. (And of course the support for some new type, like TLSA.) For the existing record types, the patch works fine. For any new types, a schema change is still required, since record types are still hardcoded in LDAP schema: LDAP error: Object class violation: attribute tlsarecord not allowed Thank you! ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0142] Improve LDAP error logging
On 05/07/2013 09:36 AM, Tomas Hozza wrote: On 04/09/2013 03:27 PM, Petr Spacek wrote: Hello, Improve LDAP error logging. Diagnostic error message is logged when it is available. Plugin with this patch produces messages like: LDAP error: Server is unwilling to perform: Minimum SSF not met.: bind to LDAP server failed intead of bind to LDAP server failed: Server is unwilling to perform Second example is: LDAP error: Object class violation: attribute mgrecord not allowed : while modifying(add) entry 'idnsName=pspacek, idnsname=example.com,cn=dns,dc=e,dc=test' instead of :-D snip diff --git a/src/log.h b/src/log.h index 312f24322fd0c6f9943c6beb810ac0bcd8f3896c..cbf1a3faaaccea7391d65d018e80d8ec688fc111 100644 --- a/src/log.h +++ b/src/log.h @@ -55,16 +55,30 @@ log_write(GET_LOG_LEVEL(level), format, ##__VA_ARGS__) /* LDAP logging functions */ -#define log_ldap_error(ld) \ - do {\ - int err;\ - char *errmsg = UNKNOWN; \ - if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, err) \ - == LDAP_OPT_SUCCESS)\ - errmsg = ldap_err2string(err); \ - log_error_position(LDAP error: %s, errmsg); \ - } while (0);\ +#define LOG_LDAP_ERR_PREFIX LDAP error: +#define log_ldap_error(ld, desc, ...) \ + do { \ + int err; \ + char *errmsg = NULL; \ + char *diagmsg = NULL; \ + if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, err) \ + == LDAP_OPT_SUCCESS) { \ + errmsg = ldap_err2string(err); \ Getting error msg for the first time here. + if (ldap_get_option(ld, LDAP_OPT_DIAGNOSTIC_MESSAGE, diagmsg) \ + == LDAP_OPT_SUCCESS diagmsg != NULL) { \ + errmsg = ldap_err2string(err); \ Again getting error msg with the same err. Maybe a copy-paste error? + log_error(LOG_LDAP_ERR_PREFIX %s: %s: desc,\ + errmsg, diagmsg, ##__VA_ARGS__);\ + ldap_memfree(diagmsg); \ + } else \ + log_error(LOG_LDAP_ERR_PREFIX %s: desc,\ + errmsg, ##__VA_ARGS__); \ + } else { \ + log_error(LOG_LDAP_ERR_PREFIX \ + unable to obtain LDAP error code: \ + desc, ##__VA_ARGS__); \ + } \ + } while (0); void log_write(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3); Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK, provides the desired info. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0143] Treat syntax errors in LDAP filters as fatal
On 05/07/2013 10:16 AM, Tomas Hozza wrote: On 04/09/2013 03:39 PM, Petr Spacek wrote: Hello, Treat syntax errors in LDAP filters as fatal. Filters are hardcoded at the moment, this is preventive action. ACK. The patch looks good. (I didn't do functional test) Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK. It does not break anything :) Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0027 Prompt for nameserver IP address in dnszone-add
On 05/13/2013 02:50 PM, Petr Vobornik wrote: On 05/12/2013 03:10 PM, Ana Krivokapic wrote: On 05/10/2013 10:37 PM, Endi Sukma Dewata wrote: On 5/10/2013 9:38 AM, Petr Viktorin wrote: On 05/10/2013 03:57 PM, Ana Krivokapic wrote: [...] Thanks for catching the bugs. Updated patches are attached. Thanks! It works nicely. Endi is doing a quick check of the Javascript, if he doesn't find an issue then ACK. If this still makes it into 3.2.0, please only push the first patch there. I tried this in the UI: Zone name: test.com Authoritative nameserver: ns.sometest.com. The 'Nameserver IP address' field is still enabled. This is because the name server is considered in the zone although it's actually not. The CLI seems to work fine, it didn't ask for IP address. The UI probably could be fixed using endsWith(ns, '.' + zone). Everything else looks fine. ACK with the fix. Fixed, updated patch attached. A nitpick for UI part which is not a blocker(nack) because we don't have any strict rules for following topic: We should avoid depending on widget's html output outside of the widget code. So we should use: zone_w.save()[0] instead of: $('input', zone_w.container).val(); same for `ns`. Thanks, fixed. Unfortunately there is no text_widget.is_enabled() method so `zone_w.input.prop('disabled')` can't be replaced. I implemented the `text_widget.is_enabled()` method, and replaced `zone_w.input.prop('disabled')` with `!zone_w.is_enabled()`. Updated patch attached. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From 89bfc2047cd6e832c36245e1a183b9269b6d8b86 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Thu, 9 May 2013 18:47:12 +0200 Subject: [PATCH] Prompt for nameserver IP address in dnszone-add Prompt for nameserver IP address in interactive mode of dnszone-add. Add a corresponding field to dnszone creation dialog in the web UI. This parameter is required if and only if: * New zone is a forward zone * Nameserver is defined inside the new zone Add a new unit test to cover this functionality. https://fedorahosted.org/freeipa/ticket/3603 --- install/ui/src/freeipa/dns.js | 58 + install/ui/src/freeipa/widget.js| 4 ++ install/ui/test/data/ipa_init_commands.json | 11 + ipalib/plugins/dns.py | 21 + tests/test_cmdline/test_cli.py | 67 + 5 files changed, 161 insertions(+) diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js index 5024e8b768ea46c86eb4db5901e71b02866432ff..897ea7114fe8e520f446f0525a039d689dcddf64 100644 --- a/install/ui/src/freeipa/dns.js +++ b/install/ui/src/freeipa/dns.js @@ -300,6 +300,11 @@ return { fields: [ 'idnssoamname', { +name: 'ip_address', +validators: [ 'ip_address' ], +metadata: '@mc-opt:dnszone_add:ip_address' +}, +{ name: 'idnssoarname', required: false }, @@ -576,11 +581,64 @@ IPA.dnszone_adder_dialog = function(spec) { var that = IPA.entity_adder_dialog(spec); +function endsWith(str, suffix) { +return str.indexOf(suffix, str.length - suffix.length) !== -1; +} + +var init = function() { +var zone_w = that.fields.get_field('idnsname').widget; +var reverse_zone_w = that.fields.get_field('name_from_ip').widget; +var ns_w = that.fields.get_field('idnssoamname').widget; + +zone_w.value_changed.attach(that.check_ns_ip); +reverse_zone_w.value_changed.attach(that.check_ns_ip); +ns_w.value_changed.attach(that.check_ns_ip); +}; + +that.check_ns_ip = function() { +var ip_address_f = that.fields.get_field('ip_address'); +var zone_w = that.fields.get_field('idnsname').widget; +var ns_w = that.fields.get_field('idnssoamname').widget; + +var zone = zone_w.save()[0]; +var ns = ns_w.save()[0]; + +var zone_is_reverse = !zone_w.is_enabled() || + endsWith(zone, '.in-addr.arpa.') || + endsWith(zone, '.ip6.arpa.'); +var relative_ns = true; +var ns_in_zone = false; + +if (ns ns[ns.length-1] === '.') { +relative_ns = false; +ns = ns.slice(0, -1); +} + +if (zone zone[zone.length-1] === '.') { +zone = zone.slice(0, -1); +} + +if (ns zone endsWith(ns, '.' + zone)) { +ns_in_zone = true; +} + +if (!zone_is_reverse (relative_ns || ns_in_zone)) { +ip_address_f.set_enabled(true); +ip_address_f.set_required(true); +} else { +ip_address_f.reset(); +
Re: [Freeipa-devel] [PATCH] 0027 Prompt for nameserver IP address in dnszone-add
On 05/14/2013 12:05 PM, Ana Krivokapic wrote: On 05/13/2013 02:50 PM, Petr Vobornik wrote: On 05/12/2013 03:10 PM, Ana Krivokapic wrote: On 05/10/2013 10:37 PM, Endi Sukma Dewata wrote: On 5/10/2013 9:38 AM, Petr Viktorin wrote: On 05/10/2013 03:57 PM, Ana Krivokapic wrote: [...] Thanks for catching the bugs. Updated patches are attached. Thanks! It works nicely. Endi is doing a quick check of the Javascript, if he doesn't find an issue then ACK. If this still makes it into 3.2.0, please only push the first patch there. I tried this in the UI: Zone name: test.com Authoritative nameserver: ns.sometest.com. The 'Nameserver IP address' field is still enabled. This is because the name server is considered in the zone although it's actually not. The CLI seems to work fine, it didn't ask for IP address. The UI probably could be fixed using endsWith(ns, '.' + zone). Everything else looks fine. ACK with the fix. Fixed, updated patch attached. A nitpick for UI part which is not a blocker(nack) because we don't have any strict rules for following topic: We should avoid depending on widget's html output outside of the widget code. So we should use: zone_w.save()[0] instead of: $('input', zone_w.container).val(); same for `ns`. Thanks, fixed. Unfortunately there is no text_widget.is_enabled() method so `zone_w.input.prop('disabled')` can't be replaced. I implemented the `text_widget.is_enabled()` method, and replaced `zone_w.input.prop('disabled')` with `!zone_w.is_enabled()`. Updated patch attached. Petr caught another bug: due to the return value of `text_widget.save()`, an exception was raised in the case of empty zone. This has been fixed in the attached patch. I also changed the name of the endsWith() function to ends_with(), to conform to our coding standard. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From 160e5a46e9a21c702d61c457343e2f0408ede2d4 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Thu, 9 May 2013 18:47:12 +0200 Subject: [PATCH] Prompt for nameserver IP address in dnszone-add Prompt for nameserver IP address in interactive mode of dnszone-add. Add a corresponding field to dnszone creation dialog in the web UI. This parameter is required if and only if: * New zone is a forward zone * Nameserver is defined inside the new zone Add a new unit test to cover this functionality. https://fedorahosted.org/freeipa/ticket/3603 --- install/ui/src/freeipa/dns.js | 58 + install/ui/src/freeipa/widget.js| 4 ++ install/ui/test/data/ipa_init_commands.json | 11 + ipalib/plugins/dns.py | 21 + tests/test_cmdline/test_cli.py | 67 + 5 files changed, 161 insertions(+) diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js index 5024e8b768ea46c86eb4db5901e71b02866432ff..52cbb81f36f863afd61bf3b6cc04affe59809a48 100644 --- a/install/ui/src/freeipa/dns.js +++ b/install/ui/src/freeipa/dns.js @@ -300,6 +300,11 @@ return { fields: [ 'idnssoamname', { +name: 'ip_address', +validators: [ 'ip_address' ], +metadata: '@mc-opt:dnszone_add:ip_address' +}, +{ name: 'idnssoarname', required: false }, @@ -576,11 +581,64 @@ IPA.dnszone_adder_dialog = function(spec) { var that = IPA.entity_adder_dialog(spec); +function ends_with(str, suffix) { +return str.indexOf(suffix, str.length - suffix.length) !== -1; +} + +var init = function() { +var zone_w = that.fields.get_field('idnsname').widget; +var reverse_zone_w = that.fields.get_field('name_from_ip').widget; +var ns_w = that.fields.get_field('idnssoamname').widget; + +zone_w.value_changed.attach(that.check_ns_ip); +reverse_zone_w.value_changed.attach(that.check_ns_ip); +ns_w.value_changed.attach(that.check_ns_ip); +}; + +that.check_ns_ip = function() { +var ip_address_f = that.fields.get_field('ip_address'); +var zone_w = that.fields.get_field('idnsname').widget; +var ns_w = that.fields.get_field('idnssoamname').widget; + +var zone = zone_w.save()[0] || ''; +var ns = ns_w.save()[0] || ''; + +var zone_is_reverse = !zone_w.is_enabled() || + ends_with(zone, '.in-addr.arpa.') || + ends_with(zone, '.ip6.arpa.'); +var relative_ns = true; +var ns_in_zone = false; + +if (ns ns[ns.length-1] === '.') { +relative_ns = false; +ns = ns.slice(0, -1); +} + +if (zone zone[zone.length-1] === '.') { +zone = zone.slice(0, -1); +
Re: [Freeipa-devel] [PATCH 0141] Generalize attribute_name-rdata_type conversions
On 14.5.2013 11:45, Tomas Babej wrote: On 05/10/2013 04:57 PM, Petr Spacek wrote: On 6.5.2013 17:40, Tomas Hozza wrote: On 04/08/2013 07:45 PM, Petr Spacek wrote: Generalize attribute_name-rdata_type conversions. Attribute names are generated on-the-fly: String Record is appended to textual representation of DNS RDATA type. String Record is cut down from the attribute name during attribute name to rdata type conversion. From now, the plugin doesn't add artificial limitation to supported record types. ACK. The patch looks good. (I didn't do functional test) Cosmetic issue: I think it would be good to dynamically allocate mod_type in LDAPMod in every case and include the mod_type memory freeing in free_ldapmod() function. Now one has to be be careful when it is statically or dynamically allocated. Before it was static in every case. It is good idea. This version of the patch contains ldap_mod_create() function which allocates the whole structure including mod_type of fixed size. All writes to mod_type checks the array length, so it should not cause any harm. The function modify_soa_record() still uses statically allocated LDAPMod structure with statically allocated strings for mod_type, but the LDAPMod structure never leave this function. There are no calls to ldap_mod_create() and ldap_mod_free(), so I think it is obvious. Tbabej, please try to dynamically update some A records with sync_ptr enabled. (And of course the support for some new type, like TLSA.) For the existing record types, the patch works fine. For any new types, a schema change is still required, since record types are still hardcoded in LDAP schema: LDAP error: Object class violation: attribute tlsarecord not allowed That is expected behaviour. The point is that schema change is much simpler than recompiling the bind-dyndb-ldap (and can be done at run-time). BTW schema file contains instructions how to add support for any record type in a way compatible with rest of the world: http://drift.uninett.no/nett/ip-nett/dnsattributes.schema Was it ACK? -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
On 05/09/2013 05:23 PM, Petr Spacek wrote: On 9.5.2013 14:53, Petr Spacek wrote: On 9.5.2013 10:59, Tomas Hozza wrote: On 04/16/2013 12:45 PM, Petr Spacek wrote: Hello, Explicitly return SERVFAIL if PTR synchronization is misconfigured. SERVFAIL will be returned if PTR synchronization is enabled in forward zone but reverse zone has dynamic updates disabled. What the patch does little bit differs from what the commit message says. Explanation follows: Snip from ldap_helper.c (starting line 2959): /* Get attribute idnsAllowDynUpdate for reverse zone or use default. */ dns_name_free(zone_name, mctx); dns_name_init(zone_name, NULL); CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, zone_name, NULL)); zone_settings = NULL; result = zr_get_zone_settings(ldap_inst-zone_register, zone_name, zone_settings); if (result != ISC_R_SUCCESS) { if (result == ISC_R_NOTFOUND) log_debug(3, active zone '%s' not found, zone_dn); goto cleanup; ^ You replaced this goto with CLEANUP_WITH(DNS_R_SERVFAIL) but the check if dynamic updates in reverse zone are enabled is done in the following IF statement } CHECK(setting_get_bool(dyn_update, zone_settings, zone_dyn_update)); if (!zone_dyn_update) { log_debug(3, dynamic update is not allowed in zone '%s', zone_dn); CLEANUP_WITH(ISC_R_NOPERM); } The patch modifies the plugin to explicitly return SERVFAIL if there was some error while getting settings of PTR zone (the zone does not exist, etc). Maybe it would be good to explicitly return SERVFAIL also if dynamic updates in PTR zone are disabled and modify the commit message to better express what this patch does. You are right. Revised patch is attached. I sent a bad patch by mistake... ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I tested the patch. Works ok, ACK. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Final OTP Review
On Fri, 2013-05-10 at 16:55 -0400, Nathaniel McCallum wrote: On Fri, 2013-05-03 at 09:08 -0400, Simo Sorce wrote: On Thu, 2013-05-02 at 23:39 -0700, Nathan Kinder wrote: On 05/02/2013 10:27 PM, Nathaniel McCallum wrote: All issues fixed unless noted below... The attached patches are tested to work. On Thu, 2013-05-02 at 17:39 -0400, Simo Sorce wrote: - (nit) slapi_ch_malloc/slapi_ch_strdup are not checked for failure (although I know slapi_ch_malloc() currently just aborts on failure, I think that is plain wrong which is why I would prefer using malloc/strdup, but well, I guess this is not something I am going to care too much about for now). Not fixed. - Is the logic with auth_type_used correct ? At least the name of the variable sounds misleading, because you set it only if the authentication was successful but not set if it was 'used' but was unsuccessful. Made me look at it multiple times to reconstuct the logic. The var name could be better, however I also want a comment that explain exactly how we are going to manage authentication and fallbacks here as that logic is critical and is useful for anyone that is going to have to change this code later in order not to break it. The variable is now gone. I re-factored the section to make the logic clearer and put a nice big comment up top. - General question: how does this PRE_BIND plugin interact with ipapwd_pre_bind() in the ipa-pwd-extop() plugin ? Are you going to cause that plugin not to run by returning a result directly in this function ? Or is this plugin configured to run only after the previous one went through ? I ask because I do not see any ordering information in the cn=config plugin configuration so it is not immediately clear to me. That is a good question for Nathan since he wrote this part of the code... We would need to set the precedence if you want a predictable/guaranteed execution order. If a pre-BIND plug-in callback returns non-zero (which you should do when the plug-in sends the result to the client directly), it will cause other pre-bind plug-ins to not be called. This is actually how all pre-op plug-ins work. If a pre-op callback returns an error, we don't call the rest of the pre-op plug-ins in the list. Ok, but this does not answer my question. We definitely need to *always* run our other preop plugin as we do sanity checks like verifying if the user is enabled/disabled etc... Also we need to understand how to deal with migrating password auth when OTP is enabeld. TBH I think we should not have a separate OTP-auth plugin but we should probably have a single plugin that handles authentication and the 2 should be merged. Keeping them separate is going to cause more harm than good with unexpected interactions. We could still have 2 plugins and simply move the prebind action currently don in ipa-pwd-extop to the otp plugin by making some more code common. But it is probably easier to just merge OTP into ipa-pwd-extop right now than try to do a huge refactoring. We can always refactor the ipa-pwd-extop plugin later. The attached patches encompass an initial review of the companion daemon and merge of ipa-otp into ipa-pwd-extop. Unfortunately, merging ipa-otp into ipa-pwd-extop appears to have broken something during install, but I don't have enough familiarity with 389 to understand what I've broken. If I upgrade after an install, it appears to work. An RPM with the patches is available here: http://koji.fedoraproject.org/koji/taskinfo?taskID=5362935 Nathan / Rob / Simo, could you take a look and see what might be broken in ipa-pwd-extop? While I'm not quite sure what the problem was, I do know it appeared on the stock 3.2 F19 RPMs. I also fixed it by accident. I am certain it is unrelated to these patches. I have now tested install and upgrade with the six patches in the previous email and everything is in order, including permissions. At this point, we just need reviews/ACKs. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] DNSSEC support design considerations
Hello list, I investigated various scenarios for DNSSEC integration and I would like to hear your opinions about proposed approach and it's effects. The most important finding is that bind-dyndb-ldap can't support DNSSEC without rewrite of the 'in-memory database' component. Fortunately, it seems that we can drop our own implementation of the internal DNS database (ldap_driver.c and cache.c) and re-use the database from BIND (so called RBTDB). I'm trying to reach Adam Tkac with the question Why we decided to implement it again rather than re-use BIND's code?. Re-usage of BIND's implementation will have following properties: == Advantages == - Big part of DNSSEC implementation from BIND9 can be reused. - Overall plugin implementation will be simpler - we can drop many lines of our code and bugs. - Run-time performance could be much much better. - We will get implementation for these tickets for free: -- #95 wildcard CNAME does NOT work -- #64 IXFR support (IMHO this is important!) -- #6 Cache non-existing records And partially: -- #7 Allow limiting of the cache == Disadvantages == - Support for configurations without persistent search will complicate things a lot. -- Proposal = Make persistent search obligatory. OpenLDAP supports LDAP SyncRepl, so it should be possible to make plugin compatible with 389 and OpenLDAP at the same time. I would defer this to somebody from users/OpenLDAP community. - Data from LDAP have to be dumped to memory (or to file) before the server will start replying to queries. = This is not nice, but servers usually are not restarted often. IMHO it is a good compromise between complexity and performance. = It should be possible to save old database to disk (during BIND shutdown or periodically) and re-use this old database during server startup. I.e. server will start replying immediately from 'old' database and then the server will switch to the new database when dump from LDAP is finished. = As a side effect, BIND can start even if connection to LDAP server is down - this can improve infrastructure resiliency a lot! == Uncertain effects == - Memory consumption will change, but I'm not sure in which direction. - SOA serial number maintenance is a open question. Decision if persistent search is a 'requirement' or not will have significant impact on the design, so I will write the design document when this decision is made. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0153] Update NEWS file for upcoming 3.2 release
Hello, Update NEWS file for upcoming 3.2 release. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0154] Bump NVR to 3.2
Hello, Bump NVR to 3.2. -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0027 Prompt for nameserver IP address in dnszone-add
On 05/14/2013 01:36 PM, Ana Krivokapic wrote: On 05/14/2013 12:05 PM, Ana Krivokapic wrote: On 05/13/2013 02:50 PM, Petr Vobornik wrote: A nitpick for UI part which is not a blocker(nack) because we don't have any strict rules for following topic: We should avoid depending on widget's html output outside of the widget code. So we should use: zone_w.save()[0] instead of: $('input', zone_w.container).val(); same for `ns`. Thanks, fixed. Unfortunately there is no text_widget.is_enabled() method so `zone_w.input.prop('disabled')` can't be replaced. I implemented the `text_widget.is_enabled()` method, and replaced `zone_w.input.prop('disabled')` with `!zone_w.is_enabled()`. Updated patch attached. Petr caught another bug: due to the return value of `text_widget.save()`, an exception was raised in the case of empty zone. This has been fixed in the attached patch. I also changed the name of the endsWith() function to ends_with(), to conform to our coding standard. ACK -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0153] Update NEWS file for upcoming 3.2 release
On 14.5.2013 16:20, Petr Spacek wrote: Update NEWS file for upcoming 3.2 release. -- Petr^2 Spacek From 44a27e5cda14cee7e22efa6bbf23e2ef2f9c1e7d Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 14 May 2013 16:18:31 +0200 Subject: [PATCH] Update NEWS file for upcoming 3.2 release. Signed-off-by: Petr Spacek pspa...@redhat.com --- NEWS | 22 ++ 1 file changed, 22 insertions(+) diff --git a/NEWS b/NEWS index 878b667633bf067124a7c05c27ae949f7983db18..e54b47194f127d77bc90eee52ea10a6fdabcf826 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,25 @@ +3.2 += +[1] An error in dynamic update/transfer/query policy is interpreted as +most restrictive policy, i.e. nobody is allowed to update/transfer/query +the zone. +https://fedorahosted.org/bind-dyndb-ldap/ticket/116 + +[2] Attempts to update zones with idnsAllowDynUpdate == FALSE are logged. + +[3] TTL values 2^31-1 are interpreted as 0. +https://fedorahosted.org/bind-dyndb-ldap/ticket/117 + +[4] All RR types supported by BIND are automatically supported by plugin. +From now it is enough to add new attribute type to LDAP schema, +no recompilation is required. + +[5] PTR record synchronization deletes only PTR records, but no other records +(e.g. TXT) under names in the reverse zone. + +[6] Various improvements related to logging (dynamic updates, PTR record +synchronization, LDAP error handling). + 3.1 = [1] Crash caused by zone deletion introduced by -- 1.7.11.7 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0154] Bump NVR to 3.2
On 14.5.2013 16:20, Petr Spacek wrote: Bump NVR to 3.2. -- Petr^2 Spacek From 6b5d5adde697ead38cff9199f6e3b126f1fcd488 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 14 May 2013 16:18:52 +0200 Subject: [PATCH] Bump NVR to 3.2. 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 984de44b1c456d8105e42920c6d158bae4676c29..f49ae583f0407fad69e7cd5e5c123f8a867183ec 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ([2.59]) -AC_INIT([bind-dyndb-ldap], [3.1], [freeipa-devel@redhat.com]) +AC_INIT([bind-dyndb-ldap], [3.2], [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 e1b4f423fe2c315e33f3f92b91cc3e6445e483de..99fd26e13045aee0ec372bb739fc2e6eeccfe3d4 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:3.1 +Version:3.2 Release:0%{?dist} Summary:LDAP back-end plug-in for BIND -- 1.7.11.7 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust
On 05/13/2013 05:22 PM, Petr Spacek wrote: On 13.5.2013 16:49, Simo Sorce wrote: On Mon, 2013-05-13 at 16:32 +0200, Petr Spacek wrote: + if ((ip = inet_addr(ip_str)) == INADDR_NONE) { This kind of construct is hard to read and debug in gdb I would suggest you do: ip = inet_addr(ip_str); if (ip == INADDER_NONE) { I agree, done. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel I tested the patch, works fine. ACK Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust
On 14.5.2013 16:41, Tomas Babej wrote: On 05/13/2013 05:22 PM, Petr Spacek wrote: On 13.5.2013 16:49, Simo Sorce wrote: On Mon, 2013-05-13 at 16:32 +0200, Petr Spacek wrote: + if ((ip = inet_addr(ip_str)) == INADDR_NONE) { This kind of construct is hard to read and debug in gdb I would suggest you do: ip = inet_addr(ip_str); if (ip == INADDER_NONE) { I agree, done. I tested the patch, works fine. ACK Pushed to master: 8d12512d0eb4445f4966fd0e326cde9823f6a0bb -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0151] Disallow all zone transfers/queries if transfer/query policy configuration failed
On 9.5.2013 14:20, Tomas Hozza wrote: On 04/19/2013 12:44 PM, Petr Spacek wrote: Hello, Disallow all zone transfers/queries if transfer/query policy configuration failed. Without this patch the old policy stays in effect if re-configuration with the new policy failed. https://fedorahosted.org/bind-dyndb-ldap/ticket/116 ACK. Patch looks OK! Pushed to master: 0a5051392e218702a37073823101cbb6553b9445 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0152] Replace TTL values 2^31-1 with 0.
On 3.5.2013 15:19, Tomas Hozza wrote: - Original Message - On 3.5.2013 14:35, Tomas Babej wrote: On 04/30/2013 03:45 PM, Petr Spacek wrote: Hello, Replace TTL values 2^31-1 with 0. The rule comes from RFC 2181 section 8. https://fedorahosted.org/bind-dyndb-ldap/ticket/117 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK, works fine. Just one question though, the patch as it is leaves the invalid TTL value in the tree, even though it is never interpreted as one (thanks to this patch). $ ipa dnsrecord-show ipa.example.com skuska --all dn: idnsname=skuska,idnsname=ipa.example.com,cn=dns,dc=ipa,dc=example,dc=com Record name: skuska Time to live: 2147483648 A record: 192.168.0.1 objectclass: top, idnsrecord from /var/log/messages: named[18275]: entry 'idnsname=skuska,idnsname=ipa.example.com,cn=dns,dc=ipa,dc=example,dc=com': entry TTL 2147483648 MAXTTL, setting TTL to 0 Wouldn't that be confusing to the user? Shouldn't we fix the TTL value set in the entry as well? It is exactly what original BIND does. I would like to imitate the same behaviour if you are not against it strongly. I think that: 1) Somebody could use bind-dyndb-ldap with read-only access to LDAP. 2) It will unnecessarily complicate the code. -- Petr^2 Spacek Review ACK. The patch looks good. I also agree with Peter's reasoning. There is also an error logged when the TTL has MSB set, so one can notice there is a bad TTL value set in LDAP. Pushed to master: ccc439e5a5d8d2e0e6dbcb85351f48c501fdad03 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
On 14.5.2013 15:07, Tomas Babej wrote: On 05/09/2013 05:23 PM, Petr Spacek wrote: On 9.5.2013 14:53, Petr Spacek wrote: On 9.5.2013 10:59, Tomas Hozza wrote: On 04/16/2013 12:45 PM, Petr Spacek wrote: Hello, Explicitly return SERVFAIL if PTR synchronization is misconfigured. SERVFAIL will be returned if PTR synchronization is enabled in forward zone but reverse zone has dynamic updates disabled. What the patch does little bit differs from what the commit message says. Explanation follows: Snip from ldap_helper.c (starting line 2959): /* Get attribute idnsAllowDynUpdate for reverse zone or use default. */ dns_name_free(zone_name, mctx); dns_name_init(zone_name, NULL); CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, zone_name, NULL)); zone_settings = NULL; result = zr_get_zone_settings(ldap_inst-zone_register, zone_name, zone_settings); if (result != ISC_R_SUCCESS) { if (result == ISC_R_NOTFOUND) log_debug(3, active zone '%s' not found, zone_dn); goto cleanup; ^ You replaced this goto with CLEANUP_WITH(DNS_R_SERVFAIL) but the check if dynamic updates in reverse zone are enabled is done in the following IF statement } CHECK(setting_get_bool(dyn_update, zone_settings, zone_dyn_update)); if (!zone_dyn_update) { log_debug(3, dynamic update is not allowed in zone '%s', zone_dn); CLEANUP_WITH(ISC_R_NOPERM); } The patch modifies the plugin to explicitly return SERVFAIL if there was some error while getting settings of PTR zone (the zone does not exist, etc). Maybe it would be good to explicitly return SERVFAIL also if dynamic updates in PTR zone are disabled and modify the commit message to better express what this patch does. You are right. Revised patch is attached. I sent a bad patch by mistake... Pushed to master: 04b48143f592541d3c98e06229987e36dbaf6ec8 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0150] Do not delete whole node during PTR record synchronization.
On 9.5.2013 14:06, Tomas Hozza wrote: On 04/18/2013 04:58 PM, Petr Spacek wrote: Hello, Do not delete whole node during PTR record synchronization. https://fedorahosted.org/bind-dyndb-ldap/ticket/115 ACK. The patch looks good. Pushed to master: 1c63c045b5238fb675b7a517876869bcace2cdab -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0141] Generalize attribute_name-rdata_type conversions
On 05/14/2013 02:01 PM, Petr Spacek wrote: On 14.5.2013 11:45, Tomas Babej wrote: On 05/10/2013 04:57 PM, Petr Spacek wrote: On 6.5.2013 17:40, Tomas Hozza wrote: On 04/08/2013 07:45 PM, Petr Spacek wrote: Generalize attribute_name-rdata_type conversions. Attribute names are generated on-the-fly: String Record is appended to textual representation of DNS RDATA type. String Record is cut down from the attribute name during attribute name to rdata type conversion. From now, the plugin doesn't add artificial limitation to supported record types. ACK. The patch looks good. (I didn't do functional test) Cosmetic issue: I think it would be good to dynamically allocate mod_type in LDAPMod in every case and include the mod_type memory freeing in free_ldapmod() function. Now one has to be be careful when it is statically or dynamically allocated. Before it was static in every case. It is good idea. This version of the patch contains ldap_mod_create() function which allocates the whole structure including mod_type of fixed size. All writes to mod_type checks the array length, so it should not cause any harm. The function modify_soa_record() still uses statically allocated LDAPMod structure with statically allocated strings for mod_type, but the LDAPMod structure never leave this function. There are no calls to ldap_mod_create() and ldap_mod_free(), so I think it is obvious. Tbabej, please try to dynamically update some A records with sync_ptr enabled. (And of course the support for some new type, like TLSA.) For the existing record types, the patch works fine. For any new types, a schema change is still required, since record types are still hardcoded in LDAP schema: LDAP error: Object class violation: attribute tlsarecord not allowed That is expected behaviour. The point is that schema change is much simpler than recompiling the bind-dyndb-ldap (and can be done at run-time). BTW schema file contains instructions how to add support for any record type in a way compatible with rest of the world: http://drift.uninett.no/nett/ip-nett/dnsattributes.schema Was it ACK? I was not nacking, just pointing out. Tested with changed schema, works as expected. Here, have my ACK. Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0141] Generalize attribute_name-rdata_type conversions
On 14.5.2013 17:42, Tomas Babej wrote: On 05/14/2013 02:01 PM, Petr Spacek wrote: On 14.5.2013 11:45, Tomas Babej wrote: On 05/10/2013 04:57 PM, Petr Spacek wrote: On 6.5.2013 17:40, Tomas Hozza wrote: On 04/08/2013 07:45 PM, Petr Spacek wrote: Generalize attribute_name-rdata_type conversions. Attribute names are generated on-the-fly: String Record is appended to textual representation of DNS RDATA type. String Record is cut down from the attribute name during attribute name to rdata type conversion. From now, the plugin doesn't add artificial limitation to supported record types. ACK. The patch looks good. (I didn't do functional test) Cosmetic issue: I think it would be good to dynamically allocate mod_type in LDAPMod in every case and include the mod_type memory freeing in free_ldapmod() function. Now one has to be be careful when it is statically or dynamically allocated. Before it was static in every case. It is good idea. This version of the patch contains ldap_mod_create() function which allocates the whole structure including mod_type of fixed size. All writes to mod_type checks the array length, so it should not cause any harm. The function modify_soa_record() still uses statically allocated LDAPMod structure with statically allocated strings for mod_type, but the LDAPMod structure never leave this function. There are no calls to ldap_mod_create() and ldap_mod_free(), so I think it is obvious. Tbabej, please try to dynamically update some A records with sync_ptr enabled. (And of course the support for some new type, like TLSA.) For the existing record types, the patch works fine. For any new types, a schema change is still required, since record types are still hardcoded in LDAP schema: LDAP error: Object class violation: attribute tlsarecord not allowed That is expected behaviour. The point is that schema change is much simpler than recompiling the bind-dyndb-ldap (and can be done at run-time). BTW schema file contains instructions how to add support for any record type in a way compatible with rest of the world: http://drift.uninett.no/nett/ip-nett/dnsattributes.schema Was it ACK? I was not nacking, just pointing out. Tested with changed schema, works as expected. Here, have my ACK. Pushed to master: 35ac3e422376cc28040d03c7550c2c68a967a0cf -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0143] Treat syntax errors in LDAP filters as fatal
On 14.5.2013 11:47, Tomas Babej wrote: On 05/07/2013 10:16 AM, Tomas Hozza wrote: On 04/09/2013 03:39 PM, Petr Spacek wrote: Hello, Treat syntax errors in LDAP filters as fatal. Filters are hardcoded at the moment, this is preventive action. ACK. The patch looks good. (I didn't do functional test) Regards, Tomas Hozza ACK. It does not break anything :) Pushed to master: 33133d9eaf757aef520e334e7e2eacce959f929c -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0142] Improve LDAP error logging
On 14.5.2013 11:46, Tomas Babej wrote: On 05/07/2013 09:36 AM, Tomas Hozza wrote: On 04/09/2013 03:27 PM, Petr Spacek wrote: Hello, Improve LDAP error logging. Diagnostic error message is logged when it is available. Plugin with this patch produces messages like: LDAP error: Server is unwilling to perform: Minimum SSF not met.: bind to LDAP server failed intead of bind to LDAP server failed: Server is unwilling to perform Second example is: LDAP error: Object class violation: attribute mgrecord not allowed : while modifying(add) entry 'idnsName=pspacek, idnsname=example.com,cn=dns,dc=e,dc=test' instead of :-D snip diff --git a/src/log.h b/src/log.h index 312f24322fd0c6f9943c6beb810ac0bcd8f3896c..cbf1a3faaaccea7391d65d018e80d8ec688fc111 100644 --- a/src/log.h +++ b/src/log.h @@ -55,16 +55,30 @@ log_write(GET_LOG_LEVEL(level), format, ##__VA_ARGS__) /* LDAP logging functions */ -#define log_ldap_error(ld) \ - do { \ - int err; \ - char *errmsg = UNKNOWN; \ - if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, err) \ - == LDAP_OPT_SUCCESS) \ - errmsg = ldap_err2string(err); \ - log_error_position(LDAP error: %s, errmsg); \ - } while (0); \ +#define LOG_LDAP_ERR_PREFIX LDAP error: +#define log_ldap_error(ld, desc, ...) \ + do { \ + int err; \ + char *errmsg = NULL; \ + char *diagmsg = NULL; \ + if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, err) \ + == LDAP_OPT_SUCCESS) { \ + errmsg = ldap_err2string(err); \ Getting error msg for the first time here. + if (ldap_get_option(ld, LDAP_OPT_DIAGNOSTIC_MESSAGE, diagmsg) \ + == LDAP_OPT_SUCCESS diagmsg != NULL) { \ + errmsg = ldap_err2string(err);\ Again getting error msg with the same err. Maybe a copy-paste error? + log_error(LOG_LDAP_ERR_PREFIX %s: %s: desc, \ + errmsg, diagmsg, ##__VA_ARGS__); \ + ldap_memfree(diagmsg); \ + } else \ + log_error(LOG_LDAP_ERR_PREFIX %s: desc, \ + errmsg, ##__VA_ARGS__); \ + } else { \ + log_error(LOG_LDAP_ERR_PREFIX \ + unable to obtain LDAP error code: \ + desc, ##__VA_ARGS__); \ + } \ + } while (0); void log_write(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3); Regards, Tomas Hozza ACK, provides the desired info. Pushed to master: af83758cb3f91129399494c95a1847814b1d71a8 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0153] Update NEWS file for upcoming 3.2 release
On 14.5.2013 16:36, Petr Spacek wrote: On 14.5.2013 16:20, Petr Spacek wrote: Update NEWS file for upcoming 3.2 release. Pushed to master: bc647206677f54ecf14eb7f77984ec25070ddfc2 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0154] Bump NVR to 3.2
On 14.5.2013 16:37, Petr Spacek wrote: On 14.5.2013 16:20, Petr Spacek wrote: Bump NVR to 3.2. Pushed to master: eebb0679be611e325ce6c2938ac6c8ca5e734ea6 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Build fixes for F19 for bind-dyndb-ldap
Hi, This patch contains various (gcc-related) fixes to make build on F19 possible. Tomas From 4374791563602f026b870b0c37979506f02683ae Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Mon, 6 May 2013 07:23:08 -0400 Subject: [PATCH] Build fixes for Fedora 19 This patch contains various (gcc-related) fixes to make build on F19 possible. --- src/acl.c | 4 ++-- src/ldap_helper.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/acl.c b/src/acl.c index 754cd53dc3d31b99d0954836feafbd46747c48c2..2d4f54db89c73b7e9387f21bbd94eedc364c152b 100644 --- a/src/acl.c +++ b/src/acl.c @@ -179,7 +179,7 @@ parse(cfg_parser_t *parser, const char *string, cfg_type_t **type, RUNTIME_CHECK(isc_once_do(once, init_cfgtypes) == ISC_R_SUCCESS); string_len = strlen(string); - isc_buffer_init(buffer, string, string_len); + isc_buffer_init(buffer, (char *)string, string_len); isc_buffer_add(buffer, string_len); result = cfg_parse_buffer(parser, buffer, *type, ret); @@ -296,7 +296,7 @@ get_fixed_name(const cfg_obj_t *obj, const char *name, dns_fixedname_t *fname) str = cfg_obj_asstring(obj); len = strlen(str); - isc_buffer_init(buf, str, len); + isc_buffer_init(buf, (char *)str, len); /* * Workaround for https://bugzilla.redhat.com/show_bug.cgi?id=728925 diff --git a/src/ldap_helper.c b/src/ldap_helper.c index 037629f3e50fdba2c17c1a180736ed3b64169aae..eda17fa8fed63e43d702c7ad9984b176e0b27065 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -2037,7 +2037,7 @@ parse_rdata(isc_mem_t *mctx, ldap_qresult_t *qresult, text.base = rdata_text; text.length = strlen(text.base); - isc_buffer_init(lex_buffer, text.base, text.length); + isc_buffer_init(lex_buffer, (char *)text.base, text.length); isc_buffer_add(lex_buffer, text.length); isc_buffer_setactive(lex_buffer, text.length); -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Build fixes for F19 for bind-dyndb-ldap
On 14.5.2013 18:16, Tomas Babej wrote: This patch contains various (gcc-related) fixes to make build on F19 possible. Congratulations to your first patch for bind-dyndb-ldap! :-) ACK Pushed to master: c5716e660e145ebec6ff091e2479f93e4c54d464 -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 413 Fix: HBAC Test tab is missing
Fix for really stupid mistake with quite big impact... Caused by typo in metadata provider source path. No metadata - no HBAC test entity - no tab https://fedorahosted.org/freeipa/ticket/3627 -- Petr Vobornik From 5b1fc24ae1e28ce8a775fb7923cedfba397a210f Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Tue, 14 May 2013 18:28:49 +0200 Subject: [PATCH] Fix: HBAC Test tab is missing Caused by typo in metadata provider source path. No metadata - no HBAC test entity - no tab https://fedorahosted.org/freeipa/ticket/3627 --- install/ui/src/freeipa/_base/metadata_provider.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/ui/src/freeipa/_base/metadata_provider.js b/install/ui/src/freeipa/_base/metadata_provider.js index fcb072395c1cf92982cc1bec66df045bb0a94abe..9a332b55600bfe0189733987edf4e1aa50f6d72c 100644 --- a/install/ui/src/freeipa/_base/metadata_provider.js +++ b/install/ui/src/freeipa/_base/metadata_provider.js @@ -32,7 +32,7 @@ define(['dojo/_base/lang', './Provider', './Search_provider'], var commmads = new Provider({ code: '@mc:', source: metadata, -path: 'commmads' +path: 'commands' }); var object_param = new Search_provider({ code: '@mo-param:', -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 407 Set KRB5CCNAME so that dirsrv can work with newer krb5-server
The DIR ccache format is now the default in krb5-server 1.11.2-4 but /run/user/uid isn't created for Apache by anything so it has no ccache (and it doesn't have SELinux permissions to write here either). Use KRB5CCNAME to set a file path instead in /etc/sysconfig/dirsrv. https://fedorahosted.org/freeipa/ticket/3628 Without this patch, replication on F19 is broken. Martin From 1be93108c4c1506ea50879d645c47ab6843a6ee1 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Tue, 14 May 2013 18:36:50 +0200 Subject: [PATCH] Set KRB5CCNAME so that dirsrv can work with newer krb5-server The DIR ccache format is now the default in krb5-server 1.11.2-4 but /run/user/uid isn't created for Apache by anything so it has no ccache (and it doesn't have SELinux permissions to write here either). Use KRB5CCNAME to set a file path instead in /etc/sysconfig/dirsrv. https://fedorahosted.org/freeipa/ticket/3628 --- install/tools/ipa-upgradeconfig | 1 + ipaserver/install/dsinstance.py | 18 ++ 2 files changed, 19 insertions(+) diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig index 8fa9b189a2dc207e2d90ab32131e65fac0f1f9e0..8e9357f20fe7c9a88908def6a2e3b2104f07d73a 100644 --- a/install/tools/ipa-upgradeconfig +++ b/install/tools/ipa-upgradeconfig @@ -919,6 +919,7 @@ def main(): http.configure_httpd_ccache() ds = dsinstance.DsInstance() +ds.configure_dirsrv_ccache() fix_schema_file_syntax(ds) diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index e6bb054ddad4a0d91d76d4c79eb477913e8776aa..3b841417e717587675d3ac748ec02182b3e14672 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -26,6 +26,7 @@ import time import tempfile import base64 +import stat from ipapython.ipa_log_manager import * from ipapython import ipautil, sysrestore, dogtag, ipaldap @@ -213,6 +214,7 @@ def __common_setup(self, enable_ssl=False): self.step(configuring certmap.conf, self.__certmap_conf) self.step(configure autobind for root, self.__root_autobind) self.step(configure new location for managed entries, self.__repoint_managed_entries) +self.step(configure dirsrv ccache, self.configure_dirsrv_ccache) self.step(restarting directory server, self.__restart_instance) def __common_post_setup(self): @@ -515,6 +517,22 @@ def __config_lockout_module(self): def __repoint_managed_entries(self): self._ldap_mod(repoint-managed-entries.ldif, self.sub_dict) +def configure_dirsrv_ccache(self): +pent = pwd.getpwnam(dirsrv) +ccache = '/tmp/krb5cc_%d' % pent.pw_uid +filepath = '/etc/sysconfig/dirsrv' +if not os.path.exists(filepath): +# file doesn't exist; create it with correct ownership mode +open(filepath, 'a').close() +os.chmod(filepath, +stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH) +os.chown(filepath, 0, 0) + +replacevars = {'KRB5CCNAME': ccache} +old_values = ipautil.backup_config_and_replace_variables( +self.fstore, filepath, replacevars=replacevars) +ipaservices.restore_context(filepath) + def __managed_entries(self): self._ldap_mod(managed-entries.ldif, self.sub_dict) -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 413 Fix: HBAC Test tab is missing
On 05/14/2013 06:37 PM, Petr Vobornik wrote: Fix for really stupid mistake with quite big impact... Caused by typo in metadata provider source path. No metadata - no HBAC test entity - no tab https://fedorahosted.org/freeipa/ticket/3627 ACK. Pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 225 Remove leading zero from IPA_NUM_VERSION
On 05/13/2013 03:40 PM, Petr Viktorin wrote: On 05/13/2013 02:58 PM, Rob Crittenden wrote: Petr Viktorin wrote: Due to my mistake in commit 9125285, the IPA_NUM_VERSION variable contained a leading zero, so it was treated as octal in Python code. Bumping the development version to 3.2.99 resulted in 030299, an invalid value. Luckily, 320 030200 30200 so the ordering is not broken. Sorry for the mistake. I think at least a comment somewhere is required too, probably in version.h, describing which versions were affected by this octal problem. rob Added. This helps, thanks. ACK, 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] Final OTP Review
On 05/14/2013 03:53 PM, Nathaniel McCallum wrote: On Fri, 2013-05-10 at 16:55 -0400, Nathaniel McCallum wrote: On Fri, 2013-05-03 at 09:08 -0400, Simo Sorce wrote: On Thu, 2013-05-02 at 23:39 -0700, Nathan Kinder wrote: On 05/02/2013 10:27 PM, Nathaniel McCallum wrote: All issues fixed unless noted below... The attached patches are tested to work. On Thu, 2013-05-02 at 17:39 -0400, Simo Sorce wrote: - (nit) slapi_ch_malloc/slapi_ch_strdup are not checked for failure (although I know slapi_ch_malloc() currently just aborts on failure, I think that is plain wrong which is why I would prefer using malloc/strdup, but well, I guess this is not something I am going to care too much about for now). Not fixed. - Is the logic with auth_type_used correct ? At least the name of the variable sounds misleading, because you set it only if the authentication was successful but not set if it was 'used' but was unsuccessful. Made me look at it multiple times to reconstuct the logic. The var name could be better, however I also want a comment that explain exactly how we are going to manage authentication and fallbacks here as that logic is critical and is useful for anyone that is going to have to change this code later in order not to break it. The variable is now gone. I re-factored the section to make the logic clearer and put a nice big comment up top. - General question: how does this PRE_BIND plugin interact with ipapwd_pre_bind() in the ipa-pwd-extop() plugin ? Are you going to cause that plugin not to run by returning a result directly in this function ? Or is this plugin configured to run only after the previous one went through ? I ask because I do not see any ordering information in the cn=config plugin configuration so it is not immediately clear to me. That is a good question for Nathan since he wrote this part of the code... We would need to set the precedence if you want a predictable/guaranteed execution order. If a pre-BIND plug-in callback returns non-zero (which you should do when the plug-in sends the result to the client directly), it will cause other pre-bind plug-ins to not be called. This is actually how all pre-op plug-ins work. If a pre-op callback returns an error, we don't call the rest of the pre-op plug-ins in the list. Ok, but this does not answer my question. We definitely need to *always* run our other preop plugin as we do sanity checks like verifying if the user is enabled/disabled etc... Also we need to understand how to deal with migrating password auth when OTP is enabeld. TBH I think we should not have a separate OTP-auth plugin but we should probably have a single plugin that handles authentication and the 2 should be merged. Keeping them separate is going to cause more harm than good with unexpected interactions. We could still have 2 plugins and simply move the prebind action currently don in ipa-pwd-extop to the otp plugin by making some more code common. But it is probably easier to just merge OTP into ipa-pwd-extop right now than try to do a huge refactoring. We can always refactor the ipa-pwd-extop plugin later. The attached patches encompass an initial review of the companion daemon and merge of ipa-otp into ipa-pwd-extop. Unfortunately, merging ipa-otp into ipa-pwd-extop appears to have broken something during install, but I don't have enough familiarity with 389 to understand what I've broken. If I upgrade after an install, it appears to work. An RPM with the patches is available here: http://koji.fedoraproject.org/koji/taskinfo?taskID=5362935 Nathan / Rob / Simo, could you take a look and see what might be broken in ipa-pwd-extop? While I'm not quite sure what the problem was, I do know it appeared on the stock 3.2 F19 RPMs. I also fixed it by accident. I am certain it is unrelated to these patches. I have now tested install and upgrade with the six patches in the previous email and everything is in order, including permissions. At this point, we just need reviews/ACKs. Nathaniel I tested IPA server upgrades, new installs and also adding 3.2+OTP replica for F18 3.1.4 IPA master. Everything seemed to work fine (when I added my patch 407 fixing the replication), I did not see any breakage. Issues I found with too much logging I reported should now be fixed on github, so this should be OK. So it is an ACK from my side if Rob does not discover some blocking issue. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 407 Set KRB5CCNAME so that dirsrv can work with newer krb5-server
Martin Kosek wrote: The DIR ccache format is now the default in krb5-server 1.11.2-4 but /run/user/uid isn't created for Apache by anything so it has no ccache (and it doesn't have SELinux permissions to write here either). Use KRB5CCNAME to set a file path instead in /etc/sysconfig/dirsrv. https://fedorahosted.org/freeipa/ticket/3628 Without this patch, replication on F19 is broken. Martin ACK, pushed to master and ipa-3-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel