Issue 479: Addressing Security vulnerabilities with authentication (#488

) * initial commit Signed-off-by: prabhaker24 <[email protected]> * added support for adding token as secret Signed-off-by: prabhaker24 <[email protected]> * added changes for exposing secret as files added Signed-off-by: prabhaker24 <[email protected]> * doc and ut added Signed-off-by: prabhaker24 <[email protected]> * fixed typo Signed-off-by: prabhaker24 <[email protected]> * addressed comments Signed-off-by: prabhaker24 <[email protected]> * Update pkg/controller/pravega/pravega_controller_test.go Co-authored-by: Christophe Balczunas <[email protected]> Co-authored-by: prabhaker24 <[email protected]> Co-authored-by: Christophe Balczunas <[email protected]>
pravega · Jan 28, 2021 · 9d15bf1 · 9d15bf1
1 parent 5dce68d
commit 9d15bf1
Show file tree

Hide file tree

Showing 14 changed files with 169 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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:       <none>
+Annotations:  <none>
+
+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:       <none>
+Annotations:  <none>
+
+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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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