From 9dbb4df7c2ea516bd28b2d6931f95df703739e94 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 290f339e7d0..4d9d3555fe6 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 3ce1f2ca189..6dce60c9bb7 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -4742,7 +4742,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); } @@ -4969,7 +4969,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; @@ -5001,7 +5001,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) && @@ -5415,7 +5415,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; @@ -5445,7 +5445,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); @@ -8705,13 +8705,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); } } @@ -9271,12 +9271,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);