miekg / dns

DNS library in Go
https://miek.nl/2014/august/16/go-dns-package
BSD 3-Clause "New" or "Revised" License
7.95k stars 1.12k forks source link

Unable to distinguish between IPV4 and IPV4-mapped IPv6 #1388

Closed monoidic closed 1 year ago

monoidic commented 2 years ago

This appears to be more of an issue with how net.IP works, with how e.g ::ffff:255.255.255.255 gets .String()-ed as 255.255.255.255. IPv4 addresses do not get parsed as IPv4-mapped IPv6 addresses, hence it is impossible to convert back and forth between RRs and the .String() output. Here is an example which makes the issue clear.

func TestParseAAAA(t *testing.T) {
    inS := "mapped.example.com. 0 IN AAAA ::ffff:255.255.255.255"
    rr := testRR(inS)

    rrS := rr.String()
    fmt.Println(rrS)

    newRR := testRR(rrS)

    if newRRS := newRR.String(); newRRS != rrS {
        t.Errorf("input: %s, intermediate: %s, out: %s\n", inS, rrS, newRRS)
    }
}

Output:

mapped.example.com.     0       IN      AAAA    255.255.255.255
--- FAIL: TestParseAAAA (0.00s)
panic: dns: bad AAAA AAAA: "255.255.255.255" at line: 1:45 [recovered]
        panic: dns: bad AAAA AAAA: "255.255.255.255" at line: 1:45

goroutine 382 [running]:
testing.tRunner.func1.2({0x73e820, 0xc0000c9900})
        /usr/lib/go/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
        /usr/lib/go/src/testing/testing.go:1392 +0x39f
panic({0x73e820, 0xc0000c9900})
        /usr/lib/go/src/runtime/panic.go:838 +0x207
github.com/miekg/dns.testRR(...)
        /home/defceb/src/dns/rr_test.go:7
github.com/miekg/dns.TestParseAAAA(0xc000203ba0)
        /home/defceb/src/dns/parse_test.go:2026 +0x1b3
testing.tRunner(0xc000203ba0, 0x7cdb98)
        /usr/lib/go/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
        /usr/lib/go/src/testing/testing.go:1486 +0x35f
exit status 2
FAIL    github.com/miekg/dns    0.446s

Just switching to net/netip would be a relatively simple fix, though changing the type of AAAA.AAAA (and A.A as well to keep them uniform) from net.IP to netip.Addr would be 1) a pretty big breaking change, 2) breaking the promise to support the last two versions of Go, at least until 1.19 is released. Though I suspect that, due to the benefits of netip.Addr over net.IP, the former will be preferred over the latter anyhow in new code.

miekg commented 2 years ago

yep, we will not switch because of backward compact reasons.

also the issue is with net.IP, not something that can be fixed here, nor do I want to work around it, because it's pretty much a niche feature.

I don't know if anyone filed a upstream Go bug for this.

lanrat commented 1 year ago

related: #1107