search

TheresAFewConors / Sooty

The SOC Analysts all-in-one CLI tool to automate and speed up workflow.
GNU General Public License v3.0
1.31k stars 205 forks source link

Added Phishtank Support #38

Closed naveci closed 4 years ago

naveci commented 4 years ago

Description

Added Phishtank support in this PR. I wrote it as a separate module for the following reasons:

The config needs extra entries to make phishtank work (there is a simple check to see if there are phishtank entries configured). The example-config.yaml file has been updated accordingly.

Does this fix a known existing bug under Issues?

N/A

Type of Change

Please delete any options that do not apply here:

Any further info related to the addition

Bumped version to 1.3.2

I think it would be wise to start describing the config options in the wiki, especially given the local database (download) option for the Phishtank module.

naveci commented 4 years ago

commit 11ca16d fixes the ascii-chars from issue 37

naveci commented 4 years ago

For Phishtank:

  • Could you answer what the 'app name' field in config is for? Is it required?
  • I think its best to keep the Phishtank module within Sooty itself, rather than as a separate tool. Streamlines the usage. Use of the menu to open the pre-existing phishing module, and from there a separate module for your Phishtank block would work well I think. (all phishing tools combined).
  • Besides that everything looks good.

Would you be able to set it that way? The phishtank module launched through Sooty only and part of the phishing tool suite? All credit would of course go to you.

Existing: Nice fix for #37

So to answer your points:

  1. The app name is part of the API. You get a key and have to set up an app name, which the tool needs to add to the requests POST. Nothing I can do about that (apart from renaming in config.yaml and the code of course (if needed). I cannot document this under the wiki as I lack the rights to do so. Documenting under a fork won't merge with a PR (i think).
  2. I understand your point. I saw the Menu being in a separate folder and thought to myself it might make more sense to do that for a new module as well. Personally, i prefer splitting up items (sometimes I go slightly overboard). Why?:
    • you can run in it standalone, which is much faster and nicer when debugging (which takes the most time by far).
    • It also keeps the code a bit more compact in the Sooty.py file and thus more easily searchable.
    • it's easier to re-use some of the code elsewhere as I've separated actual functions in different functions or methods which can be easily referenced to from within Sooty.py.
    • After getting everything to work as intended, only a minor piece of code is required in Sooty.py to integrate it.

I wrote another script that does the exact same thing (separate file) and it was super helpful having it in a separate file for troubleshooting. I haven't PR-ed this yet, awaiting this one first.

TheresAFewConors commented 4 years ago

The app name is part of the API. You get a key and have to set up an app name, which the tool needs to add to the requests POST. Nothing I can do about that (apart from renaming in config.yaml and the code of course (if needed).

In that case should the app name just be set to Sooty? Or do people have to create their own name as part of the process? I want to make sure this is documented correctly!

I understand your point. I saw the Menu being in a separate folder and thought to myself it might make more sense to do that for a new module as well. Personally, i prefer splitting up items

One of the next changes that will have to happen is another refactor to split the code out into individual modules & files. The goal here being that modules can be developed entirely separate to Sooty and joined on / hot-swapped as needed, with Sooty being a sort of manager and launcher for the modules.

I do agree that splitting files up is better, but I also want to keep the use of tool simple, if there's multiple ways to use it then it could get complicated both for users and documenting. If you prefer creating a new option in the main menu for 'Additional Tools' that could also work as the urlScan option could be moved there also.

The additional script for iplists would be beneficial to be added to the Reputation check. Once we get this PR completed we can work on that too.

Nice work btw - these are great additions.

naveci commented 4 years ago

Sorry for the late response.. life got in the way.

  1. The app name has to be unique on the site and every user needs to create their own app name. Nothing we can do about that. It's actually superstrange that you have a site-wide unique app name PLUS an API key, but sure... that's how it works.

  2. Sorry if things were unclear. It's created a separate module, because it makes my life so much easier testing and debugging it. The amount of code then to integrate it into the 'manager' like you said can really be kept to a minimum. Hence why it is separate and why I would prefer to keep it in that separate file. In the future, this would also allow you to write automated tests more easily through for example github's tasks (although my knowledge in that department is lacking).

The iplists i will merge separately, after this one, because it keeps the discussion a bit cleaner. But same thing there, tried to make it flexible and extendible.

TheresAFewConors commented 4 years ago

No worries at all.

The app name has to be unique on the site and every user needs to create their own app name.

That is strange that an app name and an API key are both required - ill update the docs and requirements with this info - thanks for the update.

It's created a separate module.

That's fine with me - as long as we can integrate it 'menu-wise' with the other phishing tools I think it will work really well.