Skip to content

add verifier public key to the session transcript#470

Merged
Sakurann merged 4 commits intomainfrom
awoie/add-pub-key-to-session-transcript
Apr 1, 2025
Merged

add verifier public key to the session transcript#470
Sakurann merged 4 commits intomainfrom
awoie/add-pub-key-to-session-transcript

Conversation

@awoie
Copy link
Copy Markdown
Contributor

@awoie awoie commented Mar 24, 2025

Depends on #448 (on the removal of client_id from SessionTranscript).

This PR includes the thumbprint of the public verification key of the verifier in the SessionTranscript.

Fixes #400

Copy link
Copy Markdown
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.

Type should either be bstr if we use the hash directly, or we need to clarify that the hash is base6url encoded

Comment thread openid-4-verifiable-presentations-1_0.md Outdated
@awoie
Copy link
Copy Markdown
Contributor Author

awoie commented Mar 24, 2025

Type should either be bstr if we use the hash directly, or we need to clarify that the hash is base6url encoded

changed to bstr.

@awoie awoie requested a review from c2bo March 24, 2025 19:01
@Sakurann Sakurann added this to the Final 1.0 milestone Mar 24, 2025
Copy link
Copy Markdown
Contributor

@paulbastian paulbastian left a comment

Choose a reason for hiding this comment

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

I think this is a substantial important changes, that needs further elaboration on the motivation:

  • add a note under this section that this is a security measurement for unsigned requests
  • please add security consideration that the RP should check that jwk_thumbprint matches its key and what to do when it does not match

@awoie
Copy link
Copy Markdown
Contributor Author

awoie commented Mar 25, 2025

I think this is a substantial important changes, that needs further elaboration on the motivation:

  • add a note under this section that this is a security measurement for unsigned requests

I can add a note regarding the purpose of the same section since it only applies to mdocs.

  • please add security consideration that the RP should check that jwk_thumbprint matches its key and what to do when it does not match

The RP does not have to check that the jwk_thumbprint has to match since the SessionTranscript is computed by both parties, verifier and wallet, individually. If the Verifier uses a different key than the wallet, the deviceSigned won't verify since the signature/mac is invalid.

@awoie awoie requested a review from paulbastian March 27, 2025 16:01
@awoie
Copy link
Copy Markdown
Contributor Author

awoie commented Mar 27, 2025

@paulbastian please review again. I added the purpose of this to the respective section.

@awoie awoie force-pushed the awoie/add-pub-key-to-session-transcript branch from 7153d73 to 4ca6448 Compare March 27, 2025 16:14
Comment thread openid-4-verifiable-presentations-1_0.md Outdated
Comment thread openid-4-verifiable-presentations-1_0.md Outdated
@awoie awoie requested a review from paulbastian March 28, 2025 15:55
@paulbastian
Copy link
Copy Markdown
Contributor

paulbastian commented Mar 28, 2025

I believe a dedicated section in the security considerations to give guidance for Verifier in these cases would be appropriate, but that shouldn't block this from being merged.

@Sakurann
Copy link
Copy Markdown
Collaborator

I believe a dedicated section in the security considerations to give guidance for Verifier in these cases would be appropriate, but that shouldn't block this from being merged.

I agree.. we should be more diligent in adding security considerations when we do PRs that apply cc @danielfett

@awoie
Copy link
Copy Markdown
Contributor Author

awoie commented Mar 31, 2025

I believe a dedicated section in the security considerations to give guidance for Verifier in these cases would be appropriate, but that shouldn't block this from being merged.

I agree.. we should be more diligent in adding security considerations when we do PRs that apply cc @danielfett

We have already a note on that in the PR as @paulbastian requested. IMO, a security considerations section would only make sense if it applies to all credential formats but we decided to do this for mdocs only to begin with. If @paulbastian could unblock since we added a note?

@Sakurann Sakurann merged commit 8eefd87 into main Apr 1, 2025
2 checks passed
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.

include verifier's public encryption key into SessionTranscript to prevent some possible attacks in case of unsigned requests

5 participants