Replace fsgofer-host-uds flag

The new `host-uds` flag has the following differences:

  * More granular control: user can specify whether to ignore host UDS,
    connect to an existing host UDS, create a host UDS, or both
  * Will shortly be used to also control pipes from the host, hence the
    removal of UDS from the name
  * Can be used with other filesystems, not gofer-based, hence the
    removal of fsgofer from the name

This change also removes support for bind() from 9P based gofer since it's
not supported and would make this change more complicated.

Updates #8037

PiperOrigin-RevId: 483538832

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,

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 {

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
+}

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)

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.")

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",
     ],
 )

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: {},

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

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
-}

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) {

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
 	}
 

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.

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")