Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: unwind: support wildcard in blacklist
To:
Solene Rapenne <solene@perso.pw>
Cc:
OpenBSD tech <tech@openbsd.org>, florian@openbsd.org
Date:
Tue, 25 Jun 2024 00:45:30 +0100

Download raw body.

Thread
On Mon, 24 Jun 2024 23:30:49 +0100,
Solene Rapenne <solene@perso.pw> wrote:
> 
> I do not have opinion about the diff itself, but I appreciate the feature
> and see a potential use if it could also be toggled to work as an allow list
> where you explicitely list which domains could be resolved, denying every
> other domains not matching the allow list.

Well... this is an interesting setup. I wonder why you need it? I can only
see one use case where the real ethernet / wifi interface on routing domain
1 is used to set up a VPN connection to a provider, and unwind is needed to
resolve the provider's domain, which can be a bit dynamic.

Anyway, here is an updated diff which also includes the allow list with
wildcard. I had brifley test it and it seems to work.

I also updated a man page a bit, but future rewording probably required.

diff --git sbin/unwind/frontend.c sbin/unwind/frontend.c
index ccbc977eb73..e6804069ecd 100644
--- sbin/unwind/frontend.c
+++ sbin/unwind/frontend.c
@@ -115,9 +115,11 @@ struct pending_query {
 
 TAILQ_HEAD(, pending_query)	 pending_queries;
 
-struct bl_node {
-	RB_ENTRY(bl_node)	 entry;
+struct l_node {
+	RB_ENTRY(l_node)	 entry;
 	char			*domain;
+	int			 len;
+	int			 wildcard;
 };
 
 __dead void		 frontend_shutdown(void);
@@ -148,8 +150,9 @@ struct pending_query	*find_pending_query(uint64_t);
 void			 parse_trust_anchor(struct trust_anchor_head *, int);
 void			 send_trust_anchors(struct trust_anchor_head *);
 void			 write_trust_anchors(struct trust_anchor_head *, int);
-void			 parse_blocklist(int);
-int			 bl_cmp(struct bl_node *, struct bl_node *);
+void			 parse_list(int, int);
+int			 l_cmp(struct l_node *, struct l_node *);
+void			 free_al(void);
 void			 free_bl(void);
 int			 pending_query_cnt(void);
 void			 check_available_af(void);
@@ -164,13 +167,29 @@ int			 ta_fd = -1;
 
 static struct trust_anchor_head	 trust_anchors, new_trust_anchors;
 
-RB_HEAD(bl_tree, bl_node)	 bl_head = RB_INITIALIZER(&bl_head);
-RB_PROTOTYPE(bl_tree, bl_node, entry, bl_cmp)
-RB_GENERATE(bl_tree, bl_node, entry, bl_cmp)
+RB_HEAD(al_tree, l_node)	 al_head = RB_INITIALIZER(&al_head);
+RB_PROTOTYPE(al_tree, l_node, entry, l_cmp)
+RB_GENERATE(al_tree, l_node, entry, l_cmp)
+
+RB_HEAD(bl_tree, l_node)	 bl_head = RB_INITIALIZER(&bl_head);
+RB_PROTOTYPE(bl_tree, l_node, entry, l_cmp)
+RB_GENERATE(bl_tree, l_node, entry, l_cmp)
 
 struct dns64_prefix	*dns64_prefixes;
 int			 dns64_prefix_count;
 
+static void
+reverse(char* begin, char* end)
+{
+	char t;
+	while (begin < --end) {
+		t = *begin;
+		*begin = *end;
+		*end = t;
+		++begin;
+	}
+}
+
 void
 frontend_sig_handler(int sig, short event, void *bula)
 {
@@ -362,6 +381,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
 			event_add(&iev_resolver->ev, NULL);
 			break;
 		case IMSG_RECONF_CONF:
+		case IMSG_RECONF_ALLOWLIST_FILE:
 		case IMSG_RECONF_BLOCKLIST_FILE:
 		case IMSG_RECONF_FORWARDER:
 		case IMSG_RECONF_DOT_FORWARDER:
@@ -375,6 +395,8 @@ frontend_dispatch_main(int fd, short event, void *bula)
 			merge_config(frontend_conf, nconf);
 			if (frontend_conf->blocklist_file == NULL)
 				free_bl();
+			if (frontend_conf->allowlist_file == NULL)
+				free_al();
 			nconf = NULL;
 			break;
 		case IMSG_UDP6SOCK:
@@ -454,11 +476,17 @@ frontend_dispatch_main(int fd, short event, void *bula)
 			if (!TAILQ_EMPTY(&trust_anchors))
 				send_trust_anchors(&trust_anchors);
 			break;
+		case IMSG_ALFD:
+			if ((fd = imsg_get_fd(&imsg)) == -1)
+				fatalx("%s: expected to receive imsg block "
+				   "list fd but didn't receive any", __func__);
+			parse_list(fd, 0);
+			break;
 		case IMSG_BLFD:
 			if ((fd = imsg_get_fd(&imsg)) == -1)
 				fatalx("%s: expected to receive imsg block "
 				   "list fd but didn't receive any", __func__);
-			parse_blocklist(fd);
+			parse_list(fd, 1);
 			break;
 		default:
 			log_debug("%s: error handling imsg %d", __func__,
@@ -732,7 +760,7 @@ void
 handle_query(struct pending_query *pq)
 {
 	struct query_imsg	 query_imsg;
-	struct bl_node		 find;
+	struct l_node		 find;
 	int			 rcode;
 	char			*str;
 	char			 dname[LDNS_MAX_DOMAINLEN + 1];
@@ -790,13 +818,24 @@ handle_query(struct pending_query *pq)
 	log_debug("%s: %s %s %s ?", ip_port((struct sockaddr *)&pq->from),
 	    dname, qclass_buf, qtype_buf);
 
+	find.len = strlen(dname);
+	find.wildcard = 0;
+	reverse(dname, dname + find.len);
 	find.domain = dname;
+	if (RB_ROOT(&al_head) != NULL &&
+	    RB_FIND(al_tree, &al_head, &find) == NULL) {
+		if (frontend_conf->allowlist_log)
+			log_info("not allowing %s", dname);
+		error_answer(pq, LDNS_RCODE_REFUSED);
+		goto send_answer;
+	}
 	if (RB_FIND(bl_tree, &bl_head, &find) != NULL) {
 		if (frontend_conf->blocklist_log)
 			log_info("blocking %s", dname);
 		error_answer(pq, LDNS_RCODE_REFUSED);
 		goto send_answer;
 	}
+	reverse(dname, dname + find.len);
 
 	if (pq->qinfo.qtype == LDNS_RR_TYPE_AXFR || pq->qinfo.qtype ==
 	    LDNS_RR_TYPE_IXFR) {
@@ -1520,39 +1559,62 @@ out:
 }
 
 void
-parse_blocklist(int fd)
+parse_list(int fd, int block)
 {
 	FILE		 *f;
-	struct bl_node	*bl_node;
+	struct l_node	 *l_node;
 	char		 *line = NULL;
 	size_t		  linesize = 0;
 	ssize_t		  linelen;
 
 	if((f = fdopen(fd, "r")) == NULL) {
-		log_warn("cannot read block list");
+		if (block)
+			log_warn("cannot read block list");
+		else
+			log_warn("cannot read allow list");
 		close(fd);
 		return;
 	}
 
-	free_bl();
+	if (block)
+		free_bl();
+	else
+		free_al();
 
 	while ((linelen = getline(&line, &linesize, f)) != -1) {
 		if (line[linelen - 1] == '\n') {
 			if (linelen >= 2 && line[linelen - 2] != '.')
 				line[linelen - 1] = '.';
 			else
-				line[linelen - 1] = '\0';
+				line[linelen-- - 1] = '\0';
 		}
 
-		bl_node = malloc(sizeof *bl_node);
-		if (bl_node == NULL)
+		l_node = malloc(sizeof *l_node);
+		if (l_node == NULL)
 			fatal("%s: malloc", __func__);
-		if ((bl_node->domain = strdup(line)) == NULL)
+		if ((l_node->domain = strdup(line)) == NULL)
 			fatal("%s: strdup", __func__);
-		if (RB_INSERT(bl_tree, &bl_head, bl_node) != NULL) {
-			log_warnx("duplicate blocked domain \"%s\"", line);
-			free(bl_node->domain);
-			free(bl_node);
+		reverse(l_node->domain, l_node->domain + linelen);
+		if (linelen > 2 && line[0] == '*' && line[1] == '.') {
+			l_node->wildcard = 1;
+			l_node->domain[linelen - 1] = '\0';
+			l_node->len = linelen - 1;
+		} else {
+			l_node->wildcard = 0;
+			l_node->len = linelen;
+		}
+		if (block) {
+			if (RB_INSERT(bl_tree, &bl_head, l_node) != NULL) {
+				log_warnx("duplicate blocked domain \"%s\"", line);
+				free(l_node->domain);
+				free(l_node);
+			}
+		} else {
+			if (RB_INSERT(al_tree, &al_head, l_node) != NULL) {
+				log_warnx("duplicate allowed domain \"%s\"", line);
+				free(l_node->domain);
+				free(l_node);
+			}
 		}
 	}
 	free(line);
@@ -1562,14 +1624,31 @@ parse_blocklist(int fd)
 }
 
 int
-bl_cmp(struct bl_node *e1, struct bl_node *e2) {
-	return (strcasecmp(e1->domain, e2->domain));
+l_cmp(struct l_node *e1, struct l_node *e2) {
+	if (e1->wildcard == e2->wildcard)
+		return (strcasecmp(e1->domain, e2->domain));
+	else if (e1->wildcard)
+		return (strncasecmp(e1->domain, e2->domain, e1->len));
+	else /* e2->wildcard */
+		return (strncasecmp(e1->domain, e2->domain, e2->len));
+}
+
+void
+free_al(void)
+{
+	struct l_node	*n, *nxt;
+
+	RB_FOREACH_SAFE(n, al_tree, &al_head, nxt) {
+		RB_REMOVE(al_tree, &al_head, n);
+		free(n->domain);
+		free(n);
+	}
 }
 
 void
 free_bl(void)
 {
-	struct bl_node	*n, *nxt;
+	struct l_node	*n, *nxt;
 
 	RB_FOREACH_SAFE(n, bl_tree, &bl_head, nxt) {
 		RB_REMOVE(bl_tree, &bl_head, n);
diff --git sbin/unwind/parse.y sbin/unwind/parse.y
index 79231e3d5be..d2b3be8d0c7 100644
--- sbin/unwind/parse.y
+++ sbin/unwind/parse.y
@@ -102,7 +102,7 @@ typedef struct {
 %token	INCLUDE ERROR
 %token	FORWARDER DOT PORT ODOT_FORWARDER ODOT_AUTOCONF ODOT_DHCP
 %token	AUTHENTICATION NAME PREFERENCE RECURSOR AUTOCONF DHCP STUB
-%token	BLOCK LIST LOG FORCE ACCEPT BOGUS
+%token	ALLOW BLOCK LIST LOG FORCE ACCEPT BOGUS
 
 %token	<v.string>	STRING
 %token	<v.number>	NUMBER
@@ -118,6 +118,7 @@ grammar		: /* empty */
 		| grammar varset '\n'
 		| grammar uw_pref '\n'
 		| grammar uw_forwarder '\n'
+		| grammar allow_list '\n'
 		| grammar block_list '\n'
 		| grammar force '\n'
 		| grammar error '\n'		{ file->errors++; }
@@ -176,6 +177,22 @@ optnl		: '\n' optnl		/* zero or more newlines */
 		| /*empty*/
 		;
 
+allow_list		: ALLOW LIST STRING log {
+				if (conf->allowlist_file != NULL) {
+					yyerror("allow list already "
+					    "configured");
+					free($3);
+					YYERROR;
+				} else {
+					conf->allowlist_file = strdup($3);
+					if (conf->allowlist_file == NULL)
+						err(1, "strdup");
+					free($3);
+					conf->allowlist_log = $4;
+				}
+			}
+			;
+
 block_list		: BLOCK LIST STRING log {
 				if (conf->blocklist_file != NULL) {
 					yyerror("block list already "
@@ -417,6 +434,7 @@ lookup(char *s)
 	static const struct keywords keywords[] = {
 		{"DoT",			DOT},
 		{"accept",		ACCEPT},
+		{"allow",		ALLOW},
 		{"authentication",	AUTHENTICATION},
 		{"autoconf",		AUTOCONF},
 		{"block",		BLOCK},
diff --git sbin/unwind/unwind.c sbin/unwind/unwind.c
index 1b70b6e3e6c..82f5be2948f 100644
--- sbin/unwind/unwind.c
+++ sbin/unwind/unwind.c
@@ -73,6 +73,7 @@ int		main_reload(void);
 int		main_sendall(enum imsg_type, void *, uint16_t);
 void		open_ports(void);
 void		solicit_dns_proposals(void);
+void		send_allowlist_fd(void);
 void		send_blocklist_fd(void);
 
 struct uw_conf		*main_conf;
@@ -290,6 +291,9 @@ main(int argc, char *argv[])
 	main_imsg_compose_frontend_fd(IMSG_ROUTESOCK, 0, frontend_routesock);
 	main_imsg_send_config(main_conf);
 
+	if (main_conf->allowlist_file != NULL)
+		send_allowlist_fd();
+
 	if (main_conf->blocklist_file != NULL)
 		send_blocklist_fd();
 
@@ -577,6 +581,9 @@ main_reload(void)
 
 	merge_config(main_conf, xconf);
 
+	if (main_conf->allowlist_file != NULL)
+		send_allowlist_fd();
+
 	if (main_conf->blocklist_file != NULL)
 		send_blocklist_fd();
 
@@ -593,6 +600,13 @@ main_imsg_send_config(struct uw_conf *xconf)
 	if (main_sendall(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
 		return (-1);
 
+	if (xconf->allowlist_file != NULL) {
+		if (main_sendall(IMSG_RECONF_ALLOWLIST_FILE,
+		    xconf->allowlist_file, strlen(xconf->allowlist_file) + 1)
+		    == -1)
+			return (-1);
+	}
+
 	if (xconf->blocklist_file != NULL) {
 		if (main_sendall(IMSG_RECONF_BLOCKLIST_FILE,
 		    xconf->blocklist_file, strlen(xconf->blocklist_file) + 1)
@@ -667,6 +681,10 @@ merge_config(struct uw_conf *conf, struct uw_conf *xconf)
 	memcpy(&conf->res_pref, &xconf->res_pref,
 	    sizeof(conf->res_pref));
 
+	free(conf->allowlist_file);
+	conf->allowlist_file = xconf->allowlist_file;
+	conf->allowlist_log = xconf->allowlist_log;
+
 	free(conf->blocklist_file);
 	conf->blocklist_file = xconf->blocklist_file;
 	conf->blocklist_log = xconf->blocklist_log;
@@ -860,6 +878,17 @@ solicit_dns_proposals(void)
 		log_warn("failed to send solicitation");
 }
 
+void
+send_allowlist_fd(void)
+{
+	int	bl_fd;
+
+	if ((bl_fd = open(main_conf->allowlist_file, O_RDONLY)) != -1)
+		main_imsg_compose_frontend_fd(IMSG_ALFD, 0, bl_fd);
+	else
+		log_warn("%s", main_conf->allowlist_file);
+}
+
 void
 send_blocklist_fd(void)
 {
@@ -896,6 +925,13 @@ imsg_receive_config(struct imsg *imsg, struct uw_conf **xconf)
 		TAILQ_INIT(&nconf->uw_dot_forwarder_list);
 		RB_INIT(&nconf->force);
 		break;
+	case IMSG_RECONF_ALLOWLIST_FILE:
+		if (((char *)imsg->data)[IMSG_DATA_SIZE(*imsg) - 1] != '\0')
+			fatalx("Invalid allowlist file");
+		if ((nconf->allowlist_file = strdup(imsg->data)) ==
+		    NULL)
+			fatal("%s: strdup", __func__);
+		break;
 	case IMSG_RECONF_BLOCKLIST_FILE:
 		if (((char *)imsg->data)[IMSG_DATA_SIZE(*imsg) - 1] != '\0')
 			fatalx("Invalid blocklist file");
diff --git sbin/unwind/unwind.conf.5 sbin/unwind/unwind.conf.5
index dd64e3e4dd2..6c1491740f7 100644
--- sbin/unwind/unwind.conf.5
+++ sbin/unwind/unwind.conf.5
@@ -63,6 +63,15 @@ forwarder { $fwd1 $fwd2 }
 .Ed
 .Sh GLOBAL CONFIGURATION
 .Bl -tag -width Ds
+.It Ic allow list Ar file Op Cm log
+A file containing the only domains which is allowed, one per line.
+If a domain not from this list is queried,
+.Nm unwind
+answers with a return code of
+.Dv REFUSED .
+With
+.Cm log
+not allowed queries are logged.
 .It Ic block list Ar file Op Cm log
 A file containing domains to block, one per line.
 If a domain from this list is queried,
diff --git sbin/unwind/unwind.h sbin/unwind/unwind.h
index f21baf72970..1fabfec80a7 100644
--- sbin/unwind/unwind.h
+++ sbin/unwind/unwind.h
@@ -90,6 +90,7 @@ enum imsg_type {
 	IMSG_CTL_AUTOCONF,
 	IMSG_CTL_MEM,
 	IMSG_RECONF_CONF,
+	IMSG_RECONF_ALLOWLIST_FILE,
 	IMSG_RECONF_BLOCKLIST_FILE,
 	IMSG_RECONF_FORWARDER,
 	IMSG_RECONF_DOT_FORWARDER,
@@ -117,6 +118,7 @@ enum imsg_type {
 	IMSG_NEW_TAS_ABORT,
 	IMSG_NEW_TAS_DONE,
 	IMSG_NETWORK_CHANGED,
+	IMSG_ALFD,
 	IMSG_BLFD,
 	IMSG_REPLACE_DNS,
 	IMSG_NEW_DNS64_PREFIXES_START,
@@ -155,6 +157,8 @@ struct uw_conf {
 	struct force_tree		 force;
 	struct resolver_preference	 res_pref;
 	int				 enabled_resolvers[UW_RES_NONE];
+	char				*allowlist_file;
+	int				 allowlist_log;
 	char				*blocklist_file;
 	int				 blocklist_log;
 };


-- 
wbr, Kirill