-----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