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

Change how host timeouts are initialized in ultrascan. #1150

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@jsiembida

jsiembida commented Mar 13, 2018

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

jarek
Change how host timeouts are initialized in ultrascan.
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

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Mar 28, 2018

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?

dmiller-nmap commented Mar 28, 2018

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

This comment has been minimized.

Show comment
Hide comment
@jsiembida

jsiembida Mar 28, 2018

  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.

jsiembida commented Mar 28, 2018

  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

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Aug 13, 2018

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?

dmiller-nmap commented Aug 13, 2018

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

This comment has been minimized.

Show comment
Hide comment
@jsiembida

jsiembida Aug 13, 2018

I am happy with whatever is more convenient to you.

jsiembida commented Aug 13, 2018

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