linux-audit / audit-kernel

GitHub mirror of the Linux Kernel's audit repository
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
Other
140 stars 37 forks source link

Q: delete redundant fields in accompanied records #51

Closed rgbriggs closed 6 years ago

rgbriggs commented 7 years ago

The following records use an existing context but also include some subject attributes that duplicate information already provided by the SYSCALL record.

It appears to me that these records should either be made autonomous (NULL context) and have the subject attributes completed, or have the redundant fields dropped (auid, ses).

pcmoore commented 7 years ago

Just a reminder that my general policy on removing existing fields is "no".

rgbriggs commented 7 years ago

@stevegrubb First thing I seek is some insight as to why these were added to records that accompany SYSCALL records and whether they were ever or are still needed to avoid adding them to any more records.

pcmoore commented 6 years ago

@rgbriggs It has been several months with no real discussion or answers here, I'm wondering if we either let this drop (see my comments about removing fields, and my general aversion to unaccompanied records) or move it to a different communications channel?

I would like to avoid letting the issue tracker fill up with open items that are dead. If this is something that you feel needs to be discussed in the next few moths, let's start that discussion now.

stevegrubb commented 6 years ago

These should be standalone records. The only events that should have a SYSCALL record is something that is triggered by a syscall rule. Typically a SYSCALL record logs way too much irrelevant information.

pcmoore commented 6 years ago

If we have an event that occurs on the system that is important enough to warrant triggering an audit event, it seems as though we would want to record, and associate/connect all the related information. You need to convince me that this is not the case, and right now "a SYSCALL record logs way too much irrelevant information" isn't very compelling.

stevegrubb commented 6 years ago

A Syscall record logs too much information that is irrelevant. This wastes disk space and slows down parsing because now we have all this extra data that is not required per CC docs. For example, if you have a AUDIT_MAC_UNLBL_ALLOW event, what does gid, egid, fsguid, or even arch or syscall add to what we know about the event? They are not used in determining access and are therefore not relevant. Not to mention, all those interpretable fields have to be resolved immediately and add even more to wasted disk space as well as performance loss. This performance loss ripples through the system because the kernel has to resolve more fields and write them via printk, selinux troubleshooter has to look at more fields, audisp has to allocate more space, remote logging has to allocate more memory and persistent queue, and the data transferred is significantly bigger. And for what gain?

rgbriggs commented 6 years ago

If syscalls are being logged, then having these as disjoint events is misleading because they are part of the same event and should be linked despite there being redundant information, to make it clear it isn't two distinct events. I was trying to understand how to make syscalls and autonomous events co-exist without duplication and yet make it clear they are related if they both happen. The best answer I have is to remove the task information from the syscall record and put it in its own auxiliary records, one for basic task info and the other for extended task info, but that won't happen in the present situation of field ordering and removal restrictions.

What happens if CONFIG_AUDIT is set while CONFIG_AUDITSYSCALL is not (as is the situation for fedora RasPi)? If that is considered an unsupported configuration, then maybe it should be removed or made obvious. Is it time to remove the CONFIG_AUDITSYSCALL config option altogether and force it on if CONFIG_AUDIT is set?

pcmoore commented 6 years ago

Is it time to remove the CONFIG_AUDITSYSCALL config option altogether and force it on if CONFIG_AUDIT is set?

Done in commit cb74ed278f8054fddf79ed930495b9e214f7c7b2:

audit: always enable syscall auditing when supported and audit is enabled

To the best of our knowledge, everyone who enables audit at compile
time also enables syscall auditing; this patch simplifies the Kconfig
menus by removing the option to disable syscall auditing when audit
is selected and the target arch supports it.

Signed-off-by: Paul Moore <pmoore@redhat.com>
pcmoore commented 6 years ago

If syscalls are being logged, then having these as disjoint events is misleading because they are part of the same event and should be linked despite there being redundant information, to make it clear it isn't two distinct events.

Agreed. This is one of the reasons why I think all records should be "associated" whenever possible.

rgbriggs commented 6 years ago

On 2018-02-14 22:01, Paul Moore wrote:

Is it time to remove the CONFIG_AUDITSYSCALL config option altogether and force it on if CONFIG_AUDIT is set?

Done in commit cb74ed278f8054fddf79ed930495b9e214f7c7b2:

audit: always enable syscall auditing when supported and audit is enabled

Ah, very good. I thought this sounded familiar. This definitely simplifies the path forward.

rgbriggs commented 6 years ago

@pcmoore I think this one can be closed since it doesn't make sense to throw out event information records and at this point deleting redundant fields isn't possible.