WireGuard Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] wg-quick: linux: Don't fail systemd service when using systemd-resolved
@ 2019-08-24  7:14 Ronan Pigott
  2019-08-27 22:51 ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Ronan Pigott @ 2019-08-24  7:14 UTC (permalink / raw)
  To: wireguard; +Cc: Ronan Pigott

From: Ronan Pigott <rpigott@berkeley.edu>

systemd-resolved has a compatibility interface for use with resolvconf
scripts when resolvectl is called from a symlink from resolvconf.
However, when tearing down the interface, cmd_down calls del_if and
then unset_dns. In the case of systemd-resolved, deleting the interface
also removes the systemd-resolved entry and causes resolvconf -d to fail
when resolvconf really is a symlink to resolvectl.

The failure is harmless, as the dns entries have already been
effectively removed, but the nonzero exit code causes the systemd
service in 'systemctl stop wg-quick@ifname' to exit with failure.

Adding real sysemd-resolved support might involve checking if resolved
is running through dbus and such, but I think thats generally beyond the
scope of the wg-quick tool. Instead, the intent of this patch is basically:

"Let users off the hook for failure in resolvconf this one time if it's
really just a symlink to resolvectl"

---
 src/tools/wg-quick/linux.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/wg-quick/linux.bash b/src/tools/wg-quick/linux.bash
index 2f36dee..63ef781 100755
--- a/src/tools/wg-quick/linux.bash
+++ b/src/tools/wg-quick/linux.bash
@@ -300,7 +300,7 @@ cmd_down() {
 	execute_hooks "${PRE_DOWN[@]}"
 	[[ $SAVE_CONFIG -eq 0 ]] || save_config
 	del_if
-	unset_dns
+	unset_dns || [[ "$(type -P resolvconf)" -ef "$(type -P resolvectl)" ]]
 	execute_hooks "${POST_DOWN[@]}"
 }
 
-- 
2.23.0

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wg-quick: linux: Don't fail systemd service when using systemd-resolved
  2019-08-24  7:14 [PATCH] wg-quick: linux: Don't fail systemd service when using systemd-resolved Ronan Pigott
@ 2019-08-27 22:51 ` Jason A. Donenfeld
  2019-08-28  1:21   ` Ronan Pigott
  0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-08-27 22:51 UTC (permalink / raw)
  To: Ronan Pigott; +Cc: Ronan Pigott, WireGuard mailing list

Could you resubmit a v2 with your Signed-off-by line?

Instead of this hack, why not just reorder unset_dns and del_if? Are
you worried about a race in between them if they're ordered as such?
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wg-quick: linux: Don't fail systemd service when using systemd-resolved
  2019-08-27 22:51 ` Jason A. Donenfeld
@ 2019-08-28  1:21   ` Ronan Pigott
  2019-08-28  2:20     ` Jason A. Donenfeld
  0 siblings, 1 reply; 4+ messages in thread
From: Ronan Pigott @ 2019-08-28  1:21 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Ronan Pigott, WireGuard mailing list

On Tue Aug 27, 2019 at 4:51 PM Jason A. Donenfeld wrote:
> Could you resubmit a v2 with your Signed-off-by line?
OK.

> Instead of this hack, why not just reorder unset_dns and del_if? Are
> you worried about a race in between them if they're ordered as such?
Yeah that was my first thought, then I supposed that perhaps it was
ordered that way intentionally to prevent a situation where the dns is
unset and a client would make a dns lookup on the interface before it is
deleted. It would use the sd-resolved 'global' configured dns servers in
that case which is maybe undesired.

I realize it's a hack. I gave the explanation somewhat in the hope
someone had a better solution to the problem I was experiencing.

The real issue here is that there are there are three common
implementations of resolvconf: debian resolvconf, openresolv, and
sd-resolved compat interface. So wg-quick uses resolvconf, but is only
fully compatible with openresolv. wg-quick is (accidentally) compatible
with sd-resolved through the resolvconf compat interface, except for
this minor hiccup. This hack allows wg-quick to work for both methods
with no extra configuration from the user, no alternative code path
for sd-resolved, and no change in behavior for non sd-resolved users.

There is another way that is might be more palatable: openresolv and
sd-resolved resolvconf both support a '-f' flag for ignoring missing
interfaces, which neatly sidesteps this error. However, I don't think
debian's resolvconf supports this flag. Now that I look into it though,
it seems debian's resolvconf does not support '-l' either, which wg-quick
uses, so maybe we don't care to support this anyway. I think v2 will
just use the flag instead, if that's alright.

Users of openvpn use sd-resolved dbus interface in place of resolvconf,
such as seen here: https://github.com/jonathanio/update-systemd-resolved
but I believe this is overkill for wg-quick.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wg-quick: linux: Don't fail systemd service when using systemd-resolved
  2019-08-28  1:21   ` Ronan Pigott
@ 2019-08-28  2:20     ` Jason A. Donenfeld
  0 siblings, 0 replies; 4+ messages in thread
From: Jason A. Donenfeld @ 2019-08-28  2:20 UTC (permalink / raw)
  To: Ronan Pigott; +Cc: Ronan Pigott, WireGuard mailing list

Hi Ronan,

On Tue, Aug 27, 2019 at 7:21 PM Ronan Pigott <rpigott314@gmail.com> wrote:
> Yeah that was my first thought, then I supposed that perhaps it was
> ordered that way intentionally to prevent a situation where the dns is
> unset and a client would make a dns lookup on the interface before it is
> deleted. It would use the sd-resolved 'global' configured dns servers in
> that case which is maybe undesired.

Right, exactly.

> I realize it's a hack. I gave the explanation somewhat in the hope
> someone had a better solution to the problem I was experiencing.

One approach to the hack would be to just take it all the way and
append `|| true`. That is, make it non-fatal. A tear down routine,
anyway, has very few choices on what to do when something goes wrong,
and sometimes continuing isn't such a bad policy.

> The real issue here is that there are there are three common
> implementations of resolvconf: debian resolvconf, openresolv, and
> sd-resolved compat interface. So wg-quick uses resolvconf, but is only
> fully compatible with openresolv. wg-quick is (accidentally) compatible
> with sd-resolved through the resolvconf compat interface, except for
> this minor hiccup. This hack allows wg-quick to work for both methods
> with no extra configuration from the user, no alternative code path
> for sd-resolved, and no change in behavior for non sd-resolved users.

"wg-quick is (accidentally) compatible with sd-resolved" That's
actually not totally true. Systemd has that resolvconf compat
interface because I annoyed Lennart about it a bunch. :-P

> There is another way that is might be more palatable: openresolv and
> sd-resolved resolvconf both support a '-f' flag for ignoring missing
> interfaces, which neatly sidesteps this error. However, I don't think
> debian's resolvconf supports this flag. Now that I look into it though,
> it seems debian's resolvconf does not support '-l' either, which wg-quick
> uses, so maybe we don't care to support this anyway. I think v2 will
> just use the flag instead, if that's alright.

Yes, that's terrific. If I recall correctly, Debian's resolvconf will
ignore arguments it doesn't understand if they're in the right
position. It seems, anyhow, that -f is exactly what we should be
using. I'll gladly merge a signed-off-by v2 that adds it.

> Users of openvpn use sd-resolved dbus interface in place of resolvconf,
> such as seen here: https://github.com/jonathanio/update-systemd-resolved
> but I believe this is overkill for wg-quick.

Ahhh yes the amazing shell-script IPv6 parser. I recall reading this
in wonder a few years ago. Indeed it's a bit "much" for wg-quick, and
I think your solution of adding -f to resolvconf is a good approach.

Jason
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-24  7:14 [PATCH] wg-quick: linux: Don't fail systemd service when using systemd-resolved Ronan Pigott
2019-08-27 22:51 ` Jason A. Donenfeld
2019-08-28  1:21   ` Ronan Pigott
2019-08-28  2:20     ` Jason A. Donenfeld

WireGuard Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/wireguard/0 wireguard/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 wireguard wireguard/ https://lore.kernel.org/wireguard \
		wireguard@lists.zx2c4.com
	public-inbox-index wireguard

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.zx2c4.lists.wireguard


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git