Re: let's revert e3cab998b48ab293a9962faf9779d70ca339c65d

[Dominick Grift <dac.override@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4328 bytes --]

On Fri, Apr 14, 2017 at 01:56:30PM -0400, Stephen Smalley wrote:
> On Fri, 2017-04-14 at 13:47 -0400, Daniel Walsh wrote:
> > On 04/14/2017 11:33 AM, Stephen Smalley wrote:
> > > On Fri, 2017-04-14 at 16:57 +0200, Dominick Grift wrote:
> > > > Bear with me please, because i might not fully grasp the issue (i
> > > > received help with diagnosing this issue):
> > > > 
> > > > This commit causes issues (and is, i think, a lousy hack):
> > > > e3cab998b48ab293a9962faf9779d70ca339c65d
> > > > 
> > > > The commit causes entities to "think" that SELinux is disabled
> > > > after
> > > > "mount -o remount,ro /sys/fs/selinux
> > > > 
> > > > It is "neat" to be able to make processes "think" that selinux is
> > > > disabled on a selinux enabled system but not if it break anything
> > > > 
> > > > The above results in the following:
> > > > 
> > > > Systemd services that have ProtectKernelTunables=yes set in their
> > > > respective service units, think that SELinux is disabled.
> > > > 
> > > > However we have found that some of these services actually rely
> > > > on
> > > > SELinux to ensure proper labeling.
> > > > 
> > > > So we have the option to make people aware that if you set
> > > > ProtectKernelTunables=yes that then the process cannot be
> > > > SELinux-
> > > > aware properly, or we can just get rid of the commit above and
> > > > just
> > > > accept that process know that SELinux is enabled.
> > > > 
> > > > Actual bug that caused me to look into this: systemd-localed
> > > > selinux
> > > > awareness is broken due it having ProtectKernelTunables=yes in
> > > > its
> > > > service unit
> > > 
> > > If selinuxfs is mounted read-only, then they can't use most of the
> > > selinuxfs interfaces, including even the ability to validate or
> > > canonicalize security contexts.  That will break most SELinux-aware
> > > services if we tell them that SELinux is enabled.  Are you sure
> > > systemd-localed would actually work if you told it SELinux was
> > > enabled
> > > when selinuxfs was mounted read-only?  What SELinux interfaces is
> > > it
> > > using?
> > > 
> > > The other question is whether ProtectKernelTunables ought to be
> > > mounting selinuxfs read-only.  SELinux already controls the ability
> > > to
> > > use its interfaces, including limiting even root, so it is unclear
> > > what
> > > benefit we derive from having systemd add a further restriction on
> > > top.
> > > 
> > 
> > Why is selinuxfs mounted readonly in this case?
> 
> I don't actually see this in upstream systemd unless I am just missing
> it.
> 
> systemd/src/core/namespace.c:
> /* ProtectKernelTunables= option and the related filesystem APIs */
> static const MountEntry protect_kernel_tunables_table[] = {
>         { "/proc/sys",           READONLY,     false },
>         { "/proc/sysrq-trigger", READONLY,     true  },
>         { "/proc/latency_stats", READONLY,     true  },
>         { "/proc/mtrr",          READONLY,     true  },
>         { "/proc/apm",           READONLY,     true  }, /* Obsolete
> API, there's no point in permitting access to this, ever */
>         { "/proc/acpi",          READONLY,     true  },
>         { "/proc/timer_stats",   READONLY,     true  },
>         { "/proc/asound",        READONLY,     true  },
>         { "/proc/bus",           READONLY,     true  },
>         { "/proc/fs",            READONLY,     true  },
>         { "/proc/irq",           READONLY,     true  },
>         { "/sys",                READONLY,     false },
>         { "/sys/kernel/debug",   READONLY,     true  },
>         { "/sys/kernel/tracing", READONLY,     true  },
>         { "/sys/fs/cgroup",      READWRITE,    false }, /* READONLY is
> set by ProtectControlGroups= option */
> };
> 
> No mention of selinuxfs at all.  Maybe it is a Fedora patch?
> 
> > The reason we want this is so that processes inside of containers do
> > not
> > attempt to do SELinux stuff. 
> > 
> > http://danwalsh.livejournal.com/73099.html

Before one dismisses my concern (8 minute proof):

https://www.youtube.com/watch?v=YqiM1MlOG0w

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]