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

OID4VC: Verify KB-JWT #1172

Merged
merged 8 commits into from
Nov 11, 2024
Merged

Conversation

TheTechmage
Copy link
Contributor

In an attempt to fix the issue raised in #1163, we should be storing the nonce generated for presentations then checking it later on. Now, we are storing it, then passing it down to the verification logic (along with the configured endpoint/client ID) to verify the KB-JWT.

Addressing openwallet-foundation#1163, we are now saving the nonce for presentations and
passing it along with the config endpoint down to the presentation
verification logic. This should begin verifying the KB-JWT, but I'm a
bit worried that the code may be fragile. I do not know the spec well
enough to know if the KB-JWT is required when the verifier passes a
nonce, or if it is completely optional.

This change effectively makes it required 100% of the time for
presentations.

Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
Apparently some changes had been made somewhere, breaking the demo. The
broken code in the demo has been changed and adapted to support the
changes that have occured within the OID4VC plugin.

Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
I originally thought I had put it on the presentation, but I stuck it on
the presentation definition instead. My bad, this change fixes that

Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
Because changes have been made to the signature for SD-JWT's KB-JWT
work, changes have been made to the regular JWT class and the base
classes.

Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
I thought I had got these, but the lines were still too long...

Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
Instead of passing down the nonce & aud parameters to the
verify_presentation method, pass down the entire presentation record
instead.

Since we're retrieving the AUD field from the profile's config, let's
just pull it out when we need it.

Also realized that verify_credential isn't even called from the route,
and so adding the presentation/nonce info may have broken whatever code
calls it (specifically in pex.py, which I did not modify and don't
understand how it works). Therefore, it was safer to remove it from the
signature.

Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
Signed-off-by: Colton Wolkins (Laptop) <[email protected]>
Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@dbluhm dbluhm merged commit 63cd990 into openwallet-foundation:main Nov 11, 2024
4 checks passed
@dbluhm dbluhm deleted the fix/kb-jwt branch November 11, 2024 23:31
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.

2 participants