Re: Suggestion for Capability Check Refinement in check_syslog_permissions()

[Petr Mladek]

On Wed 2024-01-03 07:59:18, Greg KH wrote:
> On Wed, Jan 03, 2024 at 01:00:58PM +0800, 孟敬姿 wrote:
> > Hi, we suggest revisiting the capability checks in
> > check_syslog_permissions(). Currently CAP_SYSLOG is checked first, and
> > if it’s not there but there is a CAP_SYS_ADMIN, it can also pass the
> > check. We recommend refining this check to exclusively use CAP_SYSLOG.
> > Here's our reasoning for this suggestion:
> 
> Again, have you tested this?

I guess that Meng is right. The current code looks like:

static int check_syslog_permissions(int type, int source)
{
        /*
         * If this is from /proc/kmsg and we've already opened it, then we've
         * already done the capabilities checks at open time.
         */
        if (source == SYSLOG_FROM_PROC && type != SYSLOG_ACTION_OPEN)
                goto ok;

        if (syslog_action_restricted(type)) {
                if (capable(CAP_SYSLOG))
                        goto ok;
                /*
                 * For historical reasons, accept CAP_SYS_ADMIN too, with
                 * a warning.
                 */
                if (capable(CAP_SYS_ADMIN)) {
                        pr_warn_once("%s (%d): Attempt to access syslog with "
                                     "CAP_SYS_ADMIN but no CAP_SYSLOG "
                                     "(deprecated).\n",
                                 current->comm, task_pid_nr(current));
                        goto ok;
                }
                return -EPERM;
        }
ok:
        return security_syslog(type);
}

And CAP_SYS_ADMIN has really been deprecated last 13 years, see the
commit ee24aebffb75a7f940cf ("cap_syslog: accept CAP_SYS_ADMIN for now").

Maybe, it is really time to remove it.

Best Regards,
Petr