From 1ca2f8125eeaa7331b08fbc00d004578463f7231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Mon, 27 Sep 2021 16:03:18 +0200 Subject: [PATCH] rgw: use insecure TLS for bucket health check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have seen cases where the signed certificate used for the RGW does not contain the internal DNS endpoint, resulting in the health check to fail since the certificate is not valid for this domain. People consuming the gateways by external clients and for specific domains do not necessarily have the internal DNS configured in the certificate. So let's be a bit more flexible and simply ensure a connectivity check and bypass the certificate validation. Also, this is fixing the tls code in newS3Agent and adds unit tests. Closes: #8663 Signed-off-by: Sébastien Han (cherry picked from commit cda5dad291d940b5a7f596f125c9845c64b1146a) --- .../ceph/object/bucket/provisioner.go | 3 +- pkg/operator/ceph/object/health.go | 3 +- pkg/operator/ceph/object/rgw.go | 3 +- pkg/operator/ceph/object/s3-handlers.go | 26 +++--- pkg/operator/ceph/object/s3-handlers_test.go | 80 +++++++++++++++++++ tests/integration/ceph_base_object_test.go | 2 +- 6 files changed, 98 insertions(+), 19 deletions(-) create mode 100644 pkg/operator/ceph/object/s3-handlers_test.go diff --git a/pkg/operator/ceph/object/bucket/provisioner.go b/pkg/operator/ceph/object/bucket/provisioner.go index cdac0eb1d1e8..686f8ccbde75 100644 --- a/pkg/operator/ceph/object/bucket/provisioner.go +++ b/pkg/operator/ceph/object/bucket/provisioner.go @@ -622,7 +622,8 @@ func (p *Provisioner) setAdminOpsAPIClient() error { Timeout: cephObject.HttpTimeOut, } if p.tlsCert != nil { - httpClient.Transport = cephObject.BuildTransportTLS(p.tlsCert) + insecure := false + httpClient.Transport = cephObject.BuildTransportTLS(p.tlsCert, insecure) } // Fetch the ceph object store diff --git a/pkg/operator/ceph/object/health.go b/pkg/operator/ceph/object/health.go index a4651e2a8b83..1faa7c208a38 100644 --- a/pkg/operator/ceph/object/health.go +++ b/pkg/operator/ceph/object/health.go @@ -159,14 +159,13 @@ func (c *bucketChecker) checkObjectStoreHealth() error { } // Set access and secret key - tlsCert := c.objContext.TlsCert s3endpoint := c.objContext.Endpoint s3AccessKey := user.Keys[0].AccessKey s3SecretKey := user.Keys[0].SecretKey // Initiate s3 agent logger.Debugf("initializing s3 connection for object store %q", c.namespacedName.Name) - s3client, err := NewS3Agent(s3AccessKey, s3SecretKey, s3endpoint, "", false, tlsCert) + s3client, err := NewInsecureS3Agent(s3AccessKey, s3SecretKey, s3endpoint, "", false) if err != nil { return errors.Wrap(err, "failed to initialize s3 connection") } diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go index e922731cd886..aae30efe013f 100644 --- a/pkg/operator/ceph/object/rgw.go +++ b/pkg/operator/ceph/object/rgw.go @@ -362,7 +362,8 @@ func genObjectStoreHTTPClient(objContext *Context, spec *cephv1.ObjectStoreSpec) if err != nil { return nil, tlsCert, errors.Wrapf(err, "failed to fetch CA cert to establish TLS connection with object store %q", nsName) } - c.Transport = BuildTransportTLS(tlsCert) + insecure := false + c.Transport = BuildTransportTLS(tlsCert, insecure) } return c, tlsCert, nil } diff --git a/pkg/operator/ceph/object/s3-handlers.go b/pkg/operator/ceph/object/s3-handlers.go index 74b8b76c1ae9..a76d40879ee7 100644 --- a/pkg/operator/ceph/object/s3-handlers.go +++ b/pkg/operator/ceph/object/s3-handlers.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +Copyright 2018 The Rook Authors. All rights reserved. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -40,7 +40,7 @@ func NewS3Agent(accessKey, secretKey, endpoint, region string, debug bool, tlsCe return newS3Agent(accessKey, secretKey, endpoint, region, debug, tlsCert, false) } -func NewTestOnlyS3Agent(accessKey, secretKey, endpoint, region string, debug bool) (*S3Agent, error) { +func NewInsecureS3Agent(accessKey, secretKey, endpoint, region string, debug bool) (*S3Agent, error) { return newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, true) } @@ -60,14 +60,7 @@ func newS3Agent(accessKey, secretKey, endpoint, region string, debug bool, tlsCe tlsEnabled := false if len(tlsCert) > 0 || insecure { tlsEnabled = true - if len(tlsCert) > 0 { - client.Transport = BuildTransportTLS(tlsCert) - } else if insecure { - client.Transport = &http.Transport{ - // #nosec G402 is enabled only for testing - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } - } + client.Transport = BuildTransportTLS(tlsCert, insecure) } sess, err := session.NewSession( aws.NewConfig(). @@ -205,11 +198,16 @@ func (s *S3Agent) DeleteObjectInBucket(bucketname string, key string) (bool, err return true, nil } -func BuildTransportTLS(tlsCert []byte) *http.Transport { - caCertPool := x509.NewCertPool() - caCertPool.AppendCertsFromPEM(tlsCert) +func BuildTransportTLS(tlsCert []byte, insecure bool) *http.Transport { + // #nosec G402 is enabled only for testing + tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: insecure} + if len(tlsCert) > 0 { + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(tlsCert) + tlsConfig.RootCAs = caCertPool + } return &http.Transport{ - TLSClientConfig: &tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12}, + TLSClientConfig: tlsConfig, } } diff --git a/pkg/operator/ceph/object/s3-handlers_test.go b/pkg/operator/ceph/object/s3-handlers_test.go new file mode 100644 index 000000000000..0417d7eb2549 --- /dev/null +++ b/pkg/operator/ceph/object/s3-handlers_test.go @@ -0,0 +1,80 @@ +/* +Copyright 2021 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package object + +import ( + "net/http" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/stretchr/testify/assert" +) + +func TestNewS3Agent(t *testing.T) { + accessKey := "accessKey" + secretKey := "secretKey" + endpoint := "endpoint" + region := "region" + + t.Run("test without tls/debug", func(t *testing.T) { + debug := false + insecure := false + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, insecure) + assert.NoError(t, err) + assert.NotEqual(t, aws.LogDebug, s3Agent.Client.Config.LogLevel) + assert.Equal(t, nil, s3Agent.Client.Config.HTTPClient.Transport) + assert.True(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test with debug without tls", func(t *testing.T) { + debug := true + logLevel := aws.LogDebug + insecure := false + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, insecure) + assert.NoError(t, err) + assert.Equal(t, &logLevel, s3Agent.Client.Config.LogLevel) + assert.Nil(t, s3Agent.Client.Config.HTTPClient.Transport) + assert.True(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test without tls client cert but insecure tls", func(t *testing.T) { + debug := true + insecure := true + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, nil, insecure) + assert.NoError(t, err) + assert.Nil(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.RootCAs) + assert.True(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify) + assert.False(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test with secure tls client cert", func(t *testing.T) { + debug := true + insecure := false + tlsCert := []byte("tlsCert") + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, tlsCert, insecure) + assert.NoError(t, err) + assert.NotNil(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.RootCAs) + assert.False(t, *s3Agent.Client.Config.DisableSSL) + }) + t.Run("test with insesure tls client cert", func(t *testing.T) { + debug := true + insecure := true + tlsCert := []byte("tlsCert") + s3Agent, err := newS3Agent(accessKey, secretKey, endpoint, region, debug, tlsCert, insecure) + assert.NoError(t, err) + assert.NotNil(t, s3Agent.Client.Config.HTTPClient.Transport) + assert.True(t, s3Agent.Client.Config.HTTPClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify) + assert.False(t, *s3Agent.Client.Config.DisableSSL) + }) +} diff --git a/tests/integration/ceph_base_object_test.go b/tests/integration/ceph_base_object_test.go index e8b601c23f44..ac90663596a7 100644 --- a/tests/integration/ceph_base_object_test.go +++ b/tests/integration/ceph_base_object_test.go @@ -275,7 +275,7 @@ func testObjectStoreOperations(s suite.Suite, helper *clients.TestClient, k8sh * s3AccessKey, _ := helper.BucketClient.GetAccessKey(obcName) s3SecretKey, _ := helper.BucketClient.GetSecretKey(obcName) if objectStore.Spec.IsTLSEnabled() { - s3client, err = rgw.NewTestOnlyS3Agent(s3AccessKey, s3SecretKey, s3endpoint, region, true) + s3client, err = rgw.NewInsecureS3Agent(s3AccessKey, s3SecretKey, s3endpoint, region, true) } else { s3client, err = rgw.NewS3Agent(s3AccessKey, s3SecretKey, s3endpoint, region, true, nil) }