From e9a40933f7bfbade1a38498666ca96192e7f8bd1 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Sat, 22 Feb 2020 19:55:05 +0100 Subject: [PATCH 1/5] sock_dns: fix off-by-one error in _parse_dns_reply() --- sys/net/application_layer/dns/dns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/net/application_layer/dns/dns.c b/sys/net/application_layer/dns/dns.c index cf82e10a83..990aaf50a6 100644 --- a/sys/net/application_layer/dns/dns.c +++ b/sys/net/application_layer/dns/dns.c @@ -157,7 +157,7 @@ static int _parse_dns_reply(uint8_t *buf, size_t len, void* addr_out, int family return -EBADMSG; } bufpos += RR_RDLENGTH_LENGTH; - if ((bufpos + addrlen) >= buflim) { + if ((bufpos + addrlen) > buflim) { return -EBADMSG; } From a65daf7a313d37a3ae34e1658be25771f3d13308 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Sat, 22 Feb 2020 20:04:46 +0100 Subject: [PATCH 2/5] shell/gnrc_icmpv6_echo: set success state if DNS query succeeded We need to set `res` to 0 to signal success, otherwise we end up in the print usage case. --- sys/shell/commands/sc_gnrc_icmpv6_echo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sys/shell/commands/sc_gnrc_icmpv6_echo.c b/sys/shell/commands/sc_gnrc_icmpv6_echo.c index 7241f2635c..0c9e1c349f 100644 --- a/sys/shell/commands/sc_gnrc_icmpv6_echo.c +++ b/sys/shell/commands/sc_gnrc_icmpv6_echo.c @@ -178,6 +178,7 @@ static int _configure(int argc, char **argv, _ping_data_t *data) data->hostname = arg; #ifdef MODULE_SOCK_DNS if (sock_dns_query(data->hostname, &data->host, AF_INET6) == 0) { + res = 0; continue; } #endif From 367f19d390b6216519651cfc5601cd9e455ed763 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Sun, 23 Feb 2020 18:00:40 +0100 Subject: [PATCH 3/5] shell/gnrc_icmpv6_echo: don't do DNS lookup for plain IP When using ping6 with an IP address, don't do a DNS lookup. Hostnames can't contain ':', so use that to tell them apart from plain IP addressees. --- sys/shell/commands/sc_gnrc_icmpv6_echo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sys/shell/commands/sc_gnrc_icmpv6_echo.c b/sys/shell/commands/sc_gnrc_icmpv6_echo.c index 0c9e1c349f..f07516aaa3 100644 --- a/sys/shell/commands/sc_gnrc_icmpv6_echo.c +++ b/sys/shell/commands/sc_gnrc_icmpv6_echo.c @@ -177,7 +177,8 @@ static int _configure(int argc, char **argv, _ping_data_t *data) data->hostname = arg; #ifdef MODULE_SOCK_DNS - if (sock_dns_query(data->hostname, &data->host, AF_INET6) == 0) { + if (strchr(data->hostname, ':') == NULL && + sock_dns_query(data->hostname, &data->host, AF_INET6) == 0) { res = 0; continue; } From 1de14931b8a326d12e35685bad1fd4779848401f Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Sun, 23 Feb 2020 18:59:15 +0100 Subject: [PATCH 4/5] sock_dns: use the same buffer for request & reply Saving RAM is more important than saving a few cycles used by re-creating the request buffer in the error case. Also reduce the size of the buffer to 128 bytes. If we are just requesting the AAAA record it is unlikely for the reply to take up the maximum size of 512 bytes. We were already placing restrictions on the domain name length, those are now actually a bit more relaxed (112 bytes instead of 64) --- sys/include/net/sock/dns.h | 4 +- sys/net/application_layer/dns/dns.c | 64 +++++++++++++++-------------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/sys/include/net/sock/dns.h b/sys/include/net/sock/dns.h index a1b39a4439..8bf2a1da86 100644 --- a/sys/include/net/sock/dns.h +++ b/sys/include/net/sock/dns.h @@ -57,8 +57,8 @@ typedef struct { #define SOCK_DNS_PORT (53) #define SOCK_DNS_RETRIES (2) -#define SOCK_DNS_MAX_NAME_LEN (64U) /* we're in embedded context. */ -#define SOCK_DNS_QUERYBUF_LEN (sizeof(sock_dns_hdr_t) + 4 + SOCK_DNS_MAX_NAME_LEN) +#define SOCK_DNS_BUF_LEN (128) /* we're in embedded context. */ +#define SOCK_DNS_MAX_NAME_LEN (SOCK_DNS_BUF_LEN - sizeof(sock_dns_hdr_t) - 4) /** @} */ /** diff --git a/sys/net/application_layer/dns/dns.c b/sys/net/application_layer/dns/dns.c index 990aaf50a6..f42ce3360f 100644 --- a/sys/net/application_layer/dns/dns.c +++ b/sys/net/application_layer/dns/dns.c @@ -170,8 +170,7 @@ static int _parse_dns_reply(uint8_t *buf, size_t len, void* addr_out, int family int sock_dns_query(const char *domain_name, void *addr_out, int family) { - uint8_t buf[SOCK_DNS_QUERYBUF_LEN]; - uint8_t reply_buf[512]; + static uint8_t dns_buf[SOCK_DNS_BUF_LEN]; if (sock_dns_server.port == 0) { return -ECONNREFUSED; @@ -181,33 +180,6 @@ int sock_dns_query(const char *domain_name, void *addr_out, int family) return -ENOSPC; } - sock_dns_hdr_t *hdr = (sock_dns_hdr_t*) buf; - memset(hdr, 0, sizeof(*hdr)); - hdr->id = 0; /* random? */ - hdr->flags = htons(0x0120); - hdr->qdcount = htons(1 + (family == AF_UNSPEC)); - - uint8_t *bufpos = buf + sizeof(*hdr); - - unsigned _name_ptr; - if ((family == AF_INET6) || (family == AF_UNSPEC)) { - _name_ptr = (bufpos - buf); - bufpos += _enc_domain_name(bufpos, domain_name); - bufpos += _put_short(bufpos, htons(DNS_TYPE_AAAA)); - bufpos += _put_short(bufpos, htons(DNS_CLASS_IN)); - } - - if ((family == AF_INET) || (family == AF_UNSPEC)) { - if (family == AF_UNSPEC) { - bufpos += _put_short(bufpos, htons((0xc000) | (_name_ptr))); - } - else { - bufpos += _enc_domain_name(bufpos, domain_name); - } - bufpos += _put_short(bufpos, htons(DNS_TYPE_A)); - bufpos += _put_short(bufpos, htons(DNS_CLASS_IN)); - } - sock_udp_t sock_dns; ssize_t res = sock_udp_create(&sock_dns, NULL, &sock_dns_server, 0); @@ -215,15 +187,45 @@ int sock_dns_query(const char *domain_name, void *addr_out, int family) goto out; } + uint16_t id = 0; /* random? */ for (int i = 0; i < SOCK_DNS_RETRIES; i++) { + uint8_t *buf = dns_buf; + + sock_dns_hdr_t *hdr = (sock_dns_hdr_t*) buf; + memset(hdr, 0, sizeof(*hdr)); + hdr->id = id; + hdr->flags = htons(0x0120); + hdr->qdcount = htons(1 + (family == AF_UNSPEC)); + + uint8_t *bufpos = buf + sizeof(*hdr); + + unsigned _name_ptr; + if ((family == AF_INET6) || (family == AF_UNSPEC)) { + _name_ptr = (bufpos - buf); + bufpos += _enc_domain_name(bufpos, domain_name); + bufpos += _put_short(bufpos, htons(DNS_TYPE_AAAA)); + bufpos += _put_short(bufpos, htons(DNS_CLASS_IN)); + } + + if ((family == AF_INET) || (family == AF_UNSPEC)) { + if (family == AF_UNSPEC) { + bufpos += _put_short(bufpos, htons((0xc000) | (_name_ptr))); + } + else { + bufpos += _enc_domain_name(bufpos, domain_name); + } + bufpos += _put_short(bufpos, htons(DNS_TYPE_A)); + bufpos += _put_short(bufpos, htons(DNS_CLASS_IN)); + } + res = sock_udp_send(&sock_dns, buf, (bufpos-buf), NULL); if (res <= 0) { continue; } - res = sock_udp_recv(&sock_dns, reply_buf, sizeof(reply_buf), 1000000LU, NULL); + res = sock_udp_recv(&sock_dns, dns_buf, sizeof(dns_buf), 1000000LU, NULL); if (res > 0) { if (res > (int)DNS_MIN_REPLY_LEN) { - if ((res = _parse_dns_reply(reply_buf, res, addr_out, + if ((res = _parse_dns_reply(dns_buf, res, addr_out, family)) > 0) { goto out; } From fa4447241fea157131b7c97ce742ece555e24c48 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Mon, 24 Feb 2020 13:21:18 +0100 Subject: [PATCH 5/5] sock_dns: make sock_dns_query() return the length of the address The implementation already did that, now also reflect this in the documentation. --- sys/include/net/sock/dns.h | 4 ++-- sys/shell/commands/sc_gnrc_icmpv6_echo.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sys/include/net/sock/dns.h b/sys/include/net/sock/dns.h index 8bf2a1da86..f7d4d82261 100644 --- a/sys/include/net/sock/dns.h +++ b/sys/include/net/sock/dns.h @@ -80,8 +80,8 @@ typedef struct { * @param[out] addr_out buffer to write result into * @param[in] family Either AF_INET, AF_INET6 or AF_UNSPEC * - * @return 0 on success - * @return !=0 otherwise + * @return the size of the resolved address on success + * @return < 0 otherwise */ int sock_dns_query(const char *domain_name, void *addr_out, int family); diff --git a/sys/shell/commands/sc_gnrc_icmpv6_echo.c b/sys/shell/commands/sc_gnrc_icmpv6_echo.c index f07516aaa3..b93491a6d1 100644 --- a/sys/shell/commands/sc_gnrc_icmpv6_echo.c +++ b/sys/shell/commands/sc_gnrc_icmpv6_echo.c @@ -178,7 +178,7 @@ static int _configure(int argc, char **argv, _ping_data_t *data) data->hostname = arg; #ifdef MODULE_SOCK_DNS if (strchr(data->hostname, ':') == NULL && - sock_dns_query(data->hostname, &data->host, AF_INET6) == 0) { + sock_dns_query(data->hostname, &data->host, AF_INET6) > 0) { res = 0; continue; }