New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 821236 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Split-tunnel OpenVPN stopped routing regular non-VPN traffic

Reported by michi...@gmail.com, Mar 13 2018

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS x86_64 10176.76.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.190 Safari/537.36
Platform: 10176.76.0 (Official Build) stable-channel eve

Example URL:

Steps to reproduce the problem:
1. Setup a VPN through an .onc config via chrome://net-internals#chromeos

{
"Type": "UnencryptedConfiguration",
"NetworkConfigurations": [
  {
    "GUID": "{the-vpn-20180312181236}",
    "Name": "The VPN",
    "Type": "VPN",
    "VPN": {
      "Type": "OpenVPN",
      "Host": "vpn-host.tld",
      "OpenVPN": {
        "Auth": "SHA256",
        "AuthRetry": "interact",
        "AuthNoCache": true,
        "Cipher": "AES-256-CBC",
        "ClientCertRef": "{clientcert}",
        "ClientCertType": "Ref",
        "CompLZO": "false",
        "IgnoreDefaultRoute": true,
        "KeyDirection": "1",
        "NsCertType": "server",
        "Port": 16770,
        "Proto": "udp",
        "RemoteCertEKU": "TLS Web Server Authentication",
        "RemoteCertTLS": "server",
        "RenegSec": 0,
        "SaveCredentials": false,
        "ServerCARefs": [ "{cacert}" ],
        "ServerPollTimeout": 360,
        "TLSAuthContents": "-----BEGIN OpenVPN Static key V1-----\n[...]\n-----END OpenVPN Static key V1-----\n",
        "UserAuthenticationType": "Password",
        "Username": "${LOGIN_ID}",
        "Verb": "3",
        "VerifyX509": {
          "Name": "vpn-host.tld",
          "Type": "name"
        }
      }
    }
  }
],
"Certificates": [
{
  "GUID": "{cacert}",
  "Type": "Authority",
  "X509":
"[...]"},
{
  "GUID": "{clientcert}",
  "Type": "Client",
  "PKCS12":
"[...]"}
]
}

2. The VPN should be configured to push down routes (e.g. split-tunnel).

3. Connect to the VPN via the VPN connection UI. This is expected to succeed.

3. Try to resolve a host not part of the routes pushed by the split-tunnel VPN, e.g. google.com. The host will not resolve and it will not be able to find a route to any IPs outside the routes pushed by the split-tunnel VPN.

What is the expected behavior?
Hosts outside the VPNs pushed routes should be resolvable and reachable.

What went wrong?
Right after the update from https://chromereleases.googleblog.com/2018/03/stable-channel-update-for-chrome-os.html rolled out to my Chromebook Pixel (2017), my split-tunnel VPN stopped working. As soon as I connect to the split-tunnel VPN, I am not able to resolve nor reach any hosts outside the VPN's split-tunnel routes.

Did this work before? Yes 

Chrome version: 64.0.3282.190  Channel: stable
OS Version: 10176.76.0
Flash Version:
 

Comment 1 by mattm@chromium.org, Mar 13 2018

Components: -Internals>Network Internals>Network>VPN

Comment 2 by h...@stripe.com, Mar 13 2018

We saw a similar issue last week with our OpenVPN configurations, specifically on 64.0.3282.190. The previous version, 64.0.3282.169, still worked. We spent some time digging through shill and found this commit: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/6e37823dc2c4debb84cbb37dd97bfa358f2d0682

The likely cause here is that your OpenVPN config is using the "net30" topology, which looks a lot like a PPP connection and is affected by the shill commit above. Using the "subnet" topology (via "topology subnet" in your openvpn.conf) should allow your VPN to continue working.

One issue we had while diagnosing this was that the "route" command no longer returns useful data on 64.0.3282.190. While our split tunneling works again, there doesn't appear to be a place where we can see the routes that Chrome OS has in place (and it would be really nice if we could find that debugging info somewhere).
> The host will not resolve and it will not be able to find a route to any IPs outside the routes pushed by the split-tunnel VPN.

I think this is a side effect of  bug 820781 .

i.e. OpenVPN connections that were configured for split tunnel are incorrectly treated as full tunnel connections.

The shill commit linked above is involved: OpenVPN sets properties->peer_address, triggering the newly-added logic.  The expectation was that only PPP connections, which can't support split tunneling, would set a peer address.

> The likely cause here is that your OpenVPN config is using the "net30" topology

Right, --topology net30 or p2p will set the ifconfig_remote variable, which sets properties->peer_address.  --topology subnet will push the ifconfig_subnet variable instead.

I'm leaning toward changing openvpn_driver.cc so that the ifconfig_remote variable and redirect-gateway option just create entries in properties->routes.  The gateway for each route will always be the local IP.  This will make IgnoreDefaultRoute unnecessary for most split tunnel users.  Treating the peer and gateway IP addresses as different fields with different semantics causes a lot of unexpected interactions on VPNs.

> One issue we had while diagnosing this was that the "route" command no longer returns useful data

Oops, I guess this only shows RT_TABLE_MAIN.  Thanks for the heads up.

From the command line in dev mode I've used `ip rule list` and `ip route show table N` (usually 1) to see the per-interface routing table.  Will look into having crosh iterate through every routing table defined by `ip rule list`.
Cc: cernekee@chromium.org
 Issue 820781  has been merged into this issue.
Cc: bhthompson@chromium.org
Labels: -Pri-2 M-64 Pri-1
First CL is out for review:

https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/967564

If there's a certain openvpn server configuration you'd like me to try it with, please mention it in this bug or email me privately.  So far I have tried --topology {net30,p2p,subnet} and tried with/without --redirect-gateway.  Absent a --redirect-gateway directive, the new code will not set a default route.

(You can also test it yourself locally if you switch to canary channel after the CL lands.)
Summary: Split-tunnel OpenVPN stopped routing regular non-VPN traffic (was: Split-tunnel VPN stopped routing regular non-VPN traffic)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/bf05ce7c486a89d576b1e1648226e92c15a7bfb0

commit bf05ce7c486a89d576b1e1648226e92c15a7bfb0
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Thu Mar 22 03:48:33 2018

debugd: Display all routing tables, not just RT_TABLE_MAIN

Chrome OS now uses routing policy rules + multiple routing tables.
Display the whole routing configuration when the user runs `route`
in crosh.

BUG= chromium:821236 
TEST=manually run `route` command while VPN is connected

Change-Id: I249625c315c976a06d1db5095315ae5df7c5d6de
Reviewed-on: https://chromium-review.googlesource.com/972354
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/bf05ce7c486a89d576b1e1648226e92c15a7bfb0/debugd/src/route_tool.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/b341a0f6552dcf6e48ca4829bdae3c88a5be7a23

commit b341a0f6552dcf6e48ca4829bdae3c88a5be7a23
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Mar 27 03:16:43 2018

shill: Add OpenVPN peer and subnet addresses to per-device routing tables

In split tunnel configurations, OpenVPN can push a number of different
IP addresses depending on the --topology setting:

 - A peer address (tun0 point-to-point IP)
 - A subnet range (e.g. 192.168.1.0/24)
 - A default gateway address (next hop)
 - Per-route gateway addresses

Add explicit routes to |properties->routes| for the former two items.
Ignore all gateway addresses: they aren't needed in order to send
traffic out through tun0, and if they're wrong, they can cause the
kernel to reject the route.  Instead, specify the local IP as the
gateway for each route.

Also, change IgnoreDefaultRoute so that it makes shill ignore the
redirect-gateway option.  In the past it was usually necessary to
specify this option in order to make split tunnel VPNs work properly,
but it shouldn't be necessary anymore.

BUG= chromium:370460 
BUG= chromium:821236 
TEST=manually test net30, p2p, and subnet toplogies with and without
     redirect-gateway and pushed routes

Change-Id: I73818b21111acf6739defb7337e84c0888567868
Reviewed-on: https://chromium-review.googlesource.com/967564
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/b341a0f6552dcf6e48ca4829bdae3c88a5be7a23/vpn/openvpn_driver.cc
[modify] https://crrev.com/b341a0f6552dcf6e48ca4829bdae3c88a5be7a23/vpn/openvpn_driver_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/40dcb73b1321f2df14f417b7a963815189fd2842

commit 40dcb73b1321f2df14f417b7a963815189fd2842
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Mar 27 21:58:16 2018

debugd: Remove extra blank line from GetOutputLines()

If the input file contains "a\nb\n", base::SplitString() will return a
vector {"a", "b", ""} because it treats "\n" as a delimiter, not a line
terminator.  Removing the final "\n" fixes this.

BUG= chromium:821236 
TEST=manually run `route` in crosh

Change-Id: I46f45e58195e7f3fe230d596fe74f366df676bf9
Reviewed-on: https://chromium-review.googlesource.com/972355
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/40dcb73b1321f2df14f417b7a963815189fd2842/debugd/src/process_with_output.cc
[modify] https://crrev.com/40dcb73b1321f2df14f417b7a963815189fd2842/debugd/src/route_tool.cc

Labels: Merge-Request-66
Project Member

Comment 12 by sheriffbot@chromium.org, Apr 2 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 3 2018

Labels: merge-merged-release-R66-10452.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/7d5406946675cb0d626fc1257f030ebcc7a565fd

commit 7d5406946675cb0d626fc1257f030ebcc7a565fd
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Apr 03 18:40:16 2018

shill: Add OpenVPN peer and subnet addresses to per-device routing tables

In split tunnel configurations, OpenVPN can push a number of different
IP addresses depending on the --topology setting:

 - A peer address (tun0 point-to-point IP)
 - A subnet range (e.g. 192.168.1.0/24)
 - A default gateway address (next hop)
 - Per-route gateway addresses

Add explicit routes to |properties->routes| for the former two items.
Ignore all gateway addresses: they aren't needed in order to send
traffic out through tun0, and if they're wrong, they can cause the
kernel to reject the route.  Instead, specify the local IP as the
gateway for each route.

Also, change IgnoreDefaultRoute so that it makes shill ignore the
redirect-gateway option.  In the past it was usually necessary to
specify this option in order to make split tunnel VPNs work properly,
but it shouldn't be necessary anymore.

BUG= chromium:370460 
BUG= chromium:821236 
TEST=manually test net30, p2p, and subnet toplogies with and without
     redirect-gateway and pushed routes

Change-Id: I73818b21111acf6739defb7337e84c0888567868
Reviewed-on: https://chromium-review.googlesource.com/967564
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>
(cherry picked from commit b341a0f6552dcf6e48ca4829bdae3c88a5be7a23)

[modify] https://crrev.com/7d5406946675cb0d626fc1257f030ebcc7a565fd/vpn/openvpn_driver.cc
[modify] https://crrev.com/7d5406946675cb0d626fc1257f030ebcc7a565fd/vpn/openvpn_driver_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/875ed42e32ba844353583d288d31f2427b96b3b0

commit 875ed42e32ba844353583d288d31f2427b96b3b0
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Apr 03 18:41:38 2018

debugd: Display all routing tables, not just RT_TABLE_MAIN

Chrome OS now uses routing policy rules + multiple routing tables.
Display the whole routing configuration when the user runs `route`
in crosh.

BUG= chromium:821236 
TEST=manually run `route` command while VPN is connected

Change-Id: I249625c315c976a06d1db5095315ae5df7c5d6de
Reviewed-on: https://chromium-review.googlesource.com/972354
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
(cherry picked from commit bf05ce7c486a89d576b1e1648226e92c15a7bfb0)
Reviewed-on: https://chromium-review.googlesource.com/993355
Trybot-Ready: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/875ed42e32ba844353583d288d31f2427b96b3b0/debugd/src/route_tool.cc

Labels: -Merge-Approved-66
Owner: cernekee@chromium.org
Status: Fixed (was: Unconfirmed)
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a0fa8dfa4a0bb7dd4cd0aacaef1a88c9d4b890f5

commit a0fa8dfa4a0bb7dd4cd0aacaef1a88c9d4b890f5
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Apr 03 18:42:22 2018

debugd: Remove extra blank line from GetOutputLines()

If the input file contains "a\nb\n", base::SplitString() will return a
vector {"a", "b", ""} because it treats "\n" as a delimiter, not a line
terminator.  Removing the final "\n" fixes this.

BUG= chromium:821236 
TEST=manually run `route` in crosh

Change-Id: I46f45e58195e7f3fe230d596fe74f366df676bf9
Reviewed-on: https://chromium-review.googlesource.com/972355
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kevin Cernekee <cernekee@chromium.org>
(cherry picked from commit 4576fe7dbf193dec0a27e2ac0a884b31b11df840)
Reviewed-on: https://chromium-review.googlesource.com/993356
Trybot-Ready: Kevin Cernekee <cernekee@chromium.org>

[modify] https://crrev.com/a0fa8dfa4a0bb7dd4cd0aacaef1a88c9d4b890f5/debugd/src/process_with_output.cc
[modify] https://crrev.com/a0fa8dfa4a0bb7dd4cd0aacaef1a88c9d4b890f5/debugd/src/route_tool.cc

Sign in to add a comment