rdomain IDs do not need to exist for "on rdomain N" to work

Unlike "... rtable N", pf.conf(5)'s "on rdomain N" does not alter packet
state and will always work no matter if rdomain N currently exists or not,
i.e. the rule "pass on rdomain 42" will simply match (and pass) packets if
rdomain 42 exists, and it will simply not match (neither pass nor block)
packets if 42 does not exist.

There's no need to reload the ruleset whenever routing domains are created
or deleted, which can already be observed now by creating an rdomain,
loading rules referencing it and deleting the same rdomain immediately
afterwards: pf will continue to work as expected.

Relax both pfctl(8)'s parser check as well as pf(4)'s copyin routine to
accept any valid routing domain ID without expecting it to exist at the time
of ruleset creation - this lifts the requirement to create rdomains before
referencing them in pf.conf while keeping pf behaviour unchanged.

Prompted by yasuoka's recent pfctl parse.y r1.702 commit requiring an rtable
to exist upon ruleset creation.

Discussed with claudio and bluhm at k2k20.

Feedback sashan
OK sashan yasouka claudio

2 files changed, 6 insertions, 14 deletions

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 60ef81488c3..f06171158cb 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.703 2020/09/17 14:26:59 yasuoka Exp $	*/
+/*	$OpenBSD: parse.y,v 1.704 2020/10/01 14:02:08 kn Exp $	*/
 
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -1216,7 +1216,7 @@ antispoof_opt	: LABEL label	{
 			if ($2 < 0 || $2 > RT_TABLEID_MAX) {
 				yyerror("invalid rtable id");
 				YYERROR;
-			} else if (lookup_rtable($2) < 1) {
+			} else if (!lookup_rtable($2)) {
 				yyerror("rtable %lld does not exist", $2);
 				YYERROR;
 			}
@@ -2003,7 +2003,7 @@ filter_opt	: USER uids {
 			if ($2 < 0 || $2 > RT_TABLEID_MAX) {
 				yyerror("invalid rtable id");
 				YYERROR;
-			} else if (lookup_rtable($2) < 1) {
+			} else if (!lookup_rtable($2)) {
 				yyerror("rtable %lld does not exist", $2);
 				YYERROR;
 			}
@@ -2481,8 +2481,6 @@ if_item		: STRING			{
 		| RDOMAIN NUMBER		{
 			if ($2 < 0 || $2 > RT_TABLEID_MAX)
 				yyerror("rdomain %lld outside range", $2);
-			else if (lookup_rtable($2) != 2)
-				yyerror("rdomain %lld does not exist", $2);
 
 			$$ = calloc(1, sizeof(struct node_if));
 			if ($$ == NULL)
@@ -5900,10 +5898,6 @@ lookup_rtable(u_int rtableid)
 		}
 		err(1, "%s", __func__);
 	}
-	if (info.rti_domainid == rtableid) {
-		found[rtableid] = 2;
-		return 2;
-	}
 	found[rtableid] = 1;
 	return 1;
 }
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 95bf721bc24..4f26890e9e0 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.356 2020/08/24 15:41:15 kn Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.357 2020/10/01 14:02:08 kn Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -2820,10 +2820,8 @@ pf_rule_copyin(struct pf_rule *from, struct pf_rule *to,
 	if (to->rtableid >= 0 && !rtable_exists(to->rtableid))
 		return (EBUSY);
 	to->onrdomain = from->onrdomain;
-	if (to->onrdomain >= 0 && !rtable_exists(to->onrdomain))
-		return (EBUSY);
-	if (to->onrdomain >= 0)		/* make sure it is a real rdomain */
-		to->onrdomain = rtable_l2(to->onrdomain);
+	if (to->onrdomain < 0 || to->onrdomain > RT_TABLEID_MAX)
+		return (EINVAL);
 
 	for (i = 0; i < PFTM_MAX; i++)
 		to->timeout[i] = from->timeout[i];