Clear ambient inherited #14133
Merged
Clear ambient inherited #14133
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
|
cc @anitazha |
yuwata
added
ci-fails/needs-rework 🔥
|
|
|
What happens when the following is specified? Also, I think DBus API needs to be updated. |
yuwata
added
the
reviewed/needs-rework 🔨
|
|
hmm, is this really desirable this way? why not drop all ambient caps by default, and then allow AmbientCapabilities= as a method of passing them along? |
kkuehlz
force-pushed
the
clear_ambient_inherited
branch
from
0139ed9
to
63c52dd
Compare
|
@poettering Yeah, I definitely agree. Having to specify dropping caps would be an opt in security feature, which is not good. Fixed and force pushed. cc @yuwata |
yuwata
removed
ci-fails/needs-rework 🔥
poettering
reviewed
Nov 26, 2019
src/basic/capability-util.c
Outdated
| @@ -138,6 +138,42 @@ int capability_ambient_set_apply(uint64_t set, bool also_inherit) { | |||
| return 0; | |||
| } | |||
|
|
|||
| int capability_ambient_set_clear(uint64_t set) { | |||
There was a problem hiding this comment.
hmm, iiuc the parameter specifies the ones to keep, right? hence call it "keep" or so, to make this clearer?
src/basic/capability-util.c
Outdated
| return -errno; | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
shouldn't this be done in capability_ambient_set_apply()? i.e. that the function is changed to drop all ambient caps not specified and set all ambient caps specified? that would be much shorter and simpler and still do what you need, no?
Modify the functions capability_update_inherited_set() and capability_ambient_set_apply() to drop capabilities not explicitly requested by the user.
kkuehlz
force-pushed
the
clear_ambient_inherited
branch
from
63c52dd
to
1439864
Compare
|
@poettering Changes made. |
poettering
reviewed
Nov 27, 2019
|
looks great! one more tiny fix |
kkuehlz
force-pushed
the
clear_ambient_inherited
branch
from
1439864
to
0430df3
Compare
|
@poettering Fixed and force pushed (also added the assert_se to the other call to cap_get_proc from the existing test) |
poettering
reviewed
Nov 27, 2019
Change test_set_ambient_caps() to test_apply_ambient_caps(), since the function capability_ambient_set_apply() not only sets ambient capabilities, but clears inherited capabilities that are not explicitly requested by the caller.
The function capability_ambient_set_apply() now drops capabilities not in the capability_ambient_set(), so it is necessary to call it when the ambient set is empty. Fixes systemd#13163
kkuehlz
force-pushed
the
clear_ambient_inherited
branch
from
0430df3
to
943800f
Compare
|
thanks! lgtm, but let's merge that after the 244 release |
kkuehlz
added a commit
to kkuehlz/systemd
that referenced
this pull request
Mar 27, 2020
systemd#14133 made capability_ambient_set_apply() acquire capabilities that were explicitly asked for and drop all others. This change means the function is called even with an empty capability set, opening up a code path for users without ambient capabilities to call this function. This function will error with EINVAL out on kernels < 4.3 because PR_CAP_AMBIENT is not understood. This turns capability_ambient_set_apply() into a noop for kernels < 4.3 Fixes systemd#15225
keszybz
pushed a commit
that referenced
this pull request
Mar 29, 2020
#14133 made capability_ambient_set_apply() acquire capabilities that were explicitly asked for and drop all others. This change means the function is called even with an empty capability set, opening up a code path for users without ambient capabilities to call this function. This function will error with EINVAL out on kernels < 4.3 because PR_CAP_AMBIENT is not understood. This turns capability_ambient_set_apply() into a noop for kernels < 4.3 Fixes #15225
keszybz
pushed a commit
to systemd/systemd-stable
that referenced
this pull request
Apr 1, 2020
systemd/systemd#14133 made capability_ambient_set_apply() acquire capabilities that were explicitly asked for and drop all others. This change means the function is called even with an empty capability set, opening up a code path for users without ambient capabilities to call this function. This function will error with EINVAL out on kernels < 4.3 because PR_CAP_AMBIENT is not understood. This turns capability_ambient_set_apply() into a noop for kernels < 4.3 Fixes systemd/systemd#15225 (cherry picked from commit 7ea4392)
Yamakuzure
pushed a commit
to elogind/elogind
that referenced
this pull request
Aug 31, 2020
systemd/systemd#14133 made capability_ambient_set_apply() acquire capabilities that were explicitly asked for and drop all others. This change means the function is called even with an empty capability set, opening up a code path for users without ambient capabilities to call this function. This function will error with EINVAL out on kernels < 4.3 because PR_CAP_AMBIENT is not understood. This turns capability_ambient_set_apply() into a noop for kernels < 4.3 Fixes systemd/systemd#15225
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.
No description provided.