VR: route replies to remote access VPN clients through the tunnel

systemvm/debian/opt/cloud/bin/configure.py

@@ -1216,6 +1216,75 @@ def del_l2tp_ipsec_user(self, user, obj):
 
 class CsRemoteAccessVpn(CsDataBag):
     VPNCONFDIR = "/etc/ipsec.d"
+    # Route protocol number that marks the client throw routes owned by client_routes()
+    CLIENT_ROUTE_PROTO = "147"
+
+    def client_networks(self, obj, include_local_ip=False):
+        # The client range as CIDRs, plus the router's ppp address if asked. CloudStack only refuses
+        # a range that overlaps the VPC CIDR, so a network that overlaps a subnet on the router or a
+        # site-to-site peer network is left out and logged: routing it to the tunnels and dropping it
+        # off the tunnels would take that traffic over. A static route counts only when it equals or
+        # lies inside the network; a broader one (a supernet via a private gateway) leaves it alone.
+        start, end = [ipaddress.IPv4Address(a.strip()) for a in obj['ip_range'].split('-')]
+        nets = list(ipaddress.summarize_address_range(start, end))
+        if include_local_ip:
+            nets.append(ipaddress.IPv4Network(obj['local_ip'] + '/32'))
+
+        def ipv4(cidrs):
+            return [n for n in (ipaddress.ip_network(c.strip(), strict=False) for c in cidrs if c.strip() not in ("", "ERROR")) if n.version == 4]
+
+        taken = [interface.get_network() for interface in self.config.address().get_interfaces()]
+        for peer in CsSite2SiteVpn("site2sitevpn", self.config).get_bag().values():
+            if isinstance(peer, dict) and peer.get('create', True):
+                taken += peer.get('peer_guest_cidr_list', '').split(',')
+        taken = ipv4(taken)
+        static_routes = CsStaticRoutes("staticroutes", self.config).get_bag()
+        routes = ipv4([r.get('network', '') for k, r in static_routes.items() if k != "id" and not r.get('revoke')])
+        usable = []
+        for net in nets:
+            clash = [str(t) for t in taken if net.overlaps(t)] + [str(r) for r in routes if r.subnet_of(net)]
+            if clash:
+                logging.error("VPN client network %s overlaps %s on this router, not routing it to the VPN" % (net, ", ".join(clash)))
+            else:
+                usable.append(str(net))
+        return usable
+
+    def client_routes(self):
+        # Replies the router sends to VPN clients from an address on a public NIC (an LB address,
+        # ICMP) match "from <public subnet> lookup Table_ethX" and would leave by that NIC's gateway.
+        # A throw for each client network in every per-NIC table sends them on to main, which holds
+        # the per-session ppp routes, the same way tier networks are thrown. Reconciled on every pass:
+        # NIC changes rebuild the tables, and a deleted VPN is already gone from the data bag.
+        rules = CsHelper.execute("ip rule show")
+        if not rules:
+            logging.warning("ip rule show returned nothing, leaving VPN client routes alone")
+            return
+        tables = set()
+        for line in rules:
+            m = re.search(r"\blookup (Table_eth\d+)(?=\s|$)", line)
+            if m:
+                tables.add(m.group(1))
+        nets = set()
+        for public_ip in self.dbag:
+            if public_ip == "id" or not self.dbag[public_ip].get('create'):
+                continue
+            # An entry that can't be read adds nothing, like a deleted one; its spoofing rules go too
+            try:
+                nets.update(self.client_networks(self.dbag[public_ip], include_local_ip=True))
+            except Exception as e:
+                logging.error("Failed to read the VPN client networks of %s: %s" % (public_ip, e))
+        desired = set((n, t) for n in nets for t in tables)
+        present = set()
+        for line in CsHelper.execute("ip -4 route show table all proto %s" % self.CLIENT_ROUTE_PROTO):
+            words = line.split()
+            if len(words) < 2 or words[0] != "throw":
+                continue
+            table = words[words.index("table") + 1] if "table" in words else "main"
+            present.add((str(ipaddress.IPv4Network(words[1])), table))
+        for net, table in sorted(present - desired):
+            CsHelper.execute("ip route delete throw %s table %s proto %s" % (net, table, self.CLIENT_ROUTE_PROTO))
+        for net, table in sorted(desired - present):
+            CsHelper.execute("ip route add throw %s table %s proto %s" % (net, table, self.CLIENT_ROUTE_PROTO))
 
     def process(self):
         self.confips = []
