netfilter buglog - Sep 2023 - [Bug 1707] New: iptables-extensions man page misleading for --to

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

            Bug ID: 1707
           Summary: iptables-extensions man page misleading for --to
           Product: iptables
           Version: 1.4.x
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: iptables
          Assignee: netfilter-buglog at lists.netfilter.org
          Reporter: pedretti.fabio at gmail.com

[forwarded from
https://bugs.launchpad.net/ubuntu/+source/iptables/+bug/1430757]
[apparently the user was using Ubuntu trusty -> iptables around 1.4.21]

The man page for iptables-extensions for the "--to'' option (string
module)
implies that the length of the string to match must be included in the byte
range. The example from the man page to block DNS queries for www.netfilter.org
is even more misleading because it unnecessarily searches a 33-byte range
(16+length of the string). The "--to" offset NEED NOT include the
length of the
string to be matched. For example, the following will block DNS queries for
microsoft.com and www.microsoft.com:

sudo iptables -A OUTPUT -o wlan+ -p udp --dport 53 -m string --algo bm --from
40 --to 45 --hex-string "|09|microsoft|03|com|" -j DROP

As a consequence, iptables rules may match packets that the user does not
intend to match.

(Tested on kernel 3.13.0-46-generic.)

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230926/c86fda2d/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

Phil Sutter <phil at nwl.cc> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |pablo at netfilter.org,
                   |                            |phil at nwl.cc

--- Comment #1 from Phil Sutter <phil at nwl.cc> ---
I don't quite understand the code in skb_seq_read(), but it seems to me like
this may return a larger block than was requested in skb_prepare_seq_read() if
the skb is not fragmented. So the --to value may be relevant for fragmented
traffic only. Did you try that?

If the above is right, the man page is wrong in that it promises an upper
boundary for packet data being scanned which is not. It merely ensures the
scanner is able to read up to the given offset.

The example is wrong in that it searches a string of 19 characters in a range
of 16 bytes. But given the above, it still works.

Pablo, am I on the right track? Could you please review?

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230926/18987052/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

--- Comment #2 from Pablo Neira Ayuso <pablo at netfilter.org> ---
(In reply to Phil Sutter from comment #1)
> Pablo, am I on the right track? Could you please review?
According to the code:

unsigned int skb_find_text(struct sk_buff *skb, unsigned int from,              
                           unsigned int to, struct ts_config *config)           
{                                                                               
        struct ts_state state;                                                  
        unsigned int ret;                                                       

        BUILD_BUG_ON(sizeof(struct skb_seq_state) > sizeof(state.cb));

        config->get_next_block = skb_ts_get_next_block;
        config->finish = skb_ts_finish;

        skb_prepare_seq_read(skb, from, to, TS_SKB_CB(&state));

        ret = textsearch_find(config, &state);
        return (ret <= to - from ? ret : UINT_MAX);
}                                                                               
EXPORT_SYMBOL(skb_find_text); 

commit f72b948dcbb8558d639214536c2ace1b0760f41d
Author: Phil Oester <kernel at linuxace.com>
Date:   Mon Jun 26 00:00:57 2006 -0700

    [NET]: skb_find_text ignores to argument

    skb_find_text takes a "to" argument which is supposed to limit how
    far into the skb it will search for the given text.  At present,
    it seems to ignore that argument on the first skb, and instead
    return a match even if the text occurs beyond the limit.

    Patch below fixes this, after adjusting for the "from" starting
    point.  This consequently fixes the netfilter string match's
"--to"
    handling, which currently is broken.

    Signed-off-by: Phil Oester <kernel at linuxace.com>
    Signed-off-by: David S. Miller <davem at davemloft.net>

And if I read this right, this makes sure that the first matching character of
the pattern needs to fall within the [ from - to ] range.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230926/ff844b03/attachment-0001.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

--- Comment #3 from Phil Sutter <phil at nwl.cc> ---
(In reply to Pablo Neira Ayuso from comment #2)
> (In reply to Phil Sutter from comment #1)
> > Pablo, am I on the right track? Could you please review?
> 
> According to the code:
> 
[...]
>         ret = textsearch_find(config, &state);
>         return (ret <= to - from ? ret : UINT_MAX);
[...]
> commit f72b948dcbb8558d639214536c2ace1b0760f41d
> Author: Phil Oester <kernel at linuxace.com>
> Date:   Mon Jun 26 00:00:57 2006 -0700
> 
>     [NET]: skb_find_text ignores to argument
Ah, thanks for digging this up! IIUC, it merely asserts the start of the
matching string lies within the given boundaries, it may still extend to past
the end.

Is my interpretation about skb_seq_read() right though and specifying a too
narrow range could lead to a false negative? In other words, does 'from -
to'
have to include the pattern's length?

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230926/98407db8/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

--- Comment #4 from Pablo Neira Ayuso <pablo at netfilter.org> ---
(In reply to Phil Sutter from comment #3)
> (In reply to Pablo Neira Ayuso from comment #2)
> > (In reply to Phil Sutter from comment #1)
> > > Pablo, am I on the right track? Could you please review?
> > 
> > According to the code:
> > 
> [...]
> >         ret = textsearch_find(config, &state);
> >         return (ret <= to - from ? ret : UINT_MAX);
> [...]
> > commit f72b948dcbb8558d639214536c2ace1b0760f41d
> > Author: Phil Oester <kernel at linuxace.com>
> > Date:   Mon Jun 26 00:00:57 2006 -0700
> > 
> >     [NET]: skb_find_text ignores to argument
> 
> Ah, thanks for digging this up! IIUC, it merely asserts the start of the
> matching string lies within the given boundaries, it may still extend to
past
> the end.
> 
> Is my interpretation about skb_seq_read() right though and specifying a too
> narrow range could lead to a false negative? In other words, does 'from
- to'
> have to include the pattern's length?
By false negative you mean, pattern goes over the 'to' boundary?

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230926/302c5c3d/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

--- Comment #5 from Phil Sutter <phil at nwl.cc> ---
(In reply to Pablo Neira Ayuso from comment #4)
> (In reply to Phil Sutter from comment #3)
> > (In reply to Pablo Neira Ayuso from comment #2)
> > > (In reply to Phil Sutter from comment #1)
> > > > Pablo, am I on the right track? Could you please review?
> > > 
> > > According to the code:
> > > 
> > [...]
> > >         ret = textsearch_find(config, &state);
> > >         return (ret <= to - from ? ret : UINT_MAX);
> > [...]
> > > commit f72b948dcbb8558d639214536c2ace1b0760f41d
> > > Author: Phil Oester <kernel at linuxace.com>
> > > Date:   Mon Jun 26 00:00:57 2006 -0700
> > > 
> > >     [NET]: skb_find_text ignores to argument
> > 
> > Ah, thanks for digging this up! IIUC, it merely asserts the start of
the
> > matching string lies within the given boundaries, it may still extend
to past
> > the end.
> > 
> > Is my interpretation about skb_seq_read() right though and specifying
a too
> > narrow range could lead to a false negative? In other words, does
'from - to'
> > have to include the pattern's length?
> 
> By false negative you mean, pattern goes over the 'to' boundary?
I meant not finding the string in the packet even though it's present. IIUC,
skb_seq_read() deals with fragments. Assume I search for
'www.netfilter.org' at
offset 40 to 45 (obviously too small). Fragment 1 contains data until offset
50, fragment 2 data from 50 on. The pattern is there, so the end of fragment 1
has "www.netfil" and fragment 2 starts with "ter.org". My
small range should
cause skb_seq_read() to return just the first fragment data, not the second one
and thus the match should fail although the string is present. An appropriately
large --to value (17 in this case) would cause skb_seq_read() to return the
next fragment's data in a consecutive call and thus the string is found.

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230926/c7adc79e/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

--- Comment #6 from Pablo Neira Ayuso <pablo at netfilter.org> ---
(In reply to Phil Sutter from comment #5)
> (In reply to Pablo Neira Ayuso from comment #4)
> > (In reply to Phil Sutter from comment #3)
> > > (In reply to Pablo Neira Ayuso from comment #2)
> > > > (In reply to Phil Sutter from comment #1)
> > > > > Pablo, am I on the right track? Could you please
review?
> > > > 
> > > > According to the code:
> > > > 
> > > [...]
> > > >         ret = textsearch_find(config, &state);
> > > >         return (ret <= to - from ? ret : UINT_MAX);
> > > [...]
> > > > commit f72b948dcbb8558d639214536c2ace1b0760f41d
> > > > Author: Phil Oester <kernel at linuxace.com>
> > > > Date:   Mon Jun 26 00:00:57 2006 -0700
> > > > 
> > > >     [NET]: skb_find_text ignores to argument
> > > 
> > > Ah, thanks for digging this up! IIUC, it merely asserts the start
of the
> > > matching string lies within the given boundaries, it may still
extend to past
> > > the end.
> > > 
> > > Is my interpretation about skb_seq_read() right though and
specifying a too
> > > narrow range could lead to a false negative? In other words, does
'from - to'
> > > have to include the pattern's length?
> > 
> > By false negative you mean, pattern goes over the 'to'
boundary?
> 
> I meant not finding the string in the packet even though it's present.
IIUC,
> skb_seq_read() deals with fragments. Assume I search for
'www.netfilter.org'
> at
> offset 40 to 45 (obviously too small). Fragment 1 contains data until
offset
> 50, fragment 2 data from 50 on. The pattern is there, so the end of
fragment
> 1
> has "www.netfil" and fragment 2 starts with "ter.org".
My small range should
> cause skb_seq_read() to return just the first fragment data, not the second
> one
> and thus the match should fail although the string is present. An
> appropriately
> large --to value (17 in this case) would cause skb_seq_read() to return the
> next fragment's data in a consecutive call and thus the string is
found.
Because of how skb_seq_read() works, that means even with KMP it will not find
a matching in such case.

I think your reason in correct, would you make a proof of concept with and
without fragments?

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20230928/a88003fb/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

--- Comment #7 from Phil Sutter <phil at nwl.cc> ---
Created attachment 725
  --> https://bugzilla.netfilter.org/attachment.cgi?id=725&action=edit
kselftest

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20231012/2f36b1a1/attachment.html>

https://bugzilla.netfilter.org/show_bug.cgi?id=1707

Phil Sutter <phil at nwl.cc> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Phil Sutter <phil at nwl.cc> ---
(In reply to Pablo Neira Ayuso from comment #6)
> I think your reason in correct, would you make a proof of concept with and
> without fragments?
Please see attached script. I'm not sure if it's worth submitting as a
kselftest?

Documentation issue should be fixed now, I pushed:

920ece2b392fb ("extensions: string: Clarify description of --to")

Fabio, please review current man page content if you find time. Thanks!

-- 
You are receiving this mail because:
You are watching all bug changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.netfilter.org/pipermail/netfilter-buglog/attachments/20231012/7c3c4393/attachment.html>