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

Error if a loopback name server is used with a non-loopback local address, or vice versa #396

Merged
merged 9 commits into from
Jul 11, 2024

Conversation

phillip-stephens
Copy link
Contributor

Closes #309

Changes

  • use built-in loopback detection
  • error if name server is loopback and local address. is not, or vice versa.

Description

In #389, we added this sort of "can't use a nameserver/local address that aren't both loopback or both not" validation to the config. But it is also possible to specify a name server in an individual input line: echo "cloudflare.com,127.0.0.53" | ./zdns A. In this case, our config validation will not error if the local address mismatches, since we're bypassing the config/CLI inputs to ZDNS.

Solution is to validate in the resolver.Lookup itself. I think this should be done in addition to the config validation rather than replacing it, since catching the config error earlier leads to better UX.

Even prior to this change, it was not possible to specify both a loopback and non-loopback name server in the same scan. The mismatched query would timeout. This change just adds a descriptive error for the user to remediate the issue.

Example post-fix:

~/zdns on  phillip/309-local-address-selection! ⌚ 22:43:25
$ echo "cloudflare.com,127.0.0.53\nyahoo.com,1.1.1.1" | ./zdns A --verbosity 5
INFO[0000] No name servers specified. will use: 1.1.1.1:53
INFO[0000] using local address(es): [171.67.71.209]
{"data":null,"error":"nameserver 127.0.0.53 must be reachable from the local address 171.67.71.209, ie. both must be loopback or not loopback","name":"cloudflare.com","status":"ILLEGAL_INPUT","timestamp":"2024-07-10T22:45:00Z"}

@phillip-stephens phillip-stephens marked this pull request as ready for review July 10, 2024 22:59
@phillip-stephens phillip-stephens requested a review from a team as a code owner July 10, 2024 22:59
@phillip-stephens phillip-stephens requested a review from zakird July 10, 2024 23:00
@phillip-stephens phillip-stephens merged commit dd9c896 into main Jul 11, 2024
4 checks passed
@phillip-stephens phillip-stephens deleted the phillip/309-local-address-selection branch July 11, 2024 16:25
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.

SERVFAIL
2 participants