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.

Sign up for GitHub

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

Jump to bottom

Clarify ISO mdoc Handover structure #415

Open
davidz25 opened this issue Feb 7, 2025 · 2 comments · May be fixed by #419
Open

Clarify ISO mdoc Handover structure #415

davidz25 opened this issue Feb 7, 2025 · 2 comments · May be fixed by #419

Comments

@davidz25
Copy link

davidz25 commented Feb 7, 2025

In B.3.4 it's defined how each side must construct the SessionTranscript using a handover defined in the following way

OpenID4VPDCAPIHandover = [
  "OpenID4VPDCAPIHandover", ; A fixed identifier for this handover type
  OpenID4VPDCAPIHandoverInfoHash ; A cryptographic hash of OpenID4VPDCAPIHandoverInfo
]

OpenID4VPDCAPIHandoverInfoHash = bstr  ; sha-256 hash of OpenID4VPDCAPIHandoverInfo

OpenID4VPDCAPIHandoverInfo = [
  origin,
  client_id,
  nonce
] ; Array containing handover parameters

client_id = tstr  ; UTF-8 encoded string

origin = tstr    ; UTF-8 encoded string

nonce = tstr     ; UTF-8 encoded string

There's a couple of problems here

  • All tstr are Unicode strings, it's meaningless to say "UTF-8 encoded string" because it's already tstr. Should just remove those comments.
  • OpenID4VPDCAPIHandoverInfoHash is defined as the "sha-256 hash of OpenID4VPDCAPIHandoverInfo" but doesn't make sense as you can only hash bytes (e.g. the encoded version of CBOR), not CBOR itself.

Here's what I would replace it with

OpenID4VPDCAPIHandover = [
  "OpenID4VPDCAPIHandover", ; A fixed identifier for this handover type
  OpenID4VPDCAPIHandoverInfoHash ; A cryptographic hash of OpenID4VPDCAPIHandoverInfo
]

; Contains the sha-256 hash of OpenID4VPDCAPIHandoverInfoBytes
OpenID4VPDCAPIHandoverInfoHash = bstr 

; Contains the encoding of OpenID4VPDCAPIHandoverInfo
OpenID4VPDCAPIHandoverInfoBytes = bstr .cbor OpenID4VPDCAPIHandoverInfo

OpenID4VPDCAPIHandoverInfo = [
  origin,
  client_id,
  nonce
] ; Array containing handover parameters

client_id = tstr

origin = tstr

nonce = tstr

I think this achieves what was originally intended and is more crisp. It's also consistent with what other standards are doing, for example see empty_or_serialized_map in COSE (RFC 9052).
@awoie
Copy link
Contributor

awoie commented Feb 9, 2025 •
edited
Loading

@davidz25 It is essentially what -7 rev2 is doing and -7 rev2 has similar language and CDDL. I remember that in the past somebody in ISO made a comment that it was not clear what tstr is using for the encoding, hence the inline comment on UTF-8 but I'm happy if we remove this if UTF-8 is anyhow the default encoding.

Re bstr .cbor I also agree, this CDDL is more crisp. Do you believe we would also need to adjust the text, or just the CDDL? IMO, the text is actually fine. Note that text and CDDL are similar to -7 rev2.

davidz25 added a commit to davidz25/OpenID4VP that referenced this issue Feb 10, 2025
@davidz25
Clarify ISO mdoc Handover structure.
1716677 
Fixes openid#415.

Signed-off-by: David Zeuthen <[email protected]>
@davidz25
Copy link
Author

Hi, thanks for looking. As for the text, I missed that in my original read-through (fixating on the CDDL), I think that could also be more crisp. I'm uploaded a PR with the changes, we can discuss there maybe.

@davidz25 davidz25 linked a pull request Feb 10, 2025 that will close this issue
Open
davidz25 added a commit to davidz25/OpenID4VP that referenced this issue Feb 10, 2025
@davidz25
Clarify ISO mdoc Handover structure.
15b8a1f 
Fixes openid#415.

Signed-off-by: David Zeuthen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Clarify ISO mdoc Handover structure. davidz25/OpenID4VP
2 participants
@davidz25 @awoie