From 5a3045117e105a406587fe46b2fe59bf5c4e2a96 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Mon, 7 Nov 2022 16:35:57 +0100 Subject: [PATCH 1/3] gnrc_sock_udp: make 'remote match' condition more readable --- sys/net/gnrc/sock/udp/gnrc_sock_udp.c | 42 ++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c index 6066bab78f90..b30a2502581c 100644 --- a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c +++ b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c @@ -30,6 +30,9 @@ #include "gnrc_sock_internal.h" +#define ENABLE_DEBUG 0 +#include "debug.h" + #ifdef MODULE_GNRC_SOCK_CHECK_REUSE static sock_udp_t *_udp_socks = NULL; #endif @@ -197,6 +200,37 @@ ssize_t sock_udp_recv_aux(sock_udp_t *sock, void *data, size_t max_len, return (nobufs) ? -ENOBUFS : ((res < 0) ? res : ret); } +static bool _remote_mismatch(const sock_udp_t *sock, const udp_hdr_t *hdr, const sock_ip_ep_t *remote) +{ + if (sock->remote.family == AF_UNSPEC) { + /* socket accepts any remote */ + return false; + } + + if (sock->remote.port != byteorder_ntohs(hdr->src_port)) { + DEBUG("gnrc_sock_udp: port mismatch (%u != %u)\n", + sock->remote.port, byteorder_ntohs(hdr->src_port)); + return true; + } + + /* We only have IPv6 for now, so just comparing the whole end point should suffice */ + if (memcmp(&sock->remote.addr, &ipv6_addr_unspecified, sizeof(ipv6_addr_t)) == 0) { + /* socket accepts any remote address */ + return false; + } + + if (memcmp(&sock->remote.addr, &remote->addr, sizeof(ipv6_addr_t)) != 0) { + char addr_str[IPV6_ADDR_MAX_STR_LEN]; + DEBUG("gnrc_sock_udp: socket bound to address %s", + ipv6_addr_to_str(addr_str, (ipv6_addr_t *)&sock->remote.addr, sizeof(addr_str))); + DEBUG(", source (%s) does not match\n", + ipv6_addr_to_str(addr_str, (ipv6_addr_t *)&remote->addr, sizeof(addr_str))); + return true; + } + + return false; +} + ssize_t sock_udp_recv_buf_aux(sock_udp_t *sock, void **data, void **buf_ctx, uint32_t timeout, sock_udp_ep_t *remote, sock_udp_aux_rx_t *aux) @@ -246,13 +280,7 @@ ssize_t sock_udp_recv_buf_aux(sock_udp_t *sock, void **data, void **buf_ctx, memcpy(remote, &tmp, sizeof(tmp)); remote->port = byteorder_ntohs(hdr->src_port); } - if ((sock->remote.family != AF_UNSPEC) && /* check remote end-point if set */ - ((sock->remote.port != byteorder_ntohs(hdr->src_port)) || - /* We only have IPv6 for now, so just comparing the whole end point - * should suffice */ - ((memcmp(&sock->remote.addr, &ipv6_addr_unspecified, - sizeof(ipv6_addr_t)) != 0) && - (memcmp(&sock->remote.addr, &tmp.addr, sizeof(ipv6_addr_t)) != 0)))) { + if (_remote_mismatch(sock, hdr, &tmp)) { gnrc_pktbuf_release(pkt); return -EPROTO; } From 2b92e9ec598e86cead5beb9c950849c29ec28c34 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Mon, 7 Nov 2022 16:57:33 +0100 Subject: [PATCH 2/3] gnrc_sock_udp: accept response from any address if remote is multicast --- sys/include/net/sock.h | 3 ++- sys/net/gnrc/sock/udp/gnrc_sock_udp.c | 28 ++++++++++++++++----------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/sys/include/net/sock.h b/sys/include/net/sock.h index 3487f4e1dae7..4acf9f36a267 100644 --- a/sys/include/net/sock.h +++ b/sys/include/net/sock.h @@ -143,7 +143,8 @@ extern "C" { * @anchor net_sock_flags * @{ */ -#define SOCK_FLAGS_REUSE_EP (0x0001) /**< allow to reuse end point on bind */ +#define SOCK_FLAGS_REUSE_EP (0x0001) /**< allow to reuse end point on bind */ +#define SOCK_FLAGS_CONNECT_REMOTE (0x0002) /**< restrict responses to remote address */ /** @} */ /** diff --git a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c index b30a2502581c..d3b86c2dedc5 100644 --- a/sys/net/gnrc/sock/udp/gnrc_sock_udp.c +++ b/sys/net/gnrc/sock/udp/gnrc_sock_udp.c @@ -136,6 +136,12 @@ int sock_udp_create(sock_udp_t *sock, const sock_udp_ep_t *local, } gnrc_ep_set((sock_ip_ep_t *)&sock->remote, (sock_ip_ep_t *)remote, sizeof(sock_udp_ep_t)); + + /* only accept responses from the set remote */ + if (!ipv6_addr_is_multicast((ipv6_addr_t *)&remote->addr) && + !ipv6_addr_is_unspecified((ipv6_addr_t *)&remote->addr)) { + flags |= SOCK_FLAGS_CONNECT_REMOTE; + } } if (local != NULL) { /* listen only with local given */ @@ -200,22 +206,22 @@ ssize_t sock_udp_recv_aux(sock_udp_t *sock, void *data, size_t max_len, return (nobufs) ? -ENOBUFS : ((res < 0) ? res : ret); } -static bool _remote_mismatch(const sock_udp_t *sock, const udp_hdr_t *hdr, const sock_ip_ep_t *remote) +static bool _remote_accept(const sock_udp_t *sock, const udp_hdr_t *hdr, + const sock_ip_ep_t *remote) { + if ((sock->flags & SOCK_FLAGS_CONNECT_REMOTE) == 0) { + /* socket is not bound to a remote */ + return true; + } + if (sock->remote.family == AF_UNSPEC) { /* socket accepts any remote */ - return false; + return true; } if (sock->remote.port != byteorder_ntohs(hdr->src_port)) { DEBUG("gnrc_sock_udp: port mismatch (%u != %u)\n", sock->remote.port, byteorder_ntohs(hdr->src_port)); - return true; - } - - /* We only have IPv6 for now, so just comparing the whole end point should suffice */ - if (memcmp(&sock->remote.addr, &ipv6_addr_unspecified, sizeof(ipv6_addr_t)) == 0) { - /* socket accepts any remote address */ return false; } @@ -225,10 +231,10 @@ static bool _remote_mismatch(const sock_udp_t *sock, const udp_hdr_t *hdr, const ipv6_addr_to_str(addr_str, (ipv6_addr_t *)&sock->remote.addr, sizeof(addr_str))); DEBUG(", source (%s) does not match\n", ipv6_addr_to_str(addr_str, (ipv6_addr_t *)&remote->addr, sizeof(addr_str))); - return true; + return false; } - return false; + return true; } ssize_t sock_udp_recv_buf_aux(sock_udp_t *sock, void **data, void **buf_ctx, @@ -280,7 +286,7 @@ ssize_t sock_udp_recv_buf_aux(sock_udp_t *sock, void **data, void **buf_ctx, memcpy(remote, &tmp, sizeof(tmp)); remote->port = byteorder_ntohs(hdr->src_port); } - if (_remote_mismatch(sock, hdr, &tmp)) { + if (!_remote_accept(sock, hdr, &tmp)) { gnrc_pktbuf_release(pkt); return -EPROTO; } From 920d6906940b8c0846681208acfa236f752c8de2 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Mon, 7 Nov 2022 17:13:22 +0100 Subject: [PATCH 3/3] test/gnrc_udp: add test multicast sock --- tests/gnrc_sock_udp/main.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/gnrc_sock_udp/main.c b/tests/gnrc_sock_udp/main.c index 491a49ce4412..48a349a7fb1c 100644 --- a/tests/gnrc_sock_udp/main.c +++ b/tests/gnrc_sock_udp/main.c @@ -255,6 +255,25 @@ static void test_sock_udp_recv__EPROTO(void) expect(_check_net()); } +static void test_sock_udp_recv__multicast(void) +{ + static const ipv6_addr_t src_addr = { .u8 = _TEST_ADDR_WRONG }; + static const ipv6_addr_t dst_addr = { .u8 = _TEST_ADDR_LOCAL }; + static const sock_udp_ep_t local = { .family = AF_INET6, + .port = _TEST_PORT_LOCAL }; + static const sock_udp_ep_t remote = { .addr = IPV6_ADDR_ALL_NODES_LINK_LOCAL, + .family = AF_INET6, + .port = _TEST_PORT_REMOTE }; + + expect(0 == sock_udp_create(&_sock, &local, &remote, SOCK_FLAGS_REUSE_EP)); + expect(_inject_packet(&src_addr, &dst_addr, _TEST_PORT_REMOTE, + _TEST_PORT_LOCAL, "ABCD", sizeof("ABCD"), + _TEST_NETIF)); + expect(5 == sock_udp_recv(&_sock, _test_buffer, sizeof(_test_buffer), + SOCK_NO_TIMEOUT, NULL)); + expect(_check_net()); +} + static void test_sock_udp_recv__ETIMEDOUT(void) { static const sock_udp_ep_t local = { .family = AF_INET6, .netif = _TEST_NETIF, @@ -819,6 +838,7 @@ int main(void) CALL(test_sock_udp_recv__EAGAIN()); CALL(test_sock_udp_recv__ENOBUFS()); CALL(test_sock_udp_recv__EPROTO()); + CALL(test_sock_udp_recv__multicast()); CALL(test_sock_udp_recv__ETIMEDOUT()); CALL(test_sock_udp_recv__socketed()); CALL(test_sock_udp_recv__socketed_with_remote());