lib/checkout: Ignore world-writable dirs for bare-user-only checkout #914
Conversation
|
This depends on #913 |
|
☔ The latest upstream changes (presumably c81252c) made this pull request unmergeable. Please resolve the merge conflicts. |
|
So, flatpak currently uses bare-user, although the long term plan when everything is working is to use bare-user-only. Does it ever make sense to have writable directories when you do a checkout --user, independent of the repo format? I mean, I understand the system container case where you need 100% fidelity and check out as root. But once you're changing the uid/gid of the files you check out, then the permissions recorded in the repo does not mean the same thing as they do in the repo. |
|
I was trying to be conservative initially; changing I was considering introducing a new flag for this ( |
|
Its not just about bare-user though. I mean, if you have an archive-z2 repo, and you do a checkout --user from that, do you want a world-writable directory in your homedir? I mean, you might want to if you want 100% metadata fidelity, but then you would not use --user, which breaks that anyway. I dunno. Maybe its too magic and should be a separate flag like --user-safe. |
|
I posted to the list: https://mail.gnome.org/archives/ostree-list/2017-June/msg00026.html However, I realized there is a problem; when you want to do a commit based on content you checked out. And we'll definitely pick back up directory modes from there. The only solution to this is to use a commit modifier (e.g. I think though most libostree users should start using
The special case here really is flatpak system wide installs, since there the directory obviously does need to be accessible by unprivileged users. So...I think we should go with this patch as is for now. |
I noticed my previous patches incorrectly started doing `return glnx_throw*` inside a `goto out;` function. Fix this by porting forward consistently to new style. We just do the error prefixing in the caller.
Both callers of `commit_loose_object_trusted()` were passing `OSTREE_OBJECT_TYPE_FILE`, so drop that parameter. This in turn allows us to drop lots of checking of that inside the function. Add a doc comment, and rename to `commit_loose_content_object()` for clarity.
See ostreedev#909 for more information on the rationale. Basically there's no reason for flatpak (which uses `bare-user-only`) to have world-writable dirs. Particularly with the presence of the system helper. An approach I considered instead was to parse and validate directory metadata objects at commit time. We still may do that in addition; for file objects we *had* to do it that way because the actual files would be laid down suid. But directories live only as inert `.dirmeta` objects until we do a checkout (i.e. `mkdir()`), so we can solve the problem at checkout time.
cgwalters
force-pushed
the
bareuseronly-canonical-3
branch
from
bb277a5 to
7df9f40
Compare
|
Rebased 🏄♂️ |
|
Or to elaborate, we could do this for But then again, given the impending migration to |
Both callers of `commit_loose_object_trusted()` were passing `OSTREE_OBJECT_TYPE_FILE`, so drop that parameter. This in turn allows us to drop lots of checking of that inside the function. Add a doc comment, and rename to `commit_loose_content_object()` for clarity. Closes: #914 Approved by: alexlarsson
See #909 for more information on the rationale. Basically there's no reason for flatpak (which uses `bare-user-only`) to have world-writable dirs. Particularly with the presence of the system helper. An approach I considered instead was to parse and validate directory metadata objects at commit time. We still may do that in addition; for file objects we *had* to do it that way because the actual files would be laid down suid. But directories live only as inert `.dirmeta` objects until we do a checkout (i.e. `mkdir()`), so we can solve the problem at checkout time. Closes: #914 Approved by: alexlarsson
|
☀️ Test successful - status-atomicjenkins |
See #909 for more information on the
rationale. Basically there's no reason for flatpak (which uses
bare-user-only)to have world-writable dirs. Particularly with the presence of the system
helper.
An approach I considered instead was to parse and validate directory metadata
objects at commit time. We still may do that in addition; for file objects we had
to do it that way because the actual files would be laid down suid. But directories
live only as inert
.dirmetaobjects until we do a checkout (i.e.mkdir()), sowe can solve the problem at checkout time.