Merge branch 'fix-edns0-badimpl'

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);

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);

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);

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);

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...) */
 };
 
 /**

tests/testdata/iter_formerr.rpl

+; 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