From: Petr Špaček Date: Thu, 20 Dec 2018 16:32:53 +0000 (+0100) Subject: dnssec: improve bogus logging to give more info X-Git-Tag: v3.2.1~16^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8962c21d60decfab89309d2c5ac3d673b5d21733;p=thirdparty%2Fknot-resolver.git dnssec: improve bogus logging to give more info --- diff --git a/lib/dnssec.c b/lib/dnssec.c index 97be89ff2..84cdb57c2 100644 --- a/lib/dnssec.c +++ b/lib/dnssec.c @@ -72,28 +72,33 @@ static int validate_rrsig_rr(int *flags, int cov_labels, const knot_rdata_t *rrsigs, const knot_dname_t *key_owner, const knot_rdata_t *key_rdata, uint16_t keytag, - const knot_dname_t *zone_name, uint32_t timestamp) + const knot_dname_t *zone_name, uint32_t timestamp, + kr_rrset_validation_ctx_t *vctx) { if (!flags || !rrsigs || !key_owner || !key_rdata || !zone_name) { return kr_error(EINVAL); } /* bullet 5 */ if (knot_rrsig_sig_expiration(rrsigs) < timestamp) { + vctx->rrs_counters.expired++; return kr_error(EINVAL); } /* bullet 6 */ if (knot_rrsig_sig_inception(rrsigs) > timestamp) { + vctx->rrs_counters.notyet++; return kr_error(EINVAL); } /* bullet 2 */ const knot_dname_t *signer_name = knot_rrsig_signer_name(rrsigs); if (!signer_name || !knot_dname_is_equal(signer_name, zone_name)) { + vctx->rrs_counters.signer_invalid++; return kr_error(EAGAIN); } /* bullet 4 */ { int rrsig_labels = knot_rrsig_labels(rrsigs); if (rrsig_labels > cov_labels) { + vctx->rrs_counters.labels_invalid++; return kr_error(EINVAL); } if (rrsig_labels < cov_labels) { @@ -105,6 +110,7 @@ static int validate_rrsig_rr(int *flags, int cov_labels, if ((!knot_dname_is_equal(key_owner, signer_name)) || (knot_dnskey_alg(key_rdata) != knot_rrsig_alg(rrsigs)) || (keytag != knot_rrsig_key_tag(rrsigs))) { + vctx->rrs_counters.key_invalid++; return kr_error(EINVAL); } /* bullet 8 */ @@ -141,6 +147,7 @@ int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *cover return kr_error(EINVAL); } + memset(&vctx->rrs_counters, 0, sizeof(vctx->rrs_counters)); for (unsigned i = 0; i < vctx->keys->rrs.count; ++i) { int ret = kr_rrset_validate_with_key(vctx, covered, i, NULL); if (ret == 0) { @@ -211,9 +218,10 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx, if (knot_rrsig_type_covered(rdata_j) != covered->type) { continue; } + vctx->rrs_counters.matching_name_type++; int ret = validate_rrsig_rr(&val_flgs, covered_labels, rdata_j, keys->owner, key_rdata, keytag, - zone_name, timestamp); + zone_name, timestamp, vctx); if (ret == kr_error(EAGAIN)) { kr_dnssec_key_free(&created_key); vctx->result = ret; @@ -228,6 +236,7 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx, } } if (kr_check_signature(rdata_j, (dnssec_key_t *) key, covered, trim_labels) != 0) { + vctx->rrs_counters.crypto_invalid++; continue; } if (val_flgs & FLG_WILDCARD_EXPANSION) { @@ -242,6 +251,7 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx, } } if (ret != 0) { + vctx->rrs_counters.nsec_invalid++; continue; } vctx->flags |= KR_DNSSEC_VFLG_WEXPAND; @@ -293,6 +303,7 @@ int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta) * The supplied DS record has been authenticated. * It has been validated or is part of a configured trust anchor. */ + memset(&vctx->rrs_counters, 0, sizeof(vctx->rrs_counters)); for (uint16_t i = 0; i < keys->rrs.count; ++i) { /* RFC4035 5.3.1, bullet 8 */ /* ZSK */ /* LATER(optim.): more efficient way to iterate than _at() */ diff --git a/lib/dnssec.h b/lib/dnssec.h index 7c6d2a52a..b54908c18 100644 --- a/lib/dnssec.h +++ b/lib/dnssec.h @@ -58,6 +58,16 @@ struct kr_rrset_validation_ctx { uint32_t flags; /*!< Output - Flags. */ uint32_t err_cnt; /*!< Output - Number of validation failures. */ int result; /*!< Output - 0 or error code. */ + struct { + unsigned int matching_name_type; /*!< Name + type matches */ + unsigned int expired; + unsigned int notyet; + unsigned int signer_invalid; /*!< Signer is not zone apex */ + unsigned int labels_invalid; /*!< Number of labels in RRSIG */ + unsigned int key_invalid; /*!< Algorithm/keytag/key owner */ + unsigned int crypto_invalid; + unsigned int nsec_invalid; + } rrs_counters; /*!< Error counters for single RRset validation. */ }; typedef struct kr_rrset_validation_ctx kr_rrset_validation_ctx_t; diff --git a/lib/layer/validate.c b/lib/layer/validate.c index 44d8aa3b0..55c3ad774 100644 --- a/lib/layer/validate.c +++ b/lib/layer/validate.c @@ -77,6 +77,23 @@ static bool pkt_has_type(const knot_pkt_t *pkt, uint16_t type) return section_has_type(knot_pkt_section(pkt, KNOT_ADDITIONAL), type); } +static void log_bogus_rrsig(kr_rrset_validation_ctx_t *vctx, const struct kr_query *qry, + const knot_rrset_t *rr, const char *msg) { + WITH_VERBOSE(qry) { + auto_free char *name_text = kr_dname_text(rr->owner); + auto_free char *type_text = kr_rrtype_text(rr->type); + VERBOSE_MSG(qry, ">< %s: %s %s " + "(%u matching RRSIGs, %u expired, %u not yet valid, " + "%u invalid signer, %u invalid label count, %u invalid key, " + "%u invalid crypto, %u invalid NSEC)\n", + msg, name_text, type_text, vctx->rrs_counters.matching_name_type, + vctx->rrs_counters.expired, vctx->rrs_counters.notyet, + vctx->rrs_counters.signer_invalid, vctx->rrs_counters.labels_invalid, + vctx->rrs_counters.key_invalid, vctx->rrs_counters.crypto_invalid, + vctx->rrs_counters.nsec_invalid); + } +} + static int validate_section(kr_rrset_validation_ctx_t *vctx, const struct kr_query *qry, knot_mm_t *pool) { @@ -120,11 +137,8 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, const struct kr_que kr_rank_set(&entry->rank, KR_RANK_SECURE); } else if (kr_rank_test(rank_orig, KR_RANK_TRY)) { - WITH_VERBOSE(qry) { - auto_free char *name_text = kr_dname_text(rr->owner); - auto_free char *type_text = kr_rrtype_text(rr->type); - VERBOSE_MSG(qry, ">< failed to validate non-authoritative data but continuing: %s %s\n", name_text, type_text); - } + log_bogus_rrsig(vctx, qry, rr, + "failed to validate non-authoritative data but continuing"); vctx->result = kr_ok(); kr_rank_set(&entry->rank, KR_RANK_TRY); /* ^^ BOGUS would be more accurate, but it might change @@ -135,10 +149,11 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, const struct kr_que /* no RRSIGs found */ kr_rank_set(&entry->rank, KR_RANK_MISSING); vctx->err_cnt += 1; - + log_bogus_rrsig(vctx, qry, rr, "no valid RRSIGs found"); } else { kr_rank_set(&entry->rank, KR_RANK_BOGUS); vctx->err_cnt += 1; + log_bogus_rrsig(vctx, qry, rr, "bogus signatures"); } } return kr_ok(); @@ -247,6 +262,10 @@ static int validate_keyset(struct kr_request *req, knot_pkt_t *answer, bool has_ }; int ret = kr_dnskeys_trusted(&vctx, qry->zone_cut.trust_anchor); if (ret != 0) { + if (ret != kr_error(DNSSEC_INVALID_DS_ALGORITHM) && + ret != kr_error(EAGAIN)) { + log_bogus_rrsig(&vctx, qry, qry->zone_cut.key, "bogus key"); + } knot_rrset_free(qry->zone_cut.key, qry->zone_cut.pool); qry->zone_cut.key = NULL; return ret; @@ -583,11 +602,6 @@ static int check_validation_result(kr_layer_t *ctx, ranked_rr_array_t *arr) VERBOSE_MSG(qry, ">< cut changed (new signer), needs revalidation\n"); ret = KR_STATE_YIELD; } else if (kr_rank_test(invalid_entry->rank, KR_RANK_MISSING)) { - WITH_VERBOSE(qry) { - auto_free char *name_text = kr_dname_text(invalid_entry->rr->owner); - auto_free char *type_text = kr_rrtype_text(invalid_entry->rr->type); - VERBOSE_MSG(qry, ">< no valid RRSIGs found for %s %s\n", name_text, type_text); - } ret = rrsig_not_found(ctx, rr); } else if (!kr_rank_test(invalid_entry->rank, KR_RANK_SECURE)) { qry->flags.DNSSEC_BOGUS = true;