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

Valid let's encrypt certifcate considerated as invalid by streaming client #6529

Closed
gaetanfl opened this issue Jul 24, 2019 · 12 comments · Fixed by #6549
Closed

Valid let's encrypt certifcate considerated as invalid by streaming client #6529

gaetanfl opened this issue Jul 24, 2019 · 12 comments · Fixed by #6549
Assignees
Labels
bug needs triage Issues which need to be manually labelled priority/high Super important issue

Comments

@gaetanfl
Copy link

Bug report summary

A valid let's encrypt certificate is considered as invalid for reason 20 X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY by streaming client even if openssl _sclient return 0 in verify

OS / Environment

ubuntu 18.04

Netdata version (ouput of netdata -V)

netdata v1.16.0-70-nightly

Component Name

streaming

Steps To Reproduce
  • Create a let's encrypt certificate with proper hostname
  • Configure the master with full (chain+cert) file as certificate and key as ssl key
  • Configure a slave to use SSL for streaming with tcp:hostname:port:SSL without skipping certificate validation
  • watch the stream fail in error.log with SSL RFC4158 check: We have a invalid certificate, the tests result with 20 and message error:00000000:lib(0):func(0):reason(0)

Whereas openssl s_client works fine

echo Q | openssl s_client -connect <REDACTED_HOST>:PORT | grep -i verify
depth=2 O = Digital Signature Trust Co., CN = DST Root CA X3
verify return:1
depth=1 C = US, O = Let's Encrypt, CN = Let's Encrypt Authority X3
verify return:1
depth=0 CN = <REDACTED HOST>
verify return:1
DONE
    Verify return code: 0 (ok)
Expected behavior

[send to tcp:host:port]: established communication - ready to send metrics...

@gaetanfl gaetanfl added bug needs triage Issues which need to be manually labelled labels Jul 24, 2019
@cakrit cakrit added the priority/high Super important issue label Jul 24, 2019
@cakrit
Copy link
Contributor

cakrit commented Jul 24, 2019

Please check @thiagoftsm
If we have an issue here as well, we should add the fix to #6468 so we can validate with the other changes and get the patch release out

@thiagoftsm
Copy link
Contributor

Hi @gaetanfl ,

I was studying your issue report now, and I would like to ask you more few questions for we fix this problem:

1 - What is the openssl version installed in your Ubuntu 18.04?
2 - When you access the Netdata using browser with the URL https://<REDACTED_HOST>:PORT , does your browser reports an error?
3 - Is it possible you test your certificate with the site https://www.ssllabs.com/ssltest/index.html and send us the output? Case it is not, can you at least said us how many tests had a Grade 'A'.

I saw that the error you are having is a common error with Squid too, and in the squid side at least is common the people to report a problem with "Incomplete Chain".

@gaetanfl
Copy link
Author

Hi @gaetanfl ,

Hi thank for taking care of this @thiagoftsm

I was studying your issue report now, and I would like to ask you more few questions for we fix this problem:

1 - What is the openssl version installed in your Ubuntu 18.04?

openssl version
OpenSSL 1.1.1  11 Sep 2018
libssl-dev:amd64                              1.1.1-1ubuntu2.1~18.04.4

2 - When you access the Netdata using browser with the URL https://<REDACTED_HOST>:PORT , does your browser reports an error?
Got a netdata not allowed message but https did not raise any issue

3 - Is it possible you test your certificate with the site https://www.ssllabs.com/ssltest/index.html and send us the output? Case it is not, can you at least said us how many tests had a Grade 'A'.

SSL labs does not support high ports like that but I tested on https://www.immuniweb.com/ssl and it got A- due to lack of tls v1.3 support mostly and no ocsp stappling… nothin related to bad chain

I saw that the error you are having is a common error with Squid too, and in the squid side at least is common the people to report a problem with "Incomplete Chain".

There is only two certificates in my full message, will try to add the last one but looks like the verify openssl method is not using the server recognized roots but a more restricted one

@gaetanfl
Copy link
Author

There is only two certificates in my full certificate, will try to add the last one but looks like the verify openssl method is not using the server recognized roots but a more restricted one

Adding the root X3 cert in the chain provoked an error 19 in check because of the self signed certificate

@thiagoftsm
Copy link
Contributor

Hi @gaetanfl ,

We realized a deep analises to find the best way to handle this problem that you are having and we arrive in the conclusion that we can do one of the next two options to fix this:

  • To change the documentation giving instructions for our users to do download of the server certificate and store it in the default OpenSSL directory where are stored the known certificates.
  • To bring a new option to the file stream.conf where it will be possible to specify a specific directory inside the Netdata directory strucutre that will have the Netdata master certificate. In this option, the Netdata can store automatically the certificate and keep it there, because our users will need to specify in the stream "destination" option what is the IP address of the master.

For both situation, we will create a chain of known Netdata SSL certificates to avoid the error X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY.
Right now, unless somebody give us an argument different, our preference is the last option. For sure something will be changed in Netdata for next versions to avoid this error reported by you.

Thank you very much to give us this report!

@gaetanfl
Copy link
Author

We realized a deep analises to find the best way to handle this problem that you are having and we arrive in the conclusion that we can do one of the next two options to fix this:

Thanks for investigating it. I'll let some comment from my operational point of view.

  • To change the documentation giving instructions for our users to do download of the server certificate and store it in the default OpenSSL directory where are stored the known certificates.

It seems a bit tedious regarding let's encrypt policy of short certificate. It would mean that every time the cert renew, the post-renew hook should deploy the cert on all clients. Might it be possible to just specify the path to the CA signing it like /etc/ssl/certs/DST_Root_CA_X3.pem or even better /etc/ssl/certs/ca-certifcates.crt

  • To bring a new option to the file stream.conf where it will be possible to specify a specific directory inside the Netdata directory strucutre that will have the Netdata master certificate. In this option, the Netdata can store automatically the certificate and keep it there, because our users will need to specify in the stream "destination" option what is the IP address of the master.

In this case, it seems that we have to download the certificate over an unencrypted connection, which does not look good.

For both situation, we will create a chain of known Netdata SSL certificates to avoid the error X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY.

Right now, unless somebody give us an argument different, our preference is the last option. For sure something will be changed in Netdata for next versions to avoid this error reported by you.

Thanks, in my opinion, specifying the signing ca cert is the better way to go.

Thinking about the issue I got to this idea (didn't test or else, you may yet thought about this): the signing chain is X3 CA -> LE -> my cert. So the cert given by the master is the concatenation of LE CA and my cert. Is the client verifying well thos case of two concatenate certs ? I will make some tests and keep you posted.

Thank you very much to give us this report!

Thanks for addressing it

@thiagoftsm
Copy link
Contributor

Thanks to share your point of view with us, today this will be my top priority.
I will also execute tests here in my side, for we release a fix for this ASAP.

@gaetanfl
Copy link
Author

gaetanfl commented Jul 25, 2019

I mocked the way it connects to ssl in a small C program. By adding those lines I fixed the issue

SSL_CTX_load_verify_locations(ctx, "/etc/ssl/certs/ca-certificates.crt",
                                       "/etc/ssl/certs/");

SSL_CTX_set_default_verify_paths(ctx);

making it configurable for path and file should solve the issue.
I'm out of time to try more until next Wednesday but this looks like promising

@thiagoftsm
Copy link
Contributor

Hi @gaetanfl ,

This is the exactly road that we were plainning to use, thanks to confirm!
I will do few tests with a code that looks like a lot this, and I will create PR with the solution.
@cakrit considering that @gaetanfl confirmed to myself the code that I will need to use to fix this problem, in my opinion he can be cited in our release notes for the 1.16.1.

@thiagoftsm
Copy link
Contributor

Hi @gaetanfl ,

I created a PR as I said previously for you, I saw that with the code we talked yesterday, it is also possible to enable the use of self-signed certificate without to raise a warning, case you have the possibility, please test the PR to confirm that it is killing the bug.

Best Regards!

@Daniel15
Copy link
Contributor

I know this is an old issue, however it helped me. I couldn't get streaming connections working using Let's Encrypt certs until I modified the config to set:

CApath = /etc/ssl/certs/
CAfile = /etc/ssl/certs/ca-certificates.crt

I wonder if Netdata should automatically use these paths on Debian?

@thiagoftsm
Copy link
Contributor

Hello @Daniel15 ,

As you said this looks like a Let's encrypt issue, probably we need to add a new topic to our documentation related to Let's Encrypt certificate. I remember that I also had problems with Let's encrypt certificate on CentOS. @joelhans we need to think something about this.

Best regards!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs triage Issues which need to be manually labelled priority/high Super important issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants