Commit c5654da7 authored by Vladimír Čunát's avatar Vladimír Čunát

lib/resolve: failing states in answer finalization

Mainly reduce duplication of state and request->state,
and slightly reorganize the code.
parent 746044fe
...@@ -553,7 +553,8 @@ static int answer_padding(struct kr_request *request) ...@@ -553,7 +553,8 @@ static int answer_padding(struct kr_request *request)
return kr_ok(); return kr_ok();
} }
static int answer_fail(struct kr_request *request) /* Make a clean SERVFAIL answer. */
static void answer_fail(struct kr_request *request)
{ {
/* Note: OPT in SERVFAIL response is still useful for cookies/additional info. */ /* Note: OPT in SERVFAIL response is still useful for cookies/additional info. */
knot_pkt_t *answer = request->answer; knot_pkt_t *answer = request->answer;
...@@ -566,31 +567,36 @@ static int answer_fail(struct kr_request *request) ...@@ -566,31 +567,36 @@ static int answer_fail(struct kr_request *request)
knot_pkt_begin(answer, KNOT_ADDITIONAL); knot_pkt_begin(answer, KNOT_ADDITIONAL);
answer_padding(request); /* Ignore failed padding in SERVFAIL answer. */ answer_padding(request); /* Ignore failed padding in SERVFAIL answer. */
answer->opt_rr = opt_rr; answer->opt_rr = opt_rr;
ret = edns_put(answer, false); edns_put(answer, false);
} }
return ret;
} }
static int answer_finalize(struct kr_request *request, int state) static void answer_finalize(struct kr_request *request)
{ {
struct kr_rplan *rplan = &request->rplan; struct kr_rplan *rplan = &request->rplan;
knot_pkt_t *answer = request->answer; knot_pkt_t *answer = request->answer;
/* Always set SERVFAIL for bogus answers. */ struct kr_query *const last =
if ((state & KR_STATE_FAIL) && rplan->pending.len > 0) { rplan->resolved.len > 0 ? array_tail(rplan->resolved) : NULL;
struct kr_query *last = array_tail(rplan->pending);
if ((last->flags.DNSSEC_WANT) && (last->flags.DNSSEC_BOGUS)) {
return answer_fail(request);
}
}
struct kr_query *last = rplan->resolved.len > 0 ? array_tail(rplan->resolved) : NULL;
/* TODO ^^^^ this is slightly fragile */ /* TODO ^^^^ this is slightly fragile */
if (!last) {
/* Suspicious: no kr_query got resolved (not even from cache),
* so let's (defensively) SERVFAIL the request.
* ATM many checks below depend on `last` anyway,
* so this helps to avoid surprises. */
return answer_fail(request); /*< suspicious */
}
/* TODO: clean this up in !660 or followup, and it isn't foolproof anyway. */
if (last->flags.DNSSEC_BOGUS
|| (rplan->pending.len > 0 && array_tail(rplan->pending)->flags.DNSSEC_BOGUS)) {
return answer_fail(request);
}
/* AD flag. We can only change `secure` from true to false. /* AD flag. We can only change `secure` from true to false.
* Be conservative. Primary approach: check ranks of all RRs in wire. * Be conservative. Primary approach: check ranks of all RRs in wire.
* Only "negative answers" need special handling. */ * Only "negative answers" need special handling. */
bool secure = last != NULL && state == KR_STATE_DONE /*< suspicious otherwise */ bool secure = last != NULL && request->state == KR_STATE_DONE /*< suspicious otherwise */
&& knot_pkt_qtype(answer) != KNOT_RRTYPE_RRSIG; && knot_pkt_qtype(answer) != KNOT_RRTYPE_RRSIG;
if (last && (last->flags.STUB)) { if (last && (last->flags.STUB)) {
secure = false; /* don't trust forwarding for now */ secure = false; /* don't trust forwarding for now */
...@@ -674,8 +680,6 @@ static int answer_finalize(struct kr_request *request, int state) ...@@ -674,8 +680,6 @@ static int answer_finalize(struct kr_request *request, int state)
if (!secure) { if (!secure) {
knot_wire_clear_ad(answer->wire); knot_wire_clear_ad(answer->wire);
} }
return kr_ok();
} }
static int query_finalize(struct kr_request *request, struct kr_query *qry, knot_pkt_t *pkt) static int query_finalize(struct kr_request *request, struct kr_query *qry, knot_pkt_t *pkt)
...@@ -1592,23 +1596,24 @@ int kr_resolve_checkout(struct kr_request *request, const struct sockaddr *src, ...@@ -1592,23 +1596,24 @@ int kr_resolve_checkout(struct kr_request *request, const struct sockaddr *src,
int kr_resolve_finish(struct kr_request *request, int state) int kr_resolve_finish(struct kr_request *request, int state)
{ {
request->state = state;
/* Finalize answer and construct wire-buffer. */ /* Finalize answer and construct wire-buffer. */
ITERATE_LAYERS(request, NULL, answer_finalize); ITERATE_LAYERS(request, NULL, answer_finalize);
if (answer_finalize(request, state) != 0) { answer_finalize(request);
state = KR_STATE_FAIL;
}
/* Error during processing, internal failure */ /* Defensive style, in case someone has forgotten.
if (state != KR_STATE_DONE) { * Beware: non-empty answers do make sense even with SERVFAIL case, etc. */
knot_pkt_t *answer = request->answer; if (request->state != KR_STATE_DONE) {
if (knot_wire_get_rcode(answer->wire) == KNOT_RCODE_NOERROR) { uint8_t *wire = request->answer->wire;
knot_wire_clear_ad(answer->wire); switch (knot_wire_get_rcode(wire)) {
knot_wire_clear_aa(answer->wire); case KNOT_RCODE_NOERROR:
knot_wire_set_rcode(answer->wire, KNOT_RCODE_SERVFAIL); case KNOT_RCODE_NXDOMAIN:
knot_wire_clear_ad(wire);
knot_wire_clear_aa(wire);
knot_wire_set_rcode(wire, KNOT_RCODE_SERVFAIL);
} }
} }
request->state = state;
ITERATE_LAYERS(request, NULL, finish); ITERATE_LAYERS(request, NULL, finish);
#ifndef NOVERBOSELOG #ifndef NOVERBOSELOG
......
...@@ -308,7 +308,7 @@ int kr_resolve_checkout(struct kr_request *request, const struct sockaddr *src, ...@@ -308,7 +308,7 @@ int kr_resolve_checkout(struct kr_request *request, const struct sockaddr *src,
* be destroyed, as it's owned by caller. * be destroyed, as it's owned by caller.
* *
* @param request request state * @param request request state
* @param state either DONE or FAIL state * @param state either DONE or FAIL state (to be assigned to request->state)
* @return DONE * @return DONE
*/ */
KR_EXPORT KR_EXPORT
......
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