-
Notifications
You must be signed in to change notification settings - Fork 376
Add support for x5t and x5t#S256 header #669
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
Conversation
f6e2067
to
28a5e72
Compare
@hieuk09 Did you have some reference for how this should work? Could take a look at that to educate myself a bit. |
Now when looking closer at the changes I found some links... |
Thank you for the detailed review. I'll take a look and fix all the comments tomorrow. |
@anakinj could you have another review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the JWK approach is interesting. Im a bit concerned that we are cutting some corners here, as the x5* fields are pretty certificate specific and we are currently only dealing with keypairs.
@hieuk09 Was there something pending for this one still. To my understanding the functionality for picking the key field is totally optional now? |
@anakinj I think there are no pending tasks for this PR, the key field should be totally optional now. |
Description
Close #562
Checklist
Before the PR can be merged be sure the following are checked: