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

Add Boefje and Normalizer models in the KATalogus #3011

Merged
merged 31 commits into from
Jun 18, 2024

Conversation

Donnype
Copy link
Contributor

@Donnype Donnype commented May 30, 2024

Changes

Also see #2997 and the epic https://github.com/orgs/minvws/projects/7/views/33?pane=issue&itemId=60723200.

This adds the models, APIs and logic behind the following part of the new database schema:

classDiagram

class boefje {
    id
    plugin_id
    scan_level
    consumes
    produces
    version
    created
    description
    name
    environment_keys
    oci_image
    oci_arguments
}

class normalizer {
    id
    plugin_id
    scan_level
    consumes
    produces
    version
    created
    description
    name
    environment_keys
}
Loading

Some particular choices I faced where I decided myself how this should probably work, but are up for discussion:

  • For local plugins, we can also add entries with the same id to overwrite these fields:
    • name
    • description
    • scan_level
    • oci_image
    • oci_arguments
  • I removed unused fields such as options and authors for the time being.
  • We now expose a static field to be able to differentiate between plugins that can be hard-deleted and the local plugins. For now it doesn't make sense to be able to be able to delete these local plugins, perhaps never. This field is not saved on a database level though since it can simply be configured when fetching the data. The editable-fields logic above makes sure that also for local plugins we overwrite this works well.

Issue link

Closes #2997

Demo

Although next PRs will showcase more functionality and the power in the frontend, one funny thing you could to to see that this works is go to the katalogus api and add a random plugin using the post endpoint:

image

To suddenly find a new entry in your KATalogus:
image

This entry is useless unless you configure a real oci_image pointing to a boefje image though.


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.
  • I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

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.

Donnype added 20 commits May 21, 2024 23:23
Fix the integration tests
Remove repository references in Rocky
Test upgrading and downgrading with uniqueness issue
Set empty not-nullable foreign key to repository to seeded "LOCAL" on downgrades
Improve update logic for local plugins, allowing overriding only specified fields
Set type to str again since the values were not compatible
Fix other references to the app module that has been removed
Update the other relevant documentation on seeding the KATalogus database, which is no longer needed

Signed-off-by: Donny Peeters <[email protected]>
@Donnype Donnype requested a review from a team as a code owner May 30, 2024 14:21
@underdarknl
Copy link
Contributor

underdarknl commented May 30, 2024

Would it make sense to also include a path to a normalizer image? zip file / oci image or otherwise? And would it make sense to track the author? (eg, if this plugin was created by a local user, or (if null) comes from upstream?

I'd also guess that one of the things a user Might want to overwrite (without changing the actual oci_image) is the jsonschema that contains the settings their variant of the plugin consumes/reqiures

Copy link
Contributor

@ammar92 ammar92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. I do have a few remarks to improve it even more. Let me know what you think

boefjes/boefjes/katalogus/api/plugins.py Show resolved Hide resolved
boefjes/boefjes/katalogus/api/plugins.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/api/plugins.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/models.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/api/plugins.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/api/plugins.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/api/plugins.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/tests/integration/test_api.py Outdated Show resolved Hide resolved
boefjes/boefjes/katalogus/tests/integration/test_api.py Outdated Show resolved Hide resolved
@Donnype
Copy link
Contributor Author

Donnype commented Jun 4, 2024

Would it make sense to also include a path to a normalizer image? zip file / oci image or otherwise? And would it make sense to track the author? (eg, if this plugin was created by a local user, or (if null) comes from upstream?

I'd also guess that one of the things a user Might want to overwrite (without changing the actual oci_image) is the jsonschema that contains the settings their variant of the plugin consumes/reqiures

Eventually, yes custom images make sense probably. Authors could make sense, although perhaps the first iterations could live without it, especially if authors could be inferred from the oci_image somehow. I also thought about the schema, since actually now there is not support for it for these kinds of boefjes. Perhaps we need to have a json schema field as well. Could be implemented once we tie the frontend and backend together and (probably) find out we need this, and then consider if we want to change the API with respect to the schema's?

Copy link

sonarqubecloud bot commented Jun 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Donnype
Copy link
Contributor Author

Donnype commented Jun 11, 2024

@underdarknl Follow up ticket: #3052

Copy link
Contributor

@ammar92 ammar92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 I don't have any remarks anymore

@stephanie0x00
Copy link
Contributor

stephanie0x00 commented Jun 17, 2024

Checklist for QA:

  • I have checked out this branch, and successfully ran a fresh make reset.
  • I confirmed that there are no unintended functional regressions in this branch:
    • I have managed to pass the onboarding flow
    • Objects and Findings are created properly
    • Tasks are created and completed properly
  • I confirmed that the PR's advertised feature or hotfix works as intended.
  • I checked the logs for errors and/or warnings and made issues where necessary

What works:

Nothing obvious is broken. Performed scans against various targets which all seem to work as expected.

What doesn't work:

n/a

Bug or feature?:

n/a

@underdarknl underdarknl merged commit 9d31388 into main Jun 18, 2024
15 checks passed
@underdarknl underdarknl deleted the feature/boefje-normalizer-models branch June 18, 2024 13:17
@Donnype Donnype restored the feature/boefje-normalizer-models branch June 20, 2024 14:37
@Donnype Donnype deleted the feature/boefje-normalizer-models branch June 20, 2024 14:38
jpbruinsslot added a commit that referenced this pull request Jun 27, 2024
* main: (24 commits)
  Translations update from Hosted Weblate (#3154)
  Documentation on how to make a boefje, normalizer, model, bit and report with examples. (#2967)
  Fix pydantic serialization of enum on PATCH of tasks (#2944)
  RFD 0001: Add RFD for Requests for Discussion (#3002)
  Update helper text for nmap-ports boefje (#3097)
  Match frontend with design - part 2 (#3132)
  Translations update from Hosted Weblate (#3125)
  Update report overview table in report manual (#3106)
  Match frontend with design - part 1 (#3109)
  Translations update from Hosted Weblate (#3115)
  fix consistent form width (#3104)
  feat: 📝 add human readable hostnames to vulnerability report (#2952)
  Bump braces from 3.0.2 to 3.0.3 in /rocky (#3100)
  Add burp normalizer docs (#3101)
  Update feature and PR templates (#3063)
  Add Boefje and Normalizer models in the KATalogus (#3011)
  Update README.rst, add some more details ad links to conform to iReal… (#3049)
  Updated `urllib3` (#3098)
  Add Frysk to our selectable list of languages (#3090)
  Translations update from Hosted Weblate (#3091)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce the Boefje and Normalizer models in the KATalogus
4 participants