search

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

RFE: CONFIG_CHANGE record cleanup #59

Closed stevegrubb closed 5 years ago

stevegrubb commented 7 years ago

This event should be a simple record with the following fields: pid, uid, auid, tty, session, context, comm, exe (in this order) + old value + new value if applicable.

pcmoore commented 7 years ago

This is related to #60.

For reference, the current CONFIG_CHANGE record:

# ausearch -m CONFIG_CHANGE
time->Tue Aug 15 09:20:22 2017
type=CONFIG_CHANGE msg=audit(1502803222.053:73): audit_pid=371 old=0 auid=4294967295 
  ses=4294967295 subj=system_u:system_r:auditd_t:s0 res=1

Similar question as in #60, if we associated this with a SYSCALL record, would that satisfy this requirement?

stevegrubb commented 7 years ago

This should not have a syscall record associated. This should be a simple record with all the information necessary on the event. We do not care about the syscall or arch or exit code or gid or fsuid or any of that extra info that a syscall has. All events should be like:

type=CONFIG_CHANGE msg=audit(1501845822.795:3): audit_enabled=1 old=1 pid=xxx uid=xxx auid=4294967295 tty=xxx ses=4294967295 subj=kernel comm=xxx exe=xxx res=1

Meaning what changed is first, then all the credential information. What changed is not searchable. The credentials are and should all be in a specific order. Thanks.

pcmoore commented 7 years ago

Unfortunately inserting fields into an existing audit record is not an option here, but we can add the fields you are requesting to the end of the record; assuming the information is available, I haven't checked on that yet. For example:

# ausearch -m CONFIG_CHANGE
time->Tue Aug 15 09:20:22 2017
type=CONFIG_CHANGE msg=audit(1502803222.053:73): audit_pid=371 old=0 auid=4294967295 
  ses=4294967295 subj=system_u:system_r:auditd_t:s0 res=1 pid=XXX uid=XXX tty=XXX 
  comm=XXX exe=XXX

Another option is to associate it with an existing SYSCALL record, which would be my preference if possible, although I understand you don't want that.

rgbriggs commented 7 years ago

If we require all CONFIG_CHANGE events to be recorded, then depending on a syscall record is a non-starter since they are opt-in or filtered. If any of these CONFIG_CHANGE records is to be changed, they all need to be changed (audit add rule, audit del rule, audit trim rule, audit make equivalent, audit tty set, audit set {rate limit, backlog wait time, backlog limit, enabled, failure}, audit tree remove rule, audit mark remove rule) with a similarly compatible format.

Does it make sense to create a new record that is an aux record to standalone records such as the CONFIG_CHANGE record that lists this standard information, such as: type=TASK msg=audit(1502803222.053:73): pid=xxx uid=xxx auid=4294967295 tty=xxx ses=4294967295 subj=kernel comm=xxx exe=xxx res=1

pcmoore commented 7 years ago

If we require all CONFIG_CHANGE events to be recorded, then depending on a syscall record is a non-starter since they are opt-in or filtered.

Just so I'm clear, you are talking about Fedora (and perhaps other distros as well) decision to disable syscall auditing by default, yes? Or are you talking about something else?

FWIW, I don't consider that a reason to not solve this via a SYSCALL record; if you want audit information you need to do the work to make sure audit is actually enabled properly on your system. Now, there may be other reasons why we don't want to solve this via SYSCALL records, but I don't consider the default configuration of a distro to be a non-starter.

If any of these CONFIG_CHANGE records is to be changed, they all need to be changed ...

Of course. I'm not sure why we wouldn't do that.

Does it make sense to create a new record that is an aux record ...

At this point I'm not a big fan of adding an aux record for this purpose, but I'm not going to rule it out.

rgbriggs commented 7 years ago

On 2017-10-13 21:01, Paul Moore wrote:

If we require all CONFIG_CHANGE events to be recorded, then depending on a syscall record is a non-starter since they are opt-in or filtered.

Just so I'm clear, you are talking about Fedora (and perhaps other distros as well) decision to disable syscall auditing by default, yes? Or are you talking about something else?

No, I'm not talking about this.

I don't know if the STIG covers this already, but my understanding is that config changes are to be logged by default and that there must be a way to filter them if you know you don't want them.

As it stands with the current STIG, I'm guessing that config records that are associated with a SYSCALL record will only be recorded if there is a SYSCALL rule to catch it.

FWIW, I don't consider that a reason to not solve this via a SYSCALL record; if you want audit information you need to do the work to make sure audit is actually enabled properly on your system. Now, there may be other reasons why we don't want to solve this via SYSCALL records, but I don't consider the default configuration of a distro to be a non-starter.

Again, that I wasn't considering that situation at all.

If any of these CONFIG_CHANGE records is to be changed, they all need to be changed ...

Of course. I'm not sure why we wouldn't do that.

I was hinting that this first patch doesn't close this issue since there are many other instances of this record type that haven't yet been fixed.

Does it make sense to create a new record that is an aux record ...

At this point I'm not a big fan of adding an aux record for this purpose, but I'm not going to rule it out.

I'm not eager either, but trying to find ways to solve this that everyone can live with.

rgbriggs commented 7 years ago

See also: https://github.com/linux-audit/audit-kernel/issues/50

stevegrubb commented 7 years ago

Richard, we do not need to do anything with syscall filtering to pick up CONFIG_CHANGE records. All it take is changing the logging to use

audit_log(current->audit_context, GFP_KERNEL, AUDIT_CONFIG_CHANGE, "..."

This forces a SYSCALL record on top of the current event. This is in contrast to calling audit_log_format() which does not add a syscall record.

I think this in responce to comment #4. Wished they were numbered.

stevegrubb commented 7 years ago

In response to comment #6, The STIG does require recording config changes and filtering. In this case, adding a SYSCALL record is not a function of the filters. You can add a SYSCALL record nearly any time by calling audit_log. The filtering is done only in the exclude filter which all events are subject to.

In reply to comment #5

If any of these CONFIG_CHANGE records is to be changed, they all need to be changed ...

Agreed. But 2 of them cause 95% of the search problem. I'd rather fix these two and agree on how to do it and then come back for the others another day.

rgbriggs commented 7 years ago

On 2017-10-16 21:33, Steve Grubb wrote:

Richard, we do not need to do anything with syscall filtering to pick up CONFIG_CHANGE records. All it take is changing the logging to use

audit_log(current->audit_context, GFP_KERNEL, AUDIT_CONFIG_CHANGE, "..."

This forces a SYSCALL record on top of the current event. This is in contrast to calling audit_log_format() which does not add a syscall record.

Ok, got it. This is because the generation of the CONFIG_CHANGE record doesn't check audit_dummy_context().

I think this in responce to comment #4. Wished they were numbered.

Or reply to the email and just quote it...

rgbriggs commented 7 years ago

On 2017-10-16 21:38, Steve Grubb wrote:

In response to comment #6, The STIG does require recording config changes and filtering. In this case, adding a SYSCALL record is not a function of the filters. You can add a SYSCALL record nearly any time by calling audit_log. The filtering is done only in the exclude filter which all events are subject to.

In reply to comment #5

If any of these CONFIG_CHANGE records is to be changed, they all need to be changed ...

Agreed. But 2 of them cause 95% of the search problem. I'd rather fix these two and agree on how to do it and then come back for the others another day.

Sure, this is a good start, and it should be a priority to go in early. I'm just wary of the issue being closed prematurely before the rest are addressed.

pcmoore commented 7 years ago

@rgbriggs @stevegrubb we're spreading the discussion out across the mailing list and this GH issue, which is not good. Since we have a thread going on the mailing list, let's keep the discussion there for right now.

pcmoore commented 6 years ago

While not the most recent patchset submission, this thread has a good discussion of the problems.

rgbriggs commented 6 years ago

RFC V1 posted upstream: https://www.redhat.com/archives/linux-audit/2018-June/msg00086.html https://lkml.org/lkml/2018/6/14/816

rgbriggs commented 6 years ago

Post v2 upstream: https://www.redhat.com/archives/linux-audit/2018-July/msg00163.html https://lkml.org/lkml/2018/7/27/961

rgbriggs commented 5 years ago

The sources of AUDIT_CONFIG_CHANGE records and their current [and proposed] fields:

audit_log_config_change

audit_log_rule_change

audit_log_common_recv_msg ADD/DEL_RULE

audit_log_common_recv_msg TRIM

audit_log_common_recv_msg MAKE_EQUIV

audit_log_common_recv_msg TTY_SET

audit_mark_log_rule_change

audit_tree_log_remove_rule

audit_watch_log_rule_change

audit_seccomp_actions_logged

rgbriggs commented 5 years ago

Post v3 https://www.redhat.com/archives/linux-audit/2018-December/msg00035.html

rgbriggs commented 5 years ago

v3-1/4 upstreamed: 53fc7a01df51 ("audit: give a clue what CONFIG_CHANGE op was involved") v3-3/4 upstreamed: 9e36a5d49c3 ("audit: hand taken context to audit_kill_trees for syscall logging") v4 posted (fixup v3-2/4) ("audit: add syscall information to CONFIG_CHANGE records") https://lkml.org/lkml/2019/1/18/970 https://www.redhat.com/archives/linux-audit/2019-January/msg00067.html

rgbriggs commented 5 years ago

v4 upstreamed: dc7ef993460e ("audit: add syscall information to CONFIG_CHANGE records")