Skip to content

fixed DNS via Proxy #2328

Merged
drwetter merged 5 commits into3.1devfrom
w4ntun-merge
Mar 23, 2023
Merged

fixed DNS via Proxy #2328
drwetter merged 5 commits into3.1devfrom
w4ntun-merge

Conversation

@drwetter
Copy link
Copy Markdown
Collaborator

@drwetter drwetter commented Mar 3, 2023

Merged from #2295 and whitespaces corrected, for further work.

The current version of the script returns an error when it not possible to determine IP addresses through DNS resolution, even if --proxy and --ip=proxy flags are set. The main function always tries to determine IP addresses via DNS and exits with a fatal error if it cannot do it. Although the client cannot get the IP, the proxy could, so the SSL/TLS analysis is still possible.

The version proposed allows the analysis for an HTTP service via a proxy server and the DNS traffic can be sent directly or through the proxy using the flag --ip=proxy.

@drwetter
Copy link
Copy Markdown
Collaborator Author

drwetter commented Mar 3, 2023

Comment copied over:

The logic itself seems almost fine. The whole matter not straightforward though.

What bugs me is that your PR is a breaking one for those who don't have a local resolver. They now need to add --ip=proxy to get back the old behavior. One could see that then as "works as intended" as before it did not. ATM I don't see an clean way to address that as determine_ip_addresses() checks DNS resolving but at the same time it exists if it doesn't resolve. Maybe checking $PROXY in determine_ip_addresses() somewhere at [[ -z "$IPADDRs" ]]; would help and then in the calling part remove the if condition and only use the else condition? Haven't tried.

Formally: If possible use 5 x spaces as tab and don't add white spaces after the text

@drwetter drwetter mentioned this pull request Mar 3, 2023
drwetter added 3 commits March 3, 2023 12:47
See #2328, original PR #2295 from @w4ntun .

Formally testssl.sh returned an error when it wasn't not possible to determine IP
addresses through DNS resolution, even if --proxy and --ip=proxy flags are set.
The main function always tried to determine IP addresses via DNS and exits with
a fatal error if it cannot do it. Although the client cannot get the IP, the
proxy could, so the SSL/TLS analysis is still possible.

This PR allows the analysis for an HTTP service via a proxy server and the DNS
traffic can be sent directly or through the proxy using the flag --ip=proxy.

ATTENTION: This may be a breaking change for those who don't have a local resolver.
They now have to add --ip=proxy.

In addition:
* help() was amended to add --ip=proxy (was only in the ~i/doc dir before)
* amending ~/doc dir to document it's better to add --nodns=min when there's
  no local resolver
@w4ntun
Copy link
Copy Markdown

w4ntun commented Mar 21, 2023

Hi @drwetter!

Sorry for being away for a long time. I have written a cleaner version of the code according to your comments. I totally agree with you when saying that the solution is not straightforward for the user but I wanted to keep the possibility of scanning multiple IPs resolving the same hostname even using a proxy (only if local resolver is available). If the DNS resolving is done by the proxy, only one IP address will be scanned although more than one could be available.

I think the behavior should be as follows:

  • If the --proxy option is set, the DNS request must be sent through the proxy as it is de default behavior when using a proxy server. So in the new version, only the --proxy option needs to be set.
  • If the --ip option is set, the DNS resolving is not made by the proxy and we have multiple options: the value of an specific IP (no need to DNS resolving), the value of one (when obtaining multiple IPs from the local resolver, just keep the first one) or the value of all (et the IP addresses from a local resolver and scan all of them).

I have included the $PROXY and $DNS_VIA_PROXY checks into determine_ip_addresses() as you suggested and I think that the code in the main function looks cleaner too.

I don't know if you would like to have a look at this solution or you prefer to keep the current want. If needed, I can open a new PR.

All the best.

@drwetter
Copy link
Copy Markdown
Collaborator Author

Hi @w4ntun ,

thanks! I certainly want to have a look. I'd like though to merge this one first.

@drwetter drwetter merged commit f95d0dd into 3.1dev Mar 23, 2023
@drwetter
Copy link
Copy Markdown
Collaborator Author

If the --proxy option is set, the DNS request must be sent through the proxy as it is de default behavior when using a proxy server. So in the new version, only the --proxy option needs to be set.

There are different types of DNS requests: A, AAAA, TXT, MX and the one for CAA records. Please keep in mind that if --proxy those won't work through most proxies. OTOH there might be still a local resolver which one can use. I have't looked at your suggestion yet but --ip=proxy seems to me a better fit as the user can determine that the resolution takes place exclusively at the proxy.

@drwetter drwetter deleted the w4ntun-merge branch March 23, 2023 13:39
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