summaryrefslogtreecommitdiff
path: root/sys/net
diff options
context:
space:
mode:
authorAlexandr Nedvedicky <sashan@cvs.openbsd.org>2023-09-07 09:59:44 +0000
committerAlexandr Nedvedicky <sashan@cvs.openbsd.org>2023-09-07 09:59:44 +0000
commite1b38cde5d4041a159304e0a3fc72fc4c911435b (patch)
tree40d3221787756c1d5536d4baa96c3019f1004f12 /sys/net
parentc3659c43c54df6951a857d3d416b09b0a9118c7f (diff)
pf(4) ignores 'keep state' and 'nat-to' actions for unsolicited
icmp error responses. Fix tightens rule matching logic so icmp error responses no longer match 'keep state' rule. In typical scenarios icmp errors (if solicited) should match existing state. The change is going to bite firewalls which deal with asymmetric routes. In those cases the 'keep state' action should be relaxed to sloppy or new 'no state' rule to explicitly match icmp errors should be added. The issue has been reported by Peter J. Philip (pjp _at_ delphinusdns.org). Discussed with bluhm@ and florian@ OK bluhm@
Diffstat (limited to 'sys/net')
-rw-r--r--sys/net/pf.c10
1 files changed, 9 insertions, 1 deletions
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 4f0fc3f91a9..bf6b6d06c08 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: pf.c,v 1.1184 2023/07/31 11:13:09 dlg Exp $ */
+/* $OpenBSD: pf.c,v 1.1185 2023/09/07 09:59:43 sashan Exp $ */
/*
* Copyright (c) 2001 Daniel Hartmeier
@@ -4148,6 +4148,10 @@ enter_ruleset:
(r->rule_flag & PFRULE_STATESLOPPY) == 0 &&
ctx->icmp_dir != PF_IN),
TAILQ_NEXT(r, entries));
+ /* icmp packet must match existing state */
+ PF_TEST_ATTRIB(r->keep_state && ctx->state_icmp &&
+ (r->rule_flag & PFRULE_STATESLOPPY) == 0,
+ TAILQ_NEXT(r, entries));
break;
case IPPROTO_ICMPV6:
@@ -4165,6 +4169,10 @@ enter_ruleset:
ctx->icmp_dir != PF_IN &&
ctx->icmptype != ND_NEIGHBOR_ADVERT),
TAILQ_NEXT(r, entries));
+ /* icmp packet must match existing state */
+ PF_TEST_ATTRIB(r->keep_state && ctx->state_icmp &&
+ (r->rule_flag & PFRULE_STATESLOPPY) == 0,
+ TAILQ_NEXT(r, entries));
break;
default: