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

Jump to bottom

Change how host timeouts are initialized in ultrascan. #1150

Closed
wants to merge 1 commit into from
Closed

Change how host timeouts are initialized in ultrascan. #1150

wants to merge 1 commit into from

Conversation

jsiembida
Copy link

When targets are subject to congestion control, their timeout clocks
should not be initialized to the same time, otherwise they are being
completed without even trying. Command that triggers the issue in AWS:

nmap -sT -T3 --host-timeout 5 -n --max-hostgroup 100 -Pn -p 8080 a.b.c.d/24

Change how host timeouts are initialized in ultrascan.
9ab56ab 
When targets are subject to congestion control, their timeout clocks
should not be initialized to the same time, otherwise they are being
completed without even trying. Command that triggers the issue in AWS:

  nmap -sT -T3 --host-timeout 5 -n --max-hostgroup 100 -Pn -p 8080 a.b.c.d/24
@dmiller-nmap
Copy link

This is an interesting idea. I can see how the existing behavior could be a problem. I will have to do more review and testing to see if this is a complete fix, but for now, here are my concerns:

  1. What happens to the startTimeOutClocks function? Is it called anywhere else? Could it be eliminated?
  2. Why call gettimeofday in the debug statement, when we are actually using a cached timeval (USI->now) to set the timeout clock? The debug statement should print the actual time used, or it should be eliminated.
  3. Does this problem affect the other startTimeOutClocks functions in service_scan.cc and osscan2.cc? Should those places be changed, too?

@jsiembida
Copy link
Author

  1. Left "just in case", but it is not used any more.
  2. Overlooked that.
  3. I am not sure, only fixed the scanner we are using. But a quick look suggests the timeout logic is similar, therefore likely to show the same behavior.

@dmiller-nmap
Copy link

I've made slight adjustments and will be committing this soon. Would you like a different name or full name credit in the CHANGELOG, or just your Github username?

@jsiembida
Copy link
Author

I am happy with whatever is more convenient to you.

@nmap-bot nmap-bot closed this in d8ff55b Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jsiembida @dmiller-nmap