Re: [Freeipa-devel] [PATCH] discussion needed: 0009 Support for IPv6 elements in idnsForwarders attribute
And there is the patch, sorry.
Petr^2
On 02/29/2012 03:10 PM, Petr Spacek wrote:
Hello,
this patch fixes https://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
:::1.2.3.4 (6to4, without optional port)
:::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 215e600a17c51fdb1a4a7fc667a0b62673579797 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Wed, 29 Feb 2012 14:29:35 +0100
Subject: [PATCH] Add support for IPv6 elements in idnsForwarders attribute
Fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/49
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 109 ++--
1 files changed, 71 insertions(+), 38 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d9e8629..7ebe590 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -777,22 +777,24 @@ cleanup:
/**
- * @brief
+ * @brief Convert IP address from string to sockaddr.
*
- * @param nameserver
- * @param sa
+ * Only AF_INET and AF_INET6 are supported.
*
- * @return
+ * @param[in] nameserver IP address as string
+ * @param[out] ss sockaddr with obtained IP address
+ *
+ * @return ISC_R_SUCCESS, ISC_R_FAILURE (ss untouched)
*/
static isc_result_t
-sockaddr_fromchar(char *nameserver, struct sockaddr *sa)
+sockaddr_fromchar(const char *nameserver, struct sockaddr_storage *ss)
{
isc_result_t result = ISC_R_FAILURE;
struct addrinfo *ai;
struct addrinfo hints;
int res;
- REQUIRE(sa != NULL);
+ REQUIRE(ss != NULL);
memset(hints, 0, sizeof(hints));
hints.ai_flags = AI_NUMERICHOST;
@@ -800,8 +802,11 @@ sockaddr_fromchar(char *nameserver, struct sockaddr *sa)
res = getaddrinfo(nameserver, NULL, hints, ai);
if (res == 0) {
if ((ai-ai_family == AF_INET) || (ai-ai_family == AF_INET6)) {
- memcpy(sa, ai-ai_addr, ai-ai_addrlen);
+ memcpy(ss, ai-ai_addr, ai-ai_addrlen);
result = ISC_R_SUCCESS;
+ } else {
+ log_bug(Unknown ai_family type);
+ return ISC_R_FAMILYNOSUPPORT;
}
freeaddrinfo(ai);
}
@@ -809,29 +814,37 @@ sockaddr_fromchar(char *nameserver, struct sockaddr *sa)
}
/**
- * Parse nameserver IP address with or without
- * port separated with a dot.
+ * Parse nameserver IP address with or without port separated with a dot.
+ *
+ * IPv4 and IPv6 addresses are supported.
+ *
+ * @param[in] token String with IP address and optionally port.
+ * @param[out] ss Socket address storage structure.
*
* @example
- * 192.168.0.100.53 - { address:192.168.0.100, port:53 }
+ * 192.168.0.100 - { address:192.168.0.100, port:53 }
+ * 192.168.0.100.553 - { address:192.168.0.100, port:553 }
+ * 0102:0304:0506:0708:090A:0B0C:0D0E:0FAA -
+ * { address:0102:0304:0506:0708:090A:0B0C:0D0E:0FAA, port:53 }
+ * 0102:0304:0506:0708:090A:0B0C:0D0E:0FAA.553 -
+ * { address:0102:0304:0506:0708:090A:0B0C:0D0E:0FAA, port:553 }
*
- * @param token
- * @param sa Socket address structure.
*/
static isc_result_t
-parse_nameserver(char *token, struct sockaddr *sa)
+parse_nameserver(char *token, struct sockaddr_storage *ss)
{
isc_result_t result = ISC_R_FAILURE;
char *dot;
long number;
REQUIRE(token != NULL);
- REQUIRE(sa != NULL);
+ REQUIRE(ss != NULL);
- result = sockaddr_fromchar(token, sa);
- if (result == ISC_R_SUCCESS)
+ result = sockaddr_fromchar(token, ss);
+ if (result == ISC_R_SUCCESS || result == ISC_R_FAMILYNOSUPPORT)
return result;
+ /* Address parsing failed, try to extract port and retry. */
dot = strrchr(token, '.');
if (dot == NULL)
return ISC_R_FAILURE;
@@ -841,42 +854,51 @@ parse_nameserver(char *token, struct sockaddr *sa)
return ISC_R_FAILURE;
*dot = '\0';
- result = sockaddr_fromchar(token, sa);
+ result = sockaddr_fromchar(token, ss);
*dot = '.'; /* restore value */
if (result == ISC_R_SUCCESS) {
in_port_t port = htons(number);
- switch (sa-sa_family) {
+ switch (ss-ss_family) {
case AF_INET :
- ((struct sockaddr_in *)sa)-sin_port = port;
+ ((struct sockaddr_in *)ss)-sin_port = port;
break;
case AF_INET6 :
- ((struct sockaddr_in6 *)sa)-sin6_port = port;
+ ((struct sockaddr_in6 *)ss)-sin6_port = port;
break;
default:
- log_bug(Unknown sa_family type);
- return ISC_R_FAILURE;
+ log_bug(Unknown ss_family type);
+
Re: [Freeipa-devel] [PATCH] discussion needed: 0009 Support for IPv6 elements in idnsForwarders attribute
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. 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 fixes https://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 :::1.2.3.4 (6to4, without optional port) :::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? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- 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] discussion needed: 0009 Support for IPv6 elements in idnsForwarders attribute
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 fixes https://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 :::1.2.3.4 (6to4, without optional port) :::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? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] discussion needed: 0009 Support for IPv6 elements in idnsForwarders attribute
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 fixes https://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 :::1.2.3.4 (6to4, without optional port) :::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? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
