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.

Sign up for GitHub

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

Jump to bottom

Add support for ingesting VEX documents in the CSAF format #729

Merged
merged 2 commits into from
Jul 4, 2023
Merged

Add support for ingesting VEX documents in the CSAF format #729

merged 2 commits into from
Jul 4, 2023

Conversation

lulf
Copy link
Contributor

@lulf lulf commented Apr 17, 2023 •
edited
Loading

Marking this as draft to check if I'm on the right path and waiting for a pr to go-vex to be reviewed and merged.

This adds a new parser for CSAF documents that and supporting ingestion of Vex documents. In addition it injects CertifyVulnIngest for known_affected or under_investigation statements as discussed in chat.

I'll fix/add a unit test for the parser as well.

@google-cla
Copy link

google-cla bot commented Apr 17, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

pxp928
pxp928 reviewed Apr 17, 2023
View reviewed changes
Copy link
Collaborator

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

@lulf Thanks for the PR! Yes this looks good so far. Just a couple of questions

pkg/assembler/assembler.go Outdated Show resolved Hide resolved
pkg/ingestor/parser/csaf/parser_csaf.go Show resolved Hide resolved
@lumjjb
Copy link
Contributor

lumjjb commented Apr 17, 2023

Hey @lulf ! Thanks for opening this PR! in the meantime, if you need help with resolving the DCO or CLA, let us know!

@lulf lulf mentioned this pull request May 4, 2023
Merged
@lulf
Copy link
Contributor Author

lulf commented May 4, 2023

Update: I'm still working on this, and it's almost done. I'm just awaiting a PR to be merged in go-vex and a release of that module. I have added some tests here in the PR as well and will rebase it soon.

@lulf lulf marked this pull request as ready for review May 22, 2023 09:20
@lulf lulf requested a review from mihaimaruseac as a code owner May 22, 2023 09:20
@lulf
Copy link
Contributor Author

lulf commented May 22, 2023

Sorry for the delay, but the go-vex has been merged now, so this is ready for review.

@pxp928
Copy link
Collaborator

pxp928 commented May 24, 2023

Thanks @lulf for the update! We will review this soon!

@lulf
Copy link
Contributor Author

lulf commented Jun 5, 2023

Ping :)

pxp928
pxp928 reviewed Jun 5, 2023
View reviewed changes
Copy link
Collaborator

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! Added some comments

pkg/assembler/assembler.go Outdated Show resolved Hide resolved
pkg/assembler/clients/helpers/assembler.go Outdated Show resolved Hide resolved
pkg/assembler/clients/helpers/assembler.go Outdated Show resolved Hide resolved
pkg/ingestor/parser/csaf/parser_csaf.go Show resolved Hide resolved
pxp928
pxp928 reviewed Jun 8, 2023
View reviewed changes
pkg/assembler/assembler.go Outdated Show resolved Hide resolved
pkg/assembler/clients/helpers/assembler.go Outdated Show resolved Hide resolved
pkg/ingestor/parser/csaf/parser_csaf.go Show resolved Hide resolved
@pxp928 pxp928 added the needs-review Needs writer LGTM label Jun 12, 2023
lumjjb
lumjjb approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

Awesome @lulf !! Thanks for the great PR! Looks good to me, i've added some comments but i think most of it can be part of iterative process in future PRs as we encounter docs with different data. Cheers!

go.mod Show resolved Hide resolved
pkg/assembler/assembler.go Outdated Show resolved Hide resolved
pkg/handler/processor/csaf/csaf.go Outdated Show resolved Hide resolved
pkg/ingestor/parser/csaf/parser_csaf.go Show resolved Hide resolved
pkg/ingestor/parser/csaf/parser_csaf.go Show resolved Hide resolved
pkg/ingestor/parser/csaf/parser_csaf.go Show resolved Hide resolved
@dejanb dejanb force-pushed the vex branch 2 times, most recently from 1ca6e37 to a03fb1a Compare July 3, 2023 15:54
@dejanb
Copy link
Contributor

dejanb commented Jul 3, 2023

@lumjjb @pxp928 I rebased the PR and it seems a lot comments are already addressed by #942. Can you give it another look and let me know if there's anything else we need to do before merging? And let's open issues for the future enhancements.

pxp928
pxp928 approved these changes Jul 3, 2023
View reviewed changes
Copy link
Collaborator

@pxp928 pxp928 left a comment

Choose a reason for hiding this comment

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

LGTM. We need to address if artifacts can be used by VEX (can open an issue for this). Also need to address the noVuln case.

This was referenced Jul 4, 2023
Closed
Open
@dejanb
Copy link
Contributor

dejanb commented Jul 4, 2023

Great. #1015 and #1016 has beed added.

@pxp928
Copy link
Collaborator

pxp928 commented Jul 4, 2023

@lulf a rebase will auto merge this PR

lulf and others added 2 commits July 4, 2023 16:52
@lulf
feat: add support for ingesting VEX documents in CSAF format
152b8bc 
Signed-off-by: Ulf Lilleengen <ulf.lilleengen@gmail.com>
@dejanb @lulf
fix: typo
e897825 
Signed-off-by: Dejan Bosanac <dbosanac@redhat.com>
@kodiakhq kodiakhq bot merged commit 290c1c3 into guacsec:main Jul 4, 2023
8 checks passed
rmetzman pushed a commit to rmetzman/guac that referenced this pull request Jul 5, 2023
@lulf @dejanb
Add support for ingesting VEX documents in the CSAF format (guacsec#729)
b825cf2 
* feat: add support for ingesting VEX documents in CSAF format

Signed-off-by: Ulf Lilleengen <ulf.lilleengen@gmail.com>

* fix: typo

Signed-off-by: Dejan Bosanac <dbosanac@redhat.com>

---------

Signed-off-by: Ulf Lilleengen <ulf.lilleengen@gmail.com>
Signed-off-by: Dejan Bosanac <dbosanac@redhat.com>
Co-authored-by: Dejan Bosanac <dbosanac@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dejanb dejanb dejanb left review comments

@pxp928 pxp928 pxp928 approved these changes

@lumjjb lumjjb lumjjb approved these changes

@mihaimaruseac mihaimaruseac Awaiting requested review from mihaimaruseac mihaimaruseac is a code owner

Assignees
No one assigned
Labels
needs-review Needs writer LGTM size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lulf @lumjjb @pxp928 @dejanb