Index | Thread | Search

From:
Alexandr Nedvedicky <sashan@fastmail.net>
Subject:
ifa_load() pfctl parser should adhere interface flags
To:
tech@openbsd.org
Date:
Tue, 25 Nov 2025 09:24:53 +0100

Download raw body.

Thread
Hello,

the issue has been pointed by claudio@ off-list.

the ifa_load9() function does not distinct between
broadcast address and peer address when it processes
interface item (`ifa`) obtained by getifaddrs(3) from
kernel.

as I understand it the IFF_BROADCAST and IFF_POINTOPOINT
flags are mutually exclusive so ifa_load() should use
the address either as brodacst or as peer (when dealing
with ppp interface).

the change makes code more correct. I could not spot
any change on pfctl behavior.

this is interface list on machine where I test the change;

--------8<--------
lo0: flags=2008049<UP,LOOPBACK,RUNNING,MULTICAST,LRO> mtu 32768
	index 4 priority 0 llprio 3
	groups: lo
	inet6 ::1 prefixlen 128
	inet6 fe80::1%lo0 prefixlen 64 scopeid 0x4
	inet 127.0.0.1 netmask 0xff000000
re0: flags=808843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST,AUTOCONF4> mtu 1500
	lladdr fc:5c:ee:37:bd:06
	index 1 priority 0 llprio 3
	groups: egress
	media: Ethernet autoselect (1000baseT full-duplex,master)
	status: active
	inet 192.168.2.61 netmask 0xffffff00 broadcast 192.168.2.255

....

ppp0: flags=8051<UP,POINTOPOINT,RUNNING,MULTICAST> mtu 1300
	index 21 priority 0 llprio 3
	groups: ppp
	inet 172.31.254.102 --> 172.31.254.100 netmask 0xffffffff
tap0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> mtu 1500
	lladdr fe:e1:ba:df:7a:bf
	description: vm1-if0-puffy
	index 28 priority 0 llprio 3
	groups: tap
	status: active
	inet 100.64.1.2 netmask 0xfffffffe
tap1: flags=8943<UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1500
	lladdr fe:e1:ba:d0:30:c5
	description: vm1-if1-puffy
	index 29 priority 0 llprio 3
	groups: tap
	status: active
--------8<--------

and this is pf.conf I did use to test the change:
--------8<--------
pass in from any to ppp0:peer
pass in from any to ppp0:broadcast
pass in from any to re0:broadcast
pass in from any to re0:peer
--------8<--------

trying to load the rules above with current pfctl I get error as
follows:
lifty# pfctl -f /tmp/pf.conf 
no IP address found for ppp0:peer
/tmp/pf.conf:1: could not parse host specification
no IP address found for ppp0:broadcast
/tmp/pf.conf:2: could not parse host specification
no IP address found for re0:peer
/tmp/pf.conf:4: could not parse host specification
pfctl: Syntax error in config file: pf rules not loaded


trying the same rules with fixed pfctl:
lifty# ./pfctl -f /tmp/pf.conf 
no IP address found for ppp0:peer
/tmp/pf.conf:1: could not parse host specification
no IP address found for ppp0:broadcast
/tmp/pf.conf:2: could not parse host specification
no IP address found for re0:peer
/tmp/pf.conf:4: could not parse host specification
pfctl: Syntax error in config file: pf rules not loaded


so the diff below just adds extra line of defense it does not change
a behaviour of pfctl.

thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index 3a7cc6885b3..eb29edb900a 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1459,13 +1459,14 @@ ifa_load(void)
 			copy_satopfaddr(&n->addr.v.a.addr, ifa->ifa_addr);
 			ifa->ifa_netmask->sa_family = ifa->ifa_addr->sa_family;
 			copy_satopfaddr(&n->addr.v.a.mask, ifa->ifa_netmask);
-			if (ifa->ifa_broadaddr != NULL &&
+			if (ifa->ifa_flags & IFF_BROADCAST &&
+			    ifa->ifa_broadaddr != NULL &&
 			    ifa->ifa_broadaddr->sa_len != 0) {
 				ifa->ifa_broadaddr->sa_family =
 				    ifa->ifa_addr->sa_family;
 				copy_satopfaddr(&n->bcast, ifa->ifa_broadaddr);
-			}
-			if (ifa->ifa_dstaddr != NULL &&
+			} else if (ifa->ifa_flags & IFF_POINTOPOINT &&
+			    ifa->ifa_dstaddr != NULL &&
 			    ifa->ifa_dstaddr->sa_len != 0) {
 				ifa->ifa_dstaddr->sa_family =
 				    ifa->ifa_addr->sa_family;