[Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.

[Petr Spacek]

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