Skip to content

Conversation

@angeloc
Copy link
Contributor

@angeloc angeloc commented Oct 6, 2021

Adding --pre-test-delay parameter

time.sleep(self.options.connected_delay)
log(STATUS, f"Client {station.get_peermac()} with IP {peerip} has connected")
station.set_ip_addresses(self.arp_sender_ip, peerip)

Choose a reason for hiding this comment

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

@angeloc There are few problems here:

  • Suggested-by is missing
  • The sleep should happen happen after the print
  • Suggest even to move after set_ip_address

@vanhoefm
Copy link
Owner

vanhoefm commented Oct 6, 2021

The delay was implemented in handle_authenticated in the following line:

self.time_connected = time.time() + self.options.connected_delay

This delay should happen after completing the 4-way handshake.

The delay that you seem to be adding is done after getting an IP address. I suppose this difference was important in your case. Can you confirm that a delay after getting the IP address was important? We can add a separate parameter for that.

@panicking
Copy link

Correct, if we don't have the delay the ping packet in receive interface is dropped. We will check it

@angeloc angeloc changed the title research/fragattack: implement connected-delay research/fragattack: implement --pre-test-delay Oct 15, 2021
@vanhoefm
Copy link
Owner

Thanks for the update, will try to find time to check this.

angeloc added 3 commits March 31, 2022 13:25
pre_delay can be used to add a delay before actually executing the test.

Suggested-by: Michael Trimarchi <[email protected]>
Signed-off-by: Angelo Compagnucci <[email protected]>
Adding a delay before actually executing the test. This can be useful in
all the cases the network stack of the victim is still not ready to
receive packets leading to a timed out test result.

Suggested-by: Michael Trimarchi <[email protected]>
Signed-off-by: Angelo Compagnucci <[email protected]>
This parameter can be used each time a test needs to be delayed before
actually executing it.

Suggested-by: Michael Trimarchi <[email protected]>
Signed-off-by: Angelo Compagnucci <[email protected]>
@vanhoefm
Copy link
Owner

Looks good. I've tweaked the code and pushed those changes (it's the first time I edited a pull request so hopefully that went well). If possible, can you confirm that the updated code works for you? Then I'll merge these changes.

@angeloc
Copy link
Contributor Author

angeloc commented Mar 31, 2022

Looks good to me!

@vanhoefm vanhoefm merged commit baa1c93 into vanhoefm:master Mar 31, 2022
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.

3 participants