From: phessler <phessler@openbsd.org>
Date: Fri, 28 Apr 2023 14:08:34 +0000 (+0000)
Subject: Relax the "pass all" rule so all forms of neighbor advertisements are allowed
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=49f39043a02d6011c1907e1b07eb034652a1269c;p=openbsd

Relax the "pass all" rule so all forms of neighbor advertisements are allowed
in either direction.

This more closely matches the IPv4 ARP behaviour.

From sashan@
discussed with kn@ deraadt@
---

diff --git a/sys/net/pf.c b/sys/net/pf.c
index d1d5f20414f..8af5155866a 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.1173 2023/03/23 01:41:12 jsg Exp $ */
+/*	$OpenBSD: pf.c,v 1.1174 2023/04/28 14:08:34 phessler Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -2608,6 +2608,18 @@ pf_icmp_mapping(struct pf_pdesc *pd, u_int8_t type, int *icmp_dir,
 			    nd->nd_ns_target.s6_addr32[2] ^
 			    nd->nd_ns_target.s6_addr32[3];
 			*virtual_id = (h >> 16) ^ (h & 0xffff);
+			/*
+			 * the extra work here deals with 'keep state' option
+			 * at pass rule  for unsolicited advertisement.  By
+			 * returning 1 (state_icmp = 1) we override 'keep
+			 * state' to 'no state' so we don't create state for
+			 * unsolicited advertisements. No one expects answer to
+			 * unsolicited advertisements so we should be good.
+			 */
+			if (type == ND_NEIGHBOR_ADVERT) {
+				*virtual_type = htons(*virtual_type);
+				return (1);
+			}
 			break;
 		}
 
@@ -4061,7 +4073,6 @@ enter_ruleset:
 			break;
 
 		case IPPROTO_ICMP:
-		case IPPROTO_ICMPV6:
 			/* icmp only. type always 0 in other cases */
 			PF_TEST_ATTRIB((r->type &&
 			    r->type != ctx->icmptype + 1),
@@ -4077,6 +4088,23 @@ enter_ruleset:
 				TAILQ_NEXT(r, entries));
 			break;
 
+		case IPPROTO_ICMPV6:
+			/* icmp only. type always 0 in other cases */
+			PF_TEST_ATTRIB((r->type &&
+			    r->type != ctx->icmptype + 1),
+				TAILQ_NEXT(r, entries));
+			/* icmp only. type always 0 in other cases */
+			PF_TEST_ATTRIB((r->code &&
+			    r->code != ctx->icmpcode + 1),
+				TAILQ_NEXT(r, entries));
+			/* icmp only. don't create states on replies */
+			PF_TEST_ATTRIB((r->keep_state && !ctx->state_icmp &&
+			    (r->rule_flag & PFRULE_STATESLOPPY) == 0 &&
+			    ctx->icmp_dir != PF_IN &&
+			    ctx->icmptype != ND_NEIGHBOR_ADVERT),
+				TAILQ_NEXT(r, entries));
+			break;
+
 		default:
 			break;
 		}