Skip to content

Commit 98485b1

Browse files
xianzhe-databricksgvisor-bot
authored andcommitted
Add a new RPC ConnectWithCreds to allow gofer to connect to a unix domain socket with application's credentials
Dear gvisor developers, Thank you very much for maintaining / developing gvisor! ## Motivation We had a use case (which I believe is a wide use case) that the sandboxes send requests over a unix domain socket on host, which is mapped to the container's file system and listened to by a server on the local host. The sandboxed application is started with a prescribed uid. To authenticate the request, the server verifies the request's uid. However, as the gofer process (which usually runs as root) executes [connect(unix_domain_socket) call](https://github.com/google/gvisor/blob/bd0cbf807169db29837d238209c02c816f3c8dbf/runsc/fsgofer/lisafs.go#L819) on behalf of the sandbox, the server always sees a uid 0. Hence the server cannot authenticate the UDS requests coming from the sandbox. ## Proposal I propose to Add a new RPC `ConnectWithCreds` to allow gofer to connect to a unix domain socket with application's credentials. On that gofer server thread, the euid/egid are temporarily changed to application's uid/gid and restored after the `connect(2)` call. ## Questions What do you think of this change? Is there any security/ functionality concern? Thank you so much for your feedback! FUTURE_COPYBARA_INTEGRATE_REVIEW=#11291 from xianzhe-databricks:fix-uds-auth c4f686f PiperOrigin-RevId: 712489714
1 parent bf9d60d commit 98485b1

File tree

12 files changed

+283
-55
lines changed

12 files changed

+283
-55
lines changed

images/basic/integrationtest/Dockerfile

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ RUN gcc -O2 -o tcp_server tcp_server.c
1818

1919
# Add nonprivileged regular user named "nonroot".
2020
RUN groupadd --gid 1337 nonroot && \
21-
useradd --uid 1337 --gid 1337 \
21+
useradd --uid 1338 --gid 1337 \
2222
--create-home \
2323
--shell $(which bash) \
2424
--password '' \
2525
nonroot
26+
27+
# Copy host_connect to /home/nonroot so that "nonroot" can execute it.
28+
RUN cp host_connect /home/nonroot/host_connect

pkg/lisafs/client_file.go

+26-7
Original file line numberDiff line numberDiff line change
@@ -467,13 +467,32 @@ func (f *ClientFD) BindAt(ctx context.Context, sockType linux.SockType, name str
467467
}
468468

469469
// Connect makes the Connect RPC.
470-
func (f *ClientFD) Connect(ctx context.Context, sockType linux.SockType) (int, error) {
471-
req := ConnectReq{FD: f.fd, SockType: uint32(sockType)}
472-
var resp ConnectResp
473-
var sockFD [1]int
474-
ctx.UninterruptibleSleepStart(false)
475-
err := f.client.SndRcvMessage(Connect, uint32(req.SizeBytes()), req.MarshalUnsafe, resp.CheckedUnmarshal, sockFD[:], req.String, resp.String)
476-
ctx.UninterruptibleSleepFinish(false)
470+
func (f *ClientFD) Connect(ctx context.Context, sockType linux.SockType, euid UID, egid GID) (int, error) {
471+
credsAvailable := euid != NoUID && egid != NoGID
472+
var (
473+
err error
474+
sockFD [1]int
475+
resp ConnectResp
476+
req = ConnectReq{
477+
FD: f.fd,
478+
SockType: uint32(sockType),
479+
}
480+
)
481+
if credsAvailable && f.client.IsSupported(ConnectWithCreds) {
482+
reqWithCreds := ConnectWithCredsReq{
483+
ConnectReq: req,
484+
UID: euid,
485+
GID: egid,
486+
}
487+
ctx.UninterruptibleSleepStart(false)
488+
err = f.client.SndRcvMessage(ConnectWithCreds, uint32(reqWithCreds.SizeBytes()), reqWithCreds.MarshalUnsafe, resp.CheckedUnmarshal, sockFD[:], reqWithCreds.String, resp.String)
489+
ctx.UninterruptibleSleepFinish(false)
490+
} else {
491+
ctx.UninterruptibleSleepStart(false)
492+
err = f.client.SndRcvMessage(Connect, uint32(req.SizeBytes()), req.MarshalUnsafe, resp.CheckedUnmarshal, sockFD[:], req.String, resp.String)
493+
ctx.UninterruptibleSleepFinish(false)
494+
}
495+
477496
if err == nil && sockFD[0] < 0 {
478497
err = unix.EBADF
479498
}

pkg/lisafs/fd.go

+7
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,13 @@ type ControlFDImpl interface {
484484
// On the server, Connect has a read concurrency guarantee.
485485
Connect(sockType uint32) (int, error)
486486

487+
// ConnectWithCreds is a wrapper around Connect but first changes the gofer's
488+
// euid and egid to the given uid and gid before calling Connect. It restores
489+
// the euid and egid after Connect.
490+
//
491+
// On the server, ConnectWithCreds has a read concurrency guarantee.
492+
ConnectWithCreds(sockType uint32, uid UID, gid GID) (int, error)
493+
487494
// BindAt creates a host unix domain socket of type sockType, bound to
488495
// the given namt of type sockType, bound to the given name. It returns
489496
// a ControlFD that can be used for path operations on the socket, a

pkg/lisafs/handlers.go

+63-32
Original file line numberDiff line numberDiff line change
@@ -46,38 +46,39 @@ const (
4646
type RPCHandler func(c *Connection, comm Communicator, payloadLen uint32) (uint32, error)
4747

4848
var handlers = [...]RPCHandler{
49-
Error: ErrorHandler,
50-
Mount: MountHandler,
51-
Channel: ChannelHandler,
52-
FStat: FStatHandler,
53-
SetStat: SetStatHandler,
54-
Walk: WalkHandler,
55-
WalkStat: WalkStatHandler,
56-
OpenAt: OpenAtHandler,
57-
OpenCreateAt: OpenCreateAtHandler,
58-
Close: CloseHandler,
59-
FSync: FSyncHandler,
60-
PWrite: PWriteHandler,
61-
PRead: PReadHandler,
62-
MkdirAt: MkdirAtHandler,
63-
MknodAt: MknodAtHandler,
64-
SymlinkAt: SymlinkAtHandler,
65-
LinkAt: LinkAtHandler,
66-
FStatFS: FStatFSHandler,
67-
FAllocate: FAllocateHandler,
68-
ReadLinkAt: ReadLinkAtHandler,
69-
Flush: FlushHandler,
70-
UnlinkAt: UnlinkAtHandler,
71-
RenameAt: RenameAtHandler,
72-
Getdents64: Getdents64Handler,
73-
FGetXattr: FGetXattrHandler,
74-
FSetXattr: FSetXattrHandler,
75-
FListXattr: FListXattrHandler,
76-
FRemoveXattr: FRemoveXattrHandler,
77-
Connect: ConnectHandler,
78-
BindAt: BindAtHandler,
79-
Listen: ListenHandler,
80-
Accept: AcceptHandler,
49+
Error: ErrorHandler,
50+
Mount: MountHandler,
51+
Channel: ChannelHandler,
52+
FStat: FStatHandler,
53+
SetStat: SetStatHandler,
54+
Walk: WalkHandler,
55+
WalkStat: WalkStatHandler,
56+
OpenAt: OpenAtHandler,
57+
OpenCreateAt: OpenCreateAtHandler,
58+
Close: CloseHandler,
59+
FSync: FSyncHandler,
60+
PWrite: PWriteHandler,
61+
PRead: PReadHandler,
62+
MkdirAt: MkdirAtHandler,
63+
MknodAt: MknodAtHandler,
64+
SymlinkAt: SymlinkAtHandler,
65+
LinkAt: LinkAtHandler,
66+
FStatFS: FStatFSHandler,
67+
FAllocate: FAllocateHandler,
68+
ReadLinkAt: ReadLinkAtHandler,
69+
Flush: FlushHandler,
70+
UnlinkAt: UnlinkAtHandler,
71+
RenameAt: RenameAtHandler,
72+
Getdents64: Getdents64Handler,
73+
FGetXattr: FGetXattrHandler,
74+
FSetXattr: FSetXattrHandler,
75+
FListXattr: FListXattrHandler,
76+
FRemoveXattr: FRemoveXattrHandler,
77+
Connect: ConnectHandler,
78+
BindAt: BindAtHandler,
79+
Listen: ListenHandler,
80+
Accept: AcceptHandler,
81+
ConnectWithCreds: ConnectWithCredsHandler,
8182
}
8283

8384
// ErrorHandler handles Error message.
@@ -1069,6 +1070,36 @@ func ConnectHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32
10691070
return 0, nil
10701071
}
10711072

1073+
// ConnectWithCredsHandler handles the ConnectWithCreds RPC.
1074+
func ConnectWithCredsHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32, error) {
1075+
var req ConnectWithCredsReq
1076+
if _, ok := req.CheckedUnmarshal(comm.PayloadBuf(payloadLen)); !ok {
1077+
return 0, unix.EIO
1078+
}
1079+
1080+
fd, err := c.lookupControlFD(req.FD)
1081+
if err != nil {
1082+
return 0, err
1083+
}
1084+
defer fd.DecRef(nil)
1085+
if !fd.IsSocket() {
1086+
return 0, unix.ENOTSOCK
1087+
}
1088+
var sock int
1089+
if err := fd.safelyRead(func() error {
1090+
if fd.node.isDeleted() {
1091+
return unix.EINVAL
1092+
}
1093+
sock, err = fd.impl.ConnectWithCreds(req.SockType, req.UID, req.GID)
1094+
return err
1095+
}); err != nil {
1096+
return 0, err
1097+
}
1098+
1099+
comm.DonateFD(sock)
1100+
return 0, nil
1101+
}
1102+
10721103
// BindAtHandler handles the BindAt RPC.
10731104
func BindAtHandler(c *Connection, comm Communicator, payloadLen uint32) (uint32, error) {
10741105
var req BindAtReq

pkg/lisafs/message.go

+19
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ const (
172172

173173
// Accept is analogous to accept4(2).
174174
Accept MID = 31
175+
176+
// ConnectWithCreds is analogous to connect(2) but it asks the server
177+
// to connect with the provided effective uid/gid.
178+
ConnectWithCreds MID = 32
175179
)
176180

177181
const (
@@ -1318,6 +1322,21 @@ func (*ConnectResp) String() string {
13181322
return "ConnectResp{}"
13191323
}
13201324

1325+
// ConnectWithCredsReq is used to make a ConnectWithCreds request. The response is also ConnectResp.
1326+
//
1327+
// +marshal boundCheck
1328+
type ConnectWithCredsReq struct {
1329+
ConnectReq
1330+
// UID and GID are used to specify the credentials to connect with.
1331+
UID UID
1332+
GID GID
1333+
}
1334+
1335+
// String implements fmt.Stringer.String.
1336+
func (c *ConnectWithCredsReq) String() string {
1337+
return fmt.Sprintf("ConnectWithCredsReq{FD: %d, SockType: %d, UID: %d, GID: %d}", c.FD, c.SockType, c.UID, c.GID)
1338+
}
1339+
13211340
// BindAtReq is used to make BindAt requests.
13221341
type BindAtReq struct {
13231342
createCommon

pkg/sentry/fsimpl/gofer/dentry_impl.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -452,11 +452,18 @@ func (d *dentry) allocate(ctx context.Context, mode, offset, length uint64) erro
452452
// - !d.isSynthetic().
453453
// - fs.renameMu is locked.
454454
func (d *dentry) connect(ctx context.Context, sockType linux.SockType) (int, error) {
455+
creds := auth.CredentialsOrNilFromContext(ctx)
456+
euid := lisafs.NoUID
457+
egid := lisafs.NoGID
458+
if creds != nil {
459+
euid = lisafs.UID(creds.EffectiveKUID)
460+
egid = lisafs.GID(creds.EffectiveKGID)
461+
}
455462
switch dt := d.impl.(type) {
456463
case *lisafsDentry:
457-
return dt.controlFD.Connect(ctx, sockType)
464+
return dt.controlFD.Connect(ctx, sockType, euid, egid)
458465
case *directfsDentry:
459-
return dt.connect(ctx, sockType)
466+
return dt.connect(ctx, sockType, euid, egid)
460467
default:
461468
panic("unknown dentry implementation")
462469
}

pkg/sentry/fsimpl/gofer/directfs_dentry.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -603,13 +603,13 @@ func (d *directfsDentry) getDirentsLocked(recordDirent func(name string, key ino
603603
}
604604

605605
// Precondition: fs.renameMu is locked.
606-
func (d *directfsDentry) connect(ctx context.Context, sockType linux.SockType) (int, error) {
606+
func (d *directfsDentry) connect(ctx context.Context, sockType linux.SockType, euid lisafs.UID, egid lisafs.GID) (int, error) {
607607
// There are no filesystems mounted in the sandbox process's mount namespace.
608608
// So we can't perform absolute path traversals. So fallback to using lisafs.
609609
if err := d.ensureLisafsControlFD(ctx); err != nil {
610610
return -1, err
611611
}
612-
return d.controlFDLisa.Connect(ctx, sockType)
612+
return d.controlFDLisa.Connect(ctx, sockType, euid, egid)
613613
}
614614

615615
func (d *directfsDentry) readlink() (string, error) {

pkg/sentry/kernel/auth/context.go

+9
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ func CredentialsFromContext(ctx context.Context) *Credentials {
3939
return NewAnonymousCredentials()
4040
}
4141

42+
// CredentialsOrNilFromContext returns a copy of the Credentials used by ctx,
43+
// or nil if ctx does not have Credentials.
44+
func CredentialsOrNilFromContext(ctx context.Context) *Credentials {
45+
if v := ctx.Value(CtxCredentials); v != nil {
46+
return v.(*Credentials)
47+
}
48+
return nil
49+
}
50+
4251
// ThreadGroupIDFromContext returns the current thread group ID when ctx
4352
// represents a task context.
4453
func ThreadGroupIDFromContext(ctx context.Context) (tgid int32, ok bool) {

runsc/cmd/gofer.go

+28-3
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ var caps = []string{
5353
"CAP_SYS_CHROOT",
5454
}
5555

56+
var udsOpenCaps = []string{
57+
"CAP_SETUID",
58+
"CAP_SETGID",
59+
}
60+
5661
// goferCaps is the minimal set of capabilities needed by the Gofer to operate
5762
// on files.
5863
var goferCaps = &specs.LinuxCapabilities{
@@ -61,6 +66,12 @@ var goferCaps = &specs.LinuxCapabilities{
6166
Permitted: caps,
6267
}
6368

69+
var goferUdsOpenCaps = &specs.LinuxCapabilities{
70+
Bounding: udsOpenCaps,
71+
Effective: udsOpenCaps,
72+
Permitted: udsOpenCaps,
73+
}
74+
6475
// goferSyncFDs contains file descriptors that are used for synchronization
6576
// of the Gofer startup process against other processes.
6677
type goferSyncFDs struct {
@@ -181,7 +192,11 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm
181192
overrides["apply-caps"] = "false"
182193
overrides["setup-root"] = "false"
183194
args := prepareArgs(g.Name(), f, overrides)
184-
util.Fatalf("setCapsAndCallSelf(%v, %v): %v", args, goferCaps, setCapsAndCallSelf(args, goferCaps))
195+
capsToApply := goferCaps
196+
if conf.GetHostUDS().AllowOpen() {
197+
capsToApply = specutils.MergeCapabilities(capsToApply, goferUdsOpenCaps)
198+
}
199+
util.Fatalf("setCapsAndCallSelf(%v, %v): %v", args, capsToApply, setCapsAndCallSelf(args, capsToApply))
185200
panic("unreachable")
186201
}
187202

@@ -253,6 +268,12 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm
253268
}
254269
log.Infof("Process chroot'd to %q", root)
255270

271+
ruid := unix.Getuid()
272+
euid := unix.Geteuid()
273+
rgid := unix.Getgid()
274+
egid := unix.Getegid()
275+
log.Debugf("Process running as uid=%d euid=%d gid=%d egid=%d", ruid, euid, rgid, egid)
276+
256277
// Initialize filters.
257278
opts := filter.Options{
258279
UDSOpenEnabled: conf.GetHostUDS().AllowOpen(),
@@ -265,7 +286,7 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm
265286
util.Fatalf("installing seccomp filters: %v", err)
266287
}
267288

268-
return g.serve(spec, conf, root)
289+
return g.serve(spec, conf, root, ruid, euid, rgid, egid)
269290
}
270291

271292
func newSocket(ioFD int) *unet.Socket {
@@ -276,7 +297,7 @@ func newSocket(ioFD int) *unet.Socket {
276297
return socket
277298
}
278299

279-
func (g *Gofer) serve(spec *specs.Spec, conf *config.Config, root string) subcommands.ExitStatus {
300+
func (g *Gofer) serve(spec *specs.Spec, conf *config.Config, root string, ruid int, euid int, rgid int, egid int) subcommands.ExitStatus {
280301
type connectionConfig struct {
281302
sock *unet.Socket
282303
mountPath string
@@ -289,6 +310,10 @@ func (g *Gofer) serve(spec *specs.Spec, conf *config.Config, root string) subcom
289310
HostUDS: conf.GetHostUDS(),
290311
HostFifo: conf.HostFifo,
291312
DonateMountPointFD: conf.DirectFS,
313+
RUID: ruid,
314+
EUID: euid,
315+
RGID: rgid,
316+
EGID: egid,
292317
})
293318

294319
ioFDs := g.ioFDs

runsc/fsgofer/filter/config.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,9 @@ var udsCommonSyscalls = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule
208208
})
209209

210210
var udsOpenSyscalls = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{
211-
unix.SYS_CONNECT: seccomp.MatchAll{},
211+
unix.SYS_CONNECT: seccomp.MatchAll{},
212+
unix.SYS_SETREUID: seccomp.MatchAll{},
213+
unix.SYS_SETREGID: seccomp.MatchAll{},
212214
})
213215

214216
var udsCreateSyscalls = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{

0 commit comments

Comments
 (0)