[PATCH v2] ndp.c: Turn NDP responder into more declarative implementation

[PATCH v2] ndp.c: Turn NDP responder into more declarative implementation

[AbdAlRahman Gad]

- Add structs for NA, RA, NS, MTU, prefix info, option header,
  link-layer address, RDNSS, DNSSL and link-layer for RA message.

- Turn NA message from purely imperative, going byte by byte,
  to declarative by filling it's struct.

- Turn part of RA message into declarative.

- Move packet_add() to be before the call of ndp() in tap6_handler()
  if the protocol of the packet  is ICMPv6.

- Add a pool of packets as an additional parameter to ndp().

- Check the size of NS packet with packet_get() before sending an NA
  packet.

- Add documentation for the structs.

- Add an enum for NDP option types.

Link: https://bugs.passt.top/show_bug.cgi?id=21

Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
---
 ndp.c | 317 ++++++++++++++++++++++++++++++++++++++++++++--------------
 ndp.h |   3 +-
 tap.c |   5 +-
 3 files changed, 246 insertions(+), 79 deletions(-)

diff --git a/ndp.c b/ndp.c
index cea3df5..9a80397 100644
--- a/ndp.c
+++ b/ndp.c
@@ -38,22 +38,193 @@
 #define NS	135
 #define NA	136
 
+enum ndp_option_types {
+	OPT_SRC_L2_ADDR		= 1,
+	OPT_TARGET_L2_ADDR	= 2,
+	OPT_PREFIX_INFO		= 3,
+	OPT_MTU			= 5,
+	OPT_RDNSS_TYPE		= 25,
+	OPT_DNSSL_TYPE		= 31,
+};
+
+/**
+ * struct opt_header - Option header
+ * @type:	Option type
+ * @len:	Option length, in units of 8 bytes
+*/
+struct opt_header {
+	uint8_t type;
+	uint8_t len;
+} __attribute__((packed));
+
+/**
+ * struct opt_l2_addr - Link-layer address
+ * @header:	Option header
+ * @mac:	MAC address
+ */
+struct opt_l2_addr {
+	struct opt_header header;
+	unsigned char mac[ETH_ALEN];
+} __attribute__((packed));
+
+/**
+ * struct ndp_na - NDP Neighbor Advertisement (NA) message
+ * @ih:			ICMPv6 header
+ * @target_addr:	Target IPv6 address
+ * @target_l2_addr:	Target link-layer address
+ */
+struct ndp_na {
+	struct icmp6hdr ih;
+	struct in6_addr target_addr;
+	struct opt_l2_addr target_l2_addr;
+} __attribute__((packed));
+
+/**
+ * struct opt_prefix_info - Prefix Information option
+ * @header:		Option header
+ * @prefix_len:		The number of leading bits in the Prefix that are valid
+ * @prefix_flags:	Flags associated with the prefix
+ * @valid_lifetime:	Valid lifetime (ms)
+ * @pref_lifetime:	Preferred lifetime (ms)
+ * @reserved:		Unused
+ */
+struct opt_prefix_info {
+	struct opt_header header;
+	uint8_t prefix_len;
+	uint8_t prefix_flags;
+	uint32_t valid_lifetime;
+	uint32_t pref_lifetime;
+	uint32_t reserved;
+} __attribute__((packed));
+
+/**
+ * struct mtu_opt - Maximum transmission unit (MTU) option
+ * @header:		Option header
+ * @reserved:		Unused
+ * @value:		MTU value, network order
+ */
+struct mtu_opt {
+	struct opt_header header;
+	uint16_t reserved;
+	uint32_t value;
+} __attribute__((packed));
+
+/**
+ * struct rdnss - Recursive DNS Server (RDNSS) option
+ * @header:	Option header
+ * @reserved:	Unused
+ * @lifetime:	Validity time (s)
+ * @dns:	List of DNS server addresses
+ */
+struct opt_rdnss {
+	struct opt_header header;
+	uint16_t reserved;
+	uint32_t lifetime;
+	struct in6_addr dns[MAXNS + 1];
+} __attribute__((packed));
+
+/**
+ * struct dnssl - DNS Search List (DNSSL) option
+ * @header:		Option header
+ * @reserved:		Unused
+ * @lifetime:		Validity time (s)
+ * @domains:		List of NULL-seperated search domains
+ */
+struct opt_dnssl {
+	struct opt_header header;
+	uint16_t reserved;
+	uint32_t lifetime;
+	unsigned char domains[MAXDNSRCH * NS_MAXDNAME];
+} __attribute__((packed));
+
+/**
+ * struct ndp_ra - NDP Router Advertisement (RA) message
+ * @ih:			ICMPv6 header
+ * @reachable:		Reachability time, after confirmation (ms)
+ * @retrans:		Time between retransmitted NS messages (ms)
+ * @prefix_info:	Prefix Information option
+ * @prefix:		IPv6 prefix
+ * @mtu:		MTU option
+ * @source_ll:		Target link-layer address
+ * @var:		Variable fields
+ */
+struct ndp_ra {
+	struct icmp6hdr ih;
+	uint32_t reachable;
+	uint32_t retrans;
+	struct opt_prefix_info prefix_info;
+	struct in6_addr prefix;
+	struct opt_l2_addr source_ll;
+
+	unsigned char var[sizeof(struct mtu_opt) +sizeof(struct opt_rdnss) +
+			  sizeof(struct opt_dnssl)];
+} __attribute__((packed));
+
+/**
+ * struct ndp_ns - NDP Neighbor Solicitation (NS) message
+ * @ih:			ICMPv6 header
+ * @target_addr:	Target IPv6 address
+ */
+struct ndp_ns {
+	struct icmp6hdr ih;
+	struct in6_addr target_addr;
+} __attribute__((packed));
+
 /**
  * ndp() - Check for NDP solicitations, reply as needed
  * @c:		Execution context
  * @ih:		ICMPv6 header
- * @saddr	Source IPv6 address
+ * @saddr:	Source IPv6 address
+ * @p:		Packet pool
  *
  * Return: 0 if not handled here, 1 if handled, -1 on failure
  */
