Handle invalid network families for EDNS0 subnet option

[daareiza]

Hi, while playing for a bit with DNS-over-TLS I found an issue using miekg/dns in conjunction with getdnsapi/stubby that uses getdnsapi/getdns behind the scenes.

getdns has the following configuration option:

• edns_client_subnet_private: Use EDNS0 Client Subnet privacy so the client subnet is not sent to authoritative servers

when that option is disabled, all works just as expected, but when that option is enabled, the new TLS DNS request has some extra EDNS0 flags, including one "empty" subnet that is a "Family 0" address (code).

Because of the "Family 0" address, miekg/dns just throws an dns: bad address family error while unpacking the EDNS0 message in edns.go#L266 and the request is not resolved.

I tried stubby with other TLS resolvers and they work just fine with this "Family 0" address, so I was confused about why miekg/dns has this problem.

Reading the RFC 7871 specs, I found the following:

   If there is no ADDRESS set, i.e.,
   SOURCE PREFIX-LENGTH is set to 0, then FAMILY SHOULD be set to the
   transport over which the query is sent.  This is for
   interoperability; at least one major authoritative server will ignore
   the option if FAMILY is not 1 or 2, even though it is irrelevant if
   there are no ADDRESS bits.

So, from the RFC snippet:

• getdns is not respecting the RFC because he is not including the family of the address (I will write an issue there too)
• miekg/dns can be improved by ignoring the EDNS0 subnet option if the Family is not 1 or 2

I hope this will help anyone facing similar issues.

[miekg]

Thanks for the detailed issues! Yes, we could improve here. Care to send a PR?

[m13253]

I think this bug should have been fixed in #530. Have you tried to clean up your GOPATH to fetch the latest version? Sometimes some caller-side hacks are also necessary if the crash also happens on the caller side.

Would you provide some packet capture or some error log?

By the way, you might be interested in my DNS-over-HTTPS project, which is a compatible implementation of Google's protocol.

[daareiza]

thanks @m13253, I just saw your PR and that should be enough to solve the issue with getdnsapi/getdns (btw, they already made a fix for that, 6afb02b), I am going to try it this afternoon, but, as for what RFC says, miekg/dns may just ignore all EDNS0 subnets with an invalid family so I think there is still a way to improve miegk/dns.

[miekg]

So I think this issue can be solved by not throwing an error for unknown families:

i.e.

diff --git a/edns.go b/edns.go
index a6a36bd..66da03a 100644
--- a/edns.go
+++ b/edns.go
@@ -258,7 +258,7 @@ func (e *EDNS0_SUBNET) pack() ([]byte, error) {
                needLength := (e.SourceNetmask + 8 - 1) / 8 // division rounding up
                b = append(b, ip[:needLength]...)
        default:
-               return nil, errors.New("dns: bad address family")
+               // Ignore unkown families.
        }
        return b, nil
 }