-
Notifications
You must be signed in to change notification settings - Fork 299
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
DomainTools Expert Bot (initial version) #1004
base: develop
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1004 +/- ##
===========================================
+ Coverage 75.8% 78.06% +2.26%
===========================================
Files 216 223 +7
Lines 9108 9136 +28
Branches 1215 0 -1215
===========================================
+ Hits 6904 7132 +228
- Misses 1920 2004 +84
+ Partials 284 0 -284
Continue to review full report at Codecov.
|
e848623
to
5972061
Compare
5972061
to
5348b39
Compare
pls review |
score = self.domaintools_get_score(event.get(key_fqdn)) | ||
if score is not None: | ||
extra["domaintools_score"] = score | ||
event.add("extra", extra) |
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.
This fails if extra is already present, with force it would overwrite existing data
|
||
try: | ||
score = resp['risk_score'] | ||
except exceptions.NotFoundException: |
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.
you can use except (exceptions.NotFoundException, exceptions.BadRequestException)
|
||
def domaintools_get_score(self, fqdn): | ||
score = None | ||
if fqdn: |
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.
That's already checked in the process()
method
except exceptions.NotFoundException: | ||
score = None | ||
except exceptions.BadRequestException: | ||
score = None |
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.
If nothing is returned explicitly, the return value is None
anyway
"extra": '{"domaintools_score": 0}', | ||
"time.observation": "2015-01-01T00:00:00+00:00" | ||
} | ||
NONEXISTING_INPUT = {"__type": "Event", |
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.
Actually the lib raises the BadRequestException
for this test which is not equivalent to 'not existing', but the result is similar.
@classmethod | ||
def set_bot(self): | ||
self.bot_reference = DomaintoolsExpertBot | ||
self.sysconfig = {'user': 'mkendrick_first2017', 'password': 'c0e4e-e2527-dc6af-824a4-229d5'} |
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.
Is this here intentionally?
self.logger.info("Loading Domaintools expert.") | ||
if (not API): | ||
self.logger.exception("need to have the domaintools API installed. See https://github.com/domaintools/python_api") | ||
self.stop() |
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.
Wrong usage of logger.exception
. It prints the traceback of a fromerly catched exception.
Replace both lines with raise ValueError("your error message")
. The bot class performs the stop itself.
self.logger.exception("need to have the domaintools API installed. See https://github.com/domaintools/python_api") | ||
self.stop() | ||
if (not self.parameters.user): | ||
self.logger.exception("need to specify user for domaintools expert in runtime.conf. Exiting") |
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.
Missing dot at end of line. And you can remove the "Exiting", that's done by the bot class.
score = resp['risk_score'] | ||
except exceptions.NotFoundException: | ||
score = None | ||
except exceptions.BadRequestException: |
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.
Is a bad request really the same as no result? Just raise the error, so the bot can retry.
Done |
Do you want to work on these issues or should I revise this bot? |
I did some work on this one. Waiting confirmation from @aaronkaplan |
Applied improvements on DomainTools Expert
I don't have an API key, so can't test this at the moment |
Closing because of inactivity. Please reopen if needed. |
definitely needed. Reopening. |
Is this PR still being worked on? |
demo of how to use the domaintools expert to fetch scoring for a domain