-
-
Notifications
You must be signed in to change notification settings - Fork 505
Added CommandSequenceAPI to GreedyBear #2902
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
Conversation
JSON Results
|
api_app/analyzers_manager/migrations/0158_analyzer_config_greedybear.py
Outdated
Show resolved
Hide resolved
88de46a
to
9644795
Compare
could you do a screenshot with an example of an IP with a real command sequence? I dont remember whether you already have an account on the honeynet GB to catch this info |
I tried a dozen of IPs with commandSequenceAPI and i was just getting the same result : #2902 (comment), where you can see the What do you think, should I try some more? Let me know |
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.
last tweaks and we are ready. I'll find an observable that you can test fully
command_sequence_uri = "/api/command_sequence" | ||
|
||
result = {} | ||
if get_hash_type(self.observable_name) == "sha-256": |
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.
now I notice another problem. If an user analyze an md5 it would go through the else while, on the contrary, it should get an error message saying it is not supported. Please check the obs classification first and then check the has type
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.
Earlier, I had handled the case, but then I realised, wouldn't this be taken care of by
run_hash=True,
run_hash_type="sha256",
in analyzer config, so I dropped the idea of handling it.
Let me know what you think
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.
about that, that's another unecessary addition. run_hash and run_hash_type must be set only in case we want to run a file analyzer as an observable analyzer (case when we want to check the hash of a file).
So, no, this does not solve the problem. Also, please remove that configuration from the analyzer cause it is not needed.
You can try to analyze both an md5 and a sha256 to test this
enrichment_response = requests.get( | ||
self.url + enrichment_uri, params=params_, headers=headers | ||
) | ||
result["enrichment_results"] = enrichment_response.json() |
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.
also about retrocomoatibility. Please keep old results in the same json hierarchy as they were before. Create a new hierarchy only for the new data
@mlodic, I hope the implementation is fine according to you. Let me know if we have to write seperate test for this because i guess hash of another type is being passed which is raising the error and as a result default Test fails. |
great! |
description="Scan an IP, domain or a command sequence hash against the [GreedyBear](https://www.honeynet.org/2021/12/27/new-project-available-greedybear/) service", | ||
observable_supported=["ip", "domain", "hash"], | ||
run_hash=True, | ||
run_hash_type="sha256", |
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.
as mentioned these values are not needed
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.
Sure, I'll remove them.
|
||
if ( | ||
self.observable_classification == Classification.HASH | ||
and get_hash_type(self.observable_name) != "sha-256" |
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.
in this way you are running the get hash method 2 times. please do that once
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.
Sure, I'll make the intended changes.
and get_hash_type(self.observable_name) != "sha-256" | ||
): | ||
raise AnalyzerRunException( | ||
"GreedyBear does not support hashes other than SHA-256." |
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.
considering the tests are being reworked, for this case dont raise an analyzer exception for now, make the analyzer end with a success but add an error message to the output. in this way the test will work. not the recommended way but it is fine for now
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.
Alright, I pass the error in success response. When the refactoring of the testing suite is complete, I'll add the tests for this analzyer.
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.
great! just wait for the start of the next week so that I can provide an IP address to test
Analyzer Result when
|
1a581ba
to
ffd557d
Compare
ffd557d
to
326d201
Compare
great! |
closes #2768
Description
This PR aims to support fetching details from CommandSequenceAPI from Greedybear, inside existing greedybear analyzer.
Type of change
Please delete options that are not relevant.
Checklist
develop
dumpplugin
command and added it in the project as a data migration. ("How to share a plugin with the community")test_files.zip
and you added the default tests for that mimetype in test_classes.py.FREE_TO_USE_ANALYZERS
playbook by following this guide.url
that contains this information. This is required for Health Checks (HEAD HTTP requests)._monkeypatch()
was used in its class to apply the necessary decorators.MockUpResponse
of the_monkeypatch()
method. This serves us to provide a valid sample for testing.DataModel
for the new analyzer following the documentation# This file is a part of IntelOwl https://github.com/intelowlproject/IntelOwl # See the file 'LICENSE' for copying permission.
Black
,Flake
,Isort
) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.tests
folder). All the tests (new and old ones) gave 0 errors.DeepSource
,Django Doctors
or other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.Important Rules