Re: [Dnsmasq-discuss] [PATCH] Make ECC-GOST optional only
Oh, I think that were just typo when editing that file. Of course shouln't be there. Attached fixed patch. On 11/13/22 14:44, Geert Stappers via Dnsmasq-discuss wrote: On Thu, Nov 10, 2022 at 06:02:44PM +0100, Petr Menšík wrote: Hi! I were testing my builds on rootcanary.org test, where dnsmasq is the only one failing with DNSSEC validation enabled. I am not sure why, I think gost crypto algorithm might be broken intentionally on Fedora or RHEL for legal reason. But I have tested it on Debian unstable and the result were same. It passes other algorithms, but fails on this one. I have therefore made it possible to skip GOST support. In addition it makes that default as well. Is there any distribution, which has GOST support working? Is it possible that rootcanary.org has wrong signatures? All other implementations return already insecure status - not implemented algorithm. This change makes the same for dnsmasq. --- a/src/config.h +++ b/src/config.h @@ -198,6 +201,8 @@ RESOLVFILE /* #define HAVE_CONNTRACK */ /* #define HAVE_CRYPTOHASH */ /* #define HAVE_DNSSEC */ +/* #define HAVE_GOST */ +/* #define HAVE_GOST */ /* #define HAVE_NFTSET */ /* Default locations for important system files. */ Why twice the '/* #define HAVE_GOST */' line? Groeten Geert Stappers -- Petr Menšík Software Engineer, RHEL Red Hat, https://www.redhat.com/ PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB From 45c68f29f0fa3d202072cc51c7d7d2cf38b95e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Thu, 10 Nov 2022 17:50:11 +0100 Subject: [PATCH] Make ECC-GOST algorithm 12 optional only According to my testing on rootcanary.org, dnsmasq always fails to validate the record with algorithm 12. Make it disabled by default, because it fails both on Debian and Fedora. Enable it by -DCOPTS=HAVE_GOST define. --- src/config.h | 8 src/crypto.c | 8 +--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/config.h b/src/config.h index df1d985..a9df5ee 100644 --- a/src/config.h +++ b/src/config.h @@ -131,6 +131,9 @@ HAVE_CRYPTOHASH HAVE_DNSSEC include DNSSEC validator. +HAVE_GOST + include DNSSEC algorithm 12 (ECCGOST) support + HAVE_DUMPFILE include code to dump packets to a libpcap-format file for debugging. @@ -198,6 +201,7 @@ RESOLVFILE /* #define HAVE_CONNTRACK */ /* #define HAVE_CRYPTOHASH */ /* #define HAVE_DNSSEC */ +/* #define HAVE_GOST */ /* #define HAVE_NFTSET */ /* Default locations for important system files. */ @@ -442,6 +446,10 @@ static char *compile_opts = "no-" #endif "DNSSEC " +#ifndef HAVE_GOST +"no-" +#endif +"gost " #ifdef NO_ID "no-ID " #endif diff --git a/src/crypto.c b/src/crypto.c index 060e27f..8f36839 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -39,7 +39,7 @@ #if MIN_VERSION(3, 1) #include #endif -#if MIN_VERSION(3, 6) +#if defined(HAVE_GOST) && MIN_VERSION(3, 6) # include #endif @@ -281,7 +281,7 @@ static int dnsmasq_ecdsa_verify(struct blockdata *key_data, unsigned int key_len return nettle_ecdsa_verify(key, digest_len, digest, sig_struct); } -#if MIN_VERSION(3, 6) +#if defined(HAVE_GOST) && MIN_VERSION(3, 6) static int dnsmasq_gostdsa_verify(struct blockdata *key_data, unsigned int key_len, unsigned char *sig, size_t sig_len, unsigned char *digest, size_t digest_len, int algo) @@ -381,7 +381,7 @@ static int (*verify_func(int algo))(struct blockdata *key_data, unsigned int key case 5: case 7: case 8: case 10: return dnsmasq_rsa_verify; -#if MIN_VERSION(3, 6) +#if defined(HAVE_GOST) && MIN_VERSION(3, 6) case 12: return dnsmasq_gostdsa_verify; #endif @@ -444,7 +444,9 @@ char *algo_digest_name(int algo) case 7: return "sha1";/* RSASHA1-NSEC3-SHA1 */ case 8: return "sha256"; /* RSA/SHA-256 */ case 10: return "sha512"; /* RSA/SHA-512 */ +#ifdef HAVE_GOST case 12: return "gosthash94"; /* ECC-GOST */ +#endif case 13: return "sha256"; /* ECDSAP256SHA256 */ case 14: return "sha384"; /* ECDSAP384SHA384 */ case 15: return "null_hash"; /* ED25519 */ -- 2.38.1 ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] dnsmasq mishandles some cases when bad dns response packet is received
Interesting tests. But dnsmasq is somehow naive in parsing replied queries. It tries to deliver the response exactly as it were delivered to it. I think the main reason for it is it expects trusted resolvers to be used as a forwarding servers, not something bogus. Sure, I admit that might not be correct expectation. dnsmasq is minimalistic and tries to minimize the size of code and used resources. Therefore it does not do full parsing of the message and verification of every aspect in the response. I would recommend using Unbound for less trusted forwarders. I think all other implementations do not rely on recursive server doing the hard work, so they may encounter also less trusted responses. But dnsmasq should send queries to trusted forwarders only. It can therefore trust them to do more strict checking. But I admit we should add at least the most obvious checks. Would you please make the responses in ldns-testns [1] server format, so it would be easier to test it? It allows also encoding the body in hex format, so invalid responses are broken as well. It would be easier to test the bad behaviour and prepare fixes for them. Are those links leading to DNS in wire format? It would be simpler to read if pcap with them were used, wireshark would visualise those responses well. But as I said already, unlike other mentioned implementations, dnsmasq will accept responses ONLY from configured addresses. It will never use any other for iterative queries from root. Because it does not know how to do that. So if the forwarder ensures those packets have valid format, dnsmasq just relies on it. It is not possible to send query for attacker's name and get around the forwarder's checking. I think at least the 1st bug should be fixed, others can rely on forwarder's checks. Regards, Petr [1] https://linux.die.net/man/1/ldns-testns On 11/12/22 03:30, ZhangJiangyu 张江瑜 via Dnsmasq-discuss wrote: The rcode of the dnsmasq returned -- Petr Menšík Software Engineer, RHEL Red Hat, https://www.redhat.com/ PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] dnsmasq mishandles some cases when bad dns response packet is received
Hi， On Mon, Nov 15, 2022 at 8:15:00PM +0800, Petr Menšík wrote: > Interesting tests. > > But dnsmasq is somehow naive in parsing replied queries. It tries to > deliver the response exactly as it were delivered to it. I think the > main reason for it is it expects trusted resolvers to be used as a > forwarding servers, not something bogus. Sure, I admit that might not be > correct expectation. dnsmasq is minimalistic and tries to minimize the > size of code and used resources. Therefore it does not do full parsing > of the message and verification of every aspect in the response. > > I would recommend using Unbound for less trusted forwarders. I think all > other implementations do not rely on recursive server doing the hard > work, so they may encounter also less trusted responses. But dnsmasq > should send queries to trusted forwarders only. It can therefore trust > them to do more strict checking. Unfortunately the unbound also has the same problem, I also submitted an issue. Although the forwarders configured by dnsmasq are trusted, there are also some situations that need to be considered, such as, the dnsmasq configuration file has been maliciously changed, your trusted forwarder has the same error handling problem or the host computer where the forwarder is located is attacked, so dnsmasq needs to do some obvious wrong checks, and it is also necessary.it can't rely entirely on other parsers. > > But I admit we should add at least the most obvious checks. Would you > please make the responses in ldns-testns [1] server format, so it would > be easier to test it? It allows also encoding the body in hex format, so > invalid responses are broken as well. It would be easier to test the bad > behaviour and prepare fixes for them. Are those links leading to DNS in > wire format? It would be simpler to read if pcap with them were used, > wireshark would visualise those responses well. Yes, The message I provided is a wire format, but it is a bit different from the wire format, because the first four bytes are a length field. I removed the length field, it should be all dns request and reply messages, it is wire format. Below is the download link: origin request: https://643684107.oss-cn-beijing.aliyuncs.com/packet/request0 origin response: https://643684107.oss-cn-beijing.aliyuncs.com/packet/response0 request1: https://643684107.oss-cn-beijing.aliyuncs.com/packet/request1 response1: https://643684107.oss-cn-beijing.aliyuncs.com/packet/response1 request2: https://643684107.oss-cn-beijing.aliyuncs.com/packet/request2 response2: https://643684107.oss-cn-beijing.aliyuncs.com/packet/response2 request3: https://643684107.oss-cn-beijing.aliyuncs.com/packet/request3 response3: https://643684107.oss-cn-beijing.aliyuncs.com/packet/response3 Of course, in previous comminicate, Geert Stappers has already made a reproduced git repository, here: https://git.sr.ht/~stappers/cert_check_by_dnsmasq, you can use this to reproduce it. For ldns-testns, I don't know how to construct the corresponding data format, so I can only provide complete dns request and response messages. But I think the structure of these packets is relatively simple, and it is very easy to analyze where the problem is. > > But as I said already, unlike other mentioned implementations, dnsmasq > will accept responses ONLY from configured addresses. It will never use > any other for iterative queries from root. Because it does not know how > to do that. So if the forwarder ensures those packets have valid format, > dnsmasq just relies on it. It is not possible to send query for > attacker's name and get around the forwarder's checking. I think at > least the 1st bug should be fixed, others can rely on forwarder's checks. > I think the second bug is also an obvious one, since the domain name of the query is not the same as the answer, it deserves to be fixed. Thank you very much for your reply. Regards, P1n9 ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
