Commit 2bd31a48 authored by Vladimír Čunát's avatar Vladimír Čunát Committed by Petr Špaček

validate nitpick fix: unsupported algo edge case

kr_dnskeys_trusted() semantics is changed, but I do NOT consider that
a part of public API.

Go insecure due to algorithm support even if DNSKEY is NODATA.
I can't see how that's relevant to practical usage, but I think this new
behavior makes more sense.  We still do try to fetch the DNSKEY even
though we have information about its un-usability beforehand.
I'd consider fixing that a premature optimization.
We'll still be affected if the DNSKEY query SERVFAILs or something.

Thanks to PowerDNS people for catching this!
parent 4c0e0e90
......@@ -37,6 +37,7 @@ Bugfixes
- fix flushing of messages to logs in some cases (!781)
- fix fallback when SERVFAIL or REFUSED is received from upstream (!784)
- fix crash when dealing with unknown TA key algorhitm (#449)
- go insecure due to algorithm support even if DNSKEY is NODATA (!798)
Module API changes
------------------
......
......@@ -268,8 +268,10 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx,
return vctx->result;
}
static bool kr_ds_algo_support(const knot_rrset_t *ta)
bool kr_ds_algo_support(const knot_rrset_t *ta)
{
assert(ta && ta->type == KNOT_RRTYPE_DS && ta->rclass == KNOT_CLASS_IN);
/* Check if at least one DS has a usable algorithm pair. */
knot_rdata_t *rdata_i = ta->rrs.rdata;
for (uint16_t i = 0; i < ta->rrs.count;
++i, rdata_i = knot_rdataset_next(rdata_i)) {
......@@ -293,12 +295,6 @@ int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta)
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.
......
......@@ -81,13 +81,17 @@ typedef struct kr_rrset_validation_ctx kr_rrset_validation_ctx_t;
int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx,
const knot_rrset_t *covered);
/**
* Return true iff the RRset contains at least one usable DS. See RFC6840 5.2.
*/
KR_EXPORT KR_PURE
bool kr_ds_algo_support(const knot_rrset_t *ta);
/**
* 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. In particular,
* DNSSEC_INVALID_DS_ALGORITHM if *each* DS records is unusable
* due to unimplemented DNSKEY or DS algorithm.
* @return 0 or error code, same as vctx->result.
*/
int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta);
......
......@@ -972,11 +972,8 @@ static int validate(kr_layer_t *ctx, knot_pkt_t *pkt)
}
if (knot_wire_get_aa(pkt->wire) && qtype == KNOT_RRTYPE_DNSKEY) {
ret = validate_keyset(req, pkt, has_nsec3);
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)) {
const knot_rrset_t *ds = qry->zone_cut.trust_anchor;
if (ds && !kr_ds_algo_support(ds)) {
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.DNSSEC_WANT = false;
......@@ -984,6 +981,12 @@ static int validate(kr_layer_t *ctx, knot_pkt_t *pkt)
rank_records(ctx, KR_RANK_INSECURE, qry->zone_cut.name);
mark_insecure_parents(qry);
return KR_STATE_DONE;
}
ret = validate_keyset(req, pkt, has_nsec3);
if (ret == kr_error(EAGAIN)) {
VERBOSE_MSG(qry, ">< cut changed, needs revalidation\n");
return KR_STATE_YIELD;
} else if (ret != 0) {
VERBOSE_MSG(qry, "<= bad keys, broken trust chain\n");
qry->flags.DNSSEC_BOGUS = true;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment