Message ID | 20191211134423.2357254-1-dac.override@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | [v2] mcstrans: start early and stop late | expand |
On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: > It stopped too early, exposing a bug in sudo selinux_restore_tty(): > > SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" > avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 > > If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP Unfortunately it does not quite seem to address this challenge, at least currently, but still I think systemd would need to refresh its label cache when mcstrans is started, as per systemd v245 that should be a little less painful than it is today Something like a: ExecStartPost=/bin/systemctl daemon-reload would do that then > > v2: stop late, but do stop > Signed-off-by: Dominick Grift <dac.override@gmail.com> > --- > mcstrans/src/mcstrans.service | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service > index 09529432..c13cd09a 100644 > --- a/mcstrans/src/mcstrans.service > +++ b/mcstrans/src/mcstrans.service > @@ -2,6 +2,9 @@ > Description=Translates SELinux MCS/MLS labels to human readable form > Documentation=man:mcstransd(8) > ConditionSecurity=selinux > +DefaultDependencies=no > +Before=shutdown.target sysinit.target > +Conflicts=shutdown.target > > [Service] > ExecStart=/sbin/mcstransd -f > -- > 2.24.0 >
On 12/11/19 9:21 AM, Dominick Grift wrote: > On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: >> It stopped too early, exposing a bug in sudo selinux_restore_tty(): >> >> SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" >> avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 >> >> If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP > > Unfortunately it does not quite seem to address this challenge, at least currently, but still > I think systemd would need to refresh its label cache when mcstrans is started, as per systemd v245 that should be a little less painful than it is today > Something like a: ExecStartPost=/bin/systemctl daemon-reload would do that then I'm a little unclear on where the bug lies - you show a sudo denial, but refer to systemd as the culprit? If we don't care about being able to use translated contexts in systemd unit files or options, it could always use the _raw interfaces to ensure that it is always dealing with the raw kernel contexts. The translated contexts are mostly for display purposes for MLS labels/policies. > >> >> v2: stop late, but do stop >> Signed-off-by: Dominick Grift <dac.override@gmail.com> >> --- >> mcstrans/src/mcstrans.service | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service >> index 09529432..c13cd09a 100644 >> --- a/mcstrans/src/mcstrans.service >> +++ b/mcstrans/src/mcstrans.service >> @@ -2,6 +2,9 @@ >> Description=Translates SELinux MCS/MLS labels to human readable form >> Documentation=man:mcstransd(8) >> ConditionSecurity=selinux >> +DefaultDependencies=no >> +Before=shutdown.target sysinit.target >> +Conflicts=shutdown.target >> >> [Service] >> ExecStart=/sbin/mcstransd -f >> -- >> 2.24.0 >> >
On Thu, Dec 12, 2019 at 08:45:29AM -0500, Stephen Smalley wrote: > On 12/11/19 9:21 AM, Dominick Grift wrote: > > On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: > > > It stopped too early, exposing a bug in sudo selinux_restore_tty(): > > > > > > SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" > > > avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 > > > > > > If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP > > > > Unfortunately it does not quite seem to address this challenge, at least currently, but still > > I think systemd would need to refresh its label cache when mcstrans is started, as per systemd v245 that should be a little less painful than it is today > > Something like a: ExecStartPost=/bin/systemctl daemon-reload would do that then > > I'm a little unclear on where the bug lies - you show a sudo denial, but > refer to systemd as the culprit? The sudo bug is fixed here: https://github.com/sudo-project/sudo/commit/718e6997fcaae6ea065ce74d08dd4aae5917df5e > > If we don't care about being able to use translated contexts in systemd unit > files or options, it could always use the _raw interfaces to ensure that it > is always dealing with the raw kernel contexts. The translated contexts are > mostly for display purposes for MLS labels/policies. The thing with systemd is that since systemd runs before mcstrans is started it doesnt use mcstrans. So if you try to reference translated contexts using systemd then it will refuse. Running a systemctl daemon-reexec after mcstrans is started fixes that issue but that is not really an option. I am wondering what causes this behavior, i suppose this is some libselinux thing. Regardless of all the above though, i think it makes sense for mcstrans to start early and stop late. > > > > > > > > > v2: stop late, but do stop > > > Signed-off-by: Dominick Grift <dac.override@gmail.com> > > > --- > > > mcstrans/src/mcstrans.service | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service > > > index 09529432..c13cd09a 100644 > > > --- a/mcstrans/src/mcstrans.service > > > +++ b/mcstrans/src/mcstrans.service > > > @@ -2,6 +2,9 @@ > > > Description=Translates SELinux MCS/MLS labels to human readable form > > > Documentation=man:mcstransd(8) > > > ConditionSecurity=selinux > > > +DefaultDependencies=no > > > +Before=shutdown.target sysinit.target > > > +Conflicts=shutdown.target > > > [Service] > > > ExecStart=/sbin/mcstransd -f > > > -- > > > 2.24.0 > > > > > >
On Thu, Dec 12, 2019 at 03:24:36PM +0100, Dominick Grift wrote: > On Thu, Dec 12, 2019 at 08:45:29AM -0500, Stephen Smalley wrote: > > On 12/11/19 9:21 AM, Dominick Grift wrote: > > > On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: > > > > It stopped too early, exposing a bug in sudo selinux_restore_tty(): > > > > > > > > SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" > > > > avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 > > > > > > > > If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP > > > > > > Unfortunately it does not quite seem to address this challenge, at least currently, but still > > > I think systemd would need to refresh its label cache when mcstrans is started, as per systemd v245 that should be a little less painful than it is today > > > Something like a: ExecStartPost=/bin/systemctl daemon-reload would do that then > > > > I'm a little unclear on where the bug lies - you show a sudo denial, but > > refer to systemd as the culprit? > > The sudo bug is fixed here: https://github.com/sudo-project/sudo/commit/718e6997fcaae6ea065ce74d08dd4aae5917df5e > > > > > If we don't care about being able to use translated contexts in systemd unit > > files or options, it could always use the _raw interfaces to ensure that it > > is always dealing with the raw kernel contexts. The translated contexts are > > mostly for display purposes for MLS labels/policies. The bevavior of mcstrans seems to contradict your suggestion that its mostly for display purposes. If you reference a set of categories in the raw manner then mcstrans seems to not resolve that until that same set of categories is referenced in human readable form. It think it actually makes sense that people reference catagories in human readable form rather than in a raw manner when they opt in for mcstrans. > > The thing with systemd is that since systemd runs before mcstrans is started it doesnt use mcstrans. > So if you try to reference translated contexts using systemd then it will refuse. > Running a systemctl daemon-reexec after mcstrans is started fixes that issue but that is not really an option. > > I am wondering what causes this behavior, i suppose this is some libselinux thing. > > Regardless of all the above though, i think it makes sense for mcstrans to start early and stop late. > > > > > > > > > > > > > > v2: stop late, but do stop > > > > Signed-off-by: Dominick Grift <dac.override@gmail.com> > > > > --- > > > > mcstrans/src/mcstrans.service | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service > > > > index 09529432..c13cd09a 100644 > > > > --- a/mcstrans/src/mcstrans.service > > > > +++ b/mcstrans/src/mcstrans.service > > > > @@ -2,6 +2,9 @@ > > > > Description=Translates SELinux MCS/MLS labels to human readable form > > > > Documentation=man:mcstransd(8) > > > > ConditionSecurity=selinux > > > > +DefaultDependencies=no > > > > +Before=shutdown.target sysinit.target > > > > +Conflicts=shutdown.target > > > > [Service] > > > > ExecStart=/sbin/mcstransd -f > > > > -- > > > > 2.24.0 > > > > > > > > > > > -- > Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 > https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 > Dominick Grift
On 12/12/19 9:24 AM, Dominick Grift wrote: > On Thu, Dec 12, 2019 at 08:45:29AM -0500, Stephen Smalley wrote: >> On 12/11/19 9:21 AM, Dominick Grift wrote: >>> On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: >>>> It stopped too early, exposing a bug in sudo selinux_restore_tty(): >>>> >>>> SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" >>>> avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 >>>> >>>> If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP >>> >>> Unfortunately it does not quite seem to address this challenge, at least currently, but still >>> I think systemd would need to refresh its label cache when mcstrans is started, as per systemd v245 that should be a little less painful than it is today >>> Something like a: ExecStartPost=/bin/systemctl daemon-reload would do that then >> >> I'm a little unclear on where the bug lies - you show a sudo denial, but >> refer to systemd as the culprit? > > The sudo bug is fixed here: https://github.com/sudo-project/sudo/commit/718e6997fcaae6ea065ce74d08dd4aae5917df5e > >> >> If we don't care about being able to use translated contexts in systemd unit >> files or options, it could always use the _raw interfaces to ensure that it >> is always dealing with the raw kernel contexts. The translated contexts are >> mostly for display purposes for MLS labels/policies. > > The thing with systemd is that since systemd runs before mcstrans is started it doesnt use mcstrans. > So if you try to reference translated contexts using systemd then it will refuse. > Running a systemctl daemon-reexec after mcstrans is started fixes that issue but that is not really an option. > > I am wondering what causes this behavior, i suppose this is some libselinux thing. Yes, libselinux checks for the existence of the mcstrans socket exactly once on the first attempted translation, and if it does not exist at that time, it never tries again. That is to avoid overhead on systems that are not running mcstrans, which is the majority of systems. mcstrans is only really needed for MLS policies and those are a minority of SELinux systems. > Regardless of all the above though, i think it makes sense for mcstrans to start early and stop late. Perhaps; I'll defer to the distro maintainers on that. I don't really recommend using mcstrans unless you truly need it. > >> >>> >>>> >>>> v2: stop late, but do stop >>>> Signed-off-by: Dominick Grift <dac.override@gmail.com> >>>> --- >>>> mcstrans/src/mcstrans.service | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service >>>> index 09529432..c13cd09a 100644 >>>> --- a/mcstrans/src/mcstrans.service >>>> +++ b/mcstrans/src/mcstrans.service >>>> @@ -2,6 +2,9 @@ >>>> Description=Translates SELinux MCS/MLS labels to human readable form >>>> Documentation=man:mcstransd(8) >>>> ConditionSecurity=selinux >>>> +DefaultDependencies=no >>>> +Before=shutdown.target sysinit.target >>>> +Conflicts=shutdown.target >>>> [Service] >>>> ExecStart=/sbin/mcstransd -f >>>> -- >>>> 2.24.0 >>>> >>> >> >
On Thu, Dec 12, 2019 at 09:36:19AM -0500, Stephen Smalley wrote: > On 12/12/19 9:24 AM, Dominick Grift wrote: > > On Thu, Dec 12, 2019 at 08:45:29AM -0500, Stephen Smalley wrote: > > > On 12/11/19 9:21 AM, Dominick Grift wrote: > > > > On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: > > > > > It stopped too early, exposing a bug in sudo selinux_restore_tty(): > > > > > > > > > > SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" > > > > > avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 > > > > > > > > > > If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP > > > > > > > > Unfortunately it does not quite seem to address this challenge, at least currently, but still > > > > I think systemd would need to refresh its label cache when mcstrans is started, as per systemd v245 that should be a little less painful than it is today > > > > Something like a: ExecStartPost=/bin/systemctl daemon-reload would do that then > > > > > > I'm a little unclear on where the bug lies - you show a sudo denial, but > > > refer to systemd as the culprit? > > > > The sudo bug is fixed here: https://github.com/sudo-project/sudo/commit/718e6997fcaae6ea065ce74d08dd4aae5917df5e > > > > > > > > If we don't care about being able to use translated contexts in systemd unit > > > files or options, it could always use the _raw interfaces to ensure that it > > > is always dealing with the raw kernel contexts. The translated contexts are > > > mostly for display purposes for MLS labels/policies. > > > > The thing with systemd is that since systemd runs before mcstrans is started it doesnt use mcstrans. > > So if you try to reference translated contexts using systemd then it will refuse. > > Running a systemctl daemon-reexec after mcstrans is started fixes that issue but that is not really an option. > > > > I am wondering what causes this behavior, i suppose this is some libselinux thing. > > Yes, libselinux checks for the existence of the mcstrans socket exactly once > on the first attempted translation, and if it does not exist at that time, > it never tries again. That is to avoid overhead on systems that are not > running mcstrans, which is the majority of systems. mcstrans is only really > needed for MLS policies and those are a minority of SELinux systems. > > > Regardless of all the above though, i think it makes sense for mcstrans to start early and stop late. > > Perhaps; I'll defer to the distro maintainers on that. I don't really > recommend using mcstrans unless you truly need it. Me neither. There was a request for support on IRC. So i looked into it. I certainly think I made it clear to stay away from it if possible. > > > > > > > > > > > > > > > > > > > > v2: stop late, but do stop > > > > > Signed-off-by: Dominick Grift <dac.override@gmail.com> > > > > > --- > > > > > mcstrans/src/mcstrans.service | 3 +++ > > > > > 1 file changed, 3 insertions(+) > > > > > > > > > > diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service > > > > > index 09529432..c13cd09a 100644 > > > > > --- a/mcstrans/src/mcstrans.service > > > > > +++ b/mcstrans/src/mcstrans.service > > > > > @@ -2,6 +2,9 @@ > > > > > Description=Translates SELinux MCS/MLS labels to human readable form > > > > > Documentation=man:mcstransd(8) > > > > > ConditionSecurity=selinux > > > > > +DefaultDependencies=no > > > > > +Before=shutdown.target sysinit.target > > > > > +Conflicts=shutdown.target > > > > > [Service] > > > > > ExecStart=/sbin/mcstransd -f > > > > > -- > > > > > 2.24.0 > > > > > > > > > > > > > > >
On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: > It stopped too early, exposing a bug in sudo selinux_restore_tty(): > > SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" > avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 > > If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP > > v2: stop late, but do stop > Signed-off-by: Dominick Grift <dac.override@gmail.com> I've asked for review one of Fedora systemd maintainers and he's fine with this change. He even suggested to follow udev and to drop 'Conflicts=shutdown.target' and let mstrans to be stopped by a signal when everything else is stopped as it was in your first version. I'm fine with both versions. Acked-by: Petr Lautrbach <plautrba@redhat.com> > --- > mcstrans/src/mcstrans.service | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service > index 09529432..c13cd09a 100644 > --- a/mcstrans/src/mcstrans.service > +++ b/mcstrans/src/mcstrans.service > @@ -2,6 +2,9 @@ > Description=Translates SELinux MCS/MLS labels to human readable form > Documentation=man:mcstransd(8) > ConditionSecurity=selinux > +DefaultDependencies=no > +Before=shutdown.target sysinit.target > +Conflicts=shutdown.target > > [Service] > ExecStart=/sbin/mcstransd -f
On Wed, Dec 11, 2019 at 02:44:23PM +0100, Dominick Grift wrote: > It stopped too early, exposing a bug in sudo selinux_restore_tty(): > > SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" > avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 > > If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP > > v2: stop late, but do stop > Signed-off-by: Dominick Grift <dac.override@gmail.com> > Acked-by: Petr Lautrbach <plautrba@redhat.com> Applied, thanks! > --- > mcstrans/src/mcstrans.service | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service > index 09529432..c13cd09a 100644 > --- a/mcstrans/src/mcstrans.service > +++ b/mcstrans/src/mcstrans.service > @@ -2,6 +2,9 @@ > Description=Translates SELinux MCS/MLS labels to human readable form > Documentation=man:mcstransd(8) > ConditionSecurity=selinux > +DefaultDependencies=no > +Before=shutdown.target sysinit.target > +Conflicts=shutdown.target > > [Service] > ExecStart=/sbin/mcstransd -f
diff --git a/mcstrans/src/mcstrans.service b/mcstrans/src/mcstrans.service index 09529432..c13cd09a 100644 --- a/mcstrans/src/mcstrans.service +++ b/mcstrans/src/mcstrans.service @@ -2,6 +2,9 @@ Description=Translates SELinux MCS/MLS labels to human readable form Documentation=man:mcstransd(8) ConditionSecurity=selinux +DefaultDependencies=no +Before=shutdown.target sysinit.target +Conflicts=shutdown.target [Service] ExecStart=/sbin/mcstransd -f
It stopped too early, exposing a bug in sudo selinux_restore_tty(): SELINUX_ERR op=setxattr invalid_context="wheel.id:wheel.role:users.terminals.pty.pty_file:SystemLow" avc: denied { mac_admin } for pid=859 comm="sudo" capability=33 scontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tcontext=wheel.id:wheel.role:sudo.wheel.subj:s0 tclass=capability2 permissive=0 If we want to be able to reference human readable contexts in SELinuxContext= and nspawn -Z and -L then we need mcstrans ASAP v2: stop late, but do stop Signed-off-by: Dominick Grift <dac.override@gmail.com> --- mcstrans/src/mcstrans.service | 3 +++ 1 file changed, 3 insertions(+)