Merge pull request #362 from nuclearcat/fix/ng40-additional-safeguardsHEADmaster

Additional safeguards

6 files changed, 444 insertions, 0 deletions

diff --git a/tests/unit/dhcpv6_safeguards_test.c b/tests/unit/dhcpv6_safeguards_test.c
new file mode 100644
index 00000000..63692ee3
--- /dev/null
+++ b/tests/unit/dhcpv6_safeguards_test.c
@@ -0,0 +1,154 @@
+/* Standalone tests of the production parser and reply builders. See run_safeguards.sh. */
+#include <assert.h>
+#include "triton.h"
+#undef DEFINE_INIT
+#define DEFINE_INIT(o, f)
+#include "../../accel-pppd/ipv6/dhcpv6_packet.c"
+#undef BUF_SIZE
+#include "../../accel-pppd/ipv6/dhcpv6.c"
+
+__thread struct ap_net *net;
+static unsigned int sent;
+static unsigned char response[4096];
+static size_t response_len;
+void log_warn(const char *fmt, ...) {}
+void log_emerg(const char *fmt, ...) {}
+void log_ppp_error(const char *fmt, ...) {}
+void log_ppp_info2(const char *fmt, ...) {}
+int ip6addr_add_peer(int i, struct in6_addr *a, struct in6_addr *p) { return 0; }
+int ip6route_add(int i, const struct in6_addr *a, int p, const struct in6_addr *g, int m, uint32_t prio, const char *v) { return 0; }
+struct ipv6db_prefix_t *ipdb_get_ipv6_prefix(struct ap_session *s) { return NULL; }
+
+static ssize_t capture_send(int fd, const void *buf, size_t len, int flags,
+		       const struct sockaddr *addr, socklen_t addrlen)
+{
+	assert(len <= sizeof(response));
+	memcpy(response, buf, len);
+	response_len = len;
+	sent++;
+	return len;
+}
+
+static size_t option(unsigned char *p, unsigned int code, const void *data, size_t len)
+{
+	u_write_be16(p, code);
+	u_write_be16(p + 2, len);
+	if (len)
+		memcpy(p + 4, data, len);
+	return len + 4;
+}
+
+static size_t request(unsigned char *buf)
+{
+	static const unsigned char duid[] = {0, 3, 0, 1, 1, 2, 3, 4, 5, 6};
+	size_t len = 4;
+	memset(buf, 0, 4);
+	buf[0] = D6_SOLICIT;
+	len += option(buf + len, D6_OPTION_CLIENTID, duid, sizeof(duid));
+	len += option(buf + len, D6_OPTION_SERVERID, duid, sizeof(duid));
+	return len;
+}
+
+static size_t relay(unsigned char *buf, size_t len)
+{
+	size_t hdrlen = sizeof(struct dhcpv6_relay_hdr);
+	memmove(buf + hdrlen + 4, buf, len);
+	memset(buf, 0, hdrlen);
+	buf[0] = D6_RELAY_FORW;
+	u_write_be16(buf + hdrlen, D6_OPTION_RELAY_MSG);
+	u_write_be16(buf + hdrlen + 2, len);
+	return len + hdrlen + 4;
+}
+
+int main(void)
+{
+	unsigned char buf[16000], oro[4000], name[256];
+	const unsigned char valid_name[] = {2, 'g', 'w', 0};
+	struct dhcpv6_packet *pkt, *reply;
+	struct dhcpv6_option *opt;
+	struct ap_session ses = {0};
+	struct dhcpv6_pd pd = {0};
+	struct ap_net test_net = {.sendto = capture_send};
+	size_t len, base, avail;
+	int i;
+
+	net = &test_net;
+	base = request(buf);
+	len = base + option(buf + base, D6_OPTION_RAPID_COMMIT, NULL, 0);
+	pkt = dhcpv6_packet_parse(buf, len);
+	assert(pkt && pkt->rapid_commit);
+	pkt->ses = &ses;
+	dhcpv6_send_reply(pkt, &pd, D6_REPLY);
+	assert(sent == 1);
+	reply = dhcpv6_packet_parse(response, response_len);
+	assert(reply && reply->rapid_commit);
+	dhcpv6_packet_free(reply);
+	dhcpv6_packet_free(pkt);
+	len += option(buf + len, D6_OPTION_RAPID_COMMIT, NULL, 0);
+	assert(!dhcpv6_packet_parse(buf, len));
+	len = base + option(buf + base, D6_OPTION_RAPID_COMMIT, "x", 1);
+	assert(!dhcpv6_packet_parse(buf, len));
+
+	len = base + option(buf + base, D6_OPTION_AFTR_NAME, valid_name, sizeof(valid_name));
+	pkt = dhcpv6_packet_parse(buf, len);
+	assert(pkt);
+	dhcpv6_packet_free(pkt);
+	len += option(buf + len, D6_OPTION_AFTR_NAME, valid_name, sizeof(valid_name));
+	assert(!dhcpv6_packet_parse(buf, len));
+	memset(name, 'x', sizeof(name));
+	for (i = 0; i < 5; i++) {
+		const unsigned char invalid[][4] = {{0,0,0,0}, {0xc0,1,1,0}, {4,'a','b',0}, {2,'a','b',1}, {1,'a',0,1}};
+		len = base + option(buf + base, D6_OPTION_AFTR_NAME, invalid[i], 4);
+		assert(!dhcpv6_packet_parse(buf, len));
+	}
+	len = base + option(buf + base, D6_OPTION_AFTR_NAME, name, sizeof(name));
+	assert(!dhcpv6_packet_parse(buf, len));
+
+	len = request(buf);
+	for (i = 0; i <= DHCPV6_HOP_COUNT_LIMIT; i++) {
+		pkt = dhcpv6_packet_parse(buf, len);
+		assert(pkt);
+		reply = dhcpv6_packet_alloc_reply(pkt, D6_REPLY);
+		assert(reply);
+		/* The last legal byte is writable; both allocators reject overflow. */
+		avail = (char *)(reply + 1) + 4096 - (char *)reply->endptr;
+		opt = dhcpv6_option_alloc(reply, 65000, avail - 4);
+		assert(opt);
+		memset(opt->hdr->data, 1, avail - 4);
+		assert(!dhcpv6_option_alloc(reply, 65000, 0));
+		assert(!dhcpv6_nested_option_alloc(reply, opt, 65000, 0));
+		assert(!dhcpv6_option_alloc(reply, 65000, -1));
+		dhcpv6_fill_relay_info(reply);
+		assert((char *)reply->endptr == (char *)(reply + 1) + 4096);
+		/* Verify Relay-Message wire length in every enclosing layer. */
+		if (i) {
+			struct dhcpv6_relay_hdr *h = (void *)reply->hdr;
+			struct dhcpv6_opt_hdr *o = (void *)h->data;
+			assert(ntohs(o->len) == (char *)reply->endptr - (char *)o->data);
+		}
+		dhcpv6_packet_free(reply);
+		dhcpv6_packet_free(pkt);
+		len = relay(buf, len);
+	}
+	assert(!dhcpv6_packet_parse(buf, len));
+
+	/* An ORO can amplify a small request into more than 4096 response bytes. */
+	conf_dns_count = 1;
+	for (i = 0; i < sizeof(oro); i += 2)
+		u_write_be16(oro + i, D6_OPTION_DNS_SERVERS);
+	len = request(buf);
+	len += option(buf + len, D6_OPTION_ORO, oro, sizeof(oro));
+	pkt = dhcpv6_packet_parse(buf, len);
+	assert(pkt);
+	pkt->ses = &ses;
+	sent = 0;
+	dhcpv6_send_reply(pkt, &pd, D6_REPLY);
+	assert(!sent);
+	dhcpv6_send_reply2(pkt, &pd, D6_REPLY);
+	assert(!sent);
+	dhcpv6_packet_free(pkt);
+	puts("DHCPv6 safeguards: PASS");
+	return 0;
+}
+void build_ip6_addr(struct ipv6db_addr_t *a, uint64_t id, struct in6_addr *addr) { *addr = a->addr; }
+int ip6addr_add(int i, struct in6_addr *a, int p) { return 0; }
diff --git a/tests/unit/lcp_safeguards_test.c b/tests/unit/lcp_safeguards_test.c
new file mode 100644
index 00000000..32a14dfb
--- /dev/null
+++ b/tests/unit/lcp_safeguards_test.c
@@ -0,0 +1,40 @@
+/* Exercise production Echo-Reply handling at the negotiated MTU. */
+#include <assert.h>
+#include "triton.h"
+#undef DEFINE_INIT
+#define DEFINE_INIT(o, f)
+#include "../../accel-pppd/ppp/ppp_lcp.c"
+
+int conf_ppp_verbose;
+static int sent, sent_len;
+void log_ppp_debug(const char *fmt, ...) {}
+int ppp_chan_send(struct ppp_t *ppp, void *buf, int size)
+{
+	const struct lcp_hdr_t *hdr = buf;
+	assert(size == ntohs(hdr->len) + 2);
+	assert(hdr->code == ECHOREP);
+	sent++;
+	sent_len = size;
+	return 0;
+}
+int main(void)
+{
+	unsigned char buf[1502] = {0};
+	struct ppp_t ppp = {.buf = buf, .buf_size = sizeof(buf), .mtu = 1492};
+	struct ppp_lcp_t lcp = {.ppp = &ppp, .magic = 0x12345678};
+	struct lcp_hdr_t *hdr = (void *)buf;
+
+	hdr->len = htons(1493);
+	hdr->code = ECHOREQ;
+	send_echo_reply(&lcp);
+	assert(!sent && hdr->code == ECHOREQ);
+	hdr->len = htons(1492);
+	send_echo_reply(&lcp);
+	assert(sent == 1 && sent_len == 1494);
+	assert(u_read_be32(hdr + 1) == lcp.magic);
+	hdr->len = htons(8);
+	send_echo_reply(&lcp);
+	assert(sent == 2 && sent_len == 10);
+	puts("LCP safeguards: PASS");
+	return 0;
+}
diff --git a/tests/unit/mempool_safeguards_test.c b/tests/unit/mempool_safeguards_test.c
new file mode 100644
index 00000000..b0c6a3dc
--- /dev/null
+++ b/tests/unit/mempool_safeguards_test.c
@@ -0,0 +1,38 @@
+/* Pool reuse must not expose the previous object's payload. */
+#include <assert.h>
+#include <pthread.h>
+#include "triton.h"
+#undef __init
+#define __init
+#include "../../accel-pppd/triton/mempool.c"
+
+void triton_log_error(const char *fmt, ...) {}
+void triton_stat_mempool_allocated_add(uint64_t v) {}
+void triton_stat_mempool_allocated_sub(uint64_t v) {}
+void triton_stat_mempool_available_add(uint64_t v) {}
+void triton_stat_mempool_available_sub(uint64_t v) {}
+
+int main(void)
+{
+	mempool_t *pool;
+	unsigned char *p, *q;
+	int i;
+
+	spinlock_init(&pools_lock);
+	pool = mempool_create(128);
+	p = mempool_alloc(pool);
+	assert(p);
+	memset(p, 0xa5, 128);
+	mempool_free(p);
+	q = mempool_alloc(pool);
+	assert(q == p);
+	for (i = 0; i < 128; i++)
+		assert(!q[i]);
+	memset(q, 0x5a, 128);
+	mempool_free(q);
+	mempool_clean();
+	list_del(&((struct _mempool_t *)pool)->entry);
+	free(pool);
+	puts("Mempool safeguards: PASS");
+	return 0;
+}
diff --git a/tests/unit/radius_safeguards_test.c b/tests/unit/radius_safeguards_test.c
new file mode 100644
index 00000000..6b925bf4
--- /dev/null
+++ b/tests/unit/radius_safeguards_test.c
@@ -0,0 +1,119 @@
+/* Exercise production response verification and accounting signing with real packets. */
+#include <assert.h>
+#include "triton.h"
+#undef DEFINE_INIT
+#define DEFINE_INIT(o, f)
+#include "../../accel-pppd/radius/req.c"
+#include "../../accel-pppd/radius/packet.c"
+
+static const char *server_secret = "first-secret";
+static int healthy, delivered;
+int conf_verbose;
+char *rad_server_secret_dup(struct rad_server_t *s) { return strdup(server_secret); }
+void log_emerg(const char *fmt, ...) {}
+void log_ppp_error(const char *fmt, ...) {}
+void log_ppp_warn(const char *fmt, ...) {}
+void log_switch(struct triton_context_t *ctx, void *arg) {}
+struct triton_context_t *triton_context_self(void) { return NULL; }
+void rad_server_reply(struct rad_server_t *s) { healthy++; }
+void rad_server_req_exit(struct rad_req_t *r) { r->active = 0; }
+int rad_server_req_cancel(struct rad_req_t *r, int full) { return 0; }
+void *mempool_alloc(mempool_t *p) { return calloc(1, (size_t)p); }
+void mempool_free(void *p) { free(p); }
+struct rad_dict_attr_t *rad_dict_find_attr_id(struct rad_dict_vendor_t *v, int id) { return NULL; }
+struct rad_dict_vendor_t *rad_dict_find_vendor_id(int id) { return NULL; }
+struct rad_dict_value_t *rad_dict_find_val(struct rad_dict_attr_t *a, rad_value_t v) { return NULL; }
+
+static void receive_reply(struct rad_req_t *req) { delivered++; }
+static void sign_response(unsigned char *buf, int code, const unsigned char *ra, const char *secret)
+{
+	unsigned char input[256];
+	size_t n = strlen(secret);
+	memset(buf, 0, 20);
+	buf[0] = code;
+	buf[1] = 1;
+	buf[3] = 20;
+	memcpy(input, buf, 4);
+	memcpy(input + 4, ra, 16);
+	memcpy(input + 20, secret, n);
+	MD5(input, 20 + n, buf + 4);
+}
+
+int main(void)
+{
+	struct rad_packet_t request = {.code = CODE_ACCOUNTING_REQUEST, .id = 1, .len = 20};
+	struct rad_packet_t response_packet = {.len = 20};
+	struct rad_server_t server = {0};
+	struct rad_req_t req = {.pack = &request, .serv = &server, .recv = receive_reply};
+	unsigned char response[20], expected[16], input[256], first_ra[16];
+	int pair[2], code, i;
+
+	packet_pool = (void *)sizeof(struct rad_packet_t);
+	buf_pool = (void *)REQ_LENGTH_MAX;
+	INIT_LIST_HEAD(&request.attrs);
+	assert(!rad_req_set_RA(&req));
+	memcpy(first_ra, req.RA, 16);
+	memcpy(input, request.buf, 20);
+	memset(input + 4, 0, 16);
+	memcpy(input + 20, server_secret, strlen(server_secret));
+	MD5(input, 20 + strlen(server_secret), expected);
+	assert(!memcmp(expected, req.RA, 16));
+	/* Rebuilding must zero the previous digest before hashing. */
+	assert(!rad_req_set_RA(&req));
+	assert(!memcmp(first_ra, req.RA, 16));
+	response_packet.buf = response;
+	for (i = 0; i < 4; i++) {
+		const int codes[] = {CODE_ACCESS_ACCEPT, CODE_ACCESS_REJECT, CODE_ACCESS_CHALLENGE, CODE_ACCOUNTING_RESPONSE};
+		code = codes[i];
+		sign_response(response, code, request.buf + 4, server_secret);
+		assert(!verify_response_authenticator(&req, &response_packet));
+		response[4] ^= 1;
+		assert(verify_response_authenticator(&req, &response_packet));
+		sign_response(response, code, first_ra, "wrong-secret");
+		assert(verify_response_authenticator(&req, &response_packet));
+	}
+	/* An in-flight response still verifies after the configuration changes. */
+	sign_response(response, CODE_ACCOUNTING_RESPONSE, first_ra, server_secret);
+	server_secret = "second-secret";
+	assert(!verify_response_authenticator(&req, &response_packet));
+	/* A new accounting send after failover/reload signs with the new secret. */
+	assert(!rad_req_set_RA(&req));
+	assert(memcmp(first_ra, req.RA, 16));
+	assert(verify_response_authenticator(&req, &response_packet));
+	sign_response(response, CODE_ACCOUNTING_RESPONSE, req.RA, server_secret);
+	assert(!verify_response_authenticator(&req, &response_packet));
+
+	assert(!socketpair(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0, pair));
+	req.hnd.fd = pair[1];
+	response[4] ^= 1;
+	assert(write(pair[0], response, 20) == 20);
+	assert(!rad_req_read(&req.hnd));
+	assert(!healthy && !delivered && !req.reply);
+	response[4] ^= 1;
+	assert(write(pair[0], response, 20) == 20);
+	assert(rad_req_read(&req.hnd) == 1);
+	assert(healthy == 1 && delivered == 1 && req.reply);
+	rad_packet_free(req.reply);
+	/* Outbound Message-Authenticator retransmits must hash a zeroed field. */
+	{
+		unsigned char wire[38] = {CODE_ACCESS_REQUEST, 1, 0, 38};
+		unsigned char first[38], second[38];
+		struct rad_packet_t access = {
+			.buf = wire, .len = sizeof(wire), .message_authenticator = 1,
+			.secret = (uint8_t *)"blast-secret"
+		};
+		wire[20] = 80;
+		wire[21] = 18;
+		assert(!rad_packet_send(&access, pair[0], NULL));
+		assert(read(pair[1], first, sizeof(first)) == sizeof(first));
+		assert(!rad_packet_send(&access, pair[0], NULL));
+		assert(read(pair[1], second, sizeof(second)) == sizeof(second));
+		assert(!memcmp(first, second, sizeof(first)));
+	}
+	close(pair[0]);
+	close(pair[1]);
+	free(request.secret);
+	free(request.buf);
+	puts("RADIUS safeguards: PASS");
+	return 0;
+}
diff --git a/tests/unit/run_safeguards.sh b/tests/unit/run_safeguards.sh
new file mode 100644
index 00000000..1c69efd7
--- /dev/null
+++ b/tests/unit/run_safeguards.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+# Run from any directory, passing a configured CMake build directory.
+set -eu
+root=$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd)
+build=${1:?usage: run_safeguards.sh /path/to/cmake-build}
+out=$(mktemp -d)
+trap 'rm -rf "$out"' EXIT HUP INT TERM
+cd "$root"
+for name in dhcpv6 radius vrf lcp mempool; do
+	extra=
+	# LCP's global layer table retains the whole daemon under ASan; its test
+	# checks wire lengths with UBSan. The packet/VRF tests also use ASan.
+	if [ "$name" = lcp ]; then extra=-fno-sanitize=address; fi
+	${CC:-cc} -O1 -g -Wall -Wno-unused-function -Wno-unused-result -D_GNU_SOURCE -DAP_SESSIONID_LEN=16 \
+		-DOPENSSL_API_COMPAT=0x10100000L -fno-strict-aliasing \
+		-ffunction-sections -fdata-sections -Wl,--gc-sections \
+		-fsanitize=address,undefined -fno-sanitize-recover=all $extra \
+		-I "$build" -I accel-pppd -I accel-pppd/include -I accel-pppd/triton \
+		-o "$out/$name" "tests/unit/${name}_safeguards_test.c" -lcrypto -lpthread
+	"$out/$name"
+done
diff --git a/tests/unit/vrf_safeguards_test.c b/tests/unit/vrf_safeguards_test.c
new file mode 100644
index 00000000..1fa5a414
--- /dev/null
+++ b/tests/unit/vrf_safeguards_test.c
@@ -0,0 +1,72 @@
+/* Exercise the shared interface lookup and length-delimited VRF API. */
+#include <assert.h>
+#include <pthread.h>
+#include "triton.h"
+#undef DEFINE_INIT
+#undef __init
+#define __init
+#define DEFINE_INIT(o, f)
+#include "../../accel-pppd/net.c"
+#include "../../accel-pppd/ifcfg.c"
+#define init coa_init
+#include "../../accel-pppd/radius/dm_coa.c"
+#undef init
+
+static int lookups, changes, last_master;
+static char last_name[IFNAMSIZ];
+void log_ppp_error(const char *fmt, ...) {}
+void log_ppp_info2(const char *fmt, ...) {}
+
+static int lookup(const char *name)
+{
+	lookups++;
+	assert(strlen(name) < IFNAMSIZ);
+	strcpy(last_name, name);
+	return 7;
+}
+static int set_vrf(int index, int master)
+{
+	changes++;
+	last_master = master;
+	return 0;
+}
+
+int main(void)
+{
+	struct kern_net kn = {0};
+	struct ap_net backend = {.get_ifindex = lookup, .set_vrf = set_vrf};
+	struct ap_session ses = {.net = &backend};
+	struct radius_pd_t rpd = {.ses = &ses};
+	char name[IFNAMSIZ + 1];
+
+	net = &kn.net;
+	kn.sock = socket(AF_INET, SOCK_DGRAM, 0);
+	assert(kn.sock >= 0);
+	assert(def_get_ifindex("lo") > 0);
+	assert(def_get_ifindex(NULL) == -1);
+	memset(name, 'x', sizeof(name));
+	name[IFNAMSIZ] = 0;
+	assert(def_get_ifindex(name) == -1);
+	assert(ap_session_vrf(&ses, name, -1) == -1);
+	assert(ap_session_vrf(&ses, name, IFNAMSIZ) == -1);
+	assert(ap_session_vrf(&ses, NULL, 1) == -1);
+	assert(ap_session_vrf(&ses, name, -2) == -1);
+	assert(ap_session_vrf(&ses, "a\0b", 3) == -1);
+	assert(!lookups && !changes);
+	/* Explicit length need not be followed by a NUL in caller memory. */
+	assert(!ap_session_vrf(&ses, name, IFNAMSIZ - 1));
+	assert(strlen(last_name) == IFNAMSIZ - 1 && last_master == 7);
+	assert(!ap_session_vrf(&ses, NULL, 0));
+	assert(last_master == 0 && !ses.vrf_name);
+	assert(!ap_session_vrf(&ses, NULL, -1));
+	assert(lookups == 1 && changes == 3);
+	assert(!rad_update_vrf(&rpd, "blue\0junk", 9));
+	assert(!rad_update_vrf(&rpd, name, IFNAMSIZ));
+	assert(rad_update_vrf(&rpd, "0", 1));
+	assert(last_master == 0);
+	assert(rad_update_vrf(&rpd, "0blue", 5));
+	assert(last_master == 7 && !strcmp(last_name, "0blue"));
+	close(kn.sock);
+	puts("VRF safeguards: PASS");
+	return 0;
+}