From 9d15bf13be4a8beee34a69dd2a8dba5814cf2abf Mon Sep 17 00:00:00 2001 From: Prabhaker24 <54395361+Prabhaker24@users.noreply.github.com> Date: Thu, 28 Jan 2021 09:38:36 +0530 Subject: [PATCH] Issue 479: Addressing Security vulnerabilities with authentication (#488) * initial commit Signed-off-by: prabhaker24 * added support for adding token as secret Signed-off-by: prabhaker24 * added changes for exposing secret as files added Signed-off-by: prabhaker24 * doc and ut added Signed-off-by: prabhaker24 * fixed typo Signed-off-by: prabhaker24 * addressed comments Signed-off-by: prabhaker24 * Update pkg/controller/pravega/pravega_controller_test.go Co-authored-by: Christophe Balczunas Co-authored-by: prabhaker24 Co-authored-by: Christophe Balczunas --- ...ravega.pravega.io_pravegaclusters_crd.yaml | 4 + charts/pravega/README.md | 2 + charts/pravega/templates/pravega.yaml | 3 + charts/pravega/values.yaml | 4 + ...ravega.pravega.io_pravegaclusters_crd.yaml | 4 + doc/auth.md | 79 +++++++++++++++++++ .../pravega/v1beta1/pravegacluster_types.go | 6 ++ pkg/controller/pravega/constants.go | 34 ++++---- pkg/controller/pravega/pravega_controller.go | 8 ++ .../pravega/pravega_controller_test.go | 10 ++- .../pravega/pravega_segmentstore.go | 21 +++++ .../pravega/pravega_segmentstore_test.go | 5 +- test/e2e/resources/crd.yaml | 6 ++ tools/manifest_files/crd.yaml | 4 + 14 files changed, 169 insertions(+), 21 deletions(-) diff --git a/charts/pravega-operator/templates/pravega.pravega.io_pravegaclusters_crd.yaml b/charts/pravega-operator/templates/pravega.pravega.io_pravegaclusters_crd.yaml index 71f1e2655..e3bf62429 100644 --- a/charts/pravega-operator/templates/pravega.pravega.io_pravegaclusters_crd.yaml +++ b/charts/pravega-operator/templates/pravega.pravega.io_pravegaclusters_crd.yaml @@ -74,6 +74,8 @@ spec: from clients to controller and segment store See the following file for a complete list of options: https://github.com/pravega/pravega/blob/master/documentation/src/docs/security/pravega-security-configurations.md' properties: + controllerTokenSecret: + type: string enabled: description: Enabled specifies whether or not authentication is enabled By default, authentication is not enabled @@ -83,6 +85,8 @@ spec: Parameters like username, password and acl optional - used only by PasswordAuthHandler for authentication type: string + segmentStoreTokenSecret: + type: string type: object bookkeeperUri: description: BookkeeperUri specifies the hostname/IP address and port diff --git a/charts/pravega/README.md b/charts/pravega/README.md index a3187082f..f3cebab88 100644 --- a/charts/pravega/README.md +++ b/charts/pravega/README.md @@ -63,6 +63,8 @@ The following table lists the configurable parameters of the pravega chart and t | `tls` | Pravega security configuration passed to the Pravega processes | `{}` | | `authentication.enabled` | Enable authentication to authorize client communication with Pravega | `false` | | `authentication.passwordAuthSecret` | Name of Secret containing Password based Authentication Parameters, if authentication is enabled | | +| `authentication.segmentStoreTokenSecret` | Name of Secret containing tokenSigningkey for the ss, if authentication is enabled | | +| `authentication.controllerTokenSecret` | Name of Secret containing tokenSigningkey for controller, if authentication is enabled | | | `zookeeperUri` | Zookeeper client service URI | `zookeeper-client:2181` | | `bookkeeperUri` | Bookkeeper headless service URI | `bookkeeper-bookie-headless:3181` | | `externalAccess.enabled` | Enable external access | `false` | diff --git a/charts/pravega/templates/pravega.yaml b/charts/pravega/templates/pravega.yaml index ef9834189..79e025120 100644 --- a/charts/pravega/templates/pravega.yaml +++ b/charts/pravega/templates/pravega.yaml @@ -24,7 +24,10 @@ spec: enabled: {{ .Values.authentication.enabled }} {{- if .Values.authentication.enabled }} passwordAuthSecret: {{ .Values.authentication.passwordAuthSecret }} + segmentStoreTokenSecret: {{ .Values.authentication.segmentStoreTokenSecret }} + controllerTokenSecret: {{ .Values.authentication.controllerTokenSecret }} {{- end }} + version: {{ .Values.version }} zookeeperUri: {{ .Values.zookeeperUri }} bookkeeperUri: {{ .Values.bookkeeperUri }} diff --git a/charts/pravega/values.yaml b/charts/pravega/values.yaml index aabf5a59c..9b88b17c8 100644 --- a/charts/pravega/values.yaml +++ b/charts/pravega/values.yaml @@ -14,6 +14,10 @@ authentication: enabled: false ## passwordAuthSecret is ignored if authentication is disabled passwordAuthSecret: + #segmentStoreToken is ignored if authentication is disabled + segmentStoreTokenSecret: + #controllerTokenSecret is ignored if authentication is disabled + controllerTokenSecret: zookeeperUri: zookeeper-client:2181 bookkeeperUri: bookkeeper-bookie-headless:3181 diff --git a/deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml b/deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml index 72123bb8e..90217e883 100644 --- a/deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml +++ b/deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml @@ -62,6 +62,8 @@ spec: from clients to controller and segment store See the following file for a complete list of options: https://github.com/pravega/pravega/blob/master/documentation/src/docs/security/pravega-security-configurations.md' properties: + controllerTokenSecret: + type: string enabled: description: Enabled specifies whether or not authentication is enabled By default, authentication is not enabled @@ -71,6 +73,8 @@ spec: Parameters like username, password and acl optional - used only by PasswordAuthHandler for authentication type: string + segmentStoreTokenSecret: + type: string type: object bookkeeperUri: description: BookkeeperUri specifies the hostname/IP address and port diff --git a/doc/auth.md b/doc/auth.md index 462c8bbe6..d37b28cb8 100644 --- a/doc/auth.md +++ b/doc/auth.md @@ -76,3 +76,82 @@ where username and password are credentials you intend to use. Note that Pravega operator uses `/etc/auth-passwd-volume` as the mounting directory for secrets. For more security configurations, please check [here](https://github.com/pravega/pravega/blob/master/documentation/src/docs/security/pravega-security-configurations.md). + +Pravega Operator Supports Passing of Auth Parametes as Secret which are mounted as file in both Segment Store and Controller (Operator mounts these secrets in Segementstore pod it's mounted at `/etc/ss-auth-volume` and in Controller pod at `/etc/controller-auth-volume` respectively) + +Note that Pravega has to use this feature and start Picking below specified values from file insted of jvm properties which it currently does. +(This is not implemented at pravega end currently) + +Below is how we can create secret and expose them as file for Auth related properties:- + +1. Create a File containg `controller.security.auth.delegationToken.signingKey.basis` as `delegationToken.signingKey.basis` which represent the tokensigning key used to connect to the controller: + +``` +$ cat controllerauthdata.txt +delegationToken.signingKey.basis: "secret" +``` + +2. Create a kubernetes secret with this file: + +``` +$ kubectl create secret generic controllertokensecret \ + --from-file=./controllerauthdata.txt \ +``` + +Ensure Secret is created:- + +``` +$ kubectl describe secret controllertokensecret +Name: controllertokensecret +Namespace: default +Labels: +Annotations: + +Type: Opaque + +Data +==== +controllerauthdata.txt: 67 bytes + +``` + +3. Create a File containg `autoScale.security.auth.token.signingKey.basis` as `delegationToken.signingKey.basis` which represent the tokensigning key used to connect to the Segmentstore along with other 3 values `pravega.client.auth.method`, `pravega.client.auth.token` and `controller.connect.auth.credentials.dynamic` as `controller.connect.auth.params` which contains all the 3 values in the same order seprated by semicolon: + +``` +$ cat segmentstoreauthdata.txt +delegationToken.signingKey.basis: "secret" +controller.connect.auth.params: {method};{token};{dynamic} +``` +4. Create a kubernetes secret with this file: + +``` +$ kubectl create secret generic sstokensecret \ + --from-file=./segmentstoreauthdata.txt \ +``` +Ensure Secret is created:- + +``` +$ kubectl describe secret sstokensecret +Name: sstokensecret +Namespace: default +Labels: +Annotations: + +Type: Opaque + +Data +==== +segmentstoreauthdata.txt: 106 bytes + +``` + +5. Use these secrets instead of specifying the values in the option:- + +``` +spec: + authentication: + enabled: true + passwordAuthSecret: password-auth + segmentStoreTokenSecret: sstokensecret + controllerTokenSecret: controllertokensecret +``` diff --git a/pkg/apis/pravega/v1beta1/pravegacluster_types.go b/pkg/apis/pravega/v1beta1/pravegacluster_types.go index 9cc35f732..46fb7947c 100644 --- a/pkg/apis/pravega/v1beta1/pravegacluster_types.go +++ b/pkg/apis/pravega/v1beta1/pravegacluster_types.go @@ -290,6 +290,12 @@ type AuthenticationParameters struct { // name of Secret containing Password based Authentication Parameters like username, password and acl // optional - used only by PasswordAuthHandler for authentication PasswordAuthSecret string `json:"passwordAuthSecret,omitempty"` + + //name of secret containg TokenSigningKey + ControllerTokenSecret string `json:"controllerTokenSecret,omitempty"` + + //name of secret containg TokenSigningKey and AuthToken + SegmentStoreTokenSecret string `json:"segmentStoreTokenSecret,omitempty"` } func (ap *AuthenticationParameters) IsEnabled() bool { diff --git a/pkg/controller/pravega/constants.go b/pkg/controller/pravega/constants.go index 50887810b..f5216da85 100644 --- a/pkg/controller/pravega/constants.go +++ b/pkg/controller/pravega/constants.go @@ -11,19 +11,23 @@ package pravega const ( - cacheVolumeName = "cache" - cacheVolumeMountPoint = "/tmp/pravega/cache" - ltsFileMountPoint = "/mnt/tier2" - ltsVolumeName = "tier2" - segmentStoreKind = "pravega-segmentstore" - ssSecretVolumeName = "ss-secret" - tlsVolumeName = "tls-secret" - tlsMountDir = "/etc/secret-volume" - caBundleVolumeName = "ca-bundle" - caBundleMountDir = "/etc/secret-volume/ca-bundle" - heapDumpName = "heap-dump" - heapDumpDir = "/tmp/dumpfile/heap" - authVolumeName = "auth-passwd-secret" - authMountDir = "/etc/auth-passwd-volume" - defaultTokenSigningKey = "secret" + cacheVolumeName = "cache" + cacheVolumeMountPoint = "/tmp/pravega/cache" + ltsFileMountPoint = "/mnt/tier2" + ltsVolumeName = "tier2" + segmentStoreKind = "pravega-segmentstore" + ssSecretVolumeName = "ss-secret" + tlsVolumeName = "tls-secret" + tlsMountDir = "/etc/secret-volume" + caBundleVolumeName = "ca-bundle" + caBundleMountDir = "/etc/secret-volume/ca-bundle" + heapDumpName = "heap-dump" + heapDumpDir = "/tmp/dumpfile/heap" + authVolumeName = "auth-passwd-secret" + authMountDir = "/etc/auth-passwd-volume" + defaultTokenSigningKey = "secret" + controllerAuthVolumeName = "controller-auth-secret" + ssAuthVolumeName = "ss-auth-secret" + controllerAuthMountDir = "/etc/controller-auth-volume" + ssAuthMountDir = "/etc/ss-auth-volume" ) diff --git a/pkg/controller/pravega/pravega_controller.go b/pkg/controller/pravega/pravega_controller.go index e4251ac78..5a2b824ed 100644 --- a/pkg/controller/pravega/pravega_controller.go +++ b/pkg/controller/pravega/pravega_controller.go @@ -142,6 +142,7 @@ func makeControllerPodSpec(p *api.PravegaCluster) *corev1.PodSpec { configureControllerTLSSecrets(podSpec, p) configureAuthSecrets(podSpec, p) + configureControllerAuthSecrets(podSpec, p) return podSpec } @@ -158,6 +159,13 @@ func configureAuthSecrets(podSpec *corev1.PodSpec, p *api.PravegaCluster) { } } +func configureControllerAuthSecrets(podSpec *corev1.PodSpec, p *api.PravegaCluster) { + if p.Spec.Authentication.IsEnabled() && p.Spec.Authentication.ControllerTokenSecret != "" { + addSecretVolumeWithMount(podSpec, p, controllerAuthVolumeName, p.Spec.Authentication.ControllerTokenSecret, + controllerAuthVolumeName, controllerAuthMountDir) + } +} + func addSecretVolumeWithMount(podSpec *corev1.PodSpec, p *api.PravegaCluster, volumeName string, secretName string, mountName string, mountDir string) { diff --git a/pkg/controller/pravega/pravega_controller_test.go b/pkg/controller/pravega/pravega_controller_test.go index 1031629bf..8707d22bf 100644 --- a/pkg/controller/pravega/pravega_controller_test.go +++ b/pkg/controller/pravega/pravega_controller_test.go @@ -103,8 +103,9 @@ var _ = Describe("Controller", func() { }, }, Authentication: &v1beta1.AuthenticationParameters{ - Enabled: true, - PasswordAuthSecret: "authentication-secret", + Enabled: true, + PasswordAuthSecret: "authentication-secret", + ControllerTokenSecret: "controllerauth-secret", }, } p.WithDefaults() @@ -219,8 +220,9 @@ var _ = Describe("Controller", func() { }, }, Authentication: &v1beta1.AuthenticationParameters{ - Enabled: true, - PasswordAuthSecret: "authentication-secret", + Enabled: true, + PasswordAuthSecret: "authentication-secret", + ControllerTokenSecret: "controllerauth-secret", }, } p.WithDefaults() diff --git a/pkg/controller/pravega/pravega_segmentstore.go b/pkg/controller/pravega/pravega_segmentstore.go index 11d525e1c..aeda24877 100644 --- a/pkg/controller/pravega/pravega_segmentstore.go +++ b/pkg/controller/pravega/pravega_segmentstore.go @@ -178,6 +178,8 @@ func makeSegmentstorePodSpec(p *api.PravegaCluster) corev1.PodSpec { configureLTSFilesystem(&podSpec, p.Spec.Pravega) + configureSegmentstoreAuthSecret(&podSpec, p) + return podSpec } @@ -378,6 +380,25 @@ func configureSegmentstoreTLSSecret(podSpec *corev1.PodSpec, p *api.PravegaClust } } +func configureSegmentstoreAuthSecret(podSpec *corev1.PodSpec, p *api.PravegaCluster) { + if p.Spec.Authentication.SegmentStoreTokenSecret != "" { + vol := corev1.Volume{ + Name: ssAuthVolumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: p.Spec.Authentication.SegmentStoreTokenSecret, + }, + }, + } + podSpec.Volumes = append(podSpec.Volumes, vol) + + podSpec.Containers[0].VolumeMounts = append(podSpec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: ssAuthVolumeName, + MountPath: ssAuthMountDir, + }) + } +} + func configureCaBundleSecret(podSpec *corev1.PodSpec, p *api.PravegaCluster) { if p.Spec.TLS.IsCaBundlePresent() { vol := corev1.Volume{ diff --git a/pkg/controller/pravega/pravega_segmentstore_test.go b/pkg/controller/pravega/pravega_segmentstore_test.go index 6c2fcc508..768aae5da 100644 --- a/pkg/controller/pravega/pravega_segmentstore_test.go +++ b/pkg/controller/pravega/pravega_segmentstore_test.go @@ -234,8 +234,9 @@ var _ = Describe("PravegaSegmentstore", func() { }, }, Authentication: &v1beta1.AuthenticationParameters{ - Enabled: true, - PasswordAuthSecret: "authentication-secret", + Enabled: true, + PasswordAuthSecret: "authentication-secret", + SegmentStoreTokenSecret: "authentication-secret", }, } p.WithDefaults() diff --git a/test/e2e/resources/crd.yaml b/test/e2e/resources/crd.yaml index 72123bb8e..30212f186 100644 --- a/test/e2e/resources/crd.yaml +++ b/test/e2e/resources/crd.yaml @@ -62,6 +62,9 @@ spec: from clients to controller and segment store See the following file for a complete list of options: https://github.com/pravega/pravega/blob/master/documentation/src/docs/security/pravega-security-configurations.md' properties: + controllerTokenSecret: + description: name of secret containg TokenSigningKey + type: string enabled: description: Enabled specifies whether or not authentication is enabled By default, authentication is not enabled @@ -71,6 +74,9 @@ spec: Parameters like username, password and acl optional - used only by PasswordAuthHandler for authentication type: string + segmentStoreTokenSecret: + description: name of secret containg TokenSigningKey and AuthToken + type: string type: object bookkeeperUri: description: BookkeeperUri specifies the hostname/IP address and port diff --git a/tools/manifest_files/crd.yaml b/tools/manifest_files/crd.yaml index 1c5037f05..5c2ca5b55 100644 --- a/tools/manifest_files/crd.yaml +++ b/tools/manifest_files/crd.yaml @@ -71,6 +71,8 @@ spec: from clients to controller and segment store See the following file for a complete list of options: https://github.com/pravega/pravega/blob/master/documentation/src/docs/security/pravega-security-configurations.md' properties: + controllerTokenSecret: + type: string enabled: description: Enabled specifies whether or not authentication is enabled By default, authentication is not enabled @@ -80,6 +82,8 @@ spec: Parameters like username, password and acl optional - used only by PasswordAuthHandler for authentication type: string + segmentStoreTokenSecret: + type: string type: object bookkeeperUri: description: BookkeeperUri specifies the hostname/IP address and port