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.

Sign up for GitHub

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

Jump to bottom

[WIP] SinkDB: fix multiple issues #498

Closed
wants to merge 2 commits into from
Closed

[WIP] SinkDB: fix multiple issues #498

wants to merge 2 commits into from

Conversation

ilyaglow
Copy link
Contributor

@ilyaglow ilyaglow commented Jun 16, 2019 •
edited
Loading

  • Rename SinkDB 3rd level domain that IP is resolved against.
  • Fix resulting IP that SinkDB returns for a sinkholed IP.
  • Handle additional categories: scanner, phishing awareness.

@ilyaglow
SinkDB: fix multiple issues
2670f78 
* Rename sinkdb 2nd level domain that IP is resolved against.
* Fix resulting IP that sinkdb returns for a sink-holed IP.
* Handle additional categories: scanner, phishing awareness.
@ilyaglow ilyaglow changed the base branch from master to develop June 16, 2019 08:14
@3c7 3c7 added scope:analyzer Issue is analyzer related status:needs-template Analyzer still needs a template for TheHive status:pr-submitted labels Jun 16, 2019
@ilyaglow ilyaglow changed the title SinkDB: fix multiple issues [WIP] SinkDB: fix multiple issues Jun 16, 2019
@garanews garanews added this to the 2.8.0 milestone May 7, 2020
@jeromeleonard
Copy link
Contributor

Hello @ilyaglow,

This PR is tagged [WIP]. Do you consider it ready to be merged ?

Thx.

@dadokkio
Copy link
Contributor

Hi @jeromeleonard, this pull and #483 are both supporting v2 sinkdb api.
In my opinion #483 is done in a better way, so we can close this and keep the other one.
I've also updated #483 to support fqdn and url in pull #756.

@ilyaglow
Copy link
Contributor Author

ilyaglow commented Jun 15, 2020 •
edited
Loading

Hello, this PR doesn’t have template changes for TheHive, but it works fine for me. At the time of submitting, I probably didn’t notice #483. However, this analyzer can run without API-key, compared to #483 and #756.

@dadokkio maybe we can incorporate in #483 DNS lookups as a fallback in case an API key is not specified?

@dadokkio
Copy link
Contributor

Yes, we could but I've some doubts (because I know dig just a little :P )
The API version works also for fqdn, domain and mail; this way will work just for ip?
In the code I see that also in your case the api key is attached to the query, in case of no api key it will lookup for ***.None.sinkdb-dnsapi.abuse.ch ?

@ilyaglow
Copy link
Contributor Author

Whoops! You're right. It still needs the API key for DNS lookups. Sorry for being sloppy :)

So yeah, this PR can be closed.

@ilyaglow ilyaglow closed this Jun 15, 2020
@dadokkio dadokkio removed this from the 2.8.0 milestone Jun 15, 2020
@dadokkio
Copy link
Contributor

No problem, thank you for the pull in any case 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
scope:analyzer Issue is analyzer related status:needs-template Analyzer still needs a template for TheHive status:pr-submitted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ilyaglow @jeromeleonard @dadokkio @3c7 @garanews