| Message ID | 20191218142808.30433-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | systemd: improve SELinux support | expand |
On 12/18/19 9:28 AM, Christian Göttsche wrote: > Improve the SELinux support in systemd, especially re-adding checks for > unit file operations, like enable, mask... > > The original pull request can be found at https://github.com/systemd/systemd/pull/10023 > > Patch 1 and 2 improve logging on failures in permissive mode. > > Patch 3 adds the ability to obtain the context for a masked unit. > > Patch 4 and 5 change several system und service checks. For better > distinction two new permissions are introduced: modify and listdynusers. > > Patch 6 and 7 re-introduce checking unit file install operations. > They were dropped in 8faae625dc9b6322db452937f54176e56e65265a . > For consistency in the unexpected case while perforimg a service access > check no path can be gathered, now the check will still be executed on > the service security class (currently it switches to the system security > class). > > Patch 8 adds some notes for adding future D-Bus interfaces. Thanks for working on this. Just to make sure I didn't miss anything while reading the patches, there are no new permissions being added to the system class, correct?
On Thu, Dec 19, 2019 at 03:24:34PM -0500, Chris PeBenito wrote: > On 12/18/19 9:28 AM, Christian Göttsche wrote: > > Improve the SELinux support in systemd, especially re-adding checks for > > unit file operations, like enable, mask... > > > > The original pull request can be found at https://github.com/systemd/systemd/pull/10023 > > > > Patch 1 and 2 improve logging on failures in permissive mode. > > > > Patch 3 adds the ability to obtain the context for a masked unit. > > > > Patch 4 and 5 change several system und service checks. For better > > distinction two new permissions are introduced: modify and listdynusers. > > > > Patch 6 and 7 re-introduce checking unit file install operations. > > They were dropped in 8faae625dc9b6322db452937f54176e56e65265a . > > For consistency in the unexpected case while perforimg a service access > > check no path can be gathered, now the check will still be executed on > > the service security class (currently it switches to the system security > > class). > > > > Patch 8 adds some notes for adding future D-Bus interfaces. > > Thanks for working on this. Just to make sure I didn't miss anything while > reading the patches, there are no new permissions being added to the system > class, correct? This is what i understand: Systemd first wants the regression fixed (re-add enable disable permission checks) This phase should not add anything to the system class. It merely addresses a previous regression. When that is done, then either the whole thing can be redone properly using policy capabilities, or we can add new permissions and in that scenario i guess the new permissions would be added to the existing system class (?). If we redo the whole thing without interfering with the existing implementation then we can get rid of the system class usage for systemd. One can opt in for the proper implementation by enabling the policy capability or otherwise one can stick with the current implementation. > > -- > Chris PeBenito
On 12/20/19 5:54 AM, Dominick Grift wrote: > On Thu, Dec 19, 2019 at 03:24:34PM -0500, Chris PeBenito wrote: >> On 12/18/19 9:28 AM, Christian Göttsche wrote: >>> Improve the SELinux support in systemd, especially re-adding checks for >>> unit file operations, like enable, mask... >>> >>> The original pull request can be found at https://github.com/systemd/systemd/pull/10023 >>> >>> Patch 1 and 2 improve logging on failures in permissive mode. >>> >>> Patch 3 adds the ability to obtain the context for a masked unit. >>> >>> Patch 4 and 5 change several system und service checks. For better >>> distinction two new permissions are introduced: modify and listdynusers. >>> >>> Patch 6 and 7 re-introduce checking unit file install operations. >>> They were dropped in 8faae625dc9b6322db452937f54176e56e65265a . >>> For consistency in the unexpected case while perforimg a service access >>> check no path can be gathered, now the check will still be executed on >>> the service security class (currently it switches to the system security >>> class). >>> >>> Patch 8 adds some notes for adding future D-Bus interfaces. >> >> Thanks for working on this. Just to make sure I didn't miss anything while >> reading the patches, there are no new permissions being added to the system >> class, correct? > > This is what i understand: > > Systemd first wants the regression fixed (re-add enable disable permission checks) > This phase should not add anything to the system class. It merely addresses a previous regression. I didn't look at all of the implementation details, but as long as there are no new permissions in the system class, I'm ok with the changes. > When that is done, then either the whole thing can be redone properly using policy capabilities, or we can add new permissions and in that scenario i guess the new permissions would be added to the existing system class (?). > If we redo the whole thing without interfering with the existing implementation then we can get rid of the system class usage for systemd. > One can opt in for the proper implementation by enabling the policy capability or otherwise one can stick with the current implementation. The correct thing to do with future changes is to remove the userspace permissions and put them in a new userspace object class (with appropriate compat, etc). If you're adding new permissions at the same time, a disabled policy capability would mean not checking the new permission; it wouldn't mean adding the new permission to the system class.
On Fri, Dec 20, 2019 at 09:43:09AM -0500, Chris PeBenito wrote: > On 12/20/19 5:54 AM, Dominick Grift wrote: > > On Thu, Dec 19, 2019 at 03:24:34PM -0500, Chris PeBenito wrote: > > > On 12/18/19 9:28 AM, Christian Göttsche wrote: > > > > Improve the SELinux support in systemd, especially re-adding checks for > > > > unit file operations, like enable, mask... > > > > > > > > The original pull request can be found at https://github.com/systemd/systemd/pull/10023 > > > > > > > > Patch 1 and 2 improve logging on failures in permissive mode. > > > > > > > > Patch 3 adds the ability to obtain the context for a masked unit. > > > > > > > > Patch 4 and 5 change several system und service checks. For better > > > > distinction two new permissions are introduced: modify and listdynusers. > > > > > > > > Patch 6 and 7 re-introduce checking unit file install operations. > > > > They were dropped in 8faae625dc9b6322db452937f54176e56e65265a . > > > > For consistency in the unexpected case while perforimg a service access > > > > check no path can be gathered, now the check will still be executed on > > > > the service security class (currently it switches to the system security > > > > class). > > > > > > > > Patch 8 adds some notes for adding future D-Bus interfaces. > > > > > > Thanks for working on this. Just to make sure I didn't miss anything while > > > reading the patches, there are no new permissions being added to the system > > > class, correct? > > > > This is what i understand: > > > > Systemd first wants the regression fixed (re-add enable disable permission checks) > > This phase should not add anything to the system class. It merely addresses a previous regression. > > > I didn't look at all of the implementation details, but as long as there are > no new permissions in the system class, I'm ok with the changes. This patch series has already been rejected by systemd in the mean time, the reason being that they first want to only address the existing regression. Addding anything new requires a blessing from the various (distribution) maintainers (good luck with that). > > > > When that is done, then either the whole thing can be redone properly using policy capabilities, or we can add new permissions and in that scenario i guess the new permissions would be added to the existing system class (?). > > If we redo the whole thing without interfering with the existing implementation then we can get rid of the system class usage for systemd. > > One can opt in for the proper implementation by enabling the policy capability or otherwise one can stick with the current implementation. > The correct thing to do with future changes is to remove the userspace > permissions and put them in a new userspace object class (with appropriate > compat, etc). If you're adding new permissions at the same time, a disabled > policy capability would mean not checking the new permission; it wouldn't > mean adding the new permission to the system class. > > -- > Chris PeBenito