[Dnsmasq-discuss] [PATCH] Treat records signed using unknown algorithms as unsigned instead of bogus
Simon Kelley
simon at thekelleys.org.uk
Thu Nov 19 14:17:27 GMT 2015
-----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-----
More information about the Dnsmasq-discuss
mailing list