New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor scutil parts of client scripts, fix #379 #452
Conversation
@stek29 - Thanks. This looks very interesting! One tiny point from my first look: currently Tunnelblick supports only OS X 10.7.5 and higher. So the conditional checking for OS X 10.4 in (new) line 1172 of client.up.tunnelblick.sh isn't needed. (There may be other 10.4 - 10.6 references that should be removed, but let's not add new ones!) |
@jkbullard done :) |
bump |
@stek29 - Thanks again for this. It points the way to do several things. However, I don't want to merge it because
I would like to take pieces of this PR and implement them separately if you don't want to, but that means that you wouldn't get "PR merged" credit on GitHub for this. Would that be OK with you, @stek29? Edited 2018-06-29 09:48 EDT to remove "2. I think it leaves artifacts around in SC because there aren't corresponding changes to the down script;", which was just plain wrong. My apologies to @stek29. |
I disagree. All of changes made (except DNS6 maybe) just can't be separated – you can't separate full rewrite of something.
I was only mentioning this because I was looking for feedback and hints on which parts of Tunnelblick should be changed.
I'm OK with not getting "PR merged" credit, and I think that just picking a commit would be ok (see my comments on point 1). Also, I've been using this for more than 2 months, and I've only encountered one issue:
However, I'm not sure if this is actually caused by changed shell script. But I have not tried to put old script back and check if it still crashes. |
@stek29 -
Actually, it was the IPv6 (and the copy/paste error) that I am most interested in – that's where my comment came from : )
OK, thanks. I plan to do that, along with some other changes. Thanks also for mentioning the crashes and for your clear description of when they happen. Since they were crashes of Tunnelblick itself, they probably aren't related to the up/down scripts, which are run directly by OpenVPN and would crash that process instead (although it is possible that such an unusual situation could cause Tunnelblick to crash). Sleep/wake often caused Tunnelblick crashes in versions earlier than 3.7.5beta06 build 5000 (2018-02-15) but I'll try to reproduce this since you presumably are using a more recent version. |
@jkbullard I would split it into multiple commits |
@jkbullard now it's split into |
@stek29 - I have merged the PR that fixed the typo in the down script – thanks! I have decided not to accept this PR, which changes the default up/down scripts, because I do not want to risk breaking existing setups. However I am still interested in the changes and would like you to submit a new PR which will modify two new scripts that I have just committed as 35842e5: client.4.up.tunnelblick.sh and client.4.down.tunnelblick.sh. They are available as "Set nameserver (IPv6)" under the "Set DNS/WINS" setting (in anticipation of your changes that implement IPv6 support). @stek29, I'd like you to, using the newly-updated head of master, overwrite client.4.up.tunnelblick.sh and client.4.down.tunnelblick.sh with your corresponding modified copies. (Because the existing scripts are copies of client.up.tunnelblick.sh and client.down.tunnelblick.sh, the diffs should be the same as they are now, but on different files.) Then create a PR and I'll merge it. Or let me know if you want me to do this myself (but you get no credit). Again, thanks for your work on this. |
I've done some more testing, and this fails, at least on macOS 10.11.6 – DNS stops working. I'm not sure why, but suspect it's "Don't edit old service but create new service instead". |
Don't edit old service but create new service instead Fix IPv6 addresses not working (macOS does resolve AAAA too now) Fix DNS resolver for scoped queries becoming unreachable (old interface was used with address given by vpn server, probably it could've even caused dns leak)
@jkbullard what is the output of |
--dns shows
The output from the up script is
|
@stek29 - And here is what it is before connecting to the VPN:
|
Does DNS work in Google Chrome? Do you see anything from mDNSResponder in Console.app? |
No, I get "This site can’t be reached. tunnelblick.net’s server IP address could not be found."
Only the |
@jkbullard does |
|
Ok, I've been looking into how to get more verbose logging out of configd and IPMonitor, but it didn't work on macOS 10.13 -- maybe it would on OS X 10.11: Run this with sudo: https://gist.github.com/stek29/2f237323214ba766a579ef85a32cdda7 (or maybe you can just create /Library/Preferences/SystemConfiguration/com.apple.IPMonitor.control.plist with needed key/value -- Verbose: true) Then try to see if there's something interesting in console.app (filtering by "IPMonitor" or even "configd should work). I have compared configd 802.40.13 (10.11.6) and configd 963.30.1 (10.13.3) -- but there are way too much changes to review all of them, and I can't see anything obvious there. The only problem I can suspect is that vpn service isn't added to services list.
|
I'd like to take a step back: what is the purpose of adding a new service? |
@jkbullard it seems to be cleaner than hijacking existing service, but I see your point -- just hijacking both ipv4 and ipv6 might work too. |
@stek29 - Attached are the before and after outputs from Note that /etc is a symlink to /private/etc which exists but /private/etc/resolv.conf is a symlink to /private/var/run/resolv.conf which no longer exists after connecting but is restored after disconnecting! I think this means that the SC database entries are confusing the code that generates resolv.conf, so it is not recreated (as it usually is after network changes). Output from client.4.tunnelblick.sh:
Here's a diff -u of the before and after:
|
Oops, here are the attachments. |
Well, for some reason it doesn't see any resolvers, and that's why etc/resolv isn't created -- it'd be empty anyway. I'll try to add ipv6 support in old way with service hijacking. |
How is the issue going on People? |
@theNerdFromSiliconValley Honestly I have lost interest in it since I've switched to IPSec. Feel free to adjust the code and get some PR fixing this merged. |
noted |
"Fix IPv6 addresses not working (macOS does resolve AAAA too now) Fix DNS resolver for scoped queries becoming unreachable (old interface Fix copy-paste error in down script Handle dhcp-option DNS6" You wanted to fix issues like this, After your pull request other people have committed 14 commits here, If you can tell how many problems exist among what you mentioned earlier, it will help me to fix those issues |
Don't edit old service but create new service instead
Fix IPv6 addresses not working (macOS does resolve AAAA too now)
Fix DNS resolver for scoped queries becoming unreachable (old interface
was used with address given by vpn server, probably it could've even
caused dns leak)
Fix copy-paste error in down script
Handle dhcp-option DNS6
I have not tested it on anything other than macOS High Sierra with IPv4 only network connecting to openvpn server giving out IPv4 and IPv6 addresses. Haven't tested on older OS X or on different configurations.
Also, Probably other scripts/executables (i.e. process-network-changes) need to be updated -- please tell me if there's more stuff to fix.
IMHO these scripts need to be carefully reviewed/rewritten, with different versions for different OSVER if dropping older OS X support isn't an option.
And SystemConfiguration usage/"schema" needs to be documented too.