IPC auditing (Was: Re: Another slab size-32 leak 2.6.16-rc4-mm2)

[Stephen Smalley]

On Tue, 2006-02-28 at 15:20 +0100, Stephan Mueller wrote:
> [...]
> 
> error_path:
>        kfree(ctx);
>        audit_panic("error in audit_ipc_context");
> 
> You free ctx although it has not been allocated - I do not like that ;-)

Common convention is to initialize ctx to NULL upon declaration, and
then such kfree() calls are safe on the error path.

> Please tell me if I misunderstand anything: The question for you 
> is whether to keep this function. This function is about to collect labels 
> for IPC functions. If this function is gone, you cannot audit any labels for 
> the IPC functions any more?!

IIUC, they are asking about which IPC functions/operations require
collection of the labels, not whether the labels should be audited at
all.  At present, audit_ipc_context is being called indirectly via
audit_ipc_perms from various IPC_SET operations and directly from
ipcperms() (but the latter is being proposed for removal).  I think that
there is some confusion here because in the CAPP case, the basic syscall
auditing provides much of the required information and you therefore
don't need additional hooks except in the IPC_SET case?  Whereas in the
LSPP case we need to always be collecting the label information?

> If this is the case, then this function needs to stay, because:
> 
> - syscalls msg*, sem*, shm* (except shmdt) do DAC checks - now, they also 
> perform MAC checks by calling appropriate SELinux hooks (as required by the 
> ST as IPC mechanisms are subject to MAC - I think shmdt now must also be 
> subject to MAC at least), then these syscalls must perform audit

shmdt merely detaches a previously attached segment from the address
space of the calling process - there is nothing new to check there, and
the real work is all deferred anyway until the close method is called.
If the process was authorized to shmat() the object in the first place,
then it should be allowed to shmdt() it from its own address space.

> - the audit requirement for IPC calls is specified in FAU_GEN.1.1 LSPP in the 
> table: "All decisions on requests for information flow" must be audited
> 
> - FAU_SAR.3 LSPP requires that subject and object sensitivity labels are to be 
> audited.
> 
> Ergo, the functionality in question must stay.

It sounds like the functionality should actually be expanded based on
the above.  Possibly even calling audit_ipc_context() from the SELinux
hooks themselves to capture all cases.

-- 
Stephen Smalley
National Security Agency