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: Add additional audit message types. #240

Closed pebenito closed 3 years ago

pebenito commented 3 years ago

This will cover all of the audit cases from the libselinux code.

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

pebenito commented 3 years ago

I left this as draft since it requires SELinuxProject/selinux@a4149e0 and linux-audit/audit-userspace@7eb3123 which have not yet been released in libselinux and libaudit, respectively.

pebenito commented 3 years ago

This change will result in audits like this:

Sep 14 13:53:25 qemu audit[392]: USER_MAC_POLICY_LOAD pid=392 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=?' Sep 14 13:54:13 qemu audit[392]: USER_MAC_STATUS pid=392 uid=999 auid=4294967295 ses=4294967295 subj=system_u:system_r:system_dbusd_t msg='avc: received setenforce notice (enforcing=1) exe="/usr/bin/dbus-broker" sauid=999 hostname=? addr=? terminal=?'

cc @stevegrubb

dvdhrm commented 3 years ago

This looks good to go once the dependencies are available. It will need a bump of the libaudit and libselinux dependencies, though (see meson.build and README.md). I will leave this PR open until we got those dependencies available.

stevegrubb commented 3 years ago

The audit system is very much name=value. There are a lot of text that will get discarded in the sample events. I'd recommed that get changed. It is common practice to put a dash or underscore between words that should not be discarded. And then the whole thing is anchored by some variable name. Sometimes it is op=

stevegrubb commented 3 years ago

And, all events of the same type need to have the same fields in the same order so that searches are successful.

pebenito commented 3 years ago

Thanks for the feedback @stevegrubb . The message that dbus-broker generates is actually an unmodified string from libselinux's logging callback, so I'll look in getting libselinux updated. My first thought is to try to get it to be the same format as existing MAC_POLICY_LOAD and MAC_STATUS messages.

pebenito commented 3 years ago

No change. Rebased.

stevegrubb commented 3 years ago

Was the libselinux change accepted? Might be good to cross reference that commit if so.

dvdhrm commented 3 years ago

Was the libselinux change accepted? Might be good to cross reference that commit if so.

@pebenito's initial PR messages include the reference to the commit in libselinux master.

pebenito commented 3 years ago

@dvdhrm would you be open to something like this, so this can merge now, rather than wait for the libselinux and libaudit releases? Then it can continue to compile on older systems.

util/selinux.c:

#ifdef SELINUX_POLICYLOAD
        case SELINUX_POLICYLOAD:
                audit_type = UTIL_AUDIT_TYPE_POLICYLOAD;
                break;
#endif
#ifdef SELINUX_SETENFORCE
        case SELINUX_SETENFORCE:
                audit_type = UTIL_AUDIT_TYPE_MAC_STATUS;
                break;
#endif

Same type of thing would apply in the util/audit.c blocks too.

stevegrubb commented 3 years ago

FYI, I am planning an audit release within the next 2 weeks.

pebenito commented 3 years ago

Thanks @stevegrubb . In this case I was thinking more of the SELinux userspace, which typically does minor releases every 6 months, so that puts the likely next release at mid January 2021.

dvdhrm commented 3 years ago

(Sorry for closing the PR, we switched the main branch and GitHub does not allow retargetting PRs. I will continue to keep this PR in mind.)

Regarding your other question: What is the advantage of doing conditional compilation in dbus-broker? I would much rather hard-depend on the newer libselinux+libaudit, and guarantee that all users get the new audit message types.

stevegrubb commented 3 years ago

A new audit package will be going out later this week or early next week. It will be 3.0.

pebenito commented 3 years ago

@dvdhrm I don't have a strong argument for the conditional compilation except for backwards compatibility to old libs; I didn't know if there were any backwards compatibility concerns, but it appears not. I will update when the two libraries get their updates. libselinux probably won't be until January or February 2021.

dvdhrm commented 3 years ago

Thanks a lot! I will then go ahead and merge this with a small meson.build addition.