[Freeipa-devel] discussion needed: Support for IPv6 elements in idnsForwarders attribute
Petr Spacek
pspacek at redhat.com
Mon Mar 5 13:22:49 UTC 2012
On 03/05/2012 02:03 PM, Adam Tkac wrote:
> On Mon, Mar 05, 2012 at 01:56:14PM +0100, Petr Spacek wrote:
>> Hello,
>>
>> we are back with another proposal from Adam. See last lines.
>
> Hello,
>
> reply is below...
>
>>
>> On 03/05/2012 12:32 PM, Adam Tkac wrote:
>>> On Thu, Mar 01, 2012 at 07:55:33PM +0100, Petr Spacek wrote:
>>>>> Hello,
>>>>>
>>>>> here is (again) reworked patch for
>>>>> https://fedorahosted.org/bind-dyndb-ldap/ticket/49 .
>>>>>
>>>>> Adam pointed me to existing BIND parser, which I missed. Now is all
>>>>> parsing& socket magic done inside BIND libraries. Our code is a bit
>>>>> shorter and syntax is 100% BIND-compatible. (But it means same as
>>>>> discussed yesterday:-)
>>>>>
>>>>> Adam, please review it.
>>> Thanks for the patch Petr, see my comments below.
>>>
>>>>> Petr^2 Spacek
>>>>>
>>>>> On 03/01/2012 03:22 PM, Petr Spacek wrote:
>>>>>> >Hello,
>>>>>> >
>>>>>> >here is reworked patch for
>>>>>> >https://fedorahosted.org/bind-dyndb-ldap/ticket/49 .
>>>>>> >
>>>>>> >Changes after yesterday's discussion on IRC with Simo and Mkosek:
>>>>>> >It follows BIND9 syntax for optional specification of port& adds
>>>>>> >documentation for this new syntax.
>>>>>> >
>>>>>> >Petr^2 Spacek
>>>>>> >
>>>>>> >On 02/29/2012 05:33 PM, Martin Kosek wrote:
>>>>>>> >>I agree that we should keep the BIND syntax and separate port and IP
>>>>>>> >>address with a space. We will at least avoid possible issues with IP
>>>>>>> >>address decoding in the future.
>>>>>>> >>
>>>>>>> >>Since this is a new attribute we have a good chance to do changes now so
>>>>>>> >>that it is used correctly. I created an upstream ticket to change the
>>>>>>> >>behavior and validators in FreeIPA:
>>>>>>> >>
>>>>>>> >>https://fedorahosted.org/freeipa/ticket/2462
>>>>>>> >>
>>>>>>> >>Martin
>>>>>>> >>
>>>>>>> >>On Wed, 2012-02-29 at 16:44 +0100, Petr Spacek wrote:
>>>>>>>> >>>On 02/29/2012 04:30 PM, Simo Sorce wrote:
>>>>>>>>> >>>>Either way looks ok to me.
>>>>>>>>> >>>>I agree that using a space may be less confusing if this syntax never
>>>>>>>>> >>>>allows to specify multiple addresses.
>>>>>>>>> >>>>If multiple address can be specified than it may be less ideal to use
>>>>>>>>> >>>>spaces.
>>>>>>>>> >>>>
>>>>>>>>> >>>>Simo.
>>>>>>>> >>>
>>>>>>>> >>>idnsForwarders is multi-value attribute, so each value contain single
>>>>>>>> >>>forwarder address.
>>>>>>>> >>>
>>>>>>>> >>>Petr^2 Spacek
>>>>>>>> >>>
>>>>>>>>> >>>>On Wed, 2012-02-29 at 15:14 +0100, Petr Spacek wrote:
>>>>>>>>>> >>>>>And there is the patch, sorry.
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>>Petr^2
>>>>>>>>>> >>>>>
>>>>>>>>>> >>>>>On 02/29/2012 03:10 PM, Petr Spacek wrote:
>>>>>>>>>>> >>>>>>Hello,
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>this patch fixeshttps://fedorahosted.org/bind-dyndb-ldap/ticket/49 ,
>>>>>>>>>>> >>>>>>but I want to discuss one (unimplemented) change:
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>I propose a change in (currently very strange) forwarders syntax.
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>Current syntax:
>>>>>>>>>>> >>>>>><IP>[.port]
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>examples:
>>>>>>>>>>> >>>>>>1.2.3.4 (without optional port)
>>>>>>>>>>> >>>>>>1.2.3.4.5553 (optional port 5553)
>>>>>>>>>>> >>>>>>A::B (IPv6, without optional port)
>>>>>>>>>>> >>>>>>A::B.5553
>>>>>>>>>>> >>>>>>::FFFF:1.2.3.4 (6to4, without optional port)
>>>>>>>>>>> >>>>>>::FFFF:1.2.3.4.5553 (6to4, with optional port 5553)
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>I find this syntax confusing, non-standard and not-typo-proof.
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>IMHO better choice is to follow BIND forwarders syntax:
>>>>>>>>>>> >>>>>><IP> [port ip_port] (port is string delimited with spaces)
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>(From:http://www.zytrax.com/books/dns/ch7/queries.html#forwarders)
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>*Current syntax is not documented*, so probably is not used anywhere.
>>>>>>>>>>> >>>>>>(And DNS server on non-standard port is probably useful only for
>>>>>>>>>>> >>>>>>testing
>>>>>>>>>>> >>>>>>purposes, but it's another story.)
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>
>>>>>>>>>>> >>>>>>What is you opinion?
>>>>> From 14056200915a90c2a957e8a2219819bd3b7f9365 Mon Sep 17 00:00:00 2001
>>>>> From: Petr Spacek<pspacek at redhat.com>
>>>>> Date: Thu, 1 Mar 2012 15:08:10 +0100
>>>>> Subject: [PATCH] Add support for IPv6 elements in idnsForwarders attribute
>>>>> and make syntax compatible with BIND9 forwarders.
>>>>> Signed-off-by: Petr Spacek<pspacek at redhat.com>
>>>>>
>>>>> ---
>>>>> NEWS | 3 +
>>>>> README | 8 ++-
>>>>> src/acl.c | 89 ++++++++++++++++++++++++++++++++++
>>>>> src/acl.h | 3 +
>>>>> src/ldap_helper.c | 136 ++++++-----------------------------------------------
>>>>> 5 files changed, 116 insertions(+), 123 deletions(-)
>>>>>
>>>>> diff --git a/NEWS b/NEWS
>>>>> index ced6250..a97a5f3 100644
>>>>> --- a/NEWS
>>>>> +++ b/NEWS
>>>>> @@ -1,3 +1,6 @@
>>>>> +[1] Add support for IPv6 elements in idnsForwarders attribute
>>>>> + and make syntax compatible with BIND9 forwarders.
>>>>> +
>>>>> 1.1.0a2
>>>>> ======
>>>>>
>>>>> diff --git a/README b/README
>>>>> index 914abdc..aedb88c 100644
>>>>> --- a/README
>>>>> +++ b/README
>>>>> @@ -89,8 +89,12 @@ example zone ldif is available in the doc directory.
>>>>> with a valid idnsForwarders attribute.
>>>>>
>>>>> * idnsForwarders
>>>>> - Defines multiple IP addresses (TODO: optional port numbers) to which
>>>>> - queries will be forwarded.
>>>>> + Defines multiple IP addresses to which queries will be forwarded.
>>>>> + It is multi-value attribute: Each IP address (and optional port) has to
>>>>> + be in own value. BIND9 syntax for "forwarders" is required.
>>>>> + Optional port can be specified by adding " port<number>" after IP
>>>>> + address. IPv4 and IPv6 addresses are supported.
>>>>> + Examples: "1.2.3.4" or "1.2.3.4 port 553" or "A::B" or "A::B port 553"
>>
>> On 03/05/2012 12:32 PM, Adam Tkac wrote:
>>> In my opinion we should mark the idnsForwarders attribute single-value and deal
>>> with them as with idnsAllow{Query,Transfer} (i.e. specify forwarders splitted by
>>> semi-colon). It will be more consistent with idnsAllow{Query,Transfer}. What do
>>> you think about this?
>>
>> It sounds good to me. It's more consistent with real BIND
>> configuration and other parts of plugin.
>>
>> What about upgrade process? It is necessary to change schema and
>> convert existing entries to new format.
>
> If idnsForward is already in use, I'm ok with multival attribute because
> ordering is not an issue here.
>
> Please incorporate changes which I suggested (if you don't dissagree, of course)
> and then push the patch. Thank you in advance!
>
> Regards, Adam
>
I did proposed "cosmetic" changes. Multi-value version pushed to master.
Latest patch with two cosmetic changes is attached.
https://fedorahosted.org/bind-dyndb-ldap/changeset/e114f8e2721e89a6ed84439903cc739c5f8d293a
Thanks to Adam and Simo for discussion.
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bind-dyndb-ldap-pspacek-0009-Add-support-for-IPv6-elements-in-idnsForwarders-attr.patch
Type: text/x-patch
Size: 10060 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120305/10bf2629/attachment.bin>
More information about the Freeipa-devel
mailing list