-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

My first thought on reading this is that it's a big hole. All an
attacker needs to do is supply a false DS with nonsense algorithm
numbers to turn off validation. Then I thought again, and realised
that's not actually true, as long as the DS records validate. If the
DS RRset has a valid signature, but no algorithm we can use, then it's
fine to treat it the same as an NSEC/NSEC3 asserting there's no DS
record. There are a few wrinkles in doing that, so I need to find some
time to think through the patch, but the principle looks like a good
one, and this, or something like it, will get applied.


Thanks for your efforts.


Cheers,

Simon.



On 17/11/15 11:01, Micha? K?pie? wrote:
> --- 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");
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJWTdn2AAoJEBXN2mrhkTWiNjsP/06eyffCZJJU/4a3xz7k5mPg
wxrOWBIetn0CiplrLxjzWrL98e85TFlp9Zyvbi15dXUJ+/n3CLGxulBWodgZ7lGR
43mjysUl86SMxJxHfQePCCQX8P8EkHSW3z1nDDZbZ6BmPhPSz0BkI41RpJXV689f
fiCUSjoi8LcRNKjqZBnvn7NgPZH6yiBLTtnTPPsiObI//7FkOZYnUXvzBijxjZg4
HQ5UyqasOPi2sSsA5R9C8H45vDPAr/z0d/rY6G3XJ5ZBOl+VlYqmWnVd6lFo23gj
72fLNBBbdj4nYV0QV9ELT9j/FNATqXkbZi7ddtTpy6p/Kyko5J3rtehZwcfGPArH
3zOAGYl5Hz92nJNgAKKXQrErFMdRbMFKsgQWmJrhze0+n14ebzKPMzqU4sLUNTXx
Tc4lEApoea1LLdc4peRMxyuUOMOUXJFP5kDXHFAv+XIS5zmJycjgSxMxZWiWfcNw
AR8oUaP5wdSoK+GtVJkdzuNyYDy2MezYPFICreGRkp5ak2YA50c0xAZake1gygK1
JCt3gGNl7XNgshWhCg6e6KKxA1/KoZyxu4wF76faTHnIj9+NnbcaV4qtdCXGalOr
YSEDwXPRm9REvjbrbOwOkj6+LKdtFKJ7ECNNpF+Itc1KRDj1/TJHgLBp8Ykuodcu
g+YCVKkO2PCqDFcffpoX
=4hpG
-----END PGP SIGNATURE-----

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Reply via email to