git: 948e8413aba0 - main - pflog: pass the action to pflog directly

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Thu, 04 Jan 2024 22:08:49 UTC
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=948e8413aba0ee600ceb563cee048a6ef74a6a2c

commit 948e8413aba0ee600ceb563cee048a6ef74a6a2c
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-01-02 14:52:39 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-01-04 22:08:08 +0000

    pflog: pass the action to pflog directly
    
    If a packet is malformed, it is dropped by pf(4).  The rule referenced
    in pflog(4) is the default rule.  As the default rule is a pass
    rule, tcpdump printed "pass" although the packet was actually
    dropped. Use the actual action, rather than the rule's action, or an
    attempt at guessing the correct action.
    
    Inspired by OpenBSD's 'pflog(4) logs packet dropped by default rule with block.' commit.
    
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sys/net/if_pflog.h        |  4 ++--
 sys/net/pfvar.h           |  2 +-
 sys/netpfil/pf/if_pflog.c |  4 ++--
 sys/netpfil/pf/pf.c       | 22 +++++++++++-----------
 sys/netpfil/pf/pf_norm.c  | 10 +++++-----
 5 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/sys/net/if_pflog.h b/sys/net/if_pflog.h
index c5ed062fb5f2..fb0d971d490c 100644
--- a/sys/net/if_pflog.h
+++ b/sys/net/if_pflog.h
@@ -69,9 +69,9 @@ struct pf_ruleset;
 struct pfi_kif;
 struct pf_pdesc;
 
-#define	PFLOG_PACKET(i,a,b,c,d,e,f,g,di) do {		\
+#define	PFLOG_PACKET(i,a,b,t,c,d,e,f,g,di) do {		\
 	if (pflog_packet_ptr != NULL)			\
-		pflog_packet_ptr(i,a,b,c,d,e,f,g,di);	\
+		pflog_packet_ptr(i,a,b,t,c,d,e,f,g,di);	\
 } while (0)
 #endif /* _KERNEL */
 #endif /* _NET_IF_PFLOG_H_ */
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 48162b786a86..020b79ded94c 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1208,7 +1208,7 @@ void			pf_state_export(struct pf_state_export *,
 struct pf_kruleset;
 struct pf_pdesc;
 typedef int pflog_packet_t(struct pfi_kkif *, struct mbuf *, sa_family_t,
-    u_int8_t, struct pf_krule *, struct pf_krule *, struct pf_kruleset *,
+    uint8_t, u_int8_t, struct pf_krule *, struct pf_krule *, struct pf_kruleset *,
     struct pf_pdesc *, int);
 extern pflog_packet_t		*pflog_packet_ptr;
 
diff --git a/sys/netpfil/pf/if_pflog.c b/sys/netpfil/pf/if_pflog.c
index 82e0daa01898..7ac337a84c5d 100644
--- a/sys/netpfil/pf/if_pflog.c
+++ b/sys/netpfil/pf/if_pflog.c
@@ -216,7 +216,7 @@ pflogioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 
 static int
 pflog_packet(struct pfi_kkif *kif, struct mbuf *m, sa_family_t af,
-    u_int8_t reason, struct pf_krule *rm, struct pf_krule *am,
+    uint8_t action, u_int8_t reason, struct pf_krule *rm, struct pf_krule *am,
     struct pf_kruleset *ruleset, struct pf_pdesc *pd, int lookupsafe)
 {
 	struct ifnet *ifn;
@@ -231,7 +231,7 @@ pflog_packet(struct pfi_kkif *kif, struct mbuf *m, sa_family_t af,
 	bzero(&hdr, sizeof(hdr));
 	hdr.length = PFLOG_REAL_HDRLEN;
 	hdr.af = af;
-	hdr.action = rm->action;
+	hdr.action = action;
 	hdr.reason = reason;
 	memcpy(hdr.ifname, kif->pfik_name, sizeof(hdr.ifname));
 
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 9e9743c1e5e0..9489da0b3e53 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -4475,7 +4475,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif,
 		KASSERT(nk != NULL, ("%s: null nk", __func__));
 
 		if (nr->log) {
-			PFLOG_PACKET(kif, m, af, PFRES_MATCH, nr, a,
+			PFLOG_PACKET(kif, m, af, PF_PASS, PFRES_MATCH, nr, a,
 			    ruleset, pd, 1);
 		}
 
@@ -4703,7 +4703,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif,
 					pf_rule_to_actions(r, &pd->act);
 					if (r->log)
 						PFLOG_PACKET(kif, m, af,
-						    PFRES_MATCH, r,
+						    r->action, PFRES_MATCH, r,
 						    a, ruleset, pd, 1);
 				} else {
 					match = 1;
@@ -4735,7 +4735,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif,
 	if (r->log) {
 		if (rewrite)
 			m_copyback(m, off, hdrlen, pd->hdr.any);
-		PFLOG_PACKET(kif, m, af, reason, r, a, ruleset, pd, 1);
+		PFLOG_PACKET(kif, m, af, r->action, reason, r, a, ruleset, pd, 1);
 	}
 
 	if ((r->action == PF_DROP) &&
@@ -5149,7 +5149,7 @@ pf_test_fragment(struct pf_krule **rm, struct pfi_kkif *kif,
 					pf_rule_to_actions(r, &pd->act);
 					if (r->log)
 						PFLOG_PACKET(kif, m, af,
-						    PFRES_MATCH, r,
+						    r->action, PFRES_MATCH, r,
 						    a, ruleset, pd, 1);
 				} else {
 					match = 1;
@@ -5179,7 +5179,7 @@ pf_test_fragment(struct pf_krule **rm, struct pfi_kkif *kif,
 	pf_rule_to_actions(r, &pd->act);
 
 	if (r->log)
-		PFLOG_PACKET(kif, m, af, reason, r, a, ruleset, pd, 1);
+		PFLOG_PACKET(kif, m, af, r->action, reason, r, a, ruleset, pd, 1);
 
 	if (r->action != PF_PASS)
 		return (PF_DROP);
@@ -8332,13 +8332,13 @@ done:
 			lr = r;
 
 		if (pd.act.log & PF_LOG_FORCE || lr->log & PF_LOG_ALL)
-			PFLOG_PACKET(kif, m, AF_INET, reason, lr, a, ruleset,
-			    &pd, (s == NULL));
+			PFLOG_PACKET(kif, m, AF_INET, action, reason, lr, a,
+			    ruleset, &pd, (s == NULL));
 		if (s) {
 			SLIST_FOREACH(ri, &s->match_rules, entry)
 				if (ri->r->log & PF_LOG_ALL)
-					PFLOG_PACKET(kif, m, AF_INET, reason,
-					    ri->r, a, ruleset, &pd, 0);
+					PFLOG_PACKET(kif, m, AF_INET, action,
+					    reason, ri->r, a, ruleset, &pd, 0);
 		}
 	}
 
@@ -8896,12 +8896,12 @@ done:
 			lr = r;
 
 		if (pd.act.log & PF_LOG_FORCE || lr->log & PF_LOG_ALL)
-			PFLOG_PACKET(kif, m, AF_INET6, reason, lr, a, ruleset,
+			PFLOG_PACKET(kif, m, AF_INET6, action, reason, lr, a, ruleset,
 			    &pd, (s == NULL));
 		if (s) {
 			SLIST_FOREACH(ri, &s->match_rules, entry)
 				if (ri->r->log & PF_LOG_ALL)
-					PFLOG_PACKET(kif, m, AF_INET6, reason,
+					PFLOG_PACKET(kif, m, AF_INET6, action, reason,
 					    ri->r, a, ruleset, &pd, 0);
 		}
 	}
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index a119d85f806e..3824e7b2f595 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -1184,7 +1184,7 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, u_short *reason,
 	REASON_SET(reason, PFRES_FRAG);
  drop:
 	if (r != NULL && r->log)
-		PFLOG_PACKET(kif, m, AF_INET, *reason, r, NULL, NULL, pd, 1);
+		PFLOG_PACKET(kif, m, AF_INET, PF_DROP, *reason, r, NULL, NULL, pd, 1);
 
 	return (PF_DROP);
 }
@@ -1357,13 +1357,13 @@ again:
  shortpkt:
 	REASON_SET(reason, PFRES_SHORT);
 	if (r != NULL && r->log)
-		PFLOG_PACKET(kif, m, AF_INET6, *reason, r, NULL, NULL, pd, 1);
+		PFLOG_PACKET(kif, m, AF_INET6, PF_DROP, *reason, r, NULL, NULL, pd, 1);
 	return (PF_DROP);
 
  drop:
 	REASON_SET(reason, PFRES_NORM);
 	if (r != NULL && r->log)
-		PFLOG_PACKET(kif, m, AF_INET6, *reason, r, NULL, NULL, pd, 1);
+		PFLOG_PACKET(kif, m, AF_INET6, PF_DROP, *reason, r, NULL, NULL, pd, 1);
 	return (PF_DROP);
 }
 #endif /* INET6 */
@@ -1489,7 +1489,7 @@ pf_normalize_tcp(struct pfi_kkif *kif, struct mbuf *m, int ipoff,
  tcp_drop:
 	REASON_SET(&reason, PFRES_NORM);
 	if (rm != NULL && r->log)
-		PFLOG_PACKET(kif, m, AF_INET, reason, r, NULL, NULL, pd, 1);
+		PFLOG_PACKET(kif, m, AF_INET, PF_DROP, reason, r, NULL, NULL, pd, 1);
 	return (PF_DROP);
 }
 
@@ -2251,7 +2251,7 @@ pf_normalize_sctp(int dir, struct pfi_kkif *kif, struct mbuf *m, int ipoff,
 sctp_drop:
 	REASON_SET(&reason, PFRES_NORM);
 	if (rm != NULL && r->log)
-		PFLOG_PACKET(kif, m, AF_INET, reason, r, NULL, NULL, pd,
+		PFLOG_PACKET(kif, m, AF_INET, PF_DROP, reason, r, NULL, NULL, pd,
 		    1);
 
 	return (PF_DROP);