drivers: wifi: esp_at: fix UDP socket setup

Right now AT+CIPSTART command is called with both "remote port" and "local
port" being set to the same number. This means that for outgoing UDP
traffic (e.g. when resolving DNS or when reaching out some application
server with CoAP over UDP) always the same outgoing port is used. Such
behavior is wrong, since by default a random outgoing port should be used.

Reusing the same port confuses server implementation that is reached out,
since especially in context of DTLS over UDP, outgoing port defines TLS
context/session to be used. Such servers often ignore TLS packets from new
sessions (e.g. after device reboot) and result in failed DTLS connection
attempts.

Commit dbf3d6e911 ("drivers: esp_at: implement bind() and recvfrom() for
UDP sockets") added support for "server-side listening" for incoming
traffic on UDP sockets, which introduced broken behavior of using the same
remote and local port.

In esp_bind() implementation assign newly intorduced 'src' member, instead
of reusing 'dst'. Don't call AT+CIPSTART yet at this stage, as in case of
connect() Zephyr syscall esp_bind() (via bind_default() helper function) is
called implicitly to assign random generated local port, so remote port is
yet to be assigned. Check in esp_recv() whether socket was already
connected (i.e. esp_connect() was called) and if not, invoke
AT+CIPSTART (via _sock_connect()) to start listening on "server-side" UDP
socket.

This patch fixes broken behavior of always reusing the same local port for
outgoing UDP traffic. Instead, randomly generated (by Zephyr net_context
subsys) local port is used. Additionally bind() and recvfrom()
implementation (to handle server-side UDP sockets) is improved, so that
binding to 0.0.0.0 (on any interface) is possible.

Fixes: dbf3d6e911 ("drivers: esp_at: implement bind() and recvfrom() for
  UDP sockets")
Fixes: 424ea9f5e4 ("drivers: wifi: esp_at: do not connect socket on
  bind(INADDR_ANY)")
Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>

drivers/wifi/esp_at/esp.h

@@ -175,6 +175,7 @@ struct esp_socket {
 	atomic_t flags;
 
 	/* socket info */
+	struct sockaddr src;
 	struct sockaddr dst;
 
 	/* sem */

drivers/wifi/esp_at/esp_offload.c

@@ -27,9 +27,14 @@ static int esp_listen(struct net_context *context, int backlog)
 
 static int _sock_connect(struct esp_data *dev, struct esp_socket *sock)
 {
-	char connect_msg[sizeof("AT+CIPSTART=000,\"TCP\",\"\",65535,7200") +
-			 NET_IPV4_ADDR_LEN];
-	char addr_str[NET_IPV4_ADDR_LEN];
+	/* Calculate the largest possible AT command length based on both TCP and UDP variants. */
+	char connect_msg[MAX(sizeof("AT+CIPSTART=000,\"TCP\",\"\",65535,7200") +
+				NET_IPV4_ADDR_LEN,
+			     sizeof("AT+CIPSTART=000,\"UDP\",\"\",65535,65535,0,\"\"") +
+				2 * NET_IPV4_ADDR_LEN)];
+	char dst_addr_str[NET_IPV4_ADDR_LEN];
+	char src_addr_str[NET_IPV4_ADDR_LEN];
+	struct sockaddr src;
 	struct sockaddr dst;
 	int ret;
 
@@ -38,28 +43,44 @@ static int _sock_connect(struct esp_data *dev, struct esp_socket *sock)
 	}
 
 	k_mutex_lock(&sock->lock, K_FOREVER);
+	src = sock->src;
 	dst = sock->dst;
 	k_mutex_unlock(&sock->lock);
 
-	net_addr_ntop(dst.sa_family,
-		      &net_sin(&dst)->sin_addr,
-		      addr_str, sizeof(addr_str));
+	if (dst.sa_family == AF_INET) {
+		net_addr_ntop(dst.sa_family,
+			      &net_sin(&dst)->sin_addr,
+			      dst_addr_str, sizeof(dst_addr_str));
+	} else {
+		strcpy(dst_addr_str, "0.0.0.0");
+	}
 
 	if (esp_socket_ip_proto(sock) == IPPROTO_TCP) {
 		snprintk(connect_msg, sizeof(connect_msg),
 			 "AT+CIPSTART=%d,\"TCP\",\"%s\",%d,7200",
-			 sock->link_id, addr_str,
+			 sock->link_id, dst_addr_str,
 			 ntohs(net_sin(&dst)->sin_port));
 	} else {
-		snprintk(connect_msg, sizeof(connect_msg),
-			 "AT+CIPSTART=%d,\"UDP\",\"%s\",%d,%d",
-			 sock->link_id, addr_str,
-			 ntohs(net_sin(&dst)->sin_port), ntohs(net_sin(&dst)->sin_port));
+		if (src.sa_family == AF_INET && net_sin(&src)->sin_port != 0) {
+			net_addr_ntop(src.sa_family,
+				      &net_sin(&src)->sin_addr,
+				      src_addr_str, sizeof(src_addr_str));
+			snprintk(connect_msg, sizeof(connect_msg),
+				 "AT+CIPSTART=%d,\"UDP\",\"%s\",%d,%d,0,\"%s\"",
+				 sock->link_id, dst_addr_str,
+				 ntohs(net_sin(&dst)->sin_port), ntohs(net_sin(&src)->sin_port),
+				 src_addr_str);
+		} else {
+			snprintk(connect_msg, sizeof(connect_msg),
+				 "AT+CIPSTART=%d,\"UDP\",\"%s\",%d",
+				 sock->link_id, dst_addr_str,
+				 ntohs(net_sin(&dst)->sin_port));
+		}
 	}
 
 	LOG_DBG("link %d, ip_proto %s, addr %s", sock->link_id,
 		esp_socket_ip_proto(sock) == IPPROTO_TCP ? "TCP" : "UDP",
-		addr_str);
+		dst_addr_str);
 
 	ret = esp_cmd_send(dev, NULL, 0, connect_msg, ESP_CMD_TIMEOUT);
 	if (ret == 0) {
@@ -110,26 +131,16 @@ static int esp_bind(struct net_context *context, const struct sockaddr *addr,
 	}
 
 	if (IS_ENABLED(CONFIG_NET_IPV4) && addr->sa_family == AF_INET) {
-		struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
-
 		LOG_DBG("link %d", sock->link_id);
 
-		if (addr4->sin_addr.s_addr == INADDR_ANY) {
-			return 0;
-		}
-
 		if (esp_socket_connected(sock)) {
 			return -EISCONN;
 		}
 
 		k_mutex_lock(&sock->lock, K_FOREVER);
-		sock->dst = *addr;
-		sock->connect_cb = NULL;
-		sock->conn_user_data = NULL;
+		sock->src = *addr;
 		k_mutex_unlock(&sock->lock);
 
-		_sock_connect(dev, sock);
-
 		return 0;
 	}
 
@@ -616,11 +627,23 @@ static int esp_recv(struct net_context *context,
 		    void *user_data)
 {
 	struct esp_socket *sock = context->offload_context;
+	struct esp_data *dev = esp_socket_to_dev(sock);
 	int ret;
 
 	LOG_DBG("link_id %d, timeout %d, cb %p, data %p",
 		sock->link_id, timeout, cb, user_data);
 
+	/*
+	 * UDP "listening" socket needs to be bound using AT+CIPSTART before any
+	 * traffic can be received.
+	 */
+	if (!esp_socket_connected(sock) &&
+	    esp_socket_ip_proto(sock) == IPPROTO_UDP &&
+	    sock->src.sa_family == AF_INET &&
+	    net_sin(&sock->src)->sin_port != 0) {
+		_sock_connect(dev, sock);
+	}
+
 	k_mutex_lock(&sock->lock, K_FOREVER);
 	sock->recv_cb = cb;
 	sock->recv_user_data = user_data;

drivers/wifi/esp_at/esp_socket.c

@@ -32,6 +32,8 @@ struct esp_socket *esp_socket_get(struct esp_data *data,
 
 			sock->connect_cb = NULL;
 			sock->recv_cb = NULL;
+			memset(&sock->src, 0x0, sizeof(sock->src));
+			memset(&sock->dst, 0x0, sizeof(sock->dst));
 
 			atomic_inc(&sock->refcount);
 
@@ -142,7 +144,7 @@ static struct net_pkt *esp_socket_prepare_pkt(struct esp_socket *sock,
 
 #if defined(CONFIG_WIFI_ESP_AT_CIPDINFO_USE)
 	memcpy(&pkt->remote, &sock->context->remote, sizeof(pkt->remote));
-	pkt->family = sock->dst.sa_family;
+	pkt->family = sock->src.sa_family;
 #endif
 
 	return pkt;