From cd8cd0a4ba0ae4cfb970ee52b7b77746800f46ca Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= Date: Mon, 26 Jun 2017 11:49:49 +0200 Subject: [PATCH] layer/validate: handle unknown algorithms i.e. downgrade a zone to insecure when *all* DNSKEYs of the apex are unverifiable due to unimplemented DNSKEY or DS algorithms. Fixes https://gitlab.labs.nic.cz/knot/resolver/issues/210 --- NEWS | 1 + lib/dnssec.c | 60 ++++++++++++++++++++++++++++++++++++++++-- lib/dnssec.h | 9 ++++--- lib/dnssec/signature.c | 8 +++--- lib/dnssec/signature.h | 3 ++- lib/layer/validate.c | 8 ++++++ 6 files changed, 79 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 9b9b53d24..06a803540 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,7 @@ Bugfixes - dns64: fix CNAME problems (#203) It still won't work with policy.STUB. - hints: better interpretation of hosts-like files (#204) also, error out if a bad entry is encountered in the file +- dnssec: handle unknown DNSKEY/DS algorithms (#210) Improvements ------------ diff --git a/lib/dnssec.c b/lib/dnssec.c index 6ed08a28d..dcfd82981 100644 --- a/lib/dnssec.c +++ b/lib/dnssec.c @@ -236,15 +236,70 @@ int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx, return vctx->result; } +/* Fallbacks: implemented in newer libdnssec. + * Note: changing some from true to false is NOT enough to fully remove the support. */ +#if KNOT_VERSION_HEX < ((2 << 16) | (6 << 8) | 0) + static bool dnssec_algorithm_key_support(dnssec_key_algorithm_t algo) + { + switch (algo) { + case DNSSEC_KEY_ALGORITHM_DSA_SHA1: + case DNSSEC_KEY_ALGORITHM_DSA_SHA1_NSEC3: + case DNSSEC_KEY_ALGORITHM_RSA_SHA1: + case DNSSEC_KEY_ALGORITHM_RSA_SHA1_NSEC3: + case DNSSEC_KEY_ALGORITHM_RSA_SHA256: + case DNSSEC_KEY_ALGORITHM_RSA_SHA512: + case DNSSEC_KEY_ALGORITHM_ECDSA_P256_SHA256: + case DNSSEC_KEY_ALGORITHM_ECDSA_P384_SHA384: + return true; + //case DNSSEC_KEY_ALGORITHM_ED25519: + //case DNSSEC_KEY_ALGORITHM_ED448: + default: + return false; + } + } + + static bool dnssec_algorithm_digest_support(dnssec_key_digest_t algo) + { + switch (algo) { + case DNSSEC_KEY_DIGEST_SHA1: + case DNSSEC_KEY_DIGEST_SHA256: + case DNSSEC_KEY_DIGEST_SHA384: + return true; + default: + return false; + }; + } +#endif + +static bool kr_ds_algo_support(const knot_rrset_t *ta) +{ + for (uint16_t i = 0; i < ta->rrs.rr_count; ++i) { + if (dnssec_algorithm_digest_support(knot_ds_digest_type(&ta->rrs, i)) + && dnssec_algorithm_key_support(knot_ds_alg(&ta->rrs, i))) { + return true; + } + } + return false; +} + int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta) { const knot_pkt_t *pkt = vctx->pkt; const knot_rrset_t *keys = vctx->keys; - if (!pkt || !keys || !ta) { + const bool ok = pkt && keys && ta && ta->rrs.rr_count && ta->rrs.data + && ta->type == KNOT_RRTYPE_DS; + if (!ok) { + assert(false); return kr_error(EINVAL); } + /* Check if at least one DS has a usable algorithm pair. */ + if (!kr_ds_algo_support(ta)) { + /* See RFC6840 5.2. */ + return vctx->result = kr_error(DNSSEC_INVALID_DS_ALGORITHM); + } + /* RFC4035 5.2, bullet 1 * The supplied DS record has been authenticated. * It has been validated or is part of a configured trust anchor. @@ -273,6 +328,7 @@ int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta) assert (vctx->result == 0); return vctx->result; } + /* No useable key found */ vctx->result = kr_error(ENOENT); return vctx->result; @@ -363,7 +419,7 @@ int kr_dnssec_key_from_rdata(struct dseckey **key, const knot_dname_t *kown, con ret = dnssec_key_set_rdata(new_key, &binary_key); if (ret != DNSSEC_EOK) { dnssec_key_free(new_key); - return kr_error(ENOMEM); + return kr_error(ret); } if (kown) { ret = dnssec_key_set_dname(new_key, kown); diff --git a/lib/dnssec.h b/lib/dnssec.h index 0f36a059b..ff173f692 100644 --- a/lib/dnssec.h +++ b/lib/dnssec.h @@ -85,9 +85,11 @@ int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx, size_t key_pos, const struct dseckey *key); /** * Check whether the DNSKEY rrset matches the supplied trust anchor RRSet. - * @param vctx Pointer to validation context. - * @param ta Trust anchor RRSet against which to validate the DNSKEY RRSet. - * @return 0 or error code, same as vctx->result. + * @param vctx Pointer to validation context. + * @param ta Trust anchor RRSet against which to validate the DNSKEY RRSet. + * @return 0 or error code, same as vctx->result. In particular, + * DNSSEC_INVALID_DS_ALGORITHM if *each* DS records is unusable + * due to unimplemented DNSKEY or DS algorithm. */ int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta); @@ -130,6 +132,7 @@ int kr_dnssec_key_match(const uint8_t *key_a_rdata, size_t key_a_rdlen, * @param kown DNSKEY owner name. * @param rdata DNSKEY RDATA * @param rdlen DNSKEY RDATA length + * @return 0 or error code; in particular: DNSSEC_INVALID_KEY_ALGORITHM */ int kr_dnssec_key_from_rdata(struct dseckey **key, const knot_dname_t *kown, const uint8_t *rdata, size_t rdlen); diff --git a/lib/dnssec/signature.c b/lib/dnssec/signature.c index d523d4d58..7c5ad16f9 100644 --- a/lib/dnssec/signature.c +++ b/lib/dnssec/signature.c @@ -40,7 +40,6 @@ static int authenticate_ds(const dnssec_key_t *key, dnssec_binary_t *ds_rdata, u dnssec_binary_t computed_ds = {0, }; int ret = dnssec_key_create_ds(key, digest_type, &computed_ds); if (ret != DNSSEC_EOK) { - ret = kr_error(ENOMEM); goto fail; } @@ -53,7 +52,7 @@ static int authenticate_ds(const dnssec_key_t *key, dnssec_binary_t *ds_rdata, u fail: dnssec_binary_free(&computed_ds); - return ret; + return kr_error(ret); } int kr_authenticate_referral(const knot_rrset_t *ref, const dnssec_key_t *key) @@ -73,11 +72,12 @@ int kr_authenticate_referral(const knot_rrset_t *ref, const dnssec_key_t *key) }; ret = authenticate_ds(key, &ds_rdata, knot_ds_digest_type(&ref->rrs, i)); if (ret == 0) { /* Found a good DS */ - break; + return kr_ok(); } rd = kr_rdataset_next(rd); } - return ret; + + return kr_error(ret); } /** diff --git a/lib/dnssec/signature.h b/lib/dnssec/signature.h index 72a003802..7bc2abe57 100644 --- a/lib/dnssec/signature.h +++ b/lib/dnssec/signature.h @@ -23,7 +23,8 @@ * Performs referral authentication according to RFC4035 5.2, bullet 2 * @param ref Referral RRSet. Currently only DS can be used. * @param key Already parsed key. - * @return 0 or error code. + * @return 0 or error code. In particular: DNSSEC_INVALID_DS_ALGORITHM + * in case *all* DSs in ref use an unimplemented algorithm. */ int kr_authenticate_referral(const knot_rrset_t *ref, const dnssec_key_t *key); diff --git a/lib/layer/validate.c b/lib/layer/validate.c index 0fa41ab90..da73c225a 100644 --- a/lib/layer/validate.c +++ b/lib/layer/validate.c @@ -887,6 +887,14 @@ static int validate(kr_layer_t *ctx, knot_pkt_t *pkt) if (ret == kr_error(EAGAIN)) { VERBOSE_MSG(qry, ">< cut changed, needs revalidation\n"); return KR_STATE_YIELD; + } else if (ret == kr_error(DNSSEC_INVALID_DS_ALGORITHM)) { + VERBOSE_MSG(qry, ">< all DS entries use unsupported algorithm pairs, going insecure\n"); + /* ^ the message is a bit imprecise to avoid being too verbose */ + qry->flags &= ~QUERY_DNSSEC_WANT; + qry->flags |= QUERY_DNSSEC_INSECURE; + rank_records(ctx, KR_RANK_INSECURE); + mark_insecure_parents(qry); + return KR_STATE_DONE; } else if (ret != 0) { VERBOSE_MSG(qry, "<= bad keys, broken trust chain\n"); qry->flags |= QUERY_DNSSEC_BOGUS; -- 2.47.2