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

GODRIVER-2263 Load all certs in a PEM #834

Merged
merged 2 commits into from Jan 24, 2022

Conversation

thrawn01
Copy link
Contributor

@thrawn01 thrawn01 commented Jan 10, 2022

https://jira.mongodb.org/browse/GODRIVER-2263

When using tlsCertificateKeyFile or sslClientCertificateKeyFile options, ClientOptions.ApplyURI() only loads the final cert in the provided PEM file. This is undesired when a PEM contains multiple certs to be considered during a TLS hand shake. This PR attaches all certs and private keys found in the PEM and passes them to tls.X509KeyPair() for consideration.

@kevinAlbs kevinAlbs self-requested a review January 14, 2022 13:46
Copy link
Contributor

@kevinAlbs kevinAlbs 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! Thank you again for the contribution. I have one question.

I suggest adding a regression test. I am unable to add commits to this PR, but here is a proposed test: d3b0deb

mongo/options/clientoptions.go Show resolved Hide resolved
@kevinAlbs kevinAlbs self-requested a review January 18, 2022 01:38
Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

Thank you for adding the test. LGTM! Requesting additional Go team members for further review.

Copy link
Contributor

@benjirewis benjirewis 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 to me; thanks for your contribution, @thrawn01!

@kevinAlbs
Copy link
Contributor

Created DRIVERS-2038 to request this test be added to all drivers.

Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

Apologies for the slow review, looks great! Thanks for all the helpful docs links and test 👍

@kevinAlbs kevinAlbs merged commit ae48c67 into mongodb:master Jan 24, 2022
kevinAlbs pushed a commit that referenced this pull request Jan 24, 2022
faem pushed a commit to kubedb/mongo-go-driver that referenced this pull request Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants