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

Consider adding the full CA Certificate Chain to Trusted HTTP Certs for ES client #6574

Closed
naemono opened this issue Mar 23, 2023 · 0 comments · Fixed by #6681
Closed

Consider adding the full CA Certificate Chain to Trusted HTTP Certs for ES client #6574

naemono opened this issue Mar 23, 2023 · 0 comments · Fixed by #6681
Assignees
Labels
>enhancement Enhancement of existing functionality

Comments

@naemono
Copy link
Contributor

naemono commented Mar 23, 2023

Current situation

Currently when we create a new Elasticsearch HTTP Client for things such as the Observer, we provide a slice of trusted certificates []*x509.Certificate. This supports more than one certificate, but currently we seem to only provide the tls.crt key of the generated secret.

trustedHTTPCertificates, err := certificates.ParsePEMCerts(httpCerts.CertPem())

Note the httpCerts.CertPem(), which only returns the tls.crt key in that Map.

With a simple change such as

diff --git a/pkg/controller/elasticsearch/certificates/reconcile.go b/pkg/controller/elasticsearch/certificates/reconcile.go
index 4a0c040bb..b28baa419 100644
--- a/pkg/controller/elasticsearch/certificates/reconcile.go
+++ b/pkg/controller/elasticsearch/certificates/reconcile.go
@@ -75,7 +75,7 @@ func ReconcileHTTP(
                return nil, results
        }

-       trustedHTTPCertificates, err := certificates.ParsePEMCerts(httpCerts.CertPem())
+       trustedHTTPCertificates, err := certificates.ParsePEMCerts(httpCerts.CertChain())
        if err != nil {
                return nil, results.WithError(err)
        }

We could include the whole certificate chain, including the CA.

// CertChain combines the certificate of the CA and the host certificate.
func (s *CertificatesSecret) CertChain() []byte {
	return append(s.CertPem(), s.CAPem()...)
}

Why is this an issue?

When a customer provides their own certificate for Elasticsearch

spec:
  http:
    tls:
      certificate:
        secretName: custom-cert

and the certificate expires and is renewed by something such as cert-manager, there is a brief period of time where the operator's http client has the new tls certificate, but the Elasticsearch cluster hasn't fully propagated the certificate to all nodes in the cluster, and errors are seen in the logs x509: certificate signed by unknown authority because the http client only trusts the new tls certificate, and not the CA. By providing the full chain, this error goes away.

@naemono naemono added >enhancement Enhancement of existing functionality discuss We need to figure this out labels Mar 23, 2023
@naemono naemono self-assigned this Apr 12, 2023
@naemono naemono removed the discuss We need to figure this out label Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant