diff mbox series

[Openvpn-devel] Fix error in example firewall.sh script

Message ID 20211107174000.16210-1-frank@lichtenheld.com
State Accepted
Headers show
Series [Openvpn-devel] Fix error in example firewall.sh script | expand

Commit Message

Frank Lichtenheld Nov. 7, 2021, 6:40 a.m. UTC 
From: Adrian <adrian.crespo@protonmail.com>

The man page says:
[!] -s, --source address[/mask][,...]

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
 sample/sample-config-files/firewall.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

As part of an initative to clean up the Github PR submissions, submitting
this patch to the mailing list for inclusion. Looks obviously correct to
me.

Comments

Arne Schwabe Nov. 8, 2021, 12:36 a.m. UTC | #1 
Am 07.11.21 um 18:40 schrieb Frank Lichtenheld:
> From: Adrian <adrian.crespo@protonmail.com>
> 
> The man page says:
> [!] -s, --source address[/mask][,...]
> 
> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
> ---
>   sample/sample-config-files/firewall.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> As part of an initative to clean up the Github PR submissions, submitting
> this patch to the mailing list for inclusion. Looks obviously correct to
> me.
> 
> diff --git a/sample/sample-config-files/firewall.sh b/sample/sample-config-files/firewall.sh
> index 19d75ee9..456700ca 100755
> --- a/sample/sample-config-files/firewall.sh
> +++ b/sample/sample-config-files/firewall.sh
> @@ -50,7 +50,7 @@ iptables -A OUTPUT -p tcp --sport 137:139 -o eth0 -j DROP
>   iptables -A OUTPUT -p udp --sport 137:139 -o eth0 -j DROP
>   
>   # Check source address validity on packets going out to internet
> -iptables -A FORWARD -s ! $PRIVATE -i eth1 -j DROP
> +iptables -A FORWARD ! -s $PRIVATE -i eth1 -j DROP
>   
>   # Allow local loopback
>   iptables -A INPUT -s $LOOP -j ACCEPT
> 


I have a vague idea that this is actually different. Like one is that 
condition is not fulfilled and the other is that it is not part of the 
subnet if is different when there is different protocol but I might 
misremember.

Arne
Frank Lichtenheld Nov. 8, 2021, 1:23 a.m. UTC | #2 
> Arne Schwabe <arne@rfc2549.org> hat am 08.11.2021 12:36 geschrieben:
> 
>  
> Am 07.11.21 um 18:40 schrieb Frank Lichtenheld:
> > From: Adrian <adrian.crespo@protonmail.com>
> > 
> > The man page says:
> > [!] -s, --source address[/mask][,...]
> > 
> > Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
> > ---
> >   sample/sample-config-files/firewall.sh | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > As part of an initative to clean up the Github PR submissions, submitting
> > this patch to the mailing list for inclusion. Looks obviously correct to
> > me.
> > 
> > diff --git a/sample/sample-config-files/firewall.sh b/sample/sample-config-files/firewall.sh
> > index 19d75ee9..456700ca 100755
> > --- a/sample/sample-config-files/firewall.sh
> > +++ b/sample/sample-config-files/firewall.sh
> > @@ -50,7 +50,7 @@ iptables -A OUTPUT -p tcp --sport 137:139 -o eth0 -j DROP
> >   iptables -A OUTPUT -p udp --sport 137:139 -o eth0 -j DROP
> >   
> >   # Check source address validity on packets going out to internet
> > -iptables -A FORWARD -s ! $PRIVATE -i eth1 -j DROP
> > +iptables -A FORWARD ! -s $PRIVATE -i eth1 -j DROP
> >   
> >   # Allow local loopback
> >   iptables -A INPUT -s $LOOP -j ACCEPT
> > 
> 
> 
> I have a vague idea that this is actually different. Like one is that 
> condition is not fulfilled and the other is that it is not part of the 
> subnet if is different when there is different protocol but I might 
> misremember.

Certainly does not work with my iptables:
# iptables -A OUTPUT -s ! 10.0.0.0/8 -j ACCEPT
Bad argument `10.0.0.0/8'
Try `iptables -h' or 'iptables --help' for more information.
# iptables -A OUTPUT ! -s 10.0.0.0/8 -j ACCEPT
#

Regards,
   Frank
Kristof Provost via Openvpn-devel Nov. 8, 2021, 2:55 a.m. UTC | #3 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi

Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Monday, November 8th, 2021 at 12:23, Frank Lichtenheld <frank@lichtenheld.com> wrote:

> > Arne Schwabe arne@rfc2549.org hat am 08.11.2021 12:36 geschrieben:
> >
> > Am 07.11.21 um 18:40 schrieb Frank Lichtenheld:
> >
> > > From: Adrian adrian.crespo@protonmail.com
> > >
> > > The man page says:
> > >
> > > [!] -s, --source address[/mask][,...]
> > >
> > > Signed-off-by: Frank Lichtenheld frank@lichtenheld.com
> > > ------------------------------------------------------
> > >
> > > sample/sample-config-files/firewall.sh | 2 +-
> > >
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > As part of an initative to clean up the Github PR submissions, submitting
> > >
> > > this patch to the mailing list for inclusion. Looks obviously correct to
> > >
> > > me.
> > >
> > > diff --git a/sample/sample-config-files/firewall.sh b/sample/sample-config-files/firewall.sh
> > >
> > > index 19d75ee9..456700ca 100755
> > >
> > > --- a/sample/sample-config-files/firewall.sh
> > >
> > > +++ b/sample/sample-config-files/firewall.sh
> > >
> > > @@ -50,7 +50,7 @@ iptables -A OUTPUT -p tcp --sport 137:139 -o eth0 -j DROP
> > >
> > > iptables -A OUTPUT -p udp --sport 137:139 -o eth0 -j DROP
> > >
> > > Check source address validity on packets going out to internet
> > > ==============================================================
> > >
> > > -iptables -A FORWARD -s ! $PRIVATE -i eth1 -j DROP
> > >
> > > +iptables -A FORWARD ! -s $PRIVATE -i eth1 -j DROP
> > >
> > > Allow local loopback
> > > ====================
> > >
> > > iptables -A INPUT -s $LOOP -j ACCEPT
> >
> > I have a vague idea that this is actually different. Like one is that
> >
> > condition is not fulfilled and the other is that it is not part of the
> >
> > subnet if is different when there is different protocol but I might
> >
> > misremember.
>
> Certainly does not work with my iptables:
>
> iptables -A OUTPUT -s ! 10.0.0.0/8 -j ACCEPT
> ============================================
>
> Bad argument `10.0.0.0/8' Try` iptables -h' or 'iptables --help' for more information.
>
> iptables -A OUTPUT ! -s 10.0.0.0/8 -j ACCEPT
> ============================================
>

From: https://ipset.netfilter.org/iptables.man.html

[!] -s, --source address[/mask][,...]
    Source specification. Address can be either a network name, a hostname, a network IP address (with /mask), or a plain IP address. Hostnames will be resolved once only, before the rule is submitted to the kernel. Please note that specifying any name to be resolved with a remote query such as DNS is a really bad idea. The mask can be either an ipv4 network mask (for iptables) or a plain number, specifying the number of 1's at the left side of the network mask. Thus, an iptables mask of 24 is equivalent to 255.255.255.0. A "!" argument before the address specification inverts the sense of the address. The flag --src is an alias for this option. Multiple addresses can be specified, but this will expand to multiple rules (when adding with -A), or will cause multiple rules to be deleted (with -D).

R
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBzBAEBCAAGBQJhiSw2ACEJEE+XnPZrkLidFiEECbw9RGejjXJ5xVVVT5ec
9muQuJ1jZAf/b2mzU/9kXQIIvNAhICrkyDc68AnyR5GRlMItdo91prqcH/bn
ksBxh5OolZeq7Md7K6O63DZgL3Kdj0HpUGavPonVgIrBXj1QoQW69KiEp9/A
98UixypgCCi3yy4wii510Wn9F8ZFmfQBk9l/ilRB5tT+oU6/KyvaZmwz2kRQ
pkmcvXWI40LEvjMXA1Ye5Usn7a1yf5lP2YbJyvhE8mG64mZo6/2fePyTuASd
EhCzxoQ1QIUy/jHL6FCHac6Gt2lx5JH73CI9lKzczvfZmq4Y7a3I5+rjpP2w
Gx4YdA/PkRK5QJtn/KHppCZx4FdYBSvW2Aqq+fSrhzPam//qXQLt6g==
=8IIR
-----END PGP SIGNATURE-----
David Sommerseth Nov. 9, 2021, 1:49 a.m. UTC | #4 
On 08/11/2021 13:23, Frank Lichtenheld wrote:
> 
>> Arne Schwabe <arne@rfc2549.org> hat am 08.11.2021 12:36 geschrieben:
>>
>>   
>> Am 07.11.21 um 18:40 schrieb Frank Lichtenheld:
>>> From: Adrian <adrian.crespo@protonmail.com>
>>>
>>> The man page says:
>>> [!] -s, --source address[/mask][,...]
>>>
>>> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
>>> ---
>>>    sample/sample-config-files/firewall.sh | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> As part of an initative to clean up the Github PR submissions, submitting
>>> this patch to the mailing list for inclusion. Looks obviously correct to
>>> me.
>>>
>>> diff --git a/sample/sample-config-files/firewall.sh b/sample/sample-config-files/firewall.sh
>>> index 19d75ee9..456700ca 100755
>>> --- a/sample/sample-config-files/firewall.sh
>>> +++ b/sample/sample-config-files/firewall.sh
>>> @@ -50,7 +50,7 @@ iptables -A OUTPUT -p tcp --sport 137:139 -o eth0 -j DROP
>>>    iptables -A OUTPUT -p udp --sport 137:139 -o eth0 -j DROP
>>>    
>>>    # Check source address validity on packets going out to internet
>>> -iptables -A FORWARD -s ! $PRIVATE -i eth1 -j DROP
>>> +iptables -A FORWARD ! -s $PRIVATE -i eth1 -j DROP
>>>    
>>>    # Allow local loopback
>>>    iptables -A INPUT -s $LOOP -j ACCEPT
>>>
>>
>>
>> I have a vague idea that this is actually different. Like one is that
>> condition is not fulfilled and the other is that it is not part of the
>> subnet if is different when there is different protocol but I might
>> misremember.
> 
> Certainly does not work with my iptables:
> # iptables -A OUTPUT -s ! 10.0.0.0/8 -j ACCEPT
> Bad argument `10.0.0.0/8'
> Try `iptables -h' or 'iptables --help' for more information.
> # iptables -A OUTPUT ! -s 10.0.0.0/8 -j ACCEPT
> #
> 
> Regards,
>     Frank

