diff --git a/guest/boot/boot.go b/guest/boot/boot.go index 217a4de..699d789 100644 --- a/guest/boot/boot.go +++ b/guest/boot/boot.go @@ -64,6 +64,7 @@ func Run(logger *slog.Logger, opts ...Option) (shutdown func(), err error) { cfg.workspaceUID, cfg.workspaceGID, cfg.mountRetries, + cfg.workspaceReadOnly, ); err != nil { logger.Warn("workspace mount failed, continuing without workspace", "error", err) } diff --git a/guest/boot/boot_test.go b/guest/boot/boot_test.go index c107ca2..5eeda7e 100644 --- a/guest/boot/boot_test.go +++ b/guest/boot/boot_test.go @@ -26,6 +26,7 @@ func TestDefaultConfig(t *testing.T) { assert.Equal(t, "workspace", cfg.workspaceTag) assert.Equal(t, 1000, cfg.workspaceUID) assert.Equal(t, 1000, cfg.workspaceGID) + assert.False(t, cfg.workspaceReadOnly) assert.Equal(t, 5, cfg.mountRetries) assert.Equal(t, 22, cfg.sshPort) assert.Equal(t, "/home/sandbox/.ssh/authorized_keys", cfg.sshKeysPath) @@ -38,6 +39,18 @@ func TestDefaultConfig(t *testing.T) { assert.True(t, cfg.lockdownRoot) } +func TestWithWorkspaceReadOnly(t *testing.T) { + t.Parallel() + cfg := defaultConfig() + assert.False(t, cfg.workspaceReadOnly) + + WithWorkspaceReadOnly(true).apply(cfg) + assert.True(t, cfg.workspaceReadOnly) + + WithWorkspaceReadOnly(false).apply(cfg) + assert.False(t, cfg.workspaceReadOnly) +} + func TestWithWorkspace(t *testing.T) { t.Parallel() cfg := defaultConfig() diff --git a/guest/boot/options.go b/guest/boot/options.go index e6c3555..2fe7454 100644 --- a/guest/boot/options.go +++ b/guest/boot/options.go @@ -20,6 +20,7 @@ type config struct { workspaceTag string workspaceUID int workspaceGID int + workspaceReadOnly bool mountRetries int sshPort int sshKeysPath string @@ -66,6 +67,12 @@ func WithWorkspace(mountPoint, tag string, uid, gid int) Option { }) } +// WithWorkspaceReadOnly makes the workspace virtiofs mount read-only inside the +// guest. The mount is performed with MS_RDONLY so guest processes cannot write. +func WithWorkspaceReadOnly(readOnly bool) Option { + return optionFunc(func(c *config) { c.workspaceReadOnly = readOnly }) +} + // WithMountRetries sets the maximum number of retries for workspace mount. func WithMountRetries(n int) Option { return optionFunc(func(c *config) { c.mountRetries = n }) diff --git a/guest/mount/mount.go b/guest/mount/mount.go index 661d04c..3b530f2 100644 --- a/guest/mount/mount.go +++ b/guest/mount/mount.go @@ -88,18 +88,32 @@ func Essential(logger *slog.Logger, tmpSizeMiB uint32) error { // Workspace mounts a virtiofs share at the given mount point, retrying up to // maxRetries times to allow the host to expose the filesystem. On success the -// mount point is chowned to uid:gid. -func Workspace(logger *slog.Logger, mountPoint, tag string, uid, gid, maxRetries int) error { +// mount point is chowned to uid:gid. When readOnly is true the mount is +// performed with MS_RDONLY so the guest cannot write to it. +func Workspace(logger *slog.Logger, mountPoint, tag string, uid, gid, maxRetries int, readOnly bool) error { if err := os.MkdirAll(mountPoint, 0o755); err != nil { return fmt.Errorf("creating workspace mount point %s: %w", mountPoint, err) } + flags := uintptr(syscall.MS_NOSUID | syscall.MS_NODEV) + if readOnly { + flags |= syscall.MS_RDONLY + } + var lastErr error for i := range maxRetries { - lastErr = syscall.Mount(tag, mountPoint, "virtiofs", syscall.MS_NOSUID|syscall.MS_NODEV, "") + lastErr = syscall.Mount(tag, mountPoint, "virtiofs", flags, "") if lastErr == nil { - if err := os.Chown(mountPoint, uid, gid); err != nil { - return fmt.Errorf("chown workspace %s: %w", mountPoint, err) + // Skip chown on read-only mounts: chown returns EROFS on a + // filesystem mounted with MS_RDONLY. Ownership is cosmetic + // anyway since the mount prevents writes regardless. + if !readOnly { + if err := os.Chown(mountPoint, uid, gid); err != nil { + // Clean up the mount so we don't leave a mounted + // filesystem that the caller thinks failed. + _ = syscall.Unmount(mountPoint, 0) + return fmt.Errorf("chown workspace %s: %w", mountPoint, err) + } } return nil } diff --git a/guest/mount/mount_test.go b/guest/mount/mount_test.go index a88e8e0..cb4afe8 100644 --- a/guest/mount/mount_test.go +++ b/guest/mount/mount_test.go @@ -27,7 +27,7 @@ func TestWorkspaceReturnsErrorForInvalidMount(t *testing.T) { if os.Getuid() == 0 { t.Skip("test must run as non-root") } - err := Workspace(slog.Default(), t.TempDir()+"/ws", "nonexistent-tag", 1000, 1000, 1) + err := Workspace(slog.Default(), t.TempDir()+"/ws", "nonexistent-tag", 1000, 1000, 1, false) assert.Error(t, err) } diff --git a/guest/vmconfig/vmconfig.go b/guest/vmconfig/vmconfig.go index 644e123..28be896 100644 --- a/guest/vmconfig/vmconfig.go +++ b/guest/vmconfig/vmconfig.go @@ -19,6 +19,17 @@ type Config struct { // TmpSizeMiB is the size of the /tmp tmpfs in MiB. Zero means use the // mount package default (256 MiB). TmpSizeMiB uint32 `json:"tmp_size_mib,omitempty"` + // VirtioFSMounts describes virtiofs mounts the host configured for the VM. + // The guest init uses this to determine mount-time options (e.g. read-only). + VirtioFSMounts []VirtioFSMountInfo `json:"virtiofs_mounts,omitempty"` +} + +// VirtioFSMountInfo carries mount metadata from the host to the guest init. +type VirtioFSMountInfo struct { + // Tag is the virtiofs tag identifying this mount. + Tag string `json:"tag"` + // ReadOnly indicates the mount should be read-only inside the guest. + ReadOnly bool `json:"read_only,omitempty"` } // Read loads the VM config from /etc/go-microvm.json. diff --git a/guest/vmconfig/vmconfig_test.go b/guest/vmconfig/vmconfig_test.go index d3a08f4..934c108 100644 --- a/guest/vmconfig/vmconfig_test.go +++ b/guest/vmconfig/vmconfig_test.go @@ -31,6 +31,14 @@ func TestReadFrom(t *testing.T) { content: strPtr(`{"tmp_size_mib":512}`), want: Config{TmpSizeMiB: 512}, }, + { + name: "virtiofs mounts with read-only", + content: strPtr(`{"virtiofs_mounts":[{"tag":"workspace","read_only":true},{"tag":"data"}]}`), + want: Config{VirtioFSMounts: []VirtioFSMountInfo{ + {Tag: "workspace", ReadOnly: true}, + {Tag: "data"}, + }}, + }, { name: "empty JSON object", content: strPtr(`{}`), diff --git a/hooks/hooks_test.go b/hooks/hooks_test.go index 7e25e69..58ca93f 100644 --- a/hooks/hooks_test.go +++ b/hooks/hooks_test.go @@ -59,6 +59,14 @@ func TestInjectVMConfig(t *testing.T) { name: "zero-value config", cfg: vmconfig.Config{}, }, + { + name: "config with read-only mounts", + cfg: vmconfig.Config{ + VirtioFSMounts: []vmconfig.VirtioFSMountInfo{ + {Tag: "workspace", ReadOnly: true}, + }, + }, + }, } for _, tt := range tests { diff --git a/hypervisor/backend.go b/hypervisor/backend.go index c1b1d1c..84d43d3 100644 --- a/hypervisor/backend.go +++ b/hypervisor/backend.go @@ -90,4 +90,5 @@ type PortForward struct { type FilesystemMount struct { Tag string HostPath string + ReadOnly bool } diff --git a/hypervisor/libkrun/backend.go b/hypervisor/libkrun/backend.go index 69f3f7b..15e6ee1 100644 --- a/hypervisor/libkrun/backend.go +++ b/hypervisor/libkrun/backend.go @@ -187,7 +187,7 @@ func toRunnerPortForwards(ports []hypervisor.PortForward) []runner.PortForward { func toRunnerVirtioFS(mounts []hypervisor.FilesystemMount) []runner.VirtioFSMount { out := make([]runner.VirtioFSMount, len(mounts)) for i, m := range mounts { - out[i] = runner.VirtioFSMount{Tag: m.Tag, HostPath: m.HostPath} + out[i] = runner.VirtioFSMount{Tag: m.Tag, HostPath: m.HostPath, ReadOnly: m.ReadOnly} } return out } diff --git a/hypervisor/libkrun/backend_test.go b/hypervisor/libkrun/backend_test.go index aa48e65..b100a15 100644 --- a/hypervisor/libkrun/backend_test.go +++ b/hypervisor/libkrun/backend_test.go @@ -117,6 +117,7 @@ func TestBackend_Start_Success(t *testing.T) { }, FilesystemMounts: []hypervisor.FilesystemMount{ {Tag: "workspace", HostPath: "/tmp/src"}, + {Tag: "data", HostPath: "/tmp/data", ReadOnly: true}, }, NetEndpoint: hypervisor.NetEndpoint{ Type: hypervisor.NetEndpointUnixSocket, diff --git a/microvm.go b/microvm.go index 164e19e..8f39c03 100644 --- a/microvm.go +++ b/microvm.go @@ -125,11 +125,12 @@ func Run(ctx context.Context, imageRef string, opts ...Option) (*VM, error) { } } - // 3b. Inject VM config for the guest init (e.g. /tmp size). - // Only written when a non-default value is configured, keeping the - // file absent for callers that rely on the built-in 256 MiB default. - if cfg.tmpSizeMiB > 0 { - vmCfgHook := hooks.InjectVMConfig(vmconfig.Config{TmpSizeMiB: cfg.tmpSizeMiB}) + // 3b. Inject VM config for the guest init (e.g. /tmp size, mount flags). + // Only written when non-default values are configured, keeping the + // file absent for callers that rely on built-in defaults. + guestVMCfg := buildVMConfig(cfg) + if guestVMCfg.TmpSizeMiB > 0 || len(guestVMCfg.VirtioFSMounts) > 0 { + vmCfgHook := hooks.InjectVMConfig(guestVMCfg) if err := vmCfgHook(rootfs.Path, rootfs.Config); err != nil { return nil, fmt.Errorf("inject vm config: %w", err) } @@ -395,10 +396,24 @@ func toHypervisorPorts(ports []PortForward) []hypervisor.PortForward { return out } +func buildVMConfig(cfg *config) vmconfig.Config { + var vc vmconfig.Config + vc.TmpSizeMiB = cfg.tmpSizeMiB + for _, m := range cfg.virtioFS { + if m.ReadOnly { + vc.VirtioFSMounts = append(vc.VirtioFSMounts, vmconfig.VirtioFSMountInfo{ + Tag: m.Tag, + ReadOnly: true, + }) + } + } + return vc +} + func toHypervisorMounts(mounts []VirtioFSMount) []hypervisor.FilesystemMount { out := make([]hypervisor.FilesystemMount, len(mounts)) for i, m := range mounts { - out[i] = hypervisor.FilesystemMount{Tag: m.Tag, HostPath: m.HostPath} + out[i] = hypervisor.FilesystemMount{Tag: m.Tag, HostPath: m.HostPath, ReadOnly: m.ReadOnly} } return out } diff --git a/microvm_test.go b/microvm_test.go index d472060..8168024 100644 --- a/microvm_test.go +++ b/microvm_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/stacklok/go-microvm/guest/vmconfig" "github.com/stacklok/go-microvm/hypervisor" "github.com/stacklok/go-microvm/image" "github.com/stacklok/go-microvm/internal/testutil" @@ -92,7 +93,7 @@ func TestToHypervisorMounts(t *testing.T) { mounts := []VirtioFSMount{ {Tag: "workspace", HostPath: "/home/user/src"}, - {Tag: "data", HostPath: "/var/data"}, + {Tag: "data", HostPath: "/var/data", ReadOnly: true}, } result := toHypervisorMounts(mounts) @@ -100,8 +101,10 @@ func TestToHypervisorMounts(t *testing.T) { require.Len(t, result, 2) assert.Equal(t, "workspace", result[0].Tag) assert.Equal(t, "/home/user/src", result[0].HostPath) + assert.False(t, result[0].ReadOnly) assert.Equal(t, "data", result[1].Tag) assert.Equal(t, "/var/data", result[1].HostPath) + assert.True(t, result[1].ReadOnly) } func TestToHypervisorMounts_Empty(t *testing.T) { @@ -111,6 +114,40 @@ func TestToHypervisorMounts_Empty(t *testing.T) { assert.Empty(t, result) } +func TestBuildVMConfig(t *testing.T) { + t.Parallel() + + t.Run("empty config produces zero value", func(t *testing.T) { + t.Parallel() + cfg := defaultConfig() + vc := buildVMConfig(cfg) + assert.Zero(t, vc.TmpSizeMiB) + assert.Empty(t, vc.VirtioFSMounts) + }) + + t.Run("only read-only mounts are included", func(t *testing.T) { + t.Parallel() + cfg := defaultConfig() + cfg.virtioFS = []VirtioFSMount{ + {Tag: "workspace", HostPath: "/src"}, + {Tag: "data", HostPath: "/data", ReadOnly: true}, + {Tag: "config", HostPath: "/cfg", ReadOnly: true}, + } + vc := buildVMConfig(cfg) + require.Len(t, vc.VirtioFSMounts, 2) + assert.Equal(t, vmconfig.VirtioFSMountInfo{Tag: "data", ReadOnly: true}, vc.VirtioFSMounts[0]) + assert.Equal(t, vmconfig.VirtioFSMountInfo{Tag: "config", ReadOnly: true}, vc.VirtioFSMounts[1]) + }) + + t.Run("tmpSizeMiB is propagated", func(t *testing.T) { + t.Parallel() + cfg := defaultConfig() + cfg.tmpSizeMiB = 512 + vc := buildVMConfig(cfg) + assert.Equal(t, uint32(512), vc.TmpSizeMiB) + }) +} + // --- Mock types for Run() tests --- // mockImageFetcher implements image.ImageFetcher for testing. diff --git a/options.go b/options.go index e262ec2..ed3495b 100644 --- a/options.go +++ b/options.go @@ -42,6 +42,11 @@ type PortForward struct { type VirtioFSMount struct { Tag string HostPath string + // ReadOnly makes the mount read-only inside the guest. Enforcement is + // guest-side via MS_RDONLY mount flags; libkrun does not currently + // support host-side read-only virtiofs. A compromised guest kernel + // could bypass this restriction. + ReadOnly bool } // EgressPolicy restricts outbound VM traffic to specific DNS hostnames. diff --git a/options_test.go b/options_test.go index 914ac96..cf158b8 100644 --- a/options_test.go +++ b/options_test.go @@ -145,6 +145,21 @@ func TestWithVirtioFS(t *testing.T) { require.Len(t, cfg.virtioFS, 1) assert.Equal(t, "workspace", cfg.virtioFS[0].Tag) assert.Equal(t, "/home/user/src", cfg.virtioFS[0].HostPath) + assert.False(t, cfg.virtioFS[0].ReadOnly) +} + +func TestWithVirtioFS_ReadOnly(t *testing.T) { + t.Parallel() + + cfg := defaultConfig() + WithVirtioFS( + VirtioFSMount{Tag: "data", HostPath: "/var/data", ReadOnly: true}, + ).apply(cfg) + + require.Len(t, cfg.virtioFS, 1) + assert.Equal(t, "data", cfg.virtioFS[0].Tag) + assert.Equal(t, "/var/data", cfg.virtioFS[0].HostPath) + assert.True(t, cfg.virtioFS[0].ReadOnly) } func TestWithVirtioFS_Appends(t *testing.T) { diff --git a/runner/cmd/go-microvm-runner/main.go b/runner/cmd/go-microvm-runner/main.go index 3a41d6b..0a8bbce 100644 --- a/runner/cmd/go-microvm-runner/main.go +++ b/runner/cmd/go-microvm-runner/main.go @@ -72,6 +72,8 @@ type VirtioFSMount struct { Tag string `json:"tag"` // Path is the host directory path. Path string `json:"path"` + // ReadOnly makes the mount read-only inside the guest. + ReadOnly bool `json:"read_only,omitempty"` } // Exit codes for the runner binary. @@ -201,7 +203,12 @@ func runVM(config *Config) error { } // Configure virtio-fs mounts. + // Note: libkrun's krun_add_virtiofs does not support a read-only flag. + // ReadOnly enforcement happens guest-side via MS_RDONLY mount flags. for _, mount := range config.VirtioFSMounts { + if mount.ReadOnly { + fmt.Fprintf(os.Stderr, "Warning: virtiofs mount %q is read-only but libkrun has no host-side enforcement; relying on guest-side MS_RDONLY\n", mount.Tag) + } if err := ctx.AddVirtioFS(mount.Tag, mount.Path); err != nil { _ = ctx.Free() return fmt.Errorf("add virtiofs mount %s: %w", mount.Tag, err) diff --git a/runner/config.go b/runner/config.go index 7aa2b86..f2e9de8 100644 --- a/runner/config.go +++ b/runner/config.go @@ -57,4 +57,6 @@ type VirtioFSMount struct { Tag string `json:"tag"` // HostPath is the host directory path. HostPath string `json:"path"` + // ReadOnly makes the mount read-only inside the guest. + ReadOnly bool `json:"read_only,omitempty"` } diff --git a/runner/config_test.go b/runner/config_test.go index b164ba9..bdc133c 100644 --- a/runner/config_test.go +++ b/runner/config_test.go @@ -15,11 +15,14 @@ func TestConfig_MarshalUnmarshal_RoundTrip(t *testing.T) { t.Parallel() original := Config{ - RootPath: "/var/lib/go-microvm/rootfs", - NumVCPUs: 4, - RAMMiB: 1024, - NetSocket: "/tmp/net.sock", - VirtioFS: []VirtioFSMount{{Tag: "shared", HostPath: "/home/user/data"}}, + RootPath: "/var/lib/go-microvm/rootfs", + NumVCPUs: 4, + RAMMiB: 1024, + NetSocket: "/tmp/net.sock", + VirtioFS: []VirtioFSMount{ + {Tag: "shared", HostPath: "/home/user/data"}, + {Tag: "readonly-data", HostPath: "/var/data", ReadOnly: true}, + }, ConsoleLog: "/var/log/console.log", LogLevel: 3, // These should NOT appear in JSON. @@ -42,9 +45,13 @@ func TestConfig_MarshalUnmarshal_RoundTrip(t *testing.T) { assert.Equal(t, original.NetSocket, restored.NetSocket) assert.Equal(t, original.ConsoleLog, restored.ConsoleLog) assert.Equal(t, original.LogLevel, restored.LogLevel) - require.Len(t, restored.VirtioFS, 1) + require.Len(t, restored.VirtioFS, 2) assert.Equal(t, "shared", restored.VirtioFS[0].Tag) assert.Equal(t, "/home/user/data", restored.VirtioFS[0].HostPath) + assert.False(t, restored.VirtioFS[0].ReadOnly) + assert.Equal(t, "readonly-data", restored.VirtioFS[1].Tag) + assert.Equal(t, "/var/data", restored.VirtioFS[1].HostPath) + assert.True(t, restored.VirtioFS[1].ReadOnly) // json:"-" fields should be zero in the unmarshaled result. assert.Empty(t, restored.LibDir) @@ -104,6 +111,8 @@ func TestVirtioFSMount_Serialization(t *testing.T) { assert.Contains(t, raw, "tag") assert.Contains(t, raw, "path") + // read_only should be omitted when false. + assert.NotContains(t, raw, "read_only") var restored VirtioFSMount err = json.Unmarshal(data, &restored) @@ -111,4 +120,32 @@ func TestVirtioFSMount_Serialization(t *testing.T) { assert.Equal(t, mount.Tag, restored.Tag) assert.Equal(t, mount.HostPath, restored.HostPath) + assert.False(t, restored.ReadOnly) +} + +func TestVirtioFSMount_ReadOnly_Serialization(t *testing.T) { + t.Parallel() + + mount := VirtioFSMount{ + Tag: "data", + HostPath: "/var/data", + ReadOnly: true, + } + + data, err := json.Marshal(mount) + require.NoError(t, err) + + var raw map[string]json.RawMessage + err = json.Unmarshal(data, &raw) + require.NoError(t, err) + + assert.Contains(t, raw, "read_only") + + var restored VirtioFSMount + err = json.Unmarshal(data, &restored) + require.NoError(t, err) + + assert.Equal(t, mount.Tag, restored.Tag) + assert.Equal(t, mount.HostPath, restored.HostPath) + assert.True(t, restored.ReadOnly) }