From: florian <florian@openbsd.org>
Date: Sun, 13 Mar 2022 15:14:01 +0000 (+0000)
Subject: parse_packet() is used by unbound to parse response packets, not
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=dbf56da74a9987a387577094a36f97fa1274e030;p=openbsd

parse_packet() is used by unbound to parse response packets, not
queries. There is no need to do all this work just to get access to
the query id and flags.

OK bket, sthen
---

diff --git a/sbin/unwind/frontend.c b/sbin/unwind/frontend.c
index 8bc79f95769..653e73200bc 100644
--- a/sbin/unwind/frontend.c
+++ b/sbin/unwind/frontend.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: frontend.c,v 1.72 2022/03/03 18:54:07 florian Exp $	*/
+/*	$OpenBSD: frontend.c,v 1.73 2022/03/13 15:14:01 florian Exp $	*/
 
 /*
  * Copyright (c) 2018 Florian Obser <florian@openbsd.org>
@@ -100,12 +100,13 @@ struct pending_query {
 	struct sldns_buffer		*abuf;
 	struct regional			*region;
 	struct query_info		 qinfo;
-	struct msg_parse		*qmsg;
 	struct edns_data		 edns;
 	struct event			 ev;		/* for tcp */
 	struct event			 resp_ev;	/* for tcp */
 	struct event			 tmo_ev;	/* for tcp */
 	uint64_t			 imsg_id;
+	uint16_t			 id;
+	uint16_t			 flags;
 	int				 fd;
 	int				 tcp;
 	int				 dns64_synthesize;
@@ -536,8 +537,7 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
 				break;
 			}
 
-			if (answer_header->bogus && !(pq->qmsg->flags &
-			    BIT_CD)) {
+			if (answer_header->bogus && !(pq->flags & BIT_CD)) {
 				error_answer(pq, LDNS_RCODE_SERVFAIL);
 				send_answer(pq);
 				break;
@@ -716,15 +716,13 @@ udp_receive(int fd, short events, void *arg)
 	pq->qbuf = sldns_buffer_new(len);
 	pq->abuf = sldns_buffer_new(len); /* make sure we can send errors */
 	pq->region = regional_create();
-	pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
 
-	if (!pq->qbuf || !pq->abuf || !pq->region || !pq->qmsg) {
+	if (!pq->qbuf || !pq->abuf || !pq->region) {
 		log_warnx("out of memory");
 		free_pending_query(pq);
 		return;
 	}
 
-	memset(pq->qmsg, 0, sizeof(*pq->qmsg));
 	sldns_buffer_write(pq->qbuf, udpev->query, len);
 	sldns_buffer_flip(pq->qbuf);
 	handle_query(pq);
@@ -734,7 +732,6 @@ void
 handle_query(struct pending_query *pq)
 {
 	struct query_imsg	 query_imsg;
-	struct query_info	 skip;
 	struct bl_node		 find;
 	int			 rcode;
 	char			*str;
@@ -750,16 +747,16 @@ handle_query(struct pending_query *pq)
 		free(str);
 	}
 
-	if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
-		log_warnx("query_info_parse failed");
+	if (sldns_buffer_remaining(pq->qbuf) < LDNS_HEADER_SIZE) {
+		log_warnx("bad query: too short, dropped");
 		goto drop;
 	}
 
-	sldns_buffer_rewind(pq->qbuf);
+	pq->id = sldns_buffer_read_u16_at(pq->qbuf, 0);
+	pq->flags = sldns_buffer_read_u16_at(pq->qbuf, 2);
 
-	if (parse_packet(pq->qbuf, pq->qmsg, pq->region) !=
-	    LDNS_RCODE_NOERROR) {
-		log_warnx("parse_packet failed");
+	if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
+		log_warnx("query_info_parse failed");
 		goto drop;
 	}
 
@@ -774,11 +771,6 @@ handle_query(struct pending_query *pq)
 		goto send_answer;
 	}
 
-	sldns_buffer_rewind(pq->qbuf);
-	if (!query_info_parse(&skip, pq->qbuf)) {
-		error_answer(pq, LDNS_RCODE_SERVFAIL);
-		goto send_answer;
-	}
 	rcode = parse_edns_from_query_pkt(pq->qbuf, &pq->edns, NULL, NULL,
 	    pq->region);
 	if (rcode != LDNS_RCODE_NOERROR) {
@@ -891,7 +883,7 @@ noerror_answer(struct pending_query *pq)
 	}
 
 	sldns_buffer_clear(pq->abuf);
-	if (reply_info_encode(&pq->qinfo, rinfo, pq->qmsg->id, rinfo->flags,
+	if (reply_info_encode(&pq->qinfo, rinfo, htons(pq->id), rinfo->flags,
 	    pq->abuf, 0, pq->region, pq->tcp ? UINT16_MAX : pq->edns.udp_size,
 	    pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
 		goto srvfail;
@@ -986,7 +978,7 @@ synthesize_dns64_answer(struct pending_query *pq)
 
 	sldns_buffer_clear(pq->abuf);
 
-	if (reply_info_encode(&pq->qinfo, synth_rinfo, pq->qmsg->id,
+	if (reply_info_encode(&pq->qinfo, synth_rinfo, htons(pq->id),
 	    synth_rinfo->flags, pq->abuf, 0, pq->region,
 	    pq->tcp ? UINT16_MAX : pq->edns.udp_size,
 	    pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
@@ -1006,7 +998,6 @@ void
 resend_dns64_query(struct pending_query *opq) {
 	struct pending_query	*pq;
 	struct query_imsg	 query_imsg;
-	struct query_info	 skip;
 	int			 rcode;
 	char			 dname[LDNS_MAX_DOMAINLEN + 1];
 
@@ -1028,9 +1019,8 @@ resend_dns64_query(struct pending_query *opq) {
 	pq->qbuf = sldns_buffer_new(sldns_buffer_capacity(opq->qbuf));
 	pq->abuf = sldns_buffer_new(sldns_buffer_capacity(opq->abuf));
 	pq->region = regional_create();
-	pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
 
-	if (!pq->qbuf || !pq->abuf || !pq->region || !pq->qmsg) {
+	if (!pq->qbuf || !pq->abuf || !pq->region) {
 		log_warnx("out of memory");
 		free_pending_query(pq);
 		free_pending_query(opq);
@@ -1041,7 +1031,6 @@ resend_dns64_query(struct pending_query *opq) {
 	sldns_buffer_write(pq->qbuf, sldns_buffer_current(opq->qbuf),
 	    sldns_buffer_remaining(opq->qbuf));
 	sldns_buffer_flip(pq->qbuf);
-	memset(pq->qmsg, 0, sizeof(*pq->qmsg));
 
 	if (pq->tcp) {
 		struct timeval	 timeout = {TCP_TIMEOUT, 0};
@@ -1054,24 +1043,19 @@ resend_dns64_query(struct pending_query *opq) {
 		evtimer_add(&pq->tmo_ev, &timeout);
 	}
 
-	if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
-		log_warnx("query_info_parse failed");
+	if (sldns_buffer_remaining(pq->qbuf) < LDNS_HEADER_SIZE) {
+		log_warnx("bad query: too short, dropped");
 		goto drop;
 	}
 
-	sldns_buffer_rewind(pq->qbuf);
+	pq->id = sldns_buffer_read_u16_at(pq->qbuf, 0);
+	pq->flags = sldns_buffer_read_u16_at(pq->qbuf, 2);
 
-	if (parse_packet(pq->qbuf, pq->qmsg, pq->region) !=
-	    LDNS_RCODE_NOERROR) {
-		log_warnx("parse_packet failed");
+	if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
+		log_warnx("query_info_parse failed");
 		goto drop;
 	}
 
-	sldns_buffer_rewind(pq->qbuf);
-	if (!query_info_parse(&skip, pq->qbuf)) {
-		error_answer(pq, LDNS_RCODE_SERVFAIL);
-		goto send_answer;
-	}
 	rcode = parse_edns_from_query_pkt(pq->qbuf, &pq->edns, NULL, NULL,
 	    pq->region);
 	if (rcode != LDNS_RCODE_NOERROR) {
@@ -1154,8 +1138,8 @@ void
 error_answer(struct pending_query *pq, int rcode)
 {
 	sldns_buffer_clear(pq->abuf);
-	error_encode(pq->abuf, rcode, &pq->qinfo, pq->qmsg->id,
-	    pq->qmsg->flags, pq->edns.edns_present ? &pq->edns : NULL);
+	error_encode(pq->abuf, rcode, &pq->qinfo, htons(pq->id), pq->flags,
+	    pq->edns.edns_present ? &pq->edns : NULL);
 }
 
 int
@@ -1669,15 +1653,12 @@ tcp_accept(int fd, short events, void *arg)
 	pq->tcp = 1;
 	pq->qbuf = sldns_buffer_new(DEFAULT_TCP_SIZE);
 	pq->region = regional_create();
-	pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
 
-	if (!pq->qbuf || !pq->region || !pq->qmsg) {
+	if (!pq->qbuf || !pq->region) {
 		free_pending_query(pq);
 		return;
 	}
 
-	memset(pq->qmsg, 0, sizeof(*pq->qmsg));
-
 	event_set(&pq->ev, s, EV_READ | EV_PERSIST, tcp_request, pq);
 	event_add(&pq->ev, NULL);
 	event_set(&pq->resp_ev, s, EV_WRITE | EV_PERSIST, tcp_response, pq);