[Freeipa-devel] discussion needed: Support for IPv6 elements in idnsForwarders attribute

Adam Tkac atkac at redhat.com
Mon Mar 5 13:03:53 UTC 2012


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

-- 
Adam Tkac, Red Hat, Inc.




More information about the Freeipa-devel mailing list