LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steve Grubb <sgrubb@redhat.com>
To: Tyler Hicks <tyhicks@canonical.com>
Cc: Paul Moore <paul@paul-moore.com>,
	linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>, Eric Paris <eparis@redhat.com>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-audit@redhat.com, linux-security-module@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v2 3/4] seccomp: Audit attempts to modify the actions_logged sysctl
Date: Thu, 03 May 2018 17:12:45 -0400	[thread overview]
Message-ID: <2397134.HQDQRr6h1X@x2> (raw)
In-Reply-To: <d8e1c48b-70d0-13a7-e563-755a8760b25f@canonical.com>

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@redhat.com> 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@redhat.com> 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

-Steve

  reply	other threads:[~2018-05-03 21:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-02 15:53 [PATCH v2 0/4] Better integrate seccomp logging and auditing Tyler Hicks
2018-05-02 15:53 ` [PATCH v2 1/4] seccomp: Separate read and write code for actions_logged sysctl Tyler Hicks
2018-05-02 21:01   ` James Morris
2018-05-02 15:53 ` [PATCH v2 2/4] seccomp: Configurable separator for the actions_logged string Tyler Hicks
2018-05-02 21:11   ` James Morris
2018-05-02 15:53 ` [PATCH v2 3/4] seccomp: Audit attempts to modify the actions_logged sysctl Tyler Hicks
2018-05-02 18:18   ` Steve Grubb
2018-05-03 20:18     ` Paul Moore
2018-05-03 20:42       ` Steve Grubb
2018-05-03 20:48         ` Paul Moore
2018-05-03 20:51           ` Tyler Hicks
2018-05-03 21:12             ` Steve Grubb [this message]
2018-05-03 22:36               ` Tyler Hicks
2018-05-03 23:18                 ` Steve Grubb
2018-05-02 21:17   ` James Morris
2018-05-02 15:53 ` [PATCH v2 4/4] seccomp: Don't special case audited processes when logging Tyler Hicks
2018-05-02 16:57   ` Kees Cook
2018-05-03  2:32     ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2397134.HQDQRr6h1X@x2 \
    --to=sgrubb@redhat.com \
    --cc=corbet@lwn.net \
    --cc=eparis@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=paul@paul-moore.com \
    --cc=tyhicks@canonical.com \
    --cc=wad@chromium.org \
    --subject='Re: [PATCH v2 3/4] seccomp: Audit attempts to modify the actions_logged sysctl' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).