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

parallel first dial using singleflight #287

Merged
merged 9 commits into from
May 23, 2024
Merged

Conversation

tarunKoyalwar
Copy link
Member

@tarunKoyalwar tarunKoyalwar commented May 19, 2024

Observed behaviour/issue

If a domain, such as hackerone.com, resolves to multiple IPs, Fastdialer previously used sequential iteration. This was inefficient because if a port on the address was closed, Fastdialer would iterate over all IPs and each failed iteration would add to the total time. This issue was exacerbated when Nuclei called this function on 1000 goroutines, creating a bottleneck and delay in execution.

Proposed Changes

After multiple investigations and experiments in #284, the issue is mitigated by the following changes:

  • Layer 5 dials (TLS, ZTLS) now use already created TCP/UDP Layer 4 connections for handshake. This is a no-op logic-wise if the domain resolves to only one IP or the given address is an IP.
  • If the domain has multiple IPs and fits the criteria, a new instance of dialWrap is created. This is a wrapper around net.Dialer that is synchronized across all current dials using a pattern similar to singleflight/simpleflight. This blocks all concurrent calls to this particular address temporarily until the firstdial function returns.
  • The firstdial function dials the given address using all existing IPs in parallel. If all dials to all IPs fail, we assume that this port is closed/filtered and use ErrPortClosedOrFiltered. This error is returned to all paused dials as well as subsequent dials to this address until this address is evicted from the cache. If at least one connection is established, this is considered successful and all successful connections are distributed across paused dials. For remaining paused dials, a new dial is initiated using dialWrap.
  • To further improve and distribute the load, this PR now follows the Happy Eyeballs Algorithm just like net.Dialer does internally when a domain is used instead of serial/sequential dials which happened earlier. In the Happy Eyeballs algorithm, IPs are split into IPv4 & IPv6 groups and each group is dialed sequentially. The first IPv6 group is given a head start with 300ms. The connection that is established first is used while the other is cancelled. This algorithm is derived from the understanding that IPv6 connections can easily be established to servers compared to IPv4 because there is no hole punching or NAT involved.

Misc changes

  • Introduced the errkit library to standardize error handling and facilitate the differentiation of permanent and temporary network errors in Nuclei and other upstream projects.
  • Implemented unit tests with goleak to identify potential goroutine leaks under various conditions.

Note

almost all of the new logic is implemented by abstracting net.Dialer at utils/dialwrap to keep refactor / changes minimal and easy to replace

Tip

Checkout Associated PR in nuclei for all benefits / improvements this PR provides

@tarunKoyalwar tarunKoyalwar marked this pull request as draft May 19, 2024 19:01
@tarunKoyalwar tarunKoyalwar self-assigned this May 22, 2024
@tarunKoyalwar tarunKoyalwar marked this pull request as ready for review May 22, 2024 20:49
@tarunKoyalwar tarunKoyalwar changed the title Singleflight first dial parallel first dial using singleflight May 23, 2024
@tarunKoyalwar tarunKoyalwar linked an issue May 23, 2024 that may be closed by this pull request
@tarunKoyalwar
Copy link
Member Author

context projectdiscovery/nuclei#5148

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

lgtm!

@Mzack9999 Mzack9999 merged commit 421193c into main May 23, 2024
5 checks passed
@Mzack9999 Mzack9999 deleted the singleflight-first-dial branch May 23, 2024 21:05
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.

Investigate parallel dial
2 participants