-
Notifications
You must be signed in to change notification settings - Fork 48
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
base: main
Are you sure you want to change the base?
Update vexVersion.md #649
Conversation
'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.
I noticed the CI is failing - I think this is due to some references to the old name |
Hi Gary,
Would you like me to withdraw this change?
We can track this change as part
of the broader scope of VEX related observations
and incorporate it if needed at that time.
Thanks,
Venkat.
…On Mon, Feb 26, 2024 at 12:33 AM goneall ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub
<#649 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFJ5PIOTUITULXDEWELFDR3YVODGRAVCNFSM6AAAAABDRE5LRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRTGAZTANJWGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Let's just switch it to "Draft" mode. We can switch it back after the VEX discussions. |
@VenkatTechnologist - is this PR still valid based on your analysis? |
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.
'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. |
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.
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.
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.
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
@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 If everyone is agreeable, I can open a PR with the change proposed above unless @VenkatTechnologist wants to do it. |
@rnjudge sounds good to me 👍 I can also fix, no problem. But I thought Venkat can update this PR and we're done :) |
@puerco, which SPDX field is used for doc version?
…On Tue, Apr 9, 2024 at 1:38 AM Puerco ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In model/Security/Properties/vexVersion.md
<#649 (comment)>:
>
## Summary
-Specifies the version of the VEX document.
+Specifies the minimum requirements of data elements version that the VEX document adheres to.
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."
—
Reply to this email directly, view it on GitHub
<#649 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFJ5PILZOY2QTZNQI5ME4MLY4L2LBAVCNFSM6AAAAABDRE5LRGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSOBXGMYTKOBUGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
If vexVersion is meant to be the statement version, recommend it to be
clearly named as vexStatementVersion so that there is no confusion.
…On Tue, Apr 9, 2024 at 3:12 AM Rose Judge ***@***.***> wrote:
@VenkatTechnologist <https://github.com/VenkatTechnologist> @jeff-schutt
<https://github.com/jeff-schutt> @puerco <https://github.com/puerco> What
I'm gleaning from reading through the comments on this PR and #648
<#648> is that we should *not*
merge this PR as is and, instead, 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
<#648>.
If everyone is agreeable, I can open a PR with the change proposed above
unless @VenkatTechnologist <https://github.com/VenkatTechnologist> wants
to do it.
—
Reply to this email directly, view it on GitHub
<#649 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFJ5PIIGG7UQUSRSLKQNWXDY4MFL5AVCNFSM6AAAAABDRE5LRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBTGY4DSNBZHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
That's the catch with SPDX docs: They are supposed to be immutable, so there are no versions.
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 |
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."*
Then how do we implement doc versions of VEX implementation? Each VEX
document can be updated
(and its version incremented) when there is an update to any of the
statements within. How would this be
implemented in SPDX?
***@***.***: "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."*
As I mentioned above, a VEX statement does not stand on its own. It is
embedded in a VEX document
because of the way the VEX structure is designed. Bypassing this structure
does not seem to be a
good idea in terms of consumption perspective. Consumers will be clearly
confused.
To me, there needs be a doc level above the vex assessment relationship.
If what I said makes sense, we can look at this change in SPDX 3.1 and
leave things as is for SPDX 3.0.
This will also enable us to collect feedback from consumers on the VEX
implementation in SPDX 3.0.
(I personally feel there will be a LOT of explanation to do.)
…On Tue, Apr 9, 2024 at 8:51 AM Puerco ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub
<#649 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFJ5PILM4LLXZPRFL2NPNODY4NNENAVCNFSM6AAAAABDRE5LRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBUGA4DGMBRHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@VenkatTechnologist @puerco can you both plan to join the security call this Wed to discuss and come to a conclusion on this? |
I am in the midst of a travel. My views are already recorded here.
My recommendation is to defer this to 3.1 and look at it in detail when we
have time.
Thanks.
…On Tue, 9 Apr, 2024, 9:23 pm Rose Judge, ***@***.***> wrote:
@VenkatTechnologist <https://github.com/VenkatTechnologist> @puerco
<https://github.com/puerco> can you both plan to join the security call
this Wed to discuss and come to a conclusion on this?
—
Reply to this email directly, view it on GitHub
<#649 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFJ5PIJLRBOO72YWCTR6FGLY4QFI7AVCNFSM6AAAAABDRE5LRGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBVGUZTQOJQGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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. |
This PR fixes two small Security profile issues that were discovered on the April 10th call. See spdx#649 Signed-off-by: Rose Judge <[email protected]>
This PR fixes two small Security profile issues that were discovered on the April 10th call. See #649 Signed-off-by: Rose Judge <[email protected]>
'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.