Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 16.10.2015 12:13, Martin Babinsky wrote:
> On 10/14/2015 04:05 PM, Petr Spacek wrote:
>> On 14.10.2015 14:13, Martin Babinsky wrote:
>>> On 10/13/2015 04:00 PM, Petr Spacek wrote:
On 13.10.2015 13:37, Martin Babinsky wrote:
> On 10/13/2015 09:36 AM, Petr Spacek wrote:
>> On 12.10.2015 16:35, Martin Babinsky wrote:
>>> https://fedorahosted.org/freeipa/ticket/5200
>>> ---
>>> ipalib/plugins/dns.py | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
>>> index
>>> 84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
>>>
>>>
>>> 100644
>>> --- a/ipalib/plugins/dns.py
>>> +++ b/ipalib/plugins/dns.py
>>> @@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
>>> if prefixlen is None:
>>> revzone = None
>>>
>>> -result = api.Command['dnszone_find']()['result']
>>> +result = api.Command['dnszone_find'](sizelimit=0)['result']
>>> +
>>
>> NACK, this just increases the limit because LDAP server will enforce a
>> per-user limit.
>>
>>> for zone in result:
>>> zonename = zone['idnsname'][0]
>>> if (revdns.is_subdomain(zonename.make_absolute()) and
>>
>> Generic solution should use dns.resolver.zone_for_name() to find DNS zone
>> matching the reverse name. This should also implicitly cover CNAME/DNAME
>> redirections per RFC2317.
>>
>> Using DNS implicitly means that a zone will always be found (at least the
>> root
>> zone :-). For this reason I would change final error message
>>> reason=_('DNS reverse zone for IP address %(addr)s not found')
>> to something like:
>> reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not
>> managed
>> by this server')
>>
>>
>> These changes should fix it without adding other artificial limitation.
>>
>
> Thank you for clarification Petr.
>
> Attaching the reworked patch.
>
> --
> Martin^3 Babinsky
>
> freeipa-mbabinsk-0083.1-perform-more-robust-search-for-reverse-zones-when-ad.patch
>
>
>
>
> From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
> From: Martin Babinsky
> Date: Mon, 12 Oct 2015 16:24:50 +0200
> Subject: [PATCH 1/3] perform more robust search for reverse zones when
> adding
>DNS record
>
> Instead of searching for all zones to identify the correct reverse zone,
> we
> will first ask the resolver to return the name of zone that should
> contain the
> desired record and then see if IPA manages this zone.
>
> https://fedorahosted.org/freeipa/ticket/5200
> ---
>ipalib/plugins/dns.py | 21 +
>1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
> index
> 84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8
>
> 100644
> --- a/ipalib/plugins/dns.py
> +++ b/ipalib/plugins/dns.py
> @@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
>pass # the entry already exists and matches
>
>def get_reverse_zone(ipaddr, prefixlen=None):
> +"""
> +resolve the reverse zone for IP address and see if it is managed by
> IPA
> +server
> +:param ipaddr: host IP address
> +:param prefixlen: network prefix length
> +:return: tuple containing name of the reverse zone and the name of
> the
> +record
> +"""
>ip = netaddr.IPAddress(str(ipaddr))
>revdns = DNSName(unicode(ip.reverse_dns))
> +resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
>
>if prefixlen is None:
>revzone = None
> +result = api.Command['dnszone_find'](resolved_zone)['result']
>
> -result = api.Command['dnszone_find']()['result']
>for zone in result:
>zonename = zone['idnsname'][0]
>if (revdns.is_subdomain(zonename.make_absolute()) and
> (revzone is None or zonename.is_subdomain(revzone))):
>revzone = zonename
Oh, wait, this might blow up if there is a disabled DNS zone which is
deeper
than the one returned from DNS query.
Damn, we have opened a Pandora box!
ipaserver/install/bindinstance.py has own implementation of
get_reverse_zone()
+ additional menthods find_reverse_zone().
These are using get_reverse_zone_default() from ipalib/util.py which is
duplicating code in 'if prefixlen is not None' branch.
And of course, are not correct
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 22.10.2015 16:59, Petr Spacek wrote:
On 16.10.2015 12:13, Martin Babinsky wrote:
On 10/14/2015 04:05 PM, Petr Spacek wrote:
On 14.10.2015 14:13, Martin Babinsky wrote:
On 10/13/2015 04:00 PM, Petr Spacek wrote:
On 13.10.2015 13:37, Martin Babinsky wrote:
On 10/13/2015 09:36 AM, Petr Spacek wrote:
On 12.10.2015 16:35, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
if prefixlen is None:
revzone = None
-result = api.Command['dnszone_find']()['result']
+result = api.Command['dnszone_find'](sizelimit=0)['result']
+
NACK, this just increases the limit because LDAP server will enforce a
per-user limit.
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
Generic solution should use dns.resolver.zone_for_name() to find DNS zone
matching the reverse name. This should also implicitly cover CNAME/DNAME
redirections per RFC2317.
Using DNS implicitly means that a zone will always be found (at least the
root
zone :-). For this reason I would change final error message
reason=_('DNS reverse zone for IP address %(addr)s not found')
to something like:
reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not
managed
by this server')
These changes should fix it without adding other artificial limitation.
Thank you for clarification Petr.
Attaching the reworked patch.
--
Martin^3 Babinsky
freeipa-mbabinsk-0083.1-perform-more-robust-search-for-reverse-zones-when-ad.patch
From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 12 Oct 2015 16:24:50 +0200
Subject: [PATCH 1/3] perform more robust search for reverse zones when
adding
DNS record
Instead of searching for all zones to identify the correct reverse zone, we
will first ask the resolver to return the name of zone that should
contain the
desired record and then see if IPA manages this zone.
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
pass # the entry already exists and matches
def get_reverse_zone(ipaddr, prefixlen=None):
+"""
+resolve the reverse zone for IP address and see if it is managed by IPA
+server
+:param ipaddr: host IP address
+:param prefixlen: network prefix length
+:return: tuple containing name of the reverse zone and the name of the
+record
+"""
ip = netaddr.IPAddress(str(ipaddr))
revdns = DNSName(unicode(ip.reverse_dns))
+resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
if prefixlen is None:
revzone = None
+result = api.Command['dnszone_find'](resolved_zone)['result']
-result = api.Command['dnszone_find']()['result']
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
(revzone is None or zonename.is_subdomain(revzone))):
revzone = zonename
Oh, wait, this might blow up if there is a disabled DNS zone which is deeper
than the one returned from DNS query.
Damn, we have opened a Pandora box!
ipaserver/install/bindinstance.py has own implementation of
get_reverse_zone()
+ additional menthods find_reverse_zone().
These are using get_reverse_zone_default() from ipalib/util.py which is
duplicating code in 'if prefixlen is not None' branch.
And of course, are not correct in light of this change.
My expectation would be that disabled zones should be ignored... So we should
get rid of this mess in the loop and use dnszone_show() which is called at
the
end. And fix the other places, preferably by deleting duplicate
implementations.
I would expect that you can store (converted) output of
dns.resolver.zone_for_name(revdns) into revzone and delete whole 'if
prefixlen
is None' branch.
else:
+# if prefixlen is specified, determine the zone name for the
network
+# prefix
if ip.version == 4:
pos = 4 - prefixlen / 8
elif ip.version == 6:
pos = 32 - prefixlen / 4
-items = ip.reverse_dns.split('.')
-
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 10/14/2015 04:05 PM, Petr Spacek wrote:
On 14.10.2015 14:13, Martin Babinsky wrote:
On 10/13/2015 04:00 PM, Petr Spacek wrote:
On 13.10.2015 13:37, Martin Babinsky wrote:
On 10/13/2015 09:36 AM, Petr Spacek wrote:
On 12.10.2015 16:35, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
if prefixlen is None:
revzone = None
-result = api.Command['dnszone_find']()['result']
+result = api.Command['dnszone_find'](sizelimit=0)['result']
+
NACK, this just increases the limit because LDAP server will enforce a
per-user limit.
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
Generic solution should use dns.resolver.zone_for_name() to find DNS zone
matching the reverse name. This should also implicitly cover CNAME/DNAME
redirections per RFC2317.
Using DNS implicitly means that a zone will always be found (at least the
root
zone :-). For this reason I would change final error message
reason=_('DNS reverse zone for IP address %(addr)s not found')
to something like:
reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not
managed
by this server')
These changes should fix it without adding other artificial limitation.
Thank you for clarification Petr.
Attaching the reworked patch.
--
Martin^3 Babinsky
freeipa-mbabinsk-0083.1-perform-more-robust-search-for-reverse-zones-when-ad.patch
From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 12 Oct 2015 16:24:50 +0200
Subject: [PATCH 1/3] perform more robust search for reverse zones when adding
DNS record
Instead of searching for all zones to identify the correct reverse zone, we
will first ask the resolver to return the name of zone that should contain the
desired record and then see if IPA manages this zone.
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
pass # the entry already exists and matches
def get_reverse_zone(ipaddr, prefixlen=None):
+"""
+resolve the reverse zone for IP address and see if it is managed by IPA
+server
+:param ipaddr: host IP address
+:param prefixlen: network prefix length
+:return: tuple containing name of the reverse zone and the name of the
+record
+"""
ip = netaddr.IPAddress(str(ipaddr))
revdns = DNSName(unicode(ip.reverse_dns))
+resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
if prefixlen is None:
revzone = None
+result = api.Command['dnszone_find'](resolved_zone)['result']
-result = api.Command['dnszone_find']()['result']
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
(revzone is None or zonename.is_subdomain(revzone))):
revzone = zonename
Oh, wait, this might blow up if there is a disabled DNS zone which is deeper
than the one returned from DNS query.
Damn, we have opened a Pandora box!
ipaserver/install/bindinstance.py has own implementation of get_reverse_zone()
+ additional menthods find_reverse_zone().
These are using get_reverse_zone_default() from ipalib/util.py which is
duplicating code in 'if prefixlen is not None' branch.
And of course, are not correct in light of this change.
My expectation would be that disabled zones should be ignored... So we should
get rid of this mess in the loop and use dnszone_show() which is called at the
end. And fix the other places, preferably by deleting duplicate
implementations.
I would expect that you can store (converted) output of
dns.resolver.zone_for_name(revdns) into revzone and delete whole 'if prefixlen
is None' branch.
else:
+# if prefixlen is specified, determine the zone name for the network
+# prefix
if ip.version == 4:
pos = 4 - prefixlen / 8
elif ip.version == 6:
pos = 32 - prefixlen / 4
-items = ip.reverse_dns.split('.')
-revzone = DNSName(items[pos:])
+revzone = revdns[pos:]
Hmm, and this logic will breaks CNAME/DNAME
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 10/13/2015 04:00 PM, Petr Spacek wrote:
On 13.10.2015 13:37, Martin Babinsky wrote:
On 10/13/2015 09:36 AM, Petr Spacek wrote:
On 12.10.2015 16:35, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
if prefixlen is None:
revzone = None
-result = api.Command['dnszone_find']()['result']
+result = api.Command['dnszone_find'](sizelimit=0)['result']
+
NACK, this just increases the limit because LDAP server will enforce a
per-user limit.
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
Generic solution should use dns.resolver.zone_for_name() to find DNS zone
matching the reverse name. This should also implicitly cover CNAME/DNAME
redirections per RFC2317.
Using DNS implicitly means that a zone will always be found (at least the root
zone :-). For this reason I would change final error message
reason=_('DNS reverse zone for IP address %(addr)s not found')
to something like:
reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not managed
by this server')
These changes should fix it without adding other artificial limitation.
Thank you for clarification Petr.
Attaching the reworked patch.
--
Martin^3 Babinsky
freeipa-mbabinsk-0083.1-perform-more-robust-search-for-reverse-zones-when-ad.patch
From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 12 Oct 2015 16:24:50 +0200
Subject: [PATCH 1/3] perform more robust search for reverse zones when adding
DNS record
Instead of searching for all zones to identify the correct reverse zone, we
will first ask the resolver to return the name of zone that should contain the
desired record and then see if IPA manages this zone.
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
pass # the entry already exists and matches
def get_reverse_zone(ipaddr, prefixlen=None):
+"""
+resolve the reverse zone for IP address and see if it is managed by IPA
+server
+:param ipaddr: host IP address
+:param prefixlen: network prefix length
+:return: tuple containing name of the reverse zone and the name of the
+record
+"""
ip = netaddr.IPAddress(str(ipaddr))
revdns = DNSName(unicode(ip.reverse_dns))
+resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
if prefixlen is None:
revzone = None
+result = api.Command['dnszone_find'](resolved_zone)['result']
-result = api.Command['dnszone_find']()['result']
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
(revzone is None or zonename.is_subdomain(revzone))):
revzone = zonename
Oh, wait, this might blow up if there is a disabled DNS zone which is deeper
than the one returned from DNS query.
Damn, we have opened a Pandora box!
ipaserver/install/bindinstance.py has own implementation of get_reverse_zone()
+ additional menthods find_reverse_zone().
These are using get_reverse_zone_default() from ipalib/util.py which is
duplicating code in 'if prefixlen is not None' branch.
And of course, are not correct in light of this change.
My expectation would be that disabled zones should be ignored... So we should
get rid of this mess in the loop and use dnszone_show() which is called at the
end. And fix the other places, preferably by deleting duplicate implementations.
I would expect that you can store (converted) output of
dns.resolver.zone_for_name(revdns) into revzone and delete whole 'if prefixlen
is None' branch.
else:
+# if prefixlen is specified, determine the zone name for the network
+# prefix
if ip.version == 4:
pos = 4 - prefixlen / 8
elif ip.version == 6:
pos = 32 - prefixlen / 4
-items = ip.reverse_dns.split('.')
-revzone = DNSName(items[pos:])
+revzone = revdns[pos:]
Hmm, and this logic will breaks CNAME/DNAME (RFC2317) handling ... Damn, we
need different handling when we intend to create the zone and when we want to
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 14.10.2015 14:13, Martin Babinsky wrote:
> On 10/13/2015 04:00 PM, Petr Spacek wrote:
>> On 13.10.2015 13:37, Martin Babinsky wrote:
>>> On 10/13/2015 09:36 AM, Petr Spacek wrote:
On 12.10.2015 16:35, Martin Babinsky wrote:
> https://fedorahosted.org/freeipa/ticket/5200
> ---
>ipalib/plugins/dns.py | 3 ++-
>1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
> index
> 84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
>
> 100644
> --- a/ipalib/plugins/dns.py
> +++ b/ipalib/plugins/dns.py
> @@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
>if prefixlen is None:
>revzone = None
>
> -result = api.Command['dnszone_find']()['result']
> +result = api.Command['dnszone_find'](sizelimit=0)['result']
> +
NACK, this just increases the limit because LDAP server will enforce a
per-user limit.
>for zone in result:
>zonename = zone['idnsname'][0]
>if (revdns.is_subdomain(zonename.make_absolute()) and
Generic solution should use dns.resolver.zone_for_name() to find DNS zone
matching the reverse name. This should also implicitly cover CNAME/DNAME
redirections per RFC2317.
Using DNS implicitly means that a zone will always be found (at least the
root
zone :-). For this reason I would change final error message
> reason=_('DNS reverse zone for IP address %(addr)s not found')
to something like:
reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not
managed
by this server')
These changes should fix it without adding other artificial limitation.
>>>
>>> Thank you for clarification Petr.
>>>
>>> Attaching the reworked patch.
>>>
>>> --
>>> Martin^3 Babinsky
>>>
>>> freeipa-mbabinsk-0083.1-perform-more-robust-search-for-reverse-zones-when-ad.patch
>>>
>>>
>>>
>>> From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
>>> From: Martin Babinsky
>>> Date: Mon, 12 Oct 2015 16:24:50 +0200
>>> Subject: [PATCH 1/3] perform more robust search for reverse zones when
>>> adding
>>> DNS record
>>>
>>> Instead of searching for all zones to identify the correct reverse zone, we
>>> will first ask the resolver to return the name of zone that should contain
>>> the
>>> desired record and then see if IPA manages this zone.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5200
>>> ---
>>> ipalib/plugins/dns.py | 21 +
>>> 1 file changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
>>> index
>>> 84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8
>>> 100644
>>> --- a/ipalib/plugins/dns.py
>>> +++ b/ipalib/plugins/dns.py
>>> @@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
>>> pass # the entry already exists and matches
>>>
>>> def get_reverse_zone(ipaddr, prefixlen=None):
>>> +"""
>>> +resolve the reverse zone for IP address and see if it is managed by IPA
>>> +server
>>> +:param ipaddr: host IP address
>>> +:param prefixlen: network prefix length
>>> +:return: tuple containing name of the reverse zone and the name of the
>>> +record
>>> +"""
>>> ip = netaddr.IPAddress(str(ipaddr))
>>> revdns = DNSName(unicode(ip.reverse_dns))
>>> +resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
>>>
>>> if prefixlen is None:
>>> revzone = None
>>> +result = api.Command['dnszone_find'](resolved_zone)['result']
>>>
>>> -result = api.Command['dnszone_find']()['result']
>>> for zone in result:
>>> zonename = zone['idnsname'][0]
>>> if (revdns.is_subdomain(zonename.make_absolute()) and
>>> (revzone is None or zonename.is_subdomain(revzone))):
>>> revzone = zonename
>>
>> Oh, wait, this might blow up if there is a disabled DNS zone which is deeper
>> than the one returned from DNS query.
>>
>> Damn, we have opened a Pandora box!
>>
>> ipaserver/install/bindinstance.py has own implementation of
>> get_reverse_zone()
>> + additional menthods find_reverse_zone().
>> These are using get_reverse_zone_default() from ipalib/util.py which is
>> duplicating code in 'if prefixlen is not None' branch.
>>
>> And of course, are not correct in light of this change.
>>
>> My expectation would be that disabled zones should be ignored... So we should
>> get rid of this mess in the loop and use dnszone_show() which is called at
>> the
>> end. And fix the other places, preferably by deleting duplicate
>> implementations.
>>
>> I would expect that you can store (converted) output of
>> dns.resolver.zone_for_name(revdns) into revzone and
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 12.10.2015 16:35, Martin Babinsky wrote:
> https://fedorahosted.org/freeipa/ticket/5200
> ---
> ipalib/plugins/dns.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
> index
> 84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
> 100644
> --- a/ipalib/plugins/dns.py
> +++ b/ipalib/plugins/dns.py
> @@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
> if prefixlen is None:
> revzone = None
>
> -result = api.Command['dnszone_find']()['result']
> +result = api.Command['dnszone_find'](sizelimit=0)['result']
> +
NACK, this just increases the limit because LDAP server will enforce a
per-user limit.
> for zone in result:
> zonename = zone['idnsname'][0]
> if (revdns.is_subdomain(zonename.make_absolute()) and
Generic solution should use dns.resolver.zone_for_name() to find DNS zone
matching the reverse name. This should also implicitly cover CNAME/DNAME
redirections per RFC2317.
Using DNS implicitly means that a zone will always be found (at least the root
zone :-). For this reason I would change final error message
> reason=_('DNS reverse zone for IP address %(addr)s not found')
to something like:
reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not managed
by this server')
These changes should fix it without adding other artificial limitation.
--
Petr^2 Spacek
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 13.10.2015 13:37, Martin Babinsky wrote:
> On 10/13/2015 09:36 AM, Petr Spacek wrote:
>> On 12.10.2015 16:35, Martin Babinsky wrote:
>>> https://fedorahosted.org/freeipa/ticket/5200
>>> ---
>>> ipalib/plugins/dns.py | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
>>> index
>>> 84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
>>> 100644
>>> --- a/ipalib/plugins/dns.py
>>> +++ b/ipalib/plugins/dns.py
>>> @@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
>>> if prefixlen is None:
>>> revzone = None
>>>
>>> -result = api.Command['dnszone_find']()['result']
>>> +result = api.Command['dnszone_find'](sizelimit=0)['result']
>>> +
>>
>> NACK, this just increases the limit because LDAP server will enforce a
>> per-user limit.
>>
>>> for zone in result:
>>> zonename = zone['idnsname'][0]
>>> if (revdns.is_subdomain(zonename.make_absolute()) and
>>
>> Generic solution should use dns.resolver.zone_for_name() to find DNS zone
>> matching the reverse name. This should also implicitly cover CNAME/DNAME
>> redirections per RFC2317.
>>
>> Using DNS implicitly means that a zone will always be found (at least the
>> root
>> zone :-). For this reason I would change final error message
>>> reason=_('DNS reverse zone for IP address %(addr)s not found')
>> to something like:
>>reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not managed
>> by this server')
>>
>>
>> These changes should fix it without adding other artificial limitation.
>>
>
> Thank you for clarification Petr.
>
> Attaching the reworked patch.
>
> --
> Martin^3 Babinsky
>
> freeipa-mbabinsk-0083.1-perform-more-robust-search-for-reverse-zones-when-ad.patch
>
>
> From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
> From: Martin Babinsky
> Date: Mon, 12 Oct 2015 16:24:50 +0200
> Subject: [PATCH 1/3] perform more robust search for reverse zones when adding
> DNS record
>
> Instead of searching for all zones to identify the correct reverse zone, we
> will first ask the resolver to return the name of zone that should contain the
> desired record and then see if IPA manages this zone.
>
> https://fedorahosted.org/freeipa/ticket/5200
> ---
> ipalib/plugins/dns.py | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
> index
> 84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8
> 100644
> --- a/ipalib/plugins/dns.py
> +++ b/ipalib/plugins/dns.py
> @@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
> pass # the entry already exists and matches
>
> def get_reverse_zone(ipaddr, prefixlen=None):
> +"""
> +resolve the reverse zone for IP address and see if it is managed by IPA
> +server
> +:param ipaddr: host IP address
> +:param prefixlen: network prefix length
> +:return: tuple containing name of the reverse zone and the name of the
> +record
> +"""
> ip = netaddr.IPAddress(str(ipaddr))
> revdns = DNSName(unicode(ip.reverse_dns))
> +resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
>
> if prefixlen is None:
> revzone = None
> +result = api.Command['dnszone_find'](resolved_zone)['result']
>
> -result = api.Command['dnszone_find']()['result']
> for zone in result:
> zonename = zone['idnsname'][0]
> if (revdns.is_subdomain(zonename.make_absolute()) and
> (revzone is None or zonename.is_subdomain(revzone))):
> revzone = zonename
Oh, wait, this might blow up if there is a disabled DNS zone which is deeper
than the one returned from DNS query.
Damn, we have opened a Pandora box!
ipaserver/install/bindinstance.py has own implementation of get_reverse_zone()
+ additional menthods find_reverse_zone().
These are using get_reverse_zone_default() from ipalib/util.py which is
duplicating code in 'if prefixlen is not None' branch.
And of course, are not correct in light of this change.
My expectation would be that disabled zones should be ignored... So we should
get rid of this mess in the loop and use dnszone_show() which is called at the
end. And fix the other places, preferably by deleting duplicate implementations.
I would expect that you can store (converted) output of
dns.resolver.zone_for_name(revdns) into revzone and delete whole 'if prefixlen
is None' branch.
> else:
> +# if prefixlen is specified, determine the zone name for the network
> +# prefix
> if ip.version == 4:
> pos = 4 - prefixlen / 8
> elif ip.version == 6:
> pos = 32 - prefixlen / 4
> -items = ip.reverse_dns.split('.')
> -revzone = DNSName(items[pos:])
Re: [Freeipa-devel] [PATCH 0083] perform an unlimited search for reverse zones when adding DNS records
On 10/13/2015 09:36 AM, Petr Spacek wrote:
On 12.10.2015 16:35, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
84086f4c77d02922f237937d58031cc42d55410e..c36345faecfb9db7abced1c6bd72ddcf93473a74
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -537,7 +537,8 @@ def get_reverse_zone(ipaddr, prefixlen=None):
if prefixlen is None:
revzone = None
-result = api.Command['dnszone_find']()['result']
+result = api.Command['dnszone_find'](sizelimit=0)['result']
+
NACK, this just increases the limit because LDAP server will enforce a
per-user limit.
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
Generic solution should use dns.resolver.zone_for_name() to find DNS zone
matching the reverse name. This should also implicitly cover CNAME/DNAME
redirections per RFC2317.
Using DNS implicitly means that a zone will always be found (at least the root
zone :-). For this reason I would change final error message
reason=_('DNS reverse zone for IP address %(addr)s not found')
to something like:
reason=_('DNS reverse zone %(zone)s for IP address %(addr)s is not managed
by this server')
These changes should fix it without adding other artificial limitation.
Thank you for clarification Petr.
Attaching the reworked patch.
--
Martin^3 Babinsky
From 463903f4ea4f74efeb02dbb705ca0a54fab81366 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Mon, 12 Oct 2015 16:24:50 +0200
Subject: [PATCH 1/3] perform more robust search for reverse zones when adding
DNS record
Instead of searching for all zones to identify the correct reverse zone, we
will first ask the resolver to return the name of zone that should contain the
desired record and then see if IPA manages this zone.
https://fedorahosted.org/freeipa/ticket/5200
---
ipalib/plugins/dns.py | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 84086f4c77d02922f237937d58031cc42d55410e..e3c6ce46168f8b6af607a61af1e3e431cfee32c8 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -531,25 +531,35 @@ def add_forward_record(zone, name, str_address):
pass # the entry already exists and matches
def get_reverse_zone(ipaddr, prefixlen=None):
+"""
+resolve the reverse zone for IP address and see if it is managed by IPA
+server
+:param ipaddr: host IP address
+:param prefixlen: network prefix length
+:return: tuple containing name of the reverse zone and the name of the
+record
+"""
ip = netaddr.IPAddress(str(ipaddr))
revdns = DNSName(unicode(ip.reverse_dns))
+resolved_zone = unicode(dns.resolver.zone_for_name(revdns))
if prefixlen is None:
revzone = None
+result = api.Command['dnszone_find'](resolved_zone)['result']
-result = api.Command['dnszone_find']()['result']
for zone in result:
zonename = zone['idnsname'][0]
if (revdns.is_subdomain(zonename.make_absolute()) and
(revzone is None or zonename.is_subdomain(revzone))):
revzone = zonename
else:
+# if prefixlen is specified, determine the zone name for the network
+# prefix
if ip.version == 4:
pos = 4 - prefixlen / 8
elif ip.version == 6:
pos = 32 - prefixlen / 4
-items = ip.reverse_dns.split('.')
-revzone = DNSName(items[pos:])
+revzone = revdns[pos:]
try:
api.Command['dnszone_show'](revzone)
@@ -558,7 +568,10 @@ def get_reverse_zone(ipaddr, prefixlen=None):
if revzone is None:
raise errors.NotFound(
-reason=_('DNS reverse zone for IP address %(addr)s not found') % dict(addr=ipaddr)
+reason=_(
+'DNS reverse zone %(resolved_zone)s for IP address '
+'%(addr)s is not manager by this server') % dict(
+addr=ipaddr, resolved_zone=resolved_zone)
)
revname = revdns.relativize(revzone)
--
2.4.3
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
