From: Rafael Sadowski <rafael@sizeofvoid.org>
Subject: relayd 3/3: fix duplicate lookup types
To: tech@openbsd.org
Date: Fri, 9 Oct 2026 14:19:39 +0200

This add a duplicate lookup check to avoid config errors.

Configs with duplicate lookup types in a rule, no longer parse.

OK?

commit 8839ff17380f94f7d829c79f5d8d4d98909ebe39
Author: Rafael Sadowski <rafael@sizeofvoid.org>
Date:   Fri Oct 9 13:47:22 2026 +0200

    relayd: fix duplicate lookup types
    
    A second "cookie", "header", "path", "query" or "url" clause in the same
    rule used to replace the first one and.  Make it an error.
    
    Configs with duplicate lookup types in a rule, or with a matching
    keyword together with set or append, no longer parse.

diff --git a/parse.y b/parse.y
index 765ee90..757be17 100644
--- a/parse.y
+++ b/parse.y
@@ -145,6 +145,7 @@ struct relay	*relay_inherit(struct relay *, struct relay *);
 int		 getservice(char *);
 int		 is_if_in_group(const char *, const char *);
 static struct keyname *proto_keyname(char *);
+static int	 rule_kv_dup(struct relay_rule *, enum key_type , const char *);
 static int	 rule_kv_sanity_check(enum key_option);
 static void 	 rule_kv_setvalue(struct kv *, const char *, u_int16_t);
 
@@ -1604,7 +1605,9 @@ ruleopts	: METHOD STRING					{
 			free($2);
 		}
 		| COOKIE key_option optpattern STRING value	{
-			if (rule_kv_sanity_check($2) == -1) {
+			if (rule_kv_sanity_check($2) == -1 ||
+			    (rule_kv_dup(rule, KEY_TYPE_COOKIE,
+			    "cookie") != 0)) {
 				free($4);
 				free($5);
 				YYERROR;
@@ -1628,12 +1631,16 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| COOKIE key_option				{
+			if (rule_kv_dup(rule, KEY_TYPE_COOKIE, "cookie") != 0)
+				YYERROR;
 			keytype = KEY_TYPE_COOKIE;
 			rule->rule_kv[keytype].kv_option = $2;
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| HEADER key_option optpattern STRING value		{
-			if (rule_kv_sanity_check($2) == -1) {
+			if (rule_kv_sanity_check($2) == -1 ||
+			    (rule_kv_dup(rule, KEY_TYPE_HEADER,
+			    "header") != 0)) {
 				free($4);
 				free($5);
 				YYERROR;
@@ -1665,12 +1672,15 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| HEADER key_option				{
+			if (rule_kv_dup(rule, KEY_TYPE_HEADER, "header") != 0)
+				YYERROR;
 			keytype = KEY_TYPE_HEADER;
 			rule->rule_kv[keytype].kv_option = $2;
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| PATH key_option optpattern STRING value	{
-			if (rule_kv_sanity_check($2) == -1) {
+			if (rule_kv_sanity_check($2) == -1 ||
+			    (rule_kv_dup(rule, KEY_TYPE_PATH, "path") != 0)) {
 				free($4);
 				free($5);
 				YYERROR;
@@ -1695,6 +1705,8 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| PATH key_option				{
+			if (rule_kv_dup(rule, KEY_TYPE_PATH, "path") != 0)
+				YYERROR;
 			keytype = KEY_TYPE_PATH;
 			rule->rule_kv[keytype].kv_option = $2;
 			rule->rule_kv[keytype].kv_type = keytype;
@@ -1702,6 +1714,9 @@ ruleopts	: METHOD STRING					{
 		| PATH STRIP NUMBER				{
 			char	*strip = NULL;
 
+			if (rule_kv_dup(rule, KEY_TYPE_PATH, "path") != 0)
+				YYERROR;
+
 			if ($3 < 0 || $3 > INT_MAX) {
 				yyerror("invalid strip number");
 				YYERROR;
@@ -1714,7 +1729,8 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| QUERYSTR key_option optpattern STRING value	{
-			if (rule_kv_sanity_check($2) == -1) {
+			if (rule_kv_sanity_check($2) == -1 ||
+			    (rule_kv_dup(rule, KEY_TYPE_QUERY, "query") != 0)) {
 				free($4);
 				free($5);
 				YYERROR;
@@ -1751,6 +1767,8 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| QUERYSTR key_option				{
+			if (rule_kv_dup(rule, KEY_TYPE_QUERY, "query") != 0)
+				YYERROR;
 			switch ($2) {
 			case KEY_OPTION_APPEND:
 			case KEY_OPTION_SET:
@@ -1765,7 +1783,8 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| URL key_option optpattern optdigest value	{
-			if (rule_kv_sanity_check($2) == -1) {
+			if (rule_kv_sanity_check($2) == -1 ||
+			    (rule_kv_dup(rule, KEY_TYPE_URL, "url") != 0)) {
 				free($4.digest);
 				free($5);
 				YYERROR;
@@ -1800,6 +1819,8 @@ ruleopts	: METHOD STRING					{
 			rule->rule_kv[keytype].kv_type = keytype;
 		}
 		| URL key_option					{
+			if (rule_kv_dup(rule, KEY_TYPE_URL, "url") != 0)
+				YYERROR;
 			switch ($2) {
 			case KEY_OPTION_APPEND:
 			case KEY_OPTION_SET:
@@ -3860,6 +3881,18 @@ rule_kv_sanity_check(enum key_option opt)
 	return (0);
 }
 
+static int
+rule_kv_dup(struct relay_rule *r, enum key_type type, const char *name)
+{
+	if (r == NULL)
+		return (0);
+	if (r->rule_kv[type].kv_type == KEY_TYPE_NONE)
+		return (0);
+	yyerror("duplicate %s option in rule, "
+	    "use tag/tagged to combine conditions", name);
+	return (-1);
+}
+
 /*
  * Set the value of a rule kv. An omitted value matches anything,
  * otherwise use the matching keyword or the per-type default.