Issue metadata
Sign in to add a comment
|
L2TP/IPsec VPN doesn't pass traffic if PPP remote IP is the gateway's public IP |
||||||||||||||||||||||
Issue descriptionOn PPP interfaces, shill tries to manage the IPv4 and routing configuration itself through Connection::UpdateFromIPConfig(). We run a modified version of pppd that includes a `nosystemconfig` option that inhibits pppd from configuring the interface. Unfortunately, despite this hack, the kernel still adds a /32 route to the PPP remote IP address (peer address) in the `main` routing table when shill calls RTNLHandler::AddInterfaceAddress(). As of M64 we are using per-device routing tables rather than adding VPN routes to the `main` table, so this is undesirable, but usually not fatal. i.e. VPN gateway = 45.252.191.3 EXTERNAL_IP4_ADDRESS = 172.21.18.1 (p2p IP) INTERNAL_IP4_ADDRESS = 172.21.18.2 (my IP) So `ip route show table main` would show a route to 172.21.18.1 via ppp0. In the typical case this is mostly harmless. However, some VPN providers, such as IPVanish, set the peer address to the *public* IP of the VPN gateway: VPN gateway = 45.252.191.3 EXTERNAL_IP4_ADDRESS = 45.252.191.3 INTERNAL_IP4_ADDRESS = 172.20.8.1 When the kernel adds an implicit route to this IP via ppp0, it creates a routing loop. Encrypted traffic that should egress through the LAN interface will instead be sent to the ppp0 interface, and it will never reach the internet. This has generated a number of user feedback reports, many of which explicitly mention IPVanish: https://listnr.corp.google.com/product/208/report/85076123630 https://listnr.corp.google.com/product/208/report/85074393240 https://listnr.corp.google.com/product/208/report/85046241299 https://listnr.corp.google.com/product/208/report/85006375990 https://listnr.corp.google.com/product/208/report/85030153336
,
Feb 17 2018
Fix is verified on 10413.0.0 canary -- let's merge to M64 and M65 ASAP. I also wrote a new autotest case to catch this error: https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/924764
,
Feb 17 2018
This bug requires manual review: M65 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), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 18 2018
Hi Kevin, we have a stable RC that just passed tests.. Yes, I see this is a P0, but impact to M64 if we delay since we're well into stable? How large do you think the scope impact is? Note that I'm out of town for a family emergency. Josafat will cover until I return on Wednesday.
,
Feb 18 2018
We have UMA stats on how many users L2TP/IPsec has, but unfortunately we do not have stats or estimates that would tell us how many L2TP/IPsec users are connecting to a gateway that has this configuration. You can see the relevant feedback reports at http://g/cros-vpn-feedback
,
Feb 20 2018
,
Feb 20 2018
,
Feb 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/17894ed02cec6c0671651c2492dc8be7c5ecc7b7 commit 17894ed02cec6c0671651c2492dc8be7c5ecc7b7 Author: Kevin Cernekee <cernekee@chromium.org> Date: Tue Feb 20 20:13:32 2018 shill: Fix PPP routes when remote IP points to the public gateway IP Prior to the introduction of per-device routing tables, Connection::FixGatewayReachability() used to fix the following scenario: PPP Local IP = private (VPN) IP -> local address on ppp0 PPP Remote IP = public VPN gateway IP -> p2p address on ppp0 PPP Gateway IP = public VPN gateway IP -> route next hop It was believed that this fix was unnecessary when per-device routing tables were used, because they have an explicit RTN_THROW entry for the public VPN gateway IP. However, it turns out that the kernel automatically generates a route in RT_TABLE_MAIN for a point-to-point address if it is specified (|peer| in RTNLHandler::AddressRequest()). This inadvertently creates a routing loop on certain L2TP/IPsec VPN setups. Since the peer address and gateway are unnecessary for PPP connections that use per-device routing tables, just zero them out. BUG= chromium:813199 TEST=manually connect to ipvanish VPN (which shows the problem) TEST=manually connect to a locally-administered strongSwan setup (which doesn't) TEST=unit tests TEST=autotests Change-Id: Idcbfb0310fff3ed6828cc3c04f160d48d3db1029 Reviewed-on: https://chromium-review.googlesource.com/924471 Commit-Ready: Kevin Cernekee <cernekee@chromium.org> Tested-by: Kevin Cernekee <cernekee@chromium.org> Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org> (cherry picked from commit 6e37823dc2c4debb84cbb37dd97bfa358f2d0682) [modify] https://crrev.com/17894ed02cec6c0671651c2492dc8be7c5ecc7b7/connection.cc
,
Feb 20 2018
Marking as fixed to confirm (verify) this is working properly in tot. We can revisit the merge once verification is done
,
Feb 20 2018
+harpreet
,
Feb 21 2018
aashutoshk@ to verify that this did not cause any other regressions.
,
Feb 22 2018
Verification status?
,
Feb 22 2018
As per comment# 8 @cernekee tested on ipvanish VPN (which was seeing this issue). I verified, the change has not caused any regression on our test vpn services. (ipsec-psk, ipsec-rsa and openvpn) Cave M66-10421.0.0
,
Feb 22 2018
,
Feb 22 2018
Can we get this merged to M64 soon? Hoping to create a RC... Thanks!
,
Feb 22 2018
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/82aaa8a09427449aaa01505f9869263f9dff341d commit 82aaa8a09427449aaa01505f9869263f9dff341d Author: Kevin Cernekee <cernekee@chromium.org> Date: Thu Feb 22 02:30:13 2018 shill: Fix PPP routes when remote IP points to the public gateway IP Prior to the introduction of per-device routing tables, Connection::FixGatewayReachability() used to fix the following scenario: PPP Local IP = private (VPN) IP -> local address on ppp0 PPP Remote IP = public VPN gateway IP -> p2p address on ppp0 PPP Gateway IP = public VPN gateway IP -> route next hop It was believed that this fix was unnecessary when per-device routing tables were used, because they have an explicit RTN_THROW entry for the public VPN gateway IP. However, it turns out that the kernel automatically generates a route in RT_TABLE_MAIN for a point-to-point address if it is specified (|peer| in RTNLHandler::AddressRequest()). This inadvertently creates a routing loop on certain L2TP/IPsec VPN setups. Since the peer address and gateway are unnecessary for PPP connections that use per-device routing tables, just zero them out. BUG= chromium:813199 TEST=manually connect to ipvanish VPN (which shows the problem) TEST=manually connect to a locally-administered strongSwan setup (which doesn't) TEST=unit tests TEST=autotests Change-Id: Idcbfb0310fff3ed6828cc3c04f160d48d3db1029 Reviewed-on: https://chromium-review.googlesource.com/924471 Commit-Ready: Kevin Cernekee <cernekee@chromium.org> Tested-by: Kevin Cernekee <cernekee@chromium.org> Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org> (cherry picked from commit 6e37823dc2c4debb84cbb37dd97bfa358f2d0682) [modify] https://crrev.com/82aaa8a09427449aaa01505f9869263f9dff341d/connection.cc
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/f5ac82aad1ed3b166e93abd7d2e1c57716683303 commit f5ac82aad1ed3b166e93abd7d2e1c57716683303 Author: Kevin Cernekee <cernekee@chromium.org> Date: Thu Feb 22 03:52:01 2018 network_VPNConnect: Add test case for PPP remote IP == public gateway IP Normally the PPP remote IP should be an "internal" VPN IP, but some providers use the gateway's public IP instead. This test makes sure that Chrome OS doesn't try to route traffic to the gateway's public IP through the tunnel, as this would cause a routing loop. BUG= chromium:813199 TEST=test_that and then manually check pppd IPs in /var/log/net.log TEST=verify that 10411.0.0 (not containing CL:924471) fails ping test TEST=verify that 10413.0.0 (containing CL:924471) passes Change-Id: I56a69becbeaff0d46a09f271702af1957e620415 Reviewed-on: https://chromium-review.googlesource.com/924764 Commit-Ready: Kevin Cernekee <cernekee@chromium.org> Tested-by: Kevin Cernekee <cernekee@chromium.org> Reviewed-by: Harpreet Grewal <harpreet@chromium.org> [modify] https://crrev.com/f5ac82aad1ed3b166e93abd7d2e1c57716683303/client/cros/vpn_server.py [modify] https://crrev.com/f5ac82aad1ed3b166e93abd7d2e1c57716683303/client/site_tests/network_VPNConnect/control.l2tpipsec_psk [modify] https://crrev.com/f5ac82aad1ed3b166e93abd7d2e1c57716683303/client/site_tests/network_VPNConnect/network_VPNConnect.py
,
Mar 2 2018
Issue 817736 has been merged into this issue.
,
Mar 2 2018
IPVanish has been notified of the workaround (Ticket 283686). On Feb 20 they told me that the information was forwarded to their DevOps team. Not sure if it has been rolled out yet. The Chrome OS fix should be in the next M64 stable build (if any).
,
Mar 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/14f57a40768c1ac2094b187874b225c8e9d44416 commit 14f57a40768c1ac2094b187874b225c8e9d44416 Author: Kevin Cernekee <cernekee@chromium.org> Date: Mon Mar 12 16:34:19 2018 network_VPNConnect: Add test case for PPP remote IP == public gateway IP Normally the PPP remote IP should be an "internal" VPN IP, but some providers use the gateway's public IP instead. This test makes sure that Chrome OS doesn't try to route traffic to the gateway's public IP through the tunnel, as this would cause a routing loop. BUG= chromium:813199 TEST=test_that and then manually check pppd IPs in /var/log/net.log TEST=verify that 10411.0.0 (not containing CL:924471) fails ping test TEST=verify that 10413.0.0 (containing CL:924471) passes Change-Id: I56a69becbeaff0d46a09f271702af1957e620415 Reviewed-on: https://chromium-review.googlesource.com/924764 Commit-Ready: Kevin Cernekee <cernekee@chromium.org> Tested-by: Kevin Cernekee <cernekee@chromium.org> Reviewed-by: Harpreet Grewal <harpreet@chromium.org> (cherry picked from commit f5ac82aad1ed3b166e93abd7d2e1c57716683303) Reviewed-on: https://chromium-review.googlesource.com/959183 Reviewed-by: Kevin Cernekee <cernekee@chromium.org> Commit-Queue: Kevin Cernekee <cernekee@chromium.org> [modify] https://crrev.com/14f57a40768c1ac2094b187874b225c8e9d44416/client/cros/vpn_server.py [modify] https://crrev.com/14f57a40768c1ac2094b187874b225c8e9d44416/client/site_tests/network_VPNConnect/control.l2tpipsec_psk [modify] https://crrev.com/14f57a40768c1ac2094b187874b225c8e9d44416/client/site_tests/network_VPNConnect/network_VPNConnect.py |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by bugdroid1@chromium.org
, Feb 17 2018