From d475f3aeedbb0dff940ff5bd25c71fcfc3a71f95 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Tue, 23 Jun 2020 10:26:01 +1000
Subject: [PATCH 1/4] The validator could fail when
 select_signing_key/get_dst_key failed

to select the signing key because the algorithm was not supported
and the loop was prematurely aborted.
---
 lib/dns/validator.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index 9b5c891f94..e31ad661e5 100644
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -1126,24 +1126,25 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) {
 		INSIST(val->key == NULL);
 		result = dst_key_fromdns(&siginfo->signer, rdata.rdclass, &b,
 					 val->view->mctx, &val->key);
-		if (result != ISC_R_SUCCESS) {
-			goto failure;
-		}
-		if (siginfo->algorithm == (dns_secalg_t)dst_key_alg(val->key) &&
-		    siginfo->keyid == (dns_keytag_t)dst_key_id(val->key) &&
-		    dst_key_iszonekey(val->key))
-		{
-			if (foundold) {
-				/*
-				 * This is the key we're looking for.
-				 */
-				return (ISC_R_SUCCESS);
-			} else if (dst_key_compare(oldkey, val->key)) {
-				foundold = true;
-				dst_key_free(&oldkey);
+		if (result == ISC_R_SUCCESS) {
+			if (siginfo->algorithm ==
+				    (dns_secalg_t)dst_key_alg(val->key) &&
+			    siginfo->keyid ==
+				    (dns_keytag_t)dst_key_id(val->key) &&
+			    dst_key_iszonekey(val->key))
+			{
+				if (foundold) {
+					/*
+					 * This is the key we're looking for.
+					 */
+					return (ISC_R_SUCCESS);
+				} else if (dst_key_compare(oldkey, val->key)) {
+					foundold = true;
+					dst_key_free(&oldkey);
+				}
 			}
+			dst_key_free(&val->key);
 		}
-		dst_key_free(&val->key);
 		dns_rdata_reset(&rdata);
 		result = dns_rdataset_next(rdataset);
 	} while (result == ISC_R_SUCCESS);

From b733bd6555cbbe99e5cc7b4c384475d6e8046e68 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Tue, 23 Jun 2020 10:52:09 +1000
Subject: [PATCH 2/4] Add CHANGES for [GL #1689]

---
 CHANGES | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CHANGES b/CHANGES
index 1ef3b55bf8..79790b05a9 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,9 @@
+5446.	[bug]		The validator could fail to accept a properly signed
+			RRset if an unsupported algorithm appeared earlier in
+			the DNSKEY RRset than a supported algorithm.  It could
+			also stop if it detected a malformed public key.
+			[GL #1689]
+
 5445.	[cleanup]	Disable and disallow static linking. [GL #1933]
 
 5444.	[bug]		'rndc dnstap -roll <value>' was not limiting the

From e195d4608a2990f548de6ab2fec9900436312b1f Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Tue, 23 Jun 2020 10:56:07 +1000
Subject: [PATCH 3/4] Add Release Note for [GL #1689]

---
 doc/notes/notes-current.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst
index 8c6e86a3b9..ce8ddc72b4 100644
--- a/doc/notes/notes-current.rst
+++ b/doc/notes/notes-current.rst
@@ -48,3 +48,8 @@ Bug Fixes
 
 - 'rndc dnstap -roll <value>' was not limiting the number of saved
   files to <value>. [GL !3728]
+
+- The validator could fail to accept a properly signed RRset if an
+  unsupported algorithm appeared earlier in the DNSKEY RRset than a
+  supported algorithm.  It could also stop if it detected a malformed
+  public key. [GL #1689]

From c6345fffe96fb6b71eb1a736e528a811b037288d Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Thu, 25 Jun 2020 13:39:24 +0200
Subject: [PATCH 4/4] Add todo in dnssec system test for [GL #1689]

Add a note why we don't have a test case for the issue.

It is tricky to write a good test case for this if our tools are
not allowed to create signatures for unsupported algorithms.
---
 bin/tests/system/dnssec/tests.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh
index 84e4ad627b..562be71122 100644
--- a/bin/tests/system/dnssec/tests.sh
+++ b/bin/tests/system/dnssec/tests.sh
@@ -3562,6 +3562,13 @@ n=$((n+1))
 test "$ret" -eq 0 || echo_i "failed"
 status=$((status+ret))
 
+# TODO: test case for GL #1689.
+# If we allow the dnssec tools to use deprecated algorithms (such as RSAMD5)
+# we could write a test that signs a zone with supported and unsupported
+# algorithm, apply a fixed rrset order such that the unsupported algorithm
+# precedes the supported one in the DNSKEY RRset, and verify the result still
+# validates succesfully.
+
 echo_i "check that a lone non matching CDNSKEY record is rejected ($n)"
 ret=0
 (