Merge branch 'develop'
[FIX] vpn : ne plus demander une route que le pilote ne pose pas 2 commits. Le formulaire demandait « tout le trafic ? » à un pilote dont le serveur décide du routage, ne faisait rien de la réponse, et l'état la jugeait quand même : un champ sans effet et un verdict en échec permanent sur un tunnel sain. Un drapeau dit désormais quels pilotes posent cette route ; les autres ne sont ni interrogés ni jugés. L'honorer serait pire qu'inutile — forcer une route par défaut contre une passerelle en tunnel scindé donne un trou noir. Les routes déclarées restent honorées, et le formulaire nomme « 0.0.0.0/0 ». Une recherche de préréglage sans appelant part avec. Vérifié : 274 tests unitaires du VPN. --- EN --- 2 commits. The form asked "all traffic?" of a driver whose server decides the routing, did nothing with the answer, and status judged it anyway: a field with no effect and a permanently failed verdict on a healthy tunnel. A flag now says which drivers lay that route; the others are neither asked nor judged. Honouring it would be worse than useless — forcing a default route against a split-tunnel gateway gives a black hole. Declared routes stay honoured, and the form names "0.0.0.0/0". A preset lookup with no caller goes with it. Checked: 274 VPN unit tests. Assisted-by: Claude Opus 5
This commit is contained in:
commit
16c4c932b9
11 changed files with 191 additions and 25 deletions
|
|
@ -11810,6 +11810,19 @@ TRANSLATIONS = {
|
||||||
" state a dead tunnel left behind."
|
" 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": {
|
"SSO helper": {
|
||||||
"fr": "greffon SSO",
|
"fr": "greffon SSO",
|
||||||
"en": "SSO helper",
|
"en": "SSO helper",
|
||||||
|
|
|
||||||
|
|
@ -117,6 +117,14 @@ NOT_CONNECTED_NOTE = (
|
||||||
" a dead tunnel left behind."
|
" 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 = (
|
MASTER_PASSWORD_WARNING = (
|
||||||
"The vault MASTER password is stored in the configuration in clear"
|
"The vault MASTER password is stored in the configuration in clear"
|
||||||
" text. Remove it and type it on demand."
|
" text. Remove it and type it on demand."
|
||||||
|
|
@ -566,10 +574,17 @@ class VpnMenuMixin:
|
||||||
t("Networks to reach, comma-separated"),
|
t("Networks to reach, comma-separated"),
|
||||||
", ".join(draft.get("routes", [])),
|
", ".join(draft.get("routes", [])),
|
||||||
)
|
)
|
||||||
draft["default_route"] = self._vpn_ask_flag(
|
if driver_cls.uses_default_route:
|
||||||
t("Send ALL traffic through the tunnel?"),
|
draft["default_route"] = self._vpn_ask_flag(
|
||||||
draft.get("default_route", False),
|
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(
|
draft["probe"] = self._vpn_ask(
|
||||||
t("Witness address reachable only through the tunnel (optional)"),
|
t("Witness address reachable only through the tunnel (optional)"),
|
||||||
draft.get("probe", ""),
|
draft.get("probe", ""),
|
||||||
|
|
|
||||||
|
|
@ -605,8 +605,16 @@ says so when it takes it.
|
||||||
unknown server certificate raises a question, and openconnect would read the
|
unknown server certificate raises a question, and openconnect would read the
|
||||||
answer from the standard input the password arrives on. With it, openconnect
|
answer from the standard input the password arrives on. With it, openconnect
|
||||||
refuses at once **and** prints the `--servercert sha256:…` line to paste into
|
refuses at once **and** prints the `--servercert sha256:…` line to paste into
|
||||||
the profile's `oc_servercert`. Routes belong to the server, through
|
the profile's `oc_servercert`.
|
||||||
`vpnc-script`; the profile can add to them, not replace them.
|
|
||||||
|
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**
|
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
|
(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
|
openconnect refuse tout de suite **et** imprime la ligne
|
||||||
`--servercert sha256:…` à recopier dans le champ `oc_servercert` du profil.
|
`--servercert sha256:…` à recopier dans le champ `oc_servercert` du profil.
|
||||||
Les routes appartiennent au serveur, via `vpnc-script` ; le profil peut en
|
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
|
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 à
|
web** (SAML / SSO — Azure AD, Okta, Duo). Il n'y a alors aucun mot de passe à
|
||||||
|
|
|
||||||
|
|
@ -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
|
openconnect refuse tout de suite **et** imprime la ligne
|
||||||
`--servercert sha256:…` à recopier dans le champ `oc_servercert` du profil.
|
`--servercert sha256:…` à recopier dans le champ `oc_servercert` du profil.
|
||||||
Les routes appartiennent au serveur, via `vpnc-script` ; le profil peut en
|
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
|
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 à
|
web** (SAML / SSO — Azure AD, Okta, Duo). Il n'y a alors aucun mot de passe à
|
||||||
|
|
|
||||||
|
|
@ -321,8 +321,16 @@ says so when it takes it.
|
||||||
unknown server certificate raises a question, and openconnect would read the
|
unknown server certificate raises a question, and openconnect would read the
|
||||||
answer from the standard input the password arrives on. With it, openconnect
|
answer from the standard input the password arrives on. With it, openconnect
|
||||||
refuses at once **and** prints the `--servercert sha256:…` line to paste into
|
refuses at once **and** prints the `--servercert sha256:…` line to paste into
|
||||||
the profile's `oc_servercert`. Routes belong to the server, through
|
the profile's `oc_servercert`.
|
||||||
`vpnc-script`; the profile can add to them, not replace them.
|
|
||||||
|
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**
|
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
|
(SAML / SSO — Azure AD, Okta, Duo). There is then no password to send, and
|
||||||
|
|
|
||||||
|
|
@ -301,6 +301,15 @@ class VpnDriver:
|
||||||
# Faux quand la technologie ne prend pas le MTU du profil — le demander
|
# Faux quand la technologie ne prend pas le MTU du profil — le demander
|
||||||
# serait une question sans effet.
|
# serait une question sans effet.
|
||||||
uses_mtu = True
|
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):
|
def __init__(self, profile: dict, secrets: dict | None = None):
|
||||||
self.profile = profile
|
self.profile = profile
|
||||||
|
|
@ -859,7 +868,7 @@ class VpnDriver:
|
||||||
+ (f" (attendu {iface})" if not ok else ""),
|
+ (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")
|
info = route_to("1.1.1.1")
|
||||||
checks.append(
|
checks.append(
|
||||||
(
|
(
|
||||||
|
|
|
||||||
|
|
@ -172,6 +172,12 @@ class OpenconnectDriver(VpnDriver):
|
||||||
# Le serveur pousse les routes : exiger une route déclarée serait une
|
# Le serveur pousse les routes : exiger une route déclarée serait une
|
||||||
# fausse exigence.
|
# fausse exigence.
|
||||||
needs_routes = False
|
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 = {
|
defaults = {
|
||||||
"port": 443,
|
"port": 443,
|
||||||
"oc_user": "",
|
"oc_user": "",
|
||||||
|
|
|
||||||
|
|
@ -159,15 +159,6 @@ def load_all(config=None) -> tuple[list[dict], list[str]]:
|
||||||
return list(by_id.values()), errors
|
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:
|
def label(preset: dict) -> str:
|
||||||
"""Ce qu'on affiche. Le `label` s'il est là, l'identifiant sinon : un
|
"""Ce qu'on affiche. Le `label` s'il est là, l'identifiant sinon : un
|
||||||
préréglage sans libellé reste choisissable."""
|
préréglage sans libellé reste choisissable."""
|
||||||
|
|
|
||||||
|
|
@ -416,6 +416,47 @@ class Openconnect(unittest.TestCase):
|
||||||
self.assertEqual(driver.profile["routes"], [])
|
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"]):
|
class _OpenconnectNoHelper(DRIVERS["openconnect"]):
|
||||||
"""Le pilote openconnect, mais sans greffon SSO.
|
"""Le pilote openconnect, mais sans greffon SSO.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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):
|
class ShowingWhatIsConnected(MenuBase):
|
||||||
"""L'état de chaque profil, dans la liste qui sert à choisir.
|
"""L'état de chaque profil, dans la liste qui sert à choisir.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -159,11 +159,6 @@ class PresetLoading(unittest.TestCase):
|
||||||
found, errors = presets.load_all()
|
found, errors = presets.load_all()
|
||||||
self.assertEqual((found, errors), ([], []))
|
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):
|
class PresetApplication(unittest.TestCase):
|
||||||
"""`apply` : d'un préréglage à un profil que la validation accepte."""
|
"""`apply` : d'un préréglage à un profil que la validation accepte."""
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue