OpenVAS Parser - parsing and deduplication improvments

[jostaub]

This changed updates the OpenVAS Parser to fix the issues described in #12378. This PR was initially planned as an update the the existing parser. However after all the changes and possible problems for user we decided to move the changes into a v2 version of the parser (see linked issue).

Detailed changes:
General:

• Updated the CSV implementation to behave more like the XML parser.
• Introduced de-duplication using unique_id_from_tool to OpenVAS parser.
• Increased behavior consistency between the CSV and XML parsers.
• Combined findings where the only differences are in fields that can’t be reliably hashed due to inconsistent values between scans e.g timestamps.
• parser now combines multiple identical findings with different endpoints into one findings with multiple endpoints

CSV Parser:

• removed ip from description
• added qod to description

XML Parser:

• finding name no longer includes ip and protocol
• parser no longer appends extra information to the description (same description behavior as csv)
• severity now maps to cvss v3 score
• the description xml tag now maps to reference
• the summary inside the xml tag (part of nvt tag) now maps to description
• impact is now included in finding
• same qod behavior as csv parser

TODOs:
[x] extract more information from xml
~~[] migration for OpenVAS parser ~~
[x] improve testing with better test files

Open Questions:

1. My current implementation missuses the unique_id_from_tool variable a bit. Is this Ok?
2. I currently deduplicate on port inside the parser (also using the unique_id_from_tool). Is this Ok and/or desired at all?

[valentijnscholten]

unique_id_from_tool should not be in here, you should set DEDUPLICATION_ALGORITHM_PER_PARSER to DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL_OR_HASH_CODE for this v2 parser.

[jostaub]

see my reply to your comment.

[valentijnscholten]

Open Questions:

My current implementation missuses the unique_id_from_tool variable a bit. Is this Ok?
I currently deduplicate on port inside the parser (also using the unique_id_from_tool). Is this Ok and/or desired at all?

Using the port could work, but I don't see where you're using it?

The unique_id_from_tool must contain a value that is present in the report as it's meant to link the Finding to something that is an ID in the scanner/tool we're importing from. I have been arguing for an extra field that could contain a unique_id_from_parser, but not everyone agrees with me. Could you explain what problem gets solved by letting the parser generate an ID for deduplication (outside the parser)?

[jostaub]

Using the port could work, but I don't see where you're using it?
I use the port in the common.py file in the _hash functions.

The unique_id_from_tool must contain a value that is present in the report as it's meant to link the Finding to something that is an ID in the scanner/tool we're importing from. I have been arguing for an extra field that could contain a unique_id_from_parser, but not everyone agrees with me. Could you explain what problem gets solved by letting the parser generate an ID for deduplication (outside the parser)?

For better context:
In my case, it would not be unique. I just use the field to easily store the hash of fields that are difficult to hash during the hash generation by defectdojo.

Currently, it solves the following two things for me:

1. It allows me to de-duplicate only on port, which is currently not possible in the config.dist file.
2. I can change the severity of a finding after import without conflict because the original severity is included in the hash contained in unique_id_from_parser.

However, these points could be solved differently. For example, for point 1, it would be possible to add a new field to the finding model for organization-specific severity. For point 2, it could be solved by changing the hash implementation to allow de-duplication on the endpoint port. However, this would require a few more changes to work reliably.

To move this PR forward, I will disregard these points for now in favor of an implementation that conforms better to current standards and de duplication on endpoints.

[jostaub]

Do you have a better idea for what to call the v2 version? Following the current naming scheme, the test would be called "test_openvas_v2_parser.py." However, this looks misleading since it's a v2 of the parser, not OpenVAS. I have a similar confusion problem with the Parser Class since it needs to end in Parser to be read by algo that loads the parsers. Would a PR to change these behaviors be accepted?

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[jostaub]

I will create a new, more concise pull request to improve the review experience. Also, I still find things that need changing, and I don't want to waste more CI time.