search

bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
675 stars 78 forks source link

util/selinux: Change auditing based on the log type. #238

Closed pebenito closed 4 years ago

pebenito commented 4 years ago

The SELinux log callback includes a message type. Not all messages are auditable and those that are don't have the different audit types. Update the auditing accordingly.

If the message is not auditable, write it to stderr.

Signed-off-by: Chris PeBenito chpebeni@linux.microsoft.com

dvdhrm commented 4 years ago

Before I merge this, can you give me a short background info on this? Is there any harm in passing things to libaudit which do not match AVC/ERROR? Or is this just about matching the right error to the right audit-call, so we don't accidentally trigger audit messages for things that should not have triggered them?

pebenito commented 4 years ago

Audit messages are for security events, so informational messages generally aren't audited. Not everything coming out of the libselinux log callback should be audited. This is a problem for systems that scan audit logs for certain audit types to initiate incident actions.

When you load a new SELinux policy you get this:

audit[364]: USER_AVC pid=364 uid=999 auid=4294967295 ses=4294967295 subj=system_u:system_r:system_dbusd_t msg='avc:  received policyload notice (seqno=2) exe="/usr/bin/dbus-broker" sauid=999 hostname=? addr=? terminal=?'

USER_AVC is not the correct audit message, since nothing is being denied. It's the code acknowledging that it has done operations in response to a policy load.

This PR is the first step to comprehensively fix this problem. It does have a residual problem due to a libselinux issue. libselinux isn't providing its own log message types so callers can determine if something should be audited. In the above case, it really should be audited as USER_MAC_POLICY_LOAD, but you can't tell in the libselinux callback because it currently has the policy load message as an "INFO" log level. I have a patch to that upstream to fix it. I also have PR linux-audit/audit-userspace#135 to add USER_MAC_STATUS audit event type to audit one more event coming from libselinux.

Once libselinux and libaudit is fixed I'll follow up with another PR to add on additional audit message types.

dvdhrm commented 4 years ago

Audit messages are for security events, so informational messages generally aren't audited. Not everything coming out of the libselinux log callback should be audited. This is a problem for systems that scan audit logs for certain audit types to initiate incident actions.

Yeah, fair enough. We should definitely match the logging types. Thanks for the background info!

pebenito commented 4 years ago

I think this addresses all of your comments.

dvdhrm commented 4 years ago

Thank you very much! Not sure when I will do a next release, but I guess sometime September.