Hi,

On 24/11/2022 06:22, Simon via Dnsmasq-discuss 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.
> _______________________________________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss@lists.thekelleys.org.uk
> https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss

After I read rfc5625, I understand what you mean. you are right.
|Section 4.3 writes:
|
|Similarly, all responses MUST be proxied regardless of the values of
|   the TYPE and CLASS fields of any Resource Record therein.

In fact, there are corresponding security considerations in rfc5625, and the 
requirements for packet filtering are proposed. These points are consistent 
with my previous description of dnsmasq returning SERVFAIL. The explicitly 
cited example, "incorrect counts for the Question, Answer, Authority, and 
Additional Sections", has the same cause as our finding 
"request3-response3-combination"(Thanks to Geert Stappers for the correction). 
The expected behavior for this test case should return rcode as SERVFAIL.

|Section 4.3 writes:
|
|Examples of malformed packets that MAY be dropped include:
|
|   o  invalid compression pointers (i.e., those that point outside of
|      the current packet or that might cause a parsing loop)
|
|   o  incorrect counts for the Question, Answer, Authority, and
|      Additional Sections (although care should be taken where
|      truncation is a possibility)
|
|In these circumstances, proxies SHOULD synthesise a suitable DNS
|error response to the client (i.e., SERVFAIL) instead of dropping the
|packet completely.  This will allow the client to detect the error
|immediately.

Here is another bug for the same reason:
https://www.mail-archive.com/dnsmasq-discuss@lists.thekelleys.org.uk/msg16552.html

Thank you very much for your reply.

Thanks,
P1n9
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss

Reply via email to