On Thursday, May 3, 2018 6:36:18 PM EDT Tyler Hicks wrote: > On 05/03/2018 04:12 PM, Steve Grubb wrote: > > On Thursday, May 3, 2018 4:51:36 PM EDT Tyler Hicks wrote: > >> On 05/03/2018 03:48 PM, Paul Moore wrote: > >>> On Thu, May 3, 2018 at 4:42 PM, Steve Grubb <sgrubb@xxxxxxxxxx> wrote: > >>>> 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. > >>> > >>> That's what I thought. > >>> > >>> Would you mind putting together a v3 Tyler? :) > >> > >> To be clear, "?" is only to be used when the call to > >> seccomp_names_from_actions_logged() fails, right? > > > > Yes and that is a question mark with no quotes in the audit record. > > > >> If the sysctl write fails for some other reason, such as when an invalid > >> action name is specified, can you confirm that you still want *no* > >> "actions" field, > > > > Its best that fields do not disappear. In the case of invalid input, you > > can just leave the new value as ? so that nothing malicious can be > > injected into the logs > > > >> the "old-actions" field to be the value prior to attempting the update > >> to the sysctl, and res to be 0? > > > > Yes > > I came up with one more question after hitting a corner case while testing. > > It is valid to write an empty string to the sysctl. If the sysctl was > set to "errno" and then later set to "", you'd see this with the current > revision: > > type=CONFIG_CHANGE msg=audit(1525385824.643:173): op=seccomp-logging > actions= old-actions=errno res=1 > > Is that what you want or should the value of the "actions" field be > something be something like this: > > actions=(none) This ^^^ would be preferred. However, the parenthesis is not needed. Thanks, -Steve -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
