From ede4c706204ee25e2a9a67de6a8e755637e83f4e Mon Sep 17 00:00:00 2001 From: Anish Ramasekar Date: Wed, 27 Jul 2022 18:36:53 +0000 Subject: [PATCH] fix: sanitize service account tokens in logs Signed-off-by: Anish Ramasekar --- pkg/rotation/reconciler.go | 18 ++++-------- pkg/rotation/reconciler_test.go | 2 +- pkg/secrets-store/nodeserver.go | 31 ++++++++++++-------- pkg/secrets-store/nodeserver_test.go | 30 +++++++++---------- pkg/secrets-store/server.go | 23 ++++++++++++++- pkg/secrets-store/server_test.go | 44 ++++++++++++++++++++++++++++ 6 files changed, 107 insertions(+), 41 deletions(-) diff --git a/pkg/rotation/reconciler.go b/pkg/rotation/reconciler.go index 182405551..a71d03801 100644 --- a/pkg/rotation/reconciler.go +++ b/pkg/rotation/reconciler.go @@ -55,18 +55,12 @@ import ( ) const ( - permission os.FileMode = 0644 - maxNumOfRequeues int = 5 + maxNumOfRequeues int = 5 mountRotationFailedReason = "MountRotationFailed" mountRotationCompleteReason = "MountRotationComplete" k8sSecretRotationFailedReason = "SecretRotationFailed" k8sSecretRotationCompleteReason = "SecretRotationComplete" - - csipodname = "csi.storage.k8s.io/pod.name" - csipodnamespace = "csi.storage.k8s.io/pod.namespace" - csipoduid = "csi.storage.k8s.io/pod.uid" - csipodsa = "csi.storage.k8s.io/serviceAccount.name" ) // Reconciler reconciles and rotates contents in the pod @@ -313,10 +307,10 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret parameters = spc.Spec.Parameters } // Set these parameters to mimic the exact same attributes we get as part of NodePublishVolumeRequest - parameters[csipodname] = pod.Name - parameters[csipodnamespace] = pod.Namespace - parameters[csipoduid] = string(pod.UID) - parameters[csipodsa] = pod.Spec.ServiceAccountName + parameters[secretsstore.CSIPodName] = pod.Name + parameters[secretsstore.CSIPodNamespace] = pod.Namespace + parameters[secretsstore.CSIPodUID] = string(pod.UID) + parameters[secretsstore.CSIPodServiceAccountName] = pod.Spec.ServiceAccountName // csi.storage.k8s.io/serviceAccount.tokens is empty for Kubernetes version < 1.20. // For 1.20+, if tokenRequests is set in the CSI driver spec, kubelet will generate // a token for the pod and send it to the CSI driver. @@ -336,7 +330,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret if err != nil { return fmt.Errorf("failed to marshal parameters, err: %w", err) } - permissionJSON, err := json.Marshal(permission) + permissionJSON, err := json.Marshal(secretsstore.FilePermission) if err != nil { return fmt.Errorf("failed to marshal permission, err: %w", err) } diff --git a/pkg/rotation/reconciler_test.go b/pkg/rotation/reconciler_test.go index 916312a05..1c5095921 100644 --- a/pkg/rotation/reconciler_test.go +++ b/pkg/rotation/reconciler_test.go @@ -604,7 +604,7 @@ func TestReconcileNoError(t *testing.T) { err = server.Start() g.Expect(err).NotTo(HaveOccurred()) - err = os.WriteFile(secretProviderClassPodStatusToProcess.Status.TargetPath+"/object1", []byte("newdata"), permission) + err = os.WriteFile(secretProviderClassPodStatusToProcess.Status.TargetPath+"/object1", []byte("newdata"), secretsstore.FilePermission) g.Expect(err).NotTo(HaveOccurred()) err = testReconciler.reconcile(context.TODO(), secretProviderClassPodStatusToProcess) diff --git a/pkg/secrets-store/nodeserver.go b/pkg/secrets-store/nodeserver.go index 2bbb53c16..407384787 100644 --- a/pkg/secrets-store/nodeserver.go +++ b/pkg/secrets-store/nodeserver.go @@ -52,13 +52,20 @@ type nodeServer struct { } const ( - permission os.FileMode = 0644 + // FilePermission is the permission to be used for the staging target path + FilePermission os.FileMode = 0644 + + // CSIPodName is the name of the pod that the mount is created for + CSIPodName = "csi.storage.k8s.io/pod.name" + // CSIPodNamespace is the namespace of the pod that the mount is created for + CSIPodNamespace = "csi.storage.k8s.io/pod.namespace" + // CSIPodUID is the UID of the pod that the mount is created for + CSIPodUID = "csi.storage.k8s.io/pod.uid" + // CSIPodServiceAccountName is the name of the pod service account that the mount is created for + CSIPodServiceAccountName = "csi.storage.k8s.io/serviceAccount.name" + // CSIPodServiceAccountTokens is the service account tokens of the pod that the mount is created for + CSIPodServiceAccountTokens = "csi.storage.k8s.io/serviceAccount.tokens" //nolint - csipodname = "csi.storage.k8s.io/pod.name" - csipodnamespace = "csi.storage.k8s.io/pod.namespace" - csipoduid = "csi.storage.k8s.io/pod.uid" - csipodsa = "csi.storage.k8s.io/serviceAccount.name" - csipodsatokens = "csi.storage.k8s.io/serviceAccount.tokens" //nolint secretProviderClassField = "secretProviderClass" ) @@ -111,10 +118,10 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis secretProviderClass := attrib[secretProviderClassField] providerName = attrib["providerName"] - podName = attrib[csipodname] - podNamespace = attrib[csipodnamespace] - podUID = attrib[csipoduid] - serviceAccountName = attrib[csipodsa] + podName = attrib[CSIPodName] + podNamespace = attrib[CSIPodNamespace] + podUID = attrib[CSIPodUID] + serviceAccountName = attrib[CSIPodServiceAccountName] mounted, err = ns.ensureMountPoint(targetPath) if err != nil { @@ -182,7 +189,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis // to provider irrespective of the Kubernetes version. If the token doesn't exist in the // volume request context, the CSI driver will generate the token for the configured audience // and send it to the provider in the parameters. - if parameters[csipodsatokens] == "" { + if parameters[CSIPodServiceAccountTokens] == "" { // Inject pod service account token into volume attributes serviceAccountTokenAttrs, err := ns.tokenClient.PodServiceAccountTokenAttrs(podNamespace, podName, serviceAccountName, types.UID(podUID)) if err != nil { @@ -209,7 +216,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis klog.ErrorS(err, "failed to marshal node publish secrets", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}) return nil, err } - permissionStr, err := json.Marshal(permission) + permissionStr, err := json.Marshal(FilePermission) if err != nil { klog.ErrorS(err, "failed to marshal file permission", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}) return nil, err diff --git a/pkg/secrets-store/nodeserver_test.go b/pkg/secrets-store/nodeserver_test.go index 6d91835bf..ba9594cfb 100644 --- a/pkg/secrets-store/nodeserver_test.go +++ b/pkg/secrets-store/nodeserver_test.go @@ -115,7 +115,7 @@ func TestNodePublishVolume(t *testing.T) { VolumeCapability: &csi.VolumeCapability{}, VolumeId: "testvolid1", TargetPath: tmpdir.New(t, "", "ut"), - VolumeContext: map[string]string{"secretProviderClass": "provider1", csipodname: "pod1", csipodnamespace: "default"}, + VolumeContext: map[string]string{"secretProviderClass": "provider1", CSIPodName: "pod1", CSIPodNamespace: "default"}, }, initObjects: []client.Object{ &secretsstorev1.SecretProviderClass{ @@ -134,7 +134,7 @@ func TestNodePublishVolume(t *testing.T) { VolumeCapability: &csi.VolumeCapability{}, VolumeId: "testvolid1", TargetPath: tmpdir.New(t, "", "ut"), - VolumeContext: map[string]string{"secretProviderClass": "provider1", csipodname: "pod1", csipodnamespace: "default"}, + VolumeContext: map[string]string{"secretProviderClass": "provider1", CSIPodName: "pod1", CSIPodNamespace: "default"}, }, initObjects: []client.Object{ &secretsstorev1.SecretProviderClass{ @@ -153,7 +153,7 @@ func TestNodePublishVolume(t *testing.T) { VolumeCapability: &csi.VolumeCapability{}, VolumeId: "testvolid1", TargetPath: tmpdir.New(t, "", "ut"), - VolumeContext: map[string]string{"secretProviderClass": "provider1", csipodname: "pod1", csipodnamespace: "default"}, + VolumeContext: map[string]string{"secretProviderClass": "provider1", CSIPodName: "pod1", CSIPodNamespace: "default"}, }, initObjects: []client.Object{ &secretsstorev1.SecretProviderClass{ @@ -175,7 +175,7 @@ func TestNodePublishVolume(t *testing.T) { VolumeCapability: &csi.VolumeCapability{}, VolumeId: "testvolid1", TargetPath: tmpdir.New(t, "", "ut"), - VolumeContext: map[string]string{"secretProviderClass": "provider1", csipodname: "pod1", csipodnamespace: "default"}, + VolumeContext: map[string]string{"secretProviderClass": "provider1", CSIPodName: "pod1", CSIPodNamespace: "default"}, }, initObjects: []client.Object{ &secretsstorev1.SecretProviderClass{ @@ -198,7 +198,7 @@ func TestNodePublishVolume(t *testing.T) { VolumeCapability: &csi.VolumeCapability{}, VolumeId: "testvolid1", TargetPath: tmpdir.New(t, "", "ut"), - VolumeContext: map[string]string{"secretProviderClass": "provider1", csipodname: "pod1", csipodnamespace: "default", csipoduid: "poduid1", "providerName": "mock_provider"}, + VolumeContext: map[string]string{"secretProviderClass": "provider1", CSIPodName: "pod1", CSIPodNamespace: "default", CSIPodUID: "poduid1", "providerName": "mock_provider"}, Readonly: true, }, initObjects: []client.Object{ @@ -225,12 +225,12 @@ func TestNodePublishVolume(t *testing.T) { TargetPath: tmpdir.New(t, "", "ut"), VolumeContext: map[string]string{ "secretProviderClass": "simple_provider", - csipodname: "pod1", - csipodnamespace: "default", - csipoduid: "poduid1", + CSIPodName: "pod1", + CSIPodNamespace: "default", + CSIPodUID: "poduid1", // not a real token, just for testing - csipodsatokens: `{"https://kubernetes.default.svc":{"token":"eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMyJ9.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjIl0sImV4cCI6MTYxMTk1OTM5NiwiaWF0IjoxNjExOTU4Nzk2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMiLCJrdWJlcm5ldGVzLmlvIjp7Im5hbWVzcGFjZSI6ImRlZmF1bHQiLCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoiZGVmYXVsdCIsInVpZCI6IjA5MWUyNTU3LWJkODYtNDhhMC1iZmNmLWI1YTI4ZjRjODAyNCJ9fSwibmJmIjoxNjExOTU4Nzk2LCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpkZWZhdWx0In0.YNU2Z_gEE84DGCt8lh9GuE8gmoof-Pk_7emp3fsyj9pq16DRiDaLtOdprH-njpOYqvtT5Uf_QspFc_RwD_pdq9UJWCeLxFkRTsYR5WSjhMFcl767c4Cwp_oZPYhaHd1x7aU1emH-9oarrM__tr1hSmGoAc2I0gUSkAYFueaTUSy5e5d9QKDfjVljDRc7Yrp6qAAfd1OuDdk1XYIjrqTHk1T1oqGGlcd3lRM_dKSsW5I_YqgKMrjwNt8yOKcdKBrgQhgC42GZbFDRVJDJHs_Hq32xo-2s3PJ8UZ_alN4wv8EbuwB987_FHBTc_XAULHPvp0mCv2C5h0V2A7gzccv30A","expirationTimestamp":"2021-01-29T22:29:56Z"}}`, - "providerName": "simple_provider", + CSIPodServiceAccountTokens: `{"https://kubernetes.default.svc":{"token":"eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMyJ9.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjIl0sImV4cCI6MTYxMTk1OTM5NiwiaWF0IjoxNjExOTU4Nzk2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMiLCJrdWJlcm5ldGVzLmlvIjp7Im5hbWVzcGFjZSI6ImRlZmF1bHQiLCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoiZGVmYXVsdCIsInVpZCI6IjA5MWUyNTU3LWJkODYtNDhhMC1iZmNmLWI1YTI4ZjRjODAyNCJ9fSwibmJmIjoxNjExOTU4Nzk2LCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpkZWZhdWx0In0.YNU2Z_gEE84DGCt8lh9GuE8gmoof-Pk_7emp3fsyj9pq16DRiDaLtOdprH-njpOYqvtT5Uf_QspFc_RwD_pdq9UJWCeLxFkRTsYR5WSjhMFcl767c4Cwp_oZPYhaHd1x7aU1emH-9oarrM__tr1hSmGoAc2I0gUSkAYFueaTUSy5e5d9QKDfjVljDRc7Yrp6qAAfd1OuDdk1XYIjrqTHk1T1oqGGlcd3lRM_dKSsW5I_YqgKMrjwNt8yOKcdKBrgQhgC42GZbFDRVJDJHs_Hq32xo-2s3PJ8UZ_alN4wv8EbuwB987_FHBTc_XAULHPvp0mCv2C5h0V2A7gzccv30A","expirationTimestamp":"2021-01-29T22:29:56Z"}}`, + "providerName": "simple_provider", }, Readonly: true, }, @@ -356,12 +356,12 @@ func TestTestNodePublishVolume_ProviderError(t *testing.T) { TargetPath: tmpdir.New(t, "", "ut"), VolumeContext: map[string]string{ "secretProviderClass": "simple_provider", - csipodname: "pod1", - csipodnamespace: "default", - csipoduid: "poduid1", + CSIPodName: "pod1", + CSIPodNamespace: "default", + CSIPodUID: "poduid1", // not a real token, just for testing - csipodsatokens: `{"https://kubernetes.default.svc":{"token":"eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMyJ9.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjIl0sImV4cCI6MTYxMTk1OTM5NiwiaWF0IjoxNjExOTU4Nzk2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMiLCJrdWJlcm5ldGVzLmlvIjp7Im5hbWVzcGFjZSI6ImRlZmF1bHQiLCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoiZGVmYXVsdCIsInVpZCI6IjA5MWUyNTU3LWJkODYtNDhhMC1iZmNmLWI1YTI4ZjRjODAyNCJ9fSwibmJmIjoxNjExOTU4Nzk2LCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpkZWZhdWx0In0.YNU2Z_gEE84DGCt8lh9GuE8gmoof-Pk_7emp3fsyj9pq16DRiDaLtOdprH-njpOYqvtT5Uf_QspFc_RwD_pdq9UJWCeLxFkRTsYR5WSjhMFcl767c4Cwp_oZPYhaHd1x7aU1emH-9oarrM__tr1hSmGoAc2I0gUSkAYFueaTUSy5e5d9QKDfjVljDRc7Yrp6qAAfd1OuDdk1XYIjrqTHk1T1oqGGlcd3lRM_dKSsW5I_YqgKMrjwNt8yOKcdKBrgQhgC42GZbFDRVJDJHs_Hq32xo-2s3PJ8UZ_alN4wv8EbuwB987_FHBTc_XAULHPvp0mCv2C5h0V2A7gzccv30A","expirationTimestamp":"2021-01-29T22:29:56Z"}}`, - "providerName": "simple_provider", + CSIPodServiceAccountTokens: `{"https://kubernetes.default.svc":{"token":"eyJhbGciOiJSUzI1NiIsImtpZCI6IjEyMyJ9.eyJhdWQiOlsiaHR0cHM6Ly9rdWJlcm5ldGVzLmRlZmF1bHQuc3ZjIl0sImV4cCI6MTYxMTk1OTM5NiwiaWF0IjoxNjExOTU4Nzk2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMiLCJrdWJlcm5ldGVzLmlvIjp7Im5hbWVzcGFjZSI6ImRlZmF1bHQiLCJzZXJ2aWNlYWNjb3VudCI6eyJuYW1lIjoiZGVmYXVsdCIsInVpZCI6IjA5MWUyNTU3LWJkODYtNDhhMC1iZmNmLWI1YTI4ZjRjODAyNCJ9fSwibmJmIjoxNjExOTU4Nzk2LCJzdWIiOiJzeXN0ZW06c2VydmljZWFjY291bnQ6ZGVmYXVsdDpkZWZhdWx0In0.YNU2Z_gEE84DGCt8lh9GuE8gmoof-Pk_7emp3fsyj9pq16DRiDaLtOdprH-njpOYqvtT5Uf_QspFc_RwD_pdq9UJWCeLxFkRTsYR5WSjhMFcl767c4Cwp_oZPYhaHd1x7aU1emH-9oarrM__tr1hSmGoAc2I0gUSkAYFueaTUSy5e5d9QKDfjVljDRc7Yrp6qAAfd1OuDdk1XYIjrqTHk1T1oqGGlcd3lRM_dKSsW5I_YqgKMrjwNt8yOKcdKBrgQhgC42GZbFDRVJDJHs_Hq32xo-2s3PJ8UZ_alN4wv8EbuwB987_FHBTc_XAULHPvp0mCv2C5h0V2A7gzccv30A","expirationTimestamp":"2021-01-29T22:29:56Z"}}`, + "providerName": "simple_provider", }, Readonly: true, } diff --git a/pkg/secrets-store/server.go b/pkg/secrets-store/server.go index 753007b92..7a1f7f81f 100644 --- a/pkg/secrets-store/server.go +++ b/pkg/secrets-store/server.go @@ -148,7 +148,7 @@ func logInterceptor() grpc.UnaryServerInterceptor { start := time.Now() deadline, _ := ctx.Deadline() dd := time.Until(deadline).String() - klog.V(5).InfoS("request", "method", info.FullMethod, "req", pbSanitizer.StripSecrets(req).String(), "deadline", dd) + klog.V(5).InfoS("request", "method", info.FullMethod, "req", sanitizeRequest(req), "deadline", dd) resp, err := handler(ctx, req) s, _ := status.FromError(err) @@ -156,3 +156,24 @@ func logInterceptor() grpc.UnaryServerInterceptor { return resp, err } } + +// sanitizeRequest returns a sanitized version of the request. +// protosanitizer strips out sensitive information from the secret field, however it doesn't handle +// tokens in the volume context. This function handles that. +func sanitizeRequest(req interface{}) string { + r, ok := req.(*csi.NodePublishVolumeRequest) + if !ok { + return pbSanitizer.StripSecrets(req).String() + } + + tmp := *r + volumeContext := make(map[string]string) + for k, v := range r.VolumeContext { + volumeContext[k] = v + } + if _, ok := volumeContext[CSIPodServiceAccountTokens]; ok { + volumeContext[CSIPodServiceAccountTokens] = "***stripped***" + } + tmp.VolumeContext = volumeContext + return pbSanitizer.StripSecrets(&tmp).String() +} diff --git a/pkg/secrets-store/server_test.go b/pkg/secrets-store/server_test.go index d66ea4b5b..b25244ece 100644 --- a/pkg/secrets-store/server_test.go +++ b/pkg/secrets-store/server_test.go @@ -17,8 +17,10 @@ limitations under the License. package secretsstore import ( + "fmt" "testing" + "github.com/container-storage-interface/spec/lib/go/csi" "github.com/stretchr/testify/assert" ) @@ -105,3 +107,45 @@ func TestParseEndpointError(t *testing.T) { }) } } + +func TestSanitizeRequest(t *testing.T) { + tests := []struct { + name string + req interface{} + expected string + }{ + { + name: "node unpublish volume request", + req: &csi.NodeUnpublishVolumeRequest{}, + expected: "{}", + }, + { + name: "node publish volume request without tokens", + req: &csi.NodePublishVolumeRequest{ + Secrets: map[string]string{ + "foo": "bar", + }, + }, + expected: `{"secrets":"***stripped***"}`, + }, + { + name: "node publish volume request with tokens", + req: &csi.NodePublishVolumeRequest{ + Secrets: map[string]string{ + "foo": "bar", + }, + VolumeContext: map[string]string{ + CSIPodServiceAccountTokens: "token1,token2", + }, + }, + expected: fmt.Sprintf(`{"secrets":"***stripped***","volume_context":{"%s":"***stripped***"}}`, CSIPodServiceAccountTokens), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := sanitizeRequest(test.req) + assert.Equal(t, test.expected, actual) + }) + } +}