Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Richard Guy Briggs <rgb@redhat.com>
Cc: containers@lists.linux-foundation.org, linux-api@vger.kernel.org,
	Linux-Audit Mailing List <linux-audit@redhat.com>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
	sgrubb@redhat.com, omosnace@redhat.com, dhowells@redhat.com,
	simo@redhat.com, Eric Paris <eparis@parisplace.org>,
	Serge Hallyn <serge@hallyn.com>,
	ebiederm@xmission.com, nhorman@tuxdriver.com,
	Dan Walsh <dwalsh@redhat.com>,
	mpatel@redhat.com
Subject: Re: [PATCH ghak90 V8 13/16] audit: track container nesting
Date: Wed, 22 Jan 2020 16:29:12 -0500
Message-ID: <CAHC9VhRkH=YEjAY6dJJHSp934grHnf=O4RiqLu3U8DzdVQOZkg@mail.gmail.com> (raw)
In-Reply-To: <6452955c1e038227a5cd169f689f3fd3db27513f.1577736799.git.rgb@redhat.com>

On Tue, Dec 31, 2019 at 2:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> Track the parent container of a container to be able to filter and
> report nesting.
>
> Now that we have a way to track and check the parent container of a
> container, modify the contid field format to be able to report that
> nesting using a carrat ("^") separator to indicate nesting.  The
> original field format was "contid=<contid>" for task-associated records
> and "contid=<contid>[,<contid>[...]]" for network-namespace-associated
> records.  The new field format is
> "contid=<contid>[^<contid>[...]][,<contid>[...]]".

Let's make sure we always use a comma as a separator, even when
recording the parent information, for example:
"contid=<contid>[,^<contid>[...]][,<contid>[...]]"

> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/linux/audit.h |  1 +
>  kernel/audit.c        | 53 +++++++++++++++++++++++++++++++++++++++++++--------
>  kernel/audit.h        |  1 +
>  kernel/auditfilter.c  | 17 ++++++++++++++++-
>  kernel/auditsc.c      |  2 +-
>  5 files changed, 64 insertions(+), 10 deletions(-)

...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index ef8e07524c46..68be59d1a89b 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c

> @@ -492,6 +493,7 @@ void audit_switch_task_namespaces(struct nsproxy *ns, struct task_struct *p)
>                 audit_netns_contid_add(new->net_ns, contid);
>  }
>
> +void audit_log_contid(struct audit_buffer *ab, u64 contid);

If we need a forward declaration, might as well just move it up near
the top of the file with the rest of the declarations.

> +void audit_log_contid(struct audit_buffer *ab, u64 contid)
> +{
> +       struct audit_contobj *cont = NULL, *prcont = NULL;
> +       int h;

It seems safer to pass the audit container ID object and not the u64.

> +       if (!audit_contid_valid(contid)) {
> +               audit_log_format(ab, "%llu", contid);

Do we really want to print (u64)-1 here?  Since this is a known
invalid number, would "?" be a better choice?

> +               return;
> +       }
> +       h = audit_hash_contid(contid);
> +       rcu_read_lock();
> +       list_for_each_entry_rcu(cont, &audit_contid_hash[h], list)
> +               if (cont->id == contid) {
> +                       prcont = cont;

Why not just pull the code below into the body of this if statement?
It all needs to be done under the RCU read lock anyway and the code
would read much better this way.

> +                       break;
> +               }
> +       if (!prcont) {
> +               audit_log_format(ab, "%llu", contid);
> +               goto out;
> +       }
> +       while (prcont) {
> +               audit_log_format(ab, "%llu", prcont->id);
> +               prcont = prcont->parent;
> +               if (prcont)
> +                       audit_log_format(ab, "^");

In the interest of limiting the number of calls to audit_log_format(),
how about something like the following:

  audit_log_format("%llu", cont);
  iter = cont->parent;
  while (iter) {
    if (iter->parent)
      audit_log_format("^%llu,", iter);
    else
      audit_log_format("^%llu", iter);
    iter = iter->parent;
  }

> +       }
> +out:
> +       rcu_read_unlock();
> +}
> +
>  /*
>   * audit_log_container_id - report container info
>   * @context: task or local context for record

...

> @@ -2705,9 +2741,10 @@ int audit_set_contid(struct task_struct *task, u64 contid)
>         if (!ab)
>                 return rc;
>
> -       audit_log_format(ab,
> -                        "op=set opid=%d contid=%llu old-contid=%llu",
> -                        task_tgid_nr(task), contid, oldcontid);
> +       audit_log_format(ab, "op=set opid=%d contid=", task_tgid_nr(task));
> +       audit_log_contid(ab, contid);
> +       audit_log_format(ab, " old-contid=");
> +       audit_log_contid(ab, oldcontid);

This is an interesting case where contid and old-contid are going to
be largely the same, only the first (current) ID is going to be
different; do we want to duplicate all of those IDs?


>         audit_log_end(ab);
>         return rc;
>  }
> @@ -2723,9 +2760,9 @@ void audit_log_container_drop(void)

--
paul moore
www.paul-moore.com

  reply index

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-31 19:48 [PATCH ghak90 V8 00/16] audit: implement container identifier Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 01/16] audit: collect audit task parameters Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 02/16] audit: add container id Richard Guy Briggs
2020-01-22 21:28   ` Paul Moore
2020-01-30 17:53     ` Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 03/16] audit: read container ID of a process Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 04/16] audit: convert to contid list to check for orch/engine ownership Richard Guy Briggs
2020-01-22 21:28   ` Paul Moore
2020-02-04 22:51     ` Richard Guy Briggs
2020-02-05 22:40       ` Paul Moore
2019-12-31 19:48 ` [PATCH ghak90 V8 05/16] audit: log drop of contid on exit of last task Richard Guy Briggs
2020-01-22 21:28   ` Paul Moore
2020-02-04 23:02     ` Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 06/16] audit: log container info of syscalls Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 07/16] audit: add contid support for signalling the audit daemon Richard Guy Briggs
2020-01-22 21:28   ` Paul Moore
2020-01-23 16:29     ` Richard Guy Briggs
2020-01-23 17:09       ` Paul Moore
2020-01-23 20:04         ` Richard Guy Briggs
2020-01-23 21:35           ` Paul Moore
2020-02-04 23:14             ` Richard Guy Briggs
2020-02-05 22:50               ` Paul Moore
2020-02-12 22:38                 ` Steve Grubb
2020-02-13  0:09                   ` Paul Moore
2020-02-13 21:44                     ` Paul Moore
2020-03-12 19:30                       ` Richard Guy Briggs
2020-03-13 16:29                         ` Paul Moore
2020-03-13 18:59                           ` Richard Guy Briggs
2020-03-18 20:56                             ` Paul Moore
2020-03-18 21:26                               ` Richard Guy Briggs
2020-03-18 21:42                                 ` Paul Moore
2020-03-18 21:55                                   ` Richard Guy Briggs
2020-03-18 22:06                                     ` Paul Moore
2020-03-19 22:02                                       ` Richard Guy Briggs
2020-03-24  0:16                                         ` Paul Moore
2020-03-24 21:01                                           ` Richard Guy Briggs
2020-03-29  3:11                                             ` Paul Moore
2020-03-30 13:47                                               ` Richard Guy Briggs
2020-03-30 14:26                                                 ` Paul Moore
2020-03-30 16:21                                                   ` Richard Guy Briggs
2020-03-30 17:34                                                     ` Paul Moore
2020-03-30 17:49                                                       ` Richard Guy Briggs
2020-03-30 19:55                                                         ` Paul Moore
2020-03-12 20:27                     ` Richard Guy Briggs
2020-03-13 16:42                       ` Paul Moore
2020-03-13 16:45                         ` Steve Grubb
2020-03-13 16:49                           ` Paul Moore
2020-03-13 19:23                         ` Richard Guy Briggs
2020-03-18 21:01                           ` Paul Moore
2020-03-18 21:41                             ` Richard Guy Briggs
2020-03-18 21:47                               ` Paul Moore
2020-03-19 21:47                                 ` Richard Guy Briggs
2020-03-20 21:56                                   ` Paul Moore
2020-03-25 12:29                                     ` Richard Guy Briggs
2020-03-29  3:17                                       ` Paul Moore
2020-03-30 15:23                                         ` Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 08/16] audit: add support for non-syscall auxiliary records Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 09/16] audit: add containerid support for user records Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 10/16] audit: add containerid filtering Richard Guy Briggs
2019-12-31 19:48 ` [PATCH ghak90 V8 11/16] audit: add support for containerid to network namespaces Richard Guy Briggs
2020-01-22 21:28   ` Paul Moore
2020-02-04 23:42     ` Richard Guy Briggs
2020-02-05 22:51       ` Paul Moore
2019-12-31 19:48 ` [PATCH ghak90 V8 12/16] audit: contid check descendancy and nesting Richard Guy Briggs
2020-01-22 21:29   ` Paul Moore
2020-01-23 21:02     ` Richard Guy Briggs
2020-01-23 21:47       ` Paul Moore
2019-12-31 19:48 ` [PATCH ghak90 V8 13/16] audit: track container nesting Richard Guy Briggs
2020-01-22 21:29   ` Paul Moore [this message]
2020-01-30 19:27     ` Richard Guy Briggs
2020-02-05 23:05       ` Paul Moore
2020-02-05 23:50         ` Richard Guy Briggs
2020-02-13 21:49           ` Paul Moore
2020-03-12 20:51             ` Richard Guy Briggs
2020-03-13 16:47               ` Paul Moore
2020-03-14 22:42                 ` Richard Guy Briggs
2020-03-17 18:28                   ` Richard Guy Briggs
2020-03-18 21:08                   ` Paul Moore
2020-01-31 14:50     ` Steve Grubb
2020-02-04 13:19       ` Richard Guy Briggs
2020-02-04 15:47         ` Steve Grubb
2020-02-04 15:52           ` Paul Moore
2020-02-04 18:12             ` Steve Grubb
2020-02-05 22:57               ` Paul Moore
2019-12-31 19:48 ` [PATCH ghak90 V8 14/16] audit: check contid depth and add limit config param Richard Guy Briggs
2020-01-22 21:29   ` Paul Moore
2019-12-31 19:48 ` [PATCH ghak90 V8 15/16] audit: check contid count per netns and add config param limit Richard Guy Briggs
2020-01-22 21:29   ` Paul Moore
2019-12-31 19:48 ` [PATCH ghak90 V8 16/16] audit: add capcontid to set contid outside init_user_ns Richard Guy Briggs
2020-01-22 21:29   ` Paul Moore
2020-02-05  0:39     ` Richard Guy Briggs
2020-02-05 22:56       ` Paul Moore
2020-02-06 12:51         ` Richard Guy Briggs
2020-02-13 21:58           ` Paul Moore
2020-03-12 21:58             ` Richard Guy Briggs

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='CAHC9VhRkH=YEjAY6dJJHSp934grHnf=O4RiqLu3U8DzdVQOZkg@mail.gmail.com' \
    --to=paul@paul-moore.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=dwalsh@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=eparis@parisplace.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatel@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=omosnace@redhat.com \
    --cc=rgb@redhat.com \
    --cc=serge@hallyn.com \
    --cc=sgrubb@redhat.com \
    --cc=simo@redhat.com \
    /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

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git