From bc89351c7abef2aef47877c4c0fad4393b0fc97c Mon Sep 17 00:00:00 2001 From: Charith Ellawala <52399125+charith-elastic@users.noreply.github.com> Date: Fri, 20 Sep 2019 07:46:29 +0100 Subject: [PATCH] Fix handling of HTTP CA (#1742) * Improve certificate handling * Revert to old lint config * Update docs * Add missing period to doc --- docs/accessing-services.asciidoc | 5 +- docs/api-docs.asciidoc | 4 + docs/elasticsearch-spec.asciidoc | 7 +- pkg/apis/common/v1alpha1/association.go | 54 ++++--- pkg/controller/apmserver/config/config.go | 18 ++- .../apmserver/config/config_test.go | 144 ++++++++++++++++++ ...rverelasticsearchassociation_controller.go | 9 +- pkg/controller/common/association/ca.go | 18 ++- pkg/controller/common/association/ca_test.go | 14 +- .../certificates/http/certificates_secret.go | 5 +- .../common/certificates/http/public_secret.go | 3 + .../certificates/http/public_secret_test.go | 1 + .../common/certificates/http/reconcile.go | 6 +- pkg/controller/common/certificates/pem.go | 28 +++- .../certificates/ca_reconcile.go | 5 + pkg/controller/elasticsearch/driver/driver.go | 2 +- pkg/controller/elasticsearch/driver/nodes.go | 4 +- .../elasticsearch/nodespec/podspec_test.go | 6 +- .../elasticsearch/nodespec/resources.go | 7 +- .../elasticsearch/settings/merged_config.go | 10 +- .../settings/merged_config_test.go | 11 +- pkg/controller/kibana/config/settings.go | 13 +- pkg/controller/kibana/config/settings_test.go | 53 +++++-- .../association_controller.go | 9 +- 24 files changed, 346 insertions(+), 90 deletions(-) create mode 100644 pkg/controller/apmserver/config/config_test.go diff --git a/docs/accessing-services.asciidoc b/docs/accessing-services.asciidoc index bd9db2f1de..fdf64a6982 100644 --- a/docs/accessing-services.asciidoc +++ b/docs/accessing-services.asciidoc @@ -144,12 +144,13 @@ You can bring your own certificate to configure TLS to ensure that communication Create a Kubernetes secret with: -- `tls.crt`: the certificate (or a chain). +- `ca.crt`: CA certificate (optional if `tls.crt` was issued by a well-known CA). +- `tls.crt`: the certificate. - `tls.key`: the private key to the first certificate in the certificate chain. [source,sh] ---- -kubectl create secret tls my-cert --cert tls.crt --key tls.key +kubectl create secret generic my-cert --from-file=ca.crt=tls.crt --from-file=tls.crt=tls.crt --from-file=tls.key=tls.key ---- Then you just have to reference the secret name in the `http.tls.certificate` section of the resource manifest. diff --git a/docs/api-docs.asciidoc b/docs/api-docs.asciidoc index deaeb4ad86..221976a29c 100644 --- a/docs/api-docs.asciidoc +++ b/docs/api-docs.asciidoc @@ -220,6 +220,10 @@ _string_ _string_ | --- +| *`caCertProvided`* + +_bool_ +| +--- | *`caSecretName`* + _string_ | diff --git a/docs/elasticsearch-spec.asciidoc b/docs/elasticsearch-spec.asciidoc index 7b29fe9e91..6095ec82d6 100644 --- a/docs/elasticsearch-spec.asciidoc +++ b/docs/elasticsearch-spec.asciidoc @@ -215,8 +215,9 @@ spec: === Custom HTTP certificate You can provide your own CA and certificates instead of the self-signed certificate to connect to Elasticsearch via HTTPS using a Kubernetes secret. +The certificate must be stored under `tls.crt` and the private key must be stored under `tls.key`. If your certificate was not issued by a well-known CA, you must include the trust chain under `ca.crt` as well. -You need to reference the name of a secret that contains a TLS private key and a certificate (or a chain), in the `spec.http.tls.certificate` section. +You need to reference the name of a secret that contains a TLS private key and a certificate (and optionally, a trust chain), in the `spec.http.tls.certificate` section. [source,yaml] ---- @@ -231,8 +232,8 @@ This is an example on how create a Kubernetes TLS secret with a self-signed cert [source,sh] ---- -$ openssl req -x509 -newkey rsa:4096 -keyout tls.key -out tls.crt -days 365 -nodes -$ kubectl create secret tls my-cert --cert tls.crt --key tls.key +$ openssl req -x509 -sha256 -nodes -newkey rsa:4096 -days 365 -subj "/CN=quickstart-es-http" -addext "subjectAltName=DNS:quickstart-es-http.default.svc" -keyout tls.key -out tls.crt +$ kubectl create secret generic my-cert --from-file=ca.crt=tls.crt --from-file=tls.crt=tls.crt --from-file=tls.key=tls.key ---- [id="{p}-es-secure-settings"] diff --git a/pkg/apis/common/v1alpha1/association.go b/pkg/apis/common/v1alpha1/association.go index d14984bb74..ddf8ca5e4e 100644 --- a/pkg/apis/common/v1alpha1/association.go +++ b/pkg/apis/common/v1alpha1/association.go @@ -41,63 +41,71 @@ type Associator interface { type AssociationConf struct { AuthSecretName string `json:"authSecretName"` AuthSecretKey string `json:"authSecretKey"` + CACertProvided bool `json:"caCertProvided"` CASecretName string `json:"caSecretName"` URL string `json:"url"` } // IsConfigured returns true if all the fields are set. -func (esac *AssociationConf) IsConfigured() bool { - return esac.AuthIsConfigured() && esac.CAIsConfigured() && esac.URLIsConfigured() +func (ac *AssociationConf) IsConfigured() bool { + return ac.AuthIsConfigured() && ac.CAIsConfigured() && ac.URLIsConfigured() } // AuthIsConfigured returns true if all the auth fields are set. -func (esac *AssociationConf) AuthIsConfigured() bool { - if esac == nil { +func (ac *AssociationConf) AuthIsConfigured() bool { + if ac == nil { return false } - return esac.AuthSecretName != "" && esac.AuthSecretKey != "" + return ac.AuthSecretName != "" && ac.AuthSecretKey != "" } // CAIsConfigured returns true if the CA field is set. -func (esac *AssociationConf) CAIsConfigured() bool { - if esac == nil { +func (ac *AssociationConf) CAIsConfigured() bool { + if ac == nil { return false } - return esac.CASecretName != "" + return ac.CASecretName != "" } // URLIsConfigured returns true if the URL field is set. -func (esac *AssociationConf) URLIsConfigured() bool { - if esac == nil { +func (ac *AssociationConf) URLIsConfigured() bool { + if ac == nil { return false } - return esac.URL != "" + return ac.URL != "" } -func (esac *AssociationConf) GetAuthSecretName() string { - if esac == nil { +func (ac *AssociationConf) GetAuthSecretName() string { + if ac == nil { return "" } - return esac.AuthSecretName + return ac.AuthSecretName } -func (esac *AssociationConf) GetAuthSecretKey() string { - if esac == nil { +func (ac *AssociationConf) GetAuthSecretKey() string { + if ac == nil { return "" } - return esac.AuthSecretKey + return ac.AuthSecretKey } -func (esac *AssociationConf) GetCASecretName() string { - if esac == nil { +func (ac *AssociationConf) GetCACertProvided() bool { + if ac == nil { + return false + } + return ac.CACertProvided +} + +func (ac *AssociationConf) GetCASecretName() string { + if ac == nil { return "" } - return esac.CASecretName + return ac.CASecretName } -func (esac *AssociationConf) GetURL() string { - if esac == nil { +func (ac *AssociationConf) GetURL() string { + if ac == nil { return "" } - return esac.URL + return ac.URL } diff --git a/pkg/controller/apmserver/config/config.go b/pkg/controller/apmserver/config/config.go index ba150776e4..db12e921a9 100644 --- a/pkg/controller/apmserver/config/config.go +++ b/pkg/controller/apmserver/config/config.go @@ -51,15 +51,17 @@ func NewConfigFromSpec(c k8s.Client, as *v1alpha1.ApmServer) (*settings.Canonica if err != nil { return nil, err } - outputCfg = settings.MustCanonicalConfig( - map[string]interface{}{ - "output.elasticsearch.hosts": []string{as.AssociationConf().GetURL()}, - "output.elasticsearch.username": username, - "output.elasticsearch.password": password, - "output.elasticsearch.ssl.certificate_authorities": []string{filepath.Join(CertificatesDir, certificates.CertFileName)}, - }, - ) + tmpOutputCfg := map[string]interface{}{ + "output.elasticsearch.hosts": []string{as.AssociationConf().GetURL()}, + "output.elasticsearch.username": username, + "output.elasticsearch.password": password, + } + if as.AssociationConf().GetCACertProvided() { + tmpOutputCfg["output.elasticsearch.ssl.certificate_authorities"] = []string{filepath.Join(CertificatesDir, certificates.CAFileName)} + } + + outputCfg = settings.MustCanonicalConfig(tmpOutputCfg) } // Create a base configuration. diff --git a/pkg/controller/apmserver/config/config_test.go b/pkg/controller/apmserver/config/config_test.go new file mode 100644 index 0000000000..6751672f56 --- /dev/null +++ b/pkg/controller/apmserver/config/config_test.go @@ -0,0 +1,144 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package config + +import ( + "testing" + + "github.com/elastic/cloud-on-k8s/pkg/apis/apm/v1alpha1" + commonv1alpha1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1alpha1" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/settings" + "github.com/elastic/cloud-on-k8s/pkg/utils/k8s" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestNewConfigFromSpec(t *testing.T) { + testCases := []struct { + name string + configOverrides map[string]interface{} + assocConf *commonv1alpha1.AssociationConf + wantConf map[string]interface{} + wantErr bool + }{ + { + name: "default config", + }, + { + name: "with overridden config", + configOverrides: map[string]interface{}{ + "apm-server.secret_token": "MYSECRET", + }, + wantConf: map[string]interface{}{ + "apm-server.secret_token": "MYSECRET", + }, + }, + { + name: "without Elasticsearch CA cert", + assocConf: &commonv1alpha1.AssociationConf{ + AuthSecretName: "test-es-elastic-user", + AuthSecretKey: "elastic", + CASecretName: "test-es-http-ca-public", + CACertProvided: false, + URL: "https://test-es-http.default.svc:9200", + }, + wantConf: map[string]interface{}{ + "output.elasticsearch.hosts": []string{"https://test-es-http.default.svc:9200"}, + "output.elasticsearch.username": "elastic", + "output.elasticsearch.password": "password", + }, + }, + { + name: "with Elasticsearch CA cert", + assocConf: &commonv1alpha1.AssociationConf{ + AuthSecretName: "test-es-elastic-user", + AuthSecretKey: "elastic", + CASecretName: "test-es-http-ca-public", + CACertProvided: true, + URL: "https://test-es-http.default.svc:9200", + }, + wantConf: map[string]interface{}{ + "output.elasticsearch.hosts": []string{"https://test-es-http.default.svc:9200"}, + "output.elasticsearch.username": "elastic", + "output.elasticsearch.password": "password", + "output.elasticsearch.ssl.certificate_authorities": []string{"config/elasticsearch-certs/ca.crt"}, + }, + }, + { + name: "missing auth secret", + assocConf: &commonv1alpha1.AssociationConf{ + AuthSecretName: "wrong-secret", + AuthSecretKey: "elastic", + CASecretName: "test-es-http-ca-public", + CACertProvided: true, + URL: "https://test-es-http.default.svc:9200", + }, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := k8s.WrapClient(fake.NewFakeClient(mkAuthSecret())) + apmServer := mkAPMServer(tc.configOverrides, tc.assocConf) + gotConf, err := NewConfigFromSpec(client, apmServer) + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + + wantConf := mkConf(t, tc.wantConf) + diff := wantConf.Diff(gotConf, nil) + require.Len(t, diff, 0) + }) + } +} + +func mkAPMServer(config map[string]interface{}, assocConf *commonv1alpha1.AssociationConf) *v1alpha1.ApmServer { + apmServer := &v1alpha1.ApmServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "apm-server", + }, + Spec: v1alpha1.ApmServerSpec{ + Config: &commonv1alpha1.Config{Data: config}, + }, + } + + apmServer.SetAssociationConf(assocConf) + return apmServer +} + +func mkAuthSecret() *v1.Secret { + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-es-elastic-user", + }, + Data: map[string][]byte{ + "elastic": []byte("password"), + }, + } +} + +func mkConf(t *testing.T, overrides map[string]interface{}) *settings.CanonicalConfig { + t.Helper() + cfg, err := settings.NewCanonicalConfigFrom(map[string]interface{}{ + "apm-server.host": ":8200", + "apm-server.secret_token": "${SECRET_TOKEN}", + "apm-server.ssl.certificate": "/mnt/elastic-internal/http-certs/tls.crt", + "apm-server.ssl.enabled": true, + "apm-server.ssl.key": "/mnt/elastic-internal/http-certs/tls.key", + }) + require.NoError(t, err) + + overriddenCfg, err := settings.NewCanonicalConfigFrom(overrides) + require.NoError(t, err) + + require.NoError(t, cfg.MergeWith(overriddenCfg)) + return cfg +} diff --git a/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go b/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go index c614f36674..7c963ad439 100644 --- a/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go +++ b/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go @@ -278,7 +278,7 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer return commonv1alpha1.AssociationPending, err } - caSecretName, err := r.reconcileElasticsearchCA(apmServer, elasticsearchRef.NamespacedName()) + caSecret, err := r.reconcileElasticsearchCA(apmServer, elasticsearchRef.NamespacedName()) if err != nil { return commonv1alpha1.AssociationPending, err // maybe not created yet } @@ -288,7 +288,8 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer expectedAssocConf := &commonv1alpha1.AssociationConf{ AuthSecretName: authSecretRef.Name, AuthSecretKey: authSecretRef.Key, - CASecretName: caSecretName, + CACertProvided: caSecret.CACertProvided, + CASecretName: caSecret.Name, URL: services.ExternalServiceURL(es), } @@ -311,7 +312,7 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer return commonv1alpha1.AssociationEstablished, nil } -func (r *ReconcileApmServerElasticsearchAssociation) reconcileElasticsearchCA(apm *apmtype.ApmServer, es types.NamespacedName) (string, error) { +func (r *ReconcileApmServerElasticsearchAssociation) reconcileElasticsearchCA(apm *apmtype.ApmServer, es types.NamespacedName) (association.CASecret, error) { apmKey := k8s.ExtractNamespacedName(apm) // watch ES CA secret to reconcile on any change if err := r.watches.Secrets.AddHandler(watches.NamedWatch{ @@ -319,7 +320,7 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileElasticsearchCA(ap Watched: []types.NamespacedName{http.PublicCertsSecretRef(esname.ESNamer, es)}, Watcher: apmKey, }); err != nil { - return "", err + return association.CASecret{}, err } // Build the labels applied on the secret labels := labels.NewLabels(apm.Name) diff --git a/pkg/controller/common/association/ca.go b/pkg/controller/common/association/ca.go index 35da04301c..7cb7451db4 100644 --- a/pkg/controller/common/association/ca.go +++ b/pkg/controller/common/association/ca.go @@ -8,6 +8,7 @@ import ( "reflect" "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1alpha1" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/certificates" "github.com/elastic/cloud-on-k8s/pkg/controller/common/certificates/http" "github.com/elastic/cloud-on-k8s/pkg/controller/common/reconciler" esname "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/name" @@ -19,6 +20,12 @@ import ( "k8s.io/apimachinery/pkg/types" ) +// CASecret is a container to hold information about the Elasticsearch CA secret. +type CASecret struct { + Name string + CACertProvided bool +} + // ElasticsearchCACertSecretName returns the name of the secret holding the certificate chain used // by the associated resource to establish and validate a secured HTTP connection to Elasticsearch. func ElasticsearchCACertSecretName(associated v1alpha1.Associated, suffix string) string { @@ -34,16 +41,16 @@ func ReconcileCASecret( es types.NamespacedName, labels map[string]string, suffix string, -) (string, error) { +) (CASecret, error) { publicESHTTPCertificatesNSN := http.PublicCertsSecretRef(esname.ESNamer, es) // retrieve the HTTP certificates from ES namespace var publicESHTTPCertificatesSecret corev1.Secret if err := client.Get(publicESHTTPCertificatesNSN, &publicESHTTPCertificatesSecret); err != nil { if errors.IsNotFound(err) { - return "", nil // probably not created yet, we'll be notified to reconcile later + return CASecret{}, nil // probably not created yet, we'll be notified to reconcile later } - return "", err + return CASecret{}, err } // Certificate data should be copied over a secret in the associated namespace @@ -69,8 +76,9 @@ func ReconcileCASecret( reconciledSecret.Data = expectedSecret.Data }, }); err != nil { - return "", err + return CASecret{}, err } - return expectedSecret.Name, nil + _, caCertProvided := expectedSecret.Data[certificates.CAFileName] + return CASecret{Name: expectedSecret.Name, CACertProvided: caCertProvided}, nil } diff --git a/pkg/controller/common/association/ca_test.go b/pkg/controller/common/association/ca_test.go index e56eef0626..74e1e854f3 100644 --- a/pkg/controller/common/association/ca_test.go +++ b/pkg/controller/common/association/ca_test.go @@ -44,7 +44,8 @@ func TestReconcileAssociation_reconcileCASecret(t *testing.T) { Name: certificates.PublicSecretName(esname.ESNamer, es.Name, certificates.HTTPCAType), }, Data: map[string][]byte{ - certificates.CertFileName: []byte("fake-ca-cert"), + certificates.CertFileName: []byte("fake-cert"), + certificates.CAFileName: []byte("fake-ca-cert"), }, } updatedEsCA := corev1.Secret{ @@ -53,7 +54,8 @@ func TestReconcileAssociation_reconcileCASecret(t *testing.T) { Name: certificates.PublicSecretName(esname.ESNamer, es.Name, certificates.HTTPCAType), }, Data: map[string][]byte{ - certificates.CertFileName: []byte("updated-fake-ca-cert"), + certificates.CertFileName: []byte("updated-fake-cert"), + certificates.CAFileName: []byte("updated-fake-ca-cert"), }, } // mock existing ES CA secret for Kibana @@ -63,7 +65,8 @@ func TestReconcileAssociation_reconcileCASecret(t *testing.T) { Name: ElasticsearchCACertSecretName(&kibanaFixture, ElasticsearchCASecretSuffix), }, Data: map[string][]byte{ - certificates.CertFileName: []byte("fake-ca-cert"), + certificates.CertFileName: []byte("fake-cert"), + certificates.CAFileName: []byte("fake-ca-cert"), }, } updatedKibanaEsCA := corev1.Secret{ @@ -72,7 +75,8 @@ func TestReconcileAssociation_reconcileCASecret(t *testing.T) { Name: ElasticsearchCACertSecretName(&kibanaFixture, ElasticsearchCASecretSuffix), }, Data: map[string][]byte{ - certificates.CertFileName: []byte("updated-fake-ca-cert"), + certificates.CertFileName: []byte("updated-fake-cert"), + certificates.CAFileName: []byte("updated-fake-ca-cert"), }, } tests := []struct { @@ -120,7 +124,7 @@ func TestReconcileAssociation_reconcileCASecret(t *testing.T) { ) require.NoError(t, err) - require.Equal(t, tt.want, got) + require.Equal(t, tt.want, got.Name) if tt.wantCA != nil { var updatedKibanaCA corev1.Secret diff --git a/pkg/controller/common/certificates/http/certificates_secret.go b/pkg/controller/common/certificates/http/certificates_secret.go index c901936d6f..7da3fba4d8 100644 --- a/pkg/controller/common/certificates/http/certificates_secret.go +++ b/pkg/controller/common/certificates/http/certificates_secret.go @@ -18,10 +18,7 @@ type CertificatesSecret v1.Secret // CAPem returns the certificate of the certificate authority. func (s CertificatesSecret) CAPem() []byte { - if ca, exist := s.Data[certificates.CAFileName]; exist { - return ca - } - return nil + return s.Data[certificates.CAFileName] } // CertChain combines the certificate of the CA and the host certificate. diff --git a/pkg/controller/common/certificates/http/public_secret.go b/pkg/controller/common/certificates/http/public_secret.go index 656ffcc63e..f98f52a79e 100644 --- a/pkg/controller/common/certificates/http/public_secret.go +++ b/pkg/controller/common/certificates/http/public_secret.go @@ -33,6 +33,9 @@ func ReconcileHTTPCertsPublicSecret( certificates.CertFileName: httpCertificates.CertPem(), }, } + if caPem := httpCertificates.CAPem(); caPem != nil { + expected.Data[certificates.CAFileName] = caPem + } reconciled := &corev1.Secret{} diff --git a/pkg/controller/common/certificates/http/public_secret_test.go b/pkg/controller/common/certificates/http/public_secret_test.go index b68cb7eb70..6506638efa 100644 --- a/pkg/controller/common/certificates/http/public_secret_test.go +++ b/pkg/controller/common/certificates/http/public_secret_test.go @@ -51,6 +51,7 @@ func TestReconcileHTTPCertsPublicSecret(t *testing.T) { ObjectMeta: k8s.ToObjectMeta(namespacedSecretName), Data: map[string][]byte{ certificates.CertFileName: tls, + certificates.CAFileName: ca, }, } diff --git a/pkg/controller/common/certificates/http/reconcile.go b/pkg/controller/common/certificates/http/reconcile.go index 0aee33f299..3eba3f748a 100644 --- a/pkg/controller/common/certificates/http/reconcile.go +++ b/pkg/controller/common/certificates/http/reconcile.go @@ -120,8 +120,11 @@ func reconcileHTTPInternalCertificatesSecret( return nil, err } expectedSecretData := make(map[string][]byte) - expectedSecretData[certificates.CertFileName] = customCertificates.CertChain() + expectedSecretData[certificates.CertFileName] = customCertificates.CertPem() expectedSecretData[certificates.KeyFileName] = customCertificates.KeyPem() + if caPem := customCertificates.CAPem(); caPem != nil { + expectedSecretData[certificates.CAFileName] = caPem + } if !reflect.DeepEqual(secret.Data, expectedSecretData) { needsUpdate = true @@ -228,6 +231,7 @@ func ensureInternalSelfSignedCertificateSecretContents( secretWasChanged = true // store certificate and signed certificate in a secret mounted into the pod + secret.Data[certificates.CAFileName] = certificates.EncodePEMCert(ca.Cert.Raw) secret.Data[certificates.CertFileName] = certificates.EncodePEMCert(certData, ca.Cert.Raw) } diff --git a/pkg/controller/common/certificates/pem.go b/pkg/controller/common/certificates/pem.go index a8e5c45a5c..742aa1799a 100644 --- a/pkg/controller/common/certificates/pem.go +++ b/pkg/controller/common/certificates/pem.go @@ -9,6 +9,7 @@ import ( "crypto/rsa" "crypto/x509" "encoding/pem" + "fmt" "github.com/pkg/errors" ) @@ -60,10 +61,29 @@ func EncodePEMPrivateKey(privateKey rsa.PrivateKey) []byte { func ParsePEMPrivateKey(pemData []byte) (*rsa.PrivateKey, error) { block, _ := pem.Decode(pemData) if block == nil { - return nil, errors.New("can't decode pem block") + return nil, errors.New("failed to parse PEM block containing private key") } - if block.Type != "RSA PRIVATE KEY" || len(block.Headers) != 0 { - return nil, errors.New("pem block is not an RSA private key") + + switch { + case block.Type == "PRIVATE KEY": + return parsePKCS8PrivateKey(block.Bytes) + case block.Type == "RSA PRIVATE KEY" && len(block.Headers) == 0: + return x509.ParsePKCS1PrivateKey(block.Bytes) + default: + return nil, errors.New("expected PEM block to contain an RSA private key") + } +} + +func parsePKCS8PrivateKey(block []byte) (*rsa.PrivateKey, error) { + key, err := x509.ParsePKCS8PrivateKey(block) + if err != nil { + return nil, errors.Wrap(err, "failed to parse private key") } - return x509.ParsePKCS1PrivateKey(block.Bytes) + + rsaKey, ok := key.(*rsa.PrivateKey) + if !ok { + return nil, fmt.Errorf("expected an RSA private key but got %t", key) + } + + return rsaKey, nil } diff --git a/pkg/controller/elasticsearch/certificates/ca_reconcile.go b/pkg/controller/elasticsearch/certificates/ca_reconcile.go index 4057a24152..e1dff9612a 100644 --- a/pkg/controller/elasticsearch/certificates/ca_reconcile.go +++ b/pkg/controller/elasticsearch/certificates/ca_reconcile.go @@ -27,6 +27,9 @@ type CertificateResources struct { // TransportCA is the CA used for Transport certificates TransportCA *certificates.CA + + // HTTPCACertProvided indicates whether ca.crt key is defined in the certificate secret. + HTTPCACertProvided bool } // reconcileGenericResources reconciles the expected generic resources of a cluster. @@ -118,8 +121,10 @@ func Reconcile( return nil, results.WithError(err) } + _, httpCACertProvided := httpCertificates.Data[certificates.CAFileName] return &CertificateResources{ TrustedHTTPCertificates: trustedHTTPCertificates, TransportCA: transportCA, + HTTPCACertProvided: httpCACertProvided, }, results } diff --git a/pkg/controller/elasticsearch/driver/driver.go b/pkg/controller/elasticsearch/driver/driver.go index 068c0461d5..540667c50d 100644 --- a/pkg/controller/elasticsearch/driver/driver.go +++ b/pkg/controller/elasticsearch/driver/driver.go @@ -236,7 +236,7 @@ func (d *defaultDriver) Reconcile() *reconciler.Results { } // reconcile StatefulSets and nodes configuration - res = d.reconcileNodeSpecs(esReachable, esClient, d.ReconcileState, observedState, *resourcesState, keystoreResources) + res = d.reconcileNodeSpecs(esReachable, esClient, d.ReconcileState, observedState, *resourcesState, keystoreResources, certificateResources) if results.WithResults(res).HasError() { return results } diff --git a/pkg/controller/elasticsearch/driver/nodes.go b/pkg/controller/elasticsearch/driver/nodes.go index 53eeea67eb..fa728e257a 100644 --- a/pkg/controller/elasticsearch/driver/nodes.go +++ b/pkg/controller/elasticsearch/driver/nodes.go @@ -7,6 +7,7 @@ package driver import ( "github.com/elastic/cloud-on-k8s/pkg/controller/common/keystore" "github.com/elastic/cloud-on-k8s/pkg/controller/common/reconciler" + "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/certificates" esclient "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/client" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/nodespec" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/observer" @@ -24,6 +25,7 @@ func (d *defaultDriver) reconcileNodeSpecs( observedState observer.State, resourcesState reconcile.ResourcesState, keystoreResources *keystore.Resources, + certResources *certificates.CertificateResources, ) *reconciler.Results { results := &reconciler.Results{} @@ -41,7 +43,7 @@ func (d *defaultDriver) reconcileNodeSpecs( return results.WithResult(defaultRequeue) } - expectedResources, err := nodespec.BuildExpectedResources(d.ES, keystoreResources, d.Scheme()) + expectedResources, err := nodespec.BuildExpectedResources(d.ES, keystoreResources, d.Scheme(), certResources) if err != nil { return results.WithError(err) } diff --git a/pkg/controller/elasticsearch/nodespec/podspec_test.go b/pkg/controller/elasticsearch/nodespec/podspec_test.go index b584d5be1b..12337d2b72 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec_test.go +++ b/pkg/controller/elasticsearch/nodespec/podspec_test.go @@ -12,6 +12,7 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1alpha1" "github.com/elastic/cloud-on-k8s/pkg/controller/common/defaults" "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" + "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/certificates" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/initcontainer" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" "github.com/go-test/deep" @@ -87,10 +88,11 @@ var sampleES = v1alpha1.Elasticsearch{ } func TestBuildPodTemplateSpec(t *testing.T) { + certResources := certificates.CertificateResources{HTTPCACertProvided: true} nodeSpec := sampleES.Spec.Nodes[0] ver, err := version.Parse(sampleES.Spec.Version) require.NoError(t, err) - cfg, err := settings.NewMergedESConfig(sampleES.Name, *ver, sampleES.Spec.HTTP, *nodeSpec.Config) + cfg, err := settings.NewMergedESConfig(sampleES.Name, *ver, sampleES.Spec.HTTP, *nodeSpec.Config, &certResources) require.NoError(t, err) actual, err := BuildPodTemplateSpec(sampleES, sampleES.Spec.Nodes[0], cfg, nil) @@ -138,7 +140,7 @@ func TestBuildPodTemplateSpec(t *testing.T) { Labels: map[string]string{ "common.k8s.elastic.co/type": "elasticsearch", "elasticsearch.k8s.elastic.co/cluster-name": "name", - "elasticsearch.k8s.elastic.co/config-template-hash": "2449560134", + "elasticsearch.k8s.elastic.co/config-template-hash": "593041036", "elasticsearch.k8s.elastic.co/http-scheme": "https", "elasticsearch.k8s.elastic.co/node-data": "false", "elasticsearch.k8s.elastic.co/node-ingest": "true", diff --git a/pkg/controller/elasticsearch/nodespec/resources.go b/pkg/controller/elasticsearch/nodespec/resources.go index 628aa39bd3..b144b3d1a7 100644 --- a/pkg/controller/elasticsearch/nodespec/resources.go +++ b/pkg/controller/elasticsearch/nodespec/resources.go @@ -5,7 +5,6 @@ package nodespec import ( - "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -13,6 +12,8 @@ import ( commonv1alpha1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1alpha1" "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1alpha1" "github.com/elastic/cloud-on-k8s/pkg/controller/common/keystore" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" + "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/certificates" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/sset" @@ -36,7 +37,7 @@ func (l ResourcesList) StatefulSets() sset.StatefulSetList { return ssetList } -func BuildExpectedResources(es v1alpha1.Elasticsearch, keystoreResources *keystore.Resources, scheme *runtime.Scheme) (ResourcesList, error) { +func BuildExpectedResources(es v1alpha1.Elasticsearch, keystoreResources *keystore.Resources, scheme *runtime.Scheme, certResources *certificates.CertificateResources) (ResourcesList, error) { nodesResources := make(ResourcesList, 0, len(es.Spec.Nodes)) ver, err := version.Parse(es.Spec.Version) @@ -50,7 +51,7 @@ func BuildExpectedResources(es v1alpha1.Elasticsearch, keystoreResources *keysto if nodeSpec.Config != nil { userCfg = *nodeSpec.Config } - cfg, err := settings.NewMergedESConfig(es.Name, *ver, es.Spec.HTTP, userCfg) + cfg, err := settings.NewMergedESConfig(es.Name, *ver, es.Spec.HTTP, userCfg, certResources) if err != nil { return nil, err } diff --git a/pkg/controller/elasticsearch/settings/merged_config.go b/pkg/controller/elasticsearch/settings/merged_config.go index 2dbfa6360b..e49a3a7989 100644 --- a/pkg/controller/elasticsearch/settings/merged_config.go +++ b/pkg/controller/elasticsearch/settings/merged_config.go @@ -11,6 +11,7 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/controller/common/certificates" common "github.com/elastic/cloud-on-k8s/pkg/controller/common/settings" "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" + escerts "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/certificates" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/volume" ) @@ -21,6 +22,7 @@ func NewMergedESConfig( ver version.Version, httpConfig v1alpha1.HTTPConfig, userConfig v1alpha1.Config, + certResources *escerts.CertificateResources, ) (CanonicalConfig, error) { config, err := common.NewCanonicalConfigFrom(userConfig.Data) if err != nil { @@ -28,7 +30,7 @@ func NewMergedESConfig( } err = config.MergeWith( baseConfig(clusterName).CanonicalConfig, - xpackConfig(ver, httpConfig).CanonicalConfig, + xpackConfig(ver, httpConfig, certResources).CanonicalConfig, ) if err != nil { return CanonicalConfig{}, err @@ -56,7 +58,7 @@ func baseConfig(clusterName string) *CanonicalConfig { } // xpackConfig returns the configuration bit related to XPack settings -func xpackConfig(ver version.Version, httpCfg v1alpha1.HTTPConfig) *CanonicalConfig { +func xpackConfig(ver version.Version, httpCfg v1alpha1.HTTPConfig, certResources *escerts.CertificateResources) *CanonicalConfig { // enable x-pack security, including TLS cfg := map[string]interface{}{ // x-pack security general settings @@ -86,6 +88,10 @@ func xpackConfig(ver version.Version, httpCfg v1alpha1.HTTPConfig) *CanonicalCon }, } + if certResources.HTTPCACertProvided { + cfg[XPackSecurityHttpSslCertificateAuthorities] = path.Join(volume.HTTPCertificatesSecretVolumeMountPath, certificates.CAFileName) + } + // always enable the built-in file internal realm for user auth, ordered as first if ver.Major < 7 { // 6.x syntax diff --git a/pkg/controller/elasticsearch/settings/merged_config_test.go b/pkg/controller/elasticsearch/settings/merged_config_test.go index 8d0b6e1b83..a9658ad705 100644 --- a/pkg/controller/elasticsearch/settings/merged_config_test.go +++ b/pkg/controller/elasticsearch/settings/merged_config_test.go @@ -9,6 +9,7 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1alpha1" "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" + "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/certificates" "github.com/stretchr/testify/require" ) @@ -100,9 +101,13 @@ func TestNewMergedESConfig(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ver, err := version.Parse(tt.version) require.NoError(t, err) - cfg, err := NewMergedESConfig("clusterName", *ver, v1alpha1.HTTPConfig{}, v1alpha1.Config{ - Data: tt.cfgData, - }) + cfg, err := NewMergedESConfig( + "clusterName", + *ver, + v1alpha1.HTTPConfig{}, + v1alpha1.Config{Data: tt.cfgData}, + &certificates.CertificateResources{}, + ) require.NoError(t, err) tt.assert(cfg) }) diff --git a/pkg/controller/kibana/config/settings.go b/pkg/controller/kibana/config/settings.go index 6914774d66..9aa1345eb8 100644 --- a/pkg/controller/kibana/config/settings.go +++ b/pkg/controller/kibana/config/settings.go @@ -85,9 +85,14 @@ func kibanaTLSSettings(kb v1alpha1.Kibana) map[string]interface{} { } func elasticsearchTLSSettings(kb v1alpha1.Kibana) map[string]interface{} { - esCertsVolumeMountPath := es.CaCertSecretVolume(kb).VolumeMount().MountPath - return map[string]interface{}{ - ElasticsearchSslCertificateAuthorities: path.Join(esCertsVolumeMountPath, certificates.CertFileName), - ElasticsearchSslVerificationMode: "certificate", + cfg := map[string]interface{}{ + ElasticsearchSslVerificationMode: "certificate", + } + + if kb.AssociationConf().GetCACertProvided() { + esCertsVolumeMountPath := es.CaCertSecretVolume(kb).VolumeMount().MountPath + cfg[ElasticsearchSslCertificateAuthorities] = path.Join(esCertsVolumeMountPath, certificates.CAFileName) } + + return cfg } diff --git a/pkg/controller/kibana/config/settings_test.go b/pkg/controller/kibana/config/settings_test.go index e0fa31cb6c..55febad726 100644 --- a/pkg/controller/kibana/config/settings_test.go +++ b/pkg/controller/kibana/config/settings_test.go @@ -24,12 +24,12 @@ elasticsearch: username: "" password: "" ssl: - certificateAuthorities: /usr/share/kibana/config/elasticsearch-certs/tls.crt + certificateAuthorities: /usr/share/kibana/config/elasticsearch-certs/ca.crt verificationMode: certificate server: host: "0" name: "" - ssl: + ssl: enabled: true key: /mnt/elastic-internal/http-certs/tls.key certificate: /mnt/elastic-internal/http-certs/tls.crt @@ -44,7 +44,7 @@ xpack: func TestNewConfigSettings(t *testing.T) { type args struct { client k8s.Client - kb v1alpha1.Kibana + kb func() v1alpha1.Kibana } tests := []struct { name string @@ -55,15 +55,16 @@ func TestNewConfigSettings(t *testing.T) { { name: "default config", args: args{ - kb: v1alpha1.Kibana{}, + kb: mkKibana, }, want: defaultConfig, }, { name: "without TLS", args: args{ - kb: v1alpha1.Kibana{ - Spec: v1alpha1.KibanaSpec{ + kb: func() v1alpha1.Kibana { + kb := mkKibana() + kb.Spec = v1alpha1.KibanaSpec{ HTTP: commonv1alpha1.HTTPConfig{ TLS: commonv1alpha1.TLSOptions{ SelfSignedCertificate: &commonv1alpha1.SelfSignedCertificate{ @@ -71,7 +72,8 @@ func TestNewConfigSettings(t *testing.T) { }, }, }, - }, + } + return kb }, }, want: func() []byte { @@ -86,17 +88,40 @@ func TestNewConfigSettings(t *testing.T) { }(), wantErr: false, }, + { + name: "without Elasticsearch CA", + args: args{ + kb: func() v1alpha1.Kibana { + kb := mkKibana() + kb.SetAssociationConf(&commonv1alpha1.AssociationConf{CACertProvided: false}) + return kb + }, + }, + want: func() []byte { + cfg, err := settings.ParseConfig(defaultConfig) + require.NoError(t, err) + removed, err := (*ucfg.Config)(cfg).Remove("elasticsearch.ssl.certificateAuthorities", -1, settings.Options...) + require.True(t, removed) + require.NoError(t, err) + bytes, err := cfg.Render() + require.NoError(t, err) + return bytes + }(), + wantErr: false, + }, { name: "with user config", args: args{ - kb: v1alpha1.Kibana{ - Spec: v1alpha1.KibanaSpec{ + kb: func() v1alpha1.Kibana { + kb := mkKibana() + kb.Spec = v1alpha1.KibanaSpec{ Config: &commonv1alpha1.Config{ Data: map[string]interface{}{ "foo": "bar", }, }, - }, + } + return kb }, }, want: append(defaultConfig, []byte(`foo: bar`)...), @@ -104,7 +129,7 @@ func TestNewConfigSettings(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewConfigSettings(tt.args.client, tt.args.kb) + got, err := NewConfigSettings(tt.args.client, tt.args.kb()) if tt.wantErr { require.NotNil(t, err) } @@ -125,3 +150,9 @@ func TestNewConfigSettings(t *testing.T) { }) } } + +func mkKibana() v1alpha1.Kibana { + kb := v1alpha1.Kibana{} + kb.SetAssociationConf(&commonv1alpha1.AssociationConf{CACertProvided: true}) + return kb +} diff --git a/pkg/controller/kibanaassociation/association_controller.go b/pkg/controller/kibanaassociation/association_controller.go index 5d75fb851f..d800feffe4 100644 --- a/pkg/controller/kibanaassociation/association_controller.go +++ b/pkg/controller/kibanaassociation/association_controller.go @@ -282,7 +282,7 @@ func (r *ReconcileAssociation) reconcileInternal(kibana *kbtype.Kibana) (commonv return commonv1alpha1.AssociationPending, err } - caSecretName, err := r.reconcileElasticsearchCA(kibana, esRefKey) + caSecret, err := r.reconcileElasticsearchCA(kibana, esRefKey) if err != nil { return commonv1alpha1.AssociationPending, err } @@ -292,7 +292,8 @@ func (r *ReconcileAssociation) reconcileInternal(kibana *kbtype.Kibana) (commonv expectedESAssoc := &commonv1alpha1.AssociationConf{ AuthSecretName: authSecret.Name, AuthSecretKey: authSecret.Key, - CASecretName: caSecretName, + CACertProvided: caSecret.CACertProvided, + CASecretName: caSecret.Name, URL: services.ExternalServiceURL(es), } @@ -312,7 +313,7 @@ func (r *ReconcileAssociation) reconcileInternal(kibana *kbtype.Kibana) (commonv return commonv1alpha1.AssociationEstablished, nil } -func (r *ReconcileAssociation) reconcileElasticsearchCA(kibana *kbtype.Kibana, es types.NamespacedName) (string, error) { +func (r *ReconcileAssociation) reconcileElasticsearchCA(kibana *kbtype.Kibana, es types.NamespacedName) (association.CASecret, error) { kibanaKey := k8s.ExtractNamespacedName(kibana) // watch ES CA secret to reconcile on any change if err := r.watches.Secrets.AddHandler(watches.NamedWatch{ @@ -320,7 +321,7 @@ func (r *ReconcileAssociation) reconcileElasticsearchCA(kibana *kbtype.Kibana, e Watched: []types.NamespacedName{http.PublicCertsSecretRef(esname.ESNamer, es)}, Watcher: kibanaKey, }); err != nil { - return "", err + return association.CASecret{}, err } // Build the labels applied on the secret labels := kblabel.NewLabels(kibana.Name)