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

ausearch-parse: fix parsing for success/uid/exe in some cases #394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sergio-correia
Copy link
Contributor

  1. In parse_daemon1(), we may have the uid= field appear both before and after pid=, which may cause our parsing of it to fail, as we may have skipped past it. For uid=, let us search from the beginning.

Example for this case:

type=DAEMON_END msg=audit(1709723032.140:753): op=terminate auid=0 uid=0 ses=8 pid=107086 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=success

ausearch -if sample.log -a 753 -m DAEMON_END -ui 0 --session 8 -p 107086 --success yes

  1. In a similar fashion, in parse_user(), we may also have difficulties parsing some fields, like res= or exe=, as we may move past them during the parsing, so here we will set our new search starting point to just before parsing uid=, and we will not reset it further on.

Example for this case:

type=USER_MAC_POLICY_LOAD msg=audit(1720797861.553:666): pid=614 uid=81 auid=4294967295 ses=4294967295 subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc: op=load_policy lsm=selinux seqno=2 res=1 exe="/usr/bin/dbus-broker" sauid=81 hostname=? addr=? terminal=?'

ausearch -if sample-2.log -a 666 -m USER_MAC_POLICY_LOAD -p 614 -ui 81 -ul 4294967295 --session 4294967295 -su system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 --success yes -x "/usr/bin/dbus-broker"

1) In parse_daemon1(), we may have the uid= field appear both before and
after pid=, which may cause our parsing of it to fail, as we may have
skipped past it. For uid=, let us search from the beginning.

Example for this case:

type=DAEMON_END msg=audit(1709723032.140:753): op=terminate auid=0 uid=0 ses=8 pid=107086 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 res=success

ausearch -if sample.log -a 753 -m DAEMON_END  -ui 0 --session 8 -p 107086 --success yes

2) In a similar fashion, in parse_user(), we may also have difficulties
parsing some fields, like res= or exe=, as we may move past them during
the parsing, so here we will set our new search starting point to just
before parsing  uid=, and we will not reset it further on.

Example for this case:

type=USER_MAC_POLICY_LOAD msg=audit(1720797861.553:666): pid=614 uid=81 auid=4294967295 ses=4294967295 subj=system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 msg='avc:  op=load_policy lsm=selinux seqno=2 res=1 exe="/usr/bin/dbus-broker" sauid=81 hostname=? addr=? terminal=?'

ausearch -if sample-2.log -a 666 -m USER_MAC_POLICY_LOAD -p 614 -ui 81 -ul 4294967295 --session 4294967295 -su system_u:system_r:system_dbusd_t:s0-s0:c0.c1023 --success yes -x "/usr/bin/dbus-broker"

Signed-off-by: Sergio Correia <[email protected]>
@stevegrubb
Copy link
Contributor

Just a FYI, there is an ausearch test here: https://people.redhat.com/sgrubb/audit/ausearch-test-0.6.tar.gz

@sergio-correia
Copy link
Contributor Author

Just a FYI, there is an ausearch test here: https://people.redhat.com/sgrubb/audit/ausearch-test-0.6.tar.gz

Thanks, I was not aware of this, although after a closer look, it seems it was a variation of that ausearch-test that identified the issues with the records reported here.

@stevegrubb
Copy link
Contributor

The daemon part checks OK (although I wonder if it best to just fix the event and let it work out over time).

Was the other fix only because of USER_MAC_POLICY_LOAD or are there other cases? The reason I ask is that there are 2 types of events that are fundamentally broken: USER_MAC_POLICY_LOAD and USER_AVC. Historically, they have been broken because libselinux doesn't create valid audit records.

The real fix would be to patch this: https://github.com/bus1/dbus-broker/blob/4fa73fd2dc8429ff0fbbda39bce8e8af92681b07/src/util/audit.c#L33 so that it does not use the avc logging function except when it really is an AVC. The other two cases should use audit_log_user_message(). If this is fixed at the source, is this patch still needed for USER_MAC_POLICY_LOAD?

@sergio-correia
Copy link
Contributor Author

Right, the other fix was due to USER_MAC_POLICY_LOAD, and I agree with you that the real fix would be in dbus-broker. Shall I remove that fix from this PR?

What about "just fix the event and let it work out over time"? Would you elaborate on that, please?

@stevegrubb
Copy link
Contributor

"just fix the event and let it work out over time" means that sometimes I have chosen not to fix something in current code and instead fix the problem in the event. Eventually the fixed event is in all distributions and then search works again. The idea is not to make ausearch slowed down by events that should be fixed. For example, maybe DAEMON_START should be re-arranged so that it fits all other DAEMON events and then it everything starts working next release (which should be soon).

A patch should be sent to dbus-broker. Around line 123 it should do something like
if (type == UTIL_AUDIT_TYPE_AVC)
audit_log_user_avc_message(...
else
audit_log_user_message(...

Btw, I made a repo for ausearch-test here: https://github.com/stevegrubb/ausearch-test

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

Successfully merging this pull request may close these issues.

2 participants