I remember iptables announced it would redo the parsing logic for the 
command line interfaces ages ago, where the negation needed to happen 
before the "rule parameter" (-s in this case).  It's probably closer to 
8-10 years since this change, unless my memory is completely corrupted.
David Sommerseth Nov. 9, 2021, 1:50 a.m. UTC | #5 
On 07/11/2021 18:40, Frank Lichtenheld wrote:
> From: Adrian <adrian.crespo@protonmail.com>

> 

> The man page says:

> [!] -s, --source address[/mask][,...]

> 

> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>

> ---

>   sample/sample-config-files/firewall.sh | 2 +-

>   1 file changed, 1 insertion(+), 1 deletion(-)

> 

> As part of an initative to clean up the Github PR submissions, submitting

> this patch to the mailing list for inclusion. Looks obviously correct to

> me.

> 

> diff --git a/sample/sample-config-files/firewall.sh b/sample/sample-config-files/firewall.sh

> index 19d75ee9..456700ca 100755

> --- a/sample/sample-config-files/firewall.sh

> +++ b/sample/sample-config-files/firewall.sh

> @@ -50,7 +50,7 @@ iptables -A OUTPUT -p tcp --sport 137:139 -o eth0 -j DROP

>   iptables -A OUTPUT -p udp --sport 137:139 -o eth0 -j DROP

>   

>   # Check source address validity on packets going out to internet

> -iptables -A FORWARD -s ! $PRIVATE -i eth1 -j DROP

> +iptables -A FORWARD ! -s $PRIVATE -i eth1 -j DROP

>   

>   # Allow local loopback

>   iptables -A INPUT -s $LOOP -j ACCEPT

> 


This change makes sense to me.  The syntax changed ages ago for 
iptables, where the negation needed to happen first.

Acked-By: David Sommerseth <davids@openvpn.net>


-- 
kind regards,

David Sommerseth
OpenVPN Inc
Gert Doering Nov. 9, 2021, 4:14 a.m. UTC | #6 
Your patch has been applied to the master and release/2.5 branch (bugfix).

Thanks!

commit d720c5fd45d5c61b9c797172f8d6a7eaa35b959c (master)
commit 6b2c423aa42a11b41f90aad8f53db71703cee2e2 (release/2.5)
Author: Adrian
Date:   Sun Nov 7 18:40:00 2021 +0100

     Fix error in example firewall.sh script

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: David Sommerseth <davids@openvpn.net>
     Message-Id: <20211107174000.16210-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23128.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/sample/sample-config-files/firewall.sh b/sample/sample-config-files/firewall.sh
index 19d75ee9..456700ca 100755
--- a/sample/sample-config-files/firewall.sh
+++ b/sample/sample-config-files/firewall.sh
@@ -50,7 +50,7 @@  iptables -A OUTPUT -p tcp --sport 137:139 -o eth0 -j DROP
 iptables -A OUTPUT -p udp --sport 137:139 -o eth0 -j DROP
 
 # Check source address validity on packets going out to internet
-iptables -A FORWARD -s ! $PRIVATE -i eth1 -j DROP
+iptables -A FORWARD ! -s $PRIVATE -i eth1 -j DROP
 
 # Allow local loopback
 iptables -A INPUT -s $LOOP -j ACCEPT