fsck fails after ostree commit for bare-user-only repo

[cxrvh]

Files committed with ostree commit into a bare-user-only repo [0] are found corrupted when checked with ostree fsck although the files are fine (SHA256 sum is valid after checkout):

Validating refs...
Validating refs in collections...
Enumerating objects...
Verifying content integrity of 1 commit objects...
fsck objects (1/4) [===          ]  25%
error: In commits d2d11564c36e4ce7a27063029e79e3623458354a06ac2a1a1f8d6c4e2dae6bbf:
fsck content object 840ce1349a9e6ab32c02cbe26d5f60c8c46847e1e60af545a0f0bdab571290de:
Corrupted file object; checksum expected='840ce1349a9e6ab32c02cbe26d5f60c8c46847e1e60af545a0f0bdab571290de'
actual='9ae371068638ba594f6874af81ccda2e2b5a61d87f7141acbb994355b0e410b7'

How to reproduce

mkdir commit
echo "Value" > commit/value

# This fails
ostree init --mode bare-user-only --repo=bare-user-only
ostree --repo=bare-user-only commit -s 'Commit test' -b 'test' commit
ostree --repo=bare-user-only fsck

# This works
ostree init --mode bare-user --repo=bare-user
ostree --repo=bare-user commit -s 'Commit test' -b 'test' commit
ostree --repo=bare-user fsck

ostree fsck validates Flatpak repos (bare-user-only according to the config) without issues.
On Arch Linux (without SELinux), the ostree checksum on the object files returns the expected checksum (just as in Flatpak repos):

ostree checksum commit/value
840ce1349a9e6ab32c02cbe26d5f60c8c46847e1e60af545a0f0bdab571290de

ostree checksum bare-user-only/objects/84/0ce1349a9e6ab32c02cbe26d5f60c8c46847e1e60af545a0f0bdab571290de
840ce1349a9e6ab32c02cbe26d5f60c8c46847e1e60af545a0f0bdab571290de

System

The behavior is reproducible on Fedora 33 (2021.2, SELinux enforcing) & Arch Linux (2021.3), with and without --no-xattrs:

ostree --version
libostree:
 Version: '2021.3'
 Git: v2021.3
 Features:
  - libsoup
  - gpgme
  - sign-ed25519
  - ex-fsverity
  - libarchive
  - openssl
  - avahi
  - libmount
  - systemd
  - release
  - p2p

Filesystem: ext4

[0]: The same issue occurs on Fedora with a bare repository when committed with --no-xattrs.

[lucab]

Thanks for the report. I think this due to the fact that a bare-user-only repository does not store details on uid/gid/xattrs, so the committed object only has a subset of the metadata compared to the original input. This results in a checksum mismatch when trying a fsck.

I'm not sure what's the proper fix here, as the committed object does not seem to retain enough metadata to reconstruct the original input (and thus match the hash). At the same time we could check the file-content checksum only, but I feat that such information is not recorded either (I haven't seen it so far).

As a temporary workaround, you can commit with --canonical-permissions --no-xattrs in order to get matching metadata (and thus checksum) at both commit and fsck time.

[cgwalters]

The root problem here is that on SELinux systems, we will get a security.selinux xattr by default even if we didn't set one. This is likely true on SMACK systems too.

I think ultimately for bare-user-only repos we should probably explicitly not support xattrs, and also teach ostree to ignore any xattrs on disk when computing the checksum.

[lucab]

The issue is not limited to xattrs; while I was looking into this, I spot possibly mismatched uid/gid in the picture too.

Is there a content-only checksum that can be used here instead? I'm inclined to say no as I haven't seen it in the committed metadata, but I may be wrong.
To my current understanding, there is no simple way for retro-fixing this.
But we could at least prevent future broken commits by allowing bare-user-only commit operation only if the relevant modifier-flags are set (and do automatically in CLI).

[cgwalters]

You're right, uid/gid will be whatever the repo creating process chooses for bare-user-only. There's no content-only checksum.

Another way to look at this is ostree was really designed to target the "host OS update" case, hence uid/gid/xattrs. bare-user-only is making ostree more like git, but we didn't complete the end-to-end plumbing for that.

I think the generalization then is to teach the core to return uid/gid 0 and no xattrs for bare-user-only instead of what's on disk.

[dbnicholson]

Wouldn't the simplest thing be to just add in OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS and OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS when the repo mode is OSTREE_REPO_MODE_BARE_USER_ONLY? By definition those features can't be useful in a bare-user-only repo, so we might as well help people out and not try to use them.

[wmanley]

I was thinking about this a little while ago while working my ostree fuse project. I wonder if it would make sense to have an additional store/cache for data where hard-linking doesn't make sense. That way we could store file meta separately from the file objects. Graph traversals could be a lot faster as we wouldn't need to be calling open/read/close for a bunch of small files. We wouldn't lose the ability to hard-link/reflink because the file objects would still be stored whole. It would solve the bare-user-only issue as the filemeta could still be stored.

I described one such design here: #2338 (comment) . An alternative might be a SQLite or LMDB database which would make writes much faster (and handle fsync safety for us) at the cost of an additional dependency.

[lucab]

While looking into this, I discovered that checksums/checkouts are broken in a symmetrical way and need API fixes: #2415

[lucab]

@cxrvh this should be fixed in main by a mix of #2412 plus #2418. Your reproducer now runs fine here locally, but it would be nice if you could double-check.

[cxrvh]

@lucab Yes, I can confirm that it's fixed in main (but I couldn't test a bare repository with SELinux yet). Thanks.