-
Notifications
You must be signed in to change notification settings - Fork 140
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
Port 2.x Annexes to 3.0 spec #904
Conversation
351b457
to
fa417f8
Compare
@puerco do you want to take a look at the security annex? You can view the markdown file easier here: https://github.com/rnjudge/spdx-spec/blob/3.0-annex/docs/annexes/including-security-information-in-SPDX.md. There are some TODOs noted in the file where I have questions. |
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.
Couple of suggestions for consideration on the identifier mappings
| Component Name | [Software/Classes/Package.name](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/Package/) inherited from [Core/Classes/Element.name](https://spdx.github.io/spdx-spec/v3.0/model/Core/Properties/name/) | | ||
| Version String | [Software/Classes/Package.packageVersion](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/Package/) | | ||
| Component Hash | [Core/Classes/Element.verifiedUsing](https://spdx.github.io/spdx-spec/v3.0/model/Core/Classes/Element/) | | ||
| Unique Identifier | [Software/Classes/SoftwareArtifact.contentIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/SoftwareArtifact/) for SPDX Software Artifacts <br>or [Core/Classes/ExternalIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Core/Classes/ExternalIdentifier/) for resources outside the scope of SPDX-3.0 content </br> | |
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.
| Unique Identifier | [Software/Classes/SoftwareArtifact.contentIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/SoftwareArtifact/) for SPDX Software Artifacts <br>or [Core/Classes/ExternalIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Core/Classes/ExternalIdentifier/) for resources outside the scope of SPDX-3.0 content </br> | | |
| Unique Identifier | [Software/Classes/SoftwareArtifact.contentIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/SoftwareArtifact/) for SPDX Software Artifacts <br>or [Core/Classes/Element.externalIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Core/Classes/Element/) for resources outside the scope of SPDX-3.0 content </br> | |
Suggest we refer to the property instead of the class
| Component Name | [Software/Classes/Package.name](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/Package/) inherited from [Core/Classes/Element.name](https://spdx.github.io/spdx-spec/v3.0/model/Core/Properties/name/) | | ||
| Version String | [Software/Classes/Package.packageVersion](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/Package/) | | ||
| Component Hash | [Core/Classes/Element.verifiedUsing](https://spdx.github.io/spdx-spec/v3.0/model/Core/Classes/Element/) | | ||
| Unique Identifier | [Software/Classes/SoftwareArtifact.contentIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Software/Classes/SoftwareArtifact/) for SPDX Software Artifacts <br>or [Core/Classes/ExternalIdentifier](https://spdx.github.io/spdx-spec/v3.0/model/Core/Classes/ExternalIdentifier/) for resources outside the scope of SPDX-3.0 content </br> | |
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.
There are several member of our community who would like PackageURL's to be included as a potential identifier.
There is some debate as to whether a PackageURL satisfies the uniqueness requirements of an identifier.
One compromise approach would be to add a line something like -
"or Software/Package.packageUrl if the packageUrl is considered to be unique"
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.
OK, here is a first pass with some comments.
Since folks searching for security data are likely to land on this document, I think we should add a super, super simple vex example and then link it to a larger VEX HOWTO somewhere in the docs.
WDYT @rnjudge?
``` | ||
|
||
|
||
## G.1.3 Linking to a CycloneDX |
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.
## G.1.3 Linking to a CycloneDX | |
## G.1.3 Linking to CycloneDX Security Data |
* cpe22 | ||
* cpe23 | ||
* cve | ||
* cwe |
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.
CWE is not an external identifier. It is an ExtRef, an external reference to an entry in a catalog
* cwe |
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.
Good catch, I think I forgot to update when you made this suggestion on my other PR.
|
||
This section provides usage scenarios of how to leverage the Security External References and External Identifiers specified above to refer to external security information. Examples of how to use each category can be found in the [Security/Classes](https://github.com/spdx/spdx-3-model/tree/main/model/Security/Classes) pages. Multiple instances and types of external security information may be included within a SPDX document. | ||
|
||
## G.1.1 Linking to an advisory |
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.
Let's keep the title case from the first titles
## G.1.1 Linking to an advisory | |
## G.1.1 Linking to an Advisory |
}, | ||
{ | ||
"type": "ExternalIdentifier", | ||
"externalIdentifierType": "securityOther", |
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.
I know that this is trying to show examples but this references a very well known advisory source
"externalIdentifierType": "securityOther", | |
"externalIdentifierType": "securityAdvisory", |
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.
I took this from the vulnerability class example -- good catch. I will update the Vulnerability example as well.
## G.1.5 Linking to an OmniBOR (formerly known as GitBOM) | ||
|
||
To reference an [OmniBOR](https://omnibor.io/) (Universal Bill Of Receipts) formatted security information applicable to a package you must first create a Package Element. Then use an External Identifier to link to the OmniBOR document. | ||
**TODO: Is this gitoid correct? do we need a different example here?** |
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.
Gitoids identify the package blob, they are an identifier not a reference to something else.
Regarding the TODO, yes it is correct. The string in the example is the IANA URL scheme for gitoids.
## G.1.5 Linking to an OmniBOR (formerly known as GitBOM) | |
To reference an [OmniBOR](https://omnibor.io/) (Universal Bill Of Receipts) formatted security information applicable to a package you must first create a Package Element. Then use an External Identifier to link to the OmniBOR document. | |
**TODO: Is this gitoid correct? do we need a different example here?** | |
## G.1.5 Identifying a Package with an OmniBOR Gitoid | |
To identify a Package with an [OmniBOR](https://omnibor.io/) (Universal Bill Of Receipts, formerly known as GitBOM) gitoid, use an External Identifier to add gitoid to the package. |
} | ||
``` | ||
|
||
## G.1.6 Linking to a vulnerability disclosure document |
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.
What is the difference between this and an advisory? (Especially since the links point to advisory data)
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.
Just one minor change requested
9e14c41
to
fee79a2
Compare
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.
Changes look good to me.
We may want to be more explicit about the releationships to be specified for dependencies and root, but I don't consider it blocker at this point.
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.
LGTM
This commit adds two annexes to the 3.0 spec: 1) How to include security information using SPDX-3.0 2) How to comply with various requirements (i.e. NTIA minimums) using SPDX-3.0 Signed-off-by: Rose Judge <[email protected]>
Signed-off-by: Karsten Klein <[email protected]>
Signed-off-by: Karsten Klein <[email protected]>
This commit adds two annexes to the 3.0 spec: 1) How to include security information using SPDX-3.0 2) How to comply with various requirements (i.e. NTIA minimums) using SPDX-3.0 Signed-off-by: Rose Judge <[email protected]> Signed-off-by: Karsten Klein <[email protected]>
Sounds good! I can get started on a VEX example annex to add to 3.0.1. We do have one VEX example in this annex under the CSAF section - linking to a generic VEX would be the same. |
This commit adds two annexes to the 3.0 spec:
SPDX-3.0
NOTE: This PR is still a work in progress. It is being opened so others can comment - changes are expected. Please do not merge yet. I will remove this note when the PR is ready to review for merge.