Skip to content

Conversation

stevespringett
Copy link
Member

@stevespringett stevespringett commented Jul 1, 2025

The cryptography working group has received feedback from real-world usage and have made some minor enhancements to the CBOM specificaiton.

Closes #569


RFC notice sent 2025-07-26
This RFC will be open for 4 weeks. At the end of the RFC period the CycloneDX community will vote, by lazy consensus, to accept or reject the proposal.
RFC period end: 2025-08-23


TODO/DONE

  • add examples for XML
  • add examples for JSON
  • add examples for ProtoBuf
  • implement for XML
  • implement for JSON
  • implement for ProtoBuf

stevespringett and others added 21 commits March 21, 2025 22:07
- Adds a few more algorithm
- Converts urls to standards to doi links, where available.
- Checks if urls work

Signed-off-by: Basil Hess <[email protected]>
- Adds a few more algorithm
- Converts urls to standards to doi links, where available.
- Checks if urls work

----

TODO / progress
- [x] JSON schema
- [ ] XML schema
- [ ] ProtoBugf schema

<!-- 
Thank you for taking the time to develop and contribute a core
enhancement or fix for a defect!

We kindly request that you create pull requests only for things that
have been discussed in a ticket first; exceptions may be made for
spelling or grammar fixes.
Read more about the process here:
https://cyclonedx.org/participate/standardization-process/#working-model

Please have the related ticket/issue ID ready. 
If there is none, feel free to create a new ticket:
https://github.com/CycloneDX/specification/issues/new/choose

-->

<!-- 

Please provide a brief description of what this pull request intends to
do and which ticket it fixes/closes.
Example: 
> As discussed in ticket #485, this PR adds Streebog to the hash
algorithm enum.
>
> fixes #485 

In case this is for a spelling or grammar improvement, please provide a
brief description.
Example:
> Fixe typo: color(AE) -> colour(BE)

-->
Signed-off-by: Basil Hess <[email protected]>
- Changes schma for crypto-defs to allow different variant patterns corresponding to different primitives
- Adds "key-wrap" as a new primitive

Signed-off-by: Basil Hess <[email protected]>
- Extends cryptography-defs.json list with algorithms from PKCS11
- Changes schma for crypto-defs to allow different variant patterns
corresponding to different primitives
- Adds "key-wrap" as a new primitive
{placeholder} -> required parameter with placeholder
(option1|option2) -> required parameter with fixed alternatives
[parameter] -> optional parameter
[-{placeholder}] -> optional paremeter with literal separator

Signed-off-by: Basil Hess <[email protected]>
Signed-off-by: Basil Hess <[email protected]>
This PR will add a python script that can be used to generate an
enum-object for the cyclonedx json schema that reflects algorithm
families defined in `cryptography-defs.json`.
The following rules apply for the patterns:

{placeholder} -> required parameter with placeholder
(option1|option2) -> required parameter with fixed alternatives 
[parameter] -> optional parameter
[-{placeholder}] -> optional parameter with literal separator

<!-- 
Thank you for taking the time to develop and contribute a core
enhancement or fix for a defect!

We kindly request that you create pull requests only for things that
have been discussed in a ticket first; exceptions may be made for
spelling or grammar fixes.
Read more about the process here:
https://cyclonedx.org/participate/standardization-process/#working-model

Please have the related ticket/issue ID ready. 
If there is none, feel free to create a new ticket:
https://github.com/CycloneDX/specification/issues/new/choose

-->

<!-- 

Please provide a brief description of what this pull request intends to
do and which ticket it fixes/closes.
Example: 
> As discussed in ticket #485, this PR adds Streebog to the hash
algorithm enum.
>
> fixes #485 

In case this is for a spelling or grammar improvement, please provide a
brief description.
Example:
> Fixe typo: color(AE) -> colour(BE)

-->
@stevespringett stevespringett added this to the 1.7 milestone Jul 1, 2025
stevespringett and others added 6 commits June 30, 2025 20:46
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
Copy link
Member

jkowalleck commented Aug 11, 2025

so far, I did a formal review to see if all test data was consistent. Applied a lot of fixes in the test data, and fixed some ProtoBuf schema fields that were simply forgotten.

now that the tests are consistent and schema-valid, I looked at the underlying schemas and documentations.
I found that those "some minor enhancements to the CBOM specificaiton." are not that minor - t the ProtoBuf has breaking changes at so many places (see https://github.com/CycloneDX/specification/actions/runs/16884134356/job/47827336005?pr=657#step:3:25)
that I get the impression, that the "minor changes" are actually a whole different protocol.
The same breaking changes should be visible in the XML, too - will take a look..

SO far, i will take a break from the review, and might propose critical changes to make this a non-breaking change.

Until then, I'd veto those changes as they are unexpected breaking changes.

bhess and others added 3 commits August 14, 2025 10:40
This PR further extends the list of algorithms in
cryptography-defs.json.

<!-- 
Thank you for taking the time to develop and contribute a core
enhancement or fix for a defect!

We kindly request that you create pull requests only for things that
have been discussed in a ticket first; exceptions may be made for
spelling or grammar fixes.
Read more about the process here:
https://cyclonedx.org/participate/standardization-process/#working-model

Please have the related ticket/issue ID ready. 
If there is none, feel free to create a new ticket:
https://github.com/CycloneDX/specification/issues/new/choose

-->

<!-- 

Please provide a brief description of what this pull request intends to
do and which ticket it fixes/closes.
Example: 
> As discussed in ticket #485, this PR adds Streebog to the hash
algorithm enum.
>
> fixes #485 

In case this is for a spelling or grammar improvement, please provide a
brief description.
Example:
> Fixe typo: color(AE) -> colour(BE)

-->
@@ -7387,28 +7887,28 @@ limitations under the License.
</xs:annotation>
<xs:complexType>
<xs:sequence>
<xs:element name="encr" type="bom:refType" minOccurs="0" maxOccurs="unbounded">
Copy link
Member

Choose a reason for hiding this comment

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

Incompatible change of types, from simple string to complex type.

❌ this is a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

<xs:annotation>
<xs:documentation>
Transform Type 1: encryption algorithms
</xs:documentation>
</xs:annotation>
</xs:element>
<xs:element name="prf" type="bom:refType" minOccurs="0" maxOccurs="unbounded">
Copy link
Member

Choose a reason for hiding this comment

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

Incompatible change of types, from simple string to complex type.

❌ this is a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

<xs:annotation>
<xs:documentation>
Transform Type 2: pseudorandom functions
</xs:documentation>
</xs:annotation>
</xs:element>
<xs:element name="integ" type="bom:refType" minOccurs="0" maxOccurs="unbounded">
Copy link
Member

Choose a reason for hiding this comment

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

Incompatible change of types, from simple string to complex type.

❌ this is a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

<xs:annotation>
<xs:documentation>
Transform Type 3: integrity algorithms
</xs:documentation>
</xs:annotation>
</xs:element>
<xs:element name="ke" type="bom:refType" minOccurs="0" maxOccurs="unbounded">
Copy link
Member

Choose a reason for hiding this comment

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

Incompatible change of types, from simple string to complex type.

❌ this is a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

@@ -7422,7 +7922,7 @@ limitations under the License.
</xs:documentation>
</xs:annotation>
</xs:element>
<xs:element name="auth" type="bom:refType" minOccurs="0" maxOccurs="unbounded">
Copy link
Member

Choose a reason for hiding this comment

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

incompatible change of types.
❌ this is a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

Signed-off-by: Jan Kowalleck <[email protected]>
}

// IKEv2 Transform Types
message Ikev2TransformTypes {
// Transform Type 1: encryption algorithms
repeated string encr = 1;
Copy link
Member

Choose a reason for hiding this comment

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

was a string, became a message.
❌ this is a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

// Transform Type 2: pseudorandom functions
repeated string prf = 2;
Copy link
Member

Choose a reason for hiding this comment

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

was a string, became a message.
❌ this is a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

// Transform Type 3: integrity algorithms
repeated string integ = 3;
Copy link
Member

Choose a reason for hiding this comment

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

was a string, became a message.
❌ this is a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

// Transform Type 4: Key Exchange Method (KE) per RFC9370, formerly called Diffie-Hellman Group (D-H)
repeated string ke = 4;
Copy link
Member

Choose a reason for hiding this comment

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

was a string, became a message.
❌ this is a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

// Specifies if an Extended Sequence Number (ESN) is used.
optional bool esn = 5;
// IKEv2 Authentication method
repeated string auth = 6;
Copy link
Member

Choose a reason for hiding this comment

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

was a string, became a message.
❌ this is a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

@jkowalleck
Copy link
Member

jkowalleck commented Aug 14, 2025

did a review of the practical changes.
❌ there are breaking changes in this proposal - in the spec, and all the implementations.

I'd veto this PR as is.

Furthermore, I set it to "draft" state the current state MUST NOT be merged in its current state.


PS: if you could revert those breaking changes, and mark the respective elements as deprecate, and put new ones next them, that would be great.

pseudo example:

{
  "protocolProperties": { 
    "properties": {
      "auth": { 
        "deprecated": true,
        "description": "DEPRECATED ... use `@.auth-1.7`"
        /* revert your changes */ 
      },
      "auth-1.7": { /* your new stuff here */ }
    }
  }
}

Ooooor you could revert all the small structural changes you made, and simply add a whole new data structure while deprecating the old one.

pseudo example:

{
"cryptoProperties": { "properties" {

  "protocolProperties": { 
    "deprecated": true,
    "description": "DEPRECATED ... use `@.protocolProperties-1.7`"
    /* revert your changes */
  },
  "protocolProperties-1.7": { /* the new stuff ... */ }

} }
}

@jkowalleck jkowalleck marked this pull request as draft August 14, 2025 14:40
@jkowalleck jkowalleck changed the title [1.7] - Updates from CBOM working group [1.7] - Updates from CBOM working group - BREAKING CHANGES Aug 14, 2025
@jkowalleck jkowalleck requested a review from a team August 14, 2025 15:00
@stevespringett
Copy link
Member Author

IMO, we cannot wait until 2.0 to fix what is broken and unusable today.

@stevespringett stevespringett marked this pull request as ready for review August 15, 2025 01:20
@jkowalleck
Copy link
Member

jkowalleck commented Aug 15, 2025

IMO, we cannot wait until 2.0 to fix what is broken and unusable today.

broken by design - yes.
but there might be someone using it in this broken way, already.

there are clearly options to fix this in a non-breaking backwards-compatible way.
some are described here: #657 (comment)

the most important thing is, that the spec itself has breaking changes: a thing that was a string becomes a complex structure, now. and so do the implementations(JSON/XML/BP).
why not simply deprecate the old structure and introduce a new, "fixed" one?

…ition of one. Each JOSE component (tokens, algorithms, etc) can be represented as individual components within the CBOM.

Signed-off-by: Steve Springett <[email protected]>
…ition of one. Each JOSE component (tokens, algorithms, etc) can be represented as individual components within the CBOM.

Signed-off-by: Steve Springett <[email protected]>
@stevespringett
Copy link
Member Author

@jkowalleck The broken behavior was re-added to XML and JSON. Lets get this merged.

@jkowalleck
Copy link
Member

Still reviewing the changes .
Looks like some implementation have breaking changes, stil.
I need to double check and craft the needed test resources.

Sorry, this takes some time.

Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
Copy link
Member

Still reviewing the changes . Looks like some implementation have breaking changes, stil. I need to double check and craft the needed test resources.

Sorry, this takes some time.

Background: Some people from the community approached me last year, and they complained about unexpected breaking changes in the CycloneDX PB schema - which basically rendered all their efforts for interoperability useless.
I've introduced PB checks for breaking changes for a good reason - to prevent such unexpected changes, so that ProtoBuf can actually be used in the real-world. (see #384)

@jkowalleck
Copy link
Member

re #657 (comment)

@stevespringett , what do you think about #677 ? this will remove any breaking changes in the PB implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-changes CDX 1.7 related to release v1.7 proposed core enhancement request for comment RFC notice sent A public RFC notice was distributed to the CycloneDX mailing list for consideration test-data related to test-resources and -data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants