Great! I tried comparing the implementation of GOST to bind or unbound, which I have seen before. But found they both lack any implementation of GOST. Anyway, It seems working fine. Tried it on my Fedora 36 and in CentOS 8 container. Centos as expected resolves GOST related algorithms without AD flag, but this time without SERVFAILs at all. So the issue seems fixed both on nettle 3.4 and 3.8.

But I have noticed strange if in one related changes. I believe unsupported DNSKEY algorithms checks should have been there. Attached fixup patch.

Thanks for fixing also ED448 algorithm. Of course in this case my previous patch is not needed for anything, because yours fixes it in a better way. Just take a look at this patch.

Thanks!

Petr

On 11/16/22 18:23, Simon Kelley wrote:


On 10/11/2022 17:02, 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.

Opinions on that?



This sent me down something of a rabbit hole.

I've found and fixed multiple issues.

1) The code relies on algo_digest_name() returning NULL for any signature algorithm that's not supported, but the combination of a bug in the #defines in that code and a bug in libnettle made that not always true, so the DNSSEC code tried to validate things it shouldn't and failed,resulting in a SERVFAIL.

https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=f52cfdd8c37e09d77abdc151a4ddcf94f49f4821

fixes that.


2) The code to handle the GOST algorithms was broken; it used the wrong digest parameters and got the byte-order of the keys wrong and the order of the fields in the signature wrong. libnettle needs to be version 3.6 or later to support the correct digest and signature algo.

https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=1f9215f5f92c5478c8aaba8054d192a5e6280e95


fixes.

3) The handling of unsupported hash algorithms in DS records was wrong. A correctly signed DS RRset which contains only unsupported hashes should be treated the same as a signed empty DS RRset and allow answers for the domain it covers to be returned unvalidated, and not to generate SERVFAIL responses.

https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=9ed3ee67ecd2388d319bff116b27bcc62286ccc

Fixes that.

With those three patches, rootcanary.org gives sensible results, both with dnsmasq compiled against Nettle-3.5 (which doesn't support ED488 and GOST) and complied against Nettle-3.8, which does support both those. In the nettle-3.8 case records signed with RSA-MD5, DSA and DSA-NSEC-SHA1 are returned without being verified as specified in the DNSSEC Validation column of RFC 8624 para 3.1 The GOST signature and DS hash algorithms are still marked as MAY in that column, so I think it's fine to leave them in.


Given the above, I don't think there's a need to be able to disable the GOST algorithms, but I'm happy to hear arguments to the contrary.


Cheers,

Simon.

Cheers,
Petr


--
Petr Menšík
Software Engineer, RHEL
Red Hat, https://www.redhat.com/
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From 47bafee561f492c5fad7429c9043d97f95e31d50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Mon, 21 Nov 2022 22:25:52 +0100
Subject: [PATCH] fixup! Handle DS records for unsupported crypto algorithms
 correctly.

---
 src/dnssec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dnssec.c b/src/dnssec.c
index ce01b2e..219ba9a 100644
--- a/src/dnssec.c
+++ b/src/dnssec.c
@@ -1068,7 +1068,7 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char
 	      algo = *p++;
 	      digest = *p++;
 	      
-	      if (!ds_digest_name(digest) || !ds_digest_name(digest))
+	      if (!ds_digest_name(digest) || !algo_digest_name(algo))
 		{
 		  a.log.keytag = keytag;
 		  a.log.algo = algo;
-- 
2.38.1

_______________________________________________
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