Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1066?usp=email

to review the following change.


Change subject: fix macOS dns-updown handling of parallel full redirects
......................................................................

fix macOS dns-updown handling of parallel full redirects

The script didn't handle scenarios well where two or more parallel VPN
connections want to replace the default DNS server. The DNS configuration
has a chance to get broken by the connections going down in a different
order than they came up in.

Disallowing all but the first connection to modify the default DNS server
will effectively prevent this issue. While it may break DNS for the
latter connections, it is the best we can do without knowing specifics
about the configurations.

Change-Id: I7b413578a8fc0c65fca26f72b901a9f7bc34b137
Signed-off-by: Heiko Hund <he...@ist.eigentlich.net>
---
M distro/dns-scripts/macos-dns-updown.sh
1 file changed, 9 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/66/1066/1

diff --git a/distro/dns-scripts/macos-dns-updown.sh 
b/distro/dns-scripts/macos-dns-updown.sh
index 89d6882..c15abaa 100644
--- a/distro/dns-scripts/macos-dns-updown.sh
+++ b/distro/dns-scripts/macos-dns-updown.sh
@@ -30,6 +30,7 @@

 itf_dns_key="State:/Network/Service/openvpn-${dev}/DNS"
 dns_backup_key="State:/Network/Service/openvpn-${dev}/DnsBackup"
+dns_backup_key_pattern="State:/Network/Service/openvpn-.*/DnsBackup"

 function primary_dns_key {
     local uuid=$(echo "show State:/Network/Global/IPv4" | /usr/sbin/scutil | 
grep "PrimaryService" | cut -d: -f2 | xargs)
@@ -166,6 +167,11 @@
         echo -e "${cmds}" | /usr/sbin/scutil
         set_search_domains "$search_domains"
     else
+        echo list ${dns_backup_key_pattern} | /usr/sbin/scutil | grep -q 'no 
key' || {
+            echo "setting DNS failed, already redirecting to another tunnel"
+            exit 1
+        }
+
         local cmds=""
         cmds+="get $(primary_dns_key)\n"
         cmds+="set ${dns_backup_key}\n"
@@ -200,6 +206,9 @@
         echo "remove ${itf_dns_key}" | /usr/sbin/scutil
         unset_search_domains "$search_domains"
     else
+        # Do not unset if this tunnel did not set/backup DNS before
+        echo list ${dns_backup_key} | /usr/sbin/scutil | grep -qv 'no key' || 
return
+
         local cmds=""
         cmds+="get ${dns_backup_key}\n"
         cmds+="set $(primary_dns_key)\n"

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1066?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7b413578a8fc0c65fca26f72b901a9f7bc34b137
Gerrit-Change-Number: 1066
Gerrit-PatchSet: 1
Gerrit-Owner: d12fk <he...@openvpn.net>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to