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

Clarify ISO mdoc Handover structure. #419

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davidz25
Copy link

Fixes #415.

@davidz25 davidz25 requested a review from awoie February 10, 2025 19:03
@c2bo
Copy link
Member

c2bo commented Feb 11, 2025

Would it make sense to introduce tstr and bstr briefly as Major Type 2/3 with a reference to the CBOR spec? That reference to the Major Type 3 definition would also make it pretty clear that tstr must be UTF-8 encoded.

I would also assume that not all readers of this spec will be that familiar with CBOR and CDDL - an explicit reference might help.

@Sakurann
Copy link
Collaborator

+1 to two things Christian is suggesting (introduce tstr and bstr and references) as it is very true that the audience of this spec cannot be assumed to be cbor experts

Copy link
Member

@c2bo c2bo left a comment

Choose a reason for hiding this comment

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

As mentioned, I think this section would benefit from introducing and referencing the CBOR types, but we could also do that in another PR.

I think the proposed changes to clarify the hashing are quite helpful.

* The first element MUST be the fixed UTF-8 encoded string `OpenID4VPDCAPIHandover`. This serves as a unique identifier for the handover structure to prevent misinterpretation or confusion.
* The second element MUST be the `OpenID4VPDCAPIHandoverInfoHash`, represented as a CBOR byte string which encodes the sha-256 hash of the `OpenID4VPDCAPIHandoverInfo` CBOR array.
* The first element MUST be the string `OpenID4VPDCAPIHandover`. This serves as a unique identifier for the handover structure to prevent misinterpretation or confusion.
* The second element MUST be a bytestring which contains the sha-256 hash of the bytes of `OpenID4VPDCAPIHandoverInfo` when encoded as CBOR.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* The second element MUST be a bytestring which contains the sha-256 hash of the bytes of `OpenID4VPDCAPIHandoverInfo` when encoded as CBOR.
* The second element MUST be a byte string which contains the sha-256 hash of the bytes of `OpenID4VPDCAPIHandoverInfo` when encoded as CBOR.

Copy link
Collaborator

@jogu jogu left a comment

Choose a reason for hiding this comment

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

Looks good, thanks David! I think further changes are necessary (clarifying effective client id, adding examples) but there are other issues open for that so it would be good to get this merged so the other issues can be worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify ISO mdoc Handover structure
4 participants