Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

["Zack Weinberg" <zackw@panix.com>]

[some of you have seen this already in off-list discussion, but it
really should have gone to the list, apologies for the noise]

On 11/9/06, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2006-11-08 at 20:14 -0800, Chris Wright wrote:
> > * Sergey Vlasov (vsu@altlinux.ru) wrote:
> > > Then what would you think about another solution:
> > >
> > >  1) When sys_syslog() is called with commands 2 (read) or 9 (get unread
> > >     count), additionally call security_syslog(1) to check that the
> > >     process has permissions to open the kernel log.  This change by
> > >     itself will not make any difference, because all existing
> > >     implementations of the security_ops->syslog hook treat the operation
> > >     codes 1, 2 and 9 the same way.
> > >
> > >  2) Change cap_syslog() and dummy_syslog() to permit commands 2 and 9
> > >     for unprivileged users, in addition to 3 and 10 which are currently
> > >     permitted.  This will not really permit access through sys_syslog()
> > >     due to the added security_syslog(1) check, but if a process somehow
> > >     got access to an open file descriptor for /proc/kmsg, it would be
> > >     able to read from it.  Also, because selinux_syslog() is not
> > >     changed, under SELinux the process will still need to have
> > >     additional privileges even if it has /proc/kmsg open.
> >
> > It's a bit clumsy in the extra caveats for sys_syslog and cap_syslog,
> > but does achieve what you're after.  We lose default checking on the
> > actual read access, but perhaps this is a fair tradeoff.  Stephen,
> > James do you have any issues with this for SELinux?
>
> It makes the already unfortunate coupling between the security modules
> and the core syslog code even worse, by making assumptions about how the
> security modules treat different type codes.  If you were to go down
> this route, I think you would want to map the type codes to abstract
> permissions in the core syslog code and only pass the abstract
> permissions to the security modules (so that they no longer see 2, 9,
> and 1 separately but as a single permission).  Might be nice to have
> actual #define's for the type codes too.
>
> We wouldn't want the SELinux checking changed; we can already run klogd
> confined by policy, and decomposition into privileged and unprivileged
> components is preferable to privilege bracketing within a single
> component.

I've been working on an alternate patch.  It is not ready yet, but
tell me how this sounds:

1) Move the security check from do_syslog to sys_syslog, as in my
original patch.
2) Add a call to security_syslog() to kmsg_open.
3) sys_syslog() operations 0 and 1 ("close" and "open" respectively -
both do precisely nothing) cease to call the security hook.  (This
part is so that a security module knows that a call of its ->syslog
hook with code 1 is an attempt to open /proc/kmsg, not a no-op
syslog(1, 0, 0).  Security modules never see code 0, on the principle
that close shouldn't ever fail.)

-- At this point the user visible semantics (in the default security
framework) are that a process without CAP_SYS_ADMIN cannot open
/proc/kmsg and cannot call any of the syslog() interfaces that
actually do stuff [except the one "dmesg" uses].  However, a process
without CAP_SYS_ADMIN will be able to issue a read() on a fd open on
/proc/kmsg if it somehow gets hold of one.  (This is a tidier way of
getting the semantics that Sergey's patches provide.)

Then I also want to have:

4) Change the default security policy modules (commoncap.c and
dummy.c) so that a process without CAP_SYS_ADMIN is allowed to open
/proc/kmsg provided that DAC allows it (i.e. provided that root has
gone and chowned it to the appropriate uid).  No change to SELinux is
required.

Part 4 allows me to run an *unmodified* klogd as an unprivileged user,
and just chown /proc/kmsg in the init script, rather than having to
modify klogd to open /proc/kmsg and then drop privileges.  (A SELinux
environment still requires modifications to klogd, but we were going
to have to do that anyway.)

[Possibly as long as we are futzing with syslog(), we ought to
consider renaming it to match glibc [which calls it klogctl()] and
defining some symbolic constants for all those opcode numbers.  Also I
really don't like the looks of the code for operations 3 and 4.  But
this is all tangential.]

[re Stephen's comment: I don't like the idea of mapping type codes to
abstract permissions in the core; that seems like more of a layering
violation than what we have now.  Surely it is the core's business to
say _what_ it is about to do, and the security module's to decide what
that means in terms of its privilege model.  I don't propose to change
SELinux, except insofar as (in my above plan) it would no longer see
certain operations on the syscall interface that don't do anything
anyway.]

zw