Re: [netfilter-core] [NETFILTER] Apply IPsec to ipt_REJECT packets

[Patrick McHardy <kaber@xxxxxxxxx>]

Herbert Xu wrote:

> On Tue, Nov 23, 2004 at 10:44:33PM +0100, Patrick McHardy wrote:
>
> > No. ip_forward handles the original packet, not the packet generated
> > by ipt_REJECT. RSTs generated in NF_IP_FORWARD are routed using
> > ip_route_input because they have a non-local source, so xfrm_route_forward
> > or xfrm_lookup needs to be called for them.
>
> You're absolutely right.  How about this patch then?
I would prefer something like this (based on your patch, untested). 
Currently
ICMP packets are handled different than TCP packets, saddr is set to 0 for
them if it is non-local, so they can't be source-routed properly. This patch
also uses route_reverse for ICMP packets, properly sets fl->proto for output
routed packets and adds a call to xfrm_lookup for input routed packets.

Dave, please don't apply yet, I want to test it first.

Regards
Patrick

===== net/ipv4/netfilter/ipt_REJECT.c 1.32 vs edited =====
--- 1.32/net/ipv4/netfilter/ipt_REJECT.c        2004-11-13 14:41:07 +01:00
+++ edited/net/ipv4/netfilter/ipt_REJECT.c      2004-11-24 00:40:21 +01:00
@@ -38,11 +38,11 @@
 #define DEBUGP(format, args...)
 #endif
 
-static inline struct rtable *route_reverse(struct sk_buff *skb, int hook)
+static struct rtable *
+route_reverse(struct sk_buff *skb, struct flowi *fl, int hook)
 {
        struct iphdr *iph = skb->nh.iph;
        struct dst_entry *odst;
-       struct flowi fl = {};
        struct rtable *rt;
 
        /* We don't require ip forwarding to be enabled to be able to
@@ -52,34 +52,42 @@
            || (skb->nf_bridge && skb->nf_bridge->mask & BRNF_BRIDGED)
 #endif
           ) {
-               fl.nl_u.ip4_u.daddr = iph->saddr;
+               fl->nl_u.ip4_u.daddr = iph->saddr;
                if (hook == NF_IP_LOCAL_IN)
-                       fl.nl_u.ip4_u.saddr = iph->daddr;
-               fl.nl_u.ip4_u.tos = RT_TOS(iph->tos);
+                       fl->nl_u.ip4_u.saddr = iph->daddr;
 
-               if (ip_route_output_key(&rt, &fl) != 0)
+               if (ip_route_output_key(&rt, fl) != 0)
                        return NULL;
        } else {
+               u_int8_t proto;
+
                /* non-local src, find valid iif to satisfy
                 * rp-filter when calling ip_route_input. */
-               fl.nl_u.ip4_u.daddr = iph->daddr;
-               if (ip_route_output_key(&rt, &fl) != 0)
+               proto = fl->proto;
+               fl->proto = 0;
+               fl->nl_u.ip4_u.daddr = iph->daddr;
+               if (ip_route_output_key(&rt, fl) != 0)
                        return NULL;
 
                odst = skb->dst;
                if (ip_route_input(skb, iph->saddr, iph->daddr,
-                                  RT_TOS(iph->tos), rt->u.dst.dev) != 0) {
+                                  fl->nl_u.ip4_u.tos, rt->u.dst.dev) != 0) {
                        dst_release(&rt->u.dst);
                        return NULL;
                }
                dst_release(&rt->u.dst);
                rt = (struct rtable *)skb->dst;
                skb->dst = odst;
-       }
 
-       if (rt->u.dst.error) {
-               dst_release(&rt->u.dst);
-               rt = NULL;
+               fl->nl_u.ip4_u.daddr = iph->saddr;
+               fl->nl_u.ip4_u.saddr = iph->daddr;
+               fl->proto = proto;
+
+               if (rt->u.dst.error ||
+                   xfrm_lookup((struct dst_entry **)&rt, fl, NULL, 0)) {
+                       dst_release(&rt->u.dst);
+                       rt = NULL;
+               }
        }
 
        return rt;
@@ -110,8 +118,24 @@
                return;
 
        /* FIXME: Check checksum --RR */
-       if ((rt = route_reverse(oldskb, hook)) == NULL)
-               return;
+       {
+               struct flowi fl = {
+                       .nl_u = {
+                               .ip4_u = {
+                                       .tos = RT_TOS(oldskb->nh.iph->tos)
+                               }
+                       },
+                       .proto = IPPROTO_TCP,
+                       .uli_u = {
+                               .ports = {
+                                       .sport = oth->dest,
+                                       .dport = oth->source
+                               }
+                       }
+               };
+               if ((rt = route_reverse(oldskb, &fl, hook)) == NULL)
+                       return;
+       }
 
        hh_len = LL_RESERVED_SPACE(rt->u.dst.dev);
 
@@ -205,13 +229,12 @@
        kfree_skb(nskb);
 }
 
-static void send_unreach(struct sk_buff *skb_in, int code)
+static void send_unreach(struct sk_buff *skb_in, int hook, int code)
 {
        struct iphdr *iph;
        struct udphdr *udph;
        struct icmphdr *icmph;
        struct sk_buff *nskb;
-       u32 saddr;
        u8 tos;
        int hh_len, length;
        struct rtable *rt = (struct rtable*)skb_in->dst;
@@ -275,18 +298,24 @@
                        return;
        }
 
-       saddr = iph->daddr;
-       if (!(rt->rt_flags & RTCF_LOCAL))
-               saddr = 0;
-
        tos = (iph->tos & IPTOS_TOS_MASK) | IPTOS_PREC_INTERNETCONTROL;
 
        {
-               struct flowi fl = { .nl_u = { .ip4_u =
-                                             { .daddr = skb_in->nh.iph->saddr,
-                                               .saddr = saddr,
-                                               .tos = RT_TOS(tos) } } };
-               if (ip_route_output_key(&rt, &fl))
+               struct flowi fl = {
+                       .nl_u = {
+                               .ip4_u = {
+                                       .tos = RT_TOS(tos)
+                               }
+                       },
+                       .proto = IPPROTO_ICMP,
+                       .uli_u = {
+                               .icmpt = {
+                                       .type = ICMP_DEST_UNREACH,
+                                       .code = code
+                               }
+                       }
+               };
+               if ((rt = route_reverse(skb_in, &fl, hook)) == NULL)
                        return;
        }
        /* RFC says return as much as we can without exceeding 576 bytes. */
@@ -371,25 +400,25 @@
           must return an absolute verdict. --RR */
        switch (reject->with) {
        case IPT_ICMP_NET_UNREACHABLE:
-               send_unreach(*pskb, ICMP_NET_UNREACH);
+               send_unreach(*pskb, hooknum, ICMP_NET_UNREACH);
                break;
        case IPT_ICMP_HOST_UNREACHABLE:
-               send_unreach(*pskb, ICMP_HOST_UNREACH);
+               send_unreach(*pskb, hooknum, ICMP_HOST_UNREACH);
                break;
        case IPT_ICMP_PROT_UNREACHABLE:
-               send_unreach(*pskb, ICMP_PROT_UNREACH);
+               send_unreach(*pskb, hooknum, ICMP_PROT_UNREACH);
                break;
        case IPT_ICMP_PORT_UNREACHABLE:
-               send_unreach(*pskb, ICMP_PORT_UNREACH);
+               send_unreach(*pskb, hooknum, ICMP_PORT_UNREACH);
                break;
        case IPT_ICMP_NET_PROHIBITED:
-               send_unreach(*pskb, ICMP_NET_ANO);
+               send_unreach(*pskb, hooknum, ICMP_NET_ANO);
                break;
        case IPT_ICMP_HOST_PROHIBITED:
-               send_unreach(*pskb, ICMP_HOST_ANO);
+               send_unreach(*pskb, hooknum, ICMP_HOST_ANO);
                break;
        case IPT_ICMP_ADMIN_PROHIBITED:
-               send_unreach(*pskb, ICMP_PKT_FILTERED);
+               send_unreach(*pskb, hooknum, ICMP_PKT_FILTERED);
                break;
        case IPT_TCP_RESET:
                send_reset(*pskb, hooknum);