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

Hannes's comments #195

Open
yogeshbdeshpande opened this issue Jan 25, 2024 · 27 comments
Open

Hannes's comments #195

yogeshbdeshpande opened this issue Jan 25, 2024 · 27 comments
Labels
mustfix This is essential requirement for CoRIM Publish

Comments

@yogeshbdeshpande
Copy link
Collaborator

Here are few points raised by Hannes, which are listed in sequence below, so that they are not lost and feedback incorporated.

  1. Document Readability : Overall, the document is hard to read. As a reader one is lost in details and there is no big picture. The introduction is 2 paragraphs long to be followed by lots of CDDL definitions in the terminology section.
  2. Document Purpose Need to clearly define the Goal of the document.
  3. Need a clear and consistent definition of a CoMID.
  4. Define the concept of "module composition" in context of CoRIM document
  5. Needs consistency in terminology: Example some places, language is referred as lang.
  6. Need to clearly highlight that JSON is also supported as encoding format for Triples and one should provide Examples as to how JSON Encoding will look like?
@thomas-fossati
Copy link
Collaborator

  1. Need to clearly highlight that JSON is also supported as encoding format for Triples and one should provide Examples as to how JSON Encoding will look like?

JSON is not in scope.

@yogeshbdeshpande
Copy link
Collaborator Author

If we are to be in sync with TCG Version, it clearly states about Support for JSON

@nedmsmith
Copy link
Collaborator

TCG DICE Endorsement Architecture is an information model. The data model is not normative (provided in an appendix). I-D.corim makes the data model normative.

@yogeshbdeshpande
Copy link
Collaborator Author

Too confusing Ned: Throughout the TCG document It mentions, support for JSON and CBOR
If I see March 2022 Revision of TCG DICE Endorsement Architecture: Section 5.2 is Information model
and
Section 5.5 is the Data Model.

What has changed since then?

@nedmsmith
Copy link
Collaborator

Section 5.5 is the Data Model

I meant to say data definition (i.e., CDDL representation) is in the appendix. Appendicies are non-normative in TCG specs. The references to JSON and CBOR are in gray background text blocks which are another form of informative. Effectively, the 'wire format' is not normative. CDDL, technically isn't a wire format (so it can be described as a data modelling language). But it maps to cbor readily, so it makes a reasonable substitute for a wire-format expression. CDDL can be tweaked to allow it to describe JSON as a wire format.

@yogeshbdeshpande
Copy link
Collaborator Author

I agree that expression in CDDL can be manifested in either CBOR or JSON

To this fact the specification is very clear in Section 4.4:

The CoRIM schema, expressed in CDDL, is used to generate the CoRIM file that typically is rendered in CBOR [18]
or JSON [19] format. This is illustrated in Figure 2.

@yogeshbdeshpande
Copy link
Collaborator Author

I think, this is a sensible statement and this is how it should be: Wherever CoRIM is expressed, be it in TCG Specification or IETF Specification. We should not force either way in any of the specification.

@thomas-fossati
Copy link
Collaborator

I am strongly against specifying a JSON serialisation in the IETF document.

The only serialisation we should describe is CBOR.

@thomas-fossati thomas-fossati changed the title CoRIM Comments Hannes's comments Jan 25, 2024
@yogeshbdeshpande
Copy link
Collaborator Author

yogeshbdeshpande commented Jan 26, 2024

I am strongly against specifying a JSON serialisation in the IETF document.

The only serialisation we should describe is CBOR.

Please specify your reason for the strong statement above.

@thomas-fossati
Copy link
Collaborator

I am strongly against specifying a JSON serialisation in the IETF document.
The only serialisation we should describe is CBOR.

Please specify your reason for the strong statement above.

scope creep

@yogeshbdeshpande
Copy link
Collaborator Author

Lets discuss this in our CoRIM Weekly:

We need to scope out the amount of effort before concluding that it is indeed a
scope creep, which cannot be handled OR it is a Scope Creep which can be managed:

The Benefits/PROS of including JSON is:

  1. Gives Flexibility and entails a Wider Adoption of CoRIM..

  2. The specification becomes aligned with other RATS specifications (example EAT)

  3. Makes more logical sense to handle JSON Schema (agreeing with Hannes assessment), given that:
    (a) CoRIM operates on a REST Interface (As per RATS Architecture)
    (b) JSON is more user friendly and so ease of use is better
    (c) Inter-working with SBOM Community and other types of Endorsements (like In-toto)

  4. Brings the specification in-line with TCG Specification

The CONS/Disadvantages are:

  1. Scope Creep: Yes this requires a change of direction a bit

  2. Need more work to do: Define new JWT Claim Names, registration and more.

Given that we are re-jigging Verifier Algorithm and also. the spec is
far down the line for becoming an RFC we should give this change a
serious thought before dropping this completely.

@thomas-fossati
Copy link
Collaborator

In the Abstract we state:

"[T]his document specifies the information elements for representing Endorsements and Reference Values in CBOR format."

In the Introduction we re-state:

"[T]his document specifies Concise Reference Integrity Manifests (CoRIM) a CBOR-based data model [...]"

which is the reason why we named it Co-RIM in the first place.

If there are good reasons for providing a JSON serialisation, I am sure someone will come up with a JoRIM or JRIM, or something like that at some point in the future.

At present, I see no reason to add a big chunk of extra work to ourselves: I'd rather spend our precious cycles finishing the document as is.

@nedmsmith
Copy link
Collaborator

In the Abstract we state:

"[T]his document specifies the information elements for representing Endorsements and Reference Values in CBOR format."

In the Introduction we re-state:

"[T]his document specifies Concise Reference Integrity Manifests (CoRIM) a CBOR-based data model [...]"

which is the reason why we named it Co-RIM in the first place.

If there are good reasons for providing a JSON serialisation, I am sure someone will come up with a JoRIM or JRIM, or something like that at some point in the future.

At present, I see no reason to add a big chunk of extra work to ourselves: I'd rather spend our precious cycles finishing the document as is.

+1

@yogeshbdeshpande
Copy link
Collaborator Author

Sure I understand the resistance to change. However given the fact about Endorsements which can be quite a descriptive type of information, just supporting CBOR can be a pain at some point. May be another IETF Draft down the line: Similar to CoTS draft may be the best way to compromise scope screep and requirements.

@nedmsmith
Copy link
Collaborator

just supporting CBOR can be a pain

The question is who feels the pain? There is pain on the side of the verifier who now has another format to parse and map to an internal representation. If the pain point is on rim authors, the Veraison approach seems to address this by allowing authors to use JSON while the wire format is CBOR.

@yogeshbdeshpande
Copy link
Collaborator Author

Yes, Veraison approach certainly helps to some extent. However Specific use cases have custom Endorsements, which Veraison cannot be aware of and so the Individual CoRIM Authors will have to natively encode them in CBOR before preparing an Endorsment CoRIM

@thomas-fossati
Copy link
Collaborator

Yes, Veraison approach certainly helps to some extent. However Specific use cases have custom Endorsements, which Veraison cannot be aware of and so the Individual CoRIM Authors will have to natively encode them in CBOR before preparing an Endorsment CoRIM

veraison/corim's extension mechanism should allow JSON encoding of the extension data items.

@yogeshbdeshpande
Copy link
Collaborator Author

I agree, extensions can be encoded in JSON!

@thomas-fossati
Copy link
Collaborator

I agree, extensions can be encoded in JSON!

So, the Veraison approach is sufficient, isn't it?

@yogeshbdeshpande
Copy link
Collaborator Author

Yes, I agree, Veraison approach can let encode a user a CoRIM in JSON format and then Once the JSON blob is output, then Veraison can help the user re-translate the same CoRIM in CBOR, does avoiding the pain, which I earlier commented.

@thomas-fossati
Copy link
Collaborator

I think we are talking past each other. In #195 (comment) you seemed to say that Veraison's approach was lacking in the case of custom user extensions.

@yogeshbdeshpande
Copy link
Collaborator Author

Yes, I stand corrected, I just checked and comment on #195 (comment) no longer holds true

@henkbirkholz
Copy link
Member

Amongst other things, the point of CoRIM is:

  • strong typing, including fool-proof semantic type extensibility
  • scalability down to IoT device and up to housing center scopes
  • the option to optimize on redundant strings without obfuscating content with compression
  • one standardized signature checking method

I could go on and on, but there is no point to force scope creep into the message representation. An exposition of JSON encoded data can easily happen (and is de-facto already happening) on the application layer. There is no reason to pull that down to the CoRIM structure.

@henkbirkholz
Copy link
Member

henkbirkholz commented Jan 31, 2024

Yes, Veraison approach certainly helps to some extent. However Specific use cases have custom Endorsements, which Veraison cannot be aware of and so the Individual CoRIM Authors will have to natively encode them in CBOR before preparing an Endorsment CoRIM

If that is actually a problem for a given real-world entity, I would feel quite uneasy trusting that entity to issue an endorsement in the first place, tbh.

@henkbirkholz
Copy link
Member

Sure I understand the resistance to change.

I'd also like to comment on that particular statement: As a contributor, I do not think that there is generic 'resistance to change' (see, for example, Thomas' proposal to simplify the core spec on a fundamental level). I do think there is valid resistance to your proposal, though.

@yogeshbdeshpande yogeshbdeshpande added the mustfix This is essential requirement for CoRIM Publish label May 7, 2024
@yogeshbdeshpande
Copy link
Collaborator Author

Clarity and other aspects raised in the issue are required for the first release of the specification!

@nedmsmith
Copy link
Collaborator

Recapping:

  1. Document Readability : Overall, the document is hard to read. As a reader one is lost in details and there is no big picture. The introduction is 2 paragraphs long to be followed by lots of CDDL definitions in the terminology section. (There are two requests here, one is readability - do the sentences make sense? The other is document structure. Readability is an ongoing thing. Document structure could be revisited. Should it read like a BNF grammar - which is top down, or like a reference document which defines all the parts in some sort of lexicon structure - sort of bottom up? The current structure is more the latter, but I don't think either is "right" or "wrong". Just different, different readers will have different opinions. I want to avoid rework simply due to someone's opinion because someone else will have a different opinion and you don't make progress.
  2. Document Purpose Need to clearly define the Goal of the document. (needs an owner)
  3. Need a clear and consistent definition of a CoMID. (needs an owner)
  4. Define the concept of "module composition" in context of CoRIM document (I find this ambiguous as the RATS Arch describes composition already and the schema is all about composition).
  5. Needs consistency in terminology: Example some places, language is referred as lang. (needs an owner)
  6. Need to clearly highlight that JSON is also supported as encoding format for Triples and one should provide Examples as to how JSON Encoding will look like? (Focus on CBOR only to avoid scope creep and move more quickly to WGLC)

2, 3, and 5 seem actionable.

Maybe the next step is to create separate issues for each and someone volunteers to do the work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mustfix This is essential requirement for CoRIM Publish
Projects
None yet
Development

No branches or pull requests

4 participants