-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
ipaddress IPv4Network and IPv6Network need specialized reverse_pointer property #125641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Seems, we need to get more detail about the reverse pointer from the RFC. @picnixz plz assign this issue to me, I will take handle of it . |
You can take care of the issue, I'm not here for the next days. I didn't know that the reverse pointer for network was different (clearly not my field of research) so I'm happy if you can fix the bug! |
There is no such thing as a reverse pointer for networks in dns itself. But it is related to how those domains can be delegated. Meaning where RFC 1035 can cut between zones operated by someone else. rfc2317 specifies recommended classless delegation using DNAME or CNAMEs in DNS. Classless in-addr.arpa delegation describes how to delegate
This means that in There is no similar proposal for IPv6, because common IPv6 prefixes assigned to people are divided by 4 with zero remainder. That is /48, /52, /56, /60, /64. But prefix like /54 is still valid, but needs multiple domains to hold reverse addresses contained in that prefix. |
This is affecting all bugfix+ versions (3.12+), right? |
I think we just need to backport to 3.13 if this is necessary. |
I might want to ask some questions about this issue in your test code # '8.4./.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'
assert(ip_network("2001:db8::/48").reverse_pointer == ['0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa']) I'm not sure the |
When I use
Edit: the code is correct, >>> import ipaddress
>>> n = ipaddress.ip_network("2001:db8::/48")
>>> n.broadcast_address
IPv6Address('2001:db8:0:ffff:ffff:ffff:ffff:ffff')
>>> n.broadcast_address.exploded
'2001:0db8:0000:ffff:ffff:ffff:ffff:ffff'
>>> n.network_address.exploded
'2001:0db8:0000:0000:0000:0000:0000:0000'
>>> n.network_address.exploded.replace(':','')[:n.prefixlen//4][::-1]
'00008bd01002'
>>> '.'.join(n.network_address.exploded.replace(':','')[:n.prefixlen//4][::-1])
'0.0.0.0.8.b.d.0.1.0.0.2' Because prefix is divisible by 4, just a single domain is enough to hold all possible addresses. Something like that was merged into our dnsconfd. |
In case prefix is not divisible by 4, subnets contained in parent divisible by 4 need to be generated. In fact network_address.reverse_pointer can be reused and just proper number of label splitted from it. Our example is at: |
The reverse_pointer functionality built into the ipaddress library yields undesired leading zeroes if an IPv6 network is passed. As this function is currently primarily utilized in the generation of our configuration for DNS64, it causes reverse lookups to fail, as addresses cannot be determined to belong to the given reverse domain. Prior to implementing this functionality the issue was not present, as the values were manually taken from `ipcalc`, which behaves as expected. Good: In 2a07:de40:b27e:64::/96 Out 0.0.0.0.0.0.0.0.4.6.0.0.e.7.2.b.0.4.e.d.7.0.a.2.ip6.arpa. Bad: In 2a07:de40:b27e:64::/96 Out 0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.4.6.0.0.e.7.2.b.0.4.e.d.7.0.a.2.ip6.arpa. Correct the logic by replacing it with a contraption found in the related upstream issue (thank you, @pemensik !): python/cpython#125641 Fixes: 94220a9 ("Modularize PowerDNS recursor NAT64 configuration") Fixes: a597f39 ("Add reverse_pointer function") Signed-off-by: Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com>
There's already a couple of issues with associated pull requests open for the same problem:
The first issue in particular has some more ideas and discussion on the matter, including a few other options on how to treat
The suggestion in this issue to return a list of one or more pointers is interesting, but a bit tricky in my opinion. For one, it may result in confusion where the address objects return a single string, but networks return a list for the same attribute. Also, the name RFC 2317 mentioned above only covers prefixes larger than |
@Zheaoli are you still working on this? |
Nope, that would be wrong. There might be a helper to this, but the given reverse pointer is for 192.0.2.0/24 network, not for the entry specified. Returning partially invalid data is not a good idea, especially if that desire were not explicitly requested. It may work for something until it creates visible errors. Wrong way IMO.
And when one does have networks with non-clean divisible block, he should not be able to work with them, just because we may need change return type? Why? Is there anything wrong with prefixes not divisible by 8 or 4?
Well, it it would mean we rename it to
Network is a group of address. Unlike single address it represents multiple of addresses. Well then multiple of reverse pointers should be documented, but not considered surprising.
I would be okay if reverse_pointer were kept and throwing Error if not divisible, but have reverse_pointers property providing always working list or set for any prefix.
Yes, RFC 2317 is just a hint how to configure DNS server to delegate parts of IPv4 space to other servers. It addresses just too big chunks of 8 bits per IPv4 number, which lacks better granularity. It relies on code embedded in DNS servers to generate a list from network 192.0.2.128/26 on the server side. Not always that is enough and we want ability to generate that list ourselves in any case. RFC 2317 did not have to deal with prefix smaller than 24, because those were possible to be gelegated just normal DNS delegation with common labels. But prefixes from 24 to 32, there are only final records, which should not be a CNAME. |
This sounds good to me! One could perhaps, for the sake of consistency, even implement |
Bug report
Bug description:
IPv4Network and IPv6Network get their implementation from _BaseAddress. But for network, it does not work as it should. It returns broken nonsense instead of helping data. Problem is unlike normal IP address, for a network range, it may return just single string for prefix lengths divisible by 8 for IPv4 and 4 for IPv6. But for other prefixes, it should return a list of domain names used.
I have implemented working generator for a list result at InfrastructureServices/dnsconfd#70
I think something similar should be used in base ipaddress directly. If reverse_pointer should not be fixed, it should be removed from networks instead.
Related to #123409, but that is not exactly about networks. Can be verified a bit using
ipcalc --reverse-dns
, but even that crashes on undivisible ipv6 prefixes.CPython versions tested on:
3.13
Operating systems tested on:
Linux
The text was updated successfully, but these errors were encountered: