From 18af3384cc5f0f02266d67674e860509828bf44a Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Tue, 2 Jan 2024 15:52:39 +0100 Subject: [PATCH] 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 c5ed062fb5f..fb0d971d490 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 4e2888b21a3..417c4b1de67 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 b143aae1434..6035ba63511 100644 --- a/sys/netpfil/pf/if_pflog.c +++ b/sys/netpfil/pf/if_pflog.c @@ -214,7 +214,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; @@ -230,7 +230,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 54d02d694f7..0dc72e22367 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -4500,7 +4500,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); } @@ -4728,7 +4728,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; @@ -4760,7 +4760,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) && @@ -5174,7 +5174,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; @@ -5204,7 +5204,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); @@ -8397,13 +8397,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); } } @@ -8963,12 +8963,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 a92462c53f1..f84e5e222f5 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 */ @@ -1488,7 +1488,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); } @@ -2250,7 +2250,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);