From e1b38cde5d4041a159304e0a3fc72fc4c911435b Mon Sep 17 00:00:00 2001 From: Alexandr Nedvedicky Date: Thu, 7 Sep 2023 09:59:44 +0000 Subject: 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(-) (limited to 'sys/net') 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: -- cgit v1.2.3