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

Kubevuln support for VEX document creation #179

Merged
merged 5 commits into from
Oct 24, 2023

Conversation

slashben
Copy link
Contributor

@slashben slashben commented Oct 18, 2023

PR Type:

Enhancement


PR Description:

This PR introduces the creation and update of VEX (Vulnerability EXchange) documents in Kubevuln. VEX is a document format that enables the sharing of vulnerability information about software vulnerabilities between different tools and systems. The implementation includes methods to create a new VEX document if it does not exist or update an existing one. The changes also include unit tests to verify the functionality.


PR Main Files Walkthrough:

files:

repositories/apiserver.go: Added methods to create and update VEX documents. These methods include logic to calculate a canonical hash for the VEX document, sort VEX statements, and create strings from vulnerabilities and components. Also, added the 'StoreVEX' method to the 'APIServerStore' struct.
repositories/apiserver_test.go: Added unit tests for the 'StoreVEX' method to verify the creation and update of VEX documents.
core/ports/repositories.go: Added the 'StoreVEX' method to the 'CVERepository' interface.
core/services/scan.go: Integrated the 'StoreVEX' method into the 'ScanCVE' method to store VEX documents during the scanning process.
repositories/broken.go: Implemented the 'StoreVEX' method in the 'BrokenStore' struct, which returns an expected error.
repositories/memory.go: Implemented the 'StoreVEX' method in the 'MemoryStore' struct, which currently does nothing and returns nil.
go.mod: Added the 'github.com/openvex/go-vex' package as a dependency for handling VEX documents.


User Description:

Overview

Implementation of VEX document creation and update flows in Kubevuln as per #155

Additional Information

VEX is a document format which enables the sharing of vulnerability information about software vulnerabilities in between different tools and systems.

Kubescape scans images for vulnerabilities and uses workload behavior information to determine whether these vulnerabilities are reachable for the attack (relevancy).

This enables this implementation to produce VEX document which describes which of the vulnerabilities need to be dealt with.

How to Test

Added unit tests, otherwise VEX documents are produced together with filtered vulnerability reports.

Related issues/PRs:

Checklist before requesting a review

put an [x] in the box to get it checked

  • My code follows the style guidelines of this project
  • I have commented on my code, particularly in hard-to-understand areas
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • New and existing unit tests pass locally with my changes

Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
@codiumai-pr-agent-free
Copy link

PR Analysis

  • 🎯 Main theme: Implementation of VEX document creation and update flows in Kubevuln
  • 📝 PR summary: This PR introduces the creation and update of VEX (Vulnerability EXchange) documents in Kubevuln. VEX is a document format that enables the sharing of vulnerability information about software vulnerabilities between different tools and systems. The implementation includes methods to create a new VEX document if it does not exist or update an existing one. The changes also include unit tests to verify the functionality.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: Yes
  • ⏱️ Estimated effort to review [1-5]: 4, because the PR introduces a significant amount of new functionality, including the creation and updating of VEX documents, the calculation of a canonical hash for the VEX document, and the sorting of VEX statements. It also includes changes to multiple files and requires a good understanding of the VEX document format and the existing Kubevuln codebase.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR introduces a significant amount of new functionality and appears to be well-structured and organized. It would be beneficial to ensure that all new methods and functions are covered by unit tests. Additionally, it would be helpful to include more detailed comments in the code to explain the purpose and functionality of each method, particularly for complex operations such as the calculation of the canonical hash for the VEX document.

  • 🤖 Code feedback:

    • relevant file: repositories/apiserver.go
      suggestion: Consider breaking down the StoreVEX method into smaller, more manageable methods. This method is currently quite long and performs a number of different tasks, which could make it difficult to understand and maintain. [important]
      relevant line: func (a *APIServerStore) StoreVEX(ctx context.Context, cve domain.CVEManifest, cvep domain.CVEManifest, withRelevancy bool) error {

    • relevant file: repositories/apiserver.go
      suggestion: Consider handling the error returned by calculateVexCanonicalHash in a more user-friendly way. Currently, if an error occurs, the method simply returns the error. It might be more helpful to log an error message that provides more context about what went wrong. [medium]
      relevant line: calculatedId, err := calculateVexCanonicalHash(vexDoc)

    • relevant file: repositories/apiserver.go
      suggestion: Consider adding error handling for the time.Parse function. Currently, if an error occurs when parsing the time, it is ignored. It would be better to handle this error and provide feedback to the user. [medium]
      relevant line: ts, _ := time.Parse(time.RFC3339, s.Timestamp)

    • relevant file: repositories/apiserver_test.go
      suggestion: Consider adding more assertions in your tests to verify the correctness of the VEX document creation and update functionality. For example, you could check that the VEX document contains the expected number of statements, that the statements are correctly sorted, and that the canonical hash is correctly calculated. [medium]
      relevant line: func TestAPIServerStore_storeVEX(t *testing.T) {

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@matthyx
Copy link
Contributor

matthyx commented Oct 18, 2023

At first glance, it looks OK, maybe I would add more unittest coverage for helper functions like calculateVexCanonicalHash.
We will probably need some system-tests coverage too.
cc @dwertent

Signed-off-by: Ben <ben@armosec.io>
Copy link

@dwertent dwertent left a comment

Choose a reason for hiding this comment

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

It looks like the VEX is generated by default. We should add to the configuration file the "vex capability" so it will not always be generated.

repositories/memory.go Outdated Show resolved Hide resolved
repositories/apiserver.go Show resolved Hide resolved
repositories/apiserver.go Outdated Show resolved Hide resolved
Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
@slashben
Copy link
Contributor Author

It looks like the VEX is generated by default. We should add to the configuration file the "vex capability" so it will not always be generated.

Made this optional in commit 351ebe8

@dwertent dwertent added the release Create release label Oct 24, 2023
@dwertent dwertent merged commit a24f1eb into kubescape:main Oct 24, 2023
2 checks passed
dwertent pushed a commit that referenced this pull request Dec 31, 2023
* Implementing VEX creation and update logic

Signed-off-by: Ben <ben@armosec.io>

* Connecting VEX creation to "Scanning for CVEs flow"

Signed-off-by: Ben <ben@armosec.io>

* Integration with Grype working

Signed-off-by: Ben <ben@armosec.io>

* Made the VEX generation an optional feature

Signed-off-by: Ben <ben@armosec.io>

* Removing hardcoded index

Signed-off-by: Ben <ben@armosec.io>

---------

Signed-off-by: Ben <ben@armosec.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request release Create release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants