firewall: Revert strict martian check on loopback interface

Message ID 20220214163227.10958-1-arne_f@ipfire.org
State Accepted
Commit 2b44044bcf6d4aebcccc223390cf553c68d62eab
Headers
Series firewall: Revert strict martian check on loopback interface |

Commit Message

Arne Fitzenreiter Feb. 14, 2022, 4:32 p.m. UTC
  If the firewall is talking to itself using one of its private IP
addresses (e.g. the primary green interface IP address), it will use the
loopback interface.

This is due to the local routing table which will be looked up first:

  [root@ipfire ~]# ip rule
  0:      from all lookup local
  128:    from all lookup 220
  220:    from all lookup 220
  32765:  from all lookup static
  32766:  from all lookup main
  32767:  from all lookup default

It contains:

  [root@ipfire ~]# ip route show table local
  local 8x.1x.1x.1x dev ppp0 proto kernel scope host src 8x.1x.1x.1x
  local 127.0.0.0/8 dev lo proto kernel scope host src 127.0.0.1
  local 127.0.0.1 dev lo proto kernel scope host src 127.0.0.1
  broadcast 127.255.255.255 dev lo proto kernel scope link src 127.0.0.1
  local 192.168.x.1 dev green0 proto kernel scope host src 192.168.x.1
  broadcast 192.168.x.255 dev green0 proto kernel scope link src 192.168.x.1

Any lookup for the green IP address will show this:

  local 192.168.x.1 dev lo table local src 192.168.x.1 uid 0
      cache <local>

A test ping shows this in tcpdump:

  [root@ipfire ~]# tcpdump -i any icmp -nn
  tcpdump: data link type LINUX_SLL2
  tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
  listening on any, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 262144 bytes
  17:24:22.864293 lo    In  IP 127.0.0.1 > 127.0.0.1: ICMP echo request, id 10420, seq 1, length 64
  17:24:22.864422 lo    In  IP 127.0.0.1 > 127.0.0.1: ICMP echo reply, id 10420, seq 1, length 64
  17:24:29.162021 lo    In  IP 192.168.x.1 > 192.168.x.1: ICMP echo request, id 1555, seq 1, length 64
  17:24:29.162201 lo    In  IP 192.168.x.1 > 192.168.x.1: ICMP echo reply, id 1555, seq 1, length 64

For this reason, we will have to accept any source and destination IP
address on the loopback interface, which is what this patch does.

We can however, continue to check whether we received any packets with
the loopback address on any other interface.

This regression was introduced in commit a36cd34e.

Fixes: #12776 - New spoofed or martian filter block
Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---
 src/initscripts/system/firewall | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
  

Comments

Peter Müller Feb. 14, 2022, 7:14 p.m. UTC | #1
Hello Arne,
hello Michael,
hello *,

thank you for spotting this. While I cannot explain to myself why I did not realise
this during my tests, I agree it makes sense to revert that part of the spoofed/martian
firewall changes.

Apologies for the trouble caused.

Reviewed-by: Peter Müller <peter.mueller@ipfire.org>

Thanks, and best regards,
Peter Müller


> If the firewall is talking to itself using one of its private IP
> addresses (e.g. the primary green interface IP address), it will use the
> loopback interface.
> 
> This is due to the local routing table which will be looked up first:
> 
>   [root@ipfire ~]# ip rule
>   0:      from all lookup local
>   128:    from all lookup 220
>   220:    from all lookup 220
>   32765:  from all lookup static
>   32766:  from all lookup main
>   32767:  from all lookup default
> 
> It contains:
> 
>   [root@ipfire ~]# ip route show table local
>   local 8x.1x.1x.1x dev ppp0 proto kernel scope host src 8x.1x.1x.1x
>   local 127.0.0.0/8 dev lo proto kernel scope host src 127.0.0.1
>   local 127.0.0.1 dev lo proto kernel scope host src 127.0.0.1
>   broadcast 127.255.255.255 dev lo proto kernel scope link src 127.0.0.1
>   local 192.168.x.1 dev green0 proto kernel scope host src 192.168.x.1
>   broadcast 192.168.x.255 dev green0 proto kernel scope link src 192.168.x.1
> 
> Any lookup for the green IP address will show this:
> 
>   local 192.168.x.1 dev lo table local src 192.168.x.1 uid 0
>       cache <local>
> 
> A test ping shows this in tcpdump:
> 
>   [root@ipfire ~]# tcpdump -i any icmp -nn
>   tcpdump: data link type LINUX_SLL2
>   tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
>   listening on any, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 262144 bytes
>   17:24:22.864293 lo    In  IP 127.0.0.1 > 127.0.0.1: ICMP echo request, id 10420, seq 1, length 64
>   17:24:22.864422 lo    In  IP 127.0.0.1 > 127.0.0.1: ICMP echo reply, id 10420, seq 1, length 64
>   17:24:29.162021 lo    In  IP 192.168.x.1 > 192.168.x.1: ICMP echo request, id 1555, seq 1, length 64
>   17:24:29.162201 lo    In  IP 192.168.x.1 > 192.168.x.1: ICMP echo reply, id 1555, seq 1, length 64
> 
> For this reason, we will have to accept any source and destination IP
> address on the loopback interface, which is what this patch does.
> 
> We can however, continue to check whether we received any packets with
> the loopback address on any other interface.
> 
> This regression was introduced in commit a36cd34e.
> 
> Fixes: #12776 - New spoofed or martian filter block
> Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
> ---
>  src/initscripts/system/firewall | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/src/initscripts/system/firewall b/src/initscripts/system/firewall
> index 48653ff57..fc355cd5d 100644
> --- a/src/initscripts/system/firewall
> +++ b/src/initscripts/system/firewall
> @@ -200,14 +200,10 @@ iptables_init() {
>  	iptables -A INPUT -j ICMPINPUT
>  	iptables -A ICMPINPUT -p icmp --icmp-type 8 -j ACCEPT
>  
> -	# Accept everything on loopback if source/destination is loopback space...
> +	# Accept everything on loopback
>  	iptables -N LOOPBACK
> -	iptables -A LOOPBACK -i lo -s 127.0.0.0/8 -j ACCEPT
> -	iptables -A LOOPBACK -o lo -d 127.0.0.0/8 -j ACCEPT
> -
> -	# ... and drop everything else on the loopback interface, since no other traffic should appear there
> -	iptables -A LOOPBACK -i lo -j SPOOFED_MARTIAN
> -	iptables -A LOOPBACK -o lo -j SPOOFED_MARTIAN
> +	iptables -A LOOPBACK -i lo -j ACCEPT
> +	iptables -A LOOPBACK -o lo -j ACCEPT
>  
>  	# Filter all packets with loopback addresses on non-loopback interfaces (spoofed)
>  	iptables -A LOOPBACK -s 127.0.0.0/8 -j SPOOFED_MARTIAN
  
Michael Tremer Feb. 15, 2022, 12:44 p.m. UTC | #2
Hello,

I didn’t foresee this either.

It is one of those reasons why networking is hard, because there are so many shortcuts to keep things simple and fast.

> On 14 Feb 2022, at 19:14, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> Hello Arne,
> hello Michael,
> hello *,
> 
> thank you for spotting this. While I cannot explain to myself why I did not realise
> this during my tests, I agree it makes sense to revert that part of the spoofed/martian
> firewall changes.
> 
> Apologies for the trouble caused.

No trouble caused.

-Michael

> 
> Reviewed-by: Peter Müller <peter.mueller@ipfire.org>
> 
> Thanks, and best regards,
> Peter Müller
> 
> 
>> If the firewall is talking to itself using one of its private IP
>> addresses (e.g. the primary green interface IP address), it will use the
>> loopback interface.
>> 
>> This is due to the local routing table which will be looked up first:
>> 
>>  [root@ipfire ~]# ip rule
>>  0:      from all lookup local
>>  128:    from all lookup 220
>>  220:    from all lookup 220
>>  32765:  from all lookup static
>>  32766:  from all lookup main
>>  32767:  from all lookup default
>> 
>> It contains:
>> 
>>  [root@ipfire ~]# ip route show table local
>>  local 8x.1x.1x.1x dev ppp0 proto kernel scope host src 8x.1x.1x.1x
>>  local 127.0.0.0/8 dev lo proto kernel scope host src 127.0.0.1
>>  local 127.0.0.1 dev lo proto kernel scope host src 127.0.0.1
>>  broadcast 127.255.255.255 dev lo proto kernel scope link src 127.0.0.1
>>  local 192.168.x.1 dev green0 proto kernel scope host src 192.168.x.1
>>  broadcast 192.168.x.255 dev green0 proto kernel scope link src 192.168.x.1
>> 
>> Any lookup for the green IP address will show this:
>> 
>>  local 192.168.x.1 dev lo table local src 192.168.x.1 uid 0
>>      cache <local>
>> 
>> A test ping shows this in tcpdump:
>> 
>>  [root@ipfire ~]# tcpdump -i any icmp -nn
>>  tcpdump: data link type LINUX_SLL2
>>  tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
>>  listening on any, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 262144 bytes
>>  17:24:22.864293 lo    In  IP 127.0.0.1 > 127.0.0.1: ICMP echo request, id 10420, seq 1, length 64
>>  17:24:22.864422 lo    In  IP 127.0.0.1 > 127.0.0.1: ICMP echo reply, id 10420, seq 1, length 64
>>  17:24:29.162021 lo    In  IP 192.168.x.1 > 192.168.x.1: ICMP echo request, id 1555, seq 1, length 64
>>  17:24:29.162201 lo    In  IP 192.168.x.1 > 192.168.x.1: ICMP echo reply, id 1555, seq 1, length 64
>> 
>> For this reason, we will have to accept any source and destination IP
>> address on the loopback interface, which is what this patch does.
>> 
>> We can however, continue to check whether we received any packets with
>> the loopback address on any other interface.
>> 
>> This regression was introduced in commit a36cd34e.
>> 
>> Fixes: #12776 - New spoofed or martian filter block
>> Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
>> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
>> ---
>> src/initscripts/system/firewall | 10 +++-------
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>> 
>> diff --git a/src/initscripts/system/firewall b/src/initscripts/system/firewall
>> index 48653ff57..fc355cd5d 100644
>> --- a/src/initscripts/system/firewall
>> +++ b/src/initscripts/system/firewall
>> @@ -200,14 +200,10 @@ iptables_init() {
>> 	iptables -A INPUT -j ICMPINPUT
>> 	iptables -A ICMPINPUT -p icmp --icmp-type 8 -j ACCEPT
>> 
>> -	# Accept everything on loopback if source/destination is loopback space...
>> +	# Accept everything on loopback
>> 	iptables -N LOOPBACK
>> -	iptables -A LOOPBACK -i lo -s 127.0.0.0/8 -j ACCEPT
>> -	iptables -A LOOPBACK -o lo -d 127.0.0.0/8 -j ACCEPT
>> -
>> -	# ... and drop everything else on the loopback interface, since no other traffic should appear there
>> -	iptables -A LOOPBACK -i lo -j SPOOFED_MARTIAN
>> -	iptables -A LOOPBACK -o lo -j SPOOFED_MARTIAN
>> +	iptables -A LOOPBACK -i lo -j ACCEPT
>> +	iptables -A LOOPBACK -o lo -j ACCEPT
>> 
>> 	# Filter all packets with loopback addresses on non-loopback interfaces (spoofed)
>> 	iptables -A LOOPBACK -s 127.0.0.0/8 -j SPOOFED_MARTIAN
  

Patch

diff --git a/src/initscripts/system/firewall b/src/initscripts/system/firewall
index 48653ff57..fc355cd5d 100644
--- a/src/initscripts/system/firewall
+++ b/src/initscripts/system/firewall
@@ -200,14 +200,10 @@  iptables_init() {
 	iptables -A INPUT -j ICMPINPUT
 	iptables -A ICMPINPUT -p icmp --icmp-type 8 -j ACCEPT
 
-	# Accept everything on loopback if source/destination is loopback space...
+	# Accept everything on loopback
 	iptables -N LOOPBACK
-	iptables -A LOOPBACK -i lo -s 127.0.0.0/8 -j ACCEPT
-	iptables -A LOOPBACK -o lo -d 127.0.0.0/8 -j ACCEPT
-
-	# ... and drop everything else on the loopback interface, since no other traffic should appear there
-	iptables -A LOOPBACK -i lo -j SPOOFED_MARTIAN
-	iptables -A LOOPBACK -o lo -j SPOOFED_MARTIAN
+	iptables -A LOOPBACK -i lo -j ACCEPT
+	iptables -A LOOPBACK -o lo -j ACCEPT
 
 	# Filter all packets with loopback addresses on non-loopback interfaces (spoofed)
 	iptables -A LOOPBACK -s 127.0.0.0/8 -j SPOOFED_MARTIAN