diff --git a/pkg/lisafs/testsuite/testsuite.go b/pkg/lisafs/testsuite/testsuite.go index 2c8645ba9..bf46e9786 100644 --- a/pkg/lisafs/testsuite/testsuite.go +++ b/pkg/lisafs/testsuite/testsuite.go @@ -64,7 +64,7 @@ func RunAllLocalFSTests(t *testing.T, tester Tester) { // TestFunc describes the signature of a test method. type TestFunc func(context.Context, *testing.T, Tester, lisafs.ClientFD) -var localFSTests map[string]TestFunc = map[string]TestFunc{ +var localFSTests = map[string]TestFunc{ "Stat": testStat, "RegularFileIO": testRegularFileIO, "RegularFileOpen": testRegularFileOpen, diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index a04d7a2ba..dbf9905f6 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -227,8 +227,9 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) // Initialize filters. opts := filter.Options{ - UDSEnabled: conf.FSGoferHostUDS, - ProfileEnabled: len(profileOpts) > 0, + UDSOpenEnabled: conf.GetHostUDS().AllowOpen(), + UDSCreateEnabled: conf.GetHostUDS().AllowCreate(), + ProfileEnabled: len(profileOpts) > 0, } if err := filter.Install(opts); err != nil { util.Fatalf("installing seccomp filters: %v", err) @@ -258,7 +259,7 @@ func (g *Gofer) serveLisafs(spec *specs.Spec, conf *config.Config, root string) server := fsgofer.NewLisafsServer(fsgofer.Config{ // These are global options. Ignore readonly configuration, that is set on // a per connection basis. - HostUDS: conf.FSGoferHostUDS, + HostUDS: conf.GetHostUDS(), }) // Start with root mount, then add any other additional mount as needed. @@ -318,7 +319,7 @@ func (g *Gofer) serve9P(spec *specs.Spec, conf *config.Config, root string) subc ats := make([]p9.Attacher, 0, len(spec.Mounts)+1) ap, err := fsgofer.NewAttachPoint("/", fsgofer.Config{ ROMount: spec.Root.Readonly || conf.Overlay, - HostUDS: conf.FSGoferHostUDS, + HostUDS: conf.GetHostUDS(), }) if err != nil { util.Fatalf("creating attach point: %v", err) @@ -331,7 +332,7 @@ func (g *Gofer) serve9P(spec *specs.Spec, conf *config.Config, root string) subc if specutils.IsGoferMount(m) { cfg := fsgofer.Config{ ROMount: isReadonlyMount(m.Options) || conf.Overlay, - HostUDS: conf.FSGoferHostUDS, + HostUDS: conf.GetHostUDS(), } ap, err := fsgofer.NewAttachPoint(m.Destination, cfg) if err != nil { diff --git a/runsc/config/config.go b/runsc/config/config.go index d12168a7d..cbbc98c84 100644 --- a/runsc/config/config.go +++ b/runsc/config/config.go @@ -76,10 +76,13 @@ type Config struct { // Overlay is whether to wrap the root filesystem in an overlay. Overlay bool `flag:"overlay"` - // FSGoferHostUDS enables the gofer to create and connect to host unix - // domain sockets. + // FSGoferHostUDS is deprecated: use host-uds=all. FSGoferHostUDS bool `flag:"fsgofer-host-uds"` + // HostUDS controls permission to access host Unix-domain sockets. + // DO NOT call it directly, use GetHostComm() instead. + HostUDS HostUDS `flag:"host-uds"` + // Network indicates what type of network to use. Network NetworkType `flag:"network"` @@ -283,9 +286,26 @@ func (c *Config) validate() error { if c.ProfileMutex != "" && !c.ProfileEnable { return fmt.Errorf("profile-mutex flag requires enabling profiling with profile flag") } + if c.FSGoferHostUDS && c.HostUDS != HostUDSNone { + // Deprecated flag was used together with flag that replaced it. + return fmt.Errorf("fsgofer-host-uds has been replaced with host-uds flag") + } return nil } +// GetHostUDS returns the FS gofer communication that is allowed, taking into +// consideration all flags what affect the result. +func (c *Config) GetHostUDS() HostUDS { + if c.FSGoferHostUDS { + if c.HostUDS != HostUDSNone { + panic(fmt.Sprintf("HostUDS cannot be set when --fsgofer-host-uds=true")) + } + // Using deprecated flag, honor it to avoid breaking users. + return HostUDSOpen + } + return c.HostUDS +} + // FileAccessType tells how the filesystem is accessed. type FileAccessType int @@ -442,3 +462,74 @@ func leakModePtr(v refs.LeakMode) *refs.LeakMode { func watchdogActionPtr(v watchdog.Action) *watchdog.Action { return &v } + +// HostUDS tells how much of the host UDS the file system has access to. +type HostUDS int + +const ( + // HostUDSNone doesn't allows UDS from the host to be manipulated. + HostUDSNone HostUDS = 0x0 + + // HostUDSOpen allows UDS from the host to be opened, e.g. connect(2). + HostUDSOpen HostUDS = 0x1 + + // HostUDSCreate allows UDS from the host to be created, e.g. bind(2). + HostUDSCreate HostUDS = 0x2 + + // HostUDSAll allows all form of communication with the host through UDS. + HostUDSAll = HostUDSOpen | HostUDSCreate +) + +func hostUDSPtr(v HostUDS) *HostUDS { + return &v +} + +// Set implements flag.Value. +func (g *HostUDS) Set(v string) error { + switch v { + case "", "none": + *g = HostUDSNone + case "open": + *g = HostUDSOpen + case "create": + *g = HostUDSCreate + case "all": + *g = HostUDSAll + default: + return fmt.Errorf("invalid host UDS type %q", v) + } + return nil +} + +// Get implements flag.Value. +func (g *HostUDS) Get() interface{} { + return *g +} + +// String implements flag.Value. +func (g HostUDS) String() string { + // Note: the order of operations is important given that HostUDS is a bitmap. + if g == HostUDSNone { + return "none" + } + if g == HostUDSAll { + return "all" + } + if g == HostUDSOpen { + return "open" + } + if g == HostUDSCreate { + return "create" + } + panic(fmt.Sprintf("Invalid host UDS type %d", g)) +} + +// AllowOpen returns true if it can consume UDS from the host. +func (g HostUDS) AllowOpen() bool { + return g&HostUDSOpen != 0 +} + +// AllowCreate returns true if it can create UDS in the host. +func (g HostUDS) AllowCreate() bool { + return g&HostUDSCreate != 0 +} diff --git a/runsc/config/config_test.go b/runsc/config/config_test.go index ad705479a..f67bb7592 100644 --- a/runsc/config/config_test.go +++ b/runsc/config/config_test.go @@ -137,6 +137,10 @@ func TestInvalidFlags(t *testing.T) { name: "ref-leak-mode", error: "invalid ref leak mode", }, + { + name: "host-uds", + error: "invalid host UDS", + }, } { t.Run(tc.name, func(t *testing.T) { testFlags := flag.NewFlagSet("test", flag.ContinueOnError) @@ -169,6 +173,30 @@ func TestValidationFail(t *testing.T) { }, error: "num_network_channels must be > 0", }, + { + name: "fsgofer-host-uds+comm:open", + flags: map[string]string{ + "fsgofer-host-uds": "true", + "host-uds": "open", + }, + error: "fsgofer-host-uds has been replaced with host-uds flag", + }, + { + name: "fsgofer-host-uds+comm:create", + flags: map[string]string{ + "fsgofer-host-uds": "true", + "host-uds": "create", + }, + error: "fsgofer-host-uds has been replaced with host-uds flag", + }, + { + name: "fsgofer-host-uds+comm:all", + flags: map[string]string{ + "fsgofer-host-uds": "true", + "host-uds": "all", + }, + error: "fsgofer-host-uds has been replaced with host-uds flag", + }, } { t.Run(tc.name, func(t *testing.T) { testFlags := flag.NewFlagSet("test", flag.ContinueOnError) diff --git a/runsc/config/flags.go b/runsc/config/flags.go index 2db0a5be8..f90f2b0f5 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -79,7 +79,9 @@ func RegisterFlags(flagSet *flag.FlagSet) { flagSet.Var(fileAccessTypePtr(FileAccessExclusive), "file-access", "specifies which filesystem validation to use for the root mount: exclusive (default), shared.") flagSet.Var(fileAccessTypePtr(FileAccessShared), "file-access-mounts", "specifies which filesystem validation to use for volumes other than the root mount: shared (default), exclusive.") flagSet.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") - flagSet.Bool("fsgofer-host-uds", false, "allow the gofer to mount Unix Domain Sockets.") + flagSet.Bool("fsgofer-host-uds", false, "DEPRECATED: use host-uds=all") + flagSet.Var(hostUDSPtr(0), "host-uds", "controls permission to access host Unix-domain sockets. Values: none|open|create|all, default: none") + flagSet.Bool("vfs2", true, "DEPRECATED: this flag has no effect.") flagSet.Bool("fuse", false, "TEST ONLY; use while FUSE in VFSv2 is landing. This allows the use of the new experimental FUSE filesystem.") flagSet.Bool("lisafs", true, "Enables lisafs protocol instead of 9P.") diff --git a/runsc/fsgofer/BUILD b/runsc/fsgofer/BUILD index a7ac16bac..a02c201b1 100644 --- a/runsc/fsgofer/BUILD +++ b/runsc/fsgofer/BUILD @@ -23,6 +23,7 @@ go_library( "//pkg/p9", "//pkg/sync", "//pkg/syserr", + "//runsc/config", "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/runsc/fsgofer/filter/config.go b/runsc/fsgofer/filter/config.go index 9c91149da..9e3db2703 100644 --- a/runsc/fsgofer/filter/config.go +++ b/runsc/fsgofer/filter/config.go @@ -212,11 +212,7 @@ var allowedSyscalls = seccomp.SyscallRules{ unix.SYS_WRITE: {}, } -var udsSyscalls = seccomp.SyscallRules{ - unix.SYS_ACCEPT4: {}, - unix.SYS_BIND: {}, - unix.SYS_CONNECT: {}, - unix.SYS_LISTEN: {}, +var udsCommonSyscalls = seccomp.SyscallRules{ unix.SYS_SOCKET: []seccomp.Rule{ { seccomp.EqualTo(unix.AF_UNIX), @@ -236,6 +232,16 @@ var udsSyscalls = seccomp.SyscallRules{ }, } +var udsOpenSyscalls = seccomp.SyscallRules{ + unix.SYS_CONNECT: {}, +} + +var udsCreateSyscalls = seccomp.SyscallRules{ + unix.SYS_ACCEPT4: {}, + unix.SYS_BIND: {}, + unix.SYS_LISTEN: {}, +} + var xattrSyscalls = seccomp.SyscallRules{ unix.SYS_FGETXATTR: {}, unix.SYS_FSETXATTR: {}, diff --git a/runsc/fsgofer/filter/filter.go b/runsc/fsgofer/filter/filter.go index 8665c548b..9e03f9d91 100644 --- a/runsc/fsgofer/filter/filter.go +++ b/runsc/fsgofer/filter/filter.go @@ -24,8 +24,9 @@ import ( // Options are seccomp filter related options. type Options struct { - UDSEnabled bool - ProfileEnabled bool + UDSOpenEnabled bool + UDSCreateEnabled bool + ProfileEnabled bool } // Install installs seccomp filters. @@ -37,9 +38,15 @@ func Install(opt Options) error { s.Merge(profileFilters) } - if opt.UDSEnabled { + if opt.UDSOpenEnabled || opt.UDSCreateEnabled { report("host UDS enabled: syscall filters less restrictive!") - s.Merge(udsSyscalls) + s.Merge(udsCommonSyscalls) + if opt.UDSOpenEnabled { + s.Merge(udsOpenSyscalls) + } + if opt.UDSCreateEnabled { + s.Merge(udsCreateSyscalls) + } } // Set of additional filters used by -race and -msan. Returns empty diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index 4f37b60b3..ac06deb06 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -37,6 +37,7 @@ import ( "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/p9" "gvisor.dev/gvisor/pkg/sync" + "gvisor.dev/gvisor/runsc/config" ) const ( @@ -62,9 +63,8 @@ type Config struct { // PanicOnWrite panics on attempts to write to RO mounts. PanicOnWrite bool - // HostUDS signals whether the gofer can create and connect to host - // unix domain sockets. - HostUDS bool + // HostUDS signals whether the gofer can connect to host unix domain sockets. + HostUDS config.HostUDS } type attachPoint struct { @@ -315,13 +315,13 @@ func openAnyFile(pathDebug string, fn func(mode int) (*fd.FD, error)) (*fd.FD, b return nil, false, extractErrno(err) } -func checkSupportedFileType(mode uint32, permitSocket bool) error { +func checkSupportedFileType(mode uint32, hostComm config.HostUDS) error { switch mode & unix.S_IFMT { case unix.S_IFREG, unix.S_IFDIR, unix.S_IFLNK: return nil case unix.S_IFSOCK: - if !permitSocket { + if !hostComm.AllowOpen() { return unix.EPERM } return nil @@ -1110,76 +1110,12 @@ func (l *localFile) Flush() error { // Bind implements p9.File. func (l *localFile) Bind(sockType uint32, sockName string, uid p9.UID, gid p9.GID) (p9.File, p9.QID, p9.AttrMask, p9.Attr, error) { - if !l.attachPoint.conf.HostUDS { - // Bind on host UDS is not allowed. As per mknod(2), which is invoked as - // part of bind(2), if "the filesystem containing pathname does not support - // the type of node requested." then EPERM must be returned. - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, unix.EPERM - } - - // Create socket only for supported types. - if !isSockTypeSupported(sockType) { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, unix.ENXIO - } - sock, err := unix.Socket(unix.AF_UNIX, int(sockType), 0) - if err != nil { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, extractErrno(err) - } - - // Because there is no "bindat" syscall in Linux, we must create an - // absolute path to the socket we are creating. But go through /proc/self/fd - // to avoid host path walk of the entire host path. It also helps avoid the - // UNIX_PATH_MAX bytes limit on the path, in case path is too long. - sockPath := filepath.Join("/proc/self/fd", strconv.Itoa(l.file.FD()), sockName) - - // Revert operations on error paths. - didBind := false - cu := cleanup.Make(func() { - _ = unix.Close(sock) - if didBind { - if err := unix.Unlinkat(l.file.FD(), sockName, 0); err != nil { - log.Warningf("error unlinking file %q after failure: %v", sockPath, err) - } - } - }) - defer cu.Clean() - - // socket FD must be non blocking because RPC operations like Accept on this - // socket must be non blocking. - if err := unix.SetNonblock(sock, true); err != nil { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, extractErrno(err) - } - - // Bind at the given path which should create the socket file. - if err := unix.Bind(sock, &unix.SockaddrUnix{Name: sockPath}); err != nil { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, extractErrno(err) - } - didBind = true - - // Open socket to change ownership. - tempSockFD, err := fd.OpenAt(l.file, sockName, unix.O_PATH|openFlags, 0) - if err != nil { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, extractErrno(err) - } - defer tempSockFD.Close() - - if _, err = setOwnerIfNeeded(tempSockFD.FD(), uid, gid); err != nil { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, extractErrno(err) - } - - // Generate file for this socket by walking on it. - qid, sockF, valid, attr, err := l.WalkGetAttr([]string{sockName}) - if err != nil { - return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, err - } - - cu.Release() - return &socketLocalFile{localFile: sockF.(*localFile), sock: sock}, qid[0], valid, attr, nil + return nil, p9.QID{}, p9.AttrMask{}, p9.Attr{}, unix.EPERM } // Connect implements p9.File. func (l *localFile) Connect(socketType p9.SocketType) (*fd.FD, error) { - if !l.attachPoint.conf.HostUDS { + if !l.attachPoint.conf.HostUDS.AllowOpen() { return nil, unix.ECONNREFUSED } @@ -1332,22 +1268,3 @@ func (l *localFile) MultiGetAttr(names []string) ([]p9.FullStat, error) { } return stats, nil } - -// socketLocalFile is an extension of localFile which is only created via Bind -// and additionally implements Listen and Accept. It also tracks the lifecycle -// of the socket FD created by socket(2) in addition to the FD opened on the -// socket file itself. -type socketLocalFile struct { - *localFile - sock int -} - -// Close implements p9.File. -func (l *socketLocalFile) Close() error { - err := l.localFile.Close() - err2 := unix.Close(l.sock) - if err != nil { - return err - } - return err2 -} diff --git a/runsc/fsgofer/fsgofer_test.go b/runsc/fsgofer/fsgofer_test.go index 9889807f0..63fea29a3 100644 --- a/runsc/fsgofer/fsgofer_test.go +++ b/runsc/fsgofer/fsgofer_test.go @@ -785,47 +785,6 @@ func TestReaddir(t *testing.T) { }) } -func TestUDS(t *testing.T) { - config := Config{ROMount: false, HostUDS: true} - dir, err := ioutil.TempDir("", "root-") - if err != nil { - t.Fatalf("ioutil.TempDir() failed, err: %v", err) - } - defer os.RemoveAll(dir) - - // First attach with writable configuration to setup tree. - a, err := NewAttachPoint(dir, config) - if err != nil { - t.Fatalf("NewAttachPoint failed: %v", err) - } - root, err := a.Attach() - if err != nil { - t.Fatalf("attach failed, err: %v", err) - } - defer root.Close() - - name := "sock" - uid := p9.UID(os.Getuid()) - gid := p9.GID(os.Getgid()) - sockF, _, valid, attr, err := root.Bind(unix.SOCK_STREAM, name, uid, gid) - if err != nil { - t.Fatalf("Bind failed: %v", err) - } - defer sockF.Close() - - if valid.Mode && !attr.Mode.IsSocket() { - t.Errorf("socket file mode is incorrect: want %d, got %d", p9.ModeSocket, attr.Mode) - } - if valid.UID && attr.UID != uid { - t.Errorf("socket file uid is incorrect: want %d, got %d", uid, attr.UID) - } - if valid.GID && attr.GID != gid { - t.Errorf("socket file gid is incorrect: want %d, got %d", gid, attr.GID) - } - // TODO(b/194709873): Once listen and accept are implemented, test connecting - // and accepting a connection using sockF. -} - // Test that attach point can be written to when it points to a file, e.g. // /etc/hosts. func TestAttachFile(t *testing.T) { diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go index b3998816c..d16e5c4ac 100644 --- a/runsc/fsgofer/lisafs.go +++ b/runsc/fsgofer/lisafs.go @@ -673,7 +673,7 @@ func isSockTypeSupported(sockType uint32) bool { // Connect implements lisafs.ControlFDImpl.Connect. func (fd *controlFDLisa) Connect(sockType uint32) (int, error) { - if !fd.Conn().ServerImpl().(*LisafsServer).config.HostUDS { + if !fd.Conn().ServerImpl().(*LisafsServer).config.HostUDS.AllowOpen() { return -1, unix.EPERM } @@ -699,7 +699,7 @@ func (fd *controlFDLisa) Connect(sockType uint32) (int, error) { // BindAt implements lisafs.ControlFDImpl.BindAt. func (fd *controlFDLisa) BindAt(name string, sockType uint32, mode linux.FileMode, uid lisafs.UID, gid lisafs.GID) (*lisafs.ControlFD, linux.Statx, *lisafs.BoundSocketFD, int, error) { - if !fd.Conn().ServerImpl().(*LisafsServer).config.HostUDS { + if !fd.Conn().ServerImpl().(*LisafsServer).config.HostUDS.AllowCreate() { return nil, linux.Statx{}, nil, -1, unix.EPERM } diff --git a/runsc/fsgofer/lisafs_test.go b/runsc/fsgofer/lisafs_test.go index 77bba9691..1b0e3fa64 100644 --- a/runsc/fsgofer/lisafs_test.go +++ b/runsc/fsgofer/lisafs_test.go @@ -38,7 +38,7 @@ type tester struct{} // NewServer implements testsuite.Tester.NewServer. func (tester) NewServer(t *testing.T) *lisafs.Server { - return &fsgofer.NewLisafsServer(fsgofer.Config{HostUDS: true}).Server + return &fsgofer.NewLisafsServer(fsgofer.Config{}).Server } // LinkSupported implements testsuite.Tester.LinkSupported. diff --git a/test/runner/main.go b/test/runner/main.go index 953377777..45d0158cf 100644 --- a/test/runner/main.go +++ b/test/runner/main.go @@ -218,7 +218,7 @@ func runRunsc(tc *gtest.TestCase, spec *specs.Spec) error { args = append(args, "-strace") } if *addUDSTree { - args = append(args, "-fsgofer-host-uds") + args = append(args, "-host-uds=all") } if *leakCheck { args = append(args, "-ref-leak-mode=log-names")