Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 9 Apr 2019 10:02:59 -0400
From:   Richard Guy Briggs <rgb@redhat.com>
To:     Steve Grubb <sgrubb@redhat.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linux-Audit Mailing List <linux-audit@redhat.com>,
        Paul Moore <paul@paul-moore.com>, omosnace@redhat.com,
        eparis@parisplace.org, ebiederm@xmission.com, oleg@redhat.com
Subject: Re: [PATCH ghak111 V1] audit: deliver siginfo regarless of syscall
Message-ID: <20190409140259.n4t6rxb24eu3uzvp@madcap2.tricolour.ca>
References: <c7b092894ac6d611f8c5083e712da41e6685f94a.1554781852.git.rgb@redhat.com>
 <20190409080138.745d18a1@ivy-bridge>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190409080138.745d18a1@ivy-bridge>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2019-04-09 08:01, Steve Grubb wrote:
> On Mon,  8 Apr 2019 23:52:29 -0400 Richard Guy Briggs <rgb@redhat.com> wrote:
> > When a process signals the audit daemon (shutdown, rotate, resume,
> > reconfig) but syscall auditing is not enabled, we still want to know
> > the identity of the process sending the signal to the audit daemon.
> 
> Why? If syscall auditing is disabled, then there is no requirement to
> provide anything. What is the real problem that you are seeing?

Shutdown messages with -1 in them rather than the real values.

> -Steve
> 
> > Move audit_signal_info() out of syscall auditing to general auditing
> > but create a new function audit_signal_info_syscall() to take care of
> > the syscall dependent parts for when syscall auditing is enabled.
> > 
> > Please see the github kernel audit issue
> > https://github.com/linux-audit/audit-kernel/issues/111
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  include/linux/audit.h |  6 ++++++
> >  kernel/audit.c        | 27 +++++++++++++++++++++++++++
> >  kernel/audit.h        |  4 ++--
> >  kernel/auditsc.c      | 19 +++----------------
> >  kernel/signal.c       |  2 +-
> >  5 files changed, 39 insertions(+), 19 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 1e69d9fe16da..4a22fc3f824f 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -173,6 +173,9 @@ static inline unsigned int
> > audit_get_sessionid(struct task_struct *tsk) }
> >  
> >  extern u32 audit_enabled;
> > +
> > +extern int audit_signal_info(int sig, struct task_struct *t);
> > +
> >  #else /* CONFIG_AUDIT */
> >  static inline __printf(4, 5)
> >  void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
> > @@ -226,6 +229,9 @@ static inline unsigned int
> > audit_get_sessionid(struct task_struct *tsk) }
> >  
> >  #define audit_enabled AUDIT_OFF
> > +
> > +#define audit_signal_info(s, t) AUDIT_OFF
> > +
> >  #endif /* CONFIG_AUDIT */
> >  
> >  #ifdef CONFIG_AUDIT_COMPAT_GENERIC
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index b96bf69183f4..67399ff72d43 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -2274,6 +2274,33 @@ int audit_set_loginuid(kuid_t loginuid)
> >  }
> >  
> >  /**
> > + * audit_signal_info - record signal info for shutting down audit
> > subsystem
> > + * @sig: signal value
> > + * @t: task being signaled
> > + *
> > + * If the audit subsystem is being terminated, record the task (pid)
> > + * and uid that is doing that.
> > + */
> > +int audit_signal_info(int sig, struct task_struct *t)
> > +{
> > +	kuid_t uid = current_uid(), auid;
> > +
> > +	if (auditd_test_task(t) &&
> > +	    (sig == SIGTERM || sig == SIGHUP ||
> > +	     sig == SIGUSR1 || sig == SIGUSR2)) {
> > +		audit_sig_pid = task_tgid_nr(current);
> > +		auid = audit_get_loginuid(current);
> > +		if (uid_valid(auid))
> > +			audit_sig_uid = auid;
> > +		else
> > +			audit_sig_uid = uid;
> > +		security_task_getsecid(current, &audit_sig_sid);
> > +	}
> > +
> > +	return audit_signal_info_syscall(t);
> > +}
> > +
> > +/**
> >   * audit_log_end - end one audit record
> >   * @ab: the audit_buffer
> >   *
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index 958d5b8fc1b3..18a8ae812e9f 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -299,7 +299,7 @@ extern bool audit_tree_match(struct audit_chunk
> > *chunk, extern void audit_put_tree(struct audit_tree *tree);
> >  extern void audit_kill_trees(struct audit_context *context);
> >  
> > -extern int audit_signal_info(int sig, struct task_struct *t);
> > +extern int audit_signal_info_syscall(struct task_struct *t);
> >  extern void audit_filter_inodes(struct task_struct *tsk,
> >  				struct audit_context *ctx);
> >  extern struct list_head *audit_killed_trees(void);
> > @@ -330,7 +330,7 @@ extern void audit_filter_inodes(struct
> > task_struct *tsk, #define audit_tree_path(rule) ""	/* never
> > called */ #define audit_kill_trees(context) BUG()
> >  
> > -#define audit_signal_info(s, t) AUDIT_DISABLED
> > +#define audit_signal_info_syscall(t) AUDIT_OFF
> >  #define audit_filter_inodes(t, c) AUDIT_DISABLED
> >  #endif /* CONFIG_AUDITSYSCALL */
> >  
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 98a98e6dca05..dbd43d84c347 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2370,30 +2370,17 @@ void __audit_ptrace(struct task_struct *t)
> >  }
> >  
> >  /**
> > - * audit_signal_info - record signal info for shutting down audit
> > subsystem
> > - * @sig: signal value
> > + * audit_signal_info_syscall - record signal info for syscalls
> >   * @t: task being signaled
> >   *
> >   * If the audit subsystem is being terminated, record the task (pid)
> >   * and uid that is doing that.
> >   */
> > -int audit_signal_info(int sig, struct task_struct *t)
> > +int audit_signal_info_syscall(struct task_struct *t)
> >  {
> >  	struct audit_aux_data_pids *axp;
> >  	struct audit_context *ctx = audit_context();
> > -	kuid_t uid = current_uid(), auid, t_uid = task_uid(t);
> > -
> > -	if (auditd_test_task(t) &&
> > -	    (sig == SIGTERM || sig == SIGHUP ||
> > -	     sig == SIGUSR1 || sig == SIGUSR2)) {
> > -		audit_sig_pid = task_tgid_nr(current);
> > -		auid = audit_get_loginuid(current);
> > -		if (uid_valid(auid))
> > -			audit_sig_uid = auid;
> > -		else
> > -			audit_sig_uid = uid;
> > -		security_task_getsecid(current, &audit_sig_sid);
> > -	}
> > +	kuid_t t_uid = task_uid(t);
> >  
> >  	if (!audit_signals || audit_dummy_context())
> >  		return 0;
> > diff --git a/kernel/signal.c b/kernel/signal.c
> > index b7953934aa99..73db5dfa797d 100644
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -43,6 +43,7 @@
> >  #include <linux/compiler.h>
> >  #include <linux/posix-timers.h>
> >  #include <linux/livepatch.h>
> > +#include <linux/audit.h>	/* audit_signal_info() */
> >  
> >  #define CREATE_TRACE_POINTS
> >  #include <trace/events/signal.h>
> > @@ -52,7 +53,6 @@
> >  #include <asm/unistd.h>
> >  #include <asm/siginfo.h>
> >  #include <asm/cacheflush.h>
> > -#include "audit.h"	/* audit_signal_info() */
> >  
> >  /*
> >   * SLAB caches for signal bits.
> 

- 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