linux-kernel - Re: Suggestion for Capability Check Refinement in check_syslog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZZZ92cjhyr_CgLRJ@alley>
Date: Thu, 4 Jan 2024 10:43:53 +0100
From: Petr Mladek <pmladek@...e.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: 孟敬姿 <mengjingzi@....ac.cn>, rostedt@...dmis.org,
	john.ogness@...utronix.de, senozhatsky@...omium.org,
	keescook@...omium.org, tony.luck@...el.com, gpiccoli@...lia.com,
	linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...uxfoundation.org>
Subject: Re: Suggestion for Capability Check Refinement in
 check_syslog_permissions()

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