From nobody Thu Jan 16 16:54:55 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4YYprJ0wnBz5kfn3; Thu, 16 Jan 2025 16:54:56 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4YYprH74t9z3Ktc; Thu, 16 Jan 2025 16:54:55 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737046496; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=3DIwWxyXOkwweE15Y3YAGjT3cddB8giz5OOkEd5I2Vg=; b=A/K5C4VeP3C4o+zvm0FXGdYryUHhRx5VFT5dnY8C3q6ZHhtFbvJWsyYYKFbg91fL2Q9MxF +V2VHHGaoQhJLI3pkvxVQI0ljtEftFZGXBs+GDE7G6Yzrsu0Uxl5O0z7r7SPmmaOUoQ8fB 56SbWpCNh5nU9ISqbBhLvpIWvLxsrnYJ0BXodhm5TTbv6QCtkVskUpfPzowYFO7kb0sNn8 Q+bBLf8+Aj/bMOPCLk/vKhpiCZL1lmLbz+j1DatdSmq/S4ecW17eVPh4asVdCKkeHxu4bA QrkeACr87iOWr9rXJmhQiJSW1BNz1Q1cKbEBrHDY4pFYrQn8nQn0SSyvA/4iLw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737046496; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=3DIwWxyXOkwweE15Y3YAGjT3cddB8giz5OOkEd5I2Vg=; b=jtqgsXn0GG2EK8LKDYKOTwRVZMESyVcFthXYvLvWepg1S2RGl4Z65NGpNci0cJ2Vfa/Tmz 2G4C8k9vBWH55nGzaHxsugDzMNl7P8o62I7PnYrh1IYHuMe8mwklsGSTTd4HOhT4rfzl5U JMxafI31KInYi6FXkk1TuJLUSl+sYk8l1SgUqapC1WAzzublvO4Q2NhA0boSPYzryKuWhX ywjM8rADJLZzbLne4yCKb97Q0pOY6p5XezOulVUXBN4nfKw0etyhdTEzZXM9AQmXcTOiWI +xQnIjr2nWjPlq658F86MZ5HTP+dxuSEap6sgMygsKftD/DvjF9DCZcPLkAn+g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737046496; a=rsa-sha256; cv=none; b=PpUEPMP0XixozJMgnWuvAC+KAHmNM7pJkdAiYN+ei1wdxjaajmJHlgBCGJLW7nF2z3vYz+ Jn/Jm0UoV2Twt315gf9xdFv+G5aH+CAjjWnDpnwk/xVGtBiyXCNFgsXUrXSa6k26tkEf1m DgIiiJXLQOLAhgsmFeQN3brNOE++F3rQktLBwPHQYwQjv5ftGwm5+KyhNazTbdf3fAwFbA rg3lmioztXOIahwLb6WH9m4hpRLRp7877sQvWK7vTIhc8uYrcviF+2Eqx3dIO+iIX9/me8 Yh4izAwCUGnGtcniPYeBWXJqdp3yxFxjh+Q9GeihMpJQAANk7c52R7gXmgUdfg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4YYprH6MSkzhGl; Thu, 16 Jan 2025 16:54:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 50GGstBb057888; Thu, 16 Jan 2025 16:54:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50GGstxb057884; Thu, 16 Jan 2025 16:54:55 GMT (envelope-from git) Date: Thu, 16 Jan 2025 16:54:55 GMT Message-Id: <202501161654.50GGstxb057884@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 886396f1b1a7 - main - pf: Force logging if pf_create_state() fails List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 886396f1b1a727c642071965612e2c2c9dd11d6c Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=886396f1b1a727c642071965612e2c2c9dd11d6c commit 886396f1b1a727c642071965612e2c2c9dd11d6c Author: Mark Johnston AuthorDate: 2025-01-16 15:44:40 +0000 Commit: Mark Johnston CommitDate: 2025-01-16 16:45:16 +0000 pf: Force logging if pf_create_state() fails Currently packets are logged before pf_create_state() is called, so we might log a packet as passed that is subsequently dropped due to state creation failure. In particular, the drop is not logged, which is wrong. Improve the situation a bit: force logging if state creation fails. This isn't totally right as we'll end up logging the packet twice in this case, but it's better than not logging the drop at all. Add a regression test. Discussed with: kp, ks Co-authored-by: Franco Fichtner MFC after: 2 weeks Sponsored by: Klara, Inc. Sponsored by: OPNsense Differential Revision: https://reviews.freebsd.org/D47953 --- sys/netpfil/pf/pf.c | 1 + tests/sys/netpfil/pf/pflog.sh | 64 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index a49216a9dc20..cfab6a828d5f 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -5911,6 +5911,7 @@ nextrule: &match_rules, udp_mapping); if (action != PF_PASS) { pf_udp_mapping_release(udp_mapping); + pd->act.log |= PF_LOG_FORCE; if (action == PF_DROP && (r->rule_flag & PFRULE_RETURN)) pf_return(r, nr, pd, sk, th, diff --git a/tests/sys/netpfil/pf/pflog.sh b/tests/sys/netpfil/pf/pflog.sh index f5a1241cb5a8..968d165f3dcb 100644 --- a/tests/sys/netpfil/pf/pflog.sh +++ b/tests/sys/netpfil/pf/pflog.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: BSD-2-Clause # # Copyright (c) 2023 Rubicon Communications, LLC (Netgate) +# Copyright (c) 2024 Deciso B.V. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions @@ -132,8 +133,71 @@ matches_cleanup() pft_cleanup } +atf_test_case "state_max" "cleanup" +state_max_head() +{ + atf_set descr 'Ensure that drops due to state limits are logged' + atf_set require.user root +} + +state_max_body() +{ + pflog_init + + epair=$(vnet_mkepair) + + vnet_mkjail alcatraz ${epair}a + jexec alcatraz ifconfig ${epair}a 192.0.2.1/24 up + + ifconfig ${epair}b 192.0.2.2/24 up + + # Sanity check + atf_check -s exit:0 -o ignore \ + ping -c 1 192.0.2.1 + + jexec alcatraz pfctl -e + jexec alcatraz ifconfig pflog0 up + pft_set_rules alcatraz "pass log inet keep state (max 1)" + + jexec alcatraz tcpdump -n -e -ttt --immediate-mode -l -U -i pflog0 >> ${PWD}/pflog.txt & + sleep 1 # Wait for tcpdump to start + + atf_check -s exit:0 -o ignore \ + ping -c 1 192.0.2.1 + + atf_check -s exit:2 -o ignore \ + ping -c 1 192.0.2.1 + + echo "Rules" + jexec alcatraz pfctl -sr -vv + echo "States" + jexec alcatraz pfctl -ss -vv + echo "Log" + cat ${PWD}/pflog.txt + + # First ping passes. + atf_check -o match:".*rule 0/0\(match\): pass in on ${epair}a: 192.0.2.2 > 192.0.2.1: ICMP echo request.*" \ + cat pflog.txt + + # Second ping is blocked due to the state limit. + atf_check -o match:".*rule 0/0\(match\): block in on ${epair}a: 192.0.2.2 > 192.0.2.1: ICMP echo request.*" \ + cat pflog.txt + + # At most three lines should be written: one for the first ping, and + # two for the second: one for the initial pass through the ruleset, and + # then a drop because of the state limit. Ideally only the drop would + # be logged; if this is fixed, the count will be 2 instead of 3. + atf_check -o match:3 grep -c . pflog.txt +} + +state_max_cleanup() +{ + pft_cleanup +} + atf_init_test_cases() { atf_add_test_case "malformed" atf_add_test_case "matches" + atf_add_test_case "state_max" }