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

Update vexVersion.md #649

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

VenkatTechnologist
Copy link
Contributor

'vexVersion' meant here stands for minimum requirements for data elements version released by CISA. Hence it is suggested that the field be updated to 'vexMinRequirementsVersion'. The other elements in the field like description and metadata are also being updated.

'vexVersion' meant here stands for minimum requirements for data elements version released by CISA. Hence it is suggested that the field be updated to 'vexMinRequirementsVersion'. The other elements in the field like description and metadata are also being updated.
@goneall
Copy link
Member

goneall commented Feb 25, 2024

I noticed the CI is failing - I think this is due to some references to the old name vexVersion which have not been updated to the proposed new name vexMinRequirementsVersion.

@VenkatTechnologist
Copy link
Contributor Author

VenkatTechnologist commented Feb 26, 2024 via email

@goneall
Copy link
Member

goneall commented Feb 26, 2024

Would you like me to withdraw this change?

Let's just switch it to "Draft" mode. We can switch it back after the VEX discussions.

@VenkatTechnologist VenkatTechnologist marked this pull request as draft February 27, 2024 05:08
@goneall
Copy link
Member

goneall commented Apr 3, 2024

@VenkatTechnologist - is this PR still valid based on your analysis?

@kestewart kestewart modified the milestones: 3.0-rc3, 3.0 Apr 7, 2024
Copy link
Collaborator

@jeff-schutt jeff-schutt left a comment

Choose a reason for hiding this comment

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

'vexVersion' meant here stands for minimum requirements for data elements version released by CISA.

From what I recall this was not the intended use of this field.

This PR appears to change the definition from supporting one of the data fields that is required 2.2.2 Document version [doc_version] to support one that is optional 2.0 VEX data elements, both described here. Note the differences between use of MUST and SHOULD.


## Summary

Specifies the version of the VEX document.
Specifies the minimum requirements of data elements version that the VEX document adheres to.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is not correct, but note that vexVersion defines the VEX statement version (not the document). It is used in the vex relationships. This is the version detailed in 2.4.2 of the vex Minimum Requirements linked by Jeff above. We named it vexVersion (as opposed to just version) because there was a name clash.

Essentially, the Minimum Requirements doc states the fields all vex implementations need to define. It is a spec of specs but the source of truth is the SPDX spec itself, not the CISA document. In other words, this is a field defined by SPDX not by the Minimum Reqs.

Copy link
Collaborator

@puerco puerco Apr 8, 2024

Choose a reason for hiding this comment

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

To clarify, we were relying on another field to serve as the document version and the summary of vexVersion should read "Specifies the version of a VEX statement."

Update: I just remembered that someone pointed out that SPDX documents were read-only, in other words they cannot be updated, so there is no way to have versions of documents

@rnjudge
Copy link
Collaborator

rnjudge commented Apr 8, 2024

@VenkatTechnologist @jeff-schutt @puerco What I'm gleaning from reading through the comments on this PR and #648 is that we should not merge this PR as is and, instead, clarify/change the Summary definition of vexVersion in model/Security/Properties/vexVersion.md to be: "Specifies the version of a VEX statement." This would mean also closing #648.

If everyone is agreeable, I can open a PR with the change proposed above unless @VenkatTechnologist wants to do it.

@puerco
Copy link
Collaborator

puerco commented Apr 9, 2024

@rnjudge sounds good to me 👍

I can also fix, no problem. But I thought Venkat can update this PR and we're done :)

@VenkatTechnologist
Copy link
Contributor Author

VenkatTechnologist commented Apr 9, 2024 via email

@VenkatTechnologist
Copy link
Contributor Author

VenkatTechnologist commented Apr 9, 2024 via email

@puerco
Copy link
Collaborator

puerco commented Apr 9, 2024

which SPDX field is used for doc version?

That's the catch with SPDX docs: They are supposed to be immutable, so there are no versions.

recommend it to be clearly named as vexStatementVersion

So, we don't have a vex statement in SPDX. A VEX statement is assembled by a triad of (at least):

a software package + a vex assessment relationship + a vulnerability

The vexVersion field marks is a property of the assessment. This means that naming it vexStatementVersion is not correct because there is no vex statement element. It would have been named VexVulnAssessmentRelationship.version but there was a naming collision and we had to look for an alternative.

@VenkatTechnologist
Copy link
Contributor Author

VenkatTechnologist commented Apr 9, 2024 via email

@rnjudge
Copy link
Collaborator

rnjudge commented Apr 9, 2024

@VenkatTechnologist @puerco can you both plan to join the security call this Wed to discuss and come to a conclusion on this?

@VenkatTechnologist
Copy link
Contributor Author

VenkatTechnologist commented Apr 9, 2024 via email

@rnjudge
Copy link
Collaborator

rnjudge commented Apr 10, 2024

Decision in the April 10th security call to defer this to 3.1 per Venkat's comment. I will open a PR to update the Summary definition of vexVersion in model/Security/Properties/vexVersion.md to be: "Specifies the version of a VEX statement." per puerco's recommendation and to clear up confusion in the short term.

@rnjudge rnjudge modified the milestones: 3.0, 3.1 Apr 10, 2024
rnjudge added a commit to rnjudge/spdx-3-model that referenced this pull request Apr 10, 2024
This PR fixes two small Security profile issues that were discovered on
the April 10th call.

See spdx#649

Signed-off-by: Rose Judge <rose.judge@broadcom.com>
kestewart pushed a commit that referenced this pull request Apr 11, 2024
This PR fixes two small Security profile issues that were discovered on
the April 10th call.

See #649

Signed-off-by: Rose Judge <rose.judge@broadcom.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants