Jan Engelhardt wrote:
> On Thursday 2010-04-01 12:34, Patrick McHardy wrote:
>>> +static bool
>>> +tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
>>> +{
>>> + const struct iphdr *iph = ip_hdr(skb);
>>> + struct rtable *rt;
>>> + struct flowi fl;
>>> + int err;
>>> +
>>> + memset(&fl, 0, sizeof(fl));
>>> + fl.iif = skb->skb_iif;
>> I'm not sure you really should set iif here. We usually (tunnels, REJECT
>> etc) packets generated locally as new packets.
>>> + fl.mark = skb->mark;
>> The same applies to mark.
>
> If you use TEE in PREROUTING or INPUT, teeing acts more like FORWARD than
> OUTPUT, though. All TEE does is lookup a route to a new fl.dst, but it keeps
> the original src address in fl.src, so if somebody has some source-based policy
> routing, it could suddenly behave different. What do you think?
That might make it unnessarily complicated to use src-based routing
when using TEE. I guess you'd usually have a host for logging or IDS
somewhere on a private network and TEE packets there. So specifying
oif and gateway seems most useful to me.
>>> +/*
>>> + * To detect and deter routed packet loopback when using the --tee option, we
>>> + * take a page out of the raw.patch book: on the copied skb, we set up a fake
>>> + * ->nfct entry, pointing to the local &route_tee_track. We skip routing
>>> + * packets when we see they already have that ->nfct.
>> So without conntrack, people may create loops? If that's the case,
>> I'd suggest to simply forbid TEE'ing packets to loopback. That
>> doesn't seem to be very useful anyways.
>
>>> +#ifdef WITH_CONNTRACK
>>> + if (skb->nfct == &tee_track.ct_general)
>>> + /*
>>> + * Loopback - a packet we already routed, is to be
>>> + * routed another time. Avoid that, now.
>>> + */
> printk("loopback - dropped\n");
>>> + return NF_DROP;
>>> +#endif
>
> We are looking at a historic piece of code - and comments, which
> traces back to when xt_NOTRACK was still in POM.
>
> {
> → /* Previously seen (loopback)? Ignore. */
> → if ((*pskb)->nfct != NULL)
> → → return IPT_CONTINUE;
>
> → /* Attach fake conntrack entry.·
> → If there is a real ct entry correspondig to this packet,·
> → it'll hang aroun till timing out. We don't deal with it
> → for performance reasons. JK */
> → (*pskb)->nfct = &ip_conntrack_untracked.infos[IP_CT_NEW];
> → nf_conntrack_get((*pskb)->nfct);
>
> → return IPT_CONTINUE;
> }
>
> Let's look at the condition "skb->nfct == &tee_track.ct_general" in detail. An
> skb can only already have tee_track when it has been teed.
>
> The teed packet however never traversed Xtables at all. Of course that changes
> once the nesting patch is applied. But was someone really thinking of this, 6
> years ago?
>
> That actually made me wonder and dig in history, and it turns out that
> ipt_ROUTE allowed the packet to be fed back into netif_rx (commit
> bee4e80167e3d024bdb80f400f4ecc8de47cfb03 in pom-ng.git), which would
> explain all the loopback stuff. Since modern xt_TEE does not do
> that evil thing, the comment is a walnut-hard remainder of past times.
>
> I shall remove it now that it has been spotted.
Yeah, but currently it does allow packets to be looped back. These
packets will also go through the netfilter hooks again.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
