Development discussion of WireGuard
 help / color / mirror / Atom feed
* wg-quick: darwin: DNS cleanup & disabled/inactive interface causing setup to fail
@ 2021-01-22  6:57 Loren McIntyre
  2021-01-22  6:58 ` [PATCH 1/2] wg-quick: darwin: restore original dns settings on failed up Loren McIntyre
  0 siblings, 1 reply; 3+ messages in thread
From: Loren McIntyre @ 2021-01-22  6:57 UTC (permalink / raw)
  To: wireguard

Hello!

I've been using wg-quick on macOS 10.14, with wireguard-tools at 1.0.20200827. I had one issue prompt investigation, DNS changes persisted after an interface failed to come up. I found a second, related issue, and added an extra error handling case in the wg-quick/darwin.bash script to fix the issue.
---
First, when some network interfaces or "services" are disabled, wg-quick would attempt to bring up the interface, in darwin.bash:449:cmd_up(), something errors and the trap handler runs before :446:monitor_daemon, but after / as-concluding :465:...set_dns, on INT / TERM / EXIT.

This failure occurred as set_dns concluded, but before monitor_daemon and its cleanup del_dns trap, so wg-quick leaves DNS for the system misconfigured. Adding del_dns to the cmd_up trap seems appropriate, and leaving del_dns in the monitor_daemon trap seems appropriate. 


Minimal reproduction of the DNS non-cleanup failure should require only: a network interface set to inactive/disabled in Network.prefPane, and using wg-quick to bring up an interface.
- Interface: disabled / wg-quick: fails. 
- Interface: enabled / wg-quick: succeeds. 
---
Second, when the last interface in the loop is disabled, `networksetup -setdnsservers 'EXAMPLE IF' 10.0.100.53`, returns only:
> (Please note: EXAMPLE IF is currently disabled)

Because `-e` is set for the whole script, when this line is read, the final comparison of the loop fails, causing EXIT when set_dns exits.
> src/wg-quick/darwin.bash:298:           while read -r response; do
> src/wg-quick/darwin.bash:299:                   [[ $response == *Error* ]] && echo "$response" >&2

See toy examples in:
> while read -r var ; do [[ $var == 2 ]] && echo "var: $var"; done < <(seq 3); echo ?$?
> while read -r var ; do [[ $var == 3 ]] && echo "var: $var"; done < <(seq 3); echo ?$?
---
Patches to follow

Best,
Loren

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

* [PATCH 1/2] wg-quick: darwin: restore original dns settings on failed up
  2021-01-22  6:57 wg-quick: darwin: DNS cleanup & disabled/inactive interface causing setup to fail Loren McIntyre
@ 2021-01-22  6:58 ` Loren McIntyre
  2021-01-22  6:58   ` [PATCH 2/2] wg-quick: darwin: handle disabled interface Loren McIntyre
  0 siblings, 1 reply; 3+ messages in thread
From: Loren McIntyre @ 2021-01-22  6:58 UTC (permalink / raw)
  To: wireguard; +Cc: Loren McIntyre

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

diff --git a/src/wg-quick/darwin.bash b/src/wg-quick/darwin.bash
index cde1b54..9c92f87 100755
--- a/src/wg-quick/darwin.bash
+++ b/src/wg-quick/darwin.bash
@@ -449,7 +449,7 @@ cmd_usage() {
 cmd_up() {
 	local i
 	get_real_interface && die "\`$INTERFACE' already exists as \`$REAL_INTERFACE'"
-	trap 'del_if; del_routes; exit' INT TERM EXIT
+	trap 'del_if; del_routes; del_dns; exit' INT TERM EXIT
 	execute_hooks "${PRE_UP[@]}"
 	add_if
 	set_config
-- 
2.29.2


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

* [PATCH 2/2] wg-quick: darwin: handle disabled interface
  2021-01-22  6:58 ` [PATCH 1/2] wg-quick: darwin: restore original dns settings on failed up Loren McIntyre
@ 2021-01-22  6:58   ` Loren McIntyre
  0 siblings, 0 replies; 3+ messages in thread
From: Loren McIntyre @ 2021-01-22  6:58 UTC (permalink / raw)
  To: wireguard; +Cc: Loren McIntyre

---
 src/wg-quick/darwin.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/wg-quick/darwin.bash b/src/wg-quick/darwin.bash
index 9c92f87..e9bdfea 100755
--- a/src/wg-quick/darwin.bash
+++ b/src/wg-quick/darwin.bash
@@ -296,7 +296,7 @@ set_dns() {
 	local service response
 	for service in "${!SERVICE_DNS[@]}"; do
 		while read -r response; do
-			[[ $response == *Error* ]] && echo "$response" >&2
+			[[ $response == *Error* || $response == *"disabled)" ]] && echo "$response" >&2
 		done < <(
 			cmd networksetup -setdnsservers "$service" "${DNS[@]}"
 			if [[ ${#DNS_SEARCH[@]} -eq 0 ]]; then
@@ -312,7 +312,7 @@ del_dns() {
 	local service response
 	for service in "${!SERVICE_DNS[@]}"; do
 		while read -r response; do
-			[[ $response == *Error* ]] && echo "$response" >&2
+			[[ $response == *Error* || $response == *"disabled)" ]] && echo "$response" >&2
 		done < <(
 			cmd networksetup -setdnsservers "$service" ${SERVICE_DNS["$service"]} || true
 			cmd networksetup -setsearchdomains "$service" ${SERVICE_DNS_SEARCH["$service"]} || true
-- 
2.29.2


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

end of thread, other threads:[~2021-01-24 16:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22  6:57 wg-quick: darwin: DNS cleanup & disabled/inactive interface causing setup to fail Loren McIntyre
2021-01-22  6:58 ` [PATCH 1/2] wg-quick: darwin: restore original dns settings on failed up Loren McIntyre
2021-01-22  6:58   ` [PATCH 2/2] wg-quick: darwin: handle disabled interface Loren McIntyre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).