From 9e1bed5bdb8a642f73566eb22bcbfa2f9c97f453 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 21 Aug 2024 17:12:30 +0200 Subject: [PATCH 1/2] Adjust kasp system test to get keys which signed If there is a keytag conflict between keys with different algorithms, we need to supply what key algorithm is used so we can get the right public key. For clarity, print the algorithm on the found keys after 'check_keys'. (cherry picked from commit 7bb6d8250563534cec077763f1f0887d233a413c) --- bin/tests/system/kasp.sh | 42 +++++++++++++++++++--------------- bin/tests/system/kasp/tests.sh | 8 +++---- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/bin/tests/system/kasp.sh b/bin/tests/system/kasp.sh index 60d77335ec..4ee9363817 100644 --- a/bin/tests/system/kasp.sh +++ b/bin/tests/system/kasp.sh @@ -161,13 +161,19 @@ _rndccmd() { "$RNDC" -c ../_common/rndc.conf -p "$CONTROLPORT" -s "$@" } -# Print IDs of keys used for generating RRSIG records for RRsets of type $1 -# found in dig output file $2. +# Print IDs of keys used for generating RRSIG records for RRsets of type $1, +# matching algorithm number $2, found in dig output file $3. +# If $2 is equal to 0, any algorithm matches. get_keys_which_signed() { _qtype=$1 - _output=$2 + _alg=$2 + _output=$3 # The key ID is the 11th column of the RRSIG record line. - awk -v qt="$_qtype" '$4 == "RRSIG" && $5 == qt {print $11}' <"$_output" + if [ "$_alg" = "0" ]; then + awk -v qt="$_qtype" '$4 == "RRSIG" && $5 == qt {print $11}' <"$_output" + else + awk -v alg="$_alg" -v qt="$_qtype" '$4 == "RRSIG" && $5 == qt && $6 == alg {print $11}' <"$_output" + fi } # Get the key ids from key files for zone $2 in directory $1. @@ -810,19 +816,19 @@ check_keys() { ret=0 if [ "$(key_get KEY1 EXPECT)" = "yes" ]; then - echo_i "KEY1 ID $(key_get KEY1 ID)" + echo_i "KEY1 ID $(key_get KEY1 ID) ALG $(key_get KEY1 ALG_STR)" test "no" = "$(key_get KEY1 ID)" && _log_error "No KEY1 found for zone ${ZONE}" fi if [ "$(key_get KEY2 EXPECT)" = "yes" ]; then - echo_i "KEY2 ID $(key_get KEY2 ID)" + echo_i "KEY2 ID $(key_get KEY2 ID) ALG $(key_get KEY2 ALG_STR)" test "no" = "$(key_get KEY2 ID)" && _log_error "No KEY2 found for zone ${ZONE}" fi if [ "$(key_get KEY3 EXPECT)" = "yes" ]; then - echo_i "KEY3 ID $(key_get KEY3 ID)" + echo_i "KEY3 ID $(key_get KEY3 ID) ALG $(key_get KEY3 ALG_STR)" test "no" = "$(key_get KEY3 ID)" && _log_error "No KEY3 found for zone ${ZONE}" fi if [ "$(key_get KEY4 EXPECT)" = "yes" ]; then - echo_i "KEY4 ID $(key_get KEY4 ID)" + echo_i "KEY4 ID $(key_get KEY4 ID) ALG $(key_get KEY4 ALG_STR)" test "no" = "$(key_get KEY4 ID)" && _log_error "No KEY4 found for zone ${ZONE}" fi test "$ret" -eq 0 || echo_i "failed" @@ -908,34 +914,34 @@ _check_signatures() { fi if [ "$(key_get KEY1 "$_expect_type")" = "yes" ] && [ "$(key_get KEY1 "$_role")" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY1 ID)$" >/dev/null || return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY1 ALG_NUM)" "$_file" | grep "^$(key_get KEY1 ID)$" >/dev/null || return 1 numsigs=$((numsigs + 1)) elif [ "$(key_get KEY1 EXPECT)" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY1 ID)$" >/dev/null && return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY1 ALG_NUM)" "$_file" | grep "^$(key_get KEY1 ID)$" >/dev/null && return 1 fi if [ "$(key_get KEY2 "$_expect_type")" = "yes" ] && [ "$(key_get KEY2 "$_role")" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY2 ID)$" >/dev/null || return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY2 ALG_NUM)" "$_file" | grep "^$(key_get KEY2 ID)$" >/dev/null || return 1 numsigs=$((numsigs + 1)) elif [ "$(key_get KEY2 EXPECT)" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY2 ID)$" >/dev/null && return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY2 ALG_NUM)" "$_file" | grep "^$(key_get KEY2 ID)$" >/dev/null && return 1 fi if [ "$(key_get KEY3 "$_expect_type")" = "yes" ] && [ "$(key_get KEY3 "$_role")" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY3 ID)$" >/dev/null || return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY3 ALG_NUM)" "$_file" | grep "^$(key_get KEY3 ID)$" >/dev/null || return 1 numsigs=$((numsigs + 1)) elif [ "$(key_get KEY3 EXPECT)" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY3 ID)$" >/dev/null && return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY3 ALG_NUM)" "$_file" | grep "^$(key_get KEY3 ID)$" >/dev/null && return 1 fi if [ "$(key_get KEY4 "$_expect_type")" = "yes" ] && [ "$(key_get KEY4 "$_role")" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY4 ID)$" >/dev/null || return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY4 ALG_NUM)" "$_file" | grep "^$(key_get KEY4 ID)$" >/dev/null || return 1 numsigs=$((numsigs + 1)) elif [ "$(key_get KEY4 EXPECT)" = "yes" ]; then - get_keys_which_signed "$_qtype" "$_file" | grep "^$(key_get KEY4 ID)$" >/dev/null && return 1 + get_keys_which_signed "$_qtype" "$(key_get KEY4 ALG_NUM)" "$_file" | grep "^$(key_get KEY4 ID)$" >/dev/null && return 1 fi - lines=$(get_keys_which_signed "${_qtype}" "${_file}" | wc -l) + lines=$(get_keys_which_signed "${_qtype}" "0" "${_file}" | wc -l) test "$lines" -eq "$numsigs" || echo_i "bad number of signatures for $_qtype (got $lines, expected $numsigs)" test "$lines" -eq "$numsigs" || return 1 @@ -1158,7 +1164,7 @@ check_subdomain() { _dig_with_opts "a.$ZONE" "@${SERVER}" $_qtype >"dig.out.$DIR.test$n" || _log_error "dig a.${ZONE} ${_qtype} failed" grep "status: NOERROR" "dig.out.$DIR.test$n" >/dev/null || _log_error "mismatch status in DNS response" grep "a.${ZONE}\..*${DEFAULT_TTL}.*IN.*${_qtype}.*10\.0\.0\.1" "dig.out.$DIR.test$n" >/dev/null || _log_error "missing a.${ZONE} ${_qtype} record in response" - lines=$(get_keys_which_signed $_qtype "dig.out.$DIR.test$n" | wc -l) + lines=$(get_keys_which_signed $_qtype 0 "dig.out.$DIR.test$n" | wc -l) check_signatures $_qtype "dig.out.$DIR.test$n" "ZSK" test "$ret" -eq 0 || echo_i "failed" status=$((status + ret)) diff --git a/bin/tests/system/kasp/tests.sh b/bin/tests/system/kasp/tests.sh index dbeabcf381..000d30eb16 100644 --- a/bin/tests/system/kasp/tests.sh +++ b/bin/tests/system/kasp/tests.sh @@ -353,18 +353,18 @@ update_is_signed() { dig_with_opts "a.${ZONE}" "@${SERVER}" A >"dig.out.$DIR.test$n.a" || return 1 grep "status: NOERROR" "dig.out.$DIR.test$n.a" >/dev/null || return 1 grep "a.${ZONE}\..*${DEFAULT_TTL}.*IN.*A.*${ip_a}" "dig.out.$DIR.test$n.a" >/dev/null || return 1 - lines=$(get_keys_which_signed A "dig.out.$DIR.test$n.a" | wc -l) + lines=$(get_keys_which_signed A 0 "dig.out.$DIR.test$n.a" | wc -l) test "$lines" -eq 1 || return 1 - get_keys_which_signed A "dig.out.$DIR.test$n.a" | grep "^${KEY_ID}$" >/dev/null || return 1 + get_keys_which_signed A 0 "dig.out.$DIR.test$n.a" | grep "^${KEY_ID}$" >/dev/null || return 1 fi if [ "$ip_d" != "-" ]; then dig_with_opts "d.${ZONE}" "@${SERVER}" A >"dig.out.$DIR.test$n".d || return 1 grep "status: NOERROR" "dig.out.$DIR.test$n".d >/dev/null || return 1 grep "d.${ZONE}\..*${DEFAULT_TTL}.*IN.*A.*${ip_d}" "dig.out.$DIR.test$n".d >/dev/null || return 1 - lines=$(get_keys_which_signed A "dig.out.$DIR.test$n".d | wc -l) + lines=$(get_keys_which_signed A 0 "dig.out.$DIR.test$n".d | wc -l) test "$lines" -eq 1 || return 1 - get_keys_which_signed A "dig.out.$DIR.test$n".d | grep "^${KEY_ID}$" >/dev/null || return 1 + get_keys_which_signed A 0 "dig.out.$DIR.test$n".d | grep "^${KEY_ID}$" >/dev/null || return 1 fi } From bb3bf561575371fd597b9b43c3d5cff6ec9608bb Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Wed, 21 Aug 2024 17:14:48 +0200 Subject: [PATCH 2/2] Fix algorithm rollover bug wrt keytag conflicts If there is an algorithm rollover and two keys of different algorithm share the same keytags, then there is a possibility that if we check that a key matches a specific state, we are checking against the wrong key. Fix this by not only checking for matching key id but also key algorithm. (cherry picked from commit f37eb33f29ad50cead2673f4f7634839ef7e2a26) --- lib/dns/keymgr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/dns/keymgr.c b/lib/dns/keymgr.c index 4fbebbcb6d..0f6f818e11 100644 --- a/lib/dns/keymgr.c +++ b/lib/dns/keymgr.c @@ -605,6 +605,7 @@ keymgr_key_match_state(dst_key_t *key, dst_key_t *subject, int type, continue; } if (next_state != NA && i == type && + dst_key_alg(key) == dst_key_alg(subject) && dst_key_id(key) == dst_key_id(subject)) { /* Check next state rather than current state. */