On Sat, Feb 05, 2011 at 02:03:25PM +0100, Mike Belopuhov wrote:
> we can check if "af == inet" in icmp case obviously, but how and why
> can we end up with af == inet6 and an icmp payload (or af == inet
> and icmp6 payload for that matter)?
Somebody could send us such a packet.
It is strange but not illegal. If we decide, pf should drop such
packets that is a complete different thing.
Anyway we should make sure that pf_translate() can handle these
packets by itself. Protocol 1 for IPv6 packets means no special
treatment so it should end in the else case.
I suggest that diff (untested):
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.722
diff -u -p -r1.722 pf.c
--- net/pf.c 22 Jan 2011 11:43:57 -0000 1.722
+++ net/pf.c 5 Feb 2011 13:43:58 -0000
@@ -3281,8 +3281,7 @@ pf_translate(struct pf_pdesc *pd, struct
if (PF_ANEQ(daddr, pd->dst, pd->af))
pd->destchg = 1;
- switch (pd->proto) {
- case IPPROTO_TCP:
+ if (pd->proto == IPPROTO_TCP) {
if (PF_ANEQ(saddr, pd->src, pd->af) || *pd->sport != sport) {
pf_change_ap(pd->src, pd->sport, pd->ip_sum,
&pd->hdr.tcp->th_sum, saddr, sport, 0, pd->af);
@@ -3293,9 +3292,7 @@ pf_translate(struct pf_pdesc *pd, struct
&pd->hdr.tcp->th_sum, daddr, dport, 0, pd->af);
rewrite = 1;
}
- break;
-
- case IPPROTO_UDP:
+ } else if (pd->proto == IPPROTO_UDP) {
if (PF_ANEQ(saddr, pd->src, pd->af) || *pd->sport != sport) {
pf_change_ap(pd->src, pd->sport, pd->ip_sum,
&pd->hdr.udp->uh_sum, saddr, sport, 1, pd->af);
@@ -3306,10 +3303,8 @@ pf_translate(struct pf_pdesc *pd, struct
&pd->hdr.udp->uh_sum, daddr, dport, 1, pd->af);
rewrite = 1;
}
- break;
-
#ifdef INET
- case IPPROTO_ICMP:
+ } else if (pd->proto == IPPROTO_ICMP && pd->af == AF_INET) {
if (PF_ANEQ(saddr, pd->src, pd->af)) {
pf_change_a(&pd->src->v4.s_addr, pd->ip_sum,
saddr->v4.s_addr, 0);
@@ -3331,28 +3326,21 @@ pf_translate(struct pf_pdesc *pd, struct
rewrite = 1;
}
}
- break;
#endif /* INET */
-
#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;
+ } else if (pd->proto == IPPROTO_ICMPV6 && 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;
}
- /* FALLTHROUGH */
#endif /* INET6 */
-
- default:
+ } else {
switch (pd->af) {
#ifdef INET
case AF_INET:
