Date:   Mon, 16 Jan 2017 12:04:15 +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 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 

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