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

New xrefs data model #225

Merged
merged 33 commits into from
May 16, 2019
Merged

Conversation

janowagner
Copy link
Member

@janowagner janowagner commented Apr 29, 2019

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.

@janowagner janowagner requested a review from a team April 29, 2019 09:32
@janowagner janowagner added the work in progress This pull request should not be merged yet, more commits are expected label Apr 29, 2019
@bjoernricks
Copy link
Contributor

Can we start adding tests with this PR? We really should only add changes with tests from now on.

timopollmeier
timopollmeier previously approved these changes May 14, 2019
Copy link
Member

@timopollmeier timopollmeier left a 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.

base/nvti.c Outdated Show resolved Hide resolved
base/nvti.h Outdated Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
base/nvti.c Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
base/nvti.c Outdated Show resolved Hide resolved
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.
base/nvti.c Outdated Show resolved Hide resolved
@mattmundell mattmundell merged commit 9cce503 into greenbone:master May 16, 2019
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.

4 participants