spec: loosen permissions on /usr/lib/containers/storage/* dirs #2507

dustymabe · 2025-08-11T17:15:18Z

Since /usr/lib/containers/storage/ is configured as an additional store by default [1] these directories are important to be able to be read by unprivileged users.

The problem I ran into was when generating a disk image using supermin [2] it didn't pick up the images.lock and layers.lock files when creating the resulting filesystem (because running supermin unprivileged means they weren't viewable because they were under directories that weren't accessible to non-root).

Because those files weren't there, when I eventually ran OSBuild inside the supermin VM, which happens to mount in /usr/ read-only inside the OSBuild env I'd get an error when trying to do podman/skopeo operations:

Copying blob sha256:9dad063a624b62064bf25dbbc2e802e472d636056f661f2a0be73efd8a4da98b
time="2025-08-11T15:45:48Z" level=fatal msg="trying to reuse blob
sha256:7842e86602a2d0ca8e8e12adc8520579a1fef4fad571bb83a450b5d98269ae5a at destination:
looking for layers with digest \"sha256:7842e86602a2d0ca8e8e12adc8520579a1fef4fad571bb83a450b5d98269ae5a\":
loading additional layer stores: open /usr/lib/containers/storage/overlay-layers/layers.lock: read-only file system"

If the *.lock files exist. The errors won't manifest.

[1]

common/rpm/update-config-files.sh

Lines 46 to 49 in 6f98aea

    
           # Set these on all Fedora and RHEL 10+ 
        
           if [[ -n "$FEDORA" ]] || [[ "$RHEL" -ge 10 ]]; then 
        
               sed -i -e '/^additionalimagestores\ =\ \[/a "\/usr\/lib\/containers\/storage",' storage.conf 
        
           fi

[2] https://libguestfs.org/supermin.1.html

Summary by Sourcery

Loosen directory permissions for overlay-images and overlay-layers under /usr/lib/containers/storage to allow unprivileged users to read lock files and prevent read-only filesystem errors in unprivileged workflows

Bug Fixes:

Change install mode from 700 to 755 for overlay-images directory in the RPM spec
Change install mode from 700 to 755 for overlay-layers directory in the RPM spec

Since /usr/lib/containers/storage/ is configured as an additional store by default [1] these directories are important to be able to be read by unprivileged users. The problem I ran into was when generating a disk image using supermin [2] it didn't pick up the images.lock and layers.lock files when creating the resulting filesystem (because running supermin unprivileged means they weren't viewable because they were under directories that weren't accessible to non-root). Because those files weren't there, when I eventually ran OSBuild inside the supermin VM, which happens to mount in /usr/ read-only inside the OSBuild env I'd get an error when trying to do podman/skopeo operations: ``` Copying blob sha256:9dad063a624b62064bf25dbbc2e802e472d636056f661f2a0be73efd8a4da98b time="2025-08-11T15:45:48Z" level=fatal msg="trying to reuse blob sha256:7842e86602a2d0ca8e8e12adc8520579a1fef4fad571bb83a450b5d98269ae5a at destination: looking for layers with digest \"sha256:7842e86602a2d0ca8e8e12adc8520579a1fef4fad571bb83a450b5d98269ae5a\": loading additional layer stores: open /usr/lib/containers/storage/overlay-layers/layers.lock: read-only file system" ``` If the *.lock files exist. The errors won't manifest. [1] https://github.com/containers/common/blob/6f98aea06fcd025b418e7414d6b33df02478aa51/rpm/update-config-files.sh#L46-L49 [2] https://libguestfs.org/supermin.1.html

sourcery-ai · 2025-08-11T17:15:25Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Loosen default permissions for overlay storage directories under /usr/lib/containers/storage to 755 so that unprivileged users can access the generated lock files when building disk images.

Entity relationship diagram for overlay storage directories and lock files

erDiagram
    OVERLAY_IMAGES ||--o| IMAGES_LOCK : contains
    OVERLAY_LAYERS ||--o| LAYERS_LOCK : contains
    OVERLAY_IMAGES {
        string path
        int permissions
    }
    IMAGES_LOCK {
        string filename
    }
    OVERLAY_LAYERS {
        string path
        int permissions
    }
    LAYERS_LOCK {
        string filename
    }

File-Level Changes

Change	Details	Files
Loosened directory permissions for overlay storage	Changed overlay-images directory mode from 700 to 755 Changed overlay-layers directory mode from 700 to 755	`rpm/containers-common.spec`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

dustymabe · 2025-08-11T17:15:58Z

I'm interested to have someone evaluate the security implications of making these directories viewable by non-root users.

sourcery-ai

Hey @dustymabe - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

openshift-ci · 2025-08-11T17:16:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dustymabe, sourcery-ai[bot]
Once this PR has been reviewed and has the lgtm label, please assign kolyshkin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Narrowing in further on why this is required leads to an issue with permissions when supermin is creating the filesystem for the supermin VM. Opened containers/common#2507 upstream to address this, but for now let's fixup the permissions in our image build.

Luap99

If the goal if this store is to be used by rootless users sure, but AFAIK the entire share store across users is generally quite bad because all the uids are not aligned and there are post that recommend chmod -R a+rx https://www.redhat.com/en/blog/image-stores-podman So that seem to be in-line with that.

AFAICT the actually layer files under the directories are still owned by root with no access for the rootless user so security wise it should be fine? Although logically if the user has access to the lock file I assume they can deadlock all other podman instances that access the store by simply holding the lock forever which may be a much bigger security concern?

I don't understand why we configure such a store by default. Trying to find the history brings me to https://src.fedoraproject.org/rpms/containers-common/c/719145eb7e83a4e618588c21c66f2687c2ffb458
which well has no context whatsoever.

cc @lsm5 @giuseppe @mheon

The problem I ran into was when generating a disk image using supermin [2] it didn't pick up the images.lock and layers.lock files when creating the resulting filesystem (because running supermin unprivileged means they weren't viewable because they were under directories that weren't accessible to non-root).

Isn't that a problem of the tool if it misses files that are only viewable by root? I am might be missing the purpose of supermin.

mheon · 2025-08-11T18:51:22Z

Yeah I'm similarly iffy on this globally configured default additional store... I can't imagine this actually working well by default?

dustymabe · 2025-08-11T19:29:21Z

If the goal if this store is to be used by rootless users sure, but AFAIK the entire share store across users is generally quite bad because all the uids are not aligned and there are post that recommend chmod -R a+rx https://www.redhat.com/en/blog/image-stores-podman So that seem to be in-line with that.

I'm not sure that it is intended to be used by rootless users. If I podman info as a rootless user I see configFile: /var/home/core/.config/containers/storage.conf and I don't see anything under graphOptions: {}.

I guess the additional store is just in /usr/share/containers/storage.conf, so only applies to rootful?

Isn't that a problem of the tool if it misses files that are only viewable by root? I am might be missing the purpose of supermin.

perhaps. I think it's just doing the best it can given the situation.

Another thing that may be an option here. If an additional store is configured and it's on a read-only filesystem, then don't require locking? The read-only filesystem should ensure there's no consistency issues, right?

giuseppe · 2025-08-11T21:46:41Z

I don't think it should be configured by default for rootless because it requires a specific configuration. I don't see disadvantages doing it for root-only but then we don't need to loosen the permissions?

dustymabe · 2025-08-12T03:24:26Z

I don't think it should be configured by default for rootless because it requires a specific configuration.

Right. I determined above it's not enabled for rootless by default and I agree it shouldn't be.

I don't see disadvantages doing it for root-only but then we don't need to loosen the permissions?

Yeah. I think you are right, but maybe we can shift the conversation to the idea I had in #2507 (comment)

Another thing that may be an option here. If an additional store is configured and it's on a read-only filesystem, then don't require locking? The read-only filesystem should ensure there's no consistency issues, right?

Luap99 · 2025-08-12T09:35:42Z

I guess the additional store is just in /usr/share/containers/storage.conf, so only applies to rootful?

Ah yes right that make sense of course

Another thing that may be an option here. If an additional store is configured and it's on a read-only filesystem, then don't require locking? The read-only filesystem should ensure there's no consistency issues, right?

I am not a storage maintainer and not to familiar with the store logic there to know how easy that would be to implement.
One obvious issue though is that the store can be longed lived (i.e. podman system service) and then at any time later someone can remount the fs rw and the modify the store causing inconsistency issues in theory. Not sure if that is a scenario we need to care about.

In principle if we allow this to be run on readonly without existing lock files than there likely isn't any reason to create the lock files in the spec file to begin with.

dustymabe · 2025-08-13T12:45:28Z

In principle if we allow this to be run on readonly without existing lock files than there likely isn't any reason to create the lock files in the spec file to begin with.

True..

Where should we take this? Since I think we've established that the proposed change in this PR isn't the likely best solution to the problem should I close it?

Narrowing in further on why this is required leads to an issue with permissions when supermin is creating the filesystem for the supermin VM. Opened containers/common#2507 upstream to address this, but for now let's fixup the permissions in our image build.

dustymabe · 2025-08-20T12:38:55Z

I'll close it now.

sourcery-ai bot approved these changes Aug 11, 2025

View reviewed changes

Luap99 reviewed Aug 11, 2025

View reviewed changes

dustymabe closed this Aug 20, 2025

	# Set these on all Fedora and RHEL 10+
	if [[ -n "$FEDORA" ]] \|\| [[ "$RHEL" -ge 10 ]]; then
	sed -i -e '/^additionalimagestores\ =\ \[/a "\/usr\/lib\/containers\/storage",' storage.conf
	fi

spec: loosen permissions on /usr/lib/containers/storage/* dirs #2507

spec: loosen permissions on /usr/lib/containers/storage/* dirs #2507

Uh oh!

Conversation

dustymabe commented Aug 11, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Entity relationship diagram for overlay storage directories and lock files

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

dustymabe commented Aug 11, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Aug 11, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

mheon commented Aug 11, 2025

Uh oh!

dustymabe commented Aug 11, 2025

Uh oh!

giuseppe commented Aug 11, 2025

Uh oh!

dustymabe commented Aug 12, 2025

Uh oh!

Luap99 commented Aug 12, 2025

Uh oh!

dustymabe commented Aug 13, 2025

Uh oh!

dustymabe commented Aug 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dustymabe commented Aug 11, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Aug 11, 2025 •

edited

Loading