Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

selinux: don't log SELINUX_INFO and SELINUX_WARNING messages to audit #11829

Merged
merged 1 commit into from
Feb 27, 2019

Conversation

msekletar
Copy link
Contributor

Previously we logged even info message from libselinux as USER_AVC's to
audit. For example, setting SELinux to permissive mode generated
following audit message,

time->Tue Feb 26 11:29:29 2019
type=USER_AVC msg=audit(1551198569.423:334): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 msg='avc: received setenforce notice (enforcing=0) exe="/usr/lib/systemd/systemd" sauid=0 hostname=? addr=? terminal=?'

This is unnecessary and wrong at the same time. First, kernel already
records audit event that SELinux was switched to permissive mode, also
the type of the message really shouldn't be USER_AVC.

Let's ignore SELINUX_WARNING and SELINUX_INFO and forward to audit only
USER_AVC's and errors as these two libselinux message types have clear
mapping to audit message types.

Previously we logged even info message from libselinux as USER_AVC's to
audit. For example, setting SELinux to permissive mode generated
following audit message,

time->Tue Feb 26 11:29:29 2019
type=USER_AVC msg=audit(1551198569.423:334): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 msg='avc:  received setenforce notice (enforcing=0)  exe="/usr/lib/systemd/systemd" sauid=0 hostname=? addr=? terminal=?'

This is unnecessary and wrong at the same time. First, kernel already
records audit event that SELinux was switched to permissive mode, also
the type of the message really shouldn't be USER_AVC.

Let's ignore SELINUX_WARNING and SELINUX_INFO and forward to audit only
USER_AVC's and errors as these two libselinux message types have clear
mapping to audit message types.
@poettering
Copy link
Member

i don't understand this I must say, my selinux-fu is too limited

@mgrepl, @rhatdan @wrabcak does this look good?

@rhatdan
Copy link
Contributor

rhatdan commented Feb 26, 2019

SELINUX_INFO usually indicates that the selinux policy has been modified. Either through loading policy or modifying booleans.
But if kernel is sending the message to audit log then systemd shouldn't be sending it.

Really systemd should only be generating USER_AVCS and sending them to audit.log.

@msekletar
Copy link
Contributor Author

Hmm, I think I need to rework the commit message. Actually, kernel is not sending audit message (which arguably it should), it is dbus-daemon that sends the same message as systemd. Note that our code was originally copied from dbus-daemon.

I think all user-space object managers really shouldn't log the same message and it should be the kernel who logs the audit message on enforcing status updates or on policy updates.

@wrabcak
Copy link

wrabcak commented Feb 27, 2019

Agree with @msekletar , in audit log there is already MAC_STATUS msg about changed SELinux state.
e.g:
type=MAC_STATUS msg=audit(1551259780.279:581): enforcing=0 old_enforcing=1 auid=0 ses=59 enabled=1 old-enabled=1 lsm=selinux res=1

I'm fine with this change.

FYI @bachradsusi

@msekletar
Copy link
Contributor Author

type=MAC_STATUS msg=audit(1551259780.279:581): enforcing=0 old_enforcing=1 auid=0 ses=59 enabled=1 old-enabled=1 lsm=selinux res=1

I missed that one; audit.log is a bit messy. So I will leave the commit message as it is then.

JFYI, it was actually @bachradsusi who suggested this approach. Also, in case this gets merged we should also fix dbus-daemon.

@poettering
Copy link
Member

So the patch looks good to you @wrabcak + @rhatdan?

@wrabcak
Copy link

wrabcak commented Feb 27, 2019

@poettering, yes please merge it.

Thanks,
Lukas.

@poettering poettering merged commit 6227fc1 into systemd:master Feb 27, 2019
@poettering
Copy link
Member

(btw, who is actually the right person to cc about issues like this? i cc'ed @mgrepl @rhatdan @wrabcak and you added @bachradsusi. Is all four correct? Does @mgrepl still do selinux stuff?)

BTW, if you are in a reviewing mood we still have this PR pending we need some selinux people input on: #10023. It's not entirely trivial. The PR in its current form appears to be too invasive and likely to introduce major breakages for the policy, but this is really something you guys need to decide on.

@wrabcak
Copy link

wrabcak commented Feb 27, 2019

@poettering , For SELinux issues ping @wrabcak and @bachradsusi, you can also add @rhatdan but Dan is focusing on containers. @mgrepl doesn't do technical SELinux stuff but who knows maybe he'll start again. :)

For #10023, it's really not trivial, I'll do my best to push it to our radar for near future.

@poettering
Copy link
Member

Excellent! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants