-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
[fixes 31700] Add mTLS support for http backend by way of client cert & key, as well as enterprise cacert. #31699
Conversation
Thanks for this submission! |
Thank you for this PR. I'd appreciate if you could provide a guide for manually testing this change. |
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.
Some general naming comments below! I've not closely reviewed the actual code because @kmoe is already on that. 😀
Well… I did put a unit test in there, which describes its usage - I teased out the "self-signed" certificate used in httptest and wrote its pem files out to use for trust of the server (since it's self signed it is the CA as well), and to present to the server as client cert. I guess to test this manually… one would create a cert that's used ONLY for the CA, then create a server cert and a client cert that's signed by that… then add that to the RootCAs of the client and the ClientCAs of the server as well as check assumptions about the principals' reported Common Name (CN field of subject). This technique would thwart the chicken-and-egg issue I had with using the test cert - that is, in order to get the cert, I needed to fire up the server, but in order to add to the ClientCAs, one would need to know the signing cert up front… which could be more easily done if that cert were known ahead of time and configured in the server before starting it in the |
I'm going on vacation on the 21th and I have things to wrap up at work before I go, but on my return, I could publish a test setup with our etcd backend if you like. I want this change :) |
Hi - we started using the s3+dynamo solution for a few needs so this lost urgency for me. But... I thought I had added a test to show the use of it... I'll try to make that test more specifically about this, if not also document setting up outside of test, as well as address the naming convention comments so this can merge. |
a84020e
to
2d96b2a
Compare
FWIW, I had to force push to make the license appear signed (I recall reading that commits made prior to adding a user address don't get tied to your account) so rebasing and pushing seemed to have solved that |
…ce the HTTPClient as the retryablehttp already creates one - just configure its TLS.
Co-authored-by: kmoe <[email protected]>
Co-authored-by: kmoe <[email protected]>
Co-authored-by: kmoe <[email protected]>
Co-authored-by: kmoe <[email protected]>
Co-authored-by: kmoe <[email protected]>
Co-authored-by: kmoe <[email protected]>
231c24f
to
13bbe7c
Compare
Ok - I removed use of testify and now go.mod does not contain reference to it again. |
Done - rebased and force pushed - now all changes appear directly from the merge-base (no merges) |
…nd ca cert to be the data. See also hashicorp/terraform-provider-http#211
Any news? |
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.
Thanks for your patience. Happy to merge this - will be available in the next 1.4 alpha for testing.
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
For any http servers that require mTLS (client certificates), as well as using custom cacert rather than skipping cert validation altogether, add the following settings:
cacert
: verify server certs using this trust filecert
: provide a public cert from the client to serverkey
: sign the public cert and all TLS traffic with a private key