Significant changes for Proxy DNS and SVCB request/response handling : try 2#9
Significant changes for Proxy DNS and SVCB request/response handling : try 2#9enygren wants to merge 8 commits into
Conversation
Trying to merge over my change to see if there's a way to resubmit the PR
|
Hopefully this should actually be able to be merged? |
MikeBishop
left a comment
MikeBishop
left a comment
There was a problem hiding this comment.
This is a good start, with mostly nits and comments on issues that can be deferred from this PR.
I do feel like we don't have a good story (or perhaps merely a good example?) for cases which involve a mix of CNAMEs and AliasMode entries, though perhaps that's alleviated by the current design of telling the proxy not to consider SVCB records in performing the actual connection.
| *TODO*: Do we need the "o=" parameter or is it redundant? | ||
| If we include it, should it be a MUST or MAY? | ||
|
|
||
| *TODO*: Do we need to cover DNAME as well? | ||
|
|
||
| *TODO*: Should a future version be able to include NS record | ||
| and DNSSEC information? |
There was a problem hiding this comment.
At first glance, it appears to be redundant. Is there a situation in which the owner is not simply the original hostname (for the first entry) or the preceding hostname (for subsequent entries)?
However, if we want to be able to include additional information in the future, it might make sense to swap the format around; rather than including a list of CNAME(s) and/or IP(s) and giving an owner for each, list the owner first and an inner list of useful records for each.
There was a problem hiding this comment.
The redundancy is why I'm include to leave it optional or just get rid of it. It could be helpful for cases such as DNSSEC in the future, or perhaps if we include NS records or other things,
| Proxy servers MUST NOT take action based on SVCB records. | ||
| In particular, the ipv4hint and ipv6hint SvcParams MUST NOT | ||
| be used by proxies for making connections. | ||
|
|
||
| *TODO*: Discuss if this is too restrictive |
There was a problem hiding this comment.
I think this probably is too restrictive, especially in the case of SVCB-naive clients. It means a high probability of the client having to open a new tunnel after processing the records, even though avoiding that is something this spec states as a goal.
It still seems reasonable for the proxy to chase AliasMode records, use hints, etc. to choose the target to connect to. The flip side, I suppose, would be if the proxy chooses a host whose ALPN list doesn't reflect the client's support, but that could be solved by requiring the server to choose based on the transport protocol implied by the method, and then preferring the default ALPN within that. Alternatively, the client could include an ALPN list in the request.
However, it's also totally reasonable to save that for a separate PR if we decide to do it.
There was a problem hiding this comment.
Part of why I'm wary of this is that it changes the Proxy behavior and semantics substantially. Rather than the proxy just returning additional information, here we start fundamentally changing behavior. (Would doing this count as an "Update" to any of the RFCs defining CONNECT, for example?)
| Clients that are SVCB-required ({{SVCB}} Section 3) MUST perform a DNS | ||
| resolution prior to making a CONNECT* request, as they will need to | ||
| obtain SVCB records and a TargetName. |
There was a problem hiding this comment.
Given the problems of doing such a resolution noted above, how would they do that? It might make more sense to say that a client which is SVCB-required must abandon the tunnel if it doesn't receive SVCB records from the proxy.
There was a problem hiding this comment.
They could do DoH through the proxy, for example. Or use ODoH. Or do something else. We could mention this in security considerations, but it seems like an implementation detail we don't want to get into? This is already covered in the core SVCB draft in some detail.
| *TODO*: Do we want to include text on how clients | ||
| can also use Proxy-DNS-Used for connection coalescing? | ||
| (One thing missing there is the full set of IP addresses | ||
| to look for overlaps within an address family. I'm not | ||
| sure this is worth the complexity. Or do we remove this | ||
| sub-section entirely?) |
There was a problem hiding this comment.
My vote is for removal. Coalescing based on IP address is not a battle we've been winning, and I don't think this draft is the venue to continue fighting it.
Co-authored-by: Mike Bishop <mbishop@evequefou.be>
Incorporating suggestions from Mike Bishop Co-authored-by: Mike Bishop <mbishop@evequefou.be>
Start an Acknowledgements section before we forget.
3 participants
No description provided.