Re: [PATCH v2 3/4] seccomp: Audit attempts to modify the actions_logged sysctl

[Steve Grubb]

On Thursday, May 3, 2018 4:18:26 PM EDT Paul Moore wrote:
> On Wed, May 2, 2018 at 2:18 PM, Steve Grubb <sgrubb@xxxxxxxxxx> wrote:
> > On Wednesday, May 2, 2018 11:53:19 AM EDT Tyler Hicks wrote:
> >> The decision to log a seccomp action will always be subject to the
> >> value of the kernel.seccomp.actions_logged sysctl, even for processes
> >> that are being inspected via the audit subsystem, in an upcoming patch.
> >> Therefore, we need to emit an audit record on attempts at writing to the
> >> actions_logged sysctl when auditing is enabled.
> >> 
> >> This patch updates the write handler for the actions_logged sysctl to
> >> emit an audit record on attempts to write to the sysctl. Successful
> >> writes to the sysctl will result in a record that includes a normalized
> >> list of logged actions in the "actions" field and a "res" field equal to
> >> 0. Unsuccessful writes to the sysctl will result in a record that
> >> doesn't include the "actions" field and has a "res" field equal to 1.
> >> 
> >> Not all unsuccessful writes to the sysctl are audited. For example, an
> >> audit record will not be emitted if an unprivileged process attempts to
> >> open the sysctl file for reading since that access control check is not
> >> part of the sysctl's write handler.
> >> 
> >> Below are some example audit records when writing various strings to the
> >> actions_logged sysctl.
> >> 
> >> Writing "not-a-real-action", when the kernel.seccomp.actions_logged
> >> sysctl previously was "kill_process kill_thread trap errno trace log",
> >> 
> >> emits this audit record:
> >>  type=CONFIG_CHANGE msg=audit(1525275273.537:130): op=seccomp-logging
> >>  old-actions=kill_process,kill_thread,trap,errno,trace,log res=0
> >> 
> >> If you then write "kill_process kill_thread errno trace log", this audit
> >> 
> >> record is emitted:
> >>  type=CONFIG_CHANGE msg=audit(1525275310.208:136): op=seccomp-logging
> >>  actions=kill_process,kill_thread,errno,trace,log
> >>  old-actions=kill_process,kill_thread,trap,errno,trace,log res=1
> >> 
> >> If you then write the string "log log errno trace kill_process
> >> kill_thread", which is unordered and contains the log action twice,
> >> 
> >> it results in the same actions value as the previous record:
> >>  type=CONFIG_CHANGE msg=audit(1525275325.613:142): op=seccomp-logging
> >>  actions=kill_process,kill_thread,errno,trace,log
> >>  old-actions=kill_process,kill_thread,errno,trace,log res=1
> >> 
> >> No audit records are generated when reading the actions_logged sysctl.
> > 
> > ACK for the format of the records.
> 
> I just wanted to clarify the record format with you Steve ... the
> "actions" and "old-actions" fields may not be included in the record
> in cases where there is an error building the action value string, are
> you okay with that or would you prefer the fields to always be
> included but with a "?" for the value?

A ? would be more in line with how other things are handled.

-Steve