tap_ip6_daddr() selects the reply destination based on our source address type (link-local), so it always returns addr_ll_seen. But if the client sent from a global address, we would reply to an address different from what the client is expecting. Since RFC 9915 (section 18.3.10) allows clients to use global addresses for DHCPv6, we now correct this, and always respond to the address the client was using. We also remove a redundant addr_ll_seen assignment, since this is already done by tap.c when processing IPv6 packets. Note: if the client uses a global source address, our reply will still have a link-local source, creating a scope mismatch. Fixing this properly would require a mechanism to allocate a global address for the DHCPv6 server, which we currently don't have. Responding to the client's actual source address is still a net improvement over the previous behavior of replying to an unrelated cached address. Note 2: This commit isn't actually a fix to an observed problem, but rather an answer to a theoretical issue, adding completeness to the mechanism and simplifying subsequent changes in this series. Signed-off-by: Jon Maloy --- v8: -Updated commit log, acknowledging concerns expressed by David and Stefano. -Some minor changes also addressing feedback from the same persons. --- dhcpv6.c | 16 ++++++++-------- dhcpv6.h | 2 +- tap.c | 15 --------------- tap.h | 2 -- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index 97c04e2c..29c7e320 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -370,12 +370,14 @@ notonlink: /** * dhcpv6_send_ia_notonlink() - Send NotOnLink status * @c: Execution context + * @caddr: Source address of client message (reply destination) * @ia_base: Non-appropriate IA_NA or IA_TA base * @client_id_base: Client ID message option base * @len: Client ID length * @xid: Transaction ID for message exchange */ static void dhcpv6_send_ia_notonlink(struct ctx *c, + const struct in6_addr *caddr, const struct iov_tail *ia_base, const struct iov_tail *client_id_base, int len, uint32_t xid) @@ -405,8 +407,7 @@ static void dhcpv6_send_ia_notonlink(struct ctx *c, resp_not_on_link.hdr.xid = xid; - tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, - xid, &resp_not_on_link, n); + tap_udp6_send(c, src, 547, caddr, 546, xid, &resp_not_on_link, n); } /** @@ -590,8 +591,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, if (mlen + sizeof(*uh) != ntohs(uh->len) || mlen < sizeof(*mh)) return -1; - c->ip6.addr_ll_seen = *saddr; - src = &c->ip6.our_tap_ll; mh = IOV_REMOVE_HEADER(data, mh_storage); @@ -630,8 +629,10 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, if (dhcpv6_ia_notonlink(data, &c->ip6.addr)) { - dhcpv6_send_ia_notonlink(c, data, &client_id_base, - ntohs(client_id->l), mh->xid); + dhcpv6_send_ia_notonlink(c, saddr, data, + &client_id_base, + ntohs(client_id->l), + mh->xid); return 1; } @@ -680,8 +681,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, resp.hdr.xid = mh->xid; - tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, - mh->xid, &resp, n); + tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n); c->ip6.addr_seen = c->ip6.addr; return 1; diff --git a/dhcpv6.h b/dhcpv6.h index c706dfdb..1015a1a7 100644 --- a/dhcpv6.h +++ b/dhcpv6.h @@ -7,7 +7,7 @@ #define DHCPV6_H int dhcpv6(struct ctx *c, struct iov_tail *data, - struct in6_addr *saddr, struct in6_addr *daddr); + const struct in6_addr *saddr, const struct in6_addr *daddr); void dhcpv6_init(const struct ctx *c); #endif /* DHCPV6_H */ diff --git a/tap.c b/tap.c index 6d93c7ce..d4189617 100644 --- a/tap.c +++ b/tap.c @@ -160,21 +160,6 @@ void tap_send_single(const struct ctx *c, const void *data, size_t l2len) } } -/** - * tap_ip6_daddr() - Normal IPv6 destination address for inbound packets - * @c: Execution context - * @src: Source address - * - * Return: pointer to IPv6 address - */ -const struct in6_addr *tap_ip6_daddr(const struct ctx *c, - const struct in6_addr *src) -{ - if (IN6_IS_ADDR_LINKLOCAL(src)) - return &c->ip6.addr_ll_seen; - return &c->ip6.addr_seen; -} - /** * tap_push_l2h() - Build an L2 header for an inbound packet * @c: Execution context diff --git a/tap.h b/tap.h index 07ca0965..b335933f 100644 --- a/tap.h +++ b/tap.h @@ -96,8 +96,6 @@ void tap_udp4_send(const struct ctx *c, struct in_addr src, in_port_t sport, const void *in, size_t dlen); void tap_icmp4_send(const struct ctx *c, struct in_addr src, struct in_addr dst, const void *in, const void *src_mac, size_t l4len); -const struct in6_addr *tap_ip6_daddr(const struct ctx *c, - const struct in6_addr *src); void *tap_push_ip6h(struct ipv6hdr *ip6h, const struct in6_addr *src, const struct in6_addr *dst, size_t l4len, uint8_t proto, uint32_t flow); -- 2.52.0