-
-
Notifications
You must be signed in to change notification settings - Fork 626
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
Scope Accuracy Rework #772
Conversation
90b760f
to
14f38dc
Compare
8dd6703
to
e48c0c6
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #772 +/- ##
======================================
+ Coverage 92% 92% +1%
======================================
Files 286 287 +1
Lines 17290 17901 +611
======================================
+ Hits 15831 16459 +628
+ Misses 1459 1442 -17 ☔ View full report in Codecov by Sentry. |
I am approving for dev, but my intention is to test this extensively prior to the dev->stable push. However, I can't fully grok all of it in a reasonable amount of time. I see no issues through my review, and the extensive tests add a lot of reassurance. The potential consequences of scope mishaps warrants an extra bit of caution, though. |
1736f46
to
00de082
Compare
f3840cc
to
c66c7fa
Compare
0c14b71
to
99ae8e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will test changes when able
This PR makes several important changes to BBOT's event handling logic that fixes numerous small and annoying edge cases (including #757, which prompted this effort) and generally simplifies the code.
Because BBOT does speculative discovery via internal events (events that are distributed only to scan modules, but not to output modules - such as far-out DNS resolutions, speculated open ports, unverified URLs, etc.), and because when outputting to a graph such as Neo4j, these chains of events must be preserved to prevent orphans, there was a lot of ugly logic that accumulated over time where we were making special exceptions for Neo4j. This caused unexpected behaviors, and led to even more exceptions for those exceptions, etc.
Originally, I used BBOT's built-in event deduplication logic to shepherd these chains of events and prevent orphans in the graph. But the implementation was overly complex, difficult to maintain and still had bugs. So it has been completely reworked and these two systems have been decoupled.
The result is two cleaner and simpler systems instead of one overly complex one. Extensive tests have been written to ensure the systems work as intended.
By necessity, a new DNS mock system has been added which enables more elaborate and more effective tests.
This was a challenging rework. If you're interested in the details, please see the thread for this issue where I typed out my thought process.