Merge branch 'fix-edns-opt-write' into 'master'

EDNS OPT writing

Fixes a problem, where OPT RR was not included into the packet, when a previous RRs didn't fit.

See merge request !253

src/knot/nameserver/internet.c

@@ -19,7 +19,6 @@
 #include "libknot/common.h"
 #include "libknot/rrtype/rdname.h"
 #include "libknot/rrtype/soa.h"
-#include "libknot/rrtype/opt.h"
 #include "libknot/dnssec/rrset-sign.h"
 #include "knot/nameserver/internet.h"
 #include "knot/nameserver/nsec_proofs.h"
@@ -167,50 +166,6 @@ static int put_rrsig(const knot_dname_t *sig_owner, uint16_t type,
 	return KNOT_EOK;
 }
 
-/*! \brief Put OPT RR to packet. */
-static int put_opt_rr(knot_pkt_t *pkt, struct query_data *qdata)
-{
-	/* Copy OPT RR from server. */
-	dbg_ns("%s: adding OPT RR to the response\n", __func__);
-	const knot_pkt_t *query = qdata->query;
-	knot_rrset_t opt_rr;
-	int ret = knot_edns_init(&opt_rr, conf()->max_udp_payload,
-	                         qdata->rcode_ext, KNOT_EDNS_VERSION, &pkt->mm);
-	if (ret != KNOT_EOK) {
-		return ret;
-	}
-
-	/* Append NSID if requested and available. */
-	if (knot_edns_has_nsid(query->opt_rr) && conf()->nsid_len > 0) {
-		ret = knot_edns_add_option(&opt_rr,
-		                           KNOT_EDNS_OPTION_NSID, conf()->nsid_len,
-		                           (const uint8_t*)conf()->nsid, &pkt->mm);
-		if (ret != KNOT_EOK) {
-			knot_rrset_clear(&opt_rr, &pkt->mm);
-			return ret;
-		}
-	}
-
-	/* Set DO bit if set (DNSSEC requested). */
-	if (knot_pkt_has_dnssec(query)) {
-		dbg_ns("%s: setting DO=1 in OPT RR\n", __func__);
-		knot_edns_set_do(&opt_rr);
-	}
-
-	/* Reclaim reserved size. */
-	ret = knot_pkt_reclaim(pkt, knot_edns_wire_size(&opt_rr));
-	if (ret == KNOT_EOK) {
-		/* Write to packet. */
-		ret = knot_pkt_put(pkt, COMPR_HINT_NONE, &opt_rr, KNOT_PF_FREE);
-	}
-
-	if (ret != KNOT_EOK) {
-		knot_rrset_clear(&opt_rr, &pkt->mm);
-	}
-
-	return ret;
-}
-
 /*! \brief This is a wildcard-covered or any other terminal node for QNAME.
  *         e.g. positive answer.
  */
@@ -700,14 +655,7 @@ static int solve_authority_dnssec(int state, knot_pkt_t *pkt, struct query_data
 static int solve_additional(int state, knot_pkt_t *pkt,
                             struct query_data *qdata, void *ctx)
 {
-	/* Put OPT RR to the additional section. */
 	int ret = KNOT_EOK;
-	if (knot_pkt_has_edns(qdata->query)) {
-		ret = put_opt_rr(pkt, qdata);
-		if (ret != KNOT_EOK) {
-			return ERROR;
-		}
-	}
 
 	/* Scan all RRs in ANSWER/AUTHORITY. */
 	for (uint16_t i = 0; i < pkt->rrset_count; ++i) {

src/knot/nameserver/process_query.c

@@ -60,6 +60,7 @@ static int process_query_reset(knot_process_t *ctx)
 	knot_pkt_free(&qdata->query);
 	ptrlist_free(&qdata->wildcards, qdata->mm);
 	nsec_clear_rrsigs(qdata);
+	knot_rrset_clear(&qdata->opt_rr, qdata->mm);
 	if (qdata->ext_cleanup != NULL) {
 		qdata->ext_cleanup(qdata);
 	}
@@ -260,6 +261,70 @@ static const zone_t *answer_zone_find(const knot_pkt_t *query, knot_zonedb_t *zo
 	return zone;
 }
 
+static int answer_edns_reserve(knot_pkt_t *resp, struct query_data *qdata)
+{
+	if (knot_rrset_empty(&qdata->opt_rr)) {
+		return KNOT_EOK;
+	}
+
+	/* Reserve size in the response. */
+	return knot_pkt_reserve(resp, knot_edns_wire_size(&qdata->opt_rr));
+}
+
+static int answer_edns_init(const knot_pkt_t *query, knot_pkt_t *resp,
+                            struct query_data *qdata)
+{
+	if (!knot_pkt_has_edns(query)) {
+		return KNOT_EOK;
+	}
+
+	/* Initialize OPT record. */
+	int ret = knot_edns_init(&qdata->opt_rr, conf()->max_udp_payload, 0,
+	                     KNOT_EDNS_VERSION, qdata->mm);
+	if (ret != KNOT_EOK) {
+		return ret;
+	}
+
+	/* Check supported version. */
+	if (knot_edns_get_version(query->opt_rr) != KNOT_EDNS_VERSION) {
+		qdata->rcode_ext = KNOT_EDNS_RCODE_BADVERS;
+	}
+
+	/* Set DO bit if set (DNSSEC requested). */
+	if (knot_pkt_has_dnssec(query)) {
+		knot_edns_set_do(&qdata->opt_rr);
+	}
+
+	/* Append NSID if requested and available. */
+	if (knot_edns_has_nsid(query->opt_rr) && conf()->nsid_len > 0) {
+		ret = knot_edns_add_option(&qdata->opt_rr,
+		                           KNOT_EDNS_OPTION_NSID, conf()->nsid_len,
+		                           (const uint8_t*)conf()->nsid, qdata->mm);
+		if (ret != KNOT_EOK) {
+			return ret;
+		}
+	}
+
+	return answer_edns_reserve(resp, qdata);
+}
+
+static int answer_edns_put(knot_pkt_t *resp, struct query_data *qdata)
+{
+	if (knot_rrset_empty(&qdata->opt_rr)) {
+		return KNOT_EOK;
+	}
+
+	/* Reclaim reserved size. */
+	int ret = knot_pkt_reclaim(resp, knot_edns_wire_size(&qdata->opt_rr));
+	if (ret != KNOT_EOK) {
+		return ret;
+	}
+
+	/* Write to packet. */
+	assert(resp->current == KNOT_ADDITIONAL);
+	return knot_pkt_put(resp, COMPR_HINT_NONE, &qdata->opt_rr, 0);
+}
+
 /*! \brief Initialize response, sizes and find zone from which we're going to answer. */
 static int prepare_answer(const knot_pkt_t *query, knot_pkt_t *resp, knot_process_t *ctx)
 {
@@ -294,49 +359,26 @@ static int prepare_answer(const knot_pkt_t *query, knot_pkt_t *resp, knot_proces
 	/* Find zone for QNAME. */
 	qdata->zone = answer_zone_find(query, server->zone_db);
 
-	/* Update maximal answer size. */
-	if (qdata->param->proc_flags & NS_QUERY_LIMIT_SIZE) {
-		resp->max_size = KNOT_WIRE_MIN_PKTSIZE;
-	}
-
-	/* Check if EDNS is supported. */
-	if (!knot_pkt_has_edns(query)) {
-		return KNOT_EOK;
-	}
-
-	/* Check EDNS version and return BADVERS if not supported. */
-	if (knot_edns_get_version(query->opt_rr) != KNOT_EDNS_VERSION) {
-		dbg_ns("%s: unsupported EDNS version required.\n", __func__);
-		qdata->rcode_ext = KNOT_EDNS_RCODE_BADVERS;
-	}
-
-	/* Reserve space for OPT RR in the packet. Using size of the server's
-	 * OPT RR, because that's the maximum size (RDATA may or may not be
-	 * used).
-	 */
-	uint16_t reserve_size = KNOT_EDNS_MIN_SIZE;
-	if (knot_edns_has_nsid(query->opt_rr) && conf()->nsid_len > 0) {
-		reserve_size += KNOT_EDNS_OPTION_HDRLEN + conf()->nsid_len;
-	}
-	ret = knot_pkt_reserve(resp, reserve_size);
+	/* Setup EDNS. */
+	ret = answer_edns_init(query, resp, qdata);
 	if (ret != KNOT_EOK) {
-		dbg_ns("%s: can't reserve OPT RR in response (%d)\n", __func__, ret);
 		return ret;
 	}
 
-	/* Get minimal supported size from EDNS(0). */
-	uint16_t client_maxlen = knot_edns_get_payload(query->opt_rr);
-	uint16_t server_maxlen = conf()->max_udp_payload;
-	uint16_t min_edns = MIN(client_maxlen, server_maxlen);
-
-	/* Update packet size limit. */
-	if (qdata->param->proc_flags & NS_QUERY_LIMIT_SIZE) {
-		resp->max_size =  MAX(resp->max_size, min_edns);
-		dbg_ns("%s: packet size limit <= %zuB\n", __func__, resp->max_size);
+	/* Update maximal answer size. */
+	bool has_limit = qdata->param->proc_flags & NS_QUERY_LIMIT_SIZE;
+	if (has_limit) {
+		resp->max_size = KNOT_WIRE_MIN_PKTSIZE;
+		if (knot_pkt_has_edns(query)) {
+			uint16_t client = knot_edns_get_payload(query->opt_rr);
+			uint16_t server = conf()->max_udp_payload;
+			uint16_t transfer = MIN(client, server);
+			resp->max_size = MAX(resp->max_size, transfer);
+		}
+	} else {
+		resp->max_size = KNOT_WIRE_MAX_PKTSIZE;
 	}
 
-	/* In the response, always advertise server's maximum UDP payload. */
-
 	return ret;
 }
 
@@ -361,11 +403,21 @@ static int process_query_err(knot_pkt_t *pkt, knot_process_t *ctx)
 	/* Set RCODE. */
 	knot_wire_set_rcode(pkt->wire, qdata->rcode);
 
-	/* Transaction security (if applicable). */
-	if (process_query_sign_response(pkt, qdata) != KNOT_EOK) {
-		return NS_PROC_FAIL;
+
+	/* Add OPT and TSIG (best effort, send reply anyway if fails). */
+	if (pkt->current != KNOT_ADDITIONAL) {
+		knot_pkt_begin(pkt, KNOT_ADDITIONAL);
+	}
+
+	/* Put OPT RR to the additional section. */
+	int ret = answer_edns_reserve(pkt, qdata);
+	if (ret == KNOT_EOK) {
+		(void) answer_edns_put(pkt, qdata);
 	}
 
+	/* Transaction security (if applicable). */
+	(void) process_query_sign_response(pkt, qdata);
+
 	return NS_PROC_DONE;
 }
 
@@ -464,8 +516,19 @@ static int process_query_out(knot_pkt_t *pkt, knot_process_t *ctx)
 	 * Postprocessing.
 	 */
 
-	/* Transaction security (if applicable). */
 	if (next_state == NS_PROC_DONE || next_state == NS_PROC_FULL) {
+		if (pkt->current != KNOT_ADDITIONAL) {
+			knot_pkt_begin(pkt, KNOT_ADDITIONAL);
+		}
+
+		/* Put OPT RR to the additional section. */
+		ret = answer_edns_put(pkt, qdata);
+		if (ret != KNOT_EOK) {
+			next_state = NS_PROC_FAIL;
+			goto finish;
+		}
+
+		/* Transaction security (if applicable). */
 		if (process_query_sign_response(pkt, qdata) != KNOT_EOK) {
 			next_state = NS_PROC_FAIL;
 		}

src/knot/nameserver/process_query.h

@@ -84,6 +84,9 @@ struct query_data {
 	/* Original QNAME case. */
 	uint8_t orig_qname[KNOT_DNAME_MAXLEN];
 
+	/* EDNS */
+	knot_rrset_t opt_rr;
+
 	/* Extensions. */
 	void *ext;
 	void (*ext_cleanup)(struct query_data*); /*!< Extensions cleanup callback. */

src/libknot/packet/pkt.c

@@ -655,6 +655,49 @@ static int knot_pkt_rr_from_wire(const uint8_t *wire, size_t *pos,
 	return KNOT_EOK;
 }
 
+/* \note Private for check_rr_constraints(). */
+#define CHECK_AR_CONSTRAINTS(pkt, rr, var, check_func) \
+	if ((pkt)->current != KNOT_ADDITIONAL) { \
+		dbg_packet("%s: RRTYPE%u not in AR\n", __func__, rr->type); \
+		return KNOT_EMALF; \
+	} else if ((pkt)->var != NULL) { \
+		dbg_packet("%s: found 2nd RRTYPE%u\n", __func__, rr->type); \
+		return KNOT_EMALF; \
+	} else if (!check_func(rr)) { \
+		dbg_packet("%s: bad RRTYPE%u RDATA\n", __func__, rr->type); \
+		return KNOT_EMALF; \
+	} else { \
+		(pkt)->var = rr; \
+	}
+
+/*! \brief Check constraints (position, uniqueness, validity) for special types (TSIG, OPT). */
+static int check_rr_constraints(knot_pkt_t *pkt, knot_rrset_t *rr, size_t rr_size, unsigned flags)
+{
+	/* Check RR constraints. */
+	switch(rr->type) {
+	case KNOT_RRTYPE_TSIG:
+		CHECK_AR_CONSTRAINTS(pkt, rr, tsig_rr, tsig_rdata_is_ok);
+
+		/* Strip TSIG RR from wireformat and decrease ARCOUNT. */
+		if (!(flags & KNOT_PF_KEEPWIRE)) {
+			pkt->parsed -= rr_size;
+			pkt->size -= rr_size;
+			knot_wire_set_id(pkt->wire, tsig_rdata_orig_id(rr));
+			knot_wire_set_arcount(pkt->wire, knot_wire_get_arcount(pkt->wire) - 1);
+		}
+		break;
+	case KNOT_RRTYPE_OPT:
+		CHECK_AR_CONSTRAINTS(pkt, rr, opt_rr, knot_edns_check_record);
+		break;
+	default:
+		break;
+	}
+
+	return KNOT_EOK;
+}
+
+#undef CHECK_AR_RECORD
+
 int knot_pkt_parse_rr(knot_pkt_t *pkt, unsigned flags)
 {
 	dbg_packet("%s(%p, %u)\n", __func__, pkt, flags);
@@ -688,53 +731,10 @@ int knot_pkt_parse_rr(knot_pkt_t *pkt, unsigned flags)
 
 	/* Update packet RRSet count. */
 	++pkt->rrset_count;
-
-	/* Update section RRSet count. */
 	++pkt->sections[pkt->current].count;
 
-	/* Check RR constraints. */
-	switch(rr->type) {
-	case KNOT_RRTYPE_TSIG:
-		// if there is some TSIG already, treat as malformed
-		if (pkt->tsig_rr != NULL) {
-			dbg_packet("%s: found 2nd TSIG\n", __func__);
-			return KNOT_EMALF;
-		} else if (!tsig_rdata_is_ok(rr)) {
-			dbg_packet("%s: bad TSIG RDATA\n", __func__);
-			return KNOT_EMALF;
-		}
-
-		/* Strip TSIG RR from wireformat and decrease ARCOUNT. */
-		if (!(flags & KNOT_PF_KEEPWIRE)) {
-			pkt->parsed -= rr_size;
-			pkt->size -= rr_size;
-			knot_wire_set_id(pkt->wire, tsig_rdata_orig_id(rr));
-			knot_wire_set_arcount(pkt->wire, knot_wire_get_arcount(pkt->wire) - 1);
-		}
-
-		/* Remember TSIG RR. */
-		pkt->tsig_rr = rr;
-		break;
-	case KNOT_RRTYPE_OPT:
-		/* If there is some OPT already, treat as malformed. */
-		if (pkt->opt_rr != NULL) {
-			dbg_packet("%s: found 2nd OPT\n", __func__);
-			return KNOT_EMALF;
-		}
-
-		/* Semantic checks for the OPT. */
-		if (!knot_edns_check_record(rr)) {
-			dbg_packet("%s: OPT RR check failed\n", __func__);
-			return KNOT_EMALF;
-		}
-
-		pkt->opt_rr = rr;
-		break;
-	default:
-		break;
-	}
-
-	return ret;
+	/* Check special RRs (OPT and TSIG). */
+	return check_rr_constraints(pkt, rr, rr_size, flags);
 }
 
 int knot_pkt_parse_section(knot_pkt_t *pkt, unsigned flags)