ipsec: Add block rules to avoid conntrack entries

Message ID 1443907913-919-1-git-send-email-michael.tremer@ipfire.org
State Accepted
Commit 80fbd8994934af3ac99d91a45ab1130e41a26ece
Headers

Message

Michael Tremer Oct. 4, 2015, 8:31 a.m. UTC
  If an IPsec VPN connections is not established, there are
rare cases when packets are supposed to be sent through
that said tunnel and incorrectly handled.

Those packets are sent to the default gateway an entry
for this connection is created in the connection tracking
table (usually only happens to UDP). All following packets
are sent the same route even after the tunnel has been
brought up. That leads to SIP phones not being able to
register among other things.

This patch adds firewall rules that these packets are
rejected. That will sent a notification to the client
that the tunnel is not up and avoid the connection to
be added to the connection tracking table.

Apart from a small performance penalty there should
be no other side-effects.

Fixes: #10908

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
Cc: tomvend@rymes.com
Cc: daniel.weismueller@ipfire.org
Cc: morlix@morlix.de
---
 config/firewall/ipsec-block           | 59 +++++++++++++++++++++++++++++++++++
 config/rootfiles/common/stage2        |  1 +
 config/rootfiles/common/x86_64/stage2 |  1 +
 lfs/stage2                            |  2 ++
 src/initscripts/init.d/firewall       |  8 +++++
 src/misc-progs/ipsecctrl.c            |  4 +++
 6 files changed, 75 insertions(+)
 create mode 100644 config/firewall/ipsec-block
  

Comments

Timo Eissler Oct. 5, 2015, 2:56 a.m. UTC | #1
Just by reading the code, this looks good for me.

I'm also very interested in getting feedback from Tom, but i think this will
finally solve the SIP issues we had in combination with IPSec.

Thank you very much for this!

Am 03.10.2015 um 23:31 schrieb Michael Tremer:
> If an IPsec VPN connections is not established, there are
> rare cases when packets are supposed to be sent through
> that said tunnel and incorrectly handled.
>
> Those packets are sent to the default gateway an entry
> for this connection is created in the connection tracking
> table (usually only happens to UDP). All following packets
> are sent the same route even after the tunnel has been
> brought up. That leads to SIP phones not being able to
> register among other things.
>
> This patch adds firewall rules that these packets are
> rejected. That will sent a notification to the client
> that the tunnel is not up and avoid the connection to
> be added to the connection tracking table.
>
> Apart from a small performance penalty there should
> be no other side-effects.
>
> Fixes: #10908
>
> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
> Cc: tomvend@rymes.com
> Cc: daniel.weismueller@ipfire.org
> Cc: morlix@morlix.de
> ---
>  config/firewall/ipsec-block           | 59
+++++++++++++++++++++++++++++++++++
>  config/rootfiles/common/stage2        |  1 +
>  config/rootfiles/common/x86_64/stage2 |  1 +
>  lfs/stage2                            |  2 ++
>  src/initscripts/init.d/firewall       |  8 +++++
>  src/misc-progs/ipsecctrl.c            |  4 +++
>  6 files changed, 75 insertions(+)
>  create mode 100644 config/firewall/ipsec-block
>
> diff --git a/config/firewall/ipsec-block b/config/firewall/ipsec-block
> new file mode 100644
> index 0000000..9fa8e1a
> --- /dev/null
> +++ b/config/firewall/ipsec-block
> @@ -0,0 +1,59 @@
> +#!/bin/bash
>
+###############################################################################
>
+#
      #
> +# IPFire.org - A linux based
firewall                                         #
> +# Copyright (C) 2015 IPFire
Team                                              #
>
+#
      #
> +# This program is free software: you can redistribute it and/or
modify        #
> +# it under the terms of the GNU General Public License as published
by        #
> +# the Free Software Foundation, either version 3 of the License,
or           #
> +# (at your option) any later
version.                                         #
>
+#
      #
> +# This program is distributed in the hope that it will be
useful,             #
> +# but WITHOUT ANY WARRANTY; without even the implied warranty
of              #
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
the               #
> +# GNU General Public License for more
details.                                #
>
+#
      #
> +# You should have received a copy of the GNU General Public
License           #
> +# along with this program.  If not, see
<http://www.gnu.org/licenses/>.       #
>
+#
      #
>
+###############################################################################
> +
> +VPN_CONFIG="/var/ipfire/vpn/config"
> +
> +block_subnet() {
> +    local subnet="${1}"
> +
> +    # Don't block a wildcard subnet
> +    if [ "${subnet}" = "0.0.0.0/0" ] || [ "${subnet}" =
"0.0.0.0/0.0.0.0" ]; then
> +        return 0
> +    fi
> +
> +    iptables -A IPSECBLOCK -d "${subnet}" -j REJECT --reject-with
icmp-net-unreachable
> +}
> +
> +block_ipsec() {
> +    # Flush all exists rules
> +    iptables -F IPSECBLOCK
> +
> +    local id status name lefthost type ctype unknown1 unknown2 unknown3
> +    local leftsubnets unknown4 righthost rightsubnets rest
> +    while IFS="," read -r id status name lefthost type ctype unkown1
unknown2 unknown3 \
> +            leftsubnets unknown4 righthost rightsubnets rest; do
> +        # Check if the connection is enabled
> +        [ "${status}" = "on" ] || continue
> +
> +        # Check if this a net-to-net connection
> +        [ "${type}" = "net" ] || continue
> +
> +        # Split multiple subnets
> +        rightsubnets="${rightsubnets//\|/ }"
> +
> +        local rightsubnet
> +        for rightsubnet in ${rightsubnets}; do
> +            block_subnet "${rightsubnet}"
> +        done
> +    done < "${VPN_CONFIG}"
> +}
> +
> +block_ipsec || exit $?
> diff --git a/config/rootfiles/common/stage2
b/config/rootfiles/common/stage2
> index 90e28d9..4021caf 100644
> --- a/config/rootfiles/common/stage2
> +++ b/config/rootfiles/common/stage2
> @@ -73,6 +73,7 @@ run
>  #usr/lib
>  usr/lib/firewall
>  usr/lib/firewall/firewall-lib.pl
> +usr/lib/firewall/ipsec-block
>  usr/lib/firewall/rules.pl
>  #usr/lib/libgcc_s.so
>  usr/lib/libgcc_s.so.1
> diff --git a/config/rootfiles/common/x86_64/stage2
b/config/rootfiles/common/x86_64/stage2
> index 0ac9ab5..531daaa 100644
> --- a/config/rootfiles/common/x86_64/stage2
> +++ b/config/rootfiles/common/x86_64/stage2
> @@ -74,6 +74,7 @@ run
>  #usr/lib
>  usr/lib/firewall
>  usr/lib/firewall/firewall-lib.pl
> +usr/lib/firewall/ipsec-block
>  usr/lib/firewall/rules.pl
>  #usr/lib/libgcc_s.so
>  usr/lib/libgcc_s.so.1
> diff --git a/lfs/stage2 b/lfs/stage2
> index 3244fa3..ec5d117 100644
> --- a/lfs/stage2
> +++ b/lfs/stage2
> @@ -114,6 +114,8 @@ endif
>          /usr/lib/firewall/rules.pl
>      install -m 644 $(DIR_SRC)/config/firewall/firewall-lib.pl \
>          /usr/lib/firewall/firewall-lib.pl
> +    install -m 755 $(DIR_SRC)/config/firewall/ipsec-block \
> +        /usr/lib/firewall/ipsec-block
> 
>      # Nobody user
>      -mkdir -p /home/nobody
> diff --git a/src/initscripts/init.d/firewall
b/src/initscripts/init.d/firewall
> index 8ca02bc..2d462d7 100644
> --- a/src/initscripts/init.d/firewall
> +++ b/src/initscripts/init.d/firewall
> @@ -115,6 +115,11 @@ iptables_init() {
>      iptables -A INPUT -j GUARDIAN
>      iptables -A FORWARD -j GUARDIAN
> 
> +    # Block non-established IPsec networks
> +    iptables -N IPSECBLOCK
> +    iptables -A FORWARD -m policy --dir out --pol none -j IPSECBLOCK
> +    iptables -A OUTPUT  -m policy --dir out --pol none -j IPSECBLOCK
> +
>      # Block OpenVPN transfer networks
>      iptables -N OVPNBLOCK
>      iptables -A INPUT   -i tun+ -j OVPNBLOCK
> @@ -270,6 +275,9 @@ iptables_init() {
>      iptables -t nat -N REDNAT
>      iptables -t nat -A POSTROUTING -j REDNAT
> 
> +    # Populate IPsec block chain
> +    /usr/lib/firewall/ipsec-block
> +
>      # Apply OpenVPN firewall rules
>      /usr/local/bin/openvpnctrl --firewall-rules
> 
> diff --git a/src/misc-progs/ipsecctrl.c b/src/misc-progs/ipsecctrl.c
> index e99202d..7499e94 100644
> --- a/src/misc-progs/ipsecctrl.c
> +++ b/src/misc-progs/ipsecctrl.c
> @@ -144,6 +144,9 @@ void turn_connection_on(char *name, char *type) {
>                  "/usr/sbin/ipsec down %s >/dev/null", name);
>          safe_system(command);
> 
> +    // Reload the IPsec block chain
> +    safe_system("/usr/lib/firewall/ipsec-block >/dev/null");
> +
>      // Reload the configuration into the daemon (#10339).
>      ipsec_reload();
> 
> @@ -302,6 +305,7 @@ int main(int argc, char *argv[]) {
> 
>          // start the system
>          if ((argc == 2) && strcmp(argv[1], "S") == 0) {
> +        safe_system("/usr/lib/firewall/ipsec-block >/dev/null");
>          safe_system("/usr/sbin/ipsec restart >/dev/null");
>                  exit(0);
>          }
  
Michael Tremer Oct. 5, 2015, 4:07 a.m. UTC | #2
On Sun, 2015-10-04 at 12:25 -0400, Tom Rymes wrote:
> On 10/03/2015 5:31 PM, Michael Tremer wrote:
> > If an IPsec VPN connections is not established, there are
> > rare cases when packets are supposed to be sent through
> > that said tunnel and incorrectly handled.
> 
> Michael, et. al.:
> 
> I just posted a comment on the bug before I realized that e-mail
> would 
> be more appropriate.
> 
> My apologies for not being up to speed on this, but can you hold my
> hand 
> on implementing this? I am simply not confident enough to apply these
> changes without a better understanding of what I am doing.

You got this already applied (at least the bare essence of that). I
think we should wait for someone else to confirm that this is not
crashing anything :)

Since I emailed this patch I am still wondering if we should not limit
this rule to the RED interface. We didn't do that when we tried all
this on one of your machines (
https://bugzilla.ipfire.org/show_bug.cgi?id=10908#c16). It is an easier
solution, but I am wondering if that does not have any side-effects...

@Timo: You should use the Reviewed-by: tag then.

Best,
-Michael

> 
> Thank you,
> 
> Tom
  
Timo Eissler Oct. 9, 2015, 4:11 a.m. UTC | #3
Sure.

Reviewed-by: Timo Eissler <timo.eissler@ipfire.org>

Am 04.10.2015 um 19:07 schrieb Michael Tremer:
> On Sun, 2015-10-04 at 12:25 -0400, Tom Rymes wrote:
>> On 10/03/2015 5:31 PM, Michael Tremer wrote:
>>> If an IPsec VPN connections is not established, there are
>>> rare cases when packets are supposed to be sent through
>>> that said tunnel and incorrectly handled.
>> Michael, et. al.:
>>
>> I just posted a comment on the bug before I realized that e-mail
>> would 
>> be more appropriate.
>>
>> My apologies for not being up to speed on this, but can you hold my
>> hand 
>> on implementing this? I am simply not confident enough to apply these
>> changes without a better understanding of what I am doing.
> You got this already applied (at least the bare essence of that). I
> think we should wait for someone else to confirm that this is not
> crashing anything :)
>
> Since I emailed this patch I am still wondering if we should not limit
> this rule to the RED interface. We didn't do that when we tried all
> this on one of your machines (
> https://bugzilla.ipfire.org/show_bug.cgi?id=10908#c16). It is an easier
> solution, but I am wondering if that does not have any side-effects...
>
> @Timo: You should use the Reviewed-by: tag then.
>
> Best,
> -Michael
>
>> Thank you,
>>
>> Tom
  
Michael Tremer Oct. 16, 2015, 8:40 a.m. UTC | #4
Hello Tom,

any news so far? Is everything still working?

If so I would like to merge this patch for Core Update 95.

Best,
-Michael

On Sun, 2015-10-04 at 18:07 +0100, Michael Tremer wrote:
> On Sun, 2015-10-04 at 12:25 -0400, Tom Rymes wrote:
> > On 10/03/2015 5:31 PM, Michael Tremer wrote:
> > > If an IPsec VPN connections is not established, there are
> > > rare cases when packets are supposed to be sent through
> > > that said tunnel and incorrectly handled.
> > 
> > Michael, et. al.:
> > 
> > I just posted a comment on the bug before I realized that e-mail
> > would 
> > be more appropriate.
> > 
> > My apologies for not being up to speed on this, but can you hold my
> > hand 
> > on implementing this? I am simply not confident enough to apply
> > these
> > changes without a better understanding of what I am doing.
> 
> You got this already applied (at least the bare essence of that). I
> think we should wait for someone else to confirm that this is not
> crashing anything :)
> 
> Since I emailed this patch I am still wondering if we should not
> limit
> this rule to the RED interface. We didn't do that when we tried all
> this on one of your machines (
> https://bugzilla.ipfire.org/show_bug.cgi?id=10908#c16). It is an
> easier
> solution, but I am wondering if that does not have any side
> -effects...
> 
> @Timo: You should use the Reviewed-by: tag then.
> 
> Best,
> -Michael
> 
> > 
> > Thank you,
> >