Date:   Mon, 16 Jan 2017 12:05:52 +0200
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     James Bottomley <jejb@...ux.vnet.ibm.com>
Cc:     linux-security-module@...r.kernel.org,
        tpmdd-devel@...ts.sourceforge.net,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tpm: add session handles to the save and restore of the
 tpm2 space manager

On Mon, Jan 16, 2017 at 12:04:15PM +0200, Jarkko Sakkinen wrote:
> On Fri, Jan 13, 2017 at 11:24:13AM -0800, James Bottomley wrote:
> > Session handles are slightly more difficult to manage because any TPM
> > only has a finite number of allowed handles, even if the session has
> > been saved; so when you context save a session, you must not flush it
> > because that would destroy the ability to context load it (you only
> > flush sessions when you're done with them) and the tpm won't re-use
> > the handle.  Additionally, sessions can be flushed as part of the
> > successful execution of a command if the continueSession attribute is
> > clear, so we have to mark any session we find in the command (using
> > TPM2_HT_TAG_FOR_FLUSH) so it can be cleared from the space if the
> > command successfully executes.
> > 
> > Finally, a session may also be cleared by flushing it, so we have to
> > emulate the TPM2_FlushContext command to see if a session is being
> > flushed and manually clear it from the space.
> > 
> > We also fully flush all sessions on device close.
> 
> Some big overview comments without going deeply into details. I will
> use more time for this once the 

... transient object swapping and /dev/tpms0 are in the shape :-)

/Jarkko

