From 37052907186eaecdbf43aa8a0eb182df97f7146f Mon Sep 17 00:00:00 2001 From: sashan Date: Thu, 7 Sep 2023 09:59:43 +0000 Subject: [PATCH] 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@ --- sys/net/pf.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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: -- 2.20.1