Skip to content
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

2.4/dev Analyzers for Threatfox, MalwareBazaar, Echotrail, Elasticsearch #12003

Merged
merged 27 commits into from Dec 19, 2023

Conversation

HoangLongVu
Copy link

Completed Analyzers for ThreatFox, MalwareBazaar, Echotrail, Elasticsearch
Each analyzer follows a similar format as the existing analyzers. They include the code itself, a unit test file, README, metadata and a configuration file where applicable.

@jertel
Copy link
Contributor

jertel commented Dec 13, 2023

There are a few references to "Elastic Search" and "ElasticSearch". Those should be corrected to the proper name "Elasticsearch".

@weslambert
Copy link
Contributor

weslambert commented Dec 14, 2023

Can you please update the Supported Observable Types and Authentication sections here:

https://github.com/HoangLongVu/securityonion/blob/2.4/dev/salt/sensoroni/files/analyzers/README.md

@weslambert
Copy link
Contributor

Thanks for updating the README! The pull request is definitely looking better, but It looks like some of the files are not passing the flake8 test.

https://github.com/Security-Onion-Solutions/securityonion/actions/runs/7211127313/job/19646322180?pr=12003

The build.sh script should perform the test for you, or you can use the command it uses to lint the files.

Additionally, I noticed the Elasticsearch analyzer does not have an option to fall back to verify=False for the request. While not required, it might be useful to have an option for individuals who are not able to easily provide the necessary certificate. In this case, the default behavior would be to verify unless a key is set to disable verification.

Let me know if you have any questions or thoughts. Thanks!

@HoangLongVu
Copy link
Author

Hi Mr Lambert, Could you let us know if we need to fix all the errors that are given or just the one that has letter E.

@weslambert
Copy link
Contributor

Hi Mr Lambert, Could you let us know if we need to fix all the errors that are given or just the one that has letter E.

All of the errors mentioned will need to be fixed so the build will be compliant with the requirements in the pytest.ini file.

https://github.com/Security-Onion-Solutions/securityonion/blob/2.4/dev/pytest.ini

@weslambert
Copy link
Contributor

Thanks for the additional changes @semphorin! Would it be possible to have the commit signed? It doesn't look like it is verified at the moment.

@weslambert weslambert merged commit 69472e7 into Security-Onion-Solutions:2.4/dev Dec 19, 2023
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants