Re: [iptables PATCH 9/9] extensions: DNAT: Support service names in all spots

[Phil Sutter <phil@nwl.cc>]

Hi Jan,

On Wed, Mar 30, 2022 at 08:38:28PM +0200, Jan Engelhardt wrote:
> 
> On Wednesday 2022-03-30 17:58, Phil Sutter wrote:
> 
> >When parsing (parts of) a port spec, if it doesn't start with a digit,
> >try to find the largest substring getservbyname() accepts.
> 
> > -p tcp -j DNAT --to-destination 1.1.1.1:1000-2000/65536;;FAIL
> > -p tcp -j DNAT --to-destination 1.1.1.1:ssh;-p tcp -j DNAT --to-destination 1.1.1.1:22;OK
> > -p tcp -j DNAT --to-destination 1.1.1.1:ftp-data;-p tcp -j DNAT --to-destination 1.1.1.1:20;OK
> >+-p tcp -j DNAT --to-destination 1.1.1.1:ftp-data-ssh;-p tcp -j DNAT --to-destination 1.1.1.1:20-22;OK
> >+-p tcp -j DNAT --to-destination 1.1.1.1:echo-ftp-data;-p tcp -j DNAT --to-destination 1.1.1.1:7-20;OK
> >+-p tcp -j DNAT --to-destination 1.1.1.1:ftp-data-ssh/echo;-p tcp -j DNAT --to-destination 1.1.1.1:20-22/7;OK
> >+-p tcp -j DNAT --to-destination 1.1.1.1:echo-ftp-data/ssh;-p tcp -j DNAT --to-destination 1.1.1.1:7-20/22;OK
> > -j DNAT;;FAIL
> 
> This looks dangerous. It is why I originally never allowed service names in
> port ranges that use dash as the range character. a-b-c could mean a..b-c
> today, and could mean a-b..c tomorrow, either because someone managed to
> inject a-b into the service list.

Yes, it is a rather sloppy solution. I could at least do a shortest
substring first search in addition to check if the input is ambiguous.

Guess if someone is able to manipulate /etc/services, any service names
are problematic, not just in ranges.

Another potential problem I didn't have in mind though is that 'a-b'
could mean [a; b] or [a-b] assuming that all three exist. But I haven't
found a valid example in my /etc/services, yet. :)

> The "solution" would be to use : as the range character, but that would require
> a new --dport option for reasons of command-line compatibility.

Well, we could allow both (a-b with numeric a and b only) and use it in
output only if non-numeric was requested.

Maybe also just limit service names in DNAT to non-ranges. I wanted to
write "like with REDIRECT before the merge", but it looks like it
accepted them as upper boundary, e.g. '10-ftp-data'.

Hmm. I also noticed my series drops support for port 0 from REDIRECT
which commit 84d758b3bc312 ("extensions: REDIRECT: fix --to-ports
parser") explicitly allowed.

Thanks, Phil