From 38b94d08db790e0907947d8f20e02fb283b1d004 Mon Sep 17 00:00:00 2001 From: Tomasz Spyrka Date: Fri, 14 Jul 2023 12:40:05 +0200 Subject: [PATCH 1/5] Fail TLS configuration if provided certificates do not exist --- healthcheck/tools/db/ssl.go | 63 +++++++++++------------ healthcheck/tools/db/ssl_test.go | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 35 deletions(-) create mode 100644 healthcheck/tools/db/ssl_test.go diff --git a/healthcheck/tools/db/ssl.go b/healthcheck/tools/db/ssl.go index 305301dc88..8be5cffc48 100644 --- a/healthcheck/tools/db/ssl.go +++ b/healthcheck/tools/db/ssl.go @@ -48,56 +48,49 @@ func LastSSLError() error { } func (cnf *Config) configureTLS() error { - config := &tls.Config{ - InsecureSkipVerify: cnf.SSL.Insecure, - } - - if len(cnf.SSL.PEMKeyFile) == 0 || len(cnf.SSL.CAFile) == 0 { + if !cnf.SSL.Enabled { return nil } - pemOk, err := isFileExists(cnf.SSL.PEMKeyFile) - if err != nil { - return errors.Wrapf(err, "check if file with name %s exists", cnf.SSL.PEMKeyFile) - } - - caOk, err := isFileExists(cnf.SSL.CAFile) - if err != nil { - return errors.Wrapf(err, "check if file with name %s exists", cnf.SSL.CAFile) + config := &tls.Config{ + InsecureSkipVerify: cnf.SSL.Insecure, } - if !pemOk || !caOk { - cnf.SSL = nil - return nil - } + // Configure client cert + if len(cnf.SSL.PEMKeyFile) != 0 { + if err := isFileExists(cnf.SSL.PEMKeyFile); err != nil { + return errors.Wrapf(err, "check if file with name %s exists", cnf.SSL.PEMKeyFile) + } - log.Debugf("Loading SSL/TLS PEM certificate: %s", cnf.SSL.PEMKeyFile) + log.Debugf("Loading SSL/TLS PEM certificate: %s", cnf.SSL.PEMKeyFile) + certificates, err := tls.LoadX509KeyPair(cnf.SSL.PEMKeyFile, cnf.SSL.PEMKeyFile) + if err != nil { + return errors.Wrapf(err, "load key pair from '%s' to connect to server '%s'", cnf.SSL.PEMKeyFile, cnf.Hosts) + } - certificates, err := tls.LoadX509KeyPair(cnf.SSL.PEMKeyFile, cnf.SSL.PEMKeyFile) - if err != nil { - return errors.Wrapf(err, "load key pair from '%s' to connect to server '%s'", cnf.SSL.PEMKeyFile, cnf.Hosts) + config.Certificates = []tls.Certificate{certificates} } - config.Certificates = []tls.Certificate{certificates} + // Configure CA cert + if len(cnf.SSL.CAFile) != 0 { + if err := isFileExists(cnf.SSL.CAFile); err != nil { + return errors.Wrapf(err, "check if file with name %s exists", cnf.SSL.CAFile) + } - log.Debugf("Loading SSL/TLS Certificate Authority: %s", cnf.SSL.CAFile) - ca, err := cnf.SSL.loadCaCertificate() - if err != nil { - return errors.Wrapf(err, "load client CAs from %s", cnf.SSL.CAFile) + log.Debugf("Loading SSL/TLS Certificate Authority: %s", cnf.SSL.CAFile) + ca, err := cnf.SSL.loadCaCertificate() + if err != nil { + return errors.Wrapf(err, "load client CAs from %s", cnf.SSL.CAFile) + } + + config.RootCAs = ca } - config.RootCAs = ca cnf.TLSConf = config - return nil } -func isFileExists(name string) (bool, error) { +func isFileExists(name string) error { _, err := os.Stat(name) - if os.IsNotExist(err) { - return false, nil - } else if err != nil { - return false, err - } - return true, nil + return err } diff --git a/healthcheck/tools/db/ssl_test.go b/healthcheck/tools/db/ssl_test.go new file mode 100644 index 0000000000..36f0241b85 --- /dev/null +++ b/healthcheck/tools/db/ssl_test.go @@ -0,0 +1,86 @@ +package db + +import ( + "fmt" + "testing" +) + +const ( + notExistingFilePath = "not-existing-file-path" +) + +func TestSSLNotEnabled(t *testing.T) { + cfg := &Config{ + SSL: &SSLConfig{ + Enabled: false, + }, + } + + if err := cfg.configureTLS(); err != nil { + t.Fatalf("TLS configuration failed: %s", err) + } + + if cfg.TLSConf != nil { + t.Error("Expected TLSConf to be nil") + } +} + +func TestSSLEnabled(t *testing.T) { + cfg := &Config{ + SSL: &SSLConfig{ + Enabled: true, + }, + } + + if err := cfg.configureTLS(); err != nil { + t.Fatalf("TLS configuration failed: %s", err) + } + + if cfg.TLSConf == nil { + t.Error("Expected TLSConf to not be nil") + } +} + +func TestPEMKeyFileDoesNotExists(t *testing.T) { + cfg := &Config{ + SSL: &SSLConfig{ + Enabled: true, + PEMKeyFile: notExistingFilePath, + }, + } + + err := cfg.configureTLS() + if err == nil { + t.Fatal("Expected TLS config to fail, but it returned no error") + } + + expectedErrorMessage := fmt.Sprintf( + "check if file with name %s exists: stat %s: no such file or directory", + notExistingFilePath, notExistingFilePath, + ) + if err.Error() != expectedErrorMessage { + t.Errorf("error message '%s' does not match expected '%s'", err.Error(), expectedErrorMessage) + } +} + +func TestCAFileDoesNotExists(t *testing.T) { + cfg := &Config{ + SSL: &SSLConfig{ + Enabled: true, + CAFile: notExistingFilePath, + }, + } + + err := cfg.configureTLS() + if err == nil { + t.Fatal("Expected TLS config to fail, but it returned no error") + } + + expectedErrorMessage := fmt.Sprintf( + "check if file with name %s exists: stat %s: no such file or directory", + notExistingFilePath, notExistingFilePath, + ) + if err.Error() != expectedErrorMessage { + t.Errorf("error message '%s' does not match expected '%s'", err.Error(), expectedErrorMessage) + } +} From 24b77a9e6e328ac9b11e82e7206c5f0e7f04a084 Mon Sep 17 00:00:00 2001 From: Tomasz Spyrka Date: Tue, 18 Jul 2023 16:11:34 +0200 Subject: [PATCH 2/5] Skip TLS config for probe when using unsafe config --- pkg/apis/psmdb/v1/psmdb_defaults.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/apis/psmdb/v1/psmdb_defaults.go b/pkg/apis/psmdb/v1/psmdb_defaults.go index d7333e475a..d7679a0482 100644 --- a/pkg/apis/psmdb/v1/psmdb_defaults.go +++ b/pkg/apis/psmdb/v1/psmdb_defaults.go @@ -205,7 +205,7 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log }, } - if cr.CompareVersion("1.7.0") >= 0 { + if cr.CompareVersion("1.7.0") >= 0 && !cr.Spec.UnsafeConf { cr.Spec.Sharding.Mongos.LivenessProbe.Exec.Command = append(cr.Spec.Sharding.Mongos.LivenessProbe.Exec.Command, "--ssl", "--sslInsecure", @@ -250,7 +250,7 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log }, } - if cr.CompareVersion("1.7.0") >= 0 { + if cr.CompareVersion("1.7.0") >= 0 && !cr.Spec.UnsafeConf { cr.Spec.Sharding.Mongos.ReadinessProbe.Exec.Command = append(cr.Spec.Sharding.Mongos.ReadinessProbe.Exec.Command, "--ssl", "--sslInsecure", @@ -380,7 +380,7 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log if cr.CompareVersion("1.6.0") >= 0 { replset.LivenessProbe.Probe.Exec.Command[0] = "/data/db/mongodb-healthcheck" - if cr.CompareVersion("1.7.0") >= 0 { + if cr.CompareVersion("1.7.0") >= 0 && !cr.Spec.UnsafeConf { replset.LivenessProbe.Probe.Exec.Command = append(replset.LivenessProbe.Probe.Exec.Command, "--ssl", "--sslInsecure", @@ -664,14 +664,14 @@ func (nv *NonVotingSpec) SetDefaults(cr *PerconaServerMongoDB, rs *ReplsetSpec) } if nv.LivenessProbe.ProbeHandler.Exec == nil { nv.LivenessProbe.Probe.ProbeHandler.Exec = &corev1.ExecAction{ - Command: []string{ - "/data/db/mongodb-healthcheck", - "k8s", - "liveness", - "--ssl", "--sslInsecure", - "--sslCAFile", "/etc/mongodb-ssl/ca.crt", - "--sslPEMKeyFile", "/tmp/tls.pem", - }, + Command: []string{"/data/db/mongodb-healthcheck", "k8s", "liveness"}, + } + + if !cr.Spec.UnsafeConf { + nv.LivenessProbe.Probe.ProbeHandler.Exec.Command = append( + nv.LivenessProbe.Probe.ProbeHandler.Exec.Command, + "--ssl", "--sslInsecure", "--sslCAFile", "/etc/mongodb-ssl/ca.crt", "--sslPEMKeyFile", "/tmp/tls.pem", + ) } if cr.CompareVersion("1.14.0") >= 0 { From 0ec0f3f811c2cf97fb5f631d78270e44b81cda30 Mon Sep 17 00:00:00 2001 From: Tomasz Spyrka Date: Wed, 19 Jul 2023 09:15:07 +0200 Subject: [PATCH 3/5] Remove healthcheck ssl config from e2e where it was not expected --- .../compare/statefulset_another-name-rs0-4-oc.yml | 6 ------ .../init-deploy/compare/statefulset_another-name-rs0-oc.yml | 6 ------ .../init-deploy/compare/statefulset_another-name-rs0.yml | 6 ------ .../init-deploy/compare/statefulset_some-name-rs0-oc.yml | 6 ------ e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml | 6 ------ e2e-tests/one-pod/compare/statefulset_one-pod-rs0-oc.yml | 6 ------ .../one-pod/compare/statefulset_one-pod-rs0-secret-oc.yml | 6 ------ .../one-pod/compare/statefulset_one-pod-rs0-secret.yml | 6 ------ e2e-tests/one-pod/compare/statefulset_one-pod-rs0.yml | 6 ------ 9 files changed, 54 deletions(-) diff --git a/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-4-oc.yml b/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-4-oc.yml index 96960c0370..644f10bfa0 100644 --- a/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-4-oc.yml +++ b/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-4-oc.yml @@ -76,12 +76,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-oc.yml b/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-oc.yml index f2681ed43a..737042b5bc 100644 --- a/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-oc.yml +++ b/e2e-tests/init-deploy/compare/statefulset_another-name-rs0-oc.yml @@ -76,12 +76,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/init-deploy/compare/statefulset_another-name-rs0.yml b/e2e-tests/init-deploy/compare/statefulset_another-name-rs0.yml index 510561582c..eed6dd7fc1 100644 --- a/e2e-tests/init-deploy/compare/statefulset_another-name-rs0.yml +++ b/e2e-tests/init-deploy/compare/statefulset_another-name-rs0.yml @@ -76,12 +76,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml index 53a9df1a93..e1d5e2916b 100644 --- a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml +++ b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml @@ -75,12 +75,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml index a60426e3d5..16ebdbe5c0 100644 --- a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml +++ b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml @@ -75,12 +75,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-oc.yml b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-oc.yml index bb34e38592..f74fdeed2b 100644 --- a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-oc.yml +++ b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-oc.yml @@ -86,12 +86,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret-oc.yml b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret-oc.yml index 5c41ecd51d..efff23a1c6 100644 --- a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret-oc.yml +++ b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret-oc.yml @@ -86,12 +86,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret.yml b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret.yml index 791687b1fc..0e4f2eaa64 100644 --- a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret.yml +++ b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0-secret.yml @@ -86,12 +86,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0.yml b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0.yml index 4ae7ea5f89..07133bf611 100644 --- a/e2e-tests/one-pod/compare/statefulset_one-pod-rs0.yml +++ b/e2e-tests/one-pod/compare/statefulset_one-pod-rs0.yml @@ -86,12 +86,6 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness - - --ssl - - --sslInsecure - - --sslCAFile - - /etc/mongodb-ssl/ca.crt - - --sslPEMKeyFile - - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 From bca13372dbb4023cc6cfcfddb60fc3faf6cb17c6 Mon Sep 17 00:00:00 2001 From: Tomasz Spyrka Date: Wed, 19 Jul 2023 11:09:53 +0200 Subject: [PATCH 4/5] Revert ssl config removal for 'some-name' rs 'allowUnsafeConfigurations' is set for 'another-name' rs only --- .../init-deploy/compare/statefulset_some-name-rs0-oc.yml | 6 ++++++ e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml index e1d5e2916b..53a9df1a93 100644 --- a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml +++ b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0-oc.yml @@ -75,6 +75,12 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness + - --ssl + - --sslInsecure + - --sslCAFile + - /etc/mongodb-ssl/ca.crt + - --sslPEMKeyFile + - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 diff --git a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml index 16ebdbe5c0..a60426e3d5 100644 --- a/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml +++ b/e2e-tests/init-deploy/compare/statefulset_some-name-rs0.yml @@ -75,6 +75,12 @@ spec: - /opt/percona/mongodb-healthcheck - k8s - liveness + - --ssl + - --sslInsecure + - --sslCAFile + - /etc/mongodb-ssl/ca.crt + - --sslPEMKeyFile + - /tmp/tls.pem - --startupDelaySeconds - "7200" failureThreshold: 4 From 06c966e46baa54f8135661eea844aa2f51616b04 Mon Sep 17 00:00:00 2001 From: Andrii Dema Date: Mon, 18 Sep 2023 16:56:51 +0300 Subject: [PATCH 5/5] compare version fixes --- pkg/apis/psmdb/v1/psmdb_defaults.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/apis/psmdb/v1/psmdb_defaults.go b/pkg/apis/psmdb/v1/psmdb_defaults.go index 22718c8263..9f9ef7b136 100644 --- a/pkg/apis/psmdb/v1/psmdb_defaults.go +++ b/pkg/apis/psmdb/v1/psmdb_defaults.go @@ -191,7 +191,8 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log }, } - if cr.CompareVersion("1.7.0") >= 0 && !cr.Spec.UnsafeConf { + if (cr.CompareVersion("1.7.0") >= 0 && cr.CompareVersion("1.15.0") < 0) || + cr.CompareVersion("1.15.0") >= 0 && !cr.Spec.UnsafeConf { cr.Spec.Sharding.Mongos.LivenessProbe.Exec.Command = append(cr.Spec.Sharding.Mongos.LivenessProbe.Exec.Command, "--ssl", "--sslInsecure", @@ -236,7 +237,8 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log }, } - if cr.CompareVersion("1.7.0") >= 0 && !cr.Spec.UnsafeConf { + if (cr.CompareVersion("1.7.0") >= 0 && cr.CompareVersion("1.15.0") < 0) || + cr.CompareVersion("1.15.0") >= 0 && !cr.Spec.UnsafeConf { cr.Spec.Sharding.Mongos.ReadinessProbe.Exec.Command = append(cr.Spec.Sharding.Mongos.ReadinessProbe.Exec.Command, "--ssl", "--sslInsecure", @@ -362,7 +364,8 @@ func (cr *PerconaServerMongoDB) CheckNSetDefaults(platform version.Platform, log if cr.CompareVersion("1.6.0") >= 0 { replset.LivenessProbe.Probe.Exec.Command[0] = "/data/db/mongodb-healthcheck" - if cr.CompareVersion("1.7.0") >= 0 && !cr.Spec.UnsafeConf { + if (cr.CompareVersion("1.7.0") >= 0 && cr.CompareVersion("1.15.0") < 0) || + cr.CompareVersion("1.15.0") >= 0 && !cr.Spec.UnsafeConf { replset.LivenessProbe.Probe.Exec.Command = append(replset.LivenessProbe.Probe.Exec.Command, "--ssl", "--sslInsecure", @@ -635,7 +638,7 @@ func (nv *NonVotingSpec) SetDefaults(cr *PerconaServerMongoDB, rs *ReplsetSpec) Command: []string{"/data/db/mongodb-healthcheck", "k8s", "liveness"}, } - if !cr.Spec.UnsafeConf { + if !cr.Spec.UnsafeConf || cr.CompareVersion("1.15.0") < 0 { nv.LivenessProbe.Probe.ProbeHandler.Exec.Command = append( nv.LivenessProbe.Probe.ProbeHandler.Exec.Command, "--ssl", "--sslInsecure", "--sslCAFile", "/etc/mongodb-ssl/ca.crt", "--sslPEMKeyFile", "/tmp/tls.pem",