From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steve Grubb <sgrubb@redhat.com>
Subject: Re: Limiting SECCOMP audit events
Date: Fri, 15 Dec 2017 11:09:39 -0500
Message-ID: <2178350.DaTGirQ7H8@x2>
References: <58203247.sCqcla2mis@x2>
	<1605a80e588.280e.85c95baa4474aabc7814e68940a78392@paul-moore.com>
	<20171215154713.GA16170@sec>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <linux-audit-bounces@redhat.com>
In-Reply-To: <20171215154713.GA16170@sec>
List-Unsubscribe: <https://www.redhat.com/mailman/options/linux-audit>,
	<mailto:linux-audit-request@redhat.com?subject=unsubscribe>
List-Archive: <https://www.redhat.com/archives/linux-audit>
List-Post: <mailto:linux-audit@redhat.com>
List-Help: <mailto:linux-audit-request@redhat.com?subject=help>
List-Subscribe: <https://www.redhat.com/mailman/listinfo/linux-audit>,
	<mailto:linux-audit-request@redhat.com?subject=subscribe>
Sender: linux-audit-bounces@redhat.com
Errors-To: linux-audit-bounces@redhat.com
To: Tyler Hicks <tyhicks@canonical.com>
Cc: Linux Audit <linux-audit@redhat.com>
List-Id: linux-audit@redhat.com

On Friday, December 15, 2017 10:47:14 AM EST Tyler Hicks wrote:
> > Looks good to me but two things:
> > 
> > * Change the name of __audit_seccomp() to audit_seccomp() since we don't
> > have two functions anymore.
> > 
> > * Are we sure about removing the audit_enabled check? People got pretty
> > upset when it wasn't there in the past.
> 
> Do you have any references to the complaints so that we can understand
> them better? I remember being surprised by commit 96368701 adding the
> audit_enabled check (my fault for not watching the list closer) and
> having to revert it in Ubuntu with a distro patch.
> 
> 
> After sleeping on it for a night, I'm now unsure if the patch I sent in
> this thread is what you guys really want. I'll go back to talking in
> pseudocode. This is what we have in 4.14:
> 
>   if action == RET_ALLOW:
>     do not log
>   else if action == RET_KILL && RET_KILL in actions_logged:
>     log
>   else if action == RET_LOG && RET_LOG in actions_logged:
>     log
>   else if filter-requests-logging && action in actions_logged:
>     log
>   else if audit_enabled && process-is-being-audited:
>     log
>   else:
>     do not log
> 
> This is what the patch in this thread does:
> 
> --- a/seccomp-log.pseudo
> +++ b/seccomp-log.pseudo
> @@ -6,7 +6,5 @@
>      log
>    else if filter-requests-logging && action in actions_logged:
>      log
> -  else if audit_enabled && process-is-being-audited:
> -    log
>    else:
>      do not log
> 
> Instead of that change, now I'm wondering if this is what you really
> want:
> 
> --- a/seccomp-log.pseudo
> +++ b/seccomp-log.pseudo
> @@ -6,7 +6,8 @@
>      log
>    else if filter-requests-logging && action in actions_logged:
>      log
> -  else if audit_enabled && process-is-being-audited:
> +  else if audit_enabled && process-is-being-audited &&
> +          action in actions_logged:
>      log
>    else:
>      do not log
> 
> After refactoring the 'action in actions_logged' check, it would leave
> us with this:
> 
>   if action == RET_ALLOW:
>     do not log
>   else if action not in actions_logged:
>     do not log

Yeah, this would let us drop the trap return. While errno can lead to a lot of 
logging, in practice I just don't see them very often if ever.

-Steve

>   else if action == RET_KILL:
>     log
>   else if action == RET_LOG:
>     log
>   else if filter-requests-logging:
>     log
>   else if audit_enabled && process-is-being-audited:
>     log
>   else:
>     do not log