From eba490ccef48d9ed1f90444888842c0629962d8b Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Fri, 2 Dec 2022 10:12:05 -0800 Subject: [PATCH] Check if sys view is missing in GRPC sys view (#18210) And return an error instead of panicking. This situation can occur if a plugin attempts to access the system view during setup when Vault is checking the plugin metadata. Fixes #17878. --- changelog/18210.txt | 3 +++ sdk/plugin/grpc_storage.go | 16 +++++++++++++- sdk/plugin/grpc_system.go | 38 ++++++++++++++++++++++++++++++++++ sdk/plugin/grpc_system_test.go | 9 +++++++- sdk/plugin/storage_test.go | 10 ++++++++- 5 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 changelog/18210.txt diff --git a/changelog/18210.txt b/changelog/18210.txt new file mode 100644 index 000000000..bd1c70b62 --- /dev/null +++ b/changelog/18210.txt @@ -0,0 +1,3 @@ +```release-note:bug +sdk: Don't panic if system view or storage methods called during plugin setup. +``` diff --git a/sdk/plugin/grpc_storage.go b/sdk/plugin/grpc_storage.go index b7fd0caf8..6a04b3a97 100644 --- a/sdk/plugin/grpc_storage.go +++ b/sdk/plugin/grpc_storage.go @@ -10,6 +10,8 @@ import ( "github.com/hashicorp/vault/sdk/plugin/pb" ) +var errMissingStorage = errors.New("missing storage implementation: this method should not be called during plugin Setup, but only during and after Initialize") + func newGRPCStorageClient(conn *grpc.ClientConn) *GRPCStorageClient { return &GRPCStorageClient{ client: pb.NewStorageClient(conn), @@ -74,13 +76,16 @@ func (s *GRPCStorageClient) Delete(ctx context.Context, key string) error { return nil } -// StorageServer is a net/rpc compatible structure for serving +// GRPCStorageServer is a net/rpc compatible structure for serving type GRPCStorageServer struct { pb.UnimplementedStorageServer impl logical.Storage } func (s *GRPCStorageServer) List(ctx context.Context, args *pb.StorageListArgs) (*pb.StorageListReply, error) { + if s.impl == nil { + return nil, errMissingStorage + } keys, err := s.impl.List(ctx, args.Prefix) return &pb.StorageListReply{ Keys: keys, @@ -89,6 +94,9 @@ func (s *GRPCStorageServer) List(ctx context.Context, args *pb.StorageListArgs) } func (s *GRPCStorageServer) Get(ctx context.Context, args *pb.StorageGetArgs) (*pb.StorageGetReply, error) { + if s.impl == nil { + return nil, errMissingStorage + } storageEntry, err := s.impl.Get(ctx, args.Key) if storageEntry == nil { return &pb.StorageGetReply{ @@ -103,6 +111,9 @@ func (s *GRPCStorageServer) Get(ctx context.Context, args *pb.StorageGetArgs) (* } func (s *GRPCStorageServer) Put(ctx context.Context, args *pb.StoragePutArgs) (*pb.StoragePutReply, error) { + if s.impl == nil { + return nil, errMissingStorage + } err := s.impl.Put(ctx, pb.ProtoStorageEntryToLogicalStorageEntry(args.Entry)) return &pb.StoragePutReply{ Err: pb.ErrToString(err), @@ -110,6 +121,9 @@ func (s *GRPCStorageServer) Put(ctx context.Context, args *pb.StoragePutArgs) (* } func (s *GRPCStorageServer) Delete(ctx context.Context, args *pb.StorageDeleteArgs) (*pb.StorageDeleteReply, error) { + if s.impl == nil { + return nil, errMissingStorage + } err := s.impl.Delete(ctx, args.Key) return &pb.StorageDeleteReply{ Err: pb.ErrToString(err), diff --git a/sdk/plugin/grpc_system.go b/sdk/plugin/grpc_system.go index 3761269c4..5ab680a5d 100644 --- a/sdk/plugin/grpc_system.go +++ b/sdk/plugin/grpc_system.go @@ -18,6 +18,8 @@ import ( "google.golang.org/grpc/status" ) +var errMissingSystemView = errors.New("missing system view implementation: this method should not be called during plugin Setup, but only during and after Initialize") + func newGRPCSystemView(conn *grpc.ClientConn) *gRPCSystemViewClient { return &gRPCSystemViewClient{ client: pb.NewSystemViewClient(conn), @@ -193,6 +195,9 @@ type gRPCSystemViewServer struct { } func (s *gRPCSystemViewServer) DefaultLeaseTTL(ctx context.Context, _ *pb.Empty) (*pb.TTLReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } ttl := s.impl.DefaultLeaseTTL() return &pb.TTLReply{ TTL: int64(ttl), @@ -200,6 +205,9 @@ func (s *gRPCSystemViewServer) DefaultLeaseTTL(ctx context.Context, _ *pb.Empty) } func (s *gRPCSystemViewServer) MaxLeaseTTL(ctx context.Context, _ *pb.Empty) (*pb.TTLReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } ttl := s.impl.MaxLeaseTTL() return &pb.TTLReply{ TTL: int64(ttl), @@ -207,6 +215,9 @@ func (s *gRPCSystemViewServer) MaxLeaseTTL(ctx context.Context, _ *pb.Empty) (*p } func (s *gRPCSystemViewServer) Tainted(ctx context.Context, _ *pb.Empty) (*pb.TaintedReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } tainted := s.impl.Tainted() return &pb.TaintedReply{ Tainted: tainted, @@ -214,6 +225,9 @@ func (s *gRPCSystemViewServer) Tainted(ctx context.Context, _ *pb.Empty) (*pb.Ta } func (s *gRPCSystemViewServer) CachingDisabled(ctx context.Context, _ *pb.Empty) (*pb.CachingDisabledReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } cachingDisabled := s.impl.CachingDisabled() return &pb.CachingDisabledReply{ Disabled: cachingDisabled, @@ -221,6 +235,9 @@ func (s *gRPCSystemViewServer) CachingDisabled(ctx context.Context, _ *pb.Empty) } func (s *gRPCSystemViewServer) ReplicationState(ctx context.Context, _ *pb.Empty) (*pb.ReplicationStateReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } replicationState := s.impl.ReplicationState() return &pb.ReplicationStateReply{ State: int32(replicationState), @@ -228,6 +245,9 @@ func (s *gRPCSystemViewServer) ReplicationState(ctx context.Context, _ *pb.Empty } func (s *gRPCSystemViewServer) ResponseWrapData(ctx context.Context, args *pb.ResponseWrapDataArgs) (*pb.ResponseWrapDataReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } data := map[string]interface{}{} err := json.Unmarshal([]byte(args.Data), &data) if err != nil { @@ -253,6 +273,9 @@ func (s *gRPCSystemViewServer) ResponseWrapData(ctx context.Context, args *pb.Re } func (s *gRPCSystemViewServer) MlockEnabled(ctx context.Context, _ *pb.Empty) (*pb.MlockEnabledReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } enabled := s.impl.MlockEnabled() return &pb.MlockEnabledReply{ Enabled: enabled, @@ -260,6 +283,9 @@ func (s *gRPCSystemViewServer) MlockEnabled(ctx context.Context, _ *pb.Empty) (* } func (s *gRPCSystemViewServer) LocalMount(ctx context.Context, _ *pb.Empty) (*pb.LocalMountReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } local := s.impl.LocalMount() return &pb.LocalMountReply{ Local: local, @@ -267,6 +293,9 @@ func (s *gRPCSystemViewServer) LocalMount(ctx context.Context, _ *pb.Empty) (*pb } func (s *gRPCSystemViewServer) EntityInfo(ctx context.Context, args *pb.EntityInfoArgs) (*pb.EntityInfoReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } entity, err := s.impl.EntityInfo(args.EntityID) if err != nil { return &pb.EntityInfoReply{ @@ -279,6 +308,9 @@ func (s *gRPCSystemViewServer) EntityInfo(ctx context.Context, args *pb.EntityIn } func (s *gRPCSystemViewServer) GroupsForEntity(ctx context.Context, args *pb.EntityInfoArgs) (*pb.GroupsForEntityReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } groups, err := s.impl.GroupsForEntity(args.EntityID) if err != nil { return &pb.GroupsForEntityReply{ @@ -291,6 +323,9 @@ func (s *gRPCSystemViewServer) GroupsForEntity(ctx context.Context, args *pb.Ent } func (s *gRPCSystemViewServer) PluginEnv(ctx context.Context, _ *pb.Empty) (*pb.PluginEnvReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } pluginEnv, err := s.impl.PluginEnv(ctx) if err != nil { return &pb.PluginEnvReply{ @@ -303,6 +338,9 @@ func (s *gRPCSystemViewServer) PluginEnv(ctx context.Context, _ *pb.Empty) (*pb. } func (s *gRPCSystemViewServer) GeneratePasswordFromPolicy(ctx context.Context, req *pb.GeneratePasswordFromPolicyRequest) (*pb.GeneratePasswordFromPolicyReply, error) { + if s.impl == nil { + return nil, errMissingSystemView + } policyName := req.PolicyName if policyName == "" { return &pb.GeneratePasswordFromPolicyReply{}, status.Errorf(codes.InvalidArgument, "no password policy specified") diff --git a/sdk/plugin/grpc_system_test.go b/sdk/plugin/grpc_system_test.go index 577545345..7a282608a 100644 --- a/sdk/plugin/grpc_system_test.go +++ b/sdk/plugin/grpc_system_test.go @@ -6,7 +6,7 @@ import ( "testing" "time" - plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/go-plugin" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/plugin/pb" @@ -14,6 +14,13 @@ import ( "google.golang.org/protobuf/proto" ) +func TestSystem_GRPC_ReturnsErrIfSystemViewNil(t *testing.T) { + _, err := new(gRPCSystemViewServer).ReplicationState(context.Background(), nil) + if err == nil { + t.Error("Expected error when using server with no impl") + } +} + func TestSystem_GRPC_GRPC_impl(t *testing.T) { var _ logical.SystemView = new(gRPCSystemViewClient) } diff --git a/sdk/plugin/storage_test.go b/sdk/plugin/storage_test.go index 21fc4e7ec..651d36199 100644 --- a/sdk/plugin/storage_test.go +++ b/sdk/plugin/storage_test.go @@ -1,15 +1,23 @@ package plugin import ( + "context" "testing" "google.golang.org/grpc" - plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/go-plugin" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/plugin/pb" ) +func TestStorage_GRPC_ReturnsErrIfStorageNil(t *testing.T) { + _, err := new(GRPCStorageServer).Get(context.Background(), nil) + if err == nil { + t.Error("Expected error when using server with no impl") + } +} + func TestStorage_impl(t *testing.T) { var _ logical.Storage = new(GRPCStorageClient) }