-
Notifications
You must be signed in to change notification settings - Fork 79
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
New xrefs data model #225
Merged
mattmundell
merged 33 commits into
greenbone:master
from
janowagner:new_xrefs_data_model
May 16, 2019
Merged
New xrefs data model #225
mattmundell
merged 33 commits into
greenbone:master
from
janowagner:new_xrefs_data_model
May 16, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
janowagner
added
the
work in progress
This pull request should not be merged yet, more commits are expected
label
Apr 29, 2019
Can we start adding tests with this PR? We really should only add changes with tests from now on. |
janowagner
force-pushed
the
new_xrefs_data_model
branch
from
May 7, 2019 13:26
cc53c5a
to
33da42f
Compare
janowagner
force-pushed
the
new_xrefs_data_model
branch
from
May 10, 2019 10:19
33da42f
to
016b6d4
Compare
janowagner
removed
the
work in progress
This pull request should not be merged yet, more commits are expected
label
May 10, 2019
timopollmeier
previously approved these changes
May 14, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the GVMd side this works fine together with the GVMd and Scanner PRs.
mattmundell
requested changes
May 14, 2019
This introduces a struct for VT references. The nvti objects is extended to have a list of the vtref objects. Basically the new struct is handled similar to the prefs. Also, the xref element of nvti is removed and the API uses the new list of vtrefs to mimic the previous behaviour. Actually this is transitional until all modules will use the new generic references.
Also improves the xref handling slightly.
This makes them availble to other modules.
This new method allows to add a bulk of comma-separated references to the NVTI. All references share the same type and (optional) text. A typical use case is to add bulk of CVEs.
These exception are from old OTP times and the keywords should not appear anymore.
The code in nvti_add_bid and nvti_add_cve is meanwhile redundant with the code in nvti_add_refs_from_csv and thus these methods now simply call the latter. Eventually they can be eliminated once gvmd does not call them anymore. Also occured the need to const'ify vtref_new and nvti_add_refs_from_csv for doing so.
And use nvti_add_refs_from_csv instead of nvti_set_bid.
This removes nvti_set_cve(), nvti_set_bid(), nvti_add_cve() and nvti_add_bid().
One thing that needs to be considered when changing this in modules using this function is to free the returned string.
This enables nvti_ref() to exclude types and to use either simple CVS or a CSV that includes the type of each reference.
This allows to remove nvti_set_xref.
This makes the naming consistent with other functions.
For compatibility reason the CSV is using ", " instead of just ",". This is the syntax used before and other tools using the redis-database directly rely on this syntax.
This add vref_type(), vtref_id(), nvt_ref() and nvt_ref_len() to the API. These functions allow to work with the actual reference object rather than with the serialized strings. The names are choosen to be consistent with the prefs handling.
This reduces the number of code lines slightly for the g_strsplit loops. Also adding missing free's for the newly allocated split strings.
Since the element of the struct should not be used by other than the internal functions, move the struct out of the header file into the module.
This renaming is for consistent naming scheme.
The function g_malloc0() already does the check.
Removed the "A copy will be created" notes from the doc-string where const is used anyway.
The split was done for every refrence for the very same string. Now it is done only once.
Also fix a confusion about renaming function nvti_add_vtref.
janowagner
force-pushed
the
new_xrefs_data_model
branch
from
May 14, 2019 21:45
44a8ab4
to
2b4b969
Compare
mattmundell
reviewed
May 15, 2019
It is already done in the for ().
mattmundell
approved these changes
May 15, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes reflect the unified handling of cross references in the NVT meta data
in the C-libraries, primarily the structure and API around nvti_t.
Essentially a new structure vt_ref_t is introduces, a list of references owned by a nvti_t.
The explicit storage of references as strings is removed from nvti_t.
The API for handling references is reduced to generic functions like nvti_refs(), nvti_add_ref() and nvti_add_refs() that are applicable to any type of reference. All previous API elements to handle cve, bid, xref were removed.
nvt_refs() and nvti_add_refs() are pretty powerful to handle various ways to get/add lists of references. This reflects the two ways how references were used before: cve and bid as simple lists and xref as type-including syntax.
This patch corresponds with changes in modules "openvas-scanner" and "gvmd" where
the API is used. See greenbone/openvas-scanner#317 and greenbone/gvmd#526 .
For the integration test redis was flushed and openvassd restarted to fill the redis database anew.
ospd-openvas was used to retrieve all VT meta data in xml format. This was done before and after the patch. In total over 370K references are concerned and the result lists are identical before and after the patch.