[PATCH v3 0/3] Extend the vTPM proxy driver to pass locality

[PATCH v3 0/3] Extend the vTPM proxy driver to pass locality

[Stefan Berger]

The purpose of this series of patches is to enable the passing of the locality
a command is executing in to a recipeint, i.e., TPM emulator. To enable this we
introduce a new flag for the device creation ioctl that requests that the
locality be prepended to every command. For applications to check which flags
the driver supports, we add a new ioctl that returns a bitmask of supported flags.

v2->v3:
  - addressed Jarkko's comments

v1->v2:
  - fixed return value from function in patch 3/3

Stefan Berger (3):
  tpm: vtpm_proxy: Implement new ioctl to get supported flags
  tpm: vtpm_proxy: Implement request_locality function.
  tpm: vtpm_proxy: Add flag for ioctl to request locality prepended to
    command

 drivers/char/tpm/tpm_vtpm_proxy.c | 53 ++++++++++++++++++++++++++++++++++++---
 include/uapi/linux/vtpm_proxy.h   | 17 +++++++++++--
 2 files changed, 64 insertions(+), 6 deletions(-)

-- 
2.4.3

[PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Stefan Berger]

Implement VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl to get the bitmask
of flags that the vtpm_proxy driver supports in the
VTPM_PROXY_IOC_NEW_DEV ioctl. This helps user space in deciding
which flags to set in that ioctl.

Signed-off-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 29 +++++++++++++++++++++++++++++
 include/uapi/linux/vtpm_proxy.h   | 11 +++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 751059d..fb4d207 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -592,6 +592,33 @@ static long vtpmx_ioc_new_dev(struct file *file, unsigned int ioctl,
 	return 0;
 }
 
+/**
+ * vtpmx_ioc_get_supt_flags - handler for the %VTPM_PROXY_IOC_GET_SUPT_FLAGS
+ *                            ioctl
+ * @file:	/dev/vtpmx
+ * @ioctl:	the ioctl number
+ * @arg:	pointer to the struct vtpmx_proxy_get_supt_flags
+ *
+ * Return the bitfield of supported flags
+ */
+static long vtpmx_ioc_get_supt_flags(struct file *file, unsigned int ioctl,
+				     unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	struct vtpm_proxy_supt_flags __user *vtpm_supt_flags_p = argp;
+	struct vtpm_proxy_supt_flags flags = {
+		.flags = VTPM_PROXY_FLAGS_ALL,
+	};
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_to_user(vtpm_supt_flags_p, &flags, sizeof(flags)))
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  * vtpmx_fops_ioctl: ioctl on /dev/vtpmx
  *
@@ -604,6 +631,8 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
 	switch (ioctl) {
 	case VTPM_PROXY_IOC_NEW_DEV:
 		return vtpmx_ioc_new_dev(f, ioctl, arg);
+	case VTPM_PROXY_IOC_GET_SUPT_FLAGS:
+		return vtpmx_ioc_get_supt_flags(f, ioctl, arg);
 	default:
 		return -ENOIOCTLCMD;
 	}
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index a69e991..83e64e7 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -44,6 +44,17 @@ struct vtpm_proxy_new_dev {
 	__u32 minor;         /* output */
 };
 
+/**
+ * struct vtpm_proxy_supt_flags - parameter structure for the
+ *                                %VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl
+ * @flags: flags supported by the vtpm proxy driver
+ */
+struct vtpm_proxy_supt_flags {
+	__u32 flags;         /* output */
+};
+
 #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
+#define VTPM_PROXY_IOC_GET_SUPT_FLAGS \
+				_IOR(0xa1, 0x01, struct vtpm_proxy_supt_flags)
 
 #endif /* _UAPI_LINUX_VTPM_PROXY_H */
-- 
2.4.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH v3 2/3] tpm: vtpm_proxy: Implement request_locality function.

[Stefan Berger]

Implement the request_locality function. Accept all localities assuming
that the recipient (TPM emulator) handling TPM commands with prepended
locality modifier will check for a valid locality and return a TPM error
in case the locality is invalid.

Signed-off-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index fb4d207..48b9818 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -371,6 +371,11 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
 	return ret;
 }
 
+static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
+{
+	return locality;
+}
+
 static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.recv = vtpm_proxy_tpm_op_recv,
@@ -380,6 +385,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_canceled = vtpm_proxy_tpm_req_canceled,
+	.request_locality = vtpm_proxy_request_locality,
 };
 
 /*
-- 
2.4.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH v3 3/3] tpm: vtpm_proxy: Add flag for ioctl to request locality prepended to command

[Stefan Berger]

For some TPM commands it is necessary that a TPM emulator knows the
locality in which the command is executed. To support conveying the
locality to the emulator, we implement a flag
VTPM_PROXY_FLAG_PREPEND_LOCALITY for the vtpm_proxy driver's
VTPM_PROXY_IOC_NEW_DEV ioctl to request that the locality be prepended
to every TPM command. This flag is also set in the
VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl's flags field to indicate that this
feature is supported in this version of the driver.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 18 ++++++++++++++----
 include/uapi/linux/vtpm_proxy.h   |  6 ++++--
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 48b9818..e45dd33 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -52,7 +52,8 @@ struct proxy_dev {
 };
 
 /* all supported flags */
