-
Notifications
You must be signed in to change notification settings - Fork 375
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
base: main
Are you sure you want to change the base?
Changes from all commits
8af6dbe
e1645c5
323ffc1
431b524
2ef7768
5008010
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,9 @@ def verify_key | |
def export(options = {}) | ||
exported = parameters.clone | ||
exported.reject! { |k, _| RSA_PRIVATE_KEY_ELEMENTS.include? k } unless private? && options[:include_private] == true | ||
|
||
exported[:x5t] = Base64.url_encode(OpenSSL::Digest::SHA1.new(rsa_key.to_der).digest) if options[:include_x5t] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im still a bit uncertain if we should make the decision here what the thumbprint is, the x5t is the thumbprint for the certificate, the RSA public key is just a part of the certificate. Think we could have a way to export a X509 certificate that could be adding these fields directly from the certificate itself. Also with this approach the thumbprint will differ if rsa_key is a private key or just the public key. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good point, thanks for pointing that out. Let me think a bit more about this. |
||
|
||
exported | ||
end | ||
|
||
|
@@ -67,7 +70,7 @@ def key_digest | |
def []=(key, value) | ||
raise ArgumentError, 'cannot overwrite cryptographic key attributes' if RSA_KEY_ELEMENTS.include?(key.to_sym) | ||
|
||
super(key, value) | ||
super | ||
end | ||
|
||
private | ||
|
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.
If the token happens to have both a kid and for example a x5t header it will never attempt using the x5t.
Not sure if this is a real scenario and could for sure be a later addition.
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 this is a real scenario: here is a payload and header of token from Azure:
The keys can be found in https://login.windows.net/common/discovery/keys.