Skip to content
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

Add IPv6 compatibility to the DNS handling #30

Merged
merged 2 commits into from
Jan 3, 2020
Merged

Conversation

joelbu
Copy link

@joelbu joelbu commented Nov 14, 2019

Hey Dan

They way I did it with dig is that now it splits the DNS requests into one call that includes all the IPv4 DNS server addresses and binds to the VPN internal IPv4 address and a second call that has all the IPv6 DNS server addresses and binds to the VPN internal IPv6 address.

All of those calls ask for both the A and the AAAA record, because I didn't really see a reason why I shouldn't ask a server I contact over its IPv4 address about the AAAA record or one that I contact over IPv6 about the A record. I figured asking doesn't hurt if we're doing a request anyway.

The command line strings it assembles now look like this for my school's VPN

/usr/bin/dig +short +noedns -b 5.6.7.8 @31.31.31.31 @32.32.32.32 +domain=myschool.tld www.myschool.tld A www.myschool.tld AAAA
/usr/bin/dig +short +noedns -b 2001:1:2:3:4::ab @2001:555:8888::c +domain=myschool.tld www.myschool.tld A www.myschool.tld AAAA

I thought it over as well as I could and I don't think that I broke anything for the case where the VPN only provides an IPv4 address to the client and informs us about IPv4 DNS server addresses, but I can't test that case since I only have my school VPN, so I'll leave that part to you, Dan.

For my school it seems to work just fine. I see IPv4 and IPv6 routes in the hostfile and if I ping and traceroute an internal host the IPv6 address gets used too.
Indeed everything keeps working even if I purposely filter out the IPv4 DNS server addresses, so that it's forced to only use dig with the IPv6 DNS server address.

Best
--Joel

@dlenski
Copy link
Owner

dlenski commented Nov 19, 2019

Hey, this is an excellent and well-thought-out patch. Nicely done and thank you!

I thought it over as well as I could and I don't think that I broke anything for the case where the VPN only provides an IPv4 address to the client and informs us about IPv4 DNS server addresses, but I can't test that case since I only have my school VPN, so I'll leave that part to you, Dan.

It works fine for me. This is a common problem with VPN-related tools… not many people have access to enough different ones to be able to test all the variations. (Even I don't… hence the issue you found here. 😢)

if search_domains:
all_cls = (cl + ['+domain={!s}'.format(sd), hostname] for sd in search_domains)
for cl in some_cls:
all_cls.extend(cl + ['+domain={!s}'.format(sd), hostname, 'A', hostname, 'AAAA'] for sd in search_domains)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only nitpick here is that we should probably skip IPv6 lookup if we don't have an IPv6 bind address at all (and skip IPv4 lookup if we don't have an IPv4 bind address) since we won't be able to use the result in any way.

🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it in my new commit. The consequence is that without any bind addresses a lookup isn't made anymore.

@dlenski dlenski merged commit 23ce504 into dlenski:master Jan 3, 2020
@dlenski
Copy link
Owner

dlenski commented Jan 3, 2020

Merged with a few nitpicks. Thanks for your patience… and could you please test 23ce504, @joelbu?

@dlenski dlenski mentioned this pull request Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants