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

Make KeyStoreUtil.updateWithServerPems load the full cert chain #674

Merged
merged 1 commit into from Apr 4, 2024

Conversation

shayelkin
Copy link
Contributor

updateWithServerPems uses CertificateFactory.GenerateCertificate(), which only loads the first certificate in the server certificate file. When using a certificate that is signed by an intermediate CA (like Let's Encrypt,) this would result in a certifiacte that won't be accepted by clients that only has the root CA, and not the intermediate CA, in their trusted certificates stores.

This replaces that call with a call to CertificateFactory.GenerateCertificates(), that loads the full chain, allowing clients with just the root chain to trust the server.

updateWithServerPems uses CertificateFactory.GenerateCertificate(), which only loads the
first certificate in the server certificate file. When using a certificate that is signed by
an intermediate CA (like Let's Encrypt,) this would result in a certifiacte that won't be
accepted by clients that only has the root CA, and not the intermediate CA, in their trusted
certificates stores.

This replaces that call with a call to CertificateFactory.GenerateCertificates(), that
loads the full chain, allowing clients with just the root chain to trust the server.

Signed-off-by: Shay Elkin <shay@elkin.io>
Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

LGTM.

@grgrzybek grgrzybek merged commit 58e9799 into jolokia:main Apr 4, 2024
0 of 3 checks passed
@shayelkin shayelkin deleted the load_cert_chain branch April 4, 2024 16:02
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.

None yet

3 participants