Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751201AbdFBPgm (ORCPT ); Fri, 2 Jun 2017 11:36:42 -0400 Received: from h2.hallyn.com ([78.46.35.8]:40112 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751148AbdFBPgl (ORCPT ); Fri, 2 Jun 2017 11:36:41 -0400 Date: Fri, 2 Jun 2017 10:36:47 -0500 From: "Serge E. Hallyn" To: Matt Brown Cc: Alan Cox , Kees Cook , Casey Schaufler , Boris Lukashev , Greg KH , "Serge E. Hallyn" , "kernel-hardening@lists.openwall.com" , linux-security-module , linux-kernel Subject: Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN Message-ID: <20170602153647.GA2688@mail.hallyn.com> References: <20170530132427.016053da@alans-desktop> <2ab8580e-bf8e-21bd-6bfa-33e5fa82400b@nmatt.com> <20170530235106.11aab25c@alans-desktop> <3bd4ff7b-6f7d-52b0-03f6-026bac79f11f@nmatt.com> <20170531005633.484a2e14@alans-desktop> <20170601140812.583cf0a5@alans-desktop> <20170601222432.6f593538@lxorguk.ukuu.org.uk> <2d0ad49c-886e-1caf-771a-d251957f614c@nmatt.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2d0ad49c-886e-1caf-771a-d251957f614c@nmatt.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4683 Lines: 103 Quoting Matt Brown (matt@nmatt.com): > On 6/1/17 5:24 PM, Alan Cox wrote: > >> There's a difference between "bugs" and "security bugs". Letting > > > > Not really, it's merely a matter of severity of result. A non security > > bug that hoses your hard disk is to anyone but security nutcases at > > least as bad as a security hole. > > > >> security bugs continue to get exploited because we want to flush out > >> bugs seems insensitive to the people getting attacked. I'd rather > >> protect against a class of bug than have to endless fix each bug. > > > > The others are security bugs too to varying degree > > > >>> I'm not against doing something to protect the container folks, but that > >>> something as with Android is a whitelist of ioctls. And if we need to do > >>> this with a kernel hook lets do it properly. > >>> > >>> Remember the namespace of the tty on creation > >>> If the magic security flag is set then > >>> Apply a whitelist to *any* tty ioctl call where the ns doesn't > >>> match > >>> > >>> and we might as well just take the Android whitelist since they've kindly > >>> built it for us all! > >>> > >>> In the tty layer it ends up being something around 10 lines of code and > >>> some other file somewhere in security/ that's just a switch or similar > >>> with the whitelisted ioctl codes in it. > >>> > >>> That (or a similar SELinux ruleset) would actually fix the problem. > >>> SELinux would be better because it can also apply the rules when doing > >>> things like su/sudo/... > >> > >> Just to play devil's advocate, wouldn't such a system continue to not > >> address your physical-console concerns? I wouldn't want to limit the > > > > It would for the cases that a whitelist and container check covers - > > because the whitelist wouldn't allow you to do anything but boring stuff > > on the tty. TIOCSTI is just one of a whole range of differently stupid > > and annoying opportunities. Containers do not and should not be able to > > set the keymap, change the video mode, use console selection, make funny > > beepy noises, access video I/O registers and all the other stuff like > > that. Nothing is going to break if we have a fairly conservative > > whitelist. > > > >> protection to only containers (but it's a good start), since it > >> wouldn't protect people not using containers that still have a > >> privileged TTY attached badly somewhere. > > > > How are you going to magically fix the problem. I'm not opposed to fixing > > the real problem but right now it appears to be a product of wishful > > thinking not programming. What's the piece of security code that > > magically discerns the fact you are running something untrusted at the > > other end of your tty. SELinux can do it via labelling but I don't see > > any generic automatic way for the kernel to magically work out when to > > whitelist and when not to. If there is a better magic rule than > > differing-namespace then provide the code. > > > > You can't just disable TIOCSTI, it has users deal with it. You can > > get away with disabling it for namespace crossing I think but if you do > > that you need to disable a pile of others. > > > > (If it breaks containers blocking TIOCSTI then we need to have a good > > look at algorithms for deciding when to flush the input queue on exiting > > a container or somesuch) > > > >> If you're talking about wholistic SELinux policy, sure, I could > >> imagine a wholistic fix. But for the tons of people without a > >> comprehensive SELinux policy, the proposed protection continues to > >> make sense. > > > > No it doesn't. It's completely useless unless you actually bother to > > address the other exploit opportunities. > > > > Right now the proposal is a hack to do > > > > if (TIOCSTI && different_namespace && magic_flag) > > > > > This is not what my patch does. Mine is like: > > if (TIOCSTI && !ns_capable(tty->owner_user_ns, CAP_SYS_ADMIN) && > magic_flag) > > in other words: > if (TIOCSTI && (different_owner_user_ns || !CAP_SYS_ADMIN) && > magic_flag) > > can you specify what you mean by different_namespace? which namespace? I think you're focusing on the wrong thing. Your capable check (apart from the fact that I think I've been convinced CAP_SYS_ADMIN is wrong) is fine. The key point is to not only check for TIOCSTI, but instead check for a whitelisted ioctl. What would the whitelist look like? Should configuing that be the way that you enable/disable, instead of the sysctl in this patchset? So by default the whitelist includes all ioctls (no change), but things like sandboxes/sudo/container-starts can clear out the whitelist?