diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 0eb16b2..a0fd31a 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -11810,6 +11810,19 @@ TRANSLATIONS = { " state a dead tunnel left behind." ), }, + "The gateway decides what enters this tunnel. To force a network" + " through it anyway, add it to the routes above \u2014 0.0.0.0/0 for all.": { + "fr": ( + "La passerelle décide de ce qui entre dans ce tunnel. Pour y" + " forcer un réseau tout de même, l'ajouter aux routes" + " ci-dessus — 0.0.0.0/0 pour tout." + ), + "en": ( + "The gateway decides what enters this tunnel. To force a" + " network through it anyway, add it to the routes above —" + " 0.0.0.0/0 for all." + ), + }, "SSO helper": { "fr": "greffon SSO", "en": "SSO helper", diff --git a/script/todo/vpn_menu.py b/script/todo/vpn_menu.py index 2dda221..b9cea94 100644 --- a/script/todo/vpn_menu.py +++ b/script/todo/vpn_menu.py @@ -117,6 +117,14 @@ NOT_CONNECTED_NOTE = ( " a dead tunnel left behind." ) +# Ce qu'on dit à la place de la question « tout le trafic ? », pour un +# pilote dont le SERVEUR décide du routage. Nomme l'échappatoire, sans quoi +# le retrait de la question se lit comme une capacité perdue. +SERVER_ROUTES_NOTE = ( + "The gateway decides what enters this tunnel. To force a network" + " through it anyway, add it to the routes above — 0.0.0.0/0 for all." +) + MASTER_PASSWORD_WARNING = ( "The vault MASTER password is stored in the configuration in clear" " text. Remove it and type it on demand." @@ -566,10 +574,17 @@ class VpnMenuMixin: t("Networks to reach, comma-separated"), ", ".join(draft.get("routes", [])), ) - draft["default_route"] = self._vpn_ask_flag( - t("Send ALL traffic through the tunnel?"), - draft.get("default_route", False), - ) + if driver_cls.uses_default_route: + draft["default_route"] = self._vpn_ask_flag( + t("Send ALL traffic through the tunnel?"), + draft.get("default_route", False), + ) + else: + # Ni demandée, ni conservée : un drapeau laissé à vrai sur un + # pilote qui l'ignore reste un champ sans effet, et le profil + # continuerait d'annoncer « tout le trafic » à la liste. + draft["default_route"] = False + print(f" {t(SERVER_ROUTES_NOTE)}") draft["probe"] = self._vpn_ask( t("Witness address reachable only through the tunnel (optional)"), draft.get("probe", ""), diff --git a/script/vpn/README.base.md b/script/vpn/README.base.md index ae13a87..0d831bc 100644 --- a/script/vpn/README.base.md +++ b/script/vpn/README.base.md @@ -605,8 +605,16 @@ says so when it takes it. unknown server certificate raises a question, and openconnect would read the answer from the standard input the password arrives on. With it, openconnect refuses at once **and** prints the `--servercert sha256:…` line to paste into -the profile's `oc_servercert`. Routes belong to the server, through -`vpnc-script`; the profile can add to them, not replace them. +the profile's `oc_servercert`. + +Routes belong to the server, through `vpnc-script`; the profile can add to +them, not replace them. For that reason the form does **not** ask this +driver « send ALL traffic through the tunnel? », and `status` does not judge +it: the gateway decides what enters the tunnel, and forcing a default route +against a split-tunnel gateway would not give all traffic but a black hole — +a gateway does not route what it never advertised. To force a network +through anyway, add it to `routes`, which this driver does honour, with +`0.0.0.0/0` for everything. Set **`oc_sso`** when the concentrator authenticates through a **web form** (SAML / SSO — Azure AD, Okta, Duo). There is then no password to send, and @@ -695,7 +703,14 @@ sur l'entrée standard par laquelle arrive le mot de passe. Avec lui, openconnect refuse tout de suite **et** imprime la ligne `--servercert sha256:…` à recopier dans le champ `oc_servercert` du profil. Les routes appartiennent au serveur, via `vpnc-script` ; le profil peut en -ajouter, pas les remplacer. +ajouter, pas les remplacer. Pour cette raison le formulaire ne demande +**pas** à ce pilote « envoyer TOUT le trafic dans le tunnel ? », et `status` +ne le juge pas : c'est la passerelle qui décide de ce qui entre dans le +tunnel, et forcer une route par défaut contre une passerelle en tunnel +scindé ne donnerait pas tout le trafic mais un trou noir — une passerelle ne +route pas ce qu'elle n'a jamais annoncé. Pour y forcer un réseau tout de +même, l'ajouter à `routes`, que ce pilote honore, avec `0.0.0.0/0` pour +tout. Cocher **`oc_sso`** quand le concentrateur authentifie par un **formulaire web** (SAML / SSO — Azure AD, Okta, Duo). Il n'y a alors aucun mot de passe à diff --git a/script/vpn/README.fr.md b/script/vpn/README.fr.md index f062551..4aaf57d 100644 --- a/script/vpn/README.fr.md +++ b/script/vpn/README.fr.md @@ -340,7 +340,14 @@ sur l'entrée standard par laquelle arrive le mot de passe. Avec lui, openconnect refuse tout de suite **et** imprime la ligne `--servercert sha256:…` à recopier dans le champ `oc_servercert` du profil. Les routes appartiennent au serveur, via `vpnc-script` ; le profil peut en -ajouter, pas les remplacer. +ajouter, pas les remplacer. Pour cette raison le formulaire ne demande +**pas** à ce pilote « envoyer TOUT le trafic dans le tunnel ? », et `status` +ne le juge pas : c'est la passerelle qui décide de ce qui entre dans le +tunnel, et forcer une route par défaut contre une passerelle en tunnel +scindé ne donnerait pas tout le trafic mais un trou noir — une passerelle ne +route pas ce qu'elle n'a jamais annoncé. Pour y forcer un réseau tout de +même, l'ajouter à `routes`, que ce pilote honore, avec `0.0.0.0/0` pour +tout. Cocher **`oc_sso`** quand le concentrateur authentifie par un **formulaire web** (SAML / SSO — Azure AD, Okta, Duo). Il n'y a alors aucun mot de passe à diff --git a/script/vpn/README.md b/script/vpn/README.md index fa966ef..173ce01 100644 --- a/script/vpn/README.md +++ b/script/vpn/README.md @@ -321,8 +321,16 @@ says so when it takes it. unknown server certificate raises a question, and openconnect would read the answer from the standard input the password arrives on. With it, openconnect refuses at once **and** prints the `--servercert sha256:…` line to paste into -the profile's `oc_servercert`. Routes belong to the server, through -`vpnc-script`; the profile can add to them, not replace them. +the profile's `oc_servercert`. + +Routes belong to the server, through `vpnc-script`; the profile can add to +them, not replace them. For that reason the form does **not** ask this +driver « send ALL traffic through the tunnel? », and `status` does not judge +it: the gateway decides what enters the tunnel, and forcing a default route +against a split-tunnel gateway would not give all traffic but a black hole — +a gateway does not route what it never advertised. To force a network +through anyway, add it to `routes`, which this driver does honour, with +`0.0.0.0/0` for everything. Set **`oc_sso`** when the concentrator authenticates through a **web form** (SAML / SSO — Azure AD, Okta, Duo). There is then no password to send, and diff --git a/script/vpn/drivers/base.py b/script/vpn/drivers/base.py index d09c4cd..a85fc1f 100644 --- a/script/vpn/drivers/base.py +++ b/script/vpn/drivers/base.py @@ -301,6 +301,15 @@ class VpnDriver: # Faux quand la technologie ne prend pas le MTU du profil — le demander # serait une question sans effet. uses_mtu = True + # Faux quand la technologie ne pose pas la route par défaut elle-même — + # c'est alors le SERVEUR qui décide de ce qui passe par le tunnel. Le + # champ n'est ni demandé ni jugé : demander « tout le trafic ? » à qui + # n'a pas la main dessus, puis sanctionner la réponse par un ✗ sur un + # tunnel sain, est la pire des trois façons de traiter la question. + # + # L'échappatoire reste : `routes` est honorée par tous les pilotes, et + # « 0.0.0.0/0 » y demande explicitement ce que ce drapeau n'offre plus. + uses_default_route = True def __init__(self, profile: dict, secrets: dict | None = None): self.profile = profile @@ -859,7 +868,7 @@ class VpnDriver: + (f" (attendu {iface})" if not ok else ""), ) ) - if self.profile.get("default_route"): + if self.profile.get("default_route") and self.uses_default_route: info = route_to("1.1.1.1") checks.append( ( diff --git a/script/vpn/drivers/openconnect.py b/script/vpn/drivers/openconnect.py index b1f7f1d..0dffa4a 100644 --- a/script/vpn/drivers/openconnect.py +++ b/script/vpn/drivers/openconnect.py @@ -172,6 +172,12 @@ class OpenconnectDriver(VpnDriver): # Le serveur pousse les routes : exiger une route déclarée serait une # fausse exigence. needs_routes = False + # Et pour la même raison, la route par défaut ne se demande pas ici : + # c'est le concentrateur qui décide de ce qui entre dans le tunnel, et + # `vpnc-script` pose ce qu'il pousse. La forcer côté client contre une + # passerelle en tunnel scindé ne donnerait pas « tout le trafic » mais + # un trou noir — la passerelle ne route pas ce qu'elle n'a pas annoncé. + uses_default_route = False defaults = { "port": 443, "oc_user": "", diff --git a/script/vpn/presets.py b/script/vpn/presets.py index 1dc8859..c484764 100644 --- a/script/vpn/presets.py +++ b/script/vpn/presets.py @@ -159,15 +159,6 @@ def load_all(config=None) -> tuple[list[dict], list[str]]: return list(by_id.values()), errors -def load(identifier: str, config=None) -> dict | None: - """Le préréglage `identifier`, ou None.""" - found, _ = load_all(config) - for item in found: - if item["preset"] == identifier: - return item - return None - - def label(preset: dict) -> str: """Ce qu'on affiche. Le `label` s'il est là, l'identifiant sinon : un préréglage sans libellé reste choisissable.""" diff --git a/test/test_vpn_drivers.py b/test/test_vpn_drivers.py index 6936d76..903f501 100644 --- a/test/test_vpn_drivers.py +++ b/test/test_vpn_drivers.py @@ -416,6 +416,47 @@ class Openconnect(unittest.TestCase): self.assertEqual(driver.profile["routes"], []) +class TheDefaultRouteVerdict(unittest.TestCase): + """`status` ne juge que ce que le pilote CONTRÔLE. + + Un pilote dont le serveur décide du routage ne pose pas la route par + défaut. La juger quand même rendait un ✗ permanent sur un tunnel + parfaitement sain, et un profil qui la demandait n'obtenait rien. + """ + + def _driver(self, name, **overrides): + # `driver` explicite : les échantillons ne le portent pas, et la + # validation retomberait sur le pilote par défaut. + profile = dict( + SAMPLES[name][0], + name="t-route", + driver=name, + default_route=True, + ) + profile.update(overrides) + return DRIVERS[name](profiles.validate(profile), {}) + + @staticmethod + def _labels(driver): + """Les libellés que rend `check_routes`, interrogé directement. + + `standard_status` ne l'appelle que sur un tunnel MONTÉ : passer par + lui ne rendrait aucun verdict de route sur une machine de test, et + le test passerait pour la mauvaise raison. + """ + return [label for label, _ok, _d in driver.check_routes("vpn-t")] + + def test_a_server_routed_driver_is_not_judged_on_it(self): + driver = self._driver("openconnect") + self.assertFalse(driver.uses_default_route) + self.assertNotIn("route par défaut", self._labels(driver)) + + def test_a_driver_that_lays_it_is_still_judged(self): + driver = self._driver("wireguard") + self.assertTrue(driver.uses_default_route) + self.assertIn("route par défaut", self._labels(driver)) + + class _OpenconnectNoHelper(DRIVERS["openconnect"]): """Le pilote openconnect, mais sans greffon SSO. diff --git a/test/test_vpn_menu.py b/test/test_vpn_menu.py index c521ad5..1a376e8 100644 --- a/test/test_vpn_menu.py +++ b/test/test_vpn_menu.py @@ -672,6 +672,72 @@ def largeur_affichee(texte): ) +class TheDefaultRouteQuestion(MenuBase): + """« Tout le trafic ? » n'est posée qu'aux pilotes qui posent la route. + + Demander à qui n'a pas la main dessus, puis sanctionner la réponse par + un ✗ sur un tunnel sain, était la pire des trois façons de traiter la + question : le champ existait, ne servait à rien, et faisait échouer le + diagnostic. + """ + + def filling(self, seed, *answers): + """Déroule le formulaire et rend (questions posées, sortie).""" + prompts = [] + suite = iter(answers) + + def fake(prompt=""): + prompts.append(prompt.strip()) + return next(suite) + + with patch("builtins.input", fake): + out = io.StringIO() + with redirect_stdout(out): + try: + self.todo._vpn_edit_profile(seed=dict(seed)) + except StopIteration: + pass + return prompts, out.getvalue() + + OPENCONNECT = { + "name": "oc", + "driver": "openconnect", + "server": "ssl.vpn.example-campus.net", + "oc_user": "someone", + } + WIREGUARD = { + "name": "wg", + "driver": "wireguard", + "server": "gw.example-campus.net", + "wg_address": "10.9.0.2/32", + "wg_peer_key": WG_PUBLIC, + } + + def test_a_server_routed_driver_is_not_asked(self): + prompts, printed = self.filling(self.OPENCONNECT, *[""] * 20) + self.assertFalse([p for p in prompts if "trafic" in p], prompts) + self.assertIn("passerelle décide", printed) + + def test_the_escape_hatch_is_named(self): + """Retirer la question sans dire par quoi la remplacer se lirait + comme une capacité perdue : `routes` reste honorée.""" + _, printed = self.filling(self.OPENCONNECT, *[""] * 20) + self.assertIn("0.0.0.0/0", printed) + + def test_a_driver_that_lays_the_route_is_still_asked(self): + prompts, _ = self.filling(self.WIREGUARD, *[""] * 20) + self.assertTrue([p for p in prompts if "trafic" in p], prompts) + + def test_the_flag_is_not_kept_on_a_driver_that_ignores_it(self): + """Un drapeau laissé à vrai resterait un champ sans effet, et la + liste des profils continuerait d'annoncer « tout le trafic ».""" + seed = dict(self.OPENCONNECT, default_route=True) + self.filling(seed, *([""] * 8 + ["n"])) + saved = profiles.load("oc") + self.assertIsNotNone(saved, "profil non enregistré") + self.assertFalse(saved["default_route"]) + + class ShowingWhatIsConnected(MenuBase): """L'état de chaque profil, dans la liste qui sert à choisir. diff --git a/test/test_vpn_presets.py b/test/test_vpn_presets.py index 2ba6d16..0524cf1 100644 --- a/test/test_vpn_presets.py +++ b/test/test_vpn_presets.py @@ -159,11 +159,6 @@ class PresetLoading(unittest.TestCase): found, errors = presets.load_all() self.assertEqual((found, errors), ([], [])) - def test_load_finds_one_by_identifier(self): - write_preset(self.shared, "campus.json", PRESET) - self.assertEqual(presets.load("campus")["server"], PRESET["server"]) - self.assertIsNone(presets.load("nowhere")) - class PresetApplication(unittest.TestCase): """`apply` : d'un préréglage à un profil que la validation accepte."""