rgw: use insecure TLS for bucket health check

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 <seb@redhat.com>

pkg/operator/ceph/object/bucket/provisioner.go

@@ -621,7 +621,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

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")
 	}

pkg/operator/ceph/object/rgw.go

@@ -361,7 +361,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
 }

pkg/operator/ceph/object/s3-handlers.go

@@ -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)
 }
 
@@ -59,15 +59,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 +197,15 @@ 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 {
+	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,
 	}
 }

pkg/operator/ceph/object/s3-handlers_test.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+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)
+	})
+	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)
+	})
+	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)
+	})
+	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)
+	})
+	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)
+	})
+}

tests/integration/ceph_object_test.go

@@ -41,6 +41,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var (
+	objectStoreServicePrefix = "rook-ceph-rgw-"
+)
+
 func TestCephObjectSuite(t *testing.T) {
 	if installer.SkipTestSuite(installer.CephTestSuite) {
 		t.Skip()
@@ -102,6 +106,16 @@ func (s *ObjectSuite) TestWithTLS() {
 	runObjectE2ETest(s.helper, s.k8sh, s.Suite, s.settings.Namespace, tls)
 }
 
+func (s *ObjectSuite) TestWithBrokenTLS() {
+	if utils.IsPlatformOpenShift() {
+		s.T().Skip("object store tests skipped on openshift")
+	}
+
+	tls := true
+	objectStoreServicePrefix = "broken"
+	runObjectE2ETest(s.helper, s.k8sh, s.Suite, s.settings.Namespace, tls)
+}
+
 func (s *ObjectSuite) TestWithoutTLS() {
 	if utils.IsPlatformOpenShift() {
 		s.T().Skip("object store tests skipped on openshift")
@@ -195,7 +209,7 @@ func checkCephObjectUser(
 // create a CephObjectStore and wait for it to report ready status
 func createCephObjectStore(t *testing.T, helper *clients.TestClient, k8sh *utils.K8sHelper, namespace, storeName string, replicaSize int, tlsEnable bool) {
 	logger.Infof("Create Object Store %q with replica count %d", storeName, replicaSize)
-	rgwServiceName := "rook-ceph-rgw-" + storeName
+	rgwServiceName := objectStoreServicePrefix + storeName
 	if tlsEnable {
 		t.Run("generate TLS certs", func(t *testing.T) {
 			generateRgwTlsCertSecret(t, helper, k8sh, namespace, storeName, rgwServiceName)
@@ -320,7 +334,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)
 		}