ipv6: assign DNS servers per session from RADIUS

The ipv6_nd and ipv6_dhcp modules could only advertise the DNS servers
configured in [ipv6-dns], the same set for every subscriber. RFC 6911
defines DNS-Server-IPv6-Address (attribute 169) for exactly this, and
the attribute was already in the shipped dictionary and in attr_defs.h;
nothing read it.

Give struct ap_session an ipv6_dns list, filled by the radius module
from that attribute, and have both modules advertise it when the session
has one: in the RDNSS option of the router advertisements, and in the
DNS_SERVERS option of DHCPv6 replies. Sessions without a list of their
own keep getting the configured servers, so nothing changes for anyone
not sending the attribute.

The selection is a single ipv6_dns_get() shared by both modules rather
than a copy in each: they already duplicate the whole [ipv6-dns] parser,
and two copies of a precedence rule are two chances to drift. It caps
what it returns, so neither the RDNSS option length (one byte, in units
of 8) nor the router advertisement buffer can be pushed around by what a
RADIUS server sends. The radius module caps at the same 3 servers as
[ipv6-dns] accepts and warns once when a reply carries more.

An Access-Accept which carries the attribute replaces the whole
previously assigned list rather than appending to it, so a re-authorized
session ends up with the servers of the latest reply and not with a
concatenation. One which does not carry it leaves the current list
alone, which is how the IPv4 MS-Primary-DNS-Server attribute already
behaves.

ipv6_dns_test.c covers the selection: assigned wins over configured,
empty list means "nothing assigned" rather than "no DNS", the cap holds
for both sources, and a caller with no room gets nothing rather than a
stomped buffer. Wired into the ASAN/UBSAN workflow.

Inspired by the per-session IPv6 DNS support in accel-ppp-ng (commit
2df6eb99), reimplemented against mainline's ap_session and ipdb types.

10 files changed, 293 insertions, 10 deletions

diff --git a/.github/workflows/run-tests-asan-ubsan.yml b/.github/workflows/run-tests-asan-ubsan.yml
index 8b25701e..90074463 100644
--- a/.github/workflows/run-tests-asan-ubsan.yml
+++ b/.github/workflows/run-tests-asan-ubsan.yml
@@ -72,6 +72,11 @@ jobs:
               accel-pppd/ctrl/l2tp/packet_test.c accel-pppd/ctrl/l2tp/packet.c \
               -lcrypto
           /tmp/l2tp_packet_test
+          gcc -O2 -Wall -D_GNU_SOURCE -DAP_SESSIONID_LEN=16 \
+              -fsanitize=${{ matrix.sanitizer }} -fno-sanitize-recover=all \
+              -I accel-pppd/include -I accel-pppd/triton -I build \
+              -o /tmp/ipv6_dns_test accel-pppd/ipv6/ipv6_dns_test.c
+          /tmp/ipv6_dns_test
           gcc -O2 -Wall -o /tmp/bitpool_test accel-pppd/extra/bitpool_test.c
           /tmp/bitpool_test
 
diff --git a/accel-pppd/accel-ppp.conf b/accel-pppd/accel-ppp.conf
index e34ae919..c32e7612 100644
--- a/accel-pppd/accel-ppp.conf
+++ b/accel-pppd/accel-ppp.conf
@@ -415,6 +415,8 @@ delegate=fc00:2::/36,48,name=pool3
 delegate=fc00:3::/36,48,name=pool4,next=pool3
 
 [ipv6-dns]
+# Used for sessions which were not assigned their own DNS servers, e.g. by the
+# RADIUS DNS-Server-IPv6-Address attribute (RFC 6911)
 #fc00:1::1
 #fc00:1::2
 #fc00:1::3
