Skip to content

Commit

Permalink
Clear secrets from request for klog print in logGRPC()
Browse files Browse the repository at this point in the history
Malicious user can put a secret in request as explained here: kubernetes-sigs#1372.
  • Loading branch information
mpatlasov committed Nov 3, 2023
1 parent b105d5a commit afd457a
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 2 deletions.
21 changes: 19 additions & 2 deletions pkg/gce-pd-csi-driver/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"reflect"

csi "github.com/container-storage-interface/spec/lib/go/csi"
"google.golang.org/grpc"
Expand Down Expand Up @@ -56,14 +57,30 @@ func NewNodeServiceCapability(cap csi.NodeServiceCapability_RPC_Type) *csi.NodeS
}
}

// Reflect magic below simply clears Secrets map from request.
func clearSecrets(req interface{}) interface{} {
v := reflect.ValueOf(&req).Elem()
e := reflect.New(v.Elem().Type()).Elem()
e.Set(v.Elem())
f := reflect.Indirect(e).FieldByName("Secrets")
if f.IsValid() && f.CanSet() && f.Kind() == reflect.Map {
f.Set(reflect.MakeMap(f.Type()))
v.Set(e)
}
return req
}

func logGRPC(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
if info.FullMethod == ProbeCSIFullMethod {
return handler(ctx, req)
}
// Note that secrets are not included in any RPC message. In the past protosanitizer and other log
// Note that secrets may be included in some RPC messages
// (https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues/1372),
// but the driver ignores them. In the past protosanitizer and other log
// stripping was shown to cause a significant increase of CPU usage (see
// https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/issues/356#issuecomment-550529004).
klog.V(4).Infof("%s called with request: %s", info.FullMethod, req)
// That is why we use hand-crafted clearSecrets() below rather than protosanitizer.
klog.V(4).Infof("%s called with request: %s", info.FullMethod, clearSecrets(req))
resp, err := handler(ctx, req)
if err != nil {
klog.Errorf("%s returned with error: %v", info.FullMethod, err.Error())
Expand Down
30 changes: 30 additions & 0 deletions pkg/gce-pd-csi-driver/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.
package gceGCEDriver

import (
"reflect"
"testing"

csi "github.com/container-storage-interface/spec/lib/go/csi"
Expand Down Expand Up @@ -291,3 +292,32 @@ func TestGetReadOnlyFromCapabilities(t *testing.T) {
}
}
}

func TestClearSecrets(t *testing.T) {
vc := &csi.VolumeCapability{
AccessType: &csi.VolumeCapability_Mount{
Mount: &csi.VolumeCapability_MountVolume{},
},
AccessMode: &csi.VolumeCapability_AccessMode{
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
},
}

req := &csi.NodeExpandVolumeRequest{
VolumePath: "/path",
VolumeCapability: vc,
Secrets: map[string]string{
"key": "value",
},
}

clearedReq := &csi.NodeExpandVolumeRequest{
VolumePath: "/path",
VolumeCapability: vc,
Secrets: map[string]string{},
}

if !reflect.DeepEqual(clearSecrets(req), clearedReq) {
t.Fatalf("Unexpected change: %v vs. %v", clearSecrets(req), clearedReq)
}
}

0 comments on commit afd457a

Please sign in to comment.