Tighten storage pool permissions (from Incus)

lxd/patches.go

@@ -109,6 +109,7 @@ var patches = []patch{
 	{name: "storage_update_powerflex_snapshot_prefix", stage: patchPostDaemonStorage, run: patchUpdatePowerFlexSnapshotPrefix},
 	{name: "config_remove_instances_placement_scriptlet", stage: patchPreLoadClusterConfig, run: patchRemoveInstancesPlacementScriptlet},
 	{name: "event_entitlement_rename", stage: patchPreLoadClusterConfig, run: patchEventEntitlementNames},
+	{name: "pool_fix_default_permissions", stage: patchPostDaemonStorage, run: patchDefaultStoragePermissions},
 }
 
 type patch struct {
@@ -1805,4 +1806,43 @@ func patchEventEntitlementNames(name string, d *Daemon) error {
 	})
 }
 
+// patchDefaultStoragePermissions re-applies the default modes to all storage pools.
+func patchDefaultStoragePermissions(_ string, d *Daemon) error {
+	s := d.State()
+
+	var pools []string
+
+	err := s.DB.Cluster.Transaction(s.ShutdownCtx, func(ctx context.Context, tx *db.ClusterTx) error {
+		var err error
+
+		// Get all storage pool names.
+		pools, err = tx.GetStoragePoolNames(ctx)
+
+		return err
+	})
+	if err != nil {
+		// Skip the rest of the patch if no storage pools were found.
+		if api.StatusErrorCheck(err, http.StatusNotFound) {
+			return nil
+		}
+
+		return fmt.Errorf("Failed getting storage pool names: %w", err)
+	}
+
+	for _, pool := range pools {
+		for _, volEntry := range storageDrivers.BaseDirectories {
+			for _, volDir := range volEntry.Paths {
+				path := storageDrivers.GetPoolMountPath(pool) + "/" + volDir
+
+				err := os.Chmod(path, volEntry.Mode)
+				if err != nil && !errors.Is(err, fs.ErrNotExist) {
+					return fmt.Errorf("Failed to set directory mode %q: %w", path, err)
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 // Patches end here

lxd/storage/backend_lxd.go

@@ -7237,10 +7237,10 @@ func (b *lxdBackend) RestoreCustomVolume(projectName, volName string, snapshotNa
 
 func (b *lxdBackend) createStorageStructure(path string) error {
 	for _, volType := range b.driver.Info().VolumeTypes {
-		for _, name := range drivers.BaseDirectories[volType] {
+		for _, name := range drivers.BaseDirectories[volType].Paths {
 			path := filepath.Join(path, name)
-			err := os.MkdirAll(path, 0711)
-			if err != nil && !os.IsExist(err) {
+			err := os.MkdirAll(path, drivers.BaseDirectories[volType].Mode)
+			if err != nil && !errors.Is(err, fs.ErrExist) {
 				return fmt.Errorf("Failed to create directory %q: %w", path, err)
 			}
 		}

lxd/storage/drivers/driver_btrfs.go

@@ -296,7 +296,7 @@ func (d *btrfs) Delete(op *operations.Operation) error {
 
 	// Delete potential intermediate btrfs subvolumes.
 	for _, volType := range d.Info().VolumeTypes {
-		for _, dir := range BaseDirectories[volType] {
+		for _, dir := range BaseDirectories[volType].Paths {
 			path := filepath.Join(GetPoolMountPath(d.name), dir)
 			if !shared.PathExists(path) {
 				continue

lxd/storage/drivers/driver_zfs_utils.go

@@ -329,7 +329,7 @@ func (d *zfs) initialDatasets() []string {
 
 	// Iterate over the listed supported volume types.
 	for _, volType := range d.Info().VolumeTypes {
-		entries = append(entries, BaseDirectories[volType][0], "deleted/"+BaseDirectories[volType][0])
+		entries = append(entries, BaseDirectories[volType].Paths[0], "deleted/"+BaseDirectories[volType].Paths[0])
 	}
 
 	return entries

lxd/storage/drivers/generic_vfs.go

@@ -1165,11 +1165,11 @@ func genericVFSListVolumes(d Driver) ([]Volume, error) {
 	poolMountPath := GetPoolMountPath(poolName)
 
 	for _, volType := range d.Info().VolumeTypes {
-		if len(BaseDirectories[volType]) < 1 {
+		if len(BaseDirectories[volType].Paths) < 1 {
 			return nil, fmt.Errorf("Cannot get base directory name for volume type %q", volType)
 		}
 
-		volTypePath := filepath.Join(poolMountPath, BaseDirectories[volType][0])
+		volTypePath := filepath.Join(poolMountPath, BaseDirectories[volType].Paths[0])
 		ents, err := os.ReadDir(volTypePath)
 		if err != nil {
 			return nil, fmt.Errorf("Failed to list directory %q for volume type %q: %w", volTypePath, volType, err)

lxd/storage/drivers/volume.go

@@ -77,13 +77,18 @@ const ContentTypeISO = ContentType("iso")
 // VolumePostHook function returned from a storage action that should be run later to complete the action.
 type VolumePostHook func(vol Volume) error
 
+type baseDirectory struct {
+	Paths []string
+	Mode  os.FileMode
+}
+
 // BaseDirectories maps volume types to the expected directories.
-var BaseDirectories = map[VolumeType][]string{
-	VolumeTypeBucket:    {"buckets"},
-	VolumeTypeContainer: {"containers", "containers-snapshots"},
-	VolumeTypeCustom:    {"custom", "custom-snapshots"},
-	VolumeTypeImage:     {"images"},
-	VolumeTypeVM:        {"virtual-machines", "virtual-machines-snapshots"},
+var BaseDirectories = map[VolumeType]baseDirectory{
+	VolumeTypeBucket:    {Paths: []string{"buckets"}, Mode: 0o711},                            // MinIO is run as non-root, so 0700 won't work, however as S3 interface doesn't allow creation of setuid binaries this is OK.
+	VolumeTypeContainer: {Paths: []string{"containers", "containers-snapshots"}, Mode: 0o711}, // Containers may be run as non-root, so 0700 won't work, however as containers have their own sub-directory with correct ownership that is 0100 this is OK.
+	VolumeTypeCustom:    {Paths: []string{"custom", "custom-snapshots"}, Mode: 0o700},
+	VolumeTypeImage:     {Paths: []string{"images"}, Mode: 0o700},
+	VolumeTypeVM:        {Paths: []string{"virtual-machines", "virtual-machines-snapshots"}, Mode: 0o700},
 }
 
 // Volume represents a storage volume, and provides functions to mount and unmount it.

test/suites/storage.sh

@@ -28,6 +28,35 @@ test_storage() {
 
   lxc storage volume create "$storage_pool" "$storage_volume" size=1MiB
 
+  # Test storage directory permissions
+
+  # Verify storage pool directory permissions match BaseDirectories expectations.
+  # We expect:
+  # - containers, containers-snapshots -> 0711
+  # - buckets -> 0711
+  # - custom, custom-snapshots -> 0700
+  # - images -> 0700
+  # - virtual-machines, virtual-machines-snapshots -> 0700
+  pool_path="${LXD_DIR}/storage-pools/${storage_pool}"
+
+  declare -A expected_modes
+  expected_modes[containers]=711
+  expected_modes[containers-snapshots]=711
+  if [ "${lxd_backend}" != "ceph" ] && [ "${lxd_backend}" != "pure" ]; then
+      expected_modes[buckets]=711 # Buckets are not supported on ceph and pure backends.
+  fi
+  expected_modes[custom]=700
+  expected_modes[custom-snapshots]=700
+  expected_modes[images]=700
+  expected_modes[virtual-machines]=700
+  expected_modes[virtual-machines-snapshots]=700
+
+  for dir in "${!expected_modes[@]}"; do
+    want="${expected_modes[$dir]}"
+    mode=$(stat -c %a "${pool_path}/${dir}")
+    [ "${mode}" = "${want}" ]
+  done
+
   # Test setting description on a storage volume
   lxc storage volume show "$storage_pool" "$storage_volume" | sed 's/^description:.*/description: bar/' | lxc storage volume edit "$storage_pool" "$storage_volume"
   [ "$(lxc storage volume get "$storage_pool" "$storage_volume" -p description)" = "bar" ]