> 
> Please do not use handle allocation code for sessions. This commit
> makes the implementation a mess. Just use the phandle directly and
> have array of session phandles for each space.
> 
> I would also almost require to have at minimum two patches: one that
> implements purely isolation and another that implements swapping.
> 
> It might be for example that I want to land TPM spaces with session
> isolation to one release and swapping to n+1 because my hunch tells
> me that it is better to bake the swapping part for a while.
> 
> One more thing. Maybe commit messages should speak uniformally about
> TPM spaces? They are a tool to implement resource manager, not a
> resource manager.
> 
> /Jarkko
> 
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@...senPartnership.com>
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index f5c9355..d8e896e 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -400,6 +400,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> >  	if (chip->dev.parent)
> >  		pm_runtime_get_sync(chip->dev.parent);
> >  
> > +	rc = tpm2_emulate(chip, space, ordinal, buf, bufsiz);
> > +	if (rc)
> > +		goto out;
> > +
> >  	rc = tpm2_prepare_space(chip, space, ordinal, buf, bufsiz);
> >  	if (rc)
> >  		goto out;
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index adf7810..b922652 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -136,7 +136,8 @@ enum tpm2_capabilities {
> >  };
> >  
> >  enum tpm2_properties {
> > -	TPM_PT_TOTAL_COMMANDS	= 0x0129,
> > +	TPM_PT_ACTIVE_SESSIONS_MAX	= 0x0111,
> > +	TPM_PT_TOTAL_COMMANDS		= 0x0129,
> >  };
> >  
> >  enum tpm2_startup_types {
> > @@ -214,6 +215,8 @@ struct tpm_chip {
> >  	struct tpm_space *work_space;
> >  	u32 nr_commands;
> >  	u32 *cc_attrs_tbl;
> > +	struct tpm_sessions *sessions;
> > +	int max_sessions;
> >  };
> >  
> >  #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> > @@ -583,4 +586,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
> >  		       u32 cc, u8 *buf, size_t bufsiz);
> >  int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> >  		      u32 cc, u8 *buf, size_t bufsiz);
> > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space *space);
> > +int tpm2_emulate(struct tpm_chip *chip, struct tpm_space *space,
> > +		  u32 cc, u8 *buf, size_t bufsiz);
> >  #endif
> > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> > index 285361e..e8e9f32 100644
> > --- a/drivers/char/tpm/tpm2-space.c
> > +++ b/drivers/char/tpm/tpm2-space.c
> > @@ -25,15 +25,29 @@ enum tpm2_handle_types {
> >  	TPM2_HT_TRANSIENT	= 0x80000000,
> >  };
> >  
> > -static void tpm2_flush_space(struct tpm_chip *chip)
> > +#define TPM2_HT_TAG_FOR_FLUSH	0xF0000000
> > +
> > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space *space)
> >  {
> > -	struct tpm_space *space = chip->work_space;
> >  	int i;
> >  
> > -	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
> > -		if (space->context_tbl[i] && ~space->context_tbl[i])
> > -			tpm2_flush_context_cmd(chip, space->context_tbl[i],
> > -					       TPM_TRANSMIT_UNLOCKED);
> > +	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
> > +		u32 handle = space->context_tbl[i];
> > +		u32 handle_type;
> > +
> > +		if (handle == 0 || handle == ~0)
> > +			continue;
> > +
> > +		if ((handle & TPM2_HT_TAG_FOR_FLUSH) ==
> > +		    TPM2_HT_TAG_FOR_FLUSH)
> > +			handle &= ~TPM2_HT_TAG_FOR_FLUSH;
> > +
> > +		handle_type = (handle & 0xFF000000);
> > +
> > +		tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
> > +
> > +		space->context_tbl[i] = 0;
> > +	}
> >  }
> >  
> >  struct tpm2_context {
> > @@ -54,15 +68,11 @@ static int tpm2_load_space(struct tpm_chip *chip)
> >  	u32 s;
> >  
> >  	for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
> > +		u32 phandle, phandle_type;
> > +
> >  		if (!space->context_tbl[i])
> >  			continue;
> >  
> > -		/* sanity check, should never happen */
> > -		if (~space->context_tbl[i]) {
> > -			dev_err(&chip->dev, "context table is inconsistent");
> > -			return -EFAULT;
> > -		}
> > -
> >  		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> >  				 TPM2_CC_CONTEXT_LOAD);
> >  		if (rc)
> > @@ -80,9 +90,23 @@ static int tpm2_load_space(struct tpm_chip *chip)
> >  			rc = -EFAULT;
> >  			goto out_err;
> >  		}
> > +		phandle = get_unaligned_be32((__be32 *)&buf.data[TPM_HEADER_SIZE]);
> > +		phandle_type = (phandle & 0xFF000000);
> > +		if (phandle_type == TPM2_HT_TRANSIENT &&
> > +		    space->context_tbl[i] != ~0) {
> > +			dev_err(&chip->dev, "context table is inconsistent");
> > +			rc = -EFAULT;
> > +			goto out_err;
> > +		}
> > +		if ((phandle_type == TPM2_HT_HMAC_SESSION ||
> > +		     phandle_type == TPM2_HT_POLICY_SESSION) &&
> > +		    space->context_tbl[i] != phandle) {
> > +			dev_err(&chip->dev, "session handle changed\n");
> > +			rc = -EFAULT;
> > +			goto out_err;
> > +		}
> >  
> > -		space->context_tbl[i] =
> > -			be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE]);
> > +		space->context_tbl[i] = phandle;
> >  
> >  		j += s;
> >  
> > @@ -93,10 +117,85 @@ static int tpm2_load_space(struct tpm_chip *chip)
> >  
> >  out_err:
> >  	tpm_buf_destroy(&buf);
> > -	tpm2_flush_space(chip);
> > +	tpm2_flush_space(chip, space);
> >  	return rc;
> >  }
> >  
> > +static void tpm2_unmap_sessions(struct tpm_chip *chip, u32 rc)
> > +{
> > +	struct tpm_space *space = chip->work_space;
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
> > +		if ((space->context_tbl[i] & TPM2_HT_TAG_FOR_FLUSH) !=
> > +		    TPM2_HT_TAG_FOR_FLUSH)
> > +			continue;
> > +		if (rc == TPM2_RC_SUCCESS)
> > +			space->context_tbl[i] = 0;
> > +		else
> > +			/* for unsuccessful command, keep session */
> > +			space->context_tbl[i] &= ~TPM2_HT_TAG_FOR_FLUSH;
> > +	}
> > +}
> > +
> > +static int tpm2_map_sessions(struct tpm_chip *chip, u8 *buf, size_t len,
> > +			     size_t start)
> > +{
> > +	struct tpm_space *space = chip->work_space;
> > +	u32 size = be32_to_cpup((__be32 *)&buf[start]);
> > +	int i;
> > +
> > +	/* skip over authorizationSize */
> > +	start += 4;
> > +
> > +	if (size > len - start) {
> > +		dev_err(&chip->dev, "Invalid authorization header size %u\n",
> > +			size);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = start; i < start+size; ) {
> > +		u16 skip;
> > +		__be32 *handlep;
> > +		u8 attr;
> > +		int j;
> > +		u32 handle_type;
> > +
> > +		/* TPMI_SH_AUTH_SESSION */
> > +		handlep = (__be32 *)&buf[i];
> > +		handle_type = get_unaligned_be32(handlep) & 0xFF000000;
> > +		i += 4;
> > +		/* TPM2B_DIGEST */
> > +		skip = get_unaligned_be16((__be16 *)&buf[i]);
> > +		i += skip + sizeof(skip);
> > +		/* TPMA_SESSION */
> > +		attr = buf[i++];
> > +		/* TPM2B_AUTH */
> > +		skip = get_unaligned_be16((__be16 *)&buf[i]);
> > +		i += skip + sizeof(skip);
> > +
> > +		if (handle_type != TPM2_HT_HMAC_SESSION &&
> > +		    handle_type != TPM2_HT_POLICY_SESSION)
> > +			continue;
> > +
> > +		j = 0xFFFFFF - (get_unaligned_be32(handlep) & 0xFFFFFF);
> > +		if (j > ARRAY_SIZE(space->context_tbl) ||
> > +		    !space->context_tbl[j])
> > +			return -EINVAL;
> > +		put_unaligned_be32(space->context_tbl[j], handlep);
> > +		if ((attr & 1) == 0)
> > +			/* session is flushed by the command */
> > +			space->context_tbl[j] |= TPM2_HT_TAG_FOR_FLUSH;
> > +	}
> > +
> > +	if (i != start+size) {
> > +		dev_err(&chip->dev, "Authorization session overflow\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
> >  {
> >  	struct tpm_space *space = chip->work_space;
> > @@ -104,6 +203,7 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
> >  	u32 vhandle;
> >  	u32 phandle;
> >  	u32 attrs;
> > +	u16 tag = get_unaligned_be16((__be16 *)cmd);
> >  	int i;
> >  	int j;
> >  	int rc;
> > @@ -131,11 +231,14 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
> >  		*((__be32 *)&cmd[TPM_HEADER_SIZE + 4 * i]) =
> >  			cpu_to_be32(phandle);
> >  	}
> > +	if (tag == TPM2_ST_SESSIONS)
> > +		tpm2_map_sessions(chip, cmd, len,
> > +				  TPM_HEADER_SIZE + 4*nr_handles);
> >  
> >  	return 0;
> >  
> >  out_err:
> > -	tpm2_flush_space(chip);
> > +	tpm2_flush_space(chip, space);
> >  	return rc;
> >  }
> >  
> > @@ -163,13 +266,17 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
> >  static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
> >  {
> >  	struct tpm_space *space = chip->work_space;
> > -	u32 phandle;
> > +	u32 phandle, phandle_type;
> >  	u32 vhandle;
> >  	u32 attrs;
> >  	u32 return_code = get_unaligned_be32((__be32 *)&rsp[6]);
> > +	u16 tag = get_unaligned_be16((__be16 *)rsp);
> >  	int i;
> >  	int rc;
> >  
> > +	if (tag == TPM2_ST_SESSIONS)
> > +		tpm2_unmap_sessions(chip, return_code);
> > +
> >  	if (return_code != TPM2_RC_SUCCESS)
> >  		return 0;
> >  
> > @@ -185,7 +292,10 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
> >  		return 0;
> >  
> >  	phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);
> > -	if ((phandle & 0xFF000000) != TPM2_HT_TRANSIENT)
> > +	phandle_type = (phandle & 0xFF000000);
> > +	if (phandle_type != TPM2_HT_TRANSIENT &&
> > +	    phandle_type != TPM2_HT_HMAC_SESSION &&
> > +	    phandle_type != TPM2_HT_POLICY_SESSION)
> >  		return 0;
> >  
> >  	/* Garbage collect a dead context. */
> > @@ -208,13 +318,13 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
> >  	}
> >  
> >  	space->context_tbl[i] = phandle;
> > -	vhandle = TPM2_HT_TRANSIENT | (0xFFFFFF - i);
> > +	vhandle = phandle_type | (0xFFFFFF - i);
> >  	*(__be32 *)&rsp[TPM_HEADER_SIZE] = cpu_to_be32(vhandle);
> >  
> >  	return 0;
> >  
> >  out_err:
> > -	tpm2_flush_space(chip);
> > +	tpm2_flush_space(chip, space);
> >  	return rc;
> >  }
> >  
> > @@ -228,9 +338,20 @@ static int tpm2_save_space(struct tpm_chip *chip)
> >  	u32 s;
> >  
> >  	for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
> > -		if (!(space->context_tbl[i] && ~space->context_tbl[i]))
> > +		u32 phandle, phandle_type;
> > +
> > +		phandle = space->context_tbl[i];
> > +
> > +		if (phandle == 0)
> >  			continue;
> >  
> > +		if (phandle == ~0) {
> > +			dev_err(&chip->dev, "context table is inconsistent\n");
> > +			return -EFAULT;
> > +		}
> > +
> > +		phandle_type = (phandle & 0xFF000000);
> > +
> >  		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
> >  				  TPM2_CC_CONTEXT_SAVE);
> >  		if (rc)
> > @@ -260,10 +381,11 @@ static int tpm2_save_space(struct tpm_chip *chip)
> >  
> >  		memcpy(&space->context_buf[j], &buf.data[TPM_HEADER_SIZE], s);
> >  
> > -		tpm2_flush_context_cmd(chip, space->context_tbl[i],
> > -				       TPM_TRANSMIT_UNLOCKED);
> > -
> > -		space->context_tbl[i] = ~0;
> > +		if (phandle_type == TPM2_HT_TRANSIENT) {
> > +			tpm2_flush_context_cmd(chip, space->context_tbl[i],
> > +					       TPM_TRANSMIT_UNLOCKED);
> > +			space->context_tbl[i] = ~0;
> > +		}
> >  
> >  		j += s;
> >  
> > @@ -273,7 +395,7 @@ static int tpm2_save_space(struct tpm_chip *chip)
> >  	return 0;
> >  out_err:
> >  	tpm_buf_destroy(&buf);
> > -	tpm2_flush_space(chip);
> > +	tpm2_flush_space(chip, space);
> >  	return rc;
> >  }
> >  
> > @@ -297,3 +419,60 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> >  
> >  	return 0;
> >  }
> > +
> > +/* if a space is active, emulate some commands */
> > +int tpm2_emulate(struct tpm_chip *chip, struct tpm_space *space,
> > +		 u32 cc, u8 *buf, size_t bufsiz)
> > +{
> > +	int i, j, k;
> > +	u32 vhandle, handle_type, phandle;
> > +	struct tpm2_context *ctx;
> > +	static struct tpm_output_header buf_rc = {
> > +		.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > +		.length = cpu_to_be32(sizeof(struct tpm_output_header)),
> > +	};
> > +
> > +	if (!space)
> > +		return 0;
> > +
> > +	if (cc != TPM2_CC_FLUSH_CONTEXT)
> > +		return 0;
> > +	vhandle = get_unaligned_be32((__be32 *)&buf[10]);
> > +	handle_type = (vhandle & 0xFF000000);
> > +
> > +	if (handle_type != TPM2_HT_HMAC_SESSION &&
> > +	    handle_type != TPM2_HT_POLICY_SESSION &&
> > +	    handle_type != TPM2_HT_TRANSIENT)
> > +		/* let the TPM figure out and return the error */
> > +		return 0;
> > +
> > +	buf_rc.return_code = TPM2_RC_HANDLE;
> > +
> > +	j = 0xFFFFFF - (vhandle & 0xFFFFFF);
> > +	if (j > ARRAY_SIZE(space->context_tbl))
> > +		goto error;
> > +	phandle = space->context_tbl[j];
> > +	if (phandle != ~0)
> > +		goto error;
> > +
> > +	for (i = 0, k = 0; i <= j; i++) {
> > +		ctx = (struct tpm2_context *)&space->context_buf[k];
> > +
> > +		if (space->context_tbl[i] == 0)
> > +			continue;
> > +
> > +		k += sizeof(*ctx) + get_unaligned_be16(&ctx->blob_size);
> > +	}
> > +	/* move all the contexts up */
> > +	memcpy(ctx, &space->context_buf[k], PAGE_SIZE - k);
> > +	space->context_tbl[j] = 0;
> > +
> > +	if (handle_type != TPM2_HT_TRANSIENT)
> > +		tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
> > +
> > +	buf_rc.return_code = TPM2_RC_SUCCESS;
> > +
> > + error:
> > +	memcpy(buf, &buf_rc, sizeof(buf_rc));
> > +	return sizeof(buf_rc);
> > +}
> > diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c
> > index c10b308..3eb5955 100644
> > --- a/drivers/char/tpm/tpms-dev.c
> > +++ b/drivers/char/tpm/tpms-dev.c
> > @@ -36,6 +36,7 @@ static int tpms_release(struct inode *inode, struct file *file)
> >  	struct file_priv *fpriv = file->private_data;
> >  	struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv);
> >  
> > +	tpm2_flush_space(fpriv->chip, &priv->space);
> >  	tpm_common_release(file, fpriv);
> >  	kfree(priv);
> >  
> > 

Powered by blists - more mailing lists