From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761185AbdACUpa (ORCPT ); Tue, 3 Jan 2017 15:45:30 -0500 Received: from mail-io0-f169.google.com ([209.85.223.169]:36486 "EHLO mail-io0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758299AbdACUo4 (ORCPT ); Tue, 3 Jan 2017 15:44:56 -0500 MIME-Version: 1.0 In-Reply-To: References: <1483375990-14948-1-git-send-email-tyhicks@canonical.com> <8748cee7-efe3-a603-ef2e-dc9077b6ead4@canonical.com> From: Kees Cook Date: Tue, 3 Jan 2017 12:44:41 -0800 X-Google-Sender-Auth: r60Ej98EcVajjrPD50e-aaJP1B4 Message-ID: Subject: Re: [PATCH 0/2] Begin auditing SECCOMP_RET_ERRNO return actions To: Paul Moore Cc: Tyler Hicks , Eric Paris , Andy Lutomirski , Will Drewry , linux-audit@redhat.com, LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 3, 2017 at 11:42 AM, Paul Moore wrote: > On Tue, Jan 3, 2017 at 8:31 AM, Tyler Hicks wrote: >> On 01/02/2017 04:47 PM, Paul Moore wrote: >>> On Mon, Jan 2, 2017 at 11:53 AM, Tyler Hicks wrote: >>>> This patch set creates the basis for auditing information specific to a given >>>> seccomp return action and then starts auditing SECCOMP_RET_ERRNO return >>>> actions. The audit messages for SECCOMP_RET_ERRNO return actions include the >>>> errno value that will be returned to userspace. >>> >>> I'm replying to this patchset posting because it his my inbox first, >>> but my comments here apply to both this patchset and the other >>> seccomp/audit patchset you posted. >>> >>> In my experience, we have two or three problems (the count varies >>> depending on perspective) when it comes to seccomp filter reporting: >>> >>> 1. Inability to log all filter actions. >>> 2. Inability to selectively enable filtering; e.g. devs want noisy >>> logging, users want relative quiet. >>> 3. Consistent behavior with audit enabled and disabled. >> >> Agreed. Those three logging issues are what have been nagging me the most. > > /me nods I think this sounds fine too. >>> My current thinking - forgive me, this has been kicking around in my >>> head for the better part of six months (longer?) and I haven't >>> attempted to code it up - is to create a sysctl knob for a system wide >>> seccomp logging threshold that would be applied to the high 16-bits of >>> *every* triggered action: if the action was at/below the threshold a >>> record would be emitted, otherwise silence. This should resolve >>> problems #1 and #2, and the code should be relatively straightforward >>> and small. >> >> I like that idea quite a bit. To be completely honest, for #1, I >> personally only care about logging SECCOMP_RET_ERRNO actions but this >> idea solves it in a nice and general way. > > Yeah, I'd much rather solve this problem generally; everybody has > their favorite action and I'd like to avoid solving the same problem > multiple times. > > Sooo ... you want to take a whack at coding this up? ;) > >>> As part of the code above, I expect that all seccomp logging would get >>> routed through a single logging function (sort of like a better >>> implementation of the existing audit_seccomp()) that would check the >>> threshold and trigger the logging if needed. This function could be >>> augmented to check for CONFIG_AUDIT and in the case where audit was >>> not built into the kernel, a simple printk could be used to log the >>> seccomp event; solving problem #3. >> >> That doesn't fully solve #3 for me. In Ubuntu (and I think Debian), we >> build with CONFIG_AUDIT enabled but don't ship auditd by default so >> audit_enabled is false. In that default configuration, we still want >> seccomp audit messages to be printk'ed. I'll need to figure out how to >> cleanly allow opting into seccomp audit messages when CONFIG_AUDIT is >> enabled and audit_enabled is false. > > Heh, so you've got audit built into the kernel but you're not using > it; that sounds "fun". > > Anyway, I think the logging consolidation could still help you, if for > no other reason than everything is going through the same function at > that point. We could do some other stuff there to handle the case > where audit is compiled, but auditd is not running ... we already have > some code in place to handle that for other reasons, check > kernel/audit.c for more information. I'd still work on the other > stuff first and then we can add this in at the end of the patchset. Yeah, I think the "should I report it?" threshold sysctl could just check if audit is enabled... I still wonder, though, isn't there a way to use auditctl to get all the seccomp messages you need? -Kees > >>> We could also add a SECCOMP_RET_AUDIT, or similar, if we still feel >>> that is important (I personally waffle on this), but I think that is >>> independent of the ideas above. >> >> I agree that it is independent but SECCOMP_RET_AUDIT would still be >> important to Ubuntu. >> >> Tyler > > -- > paul moore > www.paul-moore.com -- Kees Cook Nexus Security