[Dnsmasq-discuss] Question about tcp_request in 2.91

[Simon Kelley]

On 28/02/2025 15:47, Tijs Van Buggenhout via Dnsmasq-discuss wrote:
> Hi Simon,
> 
> I have a question about commit 6656790f in regard changes to tcp_request
> function.
> 
> commit 6656790f2498f2a0b21086bc4ab47a2e38429a7c
> Author: Simon Kelley <simon at thekelleys.org.uk>
> Date:   Tue Jan 7 20:46:33 2025 +0000
> 
>      Handle queries with EDNS client subnet fields better.
> 
> The call to add_edns0_config moved up, even before the "find_pseudoheader"
> condition that sets have_pseudoheader. To my understanding add_edns0_config
> prepares the original request to be forwarded (with regard to EDNS).
> Since the function adds EDNS to the packet, the original query is altered even
> before it has been processed and decided whether to handle locally or forward,
> ie. it invalidates "have_pseudoheader".
> 
> The general logic in receive_query differs from tcp_request:
> 
> 1. checks
> 2. find_pseudoheader
> 3. flags
> 4. answer : {disallowed,auth, saved_question + request (from cache)} => send
> 5. forward if not (local) answered
> 
> vs
> 
> 1. checks
> 2. saved_question
> 3. find_pseudoheader
> 4. flags
> 5. answer : {disallowed,auth,request (from cache)} => send
> 6. forward if not (local) answered
> 

You are correct: that's a bug.

Since add_edns0_config() only changes the packet if --add-cpe-id or 
--add-mac or --add-subnet is configured it's not immediately obvious.

If one of those _is_ configured then a query via TCP which doesn't have 
an EDNS0 header will get a reply which does have an EDNS0 header and 
that's strictly incorrect.


Fixing this simply required the setting of have_pseudoheader to happen 
before the call to add_edns0_config()


I'll push a patch to that effect. At this point in the 2.91 release 
cycle, and given the grief that's been caused already by ill-advised 
last-minute changes, I prefer to fix this in the post-2.91 patch stream. 
I think the risks that the patch breaks something is greater than the 
risk that the bug breaks something, if that makes sense.


Cheers,

Simon.