Commit 7b630d8d authored by Thomas Haller's avatar Thomas Haller
Browse files

clients: set ipv4.dns-priority to a exclusive value (-10) when importing WireGuard profiles

parent 9a74d2e0
Pipeline #129884 passed with stages
in 35 minutes and 19 seconds
  • mentioned in issue #405 (closed)

    Toggle commit list
  • Hi @zx2c4, @thaller, does this still make sense? NetworkManager's behavior for other VPNs has changed to use priority -50 if the VPN has the default route, since af13081b. So this code now seems redundant, yes? It also assumes that split DNS is never desired for Wireguard connections, which seems surprising and is quite different from other VPNs. Is that really true?

  • Hi @mcatanzaro

    btw, in recent versions, the priority is now set to -50.

    Whether we set it to -10 or -50 does not make a big different. What matters is whether the value is set to a negative value, to 0, or to a positive value. 0 means to inherit the default (which is configurable in NetworkManager.conf too). And since recently, that default is -50 too.

    So, it's a difference whether you explicitly set to to -50 or to 0, because the default value is also configurable -- it only has a default of -50 (the default of the default).

    If you import a profile, then the code aim to generate the profile that most closely resembles the behavior of wg-quick. wg-quick wants to set resolvconf -x, so the imported profile explicitly sets a negative priority.

    For no very strong reasons, it could also leave it at 0. But I think -50 makes sense. After all, the user is supposed to import the profile once, and then modify it to as they wish. Import has to make a choice, and I think -50 is sensible.

    -- one problem is that none of our GUIs support dns-priority yet :(

    Edited by Thomas Haller
  • Doesn't this force VPNs to be full-tunnel? (Since there is no GUI, the user cannot plausibly be expected to edit this setting.) Is it not redundant with af13081b? The VPN would get priority -50 regardless if needed due to that commit, right?

  • Is it not redundant with af13081b? The VPN would get priority -50 regardless if needed due to that commit, right?

    No. What I tried to say: the default of the default (the -50 from af13081b) is configurable. See man NetworkManager.conf. So, it's a (subtle) difference -- for users who configure the default in NetworkManager.conf.

    But yes, your concern about having no GUI is valid. But that is the same problem, regardless whether nmcli con import sets priority to -10/-50 or whether it gets the new default from af13081b. See also: #563 (comment 699930) . That is a problem.

    Of course, there are those who claim that DNS leaks are a very bad thing (which is the reason for the change in behavior of af13081b). You cannot have it both at the same time :(

    Note that 1.28 is currently pending in release candidate phase for weeks already. And having no GUI might be a good reason for reverting af13081b before 1.28.0 release.

    OK?

  • Doesn't this force VPNs to be full-tunnel?

    Well, yes+no. Only with respect to DNS lookups. Not with respect to routing. The ipv[46].route-metric is a an important aspect there.

Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment