[Dnsmasq-discuss] [PATCH] Treat records signed using unknown algorithms as unsigned instead of bogus

Michał Kępień michal.kepien at nask.pl
Tue Nov 17 11:01:04 GMT 2015


---
When dnsmasq is running with DNSSEC validation enabled, it returns
SERVFAIL when trying to resolve any record within a zone which uses a
signing algorithm it doesn't understand.  This behavior doesn't play
nicely with RFC 4035, section 5.2:

    If the resolver does not support any of the algorithms listed in an
    authenticated DS RRset, then the resolver will not be able to verify
    the authentication path to the child zone.  In this case, the
    resolver SHOULD treat the child zone as if it were unsigned.

My reading of the above is that e.g. if a resolver encounters a zone for
which there is a single DS with a signing/hashing algorithm it doesn't
understand, it should treat that zone as unsigned, not bogus.

I would venture to say that this rule should not be applied solely to DS
records, but also to record signatures in general.  A quick glance at
the latest src/dnssec.c shows that both validate_rrset() and
dnssec_validate_by_ds() return STAT_BOGUS when they are unable to
perform validation, no matter the reason.  Meanwhile, if the hash_find()
call fails for all DS records for a given zone (or for all RRSIGs for a
given RRset), it simply means that dnsmasq doesn't know how to perform
validation, not that the data is outright bogus.  The same holds true
for verify() which simply returns 0 when either signature verification
fails or the signature algorithm is not supported, which are two
distinct cases.

This patch attempts to solve the issue by tracking how many of the
attempted validations failed due to unknown algorithms being used.  It
probably needs some polishing and more thorough testing, but hopefully
conveys the idea.

 src/dnssec.c | 64 ++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/src/dnssec.c b/src/dnssec.c
index 67ce486..aa22e4e 100644
--- a/src/dnssec.c
+++ b/src/dnssec.c
@@ -291,7 +291,7 @@ static int dnsmasq_ecdsa_verify(struct blockdata *key_data, unsigned int key_len
 #endif 
 
 static int 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)
+		  unsigned char *digest, size_t digest_len, int algo, int *val_unable)
 {
   (void)digest_len;
 
@@ -309,6 +309,7 @@ static int verify(struct blockdata *key_data, unsigned int key_len, unsigned cha
 #endif
     }
   
+  (*val_unable)++;
   return 0;
 }
 
@@ -712,7 +713,7 @@ static void sort_rrset(struct dns_header *header, size_t plen, u16 *rr_desc, int
    STAT_SECURE_WILDCARD if it validates and is the result of wildcard expansion.
    (In this case *wildcard_out points to the "body" of the wildcard within name.) 
    STAT_NO_SIG no RRsigs found.
-   STAT_INSECURE RRset empty.
+   STAT_INSECURE RRset empty or all used signature algorithms are unknown
    STAT_BOGUS    signature is wrong, bad packet.
    STAT_NEED_KEY need DNSKEY to complete validation (name is returned in keyname)
 
@@ -728,7 +729,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
   static int rrset_sz = 0, sig_sz = 0;
   
   unsigned char *p;
-  int rrsetidx, sigidx, res, rdlen, j, name_labels;
+  int rrsetidx, sigidx, res, rdlen, j, name_labels, val_attempts = 0, val_unable = 0;
   struct crec *crecp = NULL;
   int type_covered, algo, labels, orig_ttl, sig_expiration, sig_inception, key_tag;
   u16 *rr_desc = get_desc(type);
@@ -820,6 +821,8 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
       char *name_start;
       u32 nsigttl;
       
+      val_attempts++;
+
       p = sigs[j];
       GETSHORT(rdlen, p); /* rdlen >= 18 checked previously */
       psav = p;
@@ -861,11 +864,16 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
       
       /* Other 5.3.1 checks */
       if (!check_date_range(sig_inception, sig_expiration) ||
-	  labels > name_labels ||
-	  !(hash = hash_find(algo_digest_name(algo))) ||
-	  !hash_init(hash, &ctx, &digest))
+	  labels > name_labels)
 	continue;
-	
+
+      if (!(hash = hash_find(algo_digest_name(algo))) ||
+	  !hash_init(hash, &ctx, &digest))
+	{
+	  val_unable++;
+	  continue;
+	}
+
       /* OK, we have the signature record, see if the relevant DNSKEY is in the cache. */
       if (!key && !(crecp = cache_find_by_name(NULL, keyname, now, F_DNSKEY)))
 	return STAT_NEED_KEY;
@@ -950,7 +958,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
       if (key)
 	{
 	  if (algo_in == algo && keytag_in == key_tag &&
-	      verify(key, keylen, sig, sig_len, digest, hash->digest_size, algo))
+	      verify(key, keylen, sig, sig_len, digest, hash->digest_size, algo, &val_unable))
 	    return STAT_SECURE;
 	}
       else
@@ -960,18 +968,20 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 	    if (crecp->addr.key.algo == algo && 
 		crecp->addr.key.keytag == key_tag &&
 		crecp->uid == (unsigned int)class &&
-		verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo))
+		verify(crecp->addr.key.keydata, crecp->addr.key.keylen, sig, sig_len, digest, hash->digest_size, algo, &val_unable))
 	      return (labels < name_labels) ? STAT_SECURE_WILDCARD : STAT_SECURE;
 	}
     }
 
-  return STAT_BOGUS;
+  return val_attempts == val_unable ? STAT_INSECURE : STAT_BOGUS;
 }
  
 /* The DNS packet is expected to contain the answer to a DNSKEY query.
    Put all DNSKEYs in the answer which are valid into the cache.
    return codes:
          STAT_SECURE   At least one valid DNSKEY found and in cache.
+	 STAT_INSECURE DNSKEY RRset could not be validated because all
+	               DS/RRSIG records are using unknown algorithms
 	 STAT_BOGUS    No DNSKEYs found, which  can be validated with DS,
 	               or self-sign for DNSKEY RRset is not valid, bad packet.
 	 STAT_NEED_DS  DS records to validate a key not found, name in keyname 
@@ -980,7 +990,7 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
 {
   unsigned char *psave, *p = (unsigned char *)(header+1);
   struct crec *crecp, *recp1;
-  int rc, j, qtype, qclass, ttl, rdlen, flags, algo, valid, keytag, type_covered;
+  int rc, j, qtype, qclass, ttl, rdlen, flags, algo, valid, keytag, type_covered, val_attempts = 0, val_unable = 0;
   struct blockdata *key;
   struct all_addr a;
 
@@ -1060,11 +1070,18 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
 	  
 	  if (recp1->addr.ds.algo == algo && 
 	      recp1->addr.ds.keytag == keytag &&
-	      recp1->uid == (unsigned int)class &&
-	      (hash = hash_find(ds_digest_name(recp1->addr.ds.digest))) &&
-	      hash_init(hash, &ctx, &digest))
-	    
+	      recp1->uid == (unsigned int)class)
 	    {
+
+	      val_attempts++;
+
+	      if (!(hash = hash_find(ds_digest_name(recp1->addr.ds.digest))) ||
+	          !hash_init(hash, &ctx, &digest))
+	        {
+		  val_unable++;
+	          continue;
+	        }
+
 	      int wire_len = to_wire(name);
 	      
 	      /* Note that digest may be different between DSs, so 
@@ -1077,11 +1094,18 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
 	      
 	      if (recp1->addr.ds.keylen == (int)hash->digest_size &&
 		  (ds_digest = blockdata_retrieve(recp1->addr.key.keydata, recp1->addr.ds.keylen, NULL)) &&
-		  memcmp(ds_digest, digest, recp1->addr.ds.keylen) == 0 &&
-		  validate_rrset(now, header, plen, class, T_DNSKEY, name, keyname, NULL, key, rdlen - 4, algo, keytag) == STAT_SECURE)
+		  memcmp(ds_digest, digest, recp1->addr.ds.keylen) == 0)
 		{
-		  valid = 1;
-		  break;
+		  rc = validate_rrset(now, header, plen, class, T_DNSKEY, name, keyname, NULL, key, rdlen - 4, algo, keytag);
+		  if (rc == STAT_SECURE)
+		    {
+		      valid = 1;
+		      break;
+		    }
+		  else if (rc == STAT_INSECURE)
+		    {
+		      val_unable++;
+		    }
 		}
 	    }
 	}
@@ -1186,6 +1210,8 @@ int dnssec_validate_by_ds(time_t now, struct dns_header *header, size_t plen, ch
       /* commit cache insert. */
       cache_end_insert();
       return STAT_SECURE;
+    } else if (val_attempts == val_unable) {
+      return STAT_INSECURE;
     }
 
   log_query(F_NOEXTRA | F_UPSTREAM, name, NULL, "BOGUS DNSKEY");
-- 
2.6.2




More information about the Dnsmasq-discuss mailing list