Skip to content

Commit

Permalink
[ICMP]: Ensure that ICMP relookup maintains status quo
Browse files Browse the repository at this point in the history
The ICMP relookup path is only meant to modify behaviour when
appropriate IPsec policies are in place and marked as requiring
relookups.  It is certainly not meant to modify behaviour when
IPsec policies don't exist at all.

However, due to an oversight on the error paths existing behaviour
may in fact change should one of the relookup steps fail.

This patch corrects this by redirecting all errors on relookup
failures to the previous code path.  That is, if the initial
xfrm_lookup let the packet pass, we will stand by that decision
should the relookup fail due to an error.

This should be safe from a security point-of-view because compliant
systems must install a default deny policy so the packet would'nt
have passed in that case.

Many thanks to Julian Anastasov for pointing out this error.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Herbert Xu authored and David S. Miller committed Apr 3, 2008
1 parent 9597362 commit af26818
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 21 deletions.
24 changes: 13 additions & 11 deletions net/ipv4/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
}

if (xfrm_decode_session_reverse(skb_in, &fl, AF_INET))
goto ende;
goto relookup_failed;

if (inet_addr_type(net, fl.fl4_src) == RTN_LOCAL)
err = __ip_route_output_key(net, &rt2, &fl);
Expand All @@ -601,7 +601,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)

fl2.fl4_dst = fl.fl4_src;
if (ip_route_output_key(net, &rt2, &fl2))
goto ende;
goto relookup_failed;

/* Ugh! */
odst = skb_in->dst;
Expand All @@ -614,21 +614,23 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
}

if (err)
goto ende;
goto relookup_failed;

err = xfrm_lookup((struct dst_entry **)&rt2, &fl, NULL,
XFRM_LOOKUP_ICMP);
if (err == -ENOENT) {
switch (err) {
case 0:
dst_release(&rt->u.dst);
rt = rt2;
break;
case -EPERM:
goto ende;
default:
relookup_failed:
if (!rt)
goto out_unlock;
goto route_done;
break;
}

dst_release(&rt->u.dst);
rt = rt2;

if (err)
goto out_unlock;
}

route_done:
Expand Down
22 changes: 12 additions & 10 deletions net/ipv6/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,24 +436,26 @@ void icmpv6_send(struct sk_buff *skb, int type, int code, __u32 info,
}

if (xfrm_decode_session_reverse(skb, &fl2, AF_INET6))
goto out_dst_release;
goto relookup_failed;

if (ip6_dst_lookup(sk, &dst2, &fl))
goto out_dst_release;
goto relookup_failed;

err = xfrm_lookup(&dst2, &fl, sk, XFRM_LOOKUP_ICMP);
if (err == -ENOENT) {
switch (err) {
case 0:
dst_release(dst);
dst = dst2;
break;
case -EPERM:
goto out_dst_release;
default:
relookup_failed:
if (!dst)
goto out;
goto route_done;
break;
}

dst_release(dst);
dst = dst2;

if (err)
goto out;

route_done:
if (ipv6_addr_is_multicast(&fl.fl6_dst))
hlimit = np->mcast_hops;
Expand Down

0 comments on commit af26818

Please sign in to comment.