Re: pflog shows 0.0.0.0.0 0.0.0.0.0
so now that alexander commited his part, I redid mine - much simpler now.
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.772
diff -u -p -r1.772 pf.c
--- net/pf.c17 Sep 2011 10:12:37 - 1.772
+++ net/pf.c17 Sep 2011 10:57:42 -
@@ -2762,9 +2762,6 @@ pf_test_rule(struct pf_rule **rm, struct
u_int16_tvirtual_type, virtual_id;
u_int8_t icmptype = 0, icmpcode = 0;
- PF_ACPY(pd-nsaddr, pd-src, pd-af);
- PF_ACPY(pd-ndaddr, pd-dst, pd-af);
-
bzero(act, sizeof(act));
act.prio[0] = act.prio[1] = PF_PRIO_NOTSET;
bzero(sns, sizeof(sns));
@@ -2782,14 +2779,6 @@ pf_test_rule(struct pf_rule **rm, struct
}
switch (pd-virtual_proto) {
- case IPPROTO_TCP:
- pd-nsport = th-th_sport;
- pd-ndport = th-th_dport;
- break;
- case IPPROTO_UDP:
- pd-nsport = pd-hdr.udp-uh_sport;
- pd-ndport = pd-hdr.udp-uh_dport;
- break;
#ifdef INET
case IPPROTO_ICMP:
icmptype = pd-hdr.icmp-icmp_type;
@@ -2820,9 +2809,6 @@ pf_test_rule(struct pf_rule **rm, struct
}
break;
#endif /* INET6 */
- default:
- pd-nsport = pd-ndport = 0;
- break;
}
pd-osport = pd-nsport;
@@ -5762,6 +5748,9 @@ pf_setup_pdesc(sa_family_t af, int dir,
}
+ PF_ACPY(pd-nsaddr, pd-src, pd-af);
+ PF_ACPY(pd-ndaddr, pd-dst, pd-af);
+
switch (pd-virtual_proto) {
case PF_VPROTO_FRAGMENT:
/*
@@ -5838,6 +5827,12 @@ pf_setup_pdesc(sa_family_t af, int dir,
}
#endif /* INET6 */
}
+
+ if (pd-sport)
+ pd-nsport = *pd-sport;
+ if (pd-dport)
+ pd-ndport = *pd-dport;
+
return (0);
}
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
Re: pflog shows 0.0.0.0.0 0.0.0.0.0
* Alexander Bluhm alexander.bl...@gmx.net [2011-08-30 20:59]:
When pf_test_rule() is called for fragments that have not been
reassembled, the address copy is not done anymore.
good catch, new diff below.
I think pf_setup_pdesc() should not call pf_test_rule() at all and
just fill the pd struct.
indeed, the test_rule call in the fragment case is a bit weird.
But that is more work so I would suggest
to copy the PF_ACPY() to the handle fragments that aren't reassembled
by normalization.
yup :)
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.771
diff -u -p -r1.771 pf.c
--- pf.c30 Aug 2011 00:40:47 - 1.771
+++ pf.c31 Aug 2011 15:01:06 -
@@ -2762,9 +2762,6 @@ pf_test_rule(struct pf_rule **rm, struct
u_int16_tvirtual_type, virtual_id;
u_int8_t icmptype = 0, icmpcode = 0;
- PF_ACPY(pd-nsaddr, pd-src, pd-af);
- PF_ACPY(pd-ndaddr, pd-dst, pd-af);
-
bzero(act, sizeof(act));
act.prio[0] = act.prio[1] = PF_PRIO_NOTSET;
bzero(sns, sizeof(sns));
@@ -2782,14 +2779,6 @@ pf_test_rule(struct pf_rule **rm, struct
}
switch (pd-virtual_proto) {
- case IPPROTO_TCP:
- pd-nsport = th-th_sport;
- pd-ndport = th-th_dport;
- break;
- case IPPROTO_UDP:
- pd-nsport = pd-hdr.udp-uh_sport;
- pd-ndport = pd-hdr.udp-uh_dport;
- break;
#ifdef INET
case IPPROTO_ICMP:
icmptype = pd-hdr.icmp-icmp_type;
@@ -2820,9 +2809,6 @@ pf_test_rule(struct pf_rule **rm, struct
}
break;
#endif /* INET6 */
- default:
- pd-nsport = pd-ndport = 0;
- break;
}
pd-osport = pd-nsport;
@@ -5679,6 +5665,13 @@ pf_setup_pdesc(sa_family_t af, int dir,
m, *off, pd, a, ruleset, *hdrlen);
if (*action != PF_PASS)
REASON_SET(reason, PFRES_FRAG);
+
+ PF_ACPY(pd-nsaddr, pd-src, pd-af);
+ PF_ACPY(pd-ndaddr, pd-dst, pd-af);
+ if (pd-sport)
+ pd-nsport = *pd-sport;
+ if (pd-dport)
+ pd-ndport = *pd-dport;
return (-1);
}
break;
@@ -5849,6 +5842,14 @@ pf_setup_pdesc(sa_family_t af, int dir,
}
#endif /* INET6 */
}
+
+ PF_ACPY(pd-nsaddr, pd-src, pd-af);
+ PF_ACPY(pd-ndaddr, pd-dst, pd-af);
+ if (pd-sport)
+ pd-nsport = *pd-sport;
+ if (pd-dport)
+ pd-ndport = *pd-dport;
+
return (0);
}
Re: pflog shows 0.0.0.0.0 0.0.0.0.0
On Wed, Aug 31, 2011 at 05:02:01PM +0200, Henning Brauer wrote:
@@ -5679,6 +5665,13 @@ pf_setup_pdesc(sa_family_t af, int dir,
m, *off, pd, a, ruleset, *hdrlen);
if (*action != PF_PASS)
REASON_SET(reason, PFRES_FRAG);
+
+ PF_ACPY(pd-nsaddr, pd-src, pd-af);
+ PF_ACPY(pd-ndaddr, pd-dst, pd-af);
You should set this a few lines above before calling pf_test_rule().
+ if (pd-sport)
+ pd-nsport = *pd-sport;
+ if (pd-dport)
+ pd-ndport = *pd-dport;
They are always NULL here.
return (-1);
}
break;
You have forgotten IPv6. What do you think about deduplicating
this code?
bluhm
Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.771
diff -u -p -r1.771 pf.c
--- net/pf.c30 Aug 2011 00:40:47 - 1.771
+++ net/pf.c31 Aug 2011 18:55:13 -
@@ -5666,21 +5697,9 @@ pf_setup_pdesc(sa_family_t af, int dir,
if (h-ip_hl 5) /* has options */
pd-badopts++;
- if (h-ip_off htons(IP_MF | IP_OFFMASK)) {
- /*
-* handle fragments that aren't reassembled by
-* normalization
-*/
+ if (h-ip_off htons(IP_MF | IP_OFFMASK))
pd-virtual_proto = PF_VPROTO_FRAGMENT;
- if (kif == NULL || r == NULL) /* pflog */
- *action = PF_DROP;
- else
- *action = pf_test_rule(r, s, dir, kif,
- m, *off, pd, a, ruleset, *hdrlen);
- if (*action != PF_PASS)
- REASON_SET(reason, PFRES_FRAG);
- return (-1);
- }
+
break;
}
#endif
@@ -5763,21 +5782,9 @@ pf_setup_pdesc(sa_family_t af, int dir,
pd-tot_len = ntohs(h-ip6_plen) + sizeof(struct ip6_hdr);
pd-virtual_proto = pd-proto = nxt;
- if (fragoff != 0) {
- /*
-* handle fragments that aren't reassembled by
-* normalization
-*/
+ if (fragoff != 0)
pd-virtual_proto = PF_VPROTO_FRAGMENT;
- if (kif == NULL || r == NULL) /* pflog */
- *action = PF_DROP;
- else
- *action = pf_test_rule(r, s, dir, kif,
- m, *off, pd, a, ruleset, *hdrlen);
- if (*action != PF_PASS)
- REASON_SET(reason, PFRES_FRAG);
- return (-1);
- }
+
break;
}
#endif
@@ -5786,7 +5793,20 @@ pf_setup_pdesc(sa_family_t af, int dir,
}
- switch (pd-proto) {
+ switch (pd-virtual_proto) {
+ case PF_VPROTO_FRAGMENT:
+ /*
+* handle fragments that aren't reassembled by
+* normalization
+*/
+ if (kif == NULL || r == NULL) /* pflog */
+ *action = PF_DROP;
+ else
+ *action = pf_test_rule(r, s, dir, kif,
+ m, *off, pd, a, ruleset, *hdrlen);
+ if (*action != PF_PASS)
+ REASON_SET(reason, PFRES_FRAG);
+ return (-1);
case IPPROTO_TCP: {
struct tcphdr *th = pd-hdr.tcp;
Re: pflog shows 0.0.0.0.0 0.0.0.0.0
* Matt Van Mater matt.vanma...@gmail.com [2011-08-22 23:14]:
I am looking into why my
pflog has these ambiguous entries that show source and destination as all
zeros e.g. 0.0.0.0.0 0.0.0.0.0.
this fixes it. nsaddr/port and ndaddr/port were set up in pf_test_rule
and thus not set up if we passed a packet statefully.
I have left the icmp dance in pf_test_rule... some of that should pbly
also move to pf_setup_pdesc.
tests, oks?
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.771
diff -u -p -r1.771 pf.c
--- pf.c30 Aug 2011 00:40:47 - 1.771
+++ pf.c30 Aug 2011 11:14:19 -
@@ -2762,9 +2762,6 @@ pf_test_rule(struct pf_rule **rm, struct
u_int16_tvirtual_type, virtual_id;
u_int8_t icmptype = 0, icmpcode = 0;
- PF_ACPY(pd-nsaddr, pd-src, pd-af);
- PF_ACPY(pd-ndaddr, pd-dst, pd-af);
-
bzero(act, sizeof(act));
act.prio[0] = act.prio[1] = PF_PRIO_NOTSET;
bzero(sns, sizeof(sns));
@@ -2782,14 +2779,6 @@ pf_test_rule(struct pf_rule **rm, struct
}
switch (pd-virtual_proto) {
- case IPPROTO_TCP:
- pd-nsport = th-th_sport;
- pd-ndport = th-th_dport;
- break;
- case IPPROTO_UDP:
- pd-nsport = pd-hdr.udp-uh_sport;
- pd-ndport = pd-hdr.udp-uh_dport;
- break;
#ifdef INET
case IPPROTO_ICMP:
icmptype = pd-hdr.icmp-icmp_type;
@@ -2820,9 +2809,6 @@ pf_test_rule(struct pf_rule **rm, struct
}
break;
#endif /* INET6 */
- default:
- pd-nsport = pd-ndport = 0;
- break;
}
pd-osport = pd-nsport;
@@ -5849,6 +5835,14 @@ pf_setup_pdesc(sa_family_t af, int dir,
}
#endif /* INET6 */
}
+
+ PF_ACPY(pd-nsaddr, pd-src, pd-af);
+ PF_ACPY(pd-ndaddr, pd-dst, pd-af);
+ if (pd-sport)
+ pd-nsport = *pd-sport;
+ if (pd-dport)
+ pd-ndport = *pd-dport;
+
return (0);
}
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services, http://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting
Re: pflog shows 0.0.0.0.0 0.0.0.0.0
On Tue, Aug 30, 2011 at 01:18:12PM +0200, Henning Brauer wrote: --- pf.c 30 Aug 2011 00:40:47 - 1.771 +++ pf.c 30 Aug 2011 11:14:19 - @@ -2762,9 +2762,6 @@ pf_test_rule(struct pf_rule **rm, struct u_int16_tvirtual_type, virtual_id; u_int8_t icmptype = 0, icmpcode = 0; - PF_ACPY(pd-nsaddr, pd-src, pd-af); - PF_ACPY(pd-ndaddr, pd-dst, pd-af); - bzero(act, sizeof(act)); act.prio[0] = act.prio[1] = PF_PRIO_NOTSET; bzero(sns, sizeof(sns)); When pf_test_rule() is called for fragments that have not been reassembled, the address copy is not done anymore. I think pf_setup_pdesc() should not call pf_test_rule() at all and just fill the pd struct. But that is more work so I would suggest to copy the PF_ACPY() to the handle fragments that aren't reassembled by normalization. bluhm
