From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965662AbdACSgl (ORCPT ); Tue, 3 Jan 2017 13:36:41 -0500 Received: from mga04.intel.com ([192.55.52.120]:50311 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760296AbdACSgL (ORCPT ); Tue, 3 Jan 2017 13:36:11 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,456,1477983600"; d="scan'208";a="45407714" Date: Tue, 3 Jan 2017 20:36:02 +0200 From: Jarkko Sakkinen To: James Bottomley Cc: linux-security-module@vger.kernel.org, tpmdd-devel@lists.sourceforge.net, open list Subject: Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Message-ID: <20170103183602.ar5typcvy2rx7cjs@intel.com> References: <20170102132213.22880-1-jarkko.sakkinen@linux.intel.com> <1483374980.2458.13.camel@HansenPartnership.com> <20170102193320.trawto65nkjccbao@intel.com> <1483393248.2458.32.camel@HansenPartnership.com> <1483421218.19261.4.camel@linux.vnet.ibm.com> <20170103134100.stgxkmzbckon4jfb@intel.com> <1483460095.2464.6.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1483460095.2464.6.camel@linux.vnet.ibm.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 03, 2017 at 08:14:55AM -0800, James Bottomley wrote: > On Tue, 2017-01-03 at 15:41 +0200, Jarkko Sakkinen wrote: > > On Mon, Jan 02, 2017 at 09:26:58PM -0800, James Bottomley wrote: > > > On Mon, 2017-01-02 at 13:40 -0800, James Bottomley wrote: > > > > On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote: > > > > > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley > > > > > wrote: > > > > > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote: > > > > > > > This patch set adds support for TPM spaces that provide a > > > > > > > context for isolating and swapping transient objects. This > > > > > > > patch set does not yet include support for isolating policy > > > > > > > and HMAC sessions but it is trivial to add once the basic > > > > > > > approach is settled (and that's why I created an RFC patch > > > > > > > set). > > > > > > > > > > > > The approach looks fine to me. The only basic query I have > > > > > > is about the default: shouldn't it be with resource manager > > > > > > on rather than off? I can't really think of a use case that > > > > > > wants the RM off (even if you're running your own, having > > > > > > another doesn't hurt anything, and it's still required to > > > > > > share with in-kernel uses). > > > > > > > > > > This is a valid question and here's a longish explanation. > > > > > > > > > > In TPM2_GetCapability and maybe couple of other commands you > > > > > can get handles in the response body. I do not want to have > > > > > special cases in the kernel for response bodies because there > > > > > is no a generic way to do the substitution. What's worse, new > > > > > commands in the standard future revisions could have such > > > > > commands requiring special cases. In addition, vendor specific > > > > > commans could have handles in the response bodies. > > > > > > > > OK, in general I buy this ... what you're effectively saying is > > > > that we need a non-RM interface for certain management type > > > > commands. > > > > > > > > However, let me expand a bit on why I'm fretting about the non-RM > > > > use case. Right at the moment, we have a single TPM device which > > > > you use for access to the kernel TPM. The current tss2 just > > > > makes direct use of this, meaning it has to have 0666 > > > > permissions. This means that any local user can simply DoS the > > > > TPM by running us out of transient resources if they don't > > > > activate the RM. If they get a connection always via the RM, > > > > this isn't a worry. Perhaps the best way of fixing this is to > > > > expose two separate device nodes: one raw to the TPM which we > > > > could keep at 0600 and one with an always RM connection > > > > which we can set to 0666. That would mean that access to the non > > > > -RM connection is either root only or governed by a system set > > > > ACL. > > > > > > OK, so I put a patch together that does this (see below). It all > > > works nicely (with a udev script that sets the resource manager > > > device to 0666): > > > > This is not yet a comment about this suggestion but I guess one thing > > is clear: the stuff in tpm2-space.c and tpm-interface.c changes are > > the thing that we can mostly agree on and the area of argumentation > > is the user space interface to it? > > Agreed. As I've already said, the space and interface code is working > well for me in production on my laptop. > > > Just thinking how to split up the non-RFC patch set. This was also > > what Jason suggested if I understood his remark correctly. > > SUre ... let's get agreement on how we move forward first. How the > patch is activated by the user has to be sorted out as well before it > can go in, but it doesn't have to be the first thing we do. I'm happy > to continue playing with the interfaces to see what works and what > doesn't. My main current feedback is that I think separate devices > works way better than an ioctl becuase the separate devices approach > allows differing system policies for who accesses the RM backed TPM vs > who accesses the raw one. I think I see your point. I would rather name the device as tpms0 but otherwise I think we could do it in the way you suggest... > James /Jarkko