huitema / dnsoquic

DNS over QUIC
10 stars 7 forks source link

Message ID rules could be clearer #97

Closed martinthomson closed 3 years ago

martinthomson commented 3 years ago

Just noting that "this has implications" isn't really very helpful. Better to state a requirement (follow the requirements of the protocol you choose) and give a bit of guidance.

saradickinson commented 3 years ago

Agree text could be more detailed - to be absolutely correct it should include that the message ID cannot already be in use by any outstanding query on the same IP/port tuple and may be subject to a maximum outstanding query limit, etc. I've suggested some text below for discussion. IIRC we did have a discussion a long time ago about the potential mis-match between the number of outstanding queries and decided to just document it as a corner case for proxies, rather than limit DoQ? I mention the second obvious case of forwarding over DoQ as I fixed one test tool that didn't do this.

This has implications for proxying DoQ message to and from other transports. 

One aspect is that a single DoQ connection can , in principle, support more outstanding queries 
at a given point in time than, for example, a single TCP connection which is limited by the Message 
ID space. Proxy implementations could choose to manage mappings where multiple TCP/DoT/DoH 
connections are required due to a large number of outstanding queries. For UDP, such a scenario 
will require other solutions e.g. delaying or dropping messages. 

When forwarding a DNS message from DoQ over another transport, a DNS Message ID MUST be 
generated according to the rules of the protocol that is in use.  When forwarding a DNS message 
from another transport over DoQ, the Message ID MUST be set to zero
martinthomson commented 3 years ago

That sounds reasonable as an addition (though I would drop DoH from your list as DoH has similar properties to DoQ when it comes to correlating queries).

saradickinson commented 3 years ago

Opened https://github.com/huitema/dnsoquic/pull/120 to resolve this