From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 04/11] icmp: Don't attempt to handle "wrong direction" ping socket traffic
Date: Mon, 18 Dec 2023 18:40:10 +1100 [thread overview]
Message-ID: <20231218074017.985092-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20231218074017.985092-1-david@gibson.dropbear.id.au>
Linux ICMP "ping" sockets are very specific in what they do. They let
userspace send ping requests (ICMP_ECHO or ICMP6_ECHO_REQUEST), and receive
matching replies (ICMP_ECHOREPLY or ICMP6_ECHO_REPLY). They don't let you
intercept or handle incoming ping requests.
In the case of passt/pasta that means we can process echo requests from tap
and forward them to a ping socket, then take the replies from the ping
socket and forward them to tap. We can't do the reverse: take echo
requests from the host and somehow forward them to the guest. There's
really no way for something outside to initiate a ping to a passt/pasta
connected guest and if there was we'd need an entirely different mechanism
to handle it.
However, we have some logic to deal with packets going in that reverse
direction. Remove it, since it can't ever be used that way. While we're
there use defines for the ICMPv6 types, instead of open coded type values.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
icmp.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/icmp.c b/icmp.c
index c745b7b..1f41440 100644
--- a/icmp.c
+++ b/icmp.c
@@ -93,8 +93,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)
icmp_id_map[V4][id].seq = seq;
}
- debug("ICMP: echo %s to tap, ID: %i, seq: %i",
- (ih->type == ICMP_ECHO) ? "request" : "reply", id, seq);
+ debug("ICMP: echo reply to tap, ID: %i, seq: %i", id, seq);
tap_icmp4_send(c, sr.sin_addr, tap_ip4_daddr(c), buf, n);
}
@@ -138,8 +137,7 @@ void icmpv6_sock_handler(const struct ctx *c, union epoll_ref ref)
icmp_id_map[V6][id].seq = seq;
}
- debug("ICMPv6: echo %s to tap, ID: %i, seq: %i",
- (ih->icmp6_type == 128) ? "request" : "reply", id, seq);
+ debug("ICMPv6: echo reply to tap, ID: %i, seq: %i", id, seq);
tap_icmp6_send(c, &sr.sin6_addr,
tap_ip6_daddr(c, &sr.sin6_addr), buf, n);
@@ -178,7 +176,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
if (!ih)
return 1;
- if (ih->type != ICMP_ECHO && ih->type != ICMP_ECHOREPLY)
+ if (ih->type != ICMP_ECHO)
return 1;
iref.id = id = ntohs(ih->un.echo.id);
@@ -205,8 +203,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
(struct sockaddr *)&sa, sizeof(sa)) < 0) {
debug("ICMP: failed to relay request to socket");
} else {
- debug("ICMP: echo %s to socket, ID: %i, seq: %i",
- (ih->type == ICMP_ECHO) ? "request" : "reply",
+ debug("ICMP: echo request to socket, ID: %i, seq: %i",
id, ntohs(ih->un.echo.sequence));
}
} else if (af == AF_INET6) {
@@ -222,7 +219,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
if (!ih)
return 1;
- if (ih->icmp6_type != 128 && ih->icmp6_type != 129)
+ if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
return 1;
iref.id = id = ntohs(ih->icmp6_identifier);
@@ -249,8 +246,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
(struct sockaddr *)&sa, sizeof(sa)) < 1) {
debug("ICMPv6: failed to relay request to socket");
} else {
- debug("ICMPv6: echo %s to socket, ID: %i, seq: %i",
- (ih->icmp6_type == 128) ? "request" : "reply",
+ debug("ICMPv6: echo request to socket, ID: %i, seq: %i",
id, ntohs(ih->icmp6_sequence));
}
}
--
@@ -93,8 +93,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)
icmp_id_map[V4][id].seq = seq;
}
- debug("ICMP: echo %s to tap, ID: %i, seq: %i",
- (ih->type == ICMP_ECHO) ? "request" : "reply", id, seq);
+ debug("ICMP: echo reply to tap, ID: %i, seq: %i", id, seq);
tap_icmp4_send(c, sr.sin_addr, tap_ip4_daddr(c), buf, n);
}
@@ -138,8 +137,7 @@ void icmpv6_sock_handler(const struct ctx *c, union epoll_ref ref)
icmp_id_map[V6][id].seq = seq;
}
- debug("ICMPv6: echo %s to tap, ID: %i, seq: %i",
- (ih->icmp6_type == 128) ? "request" : "reply", id, seq);
+ debug("ICMPv6: echo reply to tap, ID: %i, seq: %i", id, seq);
tap_icmp6_send(c, &sr.sin6_addr,
tap_ip6_daddr(c, &sr.sin6_addr), buf, n);
@@ -178,7 +176,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
if (!ih)
return 1;
- if (ih->type != ICMP_ECHO && ih->type != ICMP_ECHOREPLY)
+ if (ih->type != ICMP_ECHO)
return 1;
iref.id = id = ntohs(ih->un.echo.id);
@@ -205,8 +203,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
(struct sockaddr *)&sa, sizeof(sa)) < 0) {
debug("ICMP: failed to relay request to socket");
} else {
- debug("ICMP: echo %s to socket, ID: %i, seq: %i",
- (ih->type == ICMP_ECHO) ? "request" : "reply",
+ debug("ICMP: echo request to socket, ID: %i, seq: %i",
id, ntohs(ih->un.echo.sequence));
}
} else if (af == AF_INET6) {
@@ -222,7 +219,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
if (!ih)
return 1;
- if (ih->icmp6_type != 128 && ih->icmp6_type != 129)
+ if (ih->icmp6_type != ICMPV6_ECHO_REQUEST)
return 1;
iref.id = id = ntohs(ih->icmp6_identifier);
@@ -249,8 +246,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, int af,
(struct sockaddr *)&sa, sizeof(sa)) < 1) {
debug("ICMPv6: failed to relay request to socket");
} else {
- debug("ICMPv6: echo %s to socket, ID: %i, seq: %i",
- (ih->icmp6_type == 128) ? "request" : "reply",
+ debug("ICMPv6: echo request to socket, ID: %i, seq: %i",
id, ntohs(ih->icmp6_sequence));
}
}
--
2.43.0
next prev parent reply other threads:[~2023-12-18 7:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-18 7:40 [PATCH 00/11] RFC: ICMP reworks preliminary to flow table integration David Gibson
2023-12-18 7:40 ` [PATCH 01/11] checksum: Don't use linux/icmp.h when netinet/ip_icmp.h will do David Gibson
2023-12-18 7:40 ` [PATCH 02/11] icmp: Don't set "port" on destination sockaddr for ping sockets David Gibson
2023-12-18 7:40 ` [PATCH 03/11] icmp: Remove redundant initialisation of sendto() address David Gibson
2023-12-18 7:40 ` David Gibson [this message]
2023-12-18 7:40 ` [PATCH 05/11] icmp: Don't attempt to match host IDs to guest IDs David Gibson
2023-12-18 7:40 ` [PATCH 06/11] icmp: Use -1 to represent "missing" sockets David Gibson
2023-12-18 7:40 ` [PATCH 07/11] icmp: Simplify socket expiry scanning David Gibson
2023-12-18 7:40 ` [PATCH 08/11] icmp: Share more between IPv4 and IPv6 paths in icmp_tap_handler() David Gibson
2023-12-18 7:40 ` [PATCH 09/11] icmp: Consolidate icmp_sock_handler() with icmpv6_sock_handler() David Gibson
2023-12-18 7:40 ` [PATCH 10/11] icmp: Warn on receive errors from ping sockets David Gibson
2023-12-18 7:40 ` [PATCH 11/11] icmp: Validate packets received on " David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231218074017.985092-5-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).