Repository navigation
docs(1.7): clarify that an omitted nistQuantumSecurityLevel is not 0 - #1177
Lennonhaha wants to merge 3 commits into
Conversation
The 1.7 description only stated "A value of 0 indicates that none of the categories are met." It said nothing about the omitted case, which leaves two readings open: that omission means 0, or that omission means "not evaluated". Both readings are wrong. 0 is an assertion that none of the categories are met; an omitted property makes no category assertion at all, and says nothing about whether an assessment was performed. Encoding "unknown" as 0 turns a non-claim into a claim. Description-only change, applied to the three 1.7 artifacts so they stay in sync: bom-1.7.schema.json (description), bom-1.7.xsd (documentation) and bom-1.7.proto (comment). No structure, type, bound or required list is touched. Verified: the edited schema still compiles under ajv; valid-bom-1.7.json still validates, and two invalid 1.7 fixtures are still rejected, so the check is not vacuous; the XSD still parses as XML. No BOM, no CRLF, no U+FFFD introduced. 1.6.2 keeps its current wording here: it is released and superseded by 1.7, and changing a released artifact is a separate decision. 2.0-dev needs the same wording once this one is settled. Signed-off-by: 刘天赫 <research@fibemate.net>
Mehrn0ush
left a comment
Mehrn0ush
left a comment
There was a problem hiding this comment.
Thanks @Lennonhaha for preparing this clarification. The JSON and XML changes reflect the distinction discussed in #1175: omission makes no category assertion and establishes no assessment status, while an explicit 0 means none of the referenced categories are met. The diff leaves validation constraints unchanged.
Please address the Protobuf wording noted inline so that its accessor default is not confused with an asserted category. The three formats should express the same CycloneDX semantics, with encoding-specific guidance where necessary.
This PR targets master and updates only the 1.7 artifacts. Please link a follow-up for the equivalent clarification in 2.0-dev, or record the agreed plan in #1175, so the 2.0 work remains explicitly tracked.
Maintainers can confirm the appropriate release handling and sequencing.
| // The classical security level that a cryptographic algorithm provides (in bits). | ||
| optional int32 classicalSecurityLevel = 10; | ||
| // The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). A value of 0 indicates that none of the categories are met. | ||
| // The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). This property is optional and has no default: omitting it is not an implicit 0, and does not establish whether an assessment was performed. A value of 0 indicates that none of the categories are met. Consumers should not interpret an omitted value as 0, nor treat it as evidence that the asset is quantum-safe. |
There was a problem hiding this comment.
“has no default” is misleading for this proto3 optional int32. An unset field’s numeric accessor returns 0, while explicit field presence distinguishes absence from a supplied zero. The intended CycloneDX semantics are correct; please describe that distinction here rather than copying the JSON Schema explanation unchanged. See Protobuf field-presence semantics (https://protobuf.dev/programming-guides/field_presence/#semantic-differences).
Suggested replacement:
| // The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). This property is optional and has no default: omitting it is not an implicit 0, and does not establish whether an assessment was performed. A value of 0 indicates that none of the categories are met. Consumers should not interpret an omitted value as 0, nor treat it as evidence that the asset is quantum-safe. | |
| // The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). This field is optional. Absence makes no NIST-category assertion and does not establish whether an assessment was performed. Consumers should use field presence to distinguish absence from an explicitly supplied zero; the numeric accessor value alone is insufficient. A value of 0 indicates that none of the categories are met. Consumers should not treat absence as evidence that the asset is quantum-safe. | |
|
Verified locally on f84e0f7 with protoc: bom-1.7.proto compiles. |
|
Thanks for running Two things your result settles:
One question before this goes further: would it help to add a sentence telling consumers to test presence rather than compare to |
|
Thanks—yes, please add the presence-check guidance and replace “has no default” in the Protobuf comment. An unset field’s numeric accessor returns 0, so consumers must check field presence to distinguish absence from an explicitly supplied zero. Please keep this language-neutral; examples from individual bindings aren’t necessary. That would address my remaining concern. |
80f0713 to
40d5c0e
Compare
|
Presence wording added, and "has no default" is gone from all three 1.7 artifacts. Kept language-neutral - no per-binding accessor names. Scope, verified rather than asserted: description and documentation text only. The JSON structure is unchanged apart from description strings (compared with description keys stripped), and the XSD still parses as XML. No field number, type, optionality, bound or required list is touched. One thing I did not do: I have no |
|
Thanks, the updated presence wording addresses my concern. I reran protoc: both versions compile and produce byte-identical descriptor sets. LGTM! |
|
Thanks for the thorough review and for re-running protoc locally - good to have the descriptor sets confirmed byte-identical. One detail: the review is still recorded as "changes requested", which keeps the PR blocked. If the current wording works for you, flipping that state would let a maintainer merge. I do not have write access to this repository, so the merge itself is not mine to make. |
|
Hi @CycloneDX/core-team, just a polite ping. @Mehrn0ush has reviewed and approved this clarification, and all CI checks are green. It looks like it's waiting on a code-owner review to be merged. Thanks! |
| "type": "integer", | ||
| "title": "NIST security strength category", | ||
| "description": "The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). A value of 0 indicates that none of the categories are met.", | ||
| "description": "The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). This property is optional. Omitting it makes no NIST security category assertion and does not establish whether an assessment was performed; a value of 0 states that none of the categories are met. An unset integer field may read back as 0, so the raw value cannot distinguish absence from an explicit zero: consumers must check whether the field is present rather than comparing the value to 0. An omitted value is not 0, and is not evidence that the asset is quantum-safe.", |
There was a problem hiding this comment.
This property is optional.
This adds no value. it is already clear from the schema, that it is optional.
Furtheremore, we drive the spec to not include the word optioanl/required - since the tool that generates the standard text already gathers this information from the schemas.
Please remove it.
- "description": "The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). A value of 0 indicates that none of the categories are met.",
+ "description": "The NIST security strength category as defined in https://csrc.nist.gov/projects/post-quantum-cryptography/post-quantum-cryptography-standardization/evaluation-criteria/security-(evaluation-criteria). This property is optional. Omitting it makes no NIST security category assertion and does not establish whether an assessment was performed; a value of 0 states that none of the categories are met. An unset integer field may read back as 0, so the raw value cannot distinguish absence from an explicit zero: consumers must check whether the field is present rather than comparing the value to 0. An omitted value is not 0, and is not evidence that the asset is quantum-safe.",
To be honest, the posed changes look like they belong in the respective CycloneDX authoritative guide, not in the spec. They describe how a value can be interpreted, and how to use it. |
|
Thanks, @jkowalleck. That distinction makes sense. My review focused on preserving the semantic difference between omission and an explicit zero, and correcting the Protobuf default-value wording. The accessor behavior and presence-check instructions fit better in the guide. I’m comfortable with narrowing the schema wording or moving the clarification to the guide, as maintainers prefer. |
|
@stevespringett , what do you think, should we have this topic detailed in the guide? |
|
Agreed — the minimal semantic belongs in the spec (the field is optional; omission means neither “not assessed” nor “quantum-safe”), and the accessor/presence-check details fit better in the guides. Happy to narrow the schema wording here and open a separate PR to CycloneDX/guides once @stevespringett weighs in. |
|
I agree that the '0' case should be documented in the description since it's not technically in the NIST categories. Once sentence might be sufficient to describe that. |
…stQuantumSecurityLevel The Protobuf comment said the property "has no default", which is the wrong model for `optional int32`: an unset field's numeric accessor returns 0, so the value alone cannot be told apart from an explicitly supplied zero. That is exactly the confusion this clarification exists to prevent. Replace the phrase in all three 1.7 artifacts and state the consequence instead: an omitted value makes no category assertion and does not establish whether an assessment was performed; an explicit value of 0 indicates that none of the NIST-defined quantum security categories apply; consumers must check whether the field is present rather than comparing the value to 0. The zero case is now stated in its own sentence. It is worth saying plainly because 0 is not one of the NIST categories -- it is a value in the permitted range that asserts the absence of all of them -- and a reader who sees only "0..6" should not have to infer that. Language-neutral on purpose. No per-binding accessor names, so the wording does not need revising when bindings change. Description and documentation text only. No field number, type, optionality, bound or required list is touched: the JSON structure is unchanged apart from description strings, and the XSD still parses as XML. Signed-off-by: 刘天赫 <research@fibemate.net>
40d5c0e to
356280a
Compare
|
Pushed
It now appears verbatim in all three 1.7 artifacts — Why it is worth a sentence rather than staying a clause: Verification, all local:
Two notes on the state of this PR, so nothing here looks greener than it is:
On the scoping question raised in review — the semantic minimum (optionality, what omission means, what @bhess — does the wording above match what you had in mind? |
This sentence makes sense to me, but I don't see it it [356280a]. |
Adds the sentence requested in review, verbatim and as a standalone sentence: An explicit value of 0 indicates that none of the NIST-defined quantum security categories apply. The case is worth its own sentence rather than the previous clause: 0 is not one of the NIST categories, and a reader who only sees the 0..6 range should not have to infer that 0 asserts the absence of all of them. Also drops "This property is optional.", which duplicated what the schema already expresses, and leaves the accessor-level presence check to the guides as raised in the same thread. The omission semantics stay in the description: omitting the field makes no category assertion and is not evidence that the asset is quantum-safe. Applied to all three 1.7 artifacts: schema/bom-1.7.schema.json (description), schema/bom-1.7.xsd (xs:documentation) and schema/bom-1.7.proto (field comment). Description text only: no field number, type, bound, optionality or required list is touched. Verified locally: the JSON parses and the new text is at definitions.cryptoProperties.properties.algorithmProperties.properties .nistQuantumSecurityLevel.description; the XSD parses as well-formed XML. No protoc available here, so no descriptor-set comparison is claimed for this commit. Signed-off-by: 刘天赫 <research@fibemate.net>
|
@bhess thank you for checking — you were right, and the fault was mine. The sentence was not in It is now in
While there I also dropped "This property is optional.", as @jkowalleck asked, and left the accessor-level presence check for the guides per the narrowing discussed above. The omission semantics stay in the description: omitting the field makes no category assertion and is not evidence that the asset is quantum-safe. Verification, read back from |
Follow-up to #1175, as @Mehrn0ush suggested: a description-only clarification for 1.7.
The gap
The 1.7 description currently reads:
It defines
0but says nothing about the omitted case, which leaves two readings open:0, orNeither is right, and the two are different mistakes.
0is a positive assertion that no NIST category is met. An omitted property asserts nothing about categories and additionally says nothing about whether anyone assessed the asset. Consumers that fold absent into0therefore upgrade a non-claim into a claim — the failure mode that started this thread.The change
Description text only, applied to the three 1.7 artifacts so they do not drift apart:
schema/bom-1.7.schema.jsondescriptionschema/bom-1.7.xsdxs:documentationschema/bom-1.7.protoNew wording:
No structure, type, bound or required list is touched.
Verification
valid-bom-1.7.jsonstill validatesinvalid-bomformat,invalid-component-type) are still rejected — included so the "it validates" claim is not vacuousbom-1.7.xsdstill parses as XMLScope notes
notEvaluatedetc.). If an independent, scale-agnostic way to express "assessed / not assessed" is wanted, that is a new field with its own discussion — not something this description change implies.