- Fix #983: Sha1 runtime insecure change was incomplete.

diff --git a/doc/Changelog b/doc/Changelog
index bd916e8fe1b30844f383e9fa8901e3e855ec970c..dbc625f007c0d5f438f37019c1bac32efa3c0346 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -2,6 +2,7 @@
        - Merge #980: DoH: reject non-h2 early. To fix #979: Improve errors
          for non-HTTP/2 DoH clients.
        - Merge #985: Add DoH and DoT to dnstap message.
+       - Fix #983: Sha1 runtime insecure change was incomplete.
 
 22 December 2023: Yorgos
        - Update example.conf with cookie options.

diff --git a/validator/val_sigcrypt.c b/validator/val_sigcrypt.c
index 37730f17967623de276b2d40c910c8aa8b1c3aef..f169c64fbf0320a87b011cdca4196794297fc649 100644 (file)
--- a/validator/val_sigcrypt.c
+++ b/validator/val_sigcrypt.c
@@ -717,7 +717,6 @@ dnskey_verify_rrset(struct module_env* env, struct val_env* ve,
                if(sec == sec_status_indeterminate)
                        numindeterminate ++;
        }
-       verbose(VERB_ALGO, "rrset failed to verify: all signatures are bogus");
        if(!numchecked) {
                *reason = "signature for expected key and algorithm missing";
                if(reason_bogus)
@@ -730,6 +729,7 @@ dnskey_verify_rrset(struct module_env* env, struct val_env* ve,
                *reason = "algorithm refused by cryptolib";
                return sec_status_indeterminate;
        }
+       verbose(VERB_ALGO, "rrset failed to verify: all signatures are bogus");
        return sec_status_bogus;
 }
 

diff --git a/validator/val_utils.c b/validator/val_utils.c
index 8b388882b82a6acac93aa684cfb270f15cf21f6e..8b1267954aafe863dd8eca90798853cc063f698e 100644 (file)
--- a/validator/val_utils.c
+++ b/validator/val_utils.c
@@ -416,11 +416,13 @@ static enum sec_status
 verify_dnskeys_with_ds_rr(struct module_env* env, struct val_env* ve,
        struct ub_packed_rrset_key* dnskey_rrset,
         struct ub_packed_rrset_key* ds_rrset, size_t ds_idx, char** reason,
-       sldns_ede_code *reason_bogus, struct module_qstate* qstate)
+       sldns_ede_code *reason_bogus, struct module_qstate* qstate,
+       int *nonechecked)
 {
        enum sec_status sec = sec_status_bogus;
        size_t i, num, numchecked = 0, numhashok = 0, numsizesupp = 0;
        num = rrset_get_count(dnskey_rrset);
+       *nonechecked = 0;
        for(i=0; i<num; i++) {
                /* Skip DNSKEYs that don't match the basic criteria. */
                if(ds_get_key_algo(ds_rrset, ds_idx) 
@@ -462,13 +464,15 @@ verify_dnskeys_with_ds_rr(struct module_env* env, struct val_env* ve,
                /* there is a working DS, but that DNSKEY is not supported */
                return sec_status_insecure;
        }
-       if(numchecked == 0)
+       if(numchecked == 0) {
                algo_needs_reason(env, ds_get_key_algo(ds_rrset, ds_idx),
                        reason, "no keys have a DS");
-       else if(numhashok == 0)
+               *nonechecked = 1;
+       } else if(numhashok == 0) {
                *reason = "DS hash mismatches key";
-       else if(!*reason)
+       } else if(!*reason) {
                *reason = "keyset not secured by DNSKEY that matches DS";
+       }
        return sec_status_bogus;
 }
 
@@ -497,7 +501,8 @@ val_verify_DNSKEY_with_DS(struct module_env* env, struct val_env* ve,
 {
        /* as long as this is false, we can consider this DS rrset to be
         * equivalent to no DS rrset. */
-       int has_useful_ds = 0, digest_algo, alg;
+       int has_useful_ds = 0, digest_algo, alg, has_algo_refusal = 0,
+               nonechecked, has_checked_ds = 0;
        struct algo_needs needs;
        size_t i, num;
        enum sec_status sec;
@@ -530,9 +535,16 @@ val_verify_DNSKEY_with_DS(struct module_env* env, struct val_env* ve,
                }
 
                sec = verify_dnskeys_with_ds_rr(env, ve, dnskey_rrset,
-                       ds_rrset, i, reason, reason_bogus, qstate);
-               if(sec == sec_status_insecure)
+                       ds_rrset, i, reason, reason_bogus, qstate,
+                       &nonechecked);
+               if(sec == sec_status_insecure) {
+                       /* DNSKEY too large unsupported or algo refused by
+                        * crypto lib. */
+                       has_algo_refusal = 1;
                        continue;
+               }
+               if(!nonechecked)
+                       has_checked_ds = 1;
 
                /* Once we see a single DS with a known digestID and 
                 * algorithm, we cannot return INSECURE (with a 
@@ -557,6 +569,15 @@ val_verify_DNSKEY_with_DS(struct module_env* env, struct val_env* ve,
 
        /* None of the DS's worked out. */
 
+       /* If none of the DSes have been checked, eg. that means no matches
+        * for keytags, and the other dses are all algo_refusal, it is an
+        * insecure delegation point, since the only matched DS records
+        * have an algo refusal, or are unsupported. */
+       if(has_algo_refusal && !has_checked_ds) {
+               verbose(VERB_ALGO, "No supported DS records were found -- "
+                       "treating as insecure.");
+               return sec_status_insecure;
+       }
        /* If no DSs were understandable, then this is OK. */
        if(!has_useful_ds) {
                verbose(VERB_ALGO, "No usable DS records were found -- "
@@ -610,7 +631,8 @@ val_verify_DNSKEY_with_TA(struct module_env* env, struct val_env* ve,
 {
        /* as long as this is false, we can consider this anchor to be
         * equivalent to no anchor. */
-       int has_useful_ta = 0, digest_algo = 0, alg;
+       int has_useful_ta = 0, digest_algo = 0, alg, has_algo_refusal = 0,
+               nonechecked, has_checked_ds = 0;
        struct algo_needs needs;
        size_t i, num;
        enum sec_status sec;
@@ -656,9 +678,13 @@ val_verify_DNSKEY_with_TA(struct module_env* env, struct val_env* ve,
                        continue;
 
                sec = verify_dnskeys_with_ds_rr(env, ve, dnskey_rrset,
-                       ta_ds, i, reason, reason_bogus, qstate);
-               if(sec == sec_status_insecure)
+                       ta_ds, i, reason, reason_bogus, qstate, &nonechecked);
+               if(sec == sec_status_insecure) {
+                       has_algo_refusal = 1;
                        continue;
+               }
+               if(!nonechecked)
+                       has_checked_ds = 1;
 
                /* Once we see a single DS with a known digestID and 
                 * algorithm, we cannot return INSECURE (with a 
@@ -714,6 +740,15 @@ val_verify_DNSKEY_with_TA(struct module_env* env, struct val_env* ve,
            }
        }
 
+       /* If none of the DSes have been checked, eg. that means no matches
+        * for keytags, and the other dses are all algo_refusal, it is an
+        * insecure delegation point, since the only matched DS records
+        * have an algo refusal, or are unsupported. */
+       if(has_algo_refusal && !has_checked_ds) {
+               verbose(VERB_ALGO, "No supported trust anchors were found -- "
+                       "treating as insecure.");
+               return sec_status_insecure;
+       }
        /* If no DSs were understandable, then this is OK. */
        if(!has_useful_ta) {
                verbose(VERB_ALGO, "No usable trust anchors were found -- "