Skip to content

Update dns resolution for socks proxies #19683

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

Conversation

adfoster-r7
Copy link
Contributor

@adfoster-r7 adfoster-r7 commented Nov 28, 2024

Improves #19641

Requires rapid7/rex-socket#69

Update dns resolution for socks proxies. Now local DNS resolution will be skipped on the host machine and deferred to any configured socks proxies, i.e. with setg proxies socks5:127.0.0.1:1080

Verification

Ensure CI passes

@adfoster-r7 adfoster-r7 force-pushed the update-dns-resolution-for-socks-proxies branch 3 times, most recently from bb3d05c to 2c78d21 Compare November 28, 2024 17:44
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will extract out to a separate PR; I noticed it's possible to assign invalid values to the proxies datastore option. This should tighten things up, but the UX still isn't great IMO

@smcintyre-r7 smcintyre-r7 self-assigned this Dec 2, 2024
# @return [Boolean] True if DNS resolution should be performed the RHOST values, false otherwise
def perform_dns_resolution?(datastore)
# If a socks proxy has been configured, don't perform DNS resolution - so that it instead happens via the proxy
!(datastore['PROXIES'].to_s.include?(Rex::Socket::Proxies::ProxyType::SOCKS4) || datastore['PROXIES'].to_s.include?(Rex::Socket::Proxies::ProxyType::SOCKS5))
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably time we differentiate between SOCKS4 and SOCKS4A since 4 doesn't support hostnames while 4A and 5 do and that's what we're doing here.

Copy link
Contributor Author

@adfoster-r7 adfoster-r7 Dec 2, 2024

Choose a reason for hiding this comment

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

Let's do HTTP and socks5; and we can circle back to implementing socks4a if that comes up (as we explicitly resolve locally in rex-socket with the current implementation), similarly with the SAPNI support

@adfoster-r7 adfoster-r7 force-pushed the update-dns-resolution-for-socks-proxies branch 3 times, most recently from a7c53a7 to 0a41f51 Compare December 5, 2024 19:48
@adfoster-r7 adfoster-r7 force-pushed the update-dns-resolution-for-socks-proxies branch from 0a41f51 to 452a386 Compare December 5, 2024 20:10
@adfoster-r7 adfoster-r7 added the attic Older submissions that we still want to work on again label Feb 27, 2025
Copy link

Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it attic and closed it for now.

What does this generally mean? It could be one or more of several things:

  • It doesn't look like there has been any activity on this pull request in a while
  • We may not have the proper access or equipment to test this pull request, or the contributor doesn't have time to work on it right now.
  • Sometimes the implementation isn't quite right and a different approach is necessary.

We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this!

@github-actions github-actions bot closed this Feb 27, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in Metasploit Kanban Feb 27, 2025
@adfoster-r7
Copy link
Contributor Author

I don't have the cycles for this, passing over to @smcintyre-r7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attic Older submissions that we still want to work on again
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants