diff --git a/daemon/io.c b/daemon/io.c index 548100371c3e5cc541833ab68f95abd2f1f6e3e9..67b927cc353fdccf270b44456b29e83ade93627e 100644 --- a/daemon/io.c +++ b/daemon/io.c @@ -53,18 +53,9 @@ void udp_recv(uv_udp_t *handle, ssize_t nread, const uv_buf_t *buf, { uv_loop_t *loop = handle->loop; struct worker_ctx *worker = loop->data; - - /* UDP requests are oneshot, always close afterwards */ - if (handle->data && !uv_is_closing((uv_handle_t *)handle)) { /* Do not free master socket */ - io_close((uv_handle_t *)handle); - } - - /* Check the incoming wire length. */ - if (nread > KNOT_WIRE_HEADER_SIZE) { - knot_pkt_t *query = knot_pkt_new(buf->base, nread, worker->mm); - worker_exec(worker, (uv_handle_t *)handle, query, addr); - knot_pkt_free(&query); - } + knot_pkt_t *query = knot_pkt_new(buf->base, nread, worker->mm); + worker_exec(worker, (uv_handle_t *)handle, query, addr); + knot_pkt_free(&query); } int udp_bind(struct endpoint *ep, struct sockaddr *addr) @@ -94,23 +85,23 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) uv_loop_t *loop = handle->loop; struct worker_ctx *worker = loop->data; - /* Check for connection close */ - if (nread <= 0) { + /* Check for originator connection close */ + if (nread <= 0 && handle->data == 0) { io_close((uv_handle_t *)handle); return; } else if (nread < 2) { /* Not enough bytes to read length */ + worker_exec(worker, (uv_handle_t *)handle, NULL, NULL); return; } - /* Set packet size */ /** @todo This is not going to work if the packet is fragmented in the stream ! */ uint16_t nbytes = wire_read_u16((const uint8_t *)buf->base); - - /* Check if there's enough data and execute */ if (nbytes + 2 < nread) { + worker_exec(worker, (uv_handle_t *)handle, NULL, NULL); return; } + knot_pkt_t *query = knot_pkt_new(buf->base + 2, nbytes, worker->mm); worker_exec(worker, (uv_handle_t *)handle, query, NULL); knot_pkt_free(&query); diff --git a/daemon/worker.c b/daemon/worker.c index 55fbbd9a475b17f619c1c3967957f0b9aacc7f3f..abd8cb5ead3e7603d2c13ae7c8fcdc0cbe5ba265 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -110,9 +110,8 @@ static void qr_task_free(uv_handle_t *handle) static void qr_task_timeout(uv_timer_t *req) { struct qr_task *task = req->data; - if (!uv_is_closing(task->next_handle)) { + if (task->next_handle) { io_stop_read(task->next_handle); - uv_close(task->next_handle, (uv_close_cb) free); qr_task_step(task, NULL); } } @@ -127,6 +126,7 @@ static void qr_task_on_send(uv_req_t* req, int status) io_start_read(task->next_handle); } } else { /* Finalize task */ + uv_timer_stop(&task->timeout); uv_close((uv_handle_t *)&task->timeout, qr_task_free); } } @@ -171,9 +171,12 @@ static int qr_task_finalize(struct qr_task *task, int state) static int qr_task_step(struct qr_task *task, knot_pkt_t *packet) { - /* Cancel timeout if active */ - uv_timer_stop(&task->timeout); - task->next_handle = NULL; + /* Cancel timeout if active, close handle. */ + if (task->next_handle) { + uv_close(task->next_handle, (uv_close_cb) free); + uv_timer_stop(&task->timeout); + task->next_handle = NULL; + } /* Consume input and produce next query */ int sock_type = -1; @@ -187,10 +190,7 @@ static int qr_task_step(struct qr_task *task, knot_pkt_t *packet) /* We're done, no more iterations needed */ if (state & (KNOT_STATE_DONE|KNOT_STATE_FAIL)) { return qr_task_finalize(task, state); - } - - /* Iteration limit */ - if (++task->iter_count > KR_ITER_LIMIT) { + } else if (++task->iter_count > KR_ITER_LIMIT) { return qr_task_finalize(task, KNOT_STATE_FAIL); } @@ -206,20 +206,17 @@ static int qr_task_step(struct qr_task *task, knot_pkt_t *packet) if (sock_type == SOCK_STREAM) { uv_connect_t *connect = &task->ioreq.connect; if (uv_tcp_connect(connect, (uv_tcp_t *)task->next_handle, addr, qr_task_on_connect) != 0) { - uv_close(task->next_handle, (uv_close_cb) free); return qr_task_step(task, NULL); } connect->data = task; } else { if (qr_task_send(task, task->next_handle, addr, next_query) != 0) { - uv_close(task->next_handle, (uv_close_cb) free); return qr_task_step(task, NULL); } } - /* Start next timeout */ + /* Start next step with timeout */ uv_timer_start(&task->timeout, qr_task_timeout, KR_CONN_RTT_MAX, 0); - return kr_ok(); } @@ -231,15 +228,13 @@ int worker_exec(struct worker_ctx *worker, uv_handle_t *handle, knot_pkt_t *quer /* Parse query */ int ret = parse_query(query); - if (ret != 0) { - return ret; - } /* Start new task on master sockets, or resume existing */ struct qr_task *task = handle->data; bool is_master_socket = (!task); if (is_master_socket) { - if (knot_wire_get_qr(query->wire)) { + /* Ignore badly formed queries or responses. */ + if (ret != 0 || knot_wire_get_qr(query->wire)) { return kr_error(EINVAL); /* Ignore. */ } task = qr_task_create(worker, handle, addr); diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c index 2d49d98c59e2f31b5fbc097cb9c9fc8490982d88..b3f722aa1775fc8fc67eee846370bc1d988c586c 100644 --- a/lib/layer/iterate.c +++ b/lib/layer/iterate.c @@ -345,6 +345,24 @@ static int begin(knot_layer_t *ctx, void *module_param) return reset(ctx); } +static int prepare_additionals(knot_pkt_t *pkt) +{ + knot_rrset_t opt_rr; + int ret = knot_edns_init(&opt_rr, KR_EDNS_PAYLOAD, 0, KR_EDNS_VERSION, &pkt->mm); + if (ret != KNOT_EOK) { + return ret; + } + + knot_pkt_begin(pkt, KNOT_ADDITIONAL); + ret = knot_pkt_put(pkt, KNOT_COMPR_HINT_NONE, &opt_rr, KNOT_PF_FREE); + if (ret != KNOT_EOK) { + knot_rrset_clear(&opt_rr, &pkt->mm); + return ret; + } + + return kr_ok(); +} + static int prepare_query(knot_layer_t *ctx, knot_pkt_t *pkt) { assert(pkt && ctx); @@ -370,23 +388,28 @@ static int prepare_query(knot_layer_t *ctx, knot_pkt_t *pkt) knot_wire_set_id(pkt->wire, query->id); /* Declare EDNS0 support. */ - knot_rrset_t opt_rr; - ret = knot_edns_init(&opt_rr, KR_EDNS_PAYLOAD, 0, KR_EDNS_VERSION, &pkt->mm); - if (ret != KNOT_EOK) { - return KNOT_STATE_FAIL; - } - - knot_pkt_begin(pkt, KNOT_ADDITIONAL); - ret = knot_pkt_put(pkt, KNOT_COMPR_HINT_NONE, &opt_rr, KNOT_PF_FREE); - if (ret != KNOT_EOK) { - knot_rrset_clear(&opt_rr, &pkt->mm); - return KNOT_STATE_FAIL; + if (!(query->flags & QUERY_SAFEMODE)) { + ret = prepare_additionals(pkt); + if (ret != 0) { + return KNOT_STATE_FAIL; + } } /* Query built, expect answer. */ return KNOT_STATE_CONSUME; } +static int resolve_badmsg(knot_pkt_t *pkt, struct kr_request *req, struct kr_query *query) +{ + /* Work around broken auths/load balancers */ + if (query->flags & QUERY_SAFEMODE) { + return resolve_error(pkt, req); + } else { + query->flags |= QUERY_SAFEMODE; + return KNOT_STATE_DONE; + } +} + /** Resolve input query or continue resolution with followups. * * This roughly corresponds to RFC1034, 5.3.3 4a-d. @@ -403,7 +426,7 @@ static int resolve(knot_layer_t *ctx, knot_pkt_t *pkt) /* Check for packet processing errors first. */ if (pkt->parsed < pkt->size) { DEBUG_MSG("<= malformed response\n"); - return resolve_error(pkt, req); + return resolve_badmsg(pkt, req, query); } else if (!is_paired_to_query(pkt, query)) { DEBUG_MSG("<= ignoring mismatching response\n"); return KNOT_STATE_CONSUME; @@ -428,6 +451,10 @@ static int resolve(knot_layer_t *ctx, knot_pkt_t *pkt) case KNOT_RCODE_NOERROR: case KNOT_RCODE_NXDOMAIN: break; /* OK */ + case KNOT_RCODE_FORMERR: + case KNOT_RCODE_NOTIMPL: + DEBUG_MSG("<= rcode: %s\n", rcode ? rcode->name : "??"); + return resolve_badmsg(pkt, req, query); default: DEBUG_MSG("<= rcode: %s\n", rcode ? rcode->name : "??"); return resolve_error(pkt, req); diff --git a/lib/resolve.c b/lib/resolve.c index ce03d43ac6aa5368ff79ae84592fd55fdd352945..4ac474df109ed4e37ff32f78fab356d986abaa42 100644 --- a/lib/resolve.c +++ b/lib/resolve.c @@ -100,6 +100,7 @@ static int sendrecv(struct sockaddr *addr, int proto, const knot_pkt_t *query, k { struct timeval timeout = { KR_CONN_RTT_MAX / 1000, 0 }; auto_close int fd = connected(addr, proto, &timeout); + resp->size = 0; if (fd < 0) { return fd; } @@ -127,11 +128,7 @@ static int sendrecv(struct sockaddr *addr, int proto, const knot_pkt_t *query, k /* Parse and return */ resp->size = ret; - if (knot_pkt_parse(resp, 0) != 0) { - return kr_error(EBADMSG); - } - - return kr_ok(); + return knot_pkt_parse(resp, 0); } int kr_resolve(struct kr_context* ctx, knot_pkt_t *answer, @@ -178,7 +175,6 @@ int kr_resolve(struct kr_context* ctx, knot_pkt_t *answer, int ret = sendrecv(addr, proto, query, resp); if (ret != 0) { DEBUG_MSG("sendrecv: %s\n", kr_strerror(ret)); - resp->size = 0; } state = kr_resolve_consume(&request, resp); knot_pkt_clear(resp); diff --git a/lib/rplan.h b/lib/rplan.h index ef5786c31c8a7b387f4458b909f704dfc70d06a2..c21d75d298363db71ee9f1b215f5100b736c944b 100644 --- a/lib/rplan.h +++ b/lib/rplan.h @@ -31,7 +31,8 @@ enum kr_query_flag { QUERY_NO_MINIMIZE = 1 << 0, /**< Don't minimize QNAME. */ QUERY_TCP = 1 << 1, /**< Use TCP for this query. */ QUERY_RESOLVED = 1 << 2, /**< Query is resolved. */ - QUERY_AWAIT_ADDR = 1 << 3 /**< Query is waiting for NS address. */ + QUERY_AWAIT_ADDR = 1 << 3, /**< Query is waiting for NS address. */ + QUERY_SAFEMODE = 1 << 4 /**< Don't use fancy stuff (EDNS...) */ }; /** diff --git a/tests/testdata/iter_formerr.rpl b/tests/testdata/iter_formerr.rpl new file mode 100644 index 0000000000000000000000000000000000000000..c541f83b984babbba2d06caf5de6d6464987dab5 --- /dev/null +++ b/tests/testdata/iter_formerr.rpl @@ -0,0 +1,86 @@ +; config options +server: + harden-referral-path: no + target-fetch-policy: "0 0 0 0 0" + +stub-zone: + name: "." + stub-addr: 193.0.14.129 # K.ROOT-SERVERS.NET. +CONFIG_END + +SCENARIO_BEGIN Disable EDNS0 and fancy stuff when the server replies with FORMERR. + +STEP 10 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +cz. IN A +ENTRY_END + +; root prime +STEP 30 REPLY +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +. IN NS +SECTION ANSWER +. IN NS K.ROOT-SERVERS.NET. +SECTION ADDITIONAL +K.ROOT-SERVERS.NET. IN A 193.0.14.129 +ENTRY_END + +; query sent to root server +STEP 50 REPLY +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +cz. IN A +SECTION AUTHORITY +cz. IN NS ns1.cz. +SECTION ADDITIONAL +ns1.cz. IN A 168.192.2.2 +ENTRY_END + +; this is the formerr answer +STEP 60 REPLY +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA FORMERR +SECTION QUESTION +cz. IN A +SECTION ANSWER +ENTRY_END + +; this is the correct answer +STEP 60 REPLY +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +cz. IN A +SECTION ANSWER +cz. IN A 10.20.30.40 +SECTION AUTHORITY +cz. IN NS ns1.cz. +SECTION ADDITIONAL +ns1.cz. IN A 168.192.2.2 +ENTRY_END + +; is the final answer correct? +STEP 100 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA +SECTION QUESTION +cz. IN A +SECTION ANSWER +cz. IN A 10.20.30.40 +ENTRY_END + +SCENARIO_END