Index | Thread | Search

From:
Mark Johnston <markj@freebsd.org>
Subject:
fix a gap in relayd config validation
To:
tech@openbsd.org
Date:
Fri, 5 Jul 2024 11:02:03 -0400

Download raw body.

Thread
relayd has a -n option which lets one validate a relayd.conf(5)
configuration prior to starting the daemon.  There is a case where
validation succeeds but relayd fails to start anyway: when a redirect
rule name is longer than pf's limit.  This is fixed by the patch below.

More specifically, relayd uses a buffer of size SRV_NAME_SIZE (64) to
store the rule name, whereas pf's limit is PF_TABLE_NAME_SIZE (32).
relayd's parser checks for truncation of the name, but because of the
size mismatch, it doesn't catch the case where the name fits in relayd's
buffer but is too long for pf.  The patch simply uses pf's constants to
size the name and tag buffers, so that a rule like

	redirect "thisnameisabitlongerthanthepflimit" {
	  listen on 127.0.0.1 tcp port 80
	  forward to <localhost> port 80 check http "/" code 200
	}

triggers an error from the parser before init_tables() tries to copy it
and fails.  To make life a bit easier, the offending name is now printed
by the parser as well.

I tested the patch on an OpenBSD 7.5 system and against the regression
suite.

diff --git a/usr.sbin/relayd/parse.y b/usr.sbin/relayd/parse.y
index 739ff164e2e..a559fec40e7 100644
--- a/usr.sbin/relayd/parse.y
+++ b/usr.sbin/relayd/parse.y
@@ -490,7 +490,7 @@ rdr		: REDIRECT STRING	{
 			if (strlcpy(srv->conf.name, $2,
 			    sizeof(srv->conf.name)) >=
 			    sizeof(srv->conf.name)) {
-				yyerror("redirection name truncated");
+				yyerror("redirection name truncated: %s", $2);
 				free($2);
 				free(srv);
 				YYERROR;
@@ -628,7 +628,8 @@ rdroptsl	: forwardmode TO tablespec interface	{
 			if (strlcpy(rdr->conf.tag, $3,
 			    sizeof(rdr->conf.tag)) >=
 			    sizeof(rdr->conf.tag)) {
-				yyerror("redirection tag name truncated");
+				yyerror("redirection tag name truncated: %s",
+				    $3);
 				free($3);
 				YYERROR;
 			}
diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h
index 2f55c2bb23b..9195be57ab0 100644
--- a/usr.sbin/relayd/relayd.h
+++ b/usr.sbin/relayd/relayd.h
@@ -57,9 +57,7 @@
 #define LABEL_NAME_SIZE		1024
 #define TAG_NAME_SIZE		64
 #define TABLE_NAME_SIZE		64
-#define	RD_TAG_NAME_SIZE	64
 #define	RT_LABEL_SIZE		32
-#define SRV_NAME_SIZE		64
 #define MAX_NAME_SIZE		64
 #define SRV_MAX_VIRTS		16
 #define TLS_NAME_SIZE		512
@@ -545,8 +543,8 @@ struct rdr_config {
 	objid_t			 backup_id;
 	int			 mode;
 	union hashkey		 key;
-	char			 name[SRV_NAME_SIZE];
-	char			 tag[RD_TAG_NAME_SIZE];
+	char			 name[PF_TABLE_NAME_SIZE];
+	char			 tag[PF_TAG_NAME_SIZE];
 	struct timeval		 timeout;
 };