[Dnsmasq-discuss] dnsmasq mishandles some cases when bad dns response packet is received

Thu Nov 24 11:28:57 UTC 2022

Should we think about an option, let's say strict-dns, which would 
toggle additional checks on incoming packets? Like ensuring the packet 
is not longer than expected. New extensions have been added to EDNS 
pseudosection, I doubt that would change anytime soon. It may filter out 
also not matching class in responses, when such class were not used in 
the question. Though that would require explicit remembering of class, 
which is not stored now. Class is used only as part of hash in question, 
so checking class in answer and additional section is not possible now.

While I agree clients should be fixed too, network DNS cache is often 
the most prominent place to add checks. We support blocking lists. We 
might allow also some additional checks. Because more and more devices 
lacks updates after few years of service. At least some checks possible 
would be great option.

I don't think dnsmasq fully qualifies as this proxy. It has even EDNS 
support and is usually caching.

I agree the resolver must accept any query and class combination and 
deliver a response for it. But I am not sure whether resolver must 
deliver response with a different class from the original query. I doubt 
that is mandatory by any RFC. I am not sure this is meant by

https://www.rfc-editor.org/rfc/rfc5625.html#section-4.3

Cheers,
Petr

On 11/23/22 23:22, Simon Kelley wrote:
>
>
> On 23/11/2022 02:56, zhangjiangyu via Dnsmasq-discuss wrote:
>> Hi,
>>
>> On 23/11/2022 07:21, Simon via Dnsmasq-discuss wrote:
>>>
>>> The main argument for this seems to be a security one: the client may
>>> not handle a malformed packet, and a suitably crafted malformed packet
>>> may compromise the client with a buffer overflow or similar. If the
>>> client is sending all requests via dnsmasq, then dnsmasq should detect
>>> and eliminate malformed packets from upstream rather than sending them
>>> to the client as this protects the client from compromise.
>>>
>>> If we have a client which is vulnerable to malformed packets then the
>>> solution is to fix the client, not put it behind dnsmasq. Putting the
>>> client behind dnsmasq and relying on dnsmasq to intercept malformed
>>> packets is not fix since and attacker may be able to send malformed
>>> packets direct to the client anyway or the configuration may change 
>>> such
>>> that the client talks directly to other servers which the attacker may
>>> have control over. Using dnsmasq to filter malformed packets from the
>>> client is at best fragile and at worst doesn't work. It also transfers
>>> the responsibility to handle untrusted data from the client to dnsmasq,
>>> which is the wrong approach.
>>>
>>> Dnsmasq has to accept malformed packets from the internet without
>>> crashing or being compromised, and, as far as anyone knows, it does.
>>> Because of the fairly unique architecture of dnsmasq, altering it to
>>> only return known correctly formed packets is a large change which adds
>>> to the code footprint, increases that chance that something which is in
>>> fact correct but encodes DNS data which dnsmasq doesn't understand will
>>> be rejected by mistake, and can't guarantee to catch all malformed
>>> packets anyway.
>>>
>>> I can see the argument for flagging packets which dnsmasq detects are
>>> malformed as part of it's code to extract data for caching, but I don't
>>> think attempting to detect all malformed packets is required. the only
>>> reason to do that is to protect vulnerable clients, and that problem is
>>> fixed by fixing the clients.
>>>
>>
>> Ok, Now I understand that the fairly unique architecture of dnsmasq 
>> cannot verify all malformed packets. But I think there should be a 
>> basic detection of the packets. sure, dnsmasq does not need to judge 
>> the legality of each record resource data.
>>
>> |like this basic record structure:
>> |                                    1  1  1  1  1  1
>> |      0  1  2  3  4  5  6  7  8  9  0  1  2  3  4  5
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
>> |    |                                               |
>> |    /                                               /
>> |    /                      NAME                     /
>> |    |                                               |
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
>> |    |                      TYPE                     |
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
>> |    |                     CLASS                     |
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
>> |    |                      TTL                      |
>> |    |                                               |
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
>> |    |                   RDLENGTH                    |
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--|
>> |    /                     RDATA                     /
>> |    /                                               /
>> |    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
>>
>> The structure of the record is stationary, and the rfc stipulates 
>> that the value of the class has a certain scope and meaning, so the 
>> clear regulation of the dns rfc should be checked and necessary. But 
>> for the first bug, there is really no check, I think this should be 
>> fixed.
>>
>> |The following CLASS mnemonics and values are defined:
>> |
>> |IN              1 the Internet
>> |
>> |CS              2 the CSNET class (Obsolete - used only for examples in
>> |                some obsolete RFCs)
>> |
>> |CH              3 the CHAOS class
>> |
>> |HS              4 Hesiod [Dyer 87]
>>
>> Thank you very much for your reply.
>>
>
> This is a perfect example of why dnsmasq should not reject packets it 
> can't understand. If the IETF should define a new class tomorrow, 
> every release of dnsmasq since 2000 would be quite happy to forward 
> queries in that class and return results. The same goes for classes 
> between 65280 and 65534 which are reserved for private use in RFC 6895.
>
> https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-2 
>
>
> This is all set out explicitly in section 3 of RFC 5625
>
> https://datatracker.ietf.org/doc/html/rfc5625#page-3
>
> Quote:
>
>    The role of the proxy should therefore be no more and no less than to
>    receive DNS requests from clients on the LAN side, forward those
>    verbatim to one of the known upstream recursive resolvers on the WAN
>    side, and ensure that the whole response is returned verbatim to the
>    original client.
>
>
> Cheers,
>
> Simon.
>
>
>
>> Thanks,
>> P1n9
>>
>>>
>>> Cheers,
>>>
>>> Simon.
>>>
>>> _______________________________________________
>>> Dnsmasq-discuss mailing list
>>> Dnsmasq-discuss at lists.thekelleys.org.uk
>>> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss 
>>>
>> _______________________________________________
>> Dnsmasq-discuss mailing list
>> Dnsmasq-discuss at lists.thekelleys.org.uk
>> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>>
>
> _______________________________________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss at lists.thekelleys.org.uk
> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
>
-- 
Petr Menšík
Software Engineer, RHEL
Red Hat, https://www.redhat.com/
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB