python / cpython

The Python programming language
https://www.python.org
Other
63.46k stars 30.39k forks source link

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

Open 4980da3a-3107-46a8-b98d-461c0fb8b0e7 opened 7 years ago

4980da3a-3107-46a8-b98d-461c0fb8b0e7 commented 7 years ago
BPO 30528
Nosy @ronaldoussoren, @hvenev, @bbayles, @ekohl, @Forst
PRs
  • python/cpython#3632
  • python/cpython#25371
  • python/cpython#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: ```python assignee = None closed_at = None created_at = labels = ['3.7', 'type-bug', 'library'] title = 'ipaddress.IPv{4,6}Network.reverse_pointer is broken' updated_at = user = 'https://github.com/hvenev' ``` bugs.python.org fields: ```python activity = actor = 'forst' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = 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'] ```

    4980da3a-3107-46a8-b98d-461c0fb8b0e7 commented 7 years ago

    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.

    5e1d06d2-7ccc-4223-a88f-71d578cbf50f commented 7 years ago

    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 commented 6 years ago

    \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.

    5e1d06d2-7ccc-4223-a88f-71d578cbf50f commented 6 years ago

    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 commented 6 years ago

    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.

    b970ee2d-f46b-4aee-b6af-4b698cc91e02 commented 3 years ago

    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.

    b970ee2d-f46b-4aee-b6af-4b698cc91e02 commented 3 years ago

    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]
    09f58c37-e969-4861-b0b7-4c43186e9523 commented 3 years ago

    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 commented 1 year ago

    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")))