-#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2)
+#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2 | \
+			       VTPM_PROXY_FLAG_PREPEND_LOCALITY)
 
 static struct workqueue_struct *workqueue;
 
@@ -78,7 +79,10 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 {
 	struct proxy_dev *proxy_dev = filp->private_data;
 	size_t len;
-	int sig, rc;
+	int sig;
+	int rc = 0;
+	size_t offset = 0;
+	uint8_t locality;
 
 	sig = wait_event_interruptible(proxy_dev->wq,
 		proxy_dev->req_len != 0 ||
@@ -102,7 +106,13 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 		return -EIO;
 	}
 
-	rc = copy_to_user(buf, proxy_dev->buffer, len);
+	if (proxy_dev->flags & VTPM_PROXY_FLAG_PREPEND_LOCALITY) {
+		locality = proxy_dev->chip->locality;
+		offset = sizeof(locality);
+		rc = copy_to_user(buf, &locality, offset);
+	}
+	if (!rc)
+		rc = copy_to_user(&buf[offset], proxy_dev->buffer, len);
 	memset(proxy_dev->buffer, 0, len);
 	proxy_dev->req_len = 0;
 
@@ -114,7 +124,7 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 	if (rc)
 		return -EFAULT;
 
-	return len;
+	return offset + len;
 }
 
 /**
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index 83e64e7..ce76f0c 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -21,10 +21,12 @@
 
 /**
  * enum vtpm_proxy_flags - flags for the proxy TPM
- * @VTPM_PROXY_FLAG_TPM2:	the proxy TPM uses TPM 2.0 protocol
+ * @VTPM_PROXY_FLAG_TPM2:		the proxy TPM uses TPM 2.0 protocol
+ * @VTPM_PROXY_FLAG_PREPEND_LOCALITY:	locality byte prepended on each command
  */
 enum vtpm_proxy_flags {
-	VTPM_PROXY_FLAG_TPM2	= 1,
+	VTPM_PROXY_FLAG_TPM2			= 1,
+	VTPM_PROXY_FLAG_PREPEND_LOCALITY	= 2,
 };
 
 /**
-- 
2.4.3

Re: [PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Jason Gunthorpe]

On Thu, May 04, 2017 at 10:56:25AM -0400, Stefan Berger wrote:
> Implement VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl to get the bitmask
> of flags that the vtpm_proxy driver supports in the
> VTPM_PROXY_IOC_NEW_DEV ioctl. This helps user space in deciding
> which flags to set in that ioctl.

you might be better off just having a VTPM_PROXY_IO_ENABLE_FEATURE
.feature = LOCALITY

If that fails then the feature is not supported, no real need for the
query in that case.

Not sure about Jarkko's point on request/release locality.. Is there a
scenario where the emulator should fail the request locality?

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Stefan Berger]

On 05/04/2017 11:34 AM, Jason Gunthorpe wrote:
> On Thu, May 04, 2017 at 10:56:25AM -0400, Stefan Berger wrote:
>> Implement VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl to get the bitmask
>> of flags that the vtpm_proxy driver supports in the
>> VTPM_PROXY_IOC_NEW_DEV ioctl. This helps user space in deciding
>> which flags to set in that ioctl.
> you might be better off just having a VTPM_PROXY_IO_ENABLE_FEATURE
> .feature = LOCALITY

Do you have an example driver that shows how to do this ? Can user space 
query that feature?

>
> If that fails then the feature is not supported, no real need for the
> query in that case.
>
> Not sure about Jarkko's point on request/release locality.. Is there a
> scenario where the emulator should fail the request locality?

We could filter localities 5 and higher on the level of the driver 
(patch 2/3) since basically there are only 5 localities (0-4) in any TPM 
interface today. The typical hardware locality 4 would be filtered by 
the emulator per policy passed via command line, but I would allow it on 
the level of this driver. An error message would be returned for any 
command executed in that locality, unless the 'policy' allows it. 
Localities 0-3 should just be selectable. The TPM TIS (in the hardware) 
implements some complicated scheme when it comes to allowing the 
selection of a locality and I would say we need none of that but just 
tell the vTPM proxy driver the locality (patch 2/3) in which the next 
command will be executed.


>
> Jason
>

Re: [PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Jason Gunthorpe]

On Thu, May 04, 2017 at 01:13:18PM -0400, Stefan Berger wrote:
> On 05/04/2017 11:34 AM, Jason Gunthorpe wrote:
> >On Thu, May 04, 2017 at 10:56:25AM -0400, Stefan Berger wrote:
> >>Implement VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl to get the bitmask
> >>of flags that the vtpm_proxy driver supports in the
> >>VTPM_PROXY_IOC_NEW_DEV ioctl. This helps user space in deciding
> >>which flags to set in that ioctl.
> >you might be better off just having a VTPM_PROXY_IO_ENABLE_FEATURE
> >.feature = LOCALITY
> 
> Do you have an example driver that shows how to do this ? Can user space
> query that feature?

Try and enable the feature, if it fails then there is no feature in
the kernel.

This is the usual way to add new syscalls..

> >If that fails then the feature is not supported, no real need for the
> >query in that case.
> >
> >Not sure about Jarkko's point on request/release locality.. Is there a
> >scenario where the emulator should fail the request locality?
> 
> We could filter localities 5 and higher on the level of the driver (patch
> 2/3) since basically there are only 5 localities (0-4) in any TPM interface
> today. The typical hardware locality 4 would be filtered by the emulator per
> policy passed via command line, but I would allow it on the level of this
> driver. An error message would be returned for any command executed in that
> locality, unless the 'policy' allows it. Localities 0-3 should just be
> selectable. The TPM TIS (in the hardware) implements some complicated scheme
> when it comes to allowing the selection of a locality and I would say we
> need none of that but just tell the vTPM proxy driver the locality (patch
> 2/3) in which the next command will be executed.

Well, if TIS hardware has some scheme I feel like the emulator uAPI should
have enough fidelity to ecompass existing hardware, even if your
current emulator does not need it.

So allowing request_locality to fail from userspace seems reasonable.

Jason

Re: [PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Stefan Berger]

On 05/04/2017 01:20 PM, Jason Gunthorpe wrote:
> On Thu, May 04, 2017 at 01:13:18PM -0400, Stefan Berger wrote:
>> On 05/04/2017 11:34 AM, Jason Gunthorpe wrote:
>>> On Thu, May 04, 2017 at 10:56:25AM -0400, Stefan Berger wrote:
>>>> Implement VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl to get the bitmask
>>>> of flags that the vtpm_proxy driver supports in the
>>>> VTPM_PROXY_IOC_NEW_DEV ioctl. This helps user space in deciding
>>>> which flags to set in that ioctl.
>>> you might be better off just having a VTPM_PROXY_IO_ENABLE_FEATURE
>>> .feature = LOCALITY
>> Do you have an example driver that shows how to do this ? Can user space
>> query that feature?
> Try and enable the feature, if it fails then there is no feature in
> the kernel.
>
> This is the usual way to add new syscalls..
>
>>> If that fails then the feature is not supported, no real need for the
>>> query in that case.
>>>
>>> Not sure about Jarkko's point on request/release locality.. Is there a
>>> scenario where the emulator should fail the request locality?
>> We could filter localities 5 and higher on the level of the driver (patch
>> 2/3) since basically there are only 5 localities (0-4) in any TPM interface
>> today. The typical hardware locality 4 would be filtered by the emulator per
>> policy passed via command line, but I would allow it on the level of this
>> driver. An error message would be returned for any command executed in that
>> locality, unless the 'policy' allows it. Localities 0-3 should just be
>> selectable. The TPM TIS (in the hardware) implements some complicated scheme
>> when it comes to allowing the selection of a locality and I would say we
>> need none of that but just tell the vTPM proxy driver the locality (patch
>> 2/3) in which the next command will be executed.
> Well, if TIS hardware has some scheme I feel like the emulator uAPI should
> have enough fidelity to ecompass existing hardware, even if your
> current emulator does not need it.
>
> So allowing request_locality to fail from userspace seems reasonable.

What's the best interface to use for this ?

Re: [PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Jason Gunthorpe]

On Thu, May 04, 2017 at 01:28:17PM -0400, Stefan Berger wrote:

> >So allowing request_locality to fail from userspace seems reasonable.
> 
> What's the best interface to use for this ?

If locality support is enabled then send a request locality packet to
userspace and block until return, just like command execution?

Jason

Re: [PATCH v3 1/3] tpm: vtpm_proxy: Implement new ioctl to get supported flags

[Stefan Berger]

On 05/04/2017 01:31 PM, Jason Gunthorpe wrote:
> On Thu, May 04, 2017 at 01:28:17PM -0400, Stefan Berger wrote:
>
>>> So allowing request_locality to fail from userspace seems reasonable.
>> What's the best interface to use for this ?
> If locality support is enabled then send a request locality packet to
> userspace and block until return, just like command execution?

We would have to invent a command for that...