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

proposed fix: x509: certificate signed by unknown authority #2605

Merged
merged 2 commits into from
Mar 31, 2020

Conversation

pravus
Copy link
Contributor

@pravus pravus commented Mar 17, 2020

added loading of intermediate certificate chain into SSL context so clients can verify the server certificate properly

alpes214 and others added 2 commits March 13, 2020 11:02
Immediately kill offline backends for fast forward case
…lients can verify the server certificate properly
@pondix
Copy link
Contributor

pondix commented Mar 17, 2020

Automated message: PR pending admin approval for build testing

@JavierJF
Copy link
Collaborator

@pravus Hi! I have been reviewing the PR and I wanted to ask you which is your particular use case for needing to load the root CA certificate into the SSL context. This is an uncommon practice and it's many times contraindicated. Could you please elaborate why is this required for you? Thanks you!

@pravus
Copy link
Contributor Author

pravus commented Mar 23, 2020

@JavierJF

Hi! Our situation is that we are using a Let's Encrypt wildcard certificate that is signed by the X3 intermediary of the ISRG X1 root. Based on my own personal testing I was not able to get clients to properly connect to a ProxySQL instance with TLS enabled and the standard set of CA certificates. I was only able to get working connections after either altering the client's CA bundle, altering the client's CA/TLS settings directly, or using the patch I provided.

I am not a TLS expert but it has always been my understanding that while root certificates are installed on the system, intermediate certificates are generally distributed by services during connection negotiation. The intent of this change was to enable this behavior by providing the SSL context with the certificate chain which includes certificates that are not in the system bundle.

To be a little more generic, what I need is the ability to build a Docker container with a Go program installed along with the standard certificate bundle and have TLS connections work without having to install extra certificates or modifying the source of the Go program. The changes I have provided make this possible for me but if there is a better way to accomplish this then I am open to suggestions.

@JavierJF
Copy link
Collaborator

JavierJF commented Mar 31, 2020

@pravus Hi! So, after discussing the current behavior of certificates added to the certificate chain, we have decided that we are going to add the root certificate by default, this is because after further investigation we have found that mysql also adds the root certificate to the certificate chain, so, for honoring that behavior, we are going to do the same. This means that this PR matches this new desired approach for the SSL chain. I'm going to review and merge it. Thanks!

@JavierJF JavierJF merged commit 96516d5 into sysown:v2.0.11 Mar 31, 2020
@renecannao
Copy link
Contributor

Thank you @pravus and @JavierJF

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.

5 participants