pf: make log(matches) more useful

change log(matches) semantics slightly to make it more useful. since it
is a debug tool change of semantics not considered problematic.
up until now, log(matches) forced logging on subsequent matching rules,
the actual logging used the log settings from that matched rule.
now, log(matches) causes subsequent matches to be logged with the log settings
from the log(matches) rule. in particular (this was the driving point),
log(matches, to pflog23) allows you to have the trace log going to a seperate
pflog interface, not clobbering your regular pflogs, actually not affecting
them at all.
long conversation with bluhm about it, which didn't lead to a single bit
changed in the diff but was very very helpful. ok bluhm as well.

Obtained from:	OpenBSD, henning <henning@openbsd.org>, f61b1efcce
Sponsored by:	Rubicon Communications, LLC ("Netgate")

sys/net/if_pflog.h

@@ -69,9 +69,9 @@ struct pf_ruleset;
 struct pfi_kif;
 struct pf_pdesc;
 
-#define	PFLOG_PACKET(b,t,c,d,e,f,g) do {		\
+#define	PFLOG_PACKET(b,t,c,d,e,f,g,h) do {		\
 	if (pflog_packet_ptr != NULL)			\
-		pflog_packet_ptr(b,t,c,d,e,f,g);	\
+		pflog_packet_ptr(b,t,c,d,e,f,g,h);	\
 } while (0)
 #endif /* _KERNEL */
 #endif /* _NET_IF_PFLOG_H_ */

sys/net/pfvar.h

@@ -1277,7 +1277,7 @@ struct pf_kruleset;
 struct pf_pdesc;
 typedef int pflog_packet_t(uint8_t, u_int8_t,
     struct pf_krule *, struct pf_krule *, struct pf_kruleset *,
-    struct pf_pdesc *, int);
+    struct pf_pdesc *, int, struct pf_krule *);
 extern pflog_packet_t		*pflog_packet_ptr;
 
 #endif /* _KERNEL */

sys/netpfil/pf/if_pflog.c

@@ -243,18 +243,21 @@ pflogioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 static int
 pflog_packet(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 pf_kruleset *ruleset, struct pf_pdesc *pd, int lookupsafe,
+    struct pf_krule *trigger)
 {
 	struct ifnet *ifn;
 	struct pfloghdr hdr;
 
 	if (rm == NULL || pd == NULL)
 		return (1);
+	if (trigger == NULL)
+		trigger = rm;
 
-	if (rm->logif > V_npflogifs)
+	if (trigger->logif > V_npflogifs)
 		return (0);
 
-	ifn = V_pflogifs[rm->logif];
+	ifn = V_pflogifs[trigger->logif];
 	if (ifn == NULL || !bpf_peers_present(ifn->if_bpf))
 		return (0);
 
@@ -281,7 +284,7 @@ pflog_packet(uint8_t action, u_int8_t reason,
 	 * state lock, since this leads to unsafe LOR.
 	 * These conditions are very very rare, however.
 	 */
-	if (rm->log & PF_LOG_SOCKET_LOOKUP && !pd->lookup.done && lookupsafe)
+	if (trigger->log & PF_LOG_SOCKET_LOOKUP && !pd->lookup.done && lookupsafe)
 		pd->lookup.done = pf_socket_lookup(pd);
 	if (pd->lookup.done > 0)
 		hdr.uid = pd->lookup.uid;

sys/netpfil/pf/pf.c

@@ -386,6 +386,9 @@ static int		 pf_match_rcvif(struct mbuf *, struct pf_krule *);
 static void		 pf_counters_inc(int, struct pf_pdesc *,
 			    struct pf_kstate *, struct pf_krule *,
 			    struct pf_krule *);
+static void		 pf_log_matches(struct pf_pdesc *, struct pf_krule *,
+			    struct pf_krule *, struct pf_kruleset *,
+			    struct pf_krule_slist *);
 static void		 pf_overload_task(void *v, int pending);
 static u_short		 pf_insert_src_node(struct pf_ksrc_node *[PF_SN_MAX],
 			    struct pf_srchash *[PF_SN_MAX], struct pf_krule *,
@@ -5535,7 +5538,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 
 		if (nr->log) {
 			PFLOG_PACKET(nr->action, PFRES_MATCH, nr, a,
-			    ruleset, pd, 1);
+			    ruleset, pd, 1, NULL);
 		}
 
 		if (pd->ip_sum)
@@ -5826,18 +5829,17 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 						goto cleanup;
 					}
 				}
-				if (r->log || pd->act.log & PF_LOG_MATCHES)
+				if (r->log)
 					PFLOG_PACKET(r->action, PFRES_MATCH, r,
-					    a, ruleset, pd, 1);
+					    a, ruleset, pd, 1, NULL);
 			} else {
 				match = asd;
 				*rm = r;
 				*am = a;
 				*rsm = ruleset;
-				if (pd->act.log & PF_LOG_MATCHES)
-					PFLOG_PACKET(r->action, PFRES_MATCH, r,
-					    a, ruleset, pd, 1);
 			}
+			if (pd->act.log & PF_LOG_MATCHES)
+				pf_log_matches(pd, r, a, ruleset, &match_rules);
 			if (r->quick)
 				break;
 			r = TAILQ_NEXT(r, entries);
@@ -5866,12 +5868,13 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 		}
 	}
 
-	if (r->log || pd->act.log & PF_LOG_MATCHES) {
+	if (r->log) {
 		if (rewrite)
 			m_copyback(pd->m, pd->off, pd->hdrlen, pd->hdr.any);
-		PFLOG_PACKET(r->action, reason, r, a, ruleset, pd, 1);
+		PFLOG_PACKET(r->action, reason, r, a, ruleset, pd, 1, NULL);
 	}
-
+	if (pd->act.log & PF_LOG_MATCHES)
+		pf_log_matches(pd, r, a, ruleset, &match_rules);
 	if (pd->virtual_proto != PF_VPROTO_FRAGMENT &&
 	   (r->action == PF_DROP) &&
 	    ((r->rule_flag & PFRULE_RETURNRST) ||
@@ -10092,6 +10095,22 @@ pf_counters_inc(int action, struct pf_pdesc *pd,
 	}
 	pf_counter_u64_critical_exit();
 }
+static void
+pf_log_matches(struct pf_pdesc *pd, struct pf_krule *rm,
+    struct pf_krule *am, struct pf_kruleset *ruleset,
+    struct pf_krule_slist *matchrules)
+{
+	struct pf_krule_item	*ri;
+
+	/* if this is the log(matches) rule, packet has been logged already */
+	if (rm->log & PF_LOG_MATCHES)
+		return;
+
+	SLIST_FOREACH(ri, matchrules, entry)
+		if (ri->r->log & PF_LOG_MATCHES)
+			PFLOG_PACKET(rm->action, PFRES_MATCH, rm, am,
+			    ruleset, pd, 1, ri->r);
+}
 
 #if defined(INET) || defined(INET6)
 int
@@ -10495,12 +10514,12 @@ pf_test(sa_family_t af, int dir, int pflags, struct ifnet *ifp, struct mbuf **m0
 
 		if (pd.act.log & PF_LOG_FORCE || lr->log & PF_LOG_ALL)
 			PFLOG_PACKET(action, reason, lr, a,
-			    ruleset, &pd, (s == NULL));
+			    ruleset, &pd, (s == NULL), NULL);
 		if (s) {
 			SLIST_FOREACH(ri, &s->match_rules, entry)
 				if (ri->r->log & PF_LOG_ALL)
 					PFLOG_PACKET(action,
-					    reason, ri->r, a, ruleset, &pd, 0);
+					    reason, ri->r, a, ruleset, &pd, 0, NULL);
 		}
 	}
 

sys/netpfil/pf/pf_norm.c

@@ -1217,7 +1217,7 @@ pf_normalize_ip(u_short *reason, struct pf_pdesc *pd)
 	REASON_SET(reason, PFRES_FRAG);
  drop:
 	if (r != NULL && r->log)
-		PFLOG_PACKET(PF_DROP, *reason, r, NULL, NULL, pd, 1);
+		PFLOG_PACKET(PF_DROP, *reason, r, NULL, NULL, pd, 1, NULL);
 
 	return (PF_DROP);
 }
@@ -1421,7 +1421,7 @@ pf_normalize_tcp(struct pf_pdesc *pd)
  tcp_drop:
 	REASON_SET(&reason, PFRES_NORM);
 	if (rm != NULL && r->log)
-		PFLOG_PACKET(PF_DROP, reason, r, NULL, NULL, pd, 1);
+		PFLOG_PACKET(PF_DROP, reason, r, NULL, NULL, pd, 1, NULL);
 	return (PF_DROP);
 }
 
@@ -2185,7 +2185,7 @@ pf_normalize_sctp(struct pf_pdesc *pd)
 	REASON_SET(&reason, PFRES_NORM);
 	if (rm != NULL && r->log)
 		PFLOG_PACKET(PF_DROP, reason, r, NULL, NULL, pd,
-		    1);
+		    1, NULL);
 
 	return (PF_DROP);
 }