-
Notifications
You must be signed in to change notification settings - Fork 764
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
Address WMI hanging #3978
Address WMI hanging #3978
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3978 +/- ##
===========================================
- Coverage 77.57% 77.49% -0.09%
===========================================
Files 467 467
Lines 14759 14787 +28
===========================================
+ Hits 11450 11459 +9
- Misses 3309 3328 +19 ☔ View full report in Codecov by Sentry. |
try: | ||
rpctransport = transport.DCERPCTransportFactory(stringBinding) | ||
rpctransport.set_connect_timeout(timeout) | ||
rpctransport.connect() | ||
rpctransport.disconnect() |
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.
So this is essentially establishing another connection and then immediately disconnecting? Isn't there another way to add a timeout?
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.
Not that I found.
14dc789
to
b51551f
Compare
b51551f
to
82d3039
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.
I'm not a fan of the logic (new connection that we're disconnecting immediately) but I don't think it's worth any more time if we're going to fix fortra/impacket#1600 eventually.
This logic is used in a couple other projects that are using impacket for WMI. We will need to fix that eventually. |
82d3039
to
9240c90
Compare
What does this PR do?
Fixes part of #3654 .
We can live without the dcom_firewall_checker if we address impacket/#1600. Until we make a PR, get review, merge, release and relock, this is the solution.
Special thanks to @XiaoliChan for the suggested solution.
PR Checklist
Testing Checklist
30 seconds timeout