From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202504 header.b=LtmaXBhh; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id E298C5A0276 for ; Wed, 16 Apr 2025 11:07:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202504; t=1744794428; bh=aOgJaauq8SHaRUIEyD//y49LnwiAkG1djeNao6KrfXU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LtmaXBhh7c1vAK8bbw2usbDhGXtjcpg8uYx1z7R6Am98RiLuJV3yXNXsZzdIDd3pS LHLu4HJf2BjRy/WOo+F8i3SRxubn9L+ZIFHwx2n7F3ynEcSBfGaGpw2GqsqBinoJC9 XBUc/7z1DYi9PLWMF9ayrhRgf6hVdF0UtY2/9hcl6xUKaeSCp5tO5/j8zwHKEsZnWm KUWE07lPm11aH34u5Trk3XMrbptj53wZZg5zDpeiIOB3xNfsI2wOyXY4plT5MK3nk3 0V4v7KDu4tbaa+U8thQ7Xy4Bc2n6LLWEAy5XTq7SIMYDFwS6HqXhp3RH0bfrgcGD0f tQGPKadNlnk8Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZcwC05Zsdz4xQN; Wed, 16 Apr 2025 19:07:08 +1000 (AEST) From: David Gibson To: Stefano Brivio , Jon Maloy , passt-dev@passt.top Subject: [PATCH 3/4] udp: Rework offender address handling in udp_sock_recverr() Date: Wed, 16 Apr 2025 19:07:06 +1000 Message-ID: <20250416090707.393497-4-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250416090707.393497-1-david@gibson.dropbear.id.au> References: <20250416090707.393497-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: WHEGMXGRD2I45GLWAIHTVBH3ECPSQX3S X-Message-ID-Hash: WHEGMXGRD2I45GLWAIHTVBH3ECPSQX3S X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Make a number of changes to udp_sock_recverr() to improve the robustness of how we handle addresses. * Get the "offender" address (source of the ICMP packet) using the SO_EE_OFFENDER() macro, reducing assumptions about structure layout. * Parse the offender sockaddr using inany_from_sockaddr() * Check explicitly that the source and destination pifs are what we expect. Previously we checked something that was probably equivalent in practice, but isn't strictly speaking what we require for the rest of the code. * Verify that for an ICMPv4 error we also have an IPv4 source/offender and destination/endpoint address * Verify that for an ICMPv6 error we have an IPv6 endpoint * Improve debug reporting of any failures Signed-off-by: David Gibson --- udp.c | 67 ++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/udp.c b/udp.c index 57769d06..4352520e 100644 --- a/udp.c +++ b/udp.c @@ -159,6 +159,12 @@ udp_meta[UDP_MAX_FRAMES]; MAX(CMSG_SPACE(sizeof(struct in_pktinfo)), \ CMSG_SPACE(sizeof(struct in6_pktinfo))) +#define RECVERR_SPACE \ + MAX(CMSG_SPACE(sizeof(struct sock_extended_err) + \ + sizeof(struct sockaddr_in)), \ + CMSG_SPACE(sizeof(struct sock_extended_err) + \ + sizeof(struct sockaddr_in6))) + /** * enum udp_iov_idx - Indices for the buffers making up a single UDP frame * @UDP_IOV_TAP tap specific header @@ -516,12 +522,8 @@ static int udp_pktinfo(struct msghdr *msg, union inany_addr *dst) static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx, uint8_t pif, in_port_t port) { - struct errhdr { - struct sock_extended_err ee; - union sockaddr_inany saddr; - }; - char buf[PKTINFO_SPACE + CMSG_SPACE(sizeof(struct errhdr))]; - const struct errhdr *eh = NULL; + char buf[PKTINFO_SPACE + RECVERR_SPACE]; + const struct sock_extended_err *ee; char data[ICMP6_MAX_DLEN]; struct cmsghdr *hdr; struct iovec iov = { @@ -538,7 +540,12 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx, .msg_controllen = sizeof(buf), }; const struct flowside *toside; - flow_sidx_t tosidx; + char astr[INANY_ADDRSTRLEN]; + char sastr[SOCKADDR_STRLEN]; + union inany_addr offender; + const struct in_addr *o4; + in_port_t offender_port; + uint8_t topif; size_t dlen; ssize_t rc; @@ -569,10 +576,10 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx, return -1; } - eh = (const struct errhdr *)CMSG_DATA(hdr); + ee = (const struct sock_extended_err *)CMSG_DATA(hdr); debug("%s error on UDP socket %i: %s", - str_ee_origin(&eh->ee), s, strerror_(eh->ee.ee_errno)); + str_ee_origin(ee), s, strerror_(ee->ee_errno)); if (!flow_sidx_valid(sidx)) { /* No hint from the socket, determine flow from addresses */ @@ -588,25 +595,43 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx, debug("Ignoring UDP error without flow"); return 1; } + } else { + pif = pif_at_sidx(sidx); } - tosidx = flow_sidx_opposite(sidx); - toside = flowside_at_sidx(tosidx); + toside = flowside_at_sidx(flow_sidx_opposite(sidx)); + topif = pif_at_sidx(flow_sidx_opposite(sidx)); dlen = rc; - if (pif_is_socket(pif_at_sidx(tosidx))) { - /* XXX Is there any way to propagate ICMPs from socket to - * socket? */ - } else if (hdr->cmsg_level == IPPROTO_IP) { + if (inany_from_sockaddr(&offender, &offender_port, + SO_EE_OFFENDER(ee)) < 0) + goto fail; + + if (pif != PIF_HOST || topif != PIF_TAP) + /* XXX Can we support any other cases? */ + goto fail; + + if (hdr->cmsg_level == IPPROTO_IP && + (o4 = inany_v4(&offender)) && inany_v4(&toside->eaddr)) { dlen = MIN(dlen, ICMP4_MAX_DLEN); - udp_send_tap_icmp4(c, &eh->ee, toside, - eh->saddr.sa4.sin_addr, data, dlen); - } else if (hdr->cmsg_level == IPPROTO_IPV6) { - udp_send_tap_icmp6(c, &eh->ee, toside, - &eh->saddr.sa6.sin6_addr, data, - dlen, sidx.flowi); + udp_send_tap_icmp4(c, ee, toside, *o4, data, dlen); + return 1; + } + + if (hdr->cmsg_level == IPPROTO_IPV6 && !inany_v4(&toside->eaddr)) { + udp_send_tap_icmp6(c, ee, toside, &offender.a6, data, dlen, + sidx.flowi); + return 1; } +fail: + flow_dbg(flow_at_sidx(sidx), + "Can't propagate %s error from %s %s to %s %s", + str_ee_origin(ee), + pif_name(pif), + sockaddr_ntop(SO_EE_OFFENDER(ee), sastr, sizeof(sastr)), + pif_name(topif), + inany_ntop(&toside->eaddr, astr, sizeof(astr))); return 1; } -- 2.49.0