[security fix] Refactor intree security model #1509
Conversation
lpabon
force-pushed
the
intree-multitenant-2
branch
7 times, most recently
from
fa30066
to
486d00a
Compare
csi/node.go
Outdated
| @@ -161,6 +162,7 @@ func (s *OsdCsiServer) NodePublishVolume( | |||
| } | |||
| return nil, err | |||
| } | |||
| logrus.Infof("csi: Called mount for volume %s", volumeId) | |||
There was a problem hiding this comment.
Did you mean to leave these two log lines? this one seems repetitive with L167
There was a problem hiding this comment.
Doh! this is a debugging line. I will remove it.
| } | ||
|
|
||
| // Allow ${pvc.namespace} to be set in the storage class | ||
| namespaceParams := map[string]string{"pvc.namespace": pvc.GetNamespace()} |
There was a problem hiding this comment.
shouldn't this be done only when len(secretNamespaceValue) > 0 ?
There was a problem hiding this comment.
The ResolveTemplate function is only using this as an input to the template parser. In other words, the result will be the same if secretNamespaceValue is "" since there is nothing to replace.
|
|
||
| // Get secret name | ||
| nameParams := make(map[string]string) | ||
| // Allow ${pvc.annotations['pvcNameKey']} to be set in the storage class |
There was a problem hiding this comment.
What will the annotation look like in this case ?
There was a problem hiding this comment.
Like this: https://kubernetes-csi.github.io/docs/secrets-and-credentials-storage-class.html#per-volume-secrets
I'll add it to the comment
cmd/osd/main.go
Outdated
| @@ -183,6 +183,11 @@ func main() { | |||
| Usage: "CSI Driver name", | |||
| Value: "", | |||
| }, | |||
| cli.StringFlag{ | |||
| Name: "secrets-type", | |||
| Usage: "Secrets manager type: k8s", | |||
There was a problem hiding this comment.
nit: it could be any secrets manager and not just k8s
There was a problem hiding this comment.
True, I will add more information
* Needed to include new updated portworx/sched-ops change Signed-off-by: Luis Pabón <luis@portworx.com>
This test framework allows testing of openstorage as a provisioner for the Portworx intree Kubernetes driver. With this model, we can automate test openstorage authentication model on Kubernetes Signed-off-by: Luis Pabón <luis@portworx.com>
This change fixes a security issue where the pvc user could maliciously use the secret located in a namespace they cannot access. Instead this patch changes the model to the CSI model which is based on using the StorageClass as the source of truth on the location of the sercret which contains the authentication token. Signed-off-by: Luis Pabón <luis@portworx.com>
lpabon
force-pushed
the
intree-multitenant-2
branch
from
486d00a
to
5ba9691
Compare
What this PR does / why we need it:
From the commit:
Support for multitenancy in intree Kubernetes
This change makes it possible to use the same style as CSI where the location of the secret and its name are set in the storage class and not in the PVC.
Sample StorageClasses are under
test/setup/assets/Also:
BATS KinD test framework
Reviewers
I have broken the PR into three commits: