В письме от 15 июня 2015 19:06:39 пользователь Pablo Neira Ayuso написал:
> On Mon, Jun 15, 2015 at 12:11:58PM +0300, Roman I Khimov wrote:
> > Suppose that we're trying to use an xt_string netfilter module to match a
> > string in a specially crafted packet that has "a nice string" starting at
> > offset 28.
> >
> > It could be done in iptables like this:
> >
> > -A some_chain -m string --string "a nice string" --algo bm --from 28 --to
> > 38 -j DROP
> >
> > And it would work as expected. Now changing that to
> >
> > -A some_chain -m string --string "a nice string" --algo bm --from 29 --to
> > 38 -j DROP
> >
> > breaks the match, as expected. But, if we try to make
> >
> > -A some_chain -m string --string "a nice string" --algo bm --from 20 --to
> > 28 -j DROP
> >
> > then it suddenly works again! So the 'to' parameter seems to be inclusive,
> > not working as an offset after which no search should be done. OK, now if
> > we try:
> >
> > -A some_chain -m string --string "a nice string" --algo bm --from 28 --to
> > 28 -j DROP
> Can you reproduce the same behaviour with the km algo?
Will try tomorrow MSK time.
> > The first behaviour (matching at 'to' offset) comes from skb_find_text()
> > comparison. The second one (not matching if 'from' and 'to' are equal)
> > comes from skb_seq_read() check for (abs_offset >= st->upper_offset).
> >
> > I think that the way skb_find_text() handles 'to' is wrong and should be
> > fixed so that we always have predictable behaviour -- only match before
> > 'to' offset.
> >
> > There are currently only five usages of skb_find_text() in the kernel and
> > it looks to me that none of them expect to match something at the 'to'
> > offset, so probably this change is safe.
>
> So both 'from' and 'to' are inclusive which seems consistent to me. If
> you make 'to' non-inclusive, then that will change the third example
> above, right?
Yep.
> That will break existing setups for people that are
> relying on this behaviour. This has been exposed in this way for long
> time, so we should avoid that breakage.
Yes, that could be an issue, but there are other skb_find_text() usages and to
me they suggest that the intended behaviour is to stop search at 'to' offset.
In nf_conntrack_amanda.c, for example:
start = skb_find_text(skb, dataoff, skb->len,
search[SEARCH_CONNECT].ts);
...
stop = skb_find_text(skb, start, skb->len,
search[SEARCH_NEWLINE].ts);
...
stop += start;
...
off = skb_find_text(skb, start, stop, search[i].ts);
First of all, nothing can ever match at skb->len, and second, it looks like
the third usage is also expecting to search from offset 'start' to offset
'stop', not to 'stop + 1'.
em_text_match() in net/sched/em_text.c is also suspicious.
> I would suggest you fix the --from X --to Y where X == Y which is not
> doing what people would expect.
That would certainly make things consistent, but I'm not sure we want it to be
consistent this way.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