diff --git a/accel-pppd/accel-ppp.conf.5 b/accel-pppd/accel-ppp.conf.5
index 07fb363d..7c042541 100644
--- a/accel-pppd/accel-ppp.conf.5
+++ b/accel-pppd/accel-ppp.conf.5
@@ -671,6 +671,12 @@ Specifies primary NBNS to be sent to peer.
 .BI "wins2=" x.x.x.x
 Specifies secondary NBNS to be sent to peer.
 .SH [ipv6-dns]
+These options apply to sessions which were not assigned DNS servers of their
+own. A RADIUS server may assign per session ones by returning up to 3
+DNS-Server-IPv6-Address attributes (RFC 6911) in the Access-Accept; those
+replace, rather than extend, the servers configured here, for that session
+only. Both the ipv6_nd module (RDNSS option of the router advertisements) and
+the ipv6_dhcp module (DNS_SERVERS option) honour them.
 .TP
 .BI "dns=" IPv6_address
 Specifies IPv6 DNS to be sent to peer. You may specify up to 3 dns options.
diff --git a/accel-pppd/include/ap_session.h b/accel-pppd/include/ap_session.h
index a4d3b867..a0733986 100644
--- a/accel-pppd/include/ap_session.h
+++ b/accel-pppd/include/ap_session.h
@@ -85,6 +85,12 @@ struct ap_session
 	struct ipv4db_item_t *ipv4;
 	struct ipv6db_item_t *ipv6;
 	struct ipv6db_prefix_t *ipv6_dp;
+	/* Per session IPv6 DNS servers, NULL when none were assigned and the
+	   globally configured ones ([ipv6-dns]) should be advertised instead.
+	   Only addr_list, and only the addr member of its entries, is
+	   meaningful here. Owned by whoever sets it, currently the radius
+	   module from the DNS-Server-IPv6-Address attribute (RFC 6911) */
+	struct ipv6db_item_t *ipv6_dns;
 	char *ipv4_pool_name;
 	char *ipv6_pool_name;
 	char *dpv6_pool_name;
diff --git a/accel-pppd/include/ipv6_dns.h b/accel-pppd/include/ipv6_dns.h
new file mode 100644
index 00000000..b604d006
--- /dev/null
+++ b/accel-pppd/include/ipv6_dns.h
@@ -0,0 +1,48 @@
+#ifndef __IPV6_DNS_H
+#define __IPV6_DNS_H
+
+#include <netinet/in.h>
+
+#include "list.h"
+#include "ipdb.h"
+#include "ap_session.h"
+
+/*
+ * Pick the IPv6 DNS servers to advertise to a session.
+ *
+ * A session may have been assigned its own servers (currently by the radius
+ * module, from the DNS-Server-IPv6-Address attribute of RFC 6911); those take
+ * precedence. Sessions without any fall back to the globally configured ones,
+ * which is what every session got before per session servers existed.
+ *
+ * Up to 'max' addresses are written to 'dns', the number written is returned.
+ * Callers advertise nothing when that is 0.
+ */
+static inline int ipv6_dns_get(const struct ap_session *ses,
+			       const struct in6_addr *conf_dns, int conf_dns_count,
+			       struct in6_addr *dns, int max)
+{
+	struct ipv6db_addr_t *a;
+	int count = 0;
+
+	if (ses && ses->ipv6_dns) {
+		list_for_each_entry(a, &ses->ipv6_dns->addr_list, entry) {
+			if (count == max)
+				break;
+			dns[count++] = a->addr;
+		}
+
+		/* An empty list means "nothing assigned", not "no DNS at all" */
+		if (count)
+			return count;
+	}
+
+	while (count < conf_dns_count && count < max) {
+		dns[count] = conf_dns[count];
+		count++;
+	}
+
+	return count;
+}
+
+#endif
diff --git a/accel-pppd/ipv6/dhcpv6.c b/accel-pppd/ipv6/dhcpv6.c
index b284df5f..4a80bdb9 100644
--- a/accel-pppd/ipv6/dhcpv6.c
+++ b/accel-pppd/ipv6/dhcpv6.c
@@ -20,6 +20,7 @@
 #include "log.h"
 #include "ppp.h"
 #include "ipdb.h"
+#include "ipv6_dns.h"
 #include "events.h"
 #include "iputils.h"
 
@@ -220,16 +221,19 @@ static void insert_status(struct dhcpv6_packet *pkt, struct dhcpv6_option *opt,
 static void insert_oro(struct dhcpv6_packet *reply, struct dhcpv6_option *opt)
 {
 	struct dhcpv6_option *opt1;
-	int i, j;
+	int i, j, dns_count;
 	uint16_t *ptr;
 	struct in6_addr addr, *addr_ptr;
+	struct in6_addr dns[MAX_DNS_COUNT];
 
 	for (i = ntohs(opt->hdr->len) / 2, ptr = (uint16_t *)opt->hdr->data; i; i--, ptr++) {
 		if (ntohs(*ptr) == D6_OPTION_DNS_SERVERS) {
-			if (conf_dns_count) {
-				opt1 = dhcpv6_option_alloc(reply, D6_OPTION_DNS_SERVERS, conf_dns_count * sizeof(addr));
-				for (j = 0, addr_ptr = (struct in6_addr *)opt1->hdr->data; j < conf_dns_count; j++, addr_ptr++)
-					memcpy(addr_ptr, conf_dns + j, sizeof(addr));
+			dns_count = ipv6_dns_get(reply->ses, conf_dns, conf_dns_count,
+						 dns, MAX_DNS_COUNT);
+			if (dns_count) {
+				opt1 = dhcpv6_option_alloc(reply, D6_OPTION_DNS_SERVERS, dns_count * sizeof(addr));
+				for (j = 0, addr_ptr = (struct in6_addr *)opt1->hdr->data; j < dns_count; j++, addr_ptr++)
+					memcpy(addr_ptr, dns + j, sizeof(addr));
 			}
 		} else if (ntohs(*ptr) == D6_OPTION_DOMAIN_LIST) {
 			if (conf_dnssl_size) {
diff --git a/accel-pppd/ipv6/ipv6_dns_test.c b/accel-pppd/ipv6/ipv6_dns_test.c
new file mode 100644
index 00000000..72b4751c
--- /dev/null
+++ b/accel-pppd/ipv6/ipv6_dns_test.c
@@ -0,0 +1,157 @@
+/*
+ * Standalone test for the IPv6 DNS server selection shared by the ipv6_nd and
+ * ipv6_dhcp modules.
+ *
+ * Not part of the cmake build. Compile and run from the top of the tree, with
+ * a configured build directory around for config.h:
+ *   gcc -O2 -Wall -D_GNU_SOURCE -DAP_SESSIONID_LEN=16 \
+ *       -I accel-pppd/include -I accel-pppd/triton -I build \
+ *       -o /tmp/ipv6_dns_test accel-pppd/ipv6/ipv6_dns_test.c && /tmp/ipv6_dns_test
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <arpa/inet.h>
+
+#include "ipv6_dns.h"
+
+static int failures;
+#define CHECK(cond) do { if (!(cond)) { \
+	fprintf(stderr, "FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); failures++; } } while (0)
+
+#define MAX_DNS_COUNT 3		/* as in nd.c and dhcpv6.c */
+
+static struct in6_addr a6(const char *str)
+{
+	struct in6_addr addr;
+
+	if (inet_pton(AF_INET6, str, &addr) != 1) {
+		fprintf(stderr, "bad address %s\n", str);
+		exit(1);
+	}
+
+	return addr;
+}
+
+static int is(const struct in6_addr *addr, const char *str)
+{
+	struct in6_addr expect = a6(str);
+
+	return memcmp(addr, &expect, sizeof(expect)) == 0;
+}
+
+/* A session carrying 'count' DNS servers taken from 'str' */
+static struct ap_session *session_with(const char **str, int count)
+{
+	struct ap_session *ses = calloc(1, sizeof(*ses));
+	struct ipv6db_item_t *item = calloc(1, sizeof(*item));
+	int i;
+
+	INIT_LIST_HEAD(&item->addr_list);
+	for (i = 0; i < count; i++) {
+		struct ipv6db_addr_t *a = calloc(1, sizeof(*a));
+
+		a->addr = a6(str[i]);
+		a->prefix_len = 128;
+		list_add_tail(&a->entry, &item->addr_list);
+	}
+
+	ses->ipv6_dns = item;
+
+	return ses;
+}
+
+static void session_free(struct ap_session *ses)
+{
+	if (ses->ipv6_dns) {
+		while (!list_empty(&ses->ipv6_dns->addr_list)) {
+			struct ipv6db_addr_t *a = list_entry(ses->ipv6_dns->addr_list.next,
+							     typeof(*a), entry);
+
+			list_del(&a->entry);
+			free(a);
+		}
+		free(ses->ipv6_dns);
+	}
+
+	free(ses);
+}
+
+int main(void)
+{
+	static const char *four[] = { "2001:db8::1", "2001:db8::2",
+				      "2001:db8::3", "2001:db8::4" };
+	struct in6_addr conf_dns[MAX_DNS_COUNT];
+	struct in6_addr dns[MAX_DNS_COUNT];
+	struct ap_session *ses;
+	int n;
+
+	conf_dns[0] = a6("fc00::53");
+	conf_dns[1] = a6("fc00::54");
+
+	/* No session at all: the configured servers, as before the feature */
+	n = ipv6_dns_get(NULL, conf_dns, 2, dns, MAX_DNS_COUNT);
+	CHECK(n == 2);
+	CHECK(is(&dns[0], "fc00::53"));
+	CHECK(is(&dns[1], "fc00::54"));
+
+	/* Session without assigned servers: same fallback */
+	ses = calloc(1, sizeof(*ses));
+	n = ipv6_dns_get(ses, conf_dns, 2, dns, MAX_DNS_COUNT);
+	CHECK(n == 2);
+	CHECK(is(&dns[0], "fc00::53"));
+	free(ses);
+
+	/* Nothing configured and nothing assigned: advertise nothing */
+	n = ipv6_dns_get(NULL, conf_dns, 0, dns, MAX_DNS_COUNT);
+	CHECK(n == 0);
+
+	/* Assigned servers win over the configured ones */
+	ses = session_with(four, 2);
+	n = ipv6_dns_get(ses, conf_dns, 2, dns, MAX_DNS_COUNT);
+	CHECK(n == 2);
+	CHECK(is(&dns[0], "2001:db8::1"));
+	CHECK(is(&dns[1], "2001:db8::2"));
+
+	/* ... and win even when nothing is configured */
+	n = ipv6_dns_get(ses, conf_dns, 0, dns, MAX_DNS_COUNT);
+	CHECK(n == 2);
+	CHECK(is(&dns[0], "2001:db8::1"));
+
+	session_free(ses);
+
+	/* An empty assigned list is "nothing assigned", not "no DNS" */
+	ses = session_with(four, 0);
+	n = ipv6_dns_get(ses, conf_dns, 2, dns, MAX_DNS_COUNT);
+	CHECK(n == 2);
+	CHECK(is(&dns[0], "fc00::53"));
+	session_free(ses);
+
+	/* More assigned than fit: keep the first max, never overrun */
+	ses = session_with(four, 4);
+	memset(dns, 0, sizeof(dns));
+	n = ipv6_dns_get(ses, conf_dns, 2, dns, MAX_DNS_COUNT);
+	CHECK(n == MAX_DNS_COUNT);
+	CHECK(is(&dns[0], "2001:db8::1"));
+	CHECK(is(&dns[1], "2001:db8::2"));
+	CHECK(is(&dns[2], "2001:db8::3"));
+
+	/* Same for the configured ones, should they ever exceed max */
+	n = ipv6_dns_get(NULL, conf_dns, 99, dns, MAX_DNS_COUNT);
+	CHECK(n == MAX_DNS_COUNT);
+
+	/* A caller with no room gets nothing rather than a stomped buffer */
+	n = ipv6_dns_get(ses, conf_dns, 2, dns, 0);
+	CHECK(n == 0);
+
+	session_free(ses);
+
+	if (failures) {
+		fprintf(stderr, "%d failure(s)\n", failures);
+		return 1;
+	}
+
+	printf("all tests passed\n");
+
+	return 0;
+}
diff --git a/accel-pppd/ipv6/nd.c b/accel-pppd/ipv6/nd.c
index 6dd00ee2..8b22da14 100644
--- a/accel-pppd/ipv6/nd.c
+++ b/accel-pppd/ipv6/nd.c
@@ -17,6 +17,7 @@
 #include "events.h"
 #include "mempool.h"
 #include "ipdb.h"
+#include "ipv6_dns.h"
 #include "iputils.h"
 
 #include "memdebug.h"
@@ -107,7 +108,8 @@ static void ipv6_nd_send_ra(struct ipv6_nd_handler_t *h, struct sockaddr_in6 *ds
 	//struct nd_opt_mtu *mtu;
 	struct ipv6db_addr_t *a;
 	struct in6_addr addr, peer_addr;
-	int i, prefix_len;
+	struct in6_addr dns[MAX_DNS_COUNT];
+	int i, prefix_len, dns_count;
 
 	if (!buf) {
 		log_emerg("out of memory\n");
@@ -174,15 +176,17 @@ static void ipv6_nd_send_ra(struct ipv6_nd_handler_t *h, struct sockaddr_in6 *ds
 		rinfo++;
 	}*/
 
-	if (conf_dns_count) {
+	dns_count = ipv6_dns_get(ses, conf_dns, conf_dns_count, dns, MAX_DNS_COUNT);
+
+	if (dns_count) {
 		rdnssinfo = (struct nd_opt_rdnss_info_local *)pinfo;
 		memset(rdnssinfo, 0, sizeof(*rdnssinfo));
 		rdnssinfo->nd_opt_rdnssi_type = ND_OPT_RDNSS_INFORMATION;
-		rdnssinfo->nd_opt_rdnssi_len = 1 + 2 * conf_dns_count;
+		rdnssinfo->nd_opt_rdnssi_len = 1 + 2 * dns_count;
 		rdnssinfo->nd_opt_rdnssi_lifetime = htonl(conf_rdnss_lifetime);
 		rdnss_addr = (struct in6_addr *)rdnssinfo->nd_opt_rdnssi;
-		for (i = 0; i < conf_dns_count; i++) {
-			memcpy(rdnss_addr, &conf_dns[i], sizeof(*rdnss_addr));
+		for (i = 0; i < dns_count; i++) {
+			memcpy(rdnss_addr, &dns[i], sizeof(*rdnss_addr));
 			rdnss_addr++;
 		}
 	} else
diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c
index cb53f59a..fa77a916 100644
--- a/accel-pppd/radius/radius.c
+++ b/accel-pppd/radius/radius.c
@@ -488,12 +488,31 @@ err:
 	return -1;
 }
 
+/*
+ * Number of IPv6 DNS servers kept per session. Matches the number of dns=
+ * options the ipv6_nd and ipv6_dhcp modules accept in [ipv6-dns], and keeps
+ * the RDNSS option of a router advertisement to a sane size.
+ */
+#define MAX_DNS6_COUNT 3
+
+static void free_ipv6_dns(struct radius_pd_t *rpd)
+{
+	struct ipv6db_addr_t *a;
+
+	while (!list_empty(&rpd->ipv6_dns.addr_list)) {
+		a = list_entry(rpd->ipv6_dns.addr_list.next, typeof(*a), entry);
+		list_del(&a->entry);
+		_free(a);
+	}
+}
+
 int rad_proc_attrs(struct rad_req_t *req)
 {
 	struct ev_wins_t wins = {};
 	struct ev_dns_t dns = {};
 	struct rad_attr_t *attr;
 	struct ipv6db_addr_t *a;
+	int dns6_count = -1;
 	int res = 0;
 	struct radius_pd_t *rpd = req->rpd;
 
@@ -602,6 +621,28 @@ int rad_proc_attrs(struct rad_req_t *req)
 				a->addr = attr->val.ipv6prefix.prefix;
 				list_add_tail(&a->entry, &rpd->ipv6_dp.prefix_list);
 				break;
+			case DNS_Server_IPv6_Address:
+				if (dns6_count < 0) {
+					/* This reply carries a DNS server list of
+					   its own, it replaces whatever a previous
+					   one assigned */
+					free_ipv6_dns(rpd);
+					dns6_count = 0;
+				}
+				if (dns6_count >= MAX_DNS6_COUNT) {
+					if (dns6_count == MAX_DNS6_COUNT)
+						log_ppp_warn("radius: ignoring DNS-Server-IPv6-Address"
+							     " beyond the first %i\n", MAX_DNS6_COUNT);
+					dns6_count++;
+					break;
+				}
+				a = _malloc(sizeof(*a));
+				memset(a, 0, sizeof(*a));
+				a->prefix_len = 128;
+				a->addr = attr->val.ipv6addr;
+				list_add_tail(&a->entry, &rpd->ipv6_dns.addr_list);
+				dns6_count++;
+				break;
 			case NAS_Port:
 				rpd->ses->unit_idx = attr->val.integer;
 				break;
@@ -635,6 +676,11 @@ int rad_proc_attrs(struct rad_req_t *req)
 	if (!rpd->ses->ipv6_dp && !list_empty(&rpd->ipv6_dp.prefix_list))
 		rpd->ses->ipv6_dp = &rpd->ipv6_dp;
 
+	/* Like the IPv4 DNS servers, absent attributes leave whatever a
+	   previous reply assigned in place */
+	if (!list_empty(&rpd->ipv6_dns.addr_list))
+		rpd->ses->ipv6_dns = &rpd->ipv6_dns;
+
 	return res;
 }
 
@@ -799,6 +845,7 @@ static void ses_starting(struct ap_session *ses)
 	INIT_LIST_HEAD(&rpd->plugin_list);
 	INIT_LIST_HEAD(&rpd->ipv6_addr.addr_list);
 	INIT_LIST_HEAD(&rpd->ipv6_dp.prefix_list);
+	INIT_LIST_HEAD(&rpd->ipv6_dns.addr_list);
 
 	rpd->ipv4_addr.owner = &ipdb;
 	rpd->ipv6_addr.owner = &ipdb;
@@ -981,6 +1028,9 @@ static void ses_finished(struct ap_session *ses)
 		_free(a);
 	}
 
+	ses->ipv6_dns = NULL;
+	free_ipv6_dns(rpd);
+
 	fr6 = rpd->fr6;
 	while (fr6) {
 		struct framed_ip6_route *next = fr6->next;
diff --git a/accel-pppd/radius/radius_p.h b/accel-pppd/radius/radius_p.h
index d3a72204..e4b84740 100644
--- a/accel-pppd/radius/radius_p.h
+++ b/accel-pppd/radius/radius_p.h
@@ -65,6 +65,7 @@ struct radius_pd_t {
 	struct ipv4db_item_t ipv4_addr;
 	struct ipv6db_item_t ipv6_addr;
 	struct ipv6db_prefix_t ipv6_dp;
+	struct ipv6db_item_t ipv6_dns;
 	int acct_interim_interval;
 	int acct_interim_jitter;