lib/repo: For bare-user, make repo modes mirror checkout user-mode #908
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Oh, interesting. So this started failing in For @alexlarsson - do you want ostree to preserve suid files in |
|
No that sounds pretty bad. We probably want to strip setuid in bare-user-only repos (i.e. lose fidelity for such objects). |
cgwalters
added a commit
to cgwalters/flatpak
that referenced
this pull request
Jun 6, 2017
These are both just bad ideas. See also ostreedev/ostree#908
|
I wrote flatpak/flatpak#837 - let's discuss this there. |
cgwalters
added a commit
to cgwalters/ostree
that referenced
this pull request
Jun 6, 2017
For the flatpak use case where bare-user-only was introduced, we actually
don't want to support s{u,g} id files in particular.
Actually, I can't think of a reason to have anything outside of the
`0755 i.e. (u=rwx,g=rx,o=rx)` mask, so that's what we do here.
This will have the effect of treating existing `bare-user-only` repositories as
corrupted if they have files outside of that mask, but I think we should do this
now; most of the flatpak users will still be on `bare-user`, and we haven't
changed the semantics of that mode yet.
Note that in this patch we will also *reject* file content that doesn't
match this. This is somewhat asymmetric, since we aren't similarly rejecting
e.g. directory metadata. But, this will close off the biggest source
of the problem for flatpak (setuid binaries).
See: ostreedev#908
See: flatpak/flatpak#837
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 7, 2017
For the flatpak use case where bare-user-only was introduced, we actually
don't want to support s{u,g} id files in particular.
Actually, I can't think of a reason to have anything outside of the
`0755 i.e. (u=rwx,g=rx,o=rx)` mask, so that's what we do here.
This will have the effect of treating existing `bare-user-only` repositories as
corrupted if they have files outside of that mask, but I think we should do this
now; most of the flatpak users will still be on `bare-user`, and we haven't
changed the semantics of that mode yet.
Note that in this patch we will also *reject* file content that doesn't
match this. This is somewhat asymmetric, since we aren't similarly rejecting
e.g. directory metadata. But, this will close off the biggest source
of the problem for flatpak (setuid binaries).
See: #908
See: flatpak/flatpak#837
Closes: #909
Approved by: alexlarsson
Having every object in a bare-user repo (and checkouts) be executable is ugly. I can't think of a good reason to do that; they should only be executable if their input is. This does for `bare-user` what we did for `bare-user-only` in ostreedev#909 It's also a stronger version of what we do with `checkout -U` in suppressing suid - here we also strip world-writable files and the sticky bit (even though that's meaningless today, it might not be in the future). Closes: ostreedev#907
cgwalters
force-pushed
the
bare-user-perms
branch
from
5fb31fd to
5f31f90
Compare
|
Argh, running into |
|
☀️ Test successful - status-atomicjenkins |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Having every object in a bare-user repo (and checkouts) be executable
is ugly. I can't think of a good reason to do that. So make
the committed mode semantics mirror that for user-mode checkouts; we
strip suid/sgid bits.
However, we also do ensure that the written files are read/writable by the
owning user, since otherwise we couldn't do anything to them. I'm not aware of
any real use cases for non-readable/non-writable by owner files in ostree.
Closes: #907