From 7598d5ab710e05829c7bc4a6e30106a022f376c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sun, 9 Nov 2025 18:41:24 -0500 Subject: [PATCH 1/5] lxd/storage: Tighten storage pool volume permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related to https://github.com/lxc/incus/issues/2641 Signed-off-by: Stéphane Graber (cherry picked from commit b0c6c0bac42c6ac27d536984cc043a6ec02b9e7c) Signed-off-by: Thomas Parrott License: Apache-2.0 --- lxd/storage/backend_lxd.go | 4 ++-- lxd/storage/drivers/driver_btrfs.go | 2 +- lxd/storage/drivers/driver_zfs_utils.go | 2 +- lxd/storage/drivers/generic_vfs.go | 4 ++-- lxd/storage/drivers/volume.go | 17 +++++++++++------ 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go index 34805b5d1acd..0b9616bc4cce 100644 --- a/lxd/storage/backend_lxd.go +++ b/lxd/storage/backend_lxd.go @@ -7237,9 +7237,9 @@ 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) + err := os.MkdirAll(path, drivers.BaseDirectories[volType].Mode) if err != nil && !os.IsExist(err) { return fmt.Errorf("Failed to create directory %q: %w", path, err) } diff --git a/lxd/storage/drivers/driver_btrfs.go b/lxd/storage/drivers/driver_btrfs.go index 404a9c8e0761..a23e6ade98bc 100644 --- a/lxd/storage/drivers/driver_btrfs.go +++ b/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 diff --git a/lxd/storage/drivers/driver_zfs_utils.go b/lxd/storage/drivers/driver_zfs_utils.go index 56a111c93a86..f91d4d31eb3c 100644 --- a/lxd/storage/drivers/driver_zfs_utils.go +++ b/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 diff --git a/lxd/storage/drivers/generic_vfs.go b/lxd/storage/drivers/generic_vfs.go index a6718baffcfa..cd2b8764a776 100644 --- a/lxd/storage/drivers/generic_vfs.go +++ b/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) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index e40ca31f4b5d..38ff9582950f 100644 --- a/lxd/storage/drivers/volume.go +++ b/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}, + VolumeTypeContainer: {Paths: []string{"containers", "containers-snapshots"}, Mode: 0o711}, + 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. From 52a551c8f0452eda858ac7ba27dc250d84ce72bf Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Mon, 10 Nov 2025 10:49:26 +0000 Subject: [PATCH 2/5] lxd/storage/drivers/volume: Add comments explaining differences in BaseDirectories permissions Signed-off-by: Thomas Parrott --- lxd/storage/drivers/volume.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/storage/drivers/volume.go b/lxd/storage/drivers/volume.go index 38ff9582950f..b7e2e9a0ccab 100644 --- a/lxd/storage/drivers/volume.go +++ b/lxd/storage/drivers/volume.go @@ -84,8 +84,8 @@ type baseDirectory struct { // BaseDirectories maps volume types to the expected directories. var BaseDirectories = map[VolumeType]baseDirectory{ - VolumeTypeBucket: {Paths: []string{"buckets"}, Mode: 0o711}, - VolumeTypeContainer: {Paths: []string{"containers", "containers-snapshots"}, Mode: 0o711}, + 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}, From 89c0906e5107fa059263490d020b99dfa1c9c26b Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Mon, 10 Nov 2025 09:35:28 +0000 Subject: [PATCH 3/5] lxd/storage/backend/lxd: Replace deprecated os.IsExist Signed-off-by: Thomas Parrott --- lxd/storage/backend_lxd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go index 0b9616bc4cce..d5b0a94f84dc 100644 --- a/lxd/storage/backend_lxd.go +++ b/lxd/storage/backend_lxd.go @@ -7240,7 +7240,7 @@ func (b *lxdBackend) createStorageStructure(path string) error { for _, name := range drivers.BaseDirectories[volType].Paths { path := filepath.Join(path, name) err := os.MkdirAll(path, drivers.BaseDirectories[volType].Mode) - if err != nil && !os.IsExist(err) { + if err != nil && !errors.Is(err, fs.ErrExist) { return fmt.Errorf("Failed to create directory %q: %w", path, err) } } From 87a34bbe6bd3f918081db431ac1a6ee22346f172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Sun, 9 Nov 2025 18:41:39 -0500 Subject: [PATCH 4/5] lxd/patches: Re-apply storage permissions on update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related to https://github.com/lxc/incus/issues/2641 Signed-off-by: Stéphane Graber (cherry picked from commit 3abdc12cf6a8dce391d28d340a32c137125357dd) Signed-off-by: Thomas Parrott License: Apache-2.0 --- lxd/patches.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/lxd/patches.go b/lxd/patches.go index f8e93c0a8f8f..4e885f64c643 100644 --- a/lxd/patches.go +++ b/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 From 67a5819feb76fb9c887508d0afbaa3aa728de890 Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Mon, 10 Nov 2025 10:49:13 +0000 Subject: [PATCH 5/5] test: Add tests for storage directory permissions Signed-off-by: Thomas Parrott --- test/suites/storage.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/suites/storage.sh b/test/suites/storage.sh index 3a4880121f60..b2853ed0e53d 100644 --- a/test/suites/storage.sh +++ b/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" ]