Merge pull request #9057 from rook/mergify/bp/release-1.7/pr-9020

rgw: read tls secret hint for insecure tls (backport #9020)

Documentation/ceph-object-store-crd.md

@@ -91,7 +91,18 @@ When the `zone` section is set pools with the object stores name will not be cre
 The gateway settings correspond to the RGW daemon settings.
 
 * `type`: `S3` is supported
-* `sslCertificateRef`: If specified, this is the name of the Kubernetes secret(`opaque` or `tls` type) that contains the TLS certificate to be used for secure connections to the object store. Rook will look in the secret provided at the `cert` key name. The value of the `cert` key must be in the format expected by the [RGW service](https://docs.ceph.com/docs/master/install/ceph-deploy/install-ceph-gateway/#using-ssl-with-civetweb): "The server key, server certificate, and any other CA or intermediate certificates be supplied in one file. Each of these items must be in PEM form."
+* `sslCertificateRef`: If specified, this is the name of the Kubernetes secret(`opaque` or `tls`
+  type) that contains the TLS certificate to be used for secure connections to the object store.
+  If it is an opaque Kubernetes Secret, Rook will look in the secret provided at the `cert` key name. The value of the `cert` key must be
+  in the format expected by the [RGW
+  service](https://docs.ceph.com/docs/master/install/ceph-deploy/install-ceph-gateway/#using-ssl-with-civetweb):
+  "The server key, server certificate, and any other CA or intermediate certificates be supplied in
+  one file. Each of these items must be in PEM form." They are scenarios where the certificate DNS is set for a particular domain
+  that does not include the local Kubernetes DNS, namely the object store DNS service endpoint. If
+  adding the service DNS name to the certificate is not empty another key can be specified in the
+  secret's data: `insecureSkipVerify: true` to skip the certificate verification. It is not
+  recommended to enable this option since TLS is susceptible to machine-in-the-middle attacks unless
+  custom verification is used.
 * `port`: The port on which the Object service will be reachable. If host networking is enabled, the RGW daemons will also listen on that port. If running on SDN, the RGW daemon listening port will be 8080 internally.
 * `securePort`: The secure port on which RGW pods will be listening. A TLS certificate must be specified either via `sslCerticateRef` or `service.annotations`
 * `instances`: The number of pods that will be started to load balance this object store.

design/ceph/object/store.md

@@ -79,7 +79,15 @@ If there is a `zone` section in object-store configuration, then the pool sectio
 
 The gateway settings correspond to the RGW service.
 - `type`: Can be `s3`. In the future support for `swift` can be added.
-- `sslCertificateRef`: If specified, this is the name of the Kubernetes secret that contains the SSL certificate to be used for secure connections to the object store. The secret must be in the same namespace as the Rook cluster. Rook will look in the secret provided at the `cert` key name. The value of the `cert` key must be in the format expected by the [RGW service](https://docs.ceph.com/docs/master/install/ceph-deploy/install-ceph-gateway/#using-ssl-with-civetweb): "The server key, server certificate, and any other CA or intermediate certificates be supplied in one file. Each of these items must be in pem form." If the certificate is not specified, SSL will not be configured.
+- `sslCertificateRef`: If specified, this is the name of the Kubernetes secret that contains the SSL
+  certificate to be used for secure connections to the object store. The secret must be in the same
+  namespace as the Rook cluster. If it is an opaque Kubernetes Secret, Rook will look in the secret
+  provided at the `cert` key name. The value of the `cert` key must be in the format expected by the
+  [RGW
+  service](https://docs.ceph.com/docs/master/install/ceph-deploy/install-ceph-gateway/#using-ssl-with-civetweb):
+  "The server key, server certificate, and any other CA or intermediate certificates be supplied in
+  one file. Each of these items must be in pem form." If the certificate is not specified, SSL will
+  not be configured.
 - `caBundleRef`: If specified, this is the name of the Kubernetes secret (type `opaque`) that contains ca-bundle to use. The secret must be in the same namespace as the Rook cluster. Rook will look in the secret provided at the `cabundle` key name.
 - `port`: The service port where the RGW service will be listening (http)
 - `securePort`: The service port where the RGW service will be listening (https)

design/common/object-bucket.md

@@ -97,7 +97,19 @@ The pools are the backing data store for the object store and are created with s
 
 The gateway settings correspond to the RGW service.
 - `type`: Can be `s3`. In the future support for `swift` can be added.
-- `sslCertificateRef`: If specified, this is the name of the Kubernetes secret that contains the SSL certificate to be used for secure connections to the object store. The secret must be in the same namespace as the Rook cluster. Rook will look in the secret provided at the `cert` key name. The value of the `cert` key must be in the format expected by the [RGW service](https://docs.ceph.com/docs/master/install/ceph-deploy/install-ceph-gateway/#using-ssl-with-civetweb): "The server key, server certificate, and any other CA or intermediate certificates be supplied in one file. Each of these items must be in pem form." If the certificate is not specified, SSL will not be configured.
+- `sslCertificateRef`: If specified, this is the name of the Kubernetes secret that contains the SSL
+  certificate to be used for secure connections to the object store. The secret must be in the same
+  namespace as the Rook cluster. If it is an opaque Kubernetes Secret, Rook will look in the secret provided at the `cert` key name. The
+  value of the `cert` key must be in the format expected by the [RGW
+  service](https://docs.ceph.com/docs/master/install/ceph-deploy/install-ceph-gateway/#using-ssl-with-civetweb):
+  "The server key, server certificate, and any other CA or intermediate certificates be supplied in
+  one file. Each of these items must be in pem form." If the certificate is not specified, SSL will
+  not be configured. They are scenarios where the certificate DNS is set for a particular domain
+  that does not include the local Kubernetes DNS, namely the object store DNS service endpoint. If
+  adding the service DNS name to the certificate is not empty another key can be specified in the
+  secret's data: `insecureSkipVerify: true` to skip the certificate verification. It is not
+  recommended to enable this option since TLS is susceptible to machine-in-the-middle attacks unless
+  custom verification is used.
 - `port`: The service port where the RGW service will be listening (http)
 - `securePort`: The service port where the RGW service will be listening (https)
 - `instances`: The number of RGW pods that will be started for this object store (ignored if allNodes=true)

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

@@ -56,6 +56,7 @@ type Provisioner struct {
 	endpoint             string
 	additionalConfigData map[string]string
 	tlsCert              []byte
+	insecureTLS          bool
 	adminOpsClient       *admin.API
 }
 
@@ -607,7 +608,7 @@ func (p *Provisioner) setTlsCaCert() error {
 	}
 	p.tlsCert = make([]byte, 0)
 	if objStore.Spec.Gateway.SecurePort == p.storePort {
-		p.tlsCert, err = cephObject.GetTlsCaCert(p.objectContext, &objStore.Spec)
+		p.tlsCert, p.insecureTLS, err = cephObject.GetTlsCaCert(p.objectContext, &objStore.Spec)
 		if err != nil {
 			return err
 		}
@@ -622,8 +623,7 @@ func (p *Provisioner) setAdminOpsAPIClient() error {
 		Timeout: cephObject.HttpTimeOut,
 	}
 	if p.tlsCert != nil {
-		insecure := false
-		httpClient.Transport = cephObject.BuildTransportTLS(p.tlsCert, insecure)
+		httpClient.Transport = cephObject.BuildTransportTLS(p.tlsCert, p.insecureTLS)
 	}
 
 	// Fetch the ceph object store

pkg/operator/ceph/object/rgw.go

@@ -23,6 +23,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"reflect"
+	"strconv"
 	"syscall"
 
 	"github.com/banzaicloud/k8s-objectmatcher/patch"
@@ -62,6 +63,10 @@ type rgwConfig struct {
 
 var updateDeploymentAndWait = mon.UpdateCephDeploymentAndWait
 
+var (
+	insecureSkipVerify = "insecureSkipVerify"
+)
+
 func (c *clusterConfig) createOrUpdateStore(realmName, zoneGroupName, zoneName string) error {
 	logger.Infof("creating object store %q in namespace %q", c.store.Name, c.store.Namespace)
 
@@ -322,7 +327,8 @@ func BuildDNSEndpoint(domainName string, port int32, secure bool) string {
 }
 
 // GetTLSCACert fetch cacert for internal RGW requests
-func GetTlsCaCert(objContext *Context, objectStoreSpec *cephv1.ObjectStoreSpec) ([]byte, error) {
+func GetTlsCaCert(objContext *Context, objectStoreSpec *cephv1.ObjectStoreSpec) ([]byte, bool, error) {
+	var insecureTLS, ok bool
 	ctx := context.TODO()
 	var (
 		tlsCert []byte
@@ -332,21 +338,38 @@ func GetTlsCaCert(objContext *Context, objectStoreSpec *cephv1.ObjectStoreSpec)
 	if objectStoreSpec.Gateway.SSLCertificateRef != "" {
 		tlsSecretCert, err := objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Get(ctx, objectStoreSpec.Gateway.SSLCertificateRef, metav1.GetOptions{})
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to get secret %s containing TLS certificate defined in %s", objectStoreSpec.Gateway.SSLCertificateRef, objContext.Name)
+			return nil, false, errors.Wrapf(err, "failed to get secret %q containing TLS certificate defined in %q", objectStoreSpec.Gateway.SSLCertificateRef, objContext.Name)
 		}
 		if tlsSecretCert.Type == v1.SecretTypeOpaque {
-			tlsCert = tlsSecretCert.Data[certKeyName]
+			tlsCert, ok = tlsSecretCert.Data[certKeyName]
+			if !ok {
+				return nil, false, errors.Errorf("failed to get TLS certificate from secret, token is %q but key %q does not exist", v1.SecretTypeOpaque, certKeyName)
+			}
 		} else if tlsSecretCert.Type == v1.SecretTypeTLS {
-			tlsCert = tlsSecretCert.Data[v1.TLSCertKey]
+			tlsCert, ok = tlsSecretCert.Data[v1.TLSCertKey]
+			if !ok {
+				return nil, false, errors.Errorf("failed to get TLS certificate from secret, token is %q but key %q does not exist", v1.SecretTypeTLS, v1.TLSCertKey)
+			}
+		} else {
+			return nil, false, errors.Errorf("failed to get TLS certificate from secret, unknown secret type %q", tlsSecretCert.Type)
+		}
+		// If the secret contains an indication that the TLS connection should be insecure, then
+		// let's apply it to the client.
+		insecureTLSStr, ok := tlsSecretCert.Data[insecureSkipVerify]
+		if ok {
+			insecureTLS, err = strconv.ParseBool(string(insecureTLSStr))
+			if err != nil {
+				return nil, false, errors.Wrap(err, "failed to parse insecure tls bool option")
+			}
 		}
 	} else if objectStoreSpec.GetServiceServingCert() != "" {
 		tlsCert, err = ioutil.ReadFile(ServiceServingCertCAFile)
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to fetch TLS certificate from %q", ServiceServingCertCAFile)
+			return nil, false, errors.Wrapf(err, "failed to fetch TLS certificate from %q", ServiceServingCertCAFile)
 		}
 	}
 
-	return tlsCert, nil
+	return tlsCert, insecureTLS, nil
 }
 
 // Allow overriding this function for unit tests to mock the admin ops api
@@ -358,12 +381,11 @@ func genObjectStoreHTTPClient(objContext *Context, spec *cephv1.ObjectStoreSpec)
 	tlsCert := []byte{}
 	if spec.IsTLSEnabled() {
 		var err error
-		tlsCert, err = GetTlsCaCert(objContext, spec)
+		tlsCert, insecureTLS, err := GetTlsCaCert(objContext, spec)
 		if err != nil {
 			return nil, tlsCert, errors.Wrapf(err, "failed to fetch CA cert to establish TLS connection with object store %q", nsName)
 		}
-		insecure := false
-		c.Transport = BuildTransportTLS(tlsCert, insecure)
+		c.Transport = BuildTransportTLS(tlsCert, insecureTLS)
 	}
 	return c, tlsCert, nil
 }

pkg/operator/ceph/object/rgw_test.go

@@ -25,14 +25,14 @@ import (
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
 	"github.com/rook/rook/pkg/clusterd"
-
-	cephclient "github.com/rook/rook/pkg/daemon/ceph/client"
+	"github.com/rook/rook/pkg/daemon/ceph/client"
 	clienttest "github.com/rook/rook/pkg/daemon/ceph/client/test"
 	"github.com/rook/rook/pkg/operator/ceph/config"
 	"github.com/rook/rook/pkg/operator/k8sutil"
-	testop "github.com/rook/rook/pkg/operator/test"
+	"github.com/rook/rook/pkg/operator/test"
 	exectest "github.com/rook/rook/pkg/util/exec/test"
 	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	fclient "k8s.io/client-go/kubernetes/fake"
@@ -41,7 +41,7 @@ import (
 
 func TestStartRGW(t *testing.T) {
 	ctx := context.TODO()
-	clientset := testop.New(t, 3)
+	clientset := test.New(t, 3)
 	executor := &exectest.MockExecutor{
 		MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
 			if args[0] == "auth" && args[1] == "get-or-create-key" {
@@ -66,7 +66,7 @@ func TestStartRGW(t *testing.T) {
 	r := &ReconcileCephObjectStore{client: cl, scheme: s}
 
 	// start a basic cluster
-	ownerInfo := cephclient.NewMinimumOwnerInfoWithOwnerRef()
+	ownerInfo := client.NewMinimumOwnerInfoWithOwnerRef()
 	c := &clusterConfig{context, info, store, version, &cephv1.ClusterSpec{}, ownerInfo, data, r.client}
 	err := c.startRGWPods(store.Name, store.Name, store.Name)
 	assert.Nil(t, err)
@@ -102,7 +102,7 @@ func TestCreateObjectStore(t *testing.T) {
 	}
 
 	store := simpleStore()
-	clientset := testop.New(t, 3)
+	clientset := test.New(t, 3)
 	context := &clusterd.Context{Executor: executor, Clientset: clientset}
 	info := clienttest.CreateTestClusterInfo(1)
 	data := config.NewStatelessDaemonDataPathMap(config.RgwType, "my-fs", "rook-ceph", "/var/lib/rook/")
@@ -112,7 +112,7 @@ func TestCreateObjectStore(t *testing.T) {
 	object := []runtime.Object{&cephv1.CephObjectStore{}}
 	cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(object...).Build()
 	r := &ReconcileCephObjectStore{client: cl, scheme: s}
-	ownerInfo := cephclient.NewMinimumOwnerInfoWithOwnerRef()
+	ownerInfo := client.NewMinimumOwnerInfoWithOwnerRef()
 	c := &clusterConfig{context, info, store, "1.2.3.4", &cephv1.ClusterSpec{}, ownerInfo, data, r.client}
 	err := c.createOrUpdateStore(store.Name, store.Name, store.Name)
 	assert.Nil(t, err)
@@ -134,7 +134,7 @@ func TestGenerateSecretName(t *testing.T) {
 
 	// start a basic cluster
 	c := &clusterConfig{&clusterd.Context{},
-		&cephclient.ClusterInfo{},
+		&client.ClusterInfo{},
 		&cephv1.CephObjectStore{ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "mycluster"}},
 		"v1.1.0",
 		&cephv1.ClusterSpec{},
@@ -174,3 +174,96 @@ func TestBuildDomainNameAndEndpoint(t *testing.T) {
 	ep = BuildDNSEndpoint(dns, securePort, true)
 	assert.Equal(t, "https://rook-ceph-rgw-my-store.rook-ceph.svc:443", ep)
 }
+
+func TestGetTlsCaCert(t *testing.T) {
+	objContext := &Context{
+		Context: &clusterd.Context{
+			Clientset: test.New(t, 3),
+		},
+		clusterInfo: client.AdminClusterInfo("rook-ceph"),
+	}
+	objectStore := simpleStore()
+
+	t.Run("no gateway cert ref", func(t *testing.T) {
+		tlsCert, insesure, err := GetTlsCaCert(objContext, &objectStore.Spec)
+		assert.NoError(t, err)
+		assert.False(t, insesure)
+		assert.Nil(t, tlsCert)
+	})
+
+	t.Run("gateway cert ref but secret no found", func(t *testing.T) {
+		objectStore.Spec.Gateway.SSLCertificateRef = "my-secret"
+		tlsCert, insesure, err := GetTlsCaCert(objContext, &objectStore.Spec)
+		assert.Error(t, err)
+		assert.False(t, insesure)
+		assert.Nil(t, tlsCert)
+	})
+
+	t.Run("gateway cert ref and secret found but no key and wrong type", func(t *testing.T) {
+		s := &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret",
+				Namespace: "rook-ceph",
+			},
+			Type: "Yolo",
+		}
+		_, err := objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Create(context.TODO(), s, metav1.CreateOptions{})
+		assert.NoError(t, err)
+		objectStore.Spec.Gateway.SSLCertificateRef = "my-secret"
+		tlsCert, insesure, err := GetTlsCaCert(objContext, &objectStore.Spec)
+		assert.Error(t, err)
+		assert.EqualError(t, err, "failed to get TLS certificate from secret, unknown secret type \"Yolo\"")
+		assert.False(t, insesure)
+		assert.Nil(t, tlsCert)
+		err = objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Delete(context.TODO(), s.Name, metav1.DeleteOptions{})
+		assert.NoError(t, err)
+	})
+
+	t.Run("gateway cert ref and Opaque secret found and no key is present", func(t *testing.T) {
+		s := &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret",
+				Namespace: "rook-ceph",
+			},
+			Type: "Opaque",
+		}
+		_, err := objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Create(context.TODO(), s, metav1.CreateOptions{})
+		assert.NoError(t, err)
+		objectStore.Spec.Gateway.SSLCertificateRef = "my-secret"
+		tlsCert, insesure, err := GetTlsCaCert(objContext, &objectStore.Spec)
+		assert.Error(t, err)
+		assert.EqualError(t, err, "failed to get TLS certificate from secret, token is \"Opaque\" but key \"cert\" does not exist")
+		assert.False(t, insesure)
+		assert.Nil(t, tlsCert)
+		err = objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Delete(context.TODO(), s.Name, metav1.DeleteOptions{})
+		assert.NoError(t, err)
+	})
+
+	t.Run("gateway cert ref and Opaque secret found and key is present", func(t *testing.T) {
+		s := &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret",
+				Namespace: "rook-ceph",
+			},
+			Data: map[string][]byte{"cert": []byte(`-----BEGIN CERTIFICATE-----
+MIIBJTCB0AIJAPNFNz1CNlDOMA0GCSqGSIb3DQEBCwUAMBoxCzAJBgNVBAYTAkZS
+MQswCQYDVQQIDAJGUjAeFw0yMTA5MzAwODAzNDBaFw0yNDA2MjYwODAzNDBaMBox
+CzAJBgNVBAYTAkZSMQswCQYDVQQIDAJGUjBcMA0GCSqGSIb3DQEBAQUAA0sAMEgC
+QQDHeZ47hVBcryl6SCghM8Zj3Q6DQzJzno1J7EjPXef5m+pIVAEylS9sQuwKtFZc
+vv3qS/OVFExmMdbrvfKEIfbBAgMBAAEwDQYJKoZIhvcNAQELBQADQQAAnflLuUM3
+4Dq0v7If4cgae2mr7jj3U/lIpHVtFbF7kVjC/eqmeN1a9u0UbRHKkUr+X1mVX3rJ
+BvjQDN6didwQ
+-----END CERTIFICATE-----`)},
+			Type: "Opaque",
+		}
+		_, err := objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Create(context.TODO(), s, metav1.CreateOptions{})
+		assert.NoError(t, err)
+		objectStore.Spec.Gateway.SSLCertificateRef = "my-secret"
+		tlsCert, insesure, err := GetTlsCaCert(objContext, &objectStore.Spec)
+		assert.NoError(t, err)
+		assert.False(t, insesure)
+		assert.NotNil(t, tlsCert)
+		err = objContext.Context.Clientset.CoreV1().Secrets(objContext.clusterInfo.Namespace).Delete(context.TODO(), s.Name, metav1.DeleteOptions{})
+		assert.NoError(t, err)
+	})
+}