diff mbox series

[Openvpn-devel] Fix redirecting of IPv4 default gateway if connecting over IPv6.

Message ID 20201002175736.82609-1-gert@greenie.muc.de
State Accepted
Headers
Series [Openvpn-devel] Fix redirecting of IPv4 default gateway if connecting over IPv6. |

Commit Message

Gert Doering Oct. 2, 2020, 7:57 a.m. UTC 
  Commit aa34684972eb0 fixed a long-standing bug in setting the
"route-list" flag RTSA_REMOTE_HOST for IPv4 ("we have a well-defined
remote_host == VPN server IP address") even if connecting over IPv6.

Unfortunately the logic in redirect_default_route_to_vpn() was also
wrong, and refused cooperation if that flag is not set, triggering
the message
    "NOTE: unable to redirect IPv4 default gateway -- Cannot
     obtain current remote host address"

Correct operation: if RTSA_REMOTE_HOST is not set, or remote_host
is IPV4_INVALID_ADDR (= 255.255.255.255), do not try to install a
host route for continued connectivity to the VPN server - which is
not needed when connecting over IPv6.  But the actual *routes*
(/0 or 2 x /1) can be installed just fine.

There is a second bug here, which hits if there is no IPv4 gateway
at all.  In that case, the same function triggers the message
    "NOTE: unable to redirect IPv4 default gateway -- Cannot
     read current default gateway from system"

This is caused by using "IPV4_INVALID_ADDR" as a flag for "do we
know the remote_host?" - which worked before, but after the commit
said above, the "remote_host" field is not well-defined unless
RTSA_REMOTE_HOST is set.  So, change the condition to check that.

Reported-By: François Kooman <fkooman@tuxed.net>
Reported-By: Thomas Schäfer <tschaefer@t-online.de>
Trac: #1332

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/route.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Antonio Quartulli Oct. 4, 2020, 4:42 a.m. UTC | #1 
Hi,

On 02/10/2020 19:57, Gert Doering wrote:
> Commit aa34684972eb0 fixed a long-standing bug in setting the
> "route-list" flag RTSA_REMOTE_HOST for IPv4 ("we have a well-defined
> remote_host == VPN server IP address") even if connecting over IPv6.
> 
> Unfortunately the logic in redirect_default_route_to_vpn() was also
> wrong, and refused cooperation if that flag is not set, triggering
> the message
>     "NOTE: unable to redirect IPv4 default gateway -- Cannot
>      obtain current remote host address"
> 
> Correct operation: if RTSA_REMOTE_HOST is not set, or remote_host
> is IPV4_INVALID_ADDR (= 255.255.255.255), do not try to install a
> host route for continued connectivity to the VPN server - which is
> not needed when connecting over IPv6.  But the actual *routes*
> (/0 or 2 x /1) can be installed just fine.
> 
> There is a second bug here, which hits if there is no IPv4 gateway
> at all.  In that case, the same function triggers the message
>     "NOTE: unable to redirect IPv4 default gateway -- Cannot
>      read current default gateway from system"
> 
> This is caused by using "IPV4_INVALID_ADDR" as a flag for "do we
> know the remote_host?" - which worked before, but after the commit
> said above, the "remote_host" field is not well-defined unless
> RTSA_REMOTE_HOST is set.  So, change the condition to check that.
> 
> Reported-By: François Kooman <fkooman@tuxed.net>
> Reported-By: Thomas Schäfer <tschaefer@t-online.de>
> Trac: #1332
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>

The bug becomes obvious after reading commit aa34684972eb0
This fix is basically cleaning up some conditions which did not adapt to
the new logic.

Stared at the code and tested on my system with and without an IPv4
default route.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Gert Doering Oct. 4, 2020, 5:02 a.m. UTC | #2 
Patch has been applied to the master and release/2.5 branch.

commit 23e11e591347080efa3b933beca7f620dd059d5c (master)
commit 7b4f53095c761bde8c6b39cf645cade4c1c0c5d4 (release/2.5)
Author: Gert Doering
Date:   Fri Oct 2 19:57:36 2020 +0200

     Fix redirecting of IPv4 default gateway if connecting over IPv6.

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Message-Id: <20201002175736.82609-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21152.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering Oct. 12, 2020, 12:27 a.m. UTC | #3 
Hi,

On Sun, Oct 04, 2020 at 06:02:58PM +0200, Gert Doering wrote:
> Patch has been applied to the master and release/2.5 branch.
> 
> commit 23e11e591347080efa3b933beca7f620dd059d5c (master)
> commit 7b4f53095c761bde8c6b39cf645cade4c1c0c5d4 (release/2.5)
> Author: Gert Doering
> Date:   Fri Oct 2 19:57:36 2020 +0200
> 
>      Fix redirecting of IPv4 default gateway if connecting over IPv6.

Turns out that 2.4 is also affected by the problem (because the 
offending commit was applied to 2.4, because "bugfix").

Thus:

commit 4f6461b3dd79a4288580aeea0ba89e94e0251b06 (release/2.4)
Author: Gert Doering <gert@greenie.muc.de>
Date:   Fri Oct 2 19:57:36 2020 +0200

    Fix redirecting of IPv4 default gateway if connecting over IPv6.



sorry for not checking this right away.

gert
diff mbox series

Patch

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index d75aa5f4..8037d8b3 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1011,14 +1011,10 @@  redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt,
          * - we are connecting to a non-IPv4 remote host (i.e. we use IPv6)
          */
         else if (!(rl->rgi.flags & RGI_ADDR_DEFINED) && !local
-                 && (rl->spec.remote_host != IPV4_INVALID_ADDR))
+                 && (rl->spec.flags & RTSA_REMOTE_HOST))
         {
             msg(M_WARN, "%s Cannot read current default gateway from system", err);
         }
-        else if (!(rl->spec.flags & RTSA_REMOTE_HOST))
-        {
-            msg(M_WARN, "%s Cannot obtain current remote host address", err);
-        }
         else
         {
 #ifndef TARGET_ANDROID
@@ -1041,7 +1037,8 @@  redirect_default_route_to_vpn(struct route_list *rl, const struct tuntap *tt,
                 /* route remote host to original default gateway */
                 /* if remote_host is not ipv4 (ie: ipv6), just skip
                  * adding this special /32 route */
-                if (rl->spec.remote_host != IPV4_INVALID_ADDR)
+                if ((rl->spec.flags & RTSA_REMOTE_HOST)
+                    && rl->spec.remote_host != IPV4_INVALID_ADDR)
                 {
                     add_route3(rl->spec.remote_host,
                                IPV4_NETMASK_HOST,