From 820a3433a26de76c081a010e8e54a9af1eaf6f87 Mon Sep 17 00:00:00 2001 From: Marcin Walas Date: Thu, 14 Nov 2024 10:00:31 +0100 Subject: [PATCH 1/3] Add way to specify user for exec command. --- services/exec/client/client.go | 9 +++- services/exec/exec.pb.go | 58 +++++++++++++--------- services/exec/exec.proto | 2 + services/exec/exec_grpc.pb.go | 81 ++++++++++++++----------------- services/exec/server/exec.go | 23 ++++++++- services/exec/server/exec_test.go | 9 ++++ 6 files changed, 111 insertions(+), 71 deletions(-) diff --git a/services/exec/client/client.go b/services/exec/client/client.go index 60c6500a..45c42f4e 100644 --- a/services/exec/client/client.go +++ b/services/exec/client/client.go @@ -67,6 +67,7 @@ func (p *execCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfac type runCmd struct { streaming bool + user string // returnCode internally keeps track of the final status to return returnCode subcommands.ExitStatus @@ -75,7 +76,7 @@ type runCmd struct { func (*runCmd) Name() string { return "run" } func (*runCmd) Synopsis() string { return "Run provided command and return a response." } func (*runCmd) Usage() string { - return `run [--stream] [...]: + return `run [--stream] [--user user] [...]: Run a command remotely and return the response Note: This is not optimized for large output or long running commands. If @@ -84,11 +85,15 @@ func (*runCmd) Usage() string { The --stream flag can be used to stream back command output as the command runs. It doesn't affect the timeout. + + --user flag allows to specify a user for running command, equivalent of + sudo -u ... ` } func (p *runCmd) SetFlags(f *flag.FlagSet) { f.BoolVar(&p.streaming, "stream", DefaultStreaming, "If true, stream back stdout and stdin during the command instead of sending it all at the end.") + f.StringVar(&p.user, "user", "", "If specified, allows to run a command as a specified user. Equivalent of sudo -u ... .") } func (p *runCmd) printCommandOutput(state *util.ExecuteState, idx int, resp *pb.ExecResponse, err error) { @@ -121,7 +126,7 @@ func (p *runCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface } c := pb.NewExecClientProxy(state.Conn) - req := &pb.ExecRequest{Command: f.Args()[0], Args: f.Args()[1:]} + req := &pb.ExecRequest{Command: f.Args()[0], Args: f.Args()[1:], User: p.user} if p.streaming { resp, err := c.StreamingRunOneMany(ctx, req) diff --git a/services/exec/exec.pb.go b/services/exec/exec.pb.go index e3b7c17b..5bf3af06 100644 --- a/services/exec/exec.pb.go +++ b/services/exec/exec.pb.go @@ -15,8 +15,8 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.32.0 -// protoc v5.26.1 +// protoc-gen-go v1.34.2 +// protoc v5.28.3 // source: exec.proto package exec @@ -43,6 +43,8 @@ type ExecRequest struct { Command string `protobuf:"bytes,1,opt,name=command,proto3" json:"command,omitempty"` Args []string `protobuf:"bytes,2,rep,name=args,proto3" json:"args,omitempty"` + // User to execute command as, equivalent of `sudo -u `. + User string `protobuf:"bytes,3,opt,name=user,proto3" json:"user,omitempty"` } func (x *ExecRequest) Reset() { @@ -91,6 +93,13 @@ func (x *ExecRequest) GetArgs() []string { return nil } +func (x *ExecRequest) GetUser() string { + if x != nil { + return x.User + } + return "" +} + // ExecResponse describes output of execution type ExecResponse struct { state protoimpl.MessageState @@ -159,27 +168,28 @@ var File_exec_proto protoreflect.FileDescriptor var file_exec_proto_rawDesc = []byte{ 0x0a, 0x0a, 0x65, 0x78, 0x65, 0x63, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x04, 0x45, 0x78, - 0x65, 0x63, 0x22, 0x3b, 0x0a, 0x0b, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, + 0x65, 0x63, 0x22, 0x4f, 0x0a, 0x0b, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x18, 0x0a, 0x07, 0x63, 0x6f, 0x6d, 0x6d, 0x61, 0x6e, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x63, 0x6f, 0x6d, 0x6d, 0x61, 0x6e, 0x64, 0x12, 0x12, 0x0a, 0x04, 0x61, - 0x72, 0x67, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x09, 0x52, 0x04, 0x61, 0x72, 0x67, 0x73, 0x22, - 0x58, 0x0a, 0x0c, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, - 0x16, 0x0a, 0x06, 0x73, 0x74, 0x64, 0x6f, 0x75, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, - 0x06, 0x73, 0x74, 0x64, 0x6f, 0x75, 0x74, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x74, 0x64, 0x65, 0x72, - 0x72, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x06, 0x73, 0x74, 0x64, 0x65, 0x72, 0x72, 0x12, - 0x18, 0x0a, 0x07, 0x72, 0x65, 0x74, 0x43, 0x6f, 0x64, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x05, - 0x52, 0x07, 0x72, 0x65, 0x74, 0x43, 0x6f, 0x64, 0x65, 0x32, 0x71, 0x0a, 0x04, 0x45, 0x78, 0x65, - 0x63, 0x12, 0x2e, 0x0a, 0x03, 0x52, 0x75, 0x6e, 0x12, 0x11, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x2e, - 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x45, 0x78, - 0x65, 0x63, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, - 0x00, 0x12, 0x39, 0x0a, 0x0c, 0x53, 0x74, 0x72, 0x65, 0x61, 0x6d, 0x69, 0x6e, 0x67, 0x52, 0x75, - 0x6e, 0x12, 0x11, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, - 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x2e, 0x45, 0x78, 0x65, 0x63, - 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x30, 0x01, 0x42, 0x33, 0x5a, 0x31, - 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x53, 0x6e, 0x6f, 0x77, 0x66, - 0x6c, 0x61, 0x6b, 0x65, 0x2d, 0x4c, 0x61, 0x62, 0x73, 0x2f, 0x73, 0x61, 0x6e, 0x73, 0x73, 0x68, - 0x65, 0x6c, 0x6c, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, 0x2f, 0x65, 0x78, 0x65, - 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x72, 0x67, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x09, 0x52, 0x04, 0x61, 0x72, 0x67, 0x73, 0x12, + 0x12, 0x0a, 0x04, 0x75, 0x73, 0x65, 0x72, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x75, + 0x73, 0x65, 0x72, 0x22, 0x58, 0x0a, 0x0c, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, + 0x6e, 0x73, 0x65, 0x12, 0x16, 0x0a, 0x06, 0x73, 0x74, 0x64, 0x6f, 0x75, 0x74, 0x18, 0x01, 0x20, + 0x01, 0x28, 0x0c, 0x52, 0x06, 0x73, 0x74, 0x64, 0x6f, 0x75, 0x74, 0x12, 0x16, 0x0a, 0x06, 0x73, + 0x74, 0x64, 0x65, 0x72, 0x72, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x06, 0x73, 0x74, 0x64, + 0x65, 0x72, 0x72, 0x12, 0x18, 0x0a, 0x07, 0x72, 0x65, 0x74, 0x43, 0x6f, 0x64, 0x65, 0x18, 0x03, + 0x20, 0x01, 0x28, 0x05, 0x52, 0x07, 0x72, 0x65, 0x74, 0x43, 0x6f, 0x64, 0x65, 0x32, 0x71, 0x0a, + 0x04, 0x45, 0x78, 0x65, 0x63, 0x12, 0x2e, 0x0a, 0x03, 0x52, 0x75, 0x6e, 0x12, 0x11, 0x2e, 0x45, + 0x78, 0x65, 0x63, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, + 0x12, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, + 0x6e, 0x73, 0x65, 0x22, 0x00, 0x12, 0x39, 0x0a, 0x0c, 0x53, 0x74, 0x72, 0x65, 0x61, 0x6d, 0x69, + 0x6e, 0x67, 0x52, 0x75, 0x6e, 0x12, 0x11, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x2e, 0x45, 0x78, 0x65, + 0x63, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x12, 0x2e, 0x45, 0x78, 0x65, 0x63, 0x2e, + 0x45, 0x78, 0x65, 0x63, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x00, 0x30, 0x01, + 0x42, 0x33, 0x5a, 0x31, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x53, + 0x6e, 0x6f, 0x77, 0x66, 0x6c, 0x61, 0x6b, 0x65, 0x2d, 0x4c, 0x61, 0x62, 0x73, 0x2f, 0x73, 0x61, + 0x6e, 0x73, 0x73, 0x68, 0x65, 0x6c, 0x6c, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x73, + 0x2f, 0x65, 0x78, 0x65, 0x63, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -195,7 +205,7 @@ func file_exec_proto_rawDescGZIP() []byte { } var file_exec_proto_msgTypes = make([]protoimpl.MessageInfo, 2) -var file_exec_proto_goTypes = []interface{}{ +var file_exec_proto_goTypes = []any{ (*ExecRequest)(nil), // 0: Exec.ExecRequest (*ExecResponse)(nil), // 1: Exec.ExecResponse } @@ -217,7 +227,7 @@ func file_exec_proto_init() { return } if !protoimpl.UnsafeEnabled { - file_exec_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { + file_exec_proto_msgTypes[0].Exporter = func(v any, i int) any { switch v := v.(*ExecRequest); i { case 0: return &v.state @@ -229,7 +239,7 @@ func file_exec_proto_init() { return nil } } - file_exec_proto_msgTypes[1].Exporter = func(v interface{}, i int) interface{} { + file_exec_proto_msgTypes[1].Exporter = func(v any, i int) any { switch v := v.(*ExecResponse); i { case 0: return &v.state diff --git a/services/exec/exec.proto b/services/exec/exec.proto index b45607d7..e9dc8138 100644 --- a/services/exec/exec.proto +++ b/services/exec/exec.proto @@ -35,6 +35,8 @@ service Exec { message ExecRequest { string command = 1; repeated string args = 2; + // User to execute command as, equivalent of `sudo -u `. + string user = 3; } // ExecResponse describes output of execution diff --git a/services/exec/exec_grpc.pb.go b/services/exec/exec_grpc.pb.go index 07702ce2..25dcb8ee 100644 --- a/services/exec/exec_grpc.pb.go +++ b/services/exec/exec_grpc.pb.go @@ -15,8 +15,8 @@ // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: -// - protoc-gen-go-grpc v1.3.0 -// - protoc v5.26.1 +// - protoc-gen-go-grpc v1.5.1 +// - protoc v5.28.3 // source: exec.proto package exec @@ -30,8 +30,8 @@ import ( // This is a compile-time assertion to ensure that this generated file // is compatible with the grpc package it is being compiled against. -// Requires gRPC-Go v1.32.0 or later. -const _ = grpc.SupportPackageIsVersion7 +// Requires gRPC-Go v1.64.0 or later. +const _ = grpc.SupportPackageIsVersion9 const ( Exec_Run_FullMethodName = "/Exec.Exec/Run" @@ -41,6 +41,8 @@ const ( // ExecClient is the client API for Exec service. // // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. +// +// The Exec service definition. type ExecClient interface { // Run takes input, executes it and returns result of input execution Run(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (*ExecResponse, error) @@ -48,7 +50,7 @@ type ExecClient interface { // // A nonzero return code, if any, will be in the final response. Intermediate // responses may contain stdout and/or stderr. - StreamingRun(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (Exec_StreamingRunClient, error) + StreamingRun(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (grpc.ServerStreamingClient[ExecResponse], error) } type execClient struct { @@ -60,20 +62,22 @@ func NewExecClient(cc grpc.ClientConnInterface) ExecClient { } func (c *execClient) Run(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (*ExecResponse, error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) out := new(ExecResponse) - err := c.cc.Invoke(ctx, Exec_Run_FullMethodName, in, out, opts...) + err := c.cc.Invoke(ctx, Exec_Run_FullMethodName, in, out, cOpts...) if err != nil { return nil, err } return out, nil } -func (c *execClient) StreamingRun(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (Exec_StreamingRunClient, error) { - stream, err := c.cc.NewStream(ctx, &Exec_ServiceDesc.Streams[0], Exec_StreamingRun_FullMethodName, opts...) +func (c *execClient) StreamingRun(ctx context.Context, in *ExecRequest, opts ...grpc.CallOption) (grpc.ServerStreamingClient[ExecResponse], error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + stream, err := c.cc.NewStream(ctx, &Exec_ServiceDesc.Streams[0], Exec_StreamingRun_FullMethodName, cOpts...) if err != nil { return nil, err } - x := &execStreamingRunClient{stream} + x := &grpc.GenericClientStream[ExecRequest, ExecResponse]{ClientStream: stream} if err := x.ClientStream.SendMsg(in); err != nil { return nil, err } @@ -83,26 +87,14 @@ func (c *execClient) StreamingRun(ctx context.Context, in *ExecRequest, opts ... return x, nil } -type Exec_StreamingRunClient interface { - Recv() (*ExecResponse, error) - grpc.ClientStream -} - -type execStreamingRunClient struct { - grpc.ClientStream -} - -func (x *execStreamingRunClient) Recv() (*ExecResponse, error) { - m := new(ExecResponse) - if err := x.ClientStream.RecvMsg(m); err != nil { - return nil, err - } - return m, nil -} +// This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. +type Exec_StreamingRunClient = grpc.ServerStreamingClient[ExecResponse] // ExecServer is the server API for Exec service. // All implementations should embed UnimplementedExecServer -// for forward compatibility +// for forward compatibility. +// +// The Exec service definition. type ExecServer interface { // Run takes input, executes it and returns result of input execution Run(context.Context, *ExecRequest) (*ExecResponse, error) @@ -110,19 +102,23 @@ type ExecServer interface { // // A nonzero return code, if any, will be in the final response. Intermediate // responses may contain stdout and/or stderr. - StreamingRun(*ExecRequest, Exec_StreamingRunServer) error + StreamingRun(*ExecRequest, grpc.ServerStreamingServer[ExecResponse]) error } -// UnimplementedExecServer should be embedded to have forward compatible implementations. -type UnimplementedExecServer struct { -} +// UnimplementedExecServer should be embedded to have +// forward compatible implementations. +// +// NOTE: this should be embedded by value instead of pointer to avoid a nil +// pointer dereference when methods are called. +type UnimplementedExecServer struct{} func (UnimplementedExecServer) Run(context.Context, *ExecRequest) (*ExecResponse, error) { return nil, status.Errorf(codes.Unimplemented, "method Run not implemented") } -func (UnimplementedExecServer) StreamingRun(*ExecRequest, Exec_StreamingRunServer) error { +func (UnimplementedExecServer) StreamingRun(*ExecRequest, grpc.ServerStreamingServer[ExecResponse]) error { return status.Errorf(codes.Unimplemented, "method StreamingRun not implemented") } +func (UnimplementedExecServer) testEmbeddedByValue() {} // UnsafeExecServer may be embedded to opt out of forward compatibility for this service. // Use of this interface is not recommended, as added methods to ExecServer will @@ -132,6 +128,13 @@ type UnsafeExecServer interface { } func RegisterExecServer(s grpc.ServiceRegistrar, srv ExecServer) { + // If the following call pancis, it indicates UnimplementedExecServer was + // embedded by pointer and is nil. This will cause panics if an + // unimplemented method is ever invoked, so we test this at initialization + // time to prevent it from happening at runtime later due to I/O. + if t, ok := srv.(interface{ testEmbeddedByValue() }); ok { + t.testEmbeddedByValue() + } s.RegisterService(&Exec_ServiceDesc, srv) } @@ -158,21 +161,11 @@ func _Exec_StreamingRun_Handler(srv interface{}, stream grpc.ServerStream) error if err := stream.RecvMsg(m); err != nil { return err } - return srv.(ExecServer).StreamingRun(m, &execStreamingRunServer{stream}) -} - -type Exec_StreamingRunServer interface { - Send(*ExecResponse) error - grpc.ServerStream -} - -type execStreamingRunServer struct { - grpc.ServerStream + return srv.(ExecServer).StreamingRun(m, &grpc.GenericServerStream[ExecRequest, ExecResponse]{ServerStream: stream}) } -func (x *execStreamingRunServer) Send(m *ExecResponse) error { - return x.ServerStream.SendMsg(m) -} +// This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. +type Exec_StreamingRunServer = grpc.ServerStreamingServer[ExecResponse] // Exec_ServiceDesc is the grpc.ServiceDesc for Exec service. // It's only intended for direct use with grpc.RegisterService, diff --git a/services/exec/server/exec.go b/services/exec/server/exec.go index b1a8d645..728fe0db 100644 --- a/services/exec/server/exec.go +++ b/services/exec/server/exec.go @@ -21,7 +21,9 @@ import ( "context" "io" "os/exec" + "os/user" "path/filepath" + "strconv" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -45,7 +47,26 @@ type server struct{} // Run executes command and returns result func (s *server) Run(ctx context.Context, req *pb.ExecRequest) (res *pb.ExecResponse, err error) { recorder := metrics.RecorderFromContextOrNoop(ctx) - run, err := util.RunCommand(ctx, req.Command, req.Args) + + var opts []util.Option + if req.User != "" { + u, err := user.Lookup(req.User) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "user '%s' not found:\n%v", req.User, err) + } + // This will work only on POSIX (Windows has non-decimal uids) yet these are our targets. + uid, err := strconv.Atoi(u.Uid) + if err != nil { + return nil, status.Errorf(codes.Internal, "'%s' user's uid %s failed to convert to numeric value:\n%v", req.User, u.Uid, err) + } + gid, err := strconv.Atoi(u.Gid) + if err != nil { + return nil, status.Errorf(codes.Internal, "'%s' user's gid %s failed to convert to numeric value:\n%v", req.User, u.Gid, err) + } + opts = append(opts, util.CommandUser(uint32(uid))) + opts = append(opts, util.CommandGroup(uint32(gid))) + } + run, err := util.RunCommand(ctx, req.Command, req.Args, opts...) if err != nil { recorder.CounterOrLog(ctx, execRunFailureCounter, 1) return nil, err diff --git a/services/exec/server/exec_test.go b/services/exec/server/exec_test.go index d4657a7c..1f17b2b6 100644 --- a/services/exec/server/exec_test.go +++ b/services/exec/server/exec_test.go @@ -86,6 +86,7 @@ func TestExec(t *testing.T) { name string bin string args []string + user string wantErr bool returnCodeNonZero bool stdout string @@ -111,6 +112,13 @@ func TestExec(t *testing.T) { bin: "foo", wantErr: true, }, + { + name: "user specified -- fails as it can't setuid", + bin: testutil.ResolvePath(t, "echo"), + args: []string{"hello world"}, + user: "nobody", + wantErr: true, + }, } { tc := tc t.Run(tc.name, func(t *testing.T) { @@ -118,6 +126,7 @@ func TestExec(t *testing.T) { resp, err := client.Run(ctx, &pb.ExecRequest{ Command: tc.bin, Args: tc.args, + User: tc.user, }) t.Logf("%s: resp: %+v", tc.name, resp) t.Logf("%s: err: %v", tc.name, err) From db4871eec2c54820ff6b05b61f0c44ed845e9a3c Mon Sep 17 00:00:00 2001 From: Marcin Walas Date: Thu, 14 Nov 2024 10:15:18 +0100 Subject: [PATCH 2/3] Respect user for streaming version of exec. --- services/exec/server/exec.go | 51 ++++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/services/exec/server/exec.go b/services/exec/server/exec.go index 728fe0db..455993fc 100644 --- a/services/exec/server/exec.go +++ b/services/exec/server/exec.go @@ -20,10 +20,12 @@ package server import ( "context" "io" + "os" "os/exec" "os/user" "path/filepath" "strconv" + "syscall" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -50,18 +52,9 @@ func (s *server) Run(ctx context.Context, req *pb.ExecRequest) (res *pb.ExecResp var opts []util.Option if req.User != "" { - u, err := user.Lookup(req.User) + uid, gid, err := resolveUser(req.User) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "user '%s' not found:\n%v", req.User, err) - } - // This will work only on POSIX (Windows has non-decimal uids) yet these are our targets. - uid, err := strconv.Atoi(u.Uid) - if err != nil { - return nil, status.Errorf(codes.Internal, "'%s' user's uid %s failed to convert to numeric value:\n%v", req.User, u.Uid, err) - } - gid, err := strconv.Atoi(u.Gid) - if err != nil { - return nil, status.Errorf(codes.Internal, "'%s' user's gid %s failed to convert to numeric value:\n%v", req.User, u.Gid, err) + return nil, err } opts = append(opts, util.CommandUser(uint32(uid))) opts = append(opts, util.CommandGroup(uint32(gid))) @@ -94,6 +87,24 @@ func (s *server) StreamingRun(req *pb.ExecRequest, stream pb.Exec_StreamingRunSe } cmd := exec.CommandContext(ctx, req.Command, req.Args...) + if req.User != "" { + gid, uid, err := resolveUser(req.User) + if err != nil { + return err + } + + // Set uid/gid if needed for the sub-process to run under. + // Only do this if it's different than our current ones since + // attempting to setuid/gid() to even your current values is EPERM. + if uid != uint32(os.Geteuid()) || gid != uint32(os.Getgid()) { + cmd.SysProcAttr = &syscall.SysProcAttr{ + Credential: &syscall.Credential{ + Uid: uid, + Gid: gid, + }, + } + } + } stdout, err := cmd.StdoutPipe() if err != nil { return err @@ -145,6 +156,24 @@ func (s *server) StreamingRun(req *pb.ExecRequest, stream pb.Exec_StreamingRunSe return err } +// resolveUser +func resolveUser(username string) (uint32, uint32, error) { + u, err := user.Lookup(username) + if err != nil { + return 0, 0, status.Errorf(codes.InvalidArgument, "user '%s' not found:\n%v", username, err) + } + // This will work only on POSIX (Windows has non-decimal uids) yet these are our targets. + uid, err := strconv.Atoi(u.Uid) + if err != nil { + return 0, 0, status.Errorf(codes.Internal, "'%s' user's uid %s failed to convert to numeric value:\n%v", username, u.Uid, err) + } + gid, err := strconv.Atoi(u.Gid) + if err != nil { + return 0, 0, status.Errorf(codes.Internal, "'%s' user's gid %s failed to convert to numeric value:\n%v", username, u.Gid, err) + } + return uint32(uid), uint32(gid), nil +} + // Register is called to expose this handler to the gRPC server func (s *server) Register(gs *grpc.Server) { pb.RegisterExecServer(gs, s) From 226f9f05c82abfa30572d87e224fcd6ed7042547 Mon Sep 17 00:00:00 2001 From: Marcin Walas Date: Thu, 14 Nov 2024 13:04:27 +0100 Subject: [PATCH 3/3] Fix exec uid vs gid --- services/exec/server/exec.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/exec/server/exec.go b/services/exec/server/exec.go index 455993fc..0ed9c400 100644 --- a/services/exec/server/exec.go +++ b/services/exec/server/exec.go @@ -88,7 +88,7 @@ func (s *server) StreamingRun(req *pb.ExecRequest, stream pb.Exec_StreamingRunSe cmd := exec.CommandContext(ctx, req.Command, req.Args...) if req.User != "" { - gid, uid, err := resolveUser(req.User) + uid, gid, err := resolveUser(req.User) if err != nil { return err } @@ -156,7 +156,7 @@ func (s *server) StreamingRun(req *pb.ExecRequest, stream pb.Exec_StreamingRunSe return err } -// resolveUser +// resolveUser retruns uid and gid of provided username. func resolveUser(username string) (uint32, uint32, error) { u, err := user.Lookup(username) if err != nil {