[ghak90,(was,ghak32),V4,06/10] audit: add containerid support for tty_audit

Submitted by Richard Guy Briggs on July 31, 2018, 8:07 p.m.

Details

Message ID e41cb6760a8183c0955c378fce7b500819f9838f.1533065887.git.rgb@redhat.com
State New
Series "audit: implement container identifier"
Headers show

Commit Message

Richard Guy Briggs July 31, 2018, 8:07 p.m.
Add audit container identifier auxiliary record to tty logging rule
event standalone records.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
---
 drivers/tty/tty_audit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 50f567b..3e21477 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -66,8 +66,9 @@  static void tty_audit_log(const char *description, dev_t dev,
 	uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
 	uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
 	unsigned int sessionid = audit_get_sessionid(tsk);
+	struct audit_context *context = audit_alloc_local(GFP_KERNEL);
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_TTY);
 	if (ab) {
 		char name[sizeof(tsk->comm)];
 
@@ -80,6 +81,8 @@  static void tty_audit_log(const char *description, dev_t dev,
 		audit_log_n_hex(ab, data, size);
 		audit_log_end(ab);
 	}
+	audit_log_contid(context, "tty", audit_get_contid(tsk));
+	audit_free_context(context);
 }
 
 /**

Comments

Paul Moore Oct. 19, 2018, 11:17 p.m.
On Sun, Aug 5, 2018 at 4:33 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> Add audit container identifier auxiliary record to tty logging rule
> event standalone records.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> Acked-by: Serge Hallyn <serge@hallyn.com>
> ---
>  drivers/tty/tty_audit.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
> index 50f567b..3e21477 100644
> --- a/drivers/tty/tty_audit.c
> +++ b/drivers/tty/tty_audit.c
> @@ -66,8 +66,9 @@ static void tty_audit_log(const char *description, dev_t dev,
>         uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
>         uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
>         unsigned int sessionid = audit_get_sessionid(tsk);
> +       struct audit_context *context = audit_alloc_local(GFP_KERNEL);
>
> -       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY);
> +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_TTY);
>         if (ab) {
>                 char name[sizeof(tsk->comm)];
>
> @@ -80,6 +81,8 @@ static void tty_audit_log(const char *description, dev_t dev,
>                 audit_log_n_hex(ab, data, size);
>                 audit_log_end(ab);
>         }
> +       audit_log_contid(context, "tty", audit_get_contid(tsk));
> +       audit_free_context(context);
>  }

Since I never polished up my task_struct/current fix patch enough to
get it past RFC status during this development window (new job, stolen
laptop, etc.) *and* it looks like you are going to need at least one
more respin of this patchset, go ahead and fix this patch to use
current instead of generating a local context.  I'll deal with the
merge fallout if/when it happens.

Local contexts are a last resort.  If you ever find yourself writing
code that generates a local context, you should first be 100% certain
that the event is not the the result of a process initiated action (in
which case it should take from the task's context).

--
paul moore
www.paul-moore.com
Richard Guy Briggs Oct. 31, 2018, 9:17 p.m.
On 2018-10-19 19:17, Paul Moore wrote:
> On Sun, Aug 5, 2018 at 4:33 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > Add audit container identifier auxiliary record to tty logging rule
> > event standalone records.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> > ---
> >  drivers/tty/tty_audit.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
> > index 50f567b..3e21477 100644
> > --- a/drivers/tty/tty_audit.c
> > +++ b/drivers/tty/tty_audit.c
> > @@ -66,8 +66,9 @@ static void tty_audit_log(const char *description, dev_t dev,
> >         uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
> >         uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
> >         unsigned int sessionid = audit_get_sessionid(tsk);
> > +       struct audit_context *context = audit_alloc_local(GFP_KERNEL);
> >
> > -       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY);
> > +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_TTY);
> >         if (ab) {
> >                 char name[sizeof(tsk->comm)];
> >
> > @@ -80,6 +81,8 @@ static void tty_audit_log(const char *description, dev_t dev,
> >                 audit_log_n_hex(ab, data, size);
> >                 audit_log_end(ab);
> >         }
> > +       audit_log_contid(context, "tty", audit_get_contid(tsk));
> > +       audit_free_context(context);
> >  }
> 
> Since I never polished up my task_struct/current fix patch enough to
> get it past RFC status during this development window (new job, stolen
> laptop, etc.) *and* it looks like you are going to need at least one
> more respin of this patchset, go ahead and fix this patch to use
> current instead of generating a local context.  I'll deal with the
> merge fallout if/when it happens.

Sure, I will switch it to current in the call to audit_get_contid().

The local context is a distinct issue.  Like USER records, I prefer
local due to potential record volume, it is already trackable as far as
Steve is concerned, and if it is to be connected with the syscall
record, it should be linked to syscall records in a seperate new github
issue with its own patch.  It accumulates events until the buffer is
flushed to a record, so the timestamp only represents the flush (usually
user "CR/enter").

> Local contexts are a last resort.  If you ever find yourself writing
> code that generates a local context, you should first be 100% certain
> that the event is not the the result of a process initiated action (in
> which case it should take from the task's context).

Well, I'm 100% certain it is linked to a process, but so are USER
records that are already being discussed as the exception.  This is
basically a keystroke logger (that has a flag to omit passwords).

> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635