Re: [PATCH 2/8] integrity: IMA as an integrity service provider

[Steve Grubb <sgrubb@redhat.com>]

On Sunday 08 February 2009 09:42:42 pm Mimi Zohar wrote:
> > > diff --git a/security/integrity/ima/ima_audit.c
> > > b/security/integrity/ima/ima_audit.c new file mode 100644
> > > index 0000000..8a0f1e2
> > > --- /dev/null
> > > +++ b/security/integrity/ima/ima_audit.c
> > > +void integrity_audit_msg(int audit_msgno, struct inode *inode,
> > > +			 const unsigned char *fname, const char *op,
> > > +			 const char *cause, int result, int audit_info)
> > > +{
> > > +	struct audit_buffer *ab;
> > > +
> > > +	if (!ima_audit && audit_info == 1) /* Skip informational messages */
> > > +		return;
> > > +
> > > +	ab = audit_log_start(current->audit_context, GFP_KERNEL,
> > > audit_msgno);
> >
> > Is this a standalone event or would it be an auxiliary record added to a
> > syscall record? IOW, if I have a watch on the same file as was being
> > measured, would the event have the same serial number?
>
> Having it as an auxiliary record to syscall seems appropriate. 

OK. There are are three options then. 

1) The first is to just supplement the syscall record whether its there or 
not. This is kind of what AVCs do. In that case, everything you are logging 
is fine. 
2) Change the audit event really be an auxiliary record that is output at 
syscall exit only if an audit event has triggered the event. This is more 
like what the PATH, CWD, or EXECVE records do.
3) The last approach is to cause a syscall audit record to be emitted. This is 
what the MAC_STATUS does. For this case and #2 above, we could shorten the 
data collected since the syscall record duplicates many fields.


> There seems to be different behavior if you stop auditd and if it isn't
> enabled at boot.  In the former case the syscall message still goes
> to /var/log/messages, while in the latter case it doesn't.

It all depends on if the audit system is enabled. If its enabled, events go to 
either the daemon or syslog. If the audit system is not enabled, then the 
admin didn't want any notification.

> > If you are depending on a syscall record, it will record much of what's
> > recorded here for itself and you won't need this much. If you wanted this
> > to be standalone, I think the call to audit_ log_start has NULL as the
> > first param so that it cannot be associated with a syscall.
>
> yes that works. Some of the messages don't need the syscall info, like
> in ima_parse_rules().

The perhaps you want 2 logging functions? Or maybe pass something to the 
function that determines if it should be related to an existing event or its 
entirely async.


> > > +	audit_log_task_context(ab);
> > > +	switch (audit_msgno) {
> > > +	case AUDIT_INTEGRITY_DATA:
> > > +	case AUDIT_INTEGRITY_METADATA:
> > > +	case AUDIT_INTEGRITY_PCR:
> > > +		audit_log_format(ab, " op=%s cause=%s", op, cause);
> > > +		break;
> > > +	case AUDIT_INTEGRITY_HASH:
> > > +		audit_log_format(ab, " op=%s hash=%s", op, cause);
> > > +		break;
> > > +	case AUDIT_INTEGRITY_STATUS:
> > > +	default:
> > > +		audit_log_format(ab, " op=%s", op);
> > > +	}
> > > +	audit_log_format(ab, " comm=");
> > > +	audit_log_untrustedstring(ab, current->comm);
> > > +	if (fname) {
> > > +		audit_log_format(ab, " name=");
> > > +		audit_log_untrustedstring(ab, fname);
>
> Sure would be nice to be able to print the full pathname.

There is an AUDIT_PATH record that should have the whole path. Its an 
auxiliary record emitted with the syscall.

Maybe we should talk a little about when the different events would be 
emitted. Would it be just on creation of the hash? Would it be just on a 
miscompare? IOW, how many records are likely to be created during a typical 
session?

Thanks,
-Steve