On Sat, Feb 05, 2011 at 03:24:11PM +0100, Henning Brauer wrote:
> I'm pretty damn sure we catch that way earlier.
You are right.
pf_test():
case IPPROTO_ICMPV6: {
action = PF_DROP;
DPFPRINTF(LOG_NOTICE,
"dropping IPv4 packet with ICMPv6 payload");
goto done;
}
pf_test6():
case IPPROTO_ICMP: {
action = PF_DROP;
DPFPRINTF(LOG_NOTICE,
"dropping IPv6 packet with ICMPv4 payload");
goto done;
}
But then some more checks in pf_test_rule() are dead code and should
be removed. Either we rely on the checks in pf_test[46]() or we
don't. But we should do it consistently.
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.723
diff -u -p -r1.723 pf.c
--- net/pf.c 5 Feb 2011 17:29:05 -0000 1.723
+++ net/pf.c 5 Feb 2011 17:50:32 -0000
@@ -2776,8 +2776,6 @@ pf_test_rule(struct pf_rule **rm, struct
break;
#ifdef INET
case IPPROTO_ICMP:
- if (pd->af != AF_INET)
- break;
icmptype = pd->hdr.icmp->icmp_type;
icmpcode = pd->hdr.icmp->icmp_code;
state_icmp = pf_icmp_mapping(pd, icmptype,
@@ -2793,8 +2791,6 @@ pf_test_rule(struct pf_rule **rm, struct
#endif /* INET */
#ifdef INET6
case IPPROTO_ICMPV6:
- if (af != AF_INET6)
- break;
icmptype = pd->hdr.icmp6->icmp6_type;
icmpcode = pd->hdr.icmp6->icmp6_code;
state_icmp = pf_icmp_mapping(pd, icmptype,
@@ -3336,20 +3332,17 @@ pf_translate(struct pf_pdesc *pd, struct
#ifdef INET6
case IPPROTO_ICMPV6:
- if (pd->af == AF_INET6) {
- if (PF_ANEQ(saddr, pd->src, pd->af)) {
- pf_change_a6(pd->src,
- &pd->hdr.icmp6->icmp6_cksum, saddr, 0);
- rewrite = 1;
- }
- if (PF_ANEQ(daddr, pd->dst, pd->af)) {
- pf_change_a6(pd->dst,
- &pd->hdr.icmp6->icmp6_cksum, daddr, 0);
- rewrite = 1;
- }
- break;
+ if (PF_ANEQ(saddr, pd->src, pd->af)) {
+ pf_change_a6(pd->src, &pd->hdr.icmp6->icmp6_cksum,
+ saddr, 0);
+ rewrite = 1;
+ }
+ if (PF_ANEQ(daddr, pd->dst, pd->af)) {
+ pf_change_a6(pd->dst, &pd->hdr.icmp6->icmp6_cksum,
+ daddr, 0);
+ rewrite = 1;
}
- /* FALLTHROUGH */
+ break;
#endif /* INET6 */
default:
