journal: clean up permission setting and acl adjustements on user journals #2063
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
Most of the function is moved to acl-util.c to make it possible to add tests in subsequent commit. Setting of the mode in server_fix_perms is removed: - we either just created the file ourselves, and the permission be better right, - or the file was already there, and we should not modify the permissions. server_fix_perms is renamed to server_fix_acls to better reflect new meaning, and made static because it is only used in one file.
For now, only add_acls_for_user is tested. When run under root, it actually sets the acls. When run under non-root, it sets the acls for the user, which does nothing, but at least calls the functions.
When we have non-owner user or group entries, we need the mask for the acl to be valid. But acl_calc_mask() calculates the mask to include all permissions, even those that were masked before. Apparently this happens when we inherit *:r-x permissions from a parent directory — the kernel sets *:r-x, mask:r--, effectively masking the executable bit. acl_calc_mask() would set the mask:r-x, effectively enabling the bit. To avoid this, be more conservative when to add the mask entry: first iterate over all entries, and do nothing if a mask. This returns the code closer to J.A.Steffens' original version in v204-90-g23ad4dd884. Should fix systemd#1977.
This way, directories created later for containers or for journald-remote, will be readable by adm & wheel groups by default, similarly to /var/log/journal/%m itself. systemd#1971
|
Will merge this now, as the stuff I found isn't major. Still think this is only a partial fix, and we should do the thing suggested in #1397 (comment). |
poettering
added a commit
that referenced
this pull request
Nov 30, 2015
journal: clean up permission setting and acl adjustements on user journals
keszybz
added a commit
to keszybz/systemd
that referenced
this pull request
Nov 30, 2015
keszybz
added a commit
to keszybz/systemd
that referenced
this pull request
Nov 30, 2015
Agreed. I don't like the race either. |
martinpitt
pushed a commit
to martinpitt/systemd
that referenced
this pull request
Dec 28, 2015
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.
First commit move acl-modifying code to acl-util. It also removes the call to fchmod(): we can expect the file to have correct permissions since we just created it with some mask or we created it before with the right permissions. If an admin changed the mode of the file, we should just let it be.
Second commit adds some tests which calls the acl-modifying code. The test doesn't show any problems, but it exercises the code in question which so far didn't have any unit tests.
The third commit fixes the problem in #1977 by not setting the mask if it is already set. We were only setting it to avoid having an invalid acl, and it turns out that with current inheritable permissions set on the parent directory, we do have a mask entry by default.
The fourth commit is for #1971. The fifth is for #1397.
Version 2 of #2057.