getdnsapi / stubby

Stubby is the name given to a mode of using getdns which enables it to act as a local DNS Privacy stub resolver (using DNS-over-TLS).
https://dnsprivacy.org/dns_privacy_daemon_-_stubby/
BSD 3-Clause "New" or "Revised" License
1.19k stars 99 forks source link

EDNS should not be passed through to client #98

Closed twisteroidambassador closed 6 years ago

twisteroidambassador commented 6 years ago

It looks like Stubby passes the entire DNS response from the upstream server to the client, including the OPT pseudo-RR which specifies EDNS options.

(I discovered this by having multiple upstream servers configured with round-robin, and retrying the same request, observing different EDNS options being returned each time. See here for results of repeated dig command, and here for corresponding Stubby log.

In particular, one of the responses has an option (OPT=11: 01 2c (".,")) stating that the DNS server is willing to allow a long-lived TCP connection with a 30-second timeout, which Stubby is certainly not honoring.)

RFC 6891 - Extension Mechanisms for DNS (EDNS(0)) states:

EDNS is a hop-by-hop extension to DNS. This means the use of EDNS is negotiated between each pair of hosts in a DNS resolution process, for instance, the stub resolver communicating with the recursive resolver or the recursive resolver communicating with an authoritative server.

And:

OPT RRs MUST NOT be cached, forwarded, or stored in or loaded from master files.

So, it might be a good idea to strip out the OPT RR from upstream responses before passing through to the client.

(Optionally, Stubby may add its own OPT RR in accordance to its capabilities: perhaps UDP responses larger than 512 bytes, or long-lived TCP connections?)

wtoorop commented 6 years ago

Yes, passing through the keep-alive option is indeed just silly. I do believe Stubby as a client of the upstream will honor that setting b.t.w., but it should not pass it through to the requester no. So acknowledged.

However... Stubby is a DNS proxy. It receives queries from the system stub and forwards them to the configured resolvers. RFC6891 Section 6.2.6 states:

Middleboxes that simply forward requests to a recursive resolver MUST NOT modify and MUST NOT delete the OPT record contents in either direction.

Further in the same section it refers to RFC5625 (DNS Proxy Implementation Guidelines) which in Section 3 states:

It is RECOMMENDED that proxies should be as transparent as possible, such that any "hop-by-hop" mechanisms or newly introduced protocol extensions operate as if the proxy were not there.

So, I guess it's a balance. Stubby started out with a scheme that anticipated how to pass on, and pass back messages giving different permutations of the AD, DO and CD bits. This was later adapted to be more transparent when no DNSSEC extension is set (I have to update the doc with that).

Stubby also does honor upstream keep-alive options as a client, It may send cookies and respond to upstream cookie responses (when the extension is set) and it can also send padding (when the extension is set).

I think the best way to address this is to actively filter out those options that Stubby actively responds to. For edns keep-alive this means always filter it out, since Stubby itself is responding to it and this cannot be turned on or off with an extension.

saradickinson commented 6 years ago

Interesting and good catch... I completely agree that we should fix this as it is an error in the implementation. It is mentioned in passing in Keepalive spec , but is under-specified if you ask me (and was an author :-) ). I don't think it is mentioned at all in the padding spec. Note that in the DSO spec, there is a specific recommendation that those messages are NOT forwarded blindly.

I would also actually argue that stubby is not a 'Middlebox that simply forwards requests' and it is probably not a pure proxy by the RFC6819 definition and we should be more careful in using that phrase. Since there are a multitude of setting (DNSSEC, transport, etc.) that effect how stubby handles DNS resolution I think it more like an 'intelligent DNS forwarder'.

Note that the DNS terminology draft bis is still quite vague on what a proxy or forwarder actually is! I'm minded to think that RFC5625 should be bis-ed to clarify this, particularly since the picture with respect to transport for DNS has changed so much !

wtoorop commented 6 years ago

@saradickinson Ok sorry, won't call it that anymore then ;)

saradickinson commented 6 years ago

@wtoorop I've used it repeatedly in presentations and just fixed a couple of instances on dnsprivacy.org so it was a 'Note to self:' as much as anything! 😁

wtoorop commented 6 years ago

Fixed in the 1.4.2 release