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<pspa...@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<pspa...@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
>From e114f8e2721e89a6ed84439903cc739c5f8d293a Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@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 <pspa...@redhat.com>
---
NEWS | 3 +
README | 8 ++-
src/acl.c | 86 +++++++++++++++++++++++++++++++++
src/acl.h | 3 +
src/ldap_helper.c | 136 ++++++-----------------------------------------------
5 files changed, 113 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"
5. Configuration
================
diff --git a/src/acl.c b/src/acl.c
index 32ca702..1ce2eac 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -69,6 +69,7 @@ static isc_once_t once = ISC_ONCE_INIT;
static cfg_type_t *update_policy;
static cfg_type_t *allow_query;
static cfg_type_t *allow_transfer;
+static cfg_type_t *forwarders;
static cfg_type_t *
get_type_from_tuplefield(const cfg_type_t *cfg_type, const char *name)
@@ -139,6 +140,7 @@ init_cfgtypes(void)
update_policy = get_type_from_clause_array(zoneopts, "update-policy");
allow_query = get_type_from_clause_array(zoneopts, "allow-query");
allow_transfer = get_type_from_clause_array(zoneopts, "allow-transfer");
+ forwarders = get_type_from_clause_array(zoneopts, "forwarders");
}
static isc_result_t
@@ -351,6 +353,24 @@ cleanup:
return result;
}
+static isc_result_t
+semicolon_bracket_str(isc_mem_t *mctx, const char *str, ld_string_t **bracket_strp)
+{
+ ld_string_t *tmp = NULL;
+ isc_result_t result;
+
+ CHECK(str_new(mctx, &tmp));
+ CHECK(str_sprintf(tmp, "{ %s; }", str));
+
+ *bracket_strp = tmp;
+
+ return ISC_R_SUCCESS;
+
+cleanup:
+ str_destroy(&tmp);
+ return result;
+}
+
isc_result_t
acl_configure_zone_ssutable(const char *policy_str, dns_zone_t *zone)
{
@@ -480,3 +500,69 @@ cleanup:
return result;
}
+
+/**
+ * Parse nameserver IP address with or without port specified in BIND9 syntax.
+ * IPv4 and IPv6 addresses are supported, see examples.
+ *
+ * @param[in] forwarder_str String with IP address and optionally port.
+ * @param[in] mctx Memory for allocating temporary and sa structure.
+ * @param[out] sa Socket address structure.
+ *
+ * @return Pointer to newly allocated isc_sockaddr_t structure.
+ *
+ * @example
+ * "192.168.0.100" -> { address:192.168.0.100, port:53 }
+ * "192.168.0.100 port 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 port 553" ->
+ * { address:0102:0304:0506:0708:090A:0B0C:0D0E:0FAA, port:553 }
+ *
+ */
+isc_result_t
+acl_parse_forwarder(const char *forwarder_str, isc_mem_t *mctx, isc_sockaddr_t **sa)
+{
+ isc_result_t result = ISC_R_SUCCESS;
+ cfg_parser_t *parser = NULL;
+
+ cfg_obj_t *forwarders_cfg = NULL;
+ ld_string_t *new_forwarder_str = NULL;
+ const cfg_obj_t *faddresses;
+ const cfg_listelt_t *element;
+
+ in_port_t port = 53;
+
+ REQUIRE(forwarder_str != NULL);
+ REQUIRE(sa != NULL && *sa == NULL);
+
+ /* add semicolon and brackets as necessary for parser */
+ if (!index(forwarder_str, ';'))
+ CHECK(semicolon_bracket_str(mctx, forwarder_str, &new_forwarder_str));
+ else
+ CHECK(bracket_str(mctx, forwarder_str, &new_forwarder_str));
+
+ CHECK(cfg_parser_create(mctx, dns_lctx, &parser));
+ CHECK(parse(parser, str_buf(new_forwarder_str), &forwarders, &forwarders_cfg));
+
+ faddresses = cfg_tuple_get(forwarders_cfg, "addresses");
+ element = cfg_list_first(faddresses);
+ if (!element) {
+ result = ISC_R_FAILURE;
+ goto cleanup;
+ }
+
+ const cfg_obj_t *forwarder = cfg_listelt_value(element);
+ *sa = isc_mem_get(mctx, sizeof(isc_sockaddr_t));
+ if (*sa == NULL) {
+ result = ISC_R_NOMEMORY;
+ goto cleanup;
+ }
+ **sa = *cfg_obj_assockaddr(forwarder);
+ if (isc_sockaddr_getport(*sa) == 0)
+ isc_sockaddr_setport(*sa, port);
+
+cleanup:
+ str_destroy(&new_forwarder_str);
+ return result;
+}
diff --git a/src/acl.h b/src/acl.h
index d0ab216..7e4471b 100644
--- a/src/acl.h
+++ b/src/acl.h
@@ -42,4 +42,7 @@ acl_from_ldap(isc_mem_t *mctx, const char *aclstr, acl_type_t type,
* Please refer to BIND 9 ARM (Administrator Reference Manual) about ACLs.
*/
+isc_result_t
+acl_parse_forwarder(const char *forwarders_str, isc_mem_t *mctx, isc_sockaddr_t **sa);
+
#endif /* !_LD_ACL_H_ */
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d9e8629..21c3690 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -776,108 +776,6 @@ cleanup:
}
-/**
- * @brief
- *
- * @param nameserver
- * @param sa
- *
- * @return
- */
-static isc_result_t
-sockaddr_fromchar(char *nameserver, struct sockaddr *sa)
-{
- isc_result_t result = ISC_R_FAILURE;
- struct addrinfo *ai;
- struct addrinfo hints;
- int res;
-
- REQUIRE(sa != NULL);
-
- memset(&hints, 0, sizeof(hints));
- hints.ai_flags = AI_NUMERICHOST;
-
- 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);
- result = ISC_R_SUCCESS;
- }
- freeaddrinfo(ai);
- }
- return result;
-}
-
-/**
- * Parse nameserver IP address with or without
- * port separated with a dot.
- *
- * @example
- * "192.168.0.100.53" -> { address:192.168.0.100, port:53 }
- *
- * @param token
- * @param sa Socket address structure.
- */
-static isc_result_t
-parse_nameserver(char *token, struct sockaddr *sa)
-{
- isc_result_t result = ISC_R_FAILURE;
- char *dot;
- long number;
-
- REQUIRE(token != NULL);
- REQUIRE(sa != NULL);
-
- result = sockaddr_fromchar(token, sa);
- if (result == ISC_R_SUCCESS)
- return result;
-
- dot = strrchr(token, '.');
- if (dot == NULL)
- return ISC_R_FAILURE;
-
- number = strtol(dot + 1, NULL, 10);
- if ((number < 0) || (number > UINT16_MAX))
- return ISC_R_FAILURE;
-
- *dot = '\0';
- result = sockaddr_fromchar(token, sa);
- *dot = '.'; /* restore value */
- if (result == ISC_R_SUCCESS) {
- in_port_t port = htons(number);
- switch (sa->sa_family) {
- case AF_INET :
- ((struct sockaddr_in *)sa)->sin_port = port;
- break;
- case AF_INET6 :
- ((struct sockaddr_in6 *)sa)->sin6_port = port;
- break;
- default:
- log_bug("Unknown sa_family type");
- return ISC_R_FAILURE;
- }
- }
- return result;
-}
-
-/* TODO: Code hardering. */
-static void *
-get_in_addr(struct sockaddr *sa)
-{
- if (sa->sa_family == AF_INET)
- return &(((struct sockaddr_in*)sa)->sin_addr);
- else
- return &(((struct sockaddr_in6*)sa)->sin6_addr);
-}
-
-static in_port_t
-get_in_port(struct sockaddr *sa)
-{
- if (sa->sa_family == AF_INET)
- return (((struct sockaddr_in*)sa)->sin_port);
- else
- return (((struct sockaddr_in6*)sa)->sin6_port);
-}
static isc_result_t
configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
@@ -887,7 +785,6 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
isc_result_t result;
ldap_value_t *value;
isc_sockaddrlist_t addrs;
- isc_sockaddr_t *addr;
REQUIRE(entry != NULL && inst != NULL && name != NULL && values != NULL);
@@ -905,28 +802,19 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
ISC_LIST_INIT(addrs);
for (value = HEAD(*values); value != NULL; value = NEXT(value, link)) {
- struct sockaddr sa;
-
- addr = isc_mem_get(inst->mctx, sizeof(*addr));
- if (addr == NULL) {
- result = ISC_R_NOMEMORY;
- goto cleanup;
- }
- ISC_LINK_INIT(addr, link);
+ isc_sockaddr_t *addr = NULL;
+ char forwarder_txt[ISC_SOCKADDR_FORMATSIZE];
- result = parse_nameserver(value->value, &sa);
- if (result != ISC_R_SUCCESS) {
- log_bug("Could not convert IP address "
- "from string '%s'.", value->value);
+ if( acl_parse_forwarder(value->value, inst->mctx, &addr)
+ != ISC_R_SUCCESS) {
+ log_error("Could not parse forwarder '%s'", value->value);
+ continue;
}
- /* Convert port from network byte order. */
- in_port_t port = ntohs(get_in_port(&sa));
- port = (port != 0) ? port : 53; /* use well known port */
-
- isc_sockaddr_fromin(addr, get_in_addr(&sa), port);
+ ISC_LINK_INIT(addr, link);
ISC_LIST_APPEND(addrs, addr, link);
- log_debug(5, "Adding forwarder %s (:%d) for %s", value->value, port, dn);
+ isc_sockaddr_format(addr, forwarder_txt, ISC_SOCKADDR_FORMATSIZE);
+ log_debug(5, "Adding forwarder %s for %s", forwarder_txt, dn);
}
/*
@@ -944,6 +832,11 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
fwdpolicy = dns_fwdpolicy_only;
}
+ if (ISC_LIST_EMPTY(addrs)) {
+ log_debug(5, "No forwarders seen; disabling forwarding");
+ fwdpolicy = dns_fwdpolicy_none;
+ }
+
log_debug(5, "Forward policy: %d", fwdpolicy);
/* Set forward table up. */
@@ -951,6 +844,7 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
cleanup:
while (!ISC_LIST_EMPTY(addrs)) {
+ isc_sockaddr_t *addr = NULL;
addr = ISC_LIST_HEAD(addrs);
ISC_LIST_UNLINK(addrs, addr, link);
isc_mem_put(inst->mctx, addr, sizeof(*addr));
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
