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

External asset database boefje #1175

Merged
merged 13 commits into from
Jul 3, 2023
Merged

Conversation

zcrt
Copy link
Contributor

@zcrt zcrt commented Jun 14, 2023

Changes

This pull request is a generalization of https://github.com/minvws/nl-kat-coordination/tree/feature/dadb and introduces an external database boefje (previously DADB), where hostnames, IPs and NetBlocks can be fetched from an external API. See description.md for more details.

Proof

image

Related, not solved in this PR:

#188
#131

Code Checklist

  • All the commits in this PR are properly PGP-signed and verified;
  • This PR only contains functionality relevant to the issue; tickets have been created for newly discovered issues.
  • I have written unit tests for the changes or fixes I made.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have performed a self-review of my code and refactored it to the best of my abilities.

Communication

  • I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
  • I have made corresponding changes to the documentation, if necessary.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.


Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

@zcrt zcrt requested a review from a team as a code owner June 14, 2023 08:08
ammar92
ammar92 previously approved these changes Jun 27, 2023
@Darwinkel
Copy link
Contributor

Looks great! I'm not sure how to QA test this though, as it requires additional software which implements this API spec server-side. Is there any reference implementation which others can use?

Also, not 100% sure what our policy on unit-testing boefjes is (some, but not all appear to have them), but I'm thinking that it would be nice to have some basic tests for the parsing and yielding.

@Donnype Donnype added boefjes Issues related to boefjes backend plugins labels Jun 28, 2023
@ammar92
Copy link
Contributor

ammar92 commented Jun 28, 2023

I was testing it out but I had the following error with the input you gave me yesterday:

zcrt-normalizer-1  | [2023-06-28 08:16:58 +0000] [7] [INFO] [job_handler] Handling normalizer kat_external_db_normalize[6bf117ae397a46b5a385ae04ff68c969]
zcrt-normalizer-1  | [2023-06-28 08:16:59 +0000] [7] [INFO] [local] Running local normalizer plugin
zcrt-normalizer-1  | [2023-06-28 08:16:59 +0000] [7] [ERROR] [app] An error occurred handling scheduler item 6bf117ae397a46b5a385ae04ff68c969
zcrt-normalizer-1  | Traceback (most recent call last):
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/app.py", line 105, in start_working
zcrt-normalizer-1  |     item_handler.handle(p_item.data)
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/job_handler.py", line 183, in handle
zcrt-normalizer-1  |     results = self.job_runner.run(normalizer_meta, raw)
zcrt-normalizer-1  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/local.py", line 73, in run
zcrt-normalizer-1  |     return self._parse_results(normalizer_meta, results)
zcrt-normalizer-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/local.py", line 76, in _parse_results
zcrt-normalizer-1  |     parsed: List[NormalizerResult] = [self._parse(result) for result in results]
zcrt-normalizer-1  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/local.py", line 76, in <listcomp>
zcrt-normalizer-1  |     parsed: List[NormalizerResult] = [self._parse(result) for result in results]
zcrt-normalizer-1  |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/plugins/kat_external_db/normalize.py", line 38, in run
zcrt-normalizer-1  |     for address_item in follow_path_in_dict(path=IP_ADDRESS_LIST_PATH, path_dict=results):
zcrt-normalizer-1  |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
zcrt-normalizer-1  |   File "/app/boefjes/boefjes/plugins/kat_external_db/normalize.py", line 27, in follow_path_in_dict
zcrt-normalizer-1  |     raise KeyError(f"Key {key} not in {list(path_dict.keys())}")
zcrt-normalizer-1  | KeyError: "Key ip_addresses not in ['id', 'boefje_meta', 'mime_types', 'secure_hash', 'signing_provider_url', 'hash_retrieval_link']"

@zcrt
Copy link
Contributor Author

zcrt commented Jun 28, 2023

I was testing it out but I had the following error with the input you gave me yesterday

The file I sent you contains two files (one with metadata and one with IP/domain data). This file was downloaded directly from the KAT interface on a succesfull normalizer run. Can it be that a normalizer download does not translate back to a normalizer input?

@ammar92
Copy link
Contributor

ammar92 commented Jun 28, 2023

I was testing it out but I had the following error with the input you gave me yesterday

The file I sent you contains two files (one with metadata and one with IP/domain data). This file was downloaded directly from the KAT interface on a succesfull normalizer run. Can it be that a normalizer download does not translate back to a normalizer input?

Ah I figured it out, I accidentally loaded the wrong file. My bad!

Seems to work and ready for merge.

boefjes/boefjes/plugins/kat_external_db/boefje.json Outdated Show resolved Hide resolved
boefjes/boefjes/plugins/kat_external_db/normalize.py Outdated Show resolved Hide resolved
boefjes/boefjes/plugins/kat_external_db/normalize.py Outdated Show resolved Hide resolved
boefjes/boefjes/plugins/kat_external_db/description.md Outdated Show resolved Hide resolved
@underdarknl underdarknl merged commit 883caec into minvws:main Jul 3, 2023
jpbruinsslot added a commit that referenced this pull request Jul 5, 2023
* main:
  Fix hanging worker processes on a SIGKILL (e.g. when the container is out of memory) (#1187)
  Remove unused method from DNSReport view (#1322)
  Rename pyc test files because they are overwritten (#1331)
  Add documentation about Config OOI's (#1307)
  Add Poetry configs for `bytes`, `boefjes`, `rocky`, and `mula` (#1295)
  Fix mermaid diagrams in docs for Bytes and Octopoes (#1311)
  Retrieve and store env and code hashes of a Boefje job (#1227)
  Mute Findings in bulk at Finding list (#1165)
  change TOC for usermanual (#1298)
  Add anchor tag to jump to after submitting the "set clearance level bulk form" on OOI list (#1315)
  Align inline buttons when 2 or more buttons are available (#1321)
  fix ooi form for netblocks (#1316)
  External asset database boefje (#1175)
  Change port classification bit to ip address (#1172)
  Handle RabbitMQ channel error in bytes (#1304)
  Fix object page ignoring the filters for JSON and CSV export (#1300)
dekkers pushed a commit that referenced this pull request Jul 25, 2023
Co-authored-by: Patrick <[email protected]>
Co-authored-by: Roelof Korporaal <[email protected]>
Co-authored-by: ammar92 <[email protected]>
@dekkers dekkers mentioned this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend boefjes Issues related to boefjes plugins
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants