proc_loginuid_write() checks wrong capability?

proc_loginuid_write() checks wrong capability?

[Steve Beattie]

[-- Attachment #1.1: Type: text/plain, Size: 1425 bytes --]

Hi,

Looking at the code for proc_loginuid_write() in Linus' git tree, the
capability CAP_AUDIT_CONTROL is needed to write to /proc/pid/loginuid
and generate LOGIN type records. This seems to run counter to the
capabilities(7) manpage, which suggests that CAP_AUDIT_CONTROL is to
"Enable and disable kernel auditing; change auditing filter rules;
retrieve auditing status and filtering rules", whereas CAP_AUDIT_WRITE
is to "Allow records to be written to kernel auditing log."

Should the following patch be applied, or am I misunderstanding
something? It doesn't seem quite right that anything that makes use of
pam_loginuid.so should need to be granted the capability that allows
enabling and disabling kernel auditing or changing filter rules.

Signed-off-by: Steve Beattie <sbeattie@suse.de>
---
 fs/proc/base.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: kernel-linus/fs/proc/base.c
===================================================================
--- kernel-linus.orig/fs/proc/base.c
+++ kernel-linus/fs/proc/base.c
@@ -741,7 +741,7 @@ static ssize_t proc_loginuid_write(struc
 	ssize_t length;
 	uid_t loginuid;
 
-	if (!capable(CAP_AUDIT_CONTROL))
+	if (!capable(CAP_AUDIT_WRITE))
 		return -EPERM;
 
 	if (current != pid_task(proc_pid(inode), PIDTYPE_PID))

Thanks.
-- 
Steve Beattie
SUSE Labs, Novell Inc. 
<sbeattie@suse.de>
http://NxNW.org/~steve/

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: proc_loginuid_write() checks wrong capability?

[Stephen Smalley]

On Tue, 2007-02-06 at 10:27 -0800, Steve Beattie wrote:
> Hi,
> 
> Looking at the code for proc_loginuid_write() in Linus' git tree, the
> capability CAP_AUDIT_CONTROL is needed to write to /proc/pid/loginuid
> and generate LOGIN type records. This seems to run counter to the
> capabilities(7) manpage, which suggests that CAP_AUDIT_CONTROL is to
> "Enable and disable kernel auditing; change auditing filter rules;
> retrieve auditing status and filtering rules", whereas CAP_AUDIT_WRITE
> is to "Allow records to be written to kernel auditing log."

Setting the loginuid of a process is a form of "control" over the audit
system, as the loginuid is the basis for user accountability in the
audit framework.    It differs from merely generating a user audit
message.  There was some discussion of introducing a third audit
capability, but no support for it.

Note btw that it is possible to separately control the netlink audit
interface as SELinux does in order to impose additional requirements on
those operations (nlmsg_read, _write, _readpriv, and _relay in SELinux).

> Should the following patch be applied, or am I misunderstanding
> something? It doesn't seem quite right that anything that makes use of
> pam_loginuid.so should need to be granted the capability that allows
> enabling and disabling kernel auditing or changing filter rules.
> 
> Signed-off-by: Steve Beattie <sbeattie@suse.de>
> ---
>  fs/proc/base.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: kernel-linus/fs/proc/base.c
> ===================================================================
> --- kernel-linus.orig/fs/proc/base.c
> +++ kernel-linus/fs/proc/base.c
> @@ -741,7 +741,7 @@ static ssize_t proc_loginuid_write(struc
>  	ssize_t length;
>  	uid_t loginuid;
>  
> -	if (!capable(CAP_AUDIT_CONTROL))
> +	if (!capable(CAP_AUDIT_WRITE))
>  		return -EPERM;
>  
>  	if (current != pid_task(proc_pid(inode), PIDTYPE_PID))
> 
> Thanks.
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
-- 
Stephen Smalley
National Security Agency

Re: proc_loginuid_write() checks wrong capability?

[Steve Beattie]

[-- Attachment #1.1: Type: text/plain, Size: 663 bytes --]

On Tue, Feb 06, 2007 at 02:08:38PM -0500, Stephen Smalley wrote:
> Setting the loginuid of a process is a form of "control" over the audit
> system, as the loginuid is the basis for user accountability in the
> audit framework.    It differs from merely generating a user audit
> message.  There was some discussion of introducing a third audit
> capability, but no support for it.

Ah, thanks Stephen and Casey, for explaining the reasoning. It does have
the unfortunate side effect of causing CAP_AUDIT_CONTROL to be needed
more widely than one might expect.

-- 
Steve Beattie
SUSE Labs, Novell Inc. 
<sbeattie@suse.de>
http://NxNW.org/~steve/

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: proc_loginuid_write() checks wrong capability?

[Casey Schaufler]

--- Steve Beattie <sbeattie@suse.de> wrote:


> Ah, thanks Stephen and Casey, for explaining the
> reasoning. It does have
> the unfortunate side effect of causing
> CAP_AUDIT_CONTROL to be needed
> more widely than one might expect.

The appropriate granularity of capabilities
has always been and will always be a
contentious issue, with the fashion shifting
whimsically. Writing audit records is pretty
clearly a different beast than setting audit
attributes, but since there is significant
overlap between the programs that set audit
state and those that write audit records you
could make a case for either making a seperate
capability for setting the loginid or for
having a single CAP_AUDIT. Heck, at one time
or another I've argued each way. I expect that
the current granularity is sufficiently
obvious and useful to leave alone, at least
for the time being.


Casey Schaufler
casey@schaufler-ca.com

Re: proc_loginuid_write() checks wrong capability?

[Casey Schaufler]

--- Steve Beattie <sbeattie@suse.de> wrote:

> Hi,
> 
> Looking at the code for proc_loginuid_write() in
> Linus' git tree, the
> capability CAP_AUDIT_CONTROL is needed to write to
> /proc/pid/loginuid
> and generate LOGIN type records. This seems to run
> counter to the
> capabilities(7) manpage, which suggests that
> CAP_AUDIT_CONTROL is to
> "Enable and disable kernel auditing; change auditing
> filter rules;
> retrieve auditing status and filtering rules",
> whereas CAP_AUDIT_WRITE
> is to "Allow records to be written to kernel
> auditing log."
> 
> Should the following patch be applied, or am I
> misunderstanding something?

The latter. CAP_AUDIT_WRITE allows you to
create audit records, and that's it. It does
not allow you to change how they're managed,
which is an important aspect of the loginuid
of a process. Updating the loginuid changes
information that will go into audit records,
and that is strongly related to "filtering
rules". 

> It doesn't seem quite right that anything
> that makes use of
> pam_loginuid.so should need to be granted the
> capability that allows
> enabling and disabling kernel auditing or changing
> filter rules.

Although the current audit system doesn't
do so (at least, I don't think it does, I
could be wrong) specifiying audit charactoristics
on a per-session basis would require that
capability.


Casey Schaufler
casey@schaufler-ca.com