6:27 a.m.
Hi,
Here is v2, now with various sized explicitely stated.
Cheers,
Patrik
Patrik Flykt (2):
dnsproxy: Send a short response on error
dnsproxy: Be more strict with incoming DNS requests
src/dnsproxy.c | 78 ++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 49 insertions(+), 29 deletions(-)
--
2.11.0
6:27 a.m.
New subject: [PATCH v2 1/2] dnsproxy: Send a short response on error
On error there is no need to send the whole packet back to the
client, only the basic headers informing of the error. Also check
the length of the buffer, including protocol offsets.
---
src/dnsproxy.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 40b4f159..ef91a7cb 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -470,7 +470,7 @@ static void send_cached_response(int sk, unsigned char *buf, int len,
err, len, dns_len);
}
-static void send_response(int sk, unsigned char *buf, int len,
+static void send_response(int sk, unsigned char *buf, size_t len,
const struct sockaddr *to, socklen_t tolen,
int protocol)
{
@@ -482,21 +482,26 @@ static void send_response(int sk, unsigned char *buf, int len,
if (offset < 0)
return;
- if (len < 12)
+ if (len < sizeof(*hdr) + offset)
return;
hdr = (void *) (buf + offset);
+ if (offset) {
+ buf[0] = 0;
+ buf[1] = sizeof(*hdr);
+ }
debug("id 0x%04x qr %d opcode %d", hdr->id, hdr->qr, hdr->opcode);
hdr->qr = 1;
hdr->rcode = ns_r_servfail;
+ hdr->qdcount = 0;
hdr->ancount = 0;
hdr->nscount = 0;
hdr->arcount = 0;
- err = sendto(sk, buf, len, MSG_NOSIGNAL, to, tolen);
+ err = sendto(sk, buf, sizeof(*hdr) + offset, MSG_NOSIGNAL, to, tolen);
if (err < 0) {
connman_error("Failed to send DNS response to %d: %s",
sk, strerror(errno));
--
2.11.0
6:27 a.m.
New subject: [PATCH v2 2/2] dnsproxy: Be more strict with incoming DNS requests
Be more strict with incoming DNS requests. Verify that there is
only one entry in the question section and none in the answer or
name server sections.
Ensure that the question section has a proper IN class and
compute the remaing length and the pointer position with respect
to the end of the question section. If there is an EDNS0 extension,
log its length.
---
src/dnsproxy.c | 67 +++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 41 insertions(+), 26 deletions(-)
diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index ef91a7cb..8b2827a5 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -79,6 +79,11 @@ struct domain_hdr {
#error "Unknown byte order"
#endif
+struct qtype_qclass {
+ uint16_t qtype;
+ uint16_t qclass;
+} __attribute__ ((packed));
+
struct partial_reply {
uint16_t len;
uint16_t received;
@@ -2918,26 +2923,33 @@ static struct connman_notifier dnsproxy_notifier = {
static unsigned char opt_edns0_type[2] = { 0x00, 0x29 };
-static int parse_request(unsigned char *buf, int len,
+static int parse_request(unsigned char *buf, size_t len,
char *name, unsigned int size)
{
struct domain_hdr *hdr = (void *) buf;
uint16_t qdcount = ntohs(hdr->qdcount);
+ uint16_t ancount = ntohs(hdr->ancount);
+ uint16_t nscount = ntohs(hdr->nscount);
uint16_t arcount = ntohs(hdr->arcount);
unsigned char *ptr;
- char *last_label = NULL;
unsigned int remain, used = 0;
- if (len < 12)
+ if (len < sizeof(*hdr) + sizeof(struct qtype_qclass) ||
+ hdr->qr || qdcount != 1 || ancount || nscount) {
+ DBG("Dropped DNS request qr %d with len %zd qdcount %d "
+ "ancount %d nscount %d", hdr->qr, len, qdcount, ancount,
+ nscount);
+
+ return -EINVAL;
+ }
+
+ if (!name || !size)
return -EINVAL;
debug("id 0x%04x qr %d opcode %d qdcount %d arcount %d",
hdr->id, hdr->qr, hdr->opcode,
qdcount, arcount);
- if (hdr->qr != 0 || qdcount != 1)
- return -EINVAL;
-
name[0] = '\0';
ptr = buf + sizeof(struct domain_hdr);
@@ -2947,7 +2959,23 @@ static int parse_request(unsigned char *buf, int len,
uint8_t label_len = *ptr;
if (label_len == 0x00) {
- last_label = (char *) (ptr + 1);
+ uint8_t class;
+ struct qtype_qclass *q =
+ (struct qtype_qclass *)(ptr + 1);
+
+ if (remain < sizeof(*q)) {
+ DBG("Dropped malformed DNS query");
+ return -EINVAL;
+ }
+
+ class = ntohs(q->qclass);
+ if (class != 1 && class != 255) {
+ DBG("Dropped non-IN DNS class %d", class);
+ return -EINVAL;
+ }
+
+ ptr += sizeof(*q) + 1;
+ remain -= (sizeof(*q) + 1);
break;
}
@@ -2963,26 +2991,13 @@ static int parse_request(unsigned char *buf, int len,
remain -= label_len + 1;
}
- if (last_label && arcount && remain >= 9 && last_label[4] ==
0 &&
- !memcmp(last_label + 5, opt_edns0_type, 2)) {
- uint16_t edns0_bufsize;
+ if (arcount && remain >= sizeof(struct domain_rr) + 1 && !ptr[0]
&&
+ ptr[1] == opt_edns0_type[0] && ptr[2] == opt_edns0_type[1]) {
+ struct domain_rr *edns0 = (struct domain_rr *)(ptr + 1);
- edns0_bufsize = last_label[7] << 8 | last_label[8];
-
- debug("EDNS0 buffer size %u", edns0_bufsize);
-
- /* This is an evil hack until full TCP support has been
- * implemented.
- *
- * Somtimes the EDNS0 request gets send with a too-small
- * buffer size. Since glibc doesn't seem to crash when it
- * gets a response biffer then it requested, just bump
- * the buffer size up to 4KiB.
- */
- if (edns0_bufsize < 0x1000) {
- last_label[7] = 0x10;
- last_label[8] = 0x00;
- }
+ DBG("EDNS0 buffer size %u", ntohs(edns0->class));
+ } else if (!arcount && remain) {
+ DBG("DNS request with %d garbage bytes", remain);
}
debug("query %s", name);
--
2.11.0
1252
days inactive
1252
days old
3 comments
2 participants
participants (2)
-
Daniel Wagner -
Patrik Flykt