Skip to content

Add support for intermediate X.509 certificates #5

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Dec 27, 2022
Merged

Add support for intermediate X.509 certificates #5

merged 11 commits into from
Dec 27, 2022

Conversation

mmanes
Copy link
Contributor

@mmanes mmanes commented Dec 13, 2022

Summary:
Add support for intermediate X.509 certificates on TLS listener.

Fix:

  • When loading PEM certificates from string data, fully parse the content to include any intermediate certificates in the file. The intermediate certificates will be sent to the clients during the SSL negotiation and will be validated by the client.
  • Expose new constructors that accept Certificate[]s to supply a fully-created certificate chain.

Related:

@mmanes
Modified tests to use a 3-level CA certificate chain.
e4605a7 
Bumped generated certificate versions to v3 to allow for X509v3 extensions.
@mmanes
Merge branch 'master' into mmanes-feature_cert_chain
83f0df5
@mmanes
Add tests to verify intermediate certs used and client can validate p…
0b1ee02 
…ath up to a root.

Refactor HTTPListenerConfiguration to use a certificateChain by default while preserving the old behavior.
@mmanes
Merge branch 'master' into mmanes-feature_cert_chain
bbe1072
@mmanes
Minor cleanup and clarification.
1bbdff4
@mmanes mmanes requested review from voidmain and robotdan December 13, 2022 23:38
voidmain
voidmain approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@voidmain voidmain left a comment

Choose a reason for hiding this comment

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

Don't forget to update the version in build.savant. This looks like a compatible change, so 0.1.11 should be fine for SemVer compatibility.

robotdan
robotdan reviewed Dec 14, 2022
View reviewed changes
@mmanes mmanes requested a review from andrewpai December 16, 2022 16:24
mmanes
mmanes commented Dec 16, 2022
View reviewed changes
robotdan
robotdan reviewed Dec 19, 2022
View reviewed changes
Copy link
Member

@robotdan robotdan left a comment

Choose a reason for hiding this comment

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

Looks good, just a few comments.

@mmanes
Addressing PR comments:
25bd4c3 
* Reverted splitting of line in README.
* Added JavaDocs.
* Fix non-backwards compatible change of parseCertificate returning the first 're-ordered' certificate, rather than first certificate in PEM.
* Reverted parseDERFromPEM to original state. It is now only used for parsing keys.
@mmanes
Finish incomplete sentence in doc.
aa8dbf2
@mmanes
Added JavaDoc
9828f6f
@mmanes mmanes merged commit 296b92a into master Dec 27, 2022
@mmanes mmanes deleted the mmanes-feature_cert_chain branch December 27, 2022 21:18
@robotdan robotdan mentioned this pull request Jan 6, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robotdan robotdan robotdan left review comments

@voidmain voidmain voidmain approved these changes

@andrewpai andrewpai Awaiting requested review from andrewpai

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mmanes @voidmain @robotdan