-int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr)
+int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
+	const struct pool *p)
 {
+	struct ndp_na na = {
+		.ih = {
+			.icmp6_type		= NA,
+			.icmp6_code		= 0,
+			.icmp6_router		= 1,
+			.icmp6_solicited	= 1,
+			.icmp6_override		= 1,
+		},
+		.target_l2_addr = {
+			.header	= {
+				.type		= OPT_TARGET_L2_ADDR,
+				.len		= 1,
+			},
+		}
+	};
+	struct ndp_ra ra = {
+		.ih = {
+			.icmp6_type		= RA,
+			.icmp6_code		= 0,
+			.icmp6_hop_limit	= 255,
+			/* RFC 8319 */
+			.icmp6_rt_lifetime	= htons_constant(65535),
+			.icmp6_addrconf_managed	= 1,
+		},
+		.prefix_info = {
+			.header = {
+				.type		= OPT_PREFIX_INFO,
+				.len		= 4,
+			},
+			.prefix_len		= 64,
+			.prefix_flags		= 0xc0,	/* prefix flags: L, A */
+			.valid_lifetime		= ~0U,
+			.pref_lifetime		= ~0U,
+		},
+		.source_ll = {
+			.header = {
+				.type		= OPT_SRC_L2_ADDR,
+				.len		= 1,
+			},
+		},
+	};
 	const struct in6_addr *rsaddr; /* src addr for reply */
-	char buf[BUFSIZ] = { 0 };
-	struct ipv6hdr *ip6hr;
-	struct icmp6hdr *ihr;
-	struct ethhdr *ehr;
-	unsigned char *p;
+	unsigned char *ptr = NULL;
 	size_t dlen;
 
 	if (ih->icmp6_type < RS || ih->icmp6_type > NA)
@@ -62,28 +233,22 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr)
 	if (c->no_ndp)
 		return 1;
 
-	ehr = (struct ethhdr *)buf;
-	ip6hr = (struct ipv6hdr *)(ehr + 1);
-	ihr = (struct icmp6hdr *)(ip6hr + 1);
-
 	if (ih->icmp6_type == NS) {
+		struct ndp_ns *ns = packet_get(p, 0, 0, sizeof(struct ndp_ns),
+					       NULL);
+
+		if (!ns)
+			return -1;
+
 		if (IN6_IS_ADDR_UNSPECIFIED(saddr))
 			return 1;
 
 		info("NDP: received NS, sending NA");
-		ihr->icmp6_type = NA;
-		ihr->icmp6_code = 0;
-		ihr->icmp6_router = 1;
-		ihr->icmp6_solicited = 1;
-		ihr->icmp6_override = 1;
-
-		p = (unsigned char *)(ihr + 1);
-		memcpy(p, ih + 1, sizeof(struct in6_addr)); /* target address */
-		p += 16;
-		*p++ = 2;				    /* target ll */
-		*p++ = 1;				    /* length */
-		memcpy(p, c->mac, ETH_ALEN);
-		p += 6;
+
+		memcpy(&na.target_addr, &ns->target_addr,
+		       sizeof(na.target_addr));
+		memcpy(na.target_l2_addr.mac, c->mac, ETH_ALEN);
+
 	} else if (ih->icmp6_type == RS) {
 		size_t dns_s_len = 0;
 		int i, n;
@@ -92,31 +257,20 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr)
 			return 1;
 
 		info("NDP: received RS, sending RA");
-		ihr->icmp6_type = RA;
-		ihr->icmp6_code = 0;
-		ihr->icmp6_hop_limit = 255;
-		ihr->icmp6_rt_lifetime = htons(65535); /* RFC 8319 */
-		ihr->icmp6_addrconf_managed = 1;
-
-		p = (unsigned char *)(ihr + 1);
-		p += 8;				/* reachable, retrans time */
-		*p++ = 3;			/* prefix */
-		*p++ = 4;			/* length */
-		*p++ = 64;			/* prefix length */
-		*p++ = 0xc0;			/* prefix flags: L, A */
-		*(uint32_t *)p = (uint32_t)~0U;	/* lifetime */
-		p += 4;
-		*(uint32_t *)p = (uint32_t)~0U;	/* preferred lifetime */
-		p += 8;
-		memcpy(p, &c->ip6.addr, 8);	/* prefix */
-		p += 16;
+		memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
+
+		ptr = &ra.var[0];
 
 		if (c->mtu != -1) {
-			*p++ = 5;			/* type */
-			*p++ = 1;			/* length */
-			p += 2;				/* reserved */
-			*(uint32_t *)p = htonl(c->mtu);	/* MTU */
-			p += 4;
+			struct mtu_opt *mtu = (struct mtu_opt *)ptr;
+			*mtu = (struct mtu_opt) {
+				.header = {
+					.type		= OPT_MTU,
+					.len		= 1,
+				},
+				.value			= htonl(c->mtu),
+			};
+			ptr += sizeof(struct mtu_opt);
 		}
 
 		if (c->no_dhcp_dns)
@@ -124,59 +278,64 @@ int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr)
 
 		for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
 		if (n) {
-			*p++ = 25;				/* RDNSS */
-			*p++ = 1 + 2 * n;			/* length */
-			p += 2;					/* reserved */
-			*(uint32_t *)p = (uint32_t)~0U;		/* lifetime */
-			p += 4;
-
+			struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr;
+			*rdnss = (struct opt_rdnss) {
+				.header = {
+					.type		= OPT_RDNSS_TYPE,
+					.len		= 1 + 2 * n,
+				},
+				.lifetime		= ~0U,
+			};
 			for (i = 0; i < n; i++) {
-				memcpy(p, &c->ip6.dns[i], 16);	/* address */
-				p += 16;
+				memcpy(&rdnss->dns[i], &c->ip6.dns[i],
+				       sizeof(rdnss->dns[i]));
 			}
+			ptr += offsetof(struct opt_rdnss, dns) +
+			       i * sizeof(rdnss->dns[0]);
 
 			for (n = 0; *c->dns_search[n].n; n++)
 				dns_s_len += strlen(c->dns_search[n].n) + 2;
 		}
 
 		if (!c->no_dhcp_dns_search && dns_s_len) {
-			*p++ = 31;				/* DNSSL */
-			*p++ = (dns_s_len + 8 - 1) / 8 + 1;	/* length */
-			p += 2;					/* reserved */
-			*(uint32_t *)p = (uint32_t)~0U;		/* lifetime */
-			p += 4;
+			struct opt_dnssl *dnssl = (struct opt_dnssl *)ptr;
+			*dnssl = (struct opt_dnssl) {
+				.header = {
+					.type = OPT_DNSSL_TYPE,
+					.len  = DIV_ROUND_UP(dns_s_len, 8) + 1,
+				},
+				.lifetime     = ~0U,
+			};
+			ptr = dnssl->domains;
 
 			for (i = 0; i < n; i++) {
+				size_t len;
 				char *dot;
 
-				*(p++) = '.';
+				*(ptr++) = '.';
 
-				strncpy((char *)p, c->dns_search[i].n,
-					sizeof(buf) -
-					((intptr_t)p - (intptr_t)buf));
-				for (dot = (char *)p - 1; *dot; dot++) {
+				len = sizeof(dnssl->domains) -
+				      (ptr - dnssl->domains);
+
+				strncpy((char *)ptr, c->dns_search[i].n, len);
+				for (dot = (char *)ptr - 1; *dot; dot++) {
 					if (*dot == '.')
 						*dot = strcspn(dot + 1, ".");
 				}
-				p += strlen(c->dns_search[i].n);
-				*(p++) = 0;
+				ptr += strlen(c->dns_search[i].n);
+				*(ptr++) = 0;
 			}
 
-			memset(p, 0, 8 - dns_s_len % 8);	/* padding */
-			p += 8 - dns_s_len % 8;
+			memset(ptr, 0, 8 - dns_s_len % 8);	/* padding */
+			ptr += 8 - dns_s_len % 8;
 		}
 
 dns_done:
-		*p++ = 1;			/* source ll */
-		*p++ = 1;			/* length */
-		memcpy(p, c->mac, ETH_ALEN);
-		p += 6;
+		memcpy(&ra.source_ll.mac, c->mac, ETH_ALEN);
 	} else {
 		return 1;
 	}
 
-	dlen = (uintptr_t)p - (uintptr_t)ihr - sizeof(*ihr);
-
 	if (IN6_IS_ADDR_LINKLOCAL(saddr))
 		c->ip6.addr_ll_seen = *saddr;
 	else
@@ -187,7 +346,13 @@ dns_done:
 	else
 		rsaddr = &c->ip6.addr_ll;
 
-	tap_icmp6_send(c, rsaddr, saddr, ihr, dlen + sizeof(*ihr));
+	if (ih->icmp6_type == NS) {
+		dlen = sizeof(struct ndp_na);
+		tap_icmp6_send(c, rsaddr, saddr, &na, dlen);
+	} else if (ih->icmp6_type == RS) {
+		dlen = ptr - (unsigned char *)&ra;
+		tap_icmp6_send(c, rsaddr, saddr, &ra, dlen);
+	}
 
 	return 1;
 }
diff --git a/ndp.h b/ndp.h
index b33cd69..a786441 100644
--- a/ndp.h
+++ b/ndp.h
@@ -6,6 +6,7 @@
 #ifndef NDP_H
 #define NDP_H
 
-int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr);
+int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
+	const struct pool *p);
 
 #endif /* NDP_H */
diff --git a/tap.c b/tap.c
index 5852705..87be3a6 100644
--- a/tap.c
+++ b/tap.c
@@ -808,12 +808,13 @@ resume:
 			if (l4len < sizeof(struct icmp6hdr))
 				continue;
 
-			if (ndp(c, (struct icmp6hdr *)l4h, saddr))
+			packet_add(pkt, l4len, l4h);
+
+			if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt))
 				continue;
 
 			tap_packet_debug(NULL, ip6h, NULL, proto, NULL, 1);
 
-			packet_add(pkt, l4len, l4h);
 			icmp_tap_handler(c, PIF_TAP, AF_INET6,
 					 saddr, daddr, pkt, now);
 			continue;
-- 
@@ -808,12 +808,13 @@ resume:
 			if (l4len < sizeof(struct icmp6hdr))
 				continue;
 
-			if (ndp(c, (struct icmp6hdr *)l4h, saddr))
+			packet_add(pkt, l4len, l4h);
+
+			if (ndp(c, (struct icmp6hdr *)l4h, saddr, pkt))
 				continue;
 
 			tap_packet_debug(NULL, ip6h, NULL, proto, NULL, 1);
 
-			packet_add(pkt, l4len, l4h);
 			icmp_tap_handler(c, PIF_TAP, AF_INET6,
 					 saddr, daddr, pkt, now);
 			continue;
-- 
2.46.0

Re: [PATCH v2] ndp.c: Turn NDP responder into more declarative implementation

[Stefano Brivio]

Just two very minor details I'll fix up on merge in a bit (you don't
need to send a new version):

On Mon, 12 Aug 2024 22:33:38 +0300
AbdAlRahman Gad <abdobngad@gmail.com> wrote:

> - Add structs for NA, RA, NS, MTU, prefix info, option header,
>   link-layer address, RDNSS, DNSSL and link-layer for RA message.
> 
> - Turn NA message from purely imperative, going byte by byte,
>   to declarative by filling it's struct.
> 
> - Turn part of RA message into declarative.
> 
> - Move packet_add() to be before the call of ndp() in tap6_handler()
>   if the protocol of the packet  is ICMPv6.
> 
> - Add a pool of packets as an additional parameter to ndp().
> 
> - Check the size of NS packet with packet_get() before sending an NA
>   packet.
> 
> - Add documentation for the structs.
> 
> - Add an enum for NDP option types.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=21
> 
> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>
> ---
>  ndp.c | 317 ++++++++++++++++++++++++++++++++++++++++++++--------------
>  ndp.h |   3 +-
>  tap.c |   5 +-
>  3 files changed, 246 insertions(+), 79 deletions(-)
> 
> diff --git a/ndp.c b/ndp.c
> index cea3df5..9a80397 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -38,22 +38,193 @@
>  #define NS	135
>  #define NA	136
>  
> +enum ndp_option_types {
> +	OPT_SRC_L2_ADDR		= 1,
> +	OPT_TARGET_L2_ADDR	= 2,
> +	OPT_PREFIX_INFO		= 3,
> +	OPT_MTU			= 5,
> +	OPT_RDNSS_TYPE		= 25,
> +	OPT_DNSSL_TYPE		= 31,
> +};
> +
> +/**
> + * struct opt_header - Option header
> + * @type:	Option type
> + * @len:	Option length, in units of 8 bytes
> +*/
> +struct opt_header {
> +	uint8_t type;
> +	uint8_t len;
> +} __attribute__((packed));
> +
> +/**
> + * struct opt_l2_addr - Link-layer address
> + * @header:	Option header
> + * @mac:	MAC address
> + */
> +struct opt_l2_addr {
> +	struct opt_header header;
> +	unsigned char mac[ETH_ALEN];
> +} __attribute__((packed));
> +
> +/**
> + * struct ndp_na - NDP Neighbor Advertisement (NA) message
> + * @ih:			ICMPv6 header
> + * @target_addr:	Target IPv6 address
> + * @target_l2_addr:	Target link-layer address
> + */
> +struct ndp_na {
> +	struct icmp6hdr ih;
> +	struct in6_addr target_addr;
> +	struct opt_l2_addr target_l2_addr;
> +} __attribute__((packed));
> +
> +/**
> + * struct opt_prefix_info - Prefix Information option
> + * @header:		Option header
> + * @prefix_len:		The number of leading bits in the Prefix that are valid
> + * @prefix_flags:	Flags associated with the prefix
> + * @valid_lifetime:	Valid lifetime (ms)
> + * @pref_lifetime:	Preferred lifetime (ms)
> + * @reserved:		Unused
> + */
> +struct opt_prefix_info {
> +	struct opt_header header;
> +	uint8_t prefix_len;
> +	uint8_t prefix_flags;
> +	uint32_t valid_lifetime;
> +	uint32_t pref_lifetime;
> +	uint32_t reserved;
> +} __attribute__((packed));
> +
> +/**
> + * struct mtu_opt - Maximum transmission unit (MTU) option

If all the other structs are called opt_*, this should also be opt_mtu,
instead of mtu_opt.

> + * @header:		Option header
> + * @reserved:		Unused
> + * @value:		MTU value, network order
> + */
> +struct mtu_opt {
> +	struct opt_header header;
> +	uint16_t reserved;
> +	uint32_t value;
> +} __attribute__((packed));
> +
> +/**
> + * struct rdnss - Recursive DNS Server (RDNSS) option
> + * @header:	Option header
> + * @reserved:	Unused
> + * @lifetime:	Validity time (s)
> + * @dns:	List of DNS server addresses
> + */
> +struct opt_rdnss {
> +	struct opt_header header;
> +	uint16_t reserved;
> +	uint32_t lifetime;
> +	struct in6_addr dns[MAXNS + 1];
> +} __attribute__((packed));
> +
> +/**
> + * struct dnssl - DNS Search List (DNSSL) option
> + * @header:		Option header
> + * @reserved:		Unused
> + * @lifetime:		Validity time (s)
> + * @domains:		List of NULL-seperated search domains
> + */
> +struct opt_dnssl {
> +	struct opt_header header;
> +	uint16_t reserved;
> +	uint32_t lifetime;
> +	unsigned char domains[MAXDNSRCH * NS_MAXDNAME];
> +} __attribute__((packed));
> +
> +/**
> + * struct ndp_ra - NDP Router Advertisement (RA) message
> + * @ih:			ICMPv6 header
> + * @reachable:		Reachability time, after confirmation (ms)
> + * @retrans:		Time between retransmitted NS messages (ms)
> + * @prefix_info:	Prefix Information option
> + * @prefix:		IPv6 prefix
> + * @mtu:		MTU option
> + * @source_ll:		Target link-layer address
> + * @var:		Variable fields
> + */
> +struct ndp_ra {
> +	struct icmp6hdr ih;
> +	uint32_t reachable;
> +	uint32_t retrans;
> +	struct opt_prefix_info prefix_info;
> +	struct in6_addr prefix;
> +	struct opt_l2_addr source_ll;
> +
> +	unsigned char var[sizeof(struct mtu_opt) +sizeof(struct opt_rdnss) +

Missing whitespace before the second sizeof() here.

-- 
Stefano

Re: [PATCH v2] ndp.c: Turn NDP responder into more declarative implementation

[Stefano Brivio]

On Mon, 12 Aug 2024 22:33:38 +0300
AbdAlRahman Gad <abdobngad@gmail.com> wrote:

> - Add structs for NA, RA, NS, MTU, prefix info, option header,
>   link-layer address, RDNSS, DNSSL and link-layer for RA message.
> 
> - Turn NA message from purely imperative, going byte by byte,
>   to declarative by filling it's struct.
> 
> - Turn part of RA message into declarative.
> 
> - Move packet_add() to be before the call of ndp() in tap6_handler()
>   if the protocol of the packet  is ICMPv6.
> 
> - Add a pool of packets as an additional parameter to ndp().
> 
> - Check the size of NS packet with packet_get() before sending an NA
>   packet.
> 
> - Add documentation for the structs.
> 
> - Add an enum for NDP option types.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=21
> 
> Signed-off-by: AbdAlRahman Gad <abdobngad@gmail.com>

Applied, thanks!

I also closed https://bugs.passt.top/show_bug.cgi?id=21 as resolved.

-- 
Stefano