ipaddress.IPv{4,6}Network.reverse_pointer is broken

[hvenev]

BPO	30528
Nosy	@ronaldoussoren, @hvenev, @bbayles, @ekohl, @Forst
PRs	bpo-30528: Fix IPv{4,6}Network reverse_pointer #3632 bpo-30528 Update ipaddress.py with reverse_pointer method for networks #25371 gh-74713: ipaddress: support reverse pointer generation for network objects #29011

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2017-05-31.17:15:31.414>
labels = ['3.7', 'type-bug', 'library']
title = 'ipaddress.IPv{4,6}Network.reverse_pointer is broken'
updated_at = <Date 2021-10-17.17:38:56.782>
user = 'https://github.com/hvenev'

bugs.python.org fields:

activity = <Date 2021-10-17.17:38:56.782>
actor = 'forst'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2017-05-31.17:15:31.414>
creator = 'h.venev'
dependencies = []
files = []
hgrepos = []
issue_num = 30528
keywords = ['patch']
message_count = 8.0
messages = ['294854', '302390', '311869', '313675', '313750', '390880', '390884', '404144']
nosy_count = 8.0
nosy_names = ['ronaldoussoren', 'pmoody', 'python-dev', 'h.venev', 'bbayles', 'ekohl', 'jana', 'forst']
pr_nums = ['3632', '25371', '29011']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue30528'
versions = ['Python 3.7']

[hvenev]

ipaddress.IPv4Network('127.0.0.0/16').reverse_pointer = '0/16.0.0.127.in-addr.arpa' is definitely wrong. I think it should be '0.127.in-addr.arpa'.

A funnier case, ipaddress.IPv6Network('2001:db8::/32').reverse_pointer = '2.3./.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa'.

For the case where no single reverse pointer exists (e.g. 127.0.0.0/13), I think it should be None.

[ekohl]

Today I ran into this as well. In the case of IPv6 it's simple to decide what should be returned but on the IPv4Network I disagree. My expectation would be the domain where I would make the reverse needed. That means for 127.0.0.0/13 it should be '127.in-addr.arpa'.

I implemented it this way in the linked PR but I'm open to other opinions.

[ronaldoussoren]

<https://docs.python.org/3/library/ipaddress.html#ipaddress.IPv4Address.reverse_pointer\> documents that this attribute contains the name of the DNS name that could be used to query for PTR record.

That functionality is not well defined for a network object. It might therefore be better to just raise an exception for this attribute on network objects.

[ekohl]

It's interesting to note that neither IPv4Network1 nor IPv4Network docs mention reverse_pointer. That means it could also remove them (which essentially also throws an exception) since they don't make sense for networks. It would be useful to have the described functionality under a better name.

[ronaldoussoren]

The "reverse_pointers" attribute is implicitly documented by this phrase in the introduction of network objects:

--
All attributes implemented by address objects are implemented by network objects as well.

[jana]

Running into the same problem here. Within the zonefile rfc1035 defines a usecase for ipv4, but I can't find anything similar for IPv6. The feature is also rather obscure. The zone however is used in the zonefile as origin and in bind in the named.conf to refer to which zone is managed where.

For this you want to provide the reverse network address, minus the irrelevant zeros, so 192.168.0.0/24 => 0.168.192.in-addr.arpa and 2001:db8::/32 => 8.b.d.0.1.0.0.2.ip6.arpa

This would be to me a fitting and convenient interpretation of the function.

[jana]

This code does the trick:

ipn = ipaddress.ip_network("2a0c:ac10::/32")
prefix = ipn.prefixlen
if ipn.version == 6:
    rest = int((ipn.max_prefixlen - prefix) / 4)
elif ipn.version == 4:
    rest = int((ipn.max_prefixlen - prefix) / 8)
return ipn.network_address.reverse_pointer.split(".", rest)[-1]

[Forst]

I've stumbled upon this myself, and had a go at fixing it, before looking up this issue. My approach ended up being a bit different:

1. I rewrote the existing _reverse_pointer() methods, so that they'd handle both addresses and networks, instead of defining separate methods for the IPvXNetwork classes.
2. Trying to generate a reverse pointer for a prefix that doesn't align with the granularity of reverse pointers (8 bits for IPv4, 4 bits for IPv6) will raise an exception instead of returning a more general prefix.

I would like to bring up point 2 for discussion, since it differs from both of the other PRs submitted regarding this issue.

For example, let's take an example subnet 127.45.240.0/20. The other solutions here propose generating a reverse pointer like "45.127.in-addr.arpa", i.e. returning a reverse pointer to a bigger subnet that includes the original one. When you convert that reverse pointer back into a network, you get 127.45.0.0/16. It doesn't match the original network, thus you lose some information about it.

I'd like to propose to be more strict about it (or at least, make the strict behaviour an option) to avoid unintentional loss of precision when generating reverse pointers in these cases.

[commonism]

In case you need "Classless IN-ADDR.ARPA delegation for short prefixes" according to draft-ietf-dnsop-rfc2317bis-00

import ipaddress
from typing import Union

def reverse_pointer(a: Union[ipaddress.IPv4Network, ipaddress.IPv6Network]):
    """
    6. Classless IN-ADDR.ARPA delegation for short prefixes

    https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-rfc2317bis-00#page-8
    """
    assert isinstance(a, (ipaddress.IPv4Network, ipaddress.IPv6Network))
    if a.version == 4:
        s = a.network_address.compressed.split('.')
        s.reverse()
        first_byte_index = int(4 - (a.prefixlen // 8))
        if a.prefixlen % 8 != 0:
            assert a.prefixlen > 24
            nibblepart = "{}-{}.".format(s[3 - (a.prefixlen // 8)], a.broadcast_address.compressed.split('.')[-1])
        else:
            nibblepart = ""
        s = '.'.join(s[first_byte_index:])
        return "{}{}.in-addr.arpa.".format(nibblepart, s)
    elif a.version == 6:
        ipv4 = a.network_address.ipv4_mapped
        if ipv4:
            return reverse_pointer(ipv4)
        s = a.network_address.exploded.replace(':', '')
        if a.prefixlen % 4 != 0:
            raise ValueError("FIXME")
            nibblepart = "%s-%x" % (s[a.prefixlen:], a.network_address + 0)
            nibblepart += '.'
        else:
            nibblepart = ""
        s = '.'.join(s[::-1])
        first_nibble_index = int(32 - (a.prefixlen // 4)) * 2
        return "%s%s.ip6.arpa." % (nibblepart, s[first_nibble_index:])


print(reverse_pointer(ipaddress.ip_network("10.0.0.0/25")))
print(reverse_pointer(ipaddress.ip_network("10.0.0.0/16")))
print(reverse_pointer(ipaddress.ip_network("10.0.0.0/23")))