@@ -1318,6 +1387,17 @@ def remoteaccessvpn_iptables(self, publicdev, publicip, obj):
         self.fw.append(["", "", "-A INPUT -i ppp+ -m tcp -p tcp --dport 53 -j ACCEPT"])
         self.fw.append(["nat", "", "-I PREROUTING -i ppp+ -p tcp -m tcp --dport 53 -j DNAT --to-destination %s" % local_ip])
 
+        # A packet that claims a client's address must have come through a tunnel: the client throw
+        # routes would send replies to a spoofed one into that client's tunnel. No lo exception is
+        # needed: the router never owns a pool address (local_ip is outside the range and not listed)
+        try:
+            client_nets = self.client_networks(obj)
+        except Exception as e:
+            logging.error("Failed to read the VPN client networks, no spoofing rules: %s" % e)
+            client_nets = []
+        for net in client_nets:
+            self.fw.append(["mangle", "front", "-A PREROUTING -s %s ! -i ppp+ -j DROP" % net])
+
         if self.config.is_vpc():
             return
 
@@ -1670,6 +1750,12 @@ def main(argv):
     config.address().compare()
     config.address().process()
 
+    # Every pass, not only iptables passes: an ip_associations pass can rebuild a per-NIC table
+    try:
+        CsRemoteAccessVpn("remoteaccessvpn", config).client_routes()
+    except Exception as e:
+        logging.error("Failed to reconcile VPN client routes: %s" % e)
+
     databag_map = OrderedDict([("guest_network",       {"process_iptables": True,  "executor": [CsVpcGuestNetwork("guestnetwork", config)]}),
                                ("bgp_peers",           {"process_iptables": False,  "executor": [CsBgpPeers("bgppeers", config)]}),
                                ("ip_aliases",          {"process_iptables": True,  "executor": []}),

systemvm/test/TestCsRemoteAccessVpnClientRoutes.py

@@ -0,0 +1,204 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import ipaddress
+import re
+import unittest
+from unittest import mock
+
+from configure import CsRemoteAccessVpn
+
+PROTO = CsRemoteAccessVpn.CLIENT_ROUTE_PROTO
+# CloudStack's default client range: local_ip 10.1.2.1, pool 10.1.2.2-10.1.2.8
+CLIENT_NETS = ["10.1.2.1/32", "10.1.2.2/31", "10.1.2.4/30", "10.1.2.8/32"]
+POOL_NETS = ["10.1.2.2/31", "10.1.2.4/30", "10.1.2.8/32"]
+
+
+class FakeKernel:
+    """Policy rules and routes (type, dst, table, proto), with output shaped like iproute2's."""
+
+    def __init__(self):
+        self.rules = ["0:\tfrom all lookup local", "220:\tfrom all lookup 220",
+                      "32759:\tfrom 198.51.100.128/25 lookup Table_eth2", "32760:\tfrom all fwmark 0x67 lookup Table_eth3",
+                      "32761:\tfrom 203.0.113.0/28 lookup Table_eth1", "32762:\tfrom all fwmark 0x66 lookup Table_eth2",
+                      "32763:\tfrom all fwmark 0x65 lookup Table_eth1", "32766:\tfrom all lookup main"]
+        self.routes = {("throw", "198.51.100.128/25", "Table_eth2", "static"),
+                       ("throw", "192.0.2.0/24", "Table_eth2", "static"),
+                       ("throw", "192.0.2.0/24", "Table_eth1", "static"),
+                       # not ours, must never be touched
+                       ("throw", "203.0.113.128/25", "Table_eth9", "static"),
+                       ("10.1.2.2 dev ppp0", None, "main", "kernel")}
+        self.calls = []
+
+    def execute(self, cmd):
+        self.calls.append(cmd)
+        if cmd == "ip rule show":
+            return list(self.rules)
+        if cmd == "ip -4 route show table all proto %s" % PROTO:
+            out = []
+            for typ, dst, table, proto in sorted(self.routes, key=str):
+                if proto == PROTO:
+                    dst = dst[:-3] if dst.endswith("/32") else dst
+                    out.append("%s %s%s" % (typ, dst, "" if table == "main" else " table " + table))
+            return out
+        m = re.fullmatch(r"ip route (add|delete) throw (\S+) table (\S+) proto (\d+)", cmd)
+        if not m:
+            raise AssertionError("unexpected command: " + cmd)
+        route = ("throw", str(ipaddress.ip_network(m.group(2))), m.group(3), m.group(4))
+        if m.group(1) == "add":
+            if any(r[:3] == route[:3] for r in self.routes):
+                raise AssertionError("route exists: " + cmd)
+            self.routes.add(route)
+        else:
+            self.routes.remove(route)
+        return []
+
+    def owned(self):
+        return {(dst, table) for typ, dst, table, proto in self.routes if proto == PROTO}
+
+    def foreign(self):
+        return {r for r in self.routes if r[3] != PROTO}
+
+    def changes(self):
+        return [c for c in self.calls if c.startswith("ip route ")]
+
+
+class FakeConfig:
+
+    def __init__(self, networks):
+        self.networks = networks
+
+    def is_vpc(self):
+        return True
+
+    def address(self):
+        interfaces = [mock.Mock(**{"get_network.return_value": n}) for n in self.networks]
+        return mock.Mock(**{"get_interfaces.return_value": interfaces})
+
+
+class TestCsRemoteAccessVpnClientRoutes(unittest.TestCase):
+
+    def setUp(self):
+        self.kernel = FakeKernel()
+        self.static_routes = {"id": "staticroutes"}
+        self.peers = {"id": "site2sitevpn"}
+        self.vpn_entry = {"create": True, "local_ip": "10.1.2.1", "ip_range": "10.1.2.2-10.1.2.8",
+                          "local_cidr": "192.0.2.0/24", "public_interface": "eth1"}
+        self.dbag = {"id": "remoteaccessvpn", "203.0.113.10": self.vpn_entry}
+        self.config = FakeConfig(["203.0.113.0/28", "198.51.100.128/25", "192.0.2.0/24"])
+        for name, value in (("CsHelper.execute", self.kernel.execute),
+                            ("CsStaticRoutes", lambda *a: mock.Mock(**{"get_bag.return_value": self.static_routes})),
+                            ("CsSite2SiteVpn", lambda *a: mock.Mock(**{"get_bag.return_value": self.peers}))):
+            patcher = mock.patch("configure." + name, value)
+            patcher.start()
+            self.addCleanup(patcher.stop)
+        self.foreign = self.kernel.foreign()
+
+    def vpn(self):
+        vpn = CsRemoteAccessVpn.__new__(CsRemoteAccessVpn)
+        vpn.dbag = self.dbag
+        vpn.config = self.config
+        vpn.fw = []
+        return vpn
+
+    def run_pass(self):
+        self.kernel.calls = []
+        self.vpn().client_routes()
+        self.assertEqual(self.kernel.foreign(), self.foreign)
+        return self.kernel.changes()
+
+    def expected(self, nets=CLIENT_NETS, tables=("Table_eth1", "Table_eth2", "Table_eth3")):
+        return {(n, t) for n in nets for t in tables}
+
+    def test_throws_in_every_nic_table(self):
+        self.assertEqual(len(self.run_pass()), 12)
+        self.assertEqual(self.kernel.owned(), self.expected())
+        self.assertEqual(self.run_pass(), [])
+
+    def test_rebuilt_table_gets_its_throws_back(self):
+        self.run_pass()
+        self.kernel.routes = {r for r in self.kernel.routes if r[2] != "Table_eth2"}
+        self.foreign = self.kernel.foreign()
+        self.assertEqual(len(self.run_pass()), 4)
+        self.assertEqual(self.kernel.owned(), self.expected())
+
+    def test_new_nic_table(self):
+        self.run_pass()
+        self.kernel.rules.insert(2, "219:\tfrom 198.51.100.0/26 lookup Table_eth4")
+        self.assertEqual(len(self.run_pass()), 4)
+        self.assertEqual(self.kernel.owned(), self.expected(tables=("Table_eth1", "Table_eth2", "Table_eth3", "Table_eth4")))
+
+    def test_vpn_removed(self):
+        self.run_pass()
+        del self.dbag["203.0.113.10"]
+        self.assertEqual(len(self.run_pass()), 12)
+        self.assertEqual(self.kernel.owned(), set())
+
+    def test_vpn_disabled_entry_left_in_bag(self):
+        self.run_pass()
+        self.vpn_entry["create"] = False
+        self.run_pass()
+        self.assertEqual(self.kernel.owned(), set())
+
+    def test_range_changed(self):
+        self.run_pass()
+        self.vpn_entry["ip_range"] = "10.1.2.2-10.1.2.3"
+        self.run_pass()
+        self.assertEqual(self.kernel.owned(), self.expected(nets=["10.1.2.1/32", "10.1.2.2/31"]))
+
+    def test_no_rules_read_changes_nothing(self):
+        self.run_pass()
+        self.kernel.rules = []
+        self.assertEqual(self.run_pass(), [])
+        self.assertEqual(self.kernel.owned(), self.expected())
+
+    def test_unreadable_entry_adds_nothing(self):
+        self.vpn_entry["ip_range"] = "not-a-range"
+        self.run_pass()
+        self.assertEqual(self.kernel.owned(), set())
+
+    def test_overlapping_networks_are_skipped(self):
+        # a subnet on the router, a site-to-site peer network and a static route inside the pool
+        self.config.networks.append("10.1.2.8/32")
+        self.peers["198.51.100.1"] = {"create": True, "peer_guest_cidr_list": "192.0.2.128/25, 10.1.2.2/32"}
+        self.static_routes["10.1.2.4/31"] = {"network": "10.1.2.4/31", "revoke": False}
+        self.run_pass()
+        self.assertEqual(self.kernel.owned(), self.expected(nets=["10.1.2.1/32"]))
+        # a removed peer and a revoked route don't count
+        self.config.networks.pop()
+        self.peers["198.51.100.1"]["create"] = False
+        self.static_routes["10.1.2.4/31"]["revoke"] = True
+        self.run_pass()
+        self.assertEqual(self.kernel.owned(), self.expected())
+
+    def test_static_route_containing_the_pool_does_not_count(self):
+        self.vpn_entry["local_ip"] = "203.0.113.65"
+        self.vpn_entry["ip_range"] = "203.0.113.66-203.0.113.72"
+        self.static_routes["203.0.113.64/26"] = {"network": "203.0.113.64/26", "revoke": False}
+        self.run_pass()
+        nets = ["203.0.113.66/31", "203.0.113.68/30", "203.0.113.72/32", "203.0.113.65/32"]
+        self.assertEqual(self.kernel.owned(), self.expected(nets=nets))
+
+    def test_spoofed_client_sources_are_dropped(self):
+        vpn = self.vpn()
+        vpn.remoteaccessvpn_iptables("eth1", "203.0.113.10", self.vpn_entry)
+        drops = [fw for fw in vpn.fw if fw[0] == "mangle" and fw[2].endswith("! -i ppp+ -j DROP")]
+        self.assertEqual(drops, [["mangle", "front", "-A PREROUTING -s %s ! -i ppp+ -j DROP" % n] for n in POOL_NETS])
+
+
+if __name__ == '__main__':
+    unittest.main()