Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

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).

> There's a test script for trying out TPM spaces in
> 
>   git://git.infradead.org/users/jjs/tpm2-scripts.git
> 
> A simple smoke test can be run by
> 
>   sudo python -m unittest -v tpm2_smoke.SpaceTest   

I've also added an enabling patch to the tss

https://build.opensuse.org/package/view_file/home:jejb1:Tumbleweed/tss2/0002-tssProperties-add-TPM_USE_RESOURCE_MANAGER.patch?expand=1

And with that, I've TPM 2 enabled both gnome-keyring and openssl:

https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/gnome-keyring
https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/openssl_tpm_engine

I'm running them in production on my day to day laptop and so far
everything's working nicely (better than 1.2, in fact, since tcsd
periodically crashes necessitating a restart of everything).

So you can definitely add my Tested-By.

James

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

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):

jejb@jarvis:~> ls -l /dev/tpm*
crw------- 1 root root  10,   224 Jan  2 20:54 /dev/tpm0
crw-rw-rw- 1 root root 246, 65536 Jan  2 20:54 /dev/tpm0rm

I've modified the tss to connect to /dev/tpm0rm by default and it all
seems to work.

The patch applies on top of your tabrm branch, by the way.

James

---

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index ac4c05f..25b8d30 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -33,6 +33,7 @@ DEFINE_IDR(dev_nums_idr);
 static DEFINE_MUTEX(idr_lock);
 
 struct class *tpm_class;
+struct class *tpm_rm_class;
 dev_t tpm_devt;
 
 /**
@@ -169,27 +170,39 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->dev_num = rc;
 
 	device_initialize(&chip->dev);
+	device_initialize(&chip->devrm);
 
 	chip->dev.class = tpm_class;
 	chip->dev.release = tpm_dev_release;
 	chip->dev.parent = pdev;
 	chip->dev.groups = chip->groups;
 
+	chip->devrm.parent = pdev;
+	chip->devrm.class = tpm_rm_class;
+
 	if (chip->dev_num == 0)
 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
 	else
 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
 
+	chip->devrm.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num + TPM_NUM_DEVICES);
+
 	rc = dev_set_name(&chip->dev, "tpm%d", chip->dev_num);
 	if (rc)
 		goto out;
+	rc = dev_set_name(&chip->devrm, "tpm%drm", chip->dev_num);
+	if (rc)
+		goto out;
 
 	if (!pdev)
 		chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
 
 	cdev_init(&chip->cdev, &tpm_fops);
+	cdev_init(&chip->cdevrm, &tpm_rm_fops);
 	chip->cdev.owner = THIS_MODULE;
+	chip->cdevrm.owner = THIS_MODULE;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
+	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
 
 	chip->tr_buf.data = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
 	if (!chip->tr_buf.data) {
@@ -208,6 +221,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 
 out:
 	put_device(&chip->dev);
+	put_device(&chip->devrm);
 	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_alloc);
@@ -252,7 +266,7 @@ static int tpm_add_char_device(struct tpm_chip *chip)
 			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
-		return rc;
+		goto err_1;
 	}
 
 	rc = device_add(&chip->dev);
@@ -262,16 +276,44 @@ static int tpm_add_char_device(struct tpm_chip *chip)
 			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
-		cdev_del(&chip->cdev);
-		return rc;
+		goto err_2;
+	}
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = cdev_add(&chip->cdevrm, chip->devrm.devt, 1);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devrm), MAJOR(chip->devrm.devt),
+			MINOR(chip->devrm.devt), rc);
+
+		goto err_3;
 	}
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = device_add(&chip->devrm);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to device_register() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devrm), MAJOR(chip->devrm.devt),
+			MINOR(chip->devrm.devt), rc);
+
+		goto err_4;
+	}
 	/* Make the chip available. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, chip, chip->dev_num);
 	mutex_unlock(&idr_lock);
 
 	return rc;
+ err_4:
+	cdev_del(&chip->cdevrm);
+ err_3:
+	device_del(&chip->dev);
+ err_2:
+	cdev_del(&chip->cdev);
+ err_1:
+	return rc;
 }
 
 static void tpm_del_char_device(struct tpm_chip *chip)
@@ -279,6 +321,11 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 	cdev_del(&chip->cdev);
 	device_del(&chip->dev);
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		cdev_del(&chip->cdevrm);
+		device_del(&chip->devrm);
+	}
+
 	/* Make the chip unavailable. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 139638b..bed29f9 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -54,23 +54,28 @@ static void timeout_work(struct work_struct *work)
 	mutex_unlock(&priv->buffer_mutex);
 }
 
-static int tpm_open(struct inode *inode, struct file *file)
+static int tpm_open_internal(struct inode *inode, struct file *file, bool is_rm)
 {
-	struct tpm_chip *chip =
-		container_of(inode->i_cdev, struct tpm_chip, cdev);
+	struct tpm_chip *chip;
 	struct file_priv *priv;
 
+	if (is_rm)
+		chip = container_of(inode->i_cdev, struct tpm_chip, cdevrm);
+	else
+		chip = container_of(inode->i_cdev, struct tpm_chip, cdev);
+
 	/* It's assured that the chip will be opened just once,
 	 * by the check of is_open variable, which is protected
 	 * by driver_lock. */
-	if (test_and_set_bit(0, &chip->is_open)) {
+	if (!is_rm && test_and_set_bit(0, &chip->is_open)) {
 		dev_dbg(&chip->dev, "Another process owns this TPM\n");
 		return -EBUSY;
 	}
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (priv == NULL) {
-		clear_bit(0, &chip->is_open);
+		if (!is_rm)
+			clear_bit(0, &chip->is_open);
 		return -ENOMEM;
 	}
 
@@ -82,9 +87,27 @@ static int tpm_open(struct inode *inode, struct file *file)
 	INIT_WORK(&priv->work, timeout_work);
 
 	file->private_data = priv;
+
+	if (is_rm) {
+		priv->space.context_buf =  kzalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!priv->space.context_buf)
+			return -ENOMEM;
+		priv->has_space = true;
+	}
+
 	return 0;
 }
 
+static int tpm_open(struct inode *inode, struct file *file)
+{
+	return tpm_open_internal(inode, file, false);
+}
+
+static int tpm_rm_open(struct inode *inode, struct file *file)
+{
+	return tpm_open_internal(inode, file, true);
+}
+
 static ssize_t tpm_read(struct file *file, char __user *buf,
 			size_t size, loff_t *off)
 {
@@ -169,65 +192,6 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 	return in_size;
 }
 
-/**
- * tpm_ioc_new_space - handler for %SGX_IOC_NEW_SPACE ioctl
- *
- * Creates a new TPM space that can hold a set of transient objects. The space
- * is isolated with virtual handles that are mapped into physical handles by the
- * driver.
- */
-static long tpm_ioc_new_space(struct file *file, unsigned int ioctl,
-			      unsigned long arg)
-{
-	struct file_priv *priv = file->private_data;
-	struct tpm_chip *chip = priv->chip;
-	int rc = 0;
-
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
-		return -EOPNOTSUPP;
-
-	mutex_lock(&priv->buffer_mutex);
-
-	if (priv->has_space) {
-		rc = -EBUSY;
-		goto out;
-	}
-
-	priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!priv->space.context_buf) {
-		rc = -ENOMEM;
-		goto out;
-	}
-
-	/* The TPM device can be opened again as this file has been moved to a
-	 * TPM handle space.
-	 */
-	priv->has_space = true;
-	clear_bit(0, &chip->is_open);
-out:
-	mutex_unlock(&priv->buffer_mutex);
-	return rc;
-}
-
-static long tpm_ioctl(struct file *file, unsigned int ioctl,
-		      unsigned long arg)
-{
-	switch (ioctl) {
-	case TPM_IOC_NEW_SPACE:
-		return tpm_ioc_new_space(file, ioctl, arg);
-	default:
-		return -ENOIOCTLCMD;
-	}
-}
-
-#ifdef CONFIG_COMPAT
-static long tpm_compat_ioctl(struct file *file, unsigned int ioctl,
-			     unsigned long arg)
-{
-	return tpm_ioctl(file, ioctl, arg);
-}
-#endif
-
 /*
  * Called on file close
  */
@@ -247,7 +211,8 @@ static int tpm_release(struct inode *inode, struct file *file)
 	flush_work(&priv->work);
 	file->private_data = NULL;
 	atomic_set(&priv->data_pending, 0);
-	clear_bit(0, &priv->chip->is_open);
+	if (!priv->has_space)
+		clear_bit(0, &priv->chip->is_open);
 	kfree(priv);
 	return 0;
 }
@@ -258,10 +223,15 @@ const struct file_operations tpm_fops = {
 	.open = tpm_open,
 	.read = tpm_read,
 	.write = tpm_write,
-	.unlocked_ioctl = tpm_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = tpm_compat_ioctl,
-#endif
+	.release = tpm_release,
+};
+
+const struct file_operations tpm_rm_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpm_rm_open,
+	.read = tpm_read,
+	.write = tpm_write,
 	.release = tpm_release,
 };
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a1ae57e..c1829de 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1194,9 +1194,17 @@ static int __init tpm_init(void)
 		return PTR_ERR(tpm_class);
 	}
 
-	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm");
+	tpm_rm_class = class_create(THIS_MODULE, "tpmrm");
+	if (IS_ERR(tpm_rm_class)) {
+		pr_err("couldn't create tpmrm class\n");
+		class_destroy(tpm_class);
+		return PTR_ERR(tpm_rm_class);
+	}
+
+	rc = alloc_chrdev_region(&tpm_devt, 0, 2*TPM_NUM_DEVICES, "tpm");
 	if (rc < 0) {
 		pr_err("tpm: failed to allocate char dev region\n");
+		class_destroy(tpm_rm_class);
 		class_destroy(tpm_class);
 		return rc;
 	}
@@ -1208,7 +1216,8 @@ static void __exit tpm_exit(void)
 {
 	idr_destroy(&dev_nums_idr);
 	class_destroy(tpm_class);
-	unregister_chrdev_region(tpm_devt, TPM_NUM_DEVICES);
+	class_destroy(tpm_rm_class);
+	unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
 }
 
 subsys_initcall(tpm_init);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index c6171e5..890fb6b 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,8 +186,8 @@ struct tpm_buf {
 };
 
 struct tpm_chip {
-	struct device dev;
-	struct cdev cdev;
+	struct device dev, devrm;
+	struct cdev cdev, cdevrm;
 
 	/* A driver callback under ops cannot be run unless ops_sem is held
 	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
@@ -493,8 +493,10 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 }
 
 extern struct class *tpm_class;
+extern struct class *tpm_rm_class;
 extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
+extern const struct file_operations tpm_rm_fops;
 extern struct idr dev_nums_idr;
 
 enum tpm_transmit_flags {

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jarkko Sakkinen]

On Tue, Jan 03, 2017 at 08:36:10AM -0800, James Bottomley wrote:
> On Tue, 2017-01-03 at 15:51 +0200, Jarkko Sakkinen wrote:
> > On Mon, Jan 02, 2017 at 01:40:48PM -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.
> > 
> > Not only that.
> > 
> > Doing virtualization for commands like GetCapability is just a better
> > fit for doing in the user space. You could have a thin translation 
> > layer in your TSS library for example to handle these specific
> > messages.
> 
> Yes, we could do it that way too.  To be honest I can't see much use
> for getting the transient handles and all the other handles you'd be
> interested in aren't virtualized.
> 
> > > 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.
> > 
> > I'm not sure about this. Why you couldn't have a very thin daemon 
> > that prepares the file descriptor and sends it through UDS socket to 
> > a client.
> 
> So I'm a bit soured on daemons from the trousers experience: tcsd
> crashed regularly and when it did it took all the TPM connections down
> irrecoverably.  I'm not saying we can't write a stateless daemon to fix
> most of the trousers issues, but I think it's valuable first to ask the
> question, "can we manage without a daemon at all?"  I actually think
> the answer is "yes", so I'm interested in seeing how far that line of
> research gets us.

This was not a good argument in the first place because you could also
use daemon with tpms0. We can ignore this.

> >   The non-RFC version will also have whitelisting ioctl for
> > further restricting the file descriptor to only specific TPM
> > commands.
> > 
> > This is also architecture I preseted in my LSS presentation and I 
> > think it makes sense especially when I add the whitelisting to the
> > pack.
> 
> Do you have a link to the presentation?  The Plumbers etherpad doesn't
> contain it.  I've been trying to work out whether a properly set up TPM
> actually does need any protections at all.  As far as I can tell, once
> you've set all the hierarchy authorities and the lockout one, you're
> pretty well protected.

http://events.linuxfoundation.org/sites/events/files/slides/201608-LinuxSecuritySummit-TPM.pdf

> > > James
> > 
> > I'm more dilated to keep things way they are now. I'll stick to that 
> > at least with the first non-RFC version and hopefully get the tpm2
> > -space.c part reviewed as I split that stuff to a separate commit.
> 
> Sure, we need the patch in an acceptable form first.  I'll keep
> worrying about the systems implications, but I can layer playing with
> those on top of what you do.
> 
> James

/Jarkko

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Tue, Jan 03, 2017 at 08:36:10AM -0800, James Bottomley wrote:

> > I'm not sure about this. Why you couldn't have a very thin daemon 
> > that prepares the file descriptor and sends it through UDS socket to 
> > a client.
> 
> So I'm a bit soured on daemons from the trousers experience: tcsd
> crashed regularly and when it did it took all the TPM connections down
> irrecoverably.  I'm not saying we can't write a stateless daemon to fix
> most of the trousers issues, but I think it's valuable first to ask the
> question, "can we manage without a daemon at all?"  I actually think
> the answer is "yes", so I'm interested in seeing how far that line of
> research gets us.

There is clearly no need for a daemon to be involved when working on
simple tasks like key load and key sign/enc/dec actions, adding such a
thing only increases the complexity.

If we discover a reason to have a daemon down the road then it should
work in some way where the user space can call out to the daemon over
a different path than the kernel. (eg dbus or something)

> Do you have a link to the presentation?  The Plumbers etherpad doesn't
> contain it.  I've been trying to work out whether a properly set up TPM
> actually does need any protections at all.  As far as I can tell, once
> you've set all the hierarchy authorities and the lockout one, you're
> pretty well protected.

I think we should also consider TPM 1.2 support in all of this, it is
still a very popular peice of hardware and it is equally able to
support a RM.

So, in general, I'd prefer to see the unprivileged char dev hard
prevented by the kernel from doing certain things:

- Wipe the TPM
- Manipulate the SRK, nvram, tpm flags, change passwords etc
- Read back the EK
- Write to PCRs
- etc.

Even if TPM 2 has a stronger password based model, I still think the
kernel should hard prevent those sorts of actions even if the user
knows the TPM password.

Realistically people in less senstive environments will want to use
the well known TPM passwords and still have reasonable safety in their
unprivileged accounts.

Jason

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

On Tue, 2017-01-03 at 14:32 -0700, Jason Gunthorpe 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).
> 
> I haven't looked too closely at TPM 2.0 stuff, but at least for 1.2 
> we should have a kernel white-list of allowed commands within a RM
> context, so having the RM on by default would break all of the user
> space.
> 
> I really think the only way forward here is a new char dev that is
> safe for unprivileged/concurrent use and migrate the user space stack
> to use it instead.

That's effectively what /dev/tpms0 would be, with /dev/tpm0 giving full
fledged access.

> > And with that, I've TPM 2 enabled both gnome-keyring and openssl:
> > 
> > https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/gnome
> > -keyring
> > https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/opens
> > sl_tpm_engine
>  
> > I'm running them in production on my day to day laptop and so far
> > everything's working nicely (better than 1.2, in fact, since tcsd
> > periodically crashes necessitating a restart of everything).
> 
> You granted your unprivileged user access to /dev/tpm0 then? FYI I
> think that is a dangerous idea..

No, I granted access to the resource manager device (I'm running with a
combination of Jarkko's and my patches).

James

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Tue, Jan 03, 2017 at 02:39:58PM -0800, James Bottomley wrote:

> > I think we should also consider TPM 1.2 support in all of this, it is
> > still a very popular peice of hardware and it is equally able to
> > support a RM.
> 
> I've been running with the openssl and gnome-keyring patches in 1.2 for
> months now.  The thing about 1.2 is that the volatile store is much
> larger, so there's a lot less of a need for a RM.  It's only a
> requirement in 2.0 because most shipping TPMs only seem to have room
> for about 3 objects.

It would be great if the 1.2 RM could support just enough to allow RSA
key operations from userspace, without key virtualization. That would
allow the plugins that already exist to move to the RM interface and
we can get rid of the hard dependency on trousers.

I honestly don't think this should be much work beyond what Jarkko has
already done...

> > So, in general, I'd prefer to see the unprivileged char dev hard
> > prevented by the kernel from doing certain things:
> > 
> > - Wipe the TPM
> > - Manipulate the SRK, nvram, tpm flags, change passwords etc
> > - Read back the EK
> 
> These are all things that the TPM itself is capable of enforcing a
> policy for.  I think we should aim for correct setup of the TPM in the
> first place so it enforces the policy in a standard manner rather than
> having an artificial policy enforcement in the kernel.

Well, by policy you mean 'know the owner password' which at least I am
*very* nervous about exposing beyond the super user - certainly in my
embedded systems.

On a desktop I think these actions should be protected by the usual
'sudo' scheme dbus has *in addition* to the owner password.

It is rare that anyone would want to do these actions this seems like
the right choice from a security perspective.

> > - Write to PCRs
> 
> The design of a TPM is mostly that it's up to user space to deal with
> this.  Userspace can, of course, kill the TPM ability to quote and seal
> to PCRs by inappropriately extending them.  However, there are a lot of
> responsible applications that want to use PCRs in userspace; for
> instance cloud boot and attestation.  We don't really want to restrict
> their ability arbitrarily.

The entire RM model is that of a sandbox, so if extending the PCR is
viewable by other RM clients it must be prevented. We don't want a
user to be able to DOS other users by extending a PCR and breaking
system attestation or unsealing.

Like you say below localities may be part of the answer here, and I
also recall that various PCRs become read-only at certain localities.

However, until we figure out a security model for writing PCRs I think
the RM has to ban them.

> > Even if TPM 2 has a stronger password based model, I still think the
> > kernel should hard prevent those sorts of actions even if the user
> > knows the TPM password.
> 
> That would make us different from TPM1.2: there, if you know the owner
> authorisation, trousers will pretty much let you do anything.

Well, I also think trousers is wrong to do that. :)

But this is not trousers, this is an in-kernel 0666 char dev that will
be active on basically every Linux system with a TPM. I think we have
a duty to be very conservative here.

This is why I want to see a command white list in Jarkko's patches to
start. Every command exposed needs a very careful security analysis
first, and we should start with only the commands we know are safe :\

> > Realistically people in less senstive environments will want to use
> > the well known TPM passwords and still have reasonable safety in 
> > their unprivileged accounts.
> 
> Can we not do most of this with localities?  In theory locality 0 is
> supposed to be only the bios and the boot manager and the OS gets to
> access 1-3.  We could reserve one for the internal kernel and still
> have a couple for userspace (I'll have to go back and check numbers; I
> seem to remember there were odd restrictions on which PCR you can reset
> and extend in which locality).  If we have two devices (one for each
> locality) we could define a UNIX ACL on the devices to achieve what you
> want.

Good point, yes, localities should be thought about when designing
this new RM char dev uAPI...

Our support for localities in the kernel today uses some really gross
sysfs file and is basically insane, IMHO.

Maybe there should be a /dev/tpmrm for each locality? If so then only
the safe one with unwritable localities can be 0666 by default..

Jason

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Andy Lutomirski]

On 01/02/2017 09:26 PM, 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):
>
> jejb@jarvis:~> ls -l /dev/tpm*
> crw------- 1 root root  10,   224 Jan  2 20:54 /dev/tpm0
> crw-rw-rw- 1 root root 246, 65536 Jan  2 20:54 /dev/tpm0rm
>
> I've modified the tss to connect to /dev/tpm0rm by default and it all
> seems to work.
>
> The patch applies on top of your tabrm branch, by the way.

Conceptually I like this a *lot* better.  I believe that this 
effectively solves my major gripe with the TPM 1.2 ecosystem.

However, can this be taken just a little farther?  IMO the tpm0rm (or 
tpms0 or whatever) node should also restrict commands that can be sent 
(perhaps by in-kernel whitelist?) to those that shouldn't be restricted 
to the owner (by which I probably mean the Owner, the Platform, etc)? 
For example, someone with tpm0rm open should not be able to change key 
hierarchy passwords, write to NV memory, clear hierarchies, etc.

Hmm.  Maybe there should be a way to allocate NV slots to users. 
/dev/tpm/nv0?  I don't really like that idea, though.

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jarkko Sakkinen]

On Tue, Jan 03, 2017 at 09:47:21PM -0800, Andy Lutomirski wrote:
> On 01/02/2017 09:26 PM, 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):
> > 
> > jejb@jarvis:~> ls -l /dev/tpm*
> > crw------- 1 root root  10,   224 Jan  2 20:54 /dev/tpm0
> > crw-rw-rw- 1 root root 246, 65536 Jan  2 20:54 /dev/tpm0rm
> > 
> > I've modified the tss to connect to /dev/tpm0rm by default and it all
> > seems to work.
> > 
> > The patch applies on top of your tabrm branch, by the way.
> 
> Conceptually I like this a *lot* better.  I believe that this effectively
> solves my major gripe with the TPM 1.2 ecosystem.
> 
> However, can this be taken just a little farther?  IMO the tpm0rm (or tpms0
> or whatever) node should also restrict commands that can be sent (perhaps by
> in-kernel whitelist?) to those that shouldn't be restricted to the owner (by
> which I probably mean the Owner, the Platform, etc)? For example, someone
> with tpm0rm open should not be able to change key hierarchy passwords, write
> to NV memory, clear hierarchies, etc.

Yes. This was already discussed in Linux Plumbers. It is trivial to have
that. I just left it out from this RFC patch set to get something not
too complicated out quickly. Whitelist is coming to the non-RFC version.

> Hmm.  Maybe there should be a way to allocate NV slots to users.
> /dev/tpm/nv0?  I don't really like that idea, though.

/Jarkko

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Dr. Greg Wettstein]

On Jan 3,  5:21pm, Ken Goldman wrote:
} Subject: Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

Good morning, I hope this note finds the day going well for everyone.

> On 1/3/2017 4:47 PM, Jason Gunthorpe wrote:
> >
> > I think we should also consider TPM 1.2 support in all of this, it is
> > still a very popular piece of hardware and it is equally able to
> > support a RM.

> I suspect that TPM 2.0 and TPM 1.2 are so different that there may
> be little or no code in common.
>
> My immediate need is for a 2.0 resource manager, since it's a gap in
> the technology, while 1.2 does have tcsd.

In the FWIW department.

I influence architecture and engineering for a company which builds
deterministically modeled and attested computing platforms for high
security assurance environments.  This entity actually builds systems
based on TPM1.2 and TPM2 hardware.  TPM2 prototypes were being
developed based on the simulator which came out of Ken's lab as soon
as it was first made available.

The kernel needs a resource manager.  Everyone needs to think VERY
hard and VERY, VERY carefully about what gets put into the kernel.  In
making a decision, put the ABSOLUTE smallest amount of code into the
kernel which allows various 'TPM2 personalities' to be implemented in
userspace and functionally verified and protected by the physical
instance.  The emergence of commodity TEE's (SGX, et.al) should be in
the back of everyone's mind as a factor in the roadmap.

Repeat incessantly to oneself, TPM1.2 and TPM2 are only similar by
virtue of sharing three ASCII characters.

DO NOT rush this process.  If we do not get this right we will
ultimately end up trying to shove something which is conceptually
worse then tss/tscd into the kernel.

Repeat incesssantly to oneself, policy does not belong in the kernel.

Pay homage to Ken, his TSS2 and TPM2 simulator work are beyond
excellent...

Greg

}-- End of excerpt from Ken Goldman

As always,
Dr. G.W. Wettstein, Ph.D.   Enjellic Systems Development, LLC.
4206 N. 19th Ave.           Specializing in information infra-structure
Fargo, ND  58102            development.
PH: 701-281-1686
FAX: 701-281-3949           EMAIL: greg@enjellic.com
------------------------------------------------------------------------------
"... you should really focus more on simplifying your life.  I
 actually spend most of my time finding ways to de-clog my brain."
                                -- Sarah Wettstein
                                   At the lake

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Kenneth Goldman]

[-- Attachment #1.1: Type: text/plain, Size: 1177 bytes --]

"Dr. Greg Wettstein" <greg-DHO+NtfOqB5PEDpkEIzg7wC/G2K4zDHf@public.gmane.org> wrote on 01/04/2017 11:12:41 
AM:

> The kernel needs a resource manager.  Everyone needs to think VERY
> hard and VERY, VERY carefully about what gets put into the kernel.  In
> making a decision, put the ABSOLUTE smallest amount of code into the
> kernel ...

If you're a TCG member, I invite you to join the TSS WG.  I'd like
an ally for simplicity.
 
> Repeat incessantly to oneself, TPM1.2 and TPM2 are only similar by
> virtue of sharing three ASCII characters.

:-) I have to remember that one!

But, to be accurate, the hardware interface is nearly compatible, and
the high level concepts (key hierarchy, authorization, PCRs, BV space) 
are similar.  The API and implementation are new.

> DO NOT rush this process.  If we do not get this right we will
> ultimately end up trying to shove something which is conceptually
> worse then tss/tscd into the kernel.

The (only?) reason to put any of this in the kernel is that the kernel
also needs access to the TPM. 

> Pay homage to Ken, his TSS2 and TPM2 simulator work are beyond
> excellent...

Thank you.



[-- Attachment #1.2: Type: text/html, Size: 1690 bytes --]

[-- Attachment #2: Type: text/plain, Size: 203 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

On Thu, 2017-01-05 at 10:27 -0700, Jason Gunthorpe wrote:
> On Thu, Jan 05, 2017 at 03:52:02PM +0000, Fuchs, Andreas wrote:
[...]
> > - Session Limits (here it gets ugly):
> 
> > Even thought the TPM supports the same swapping-scheme for sessions
> > as it does for transient objects, it only allows for a limited
> > number of session to be opened (64 in case of PC-Client), called
> > active sessions.  This means that a single process can still DoS 
> > the TPM if it allocates 64 sessions, or 64 processes can DoS the
> > TPM if
> 
> Well, if we have an unpriv fd then it should not be able to DOS the
> system - that would suggest either that FD cannot use sessions or we
> need some kernel solution to guarentee the DOS is not possible.

The sessions issue is a bit of a monster nightmare.  The basic problem
is this issue that they're not fully virtualizable: the TPM retains
knowledge that a session existed on this handle even if the session
context is offloaded. It's this space for "session exists" that the TPM
has a limited space for.

> A combo ioctl that could setup the session, issue an operation in it
> and then delete the session, for instance.

This would work for encryption or HMAC sessions, but probably not for
policy sessions, because they can have an arbitrarily large command
sequence to construct them.

The other issue we're likely to run into if we do it this way is
delayed error reporting.

How about a more traditional approach which would be leasing (basically
what we use for NFS).  Any application opening a session would have a
certain time (probably in ms) to complete it or we'd close the handle
and flush it.  We'd store the jiffies time the session was first
requested and loop over all the extent sessions to find ones which get
too old.  We know there can only be TPM_PT_ACTIVE_SESSIONS_MAX of
these, so it's a cheap operation. It's only a small extra bit of logic
to take care of the GAP problem as well, I think.  If we're full when
you try and start a session, we block you until a handle becomes free
and the max lease time guarantees when this is.

James

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

On Thu, 2017-01-05 at 12:20 -0700, Jason Gunthorpe wrote:
> On Thu, Jan 05, 2017 at 10:33:43AM -0800, James Bottomley wrote:
> 
> > > A combo ioctl that could setup the session, issue an operation in
> > > it
> > > and then delete the session, for instance.
> > 
> > This would work for encryption or HMAC sessions, but probably not 
> > for policy sessions, because they can have an arbitrarily large 
> > command sequence to construct them.
> 
> I'd rather give up features (eg policy sessions, if necessary) for 
> the unpriv fd than give up security of the unpriv fd.

We don't really have that choice: Keys require authorization, so you
have to have an auth session.  If you want things like PCR sealed or
time limited keys, you don't really have a choice on policy sessions
either.

I think we've got to the point where arguing about our divergent use
requirements shows the default should be 0600 and every command enabled
so that whatever changes the device to 0666 also applies the command
filter policy.  It's the fully safe default that satisfies everyone.
Once you have the command blacklist, you can blacklist every policy
command before you make your device 0666.  That effecively achieves
what you want because no-one can now initiate a policy session.

> We always have the out that special stuff can go down the priv path.
> 
> > The other issue we're likely to run into if we do it this way is
> > delayed error reporting.
> 
> Not sure I follow.

If we try to issue a load of commands as a transaction, you only get
the error when the transaction is executed, even if the entity causing
the problem is way back in the command sequence.

> > How about a more traditional approach which would be leasing 
> > (basically what we use for NFS).  Any application opening a session
> > would have
> 
> Doesn't this just change the DOS vector? Now the attacker has to 
> delay execution of TPM commands long enough to force session leases 
> to time out. That isn't too hard to do, asking the TPM to make a RSA 
> key can take seconds, for instance.

I really don't think we can prevent all types of DoS in the TPM.  After
all, in a lot of current silicon, it can take up to 90 seconds to
generate an RSA key using the KDF ... the TPM is unavailable while this
is happening, so there's your DoS on a standard command.

What we need to aim for is effective resource sharing.  The 64 handle
limit is one of the nasty ones that a bunch of applications could
accidentally overflow.  A lease model bounds the TPM blocked time while
you're waiting for an available handle and coping with a lease timeout
can be done in the TSS.  TPM execution time could be added to lease
expiry, so even if someone's doing a sequence of RSA KDFs, you
eventually get your job executed as long as we do the execution of
blocked commands in a strict order.  In this model, you can "DoS" me by
substantially delaying the execution of my commands, but you can't
prevent it from executing within a bounded time.  I suppose we could
even make TPM execution time an RLIMIT.

James

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

On Thu, 2017-01-05 at 16:50 -0700, Jason Gunthorpe wrote:
> On Thu, Jan 05, 2017 at 02:58:46PM -0800, James Bottomley wrote:
> > On Thu, 2017-01-05 at 15:21 -0700, Jason Gunthorpe wrote:
> > > On Thu, Jan 05, 2017 at 11:55:49AM -0800, James Bottomley wrote:
> > > 
> > > > We don't really have that choice: Keys require authorization, 
> > > > so you have to have an auth session.
> > > 
> > > I know, this is why I suggested a combo op (kernel level 
> > > atomicity is clearly DOS safe)..
> > 
> > Transactions are a hard thing to guarantee to be DoS safe and the 
> > more complex they get, the more difficult they are to police within 
> > the kernel.  Plus we have to keep the R/W interface for backwards
> > compatibility now that we have it and I just don't see how we could
> > layer transactions into it without having some sort of in-kernel
> > emulator.
> 
> Again, this was only to make the unpriv FD usable and safe against
> session DOS and that FD wouldn't use the legacy r/w interface. I 
> don't care if root can DOS the TPM via /dev/tpm0. combo ops would 
> need to be simple enough to reason about. (in my TPM libaries API 
> calls are combo'd with session anyhow, and that works quite well for 
> my use models)

In Ken's TSS2 they are for simple authorisation.  However, policy auth
is where it gets tricky and right at the moment I believe I need policy
based authorisation for keys.

> > > Lets stick with the user space broker process and just introduce
> > > enough kernel RM to enable co-existance with kernel users and 
> > > clean -up on crash. This should be enough to make a user space 
> > > broker much simpler.
> > 
> > I wouldn't go that far.  I'm still planning a userspace tss2 
> > without any access broker daemon, but let's see how far I get on 
> > top of the RM.  I think building in stages is a good way to get 
> > actual use experience to guide the next stage.
> 
> I'm sure you can implement what you are doing on top of the RM -
> that isn't a question in my mind.
> 
> My question has always been how does your plugin deliver messages to
> the kernel RM in a way that does not compromise the security of the
> TPM system.

So currently, it doesn't: I have to either run as root or run a udev
script to give me access to the device, neither of which will work out
of the box for distributions because of the security risks you
identified.  I think this is OK for now because the security issue only
has to be sorted out before we make this ready for general release
(i.e. advise the distros how to expose the TPM) and we need to gather
use case data before we do that.

> > > So Jarkko's uapi is basically fine.. No need for a kernel white
> > > list/etc
> > 
> > I suspect we'll eventually get to needing one, but I'm happy to
> > begin
> 
> IMHO the problem with trousers as a broker is just how horribly
> complex it was.
> 
> With the kernel RM this problem becomes very simple:
> 
>  - 1:1 relationship between incoming clients and kernel fds (Jarkko's
>    existing design allows a broker to safely create many RM fds)
>  - Trivial inspection of messages to determine op and check whitelist
>    (just the first couple bytes give you the opcode, easy to deep
>     inspect things like get capability)
>  - No marshal/demarshal, no virtualization, no crypto.
>    (kernel does virtualization, client does marshal and crypto)
>    
> I'm not sure what reason would be big enough to put it in the kernel
> when we seem to have irreconcilable use models for the security 
> policy it needs to implement...
> 
> Maybe that is OK, but it isn't what I was hoping for at the start :)

I actually think this is OK for now.  I have theories about how I'm
going to use policy in TPM keys but I haven't written any code yet. 
 The only actual code I have is the openssl and gnome-keyring patches I
posted to make use of password authorized TPM based RSA keys.

I'd really rather not say what is and isn't safe to blacklist until I
have some use experience of policy based keys: I'm going to continue
working on them (although it looks like a hard problem because we need
to standardise an ASN.1 form of key policy as well) so hopefully I can
acquire the necessary experience over the next few weeks.

I'm seriously pissed of with trousers and will port the trousers based
TPM1.2 RSA key patches I've done to whatever direct connect API you
come up with (just send me a link to the git tree or package or
whatever), so this should generate some use experience for 1.2, like
whether we actually need a RM without trousers and also what the safety
issues might be.  My main laptop is now TPM2, but I have a backup one
that's TPM1.2 and also has all the TPM key stuff as well.

Perhaps our models in practice may turn out not to be as diverse as
they appear in theory, so let's try it out.  This way we can build on
actual use experience for extending the API (and in the mean time we
keep the devices root only like they were previously)

James

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Thu, Jan 05, 2017 at 04:36:42PM -0800, James Bottomley wrote:

> I'm seriously pissed of with trousers and will port the trousers based
> TPM1.2 RSA key patches I've done to whatever direct connect API you
> come up with (just send me a link to the git tree or package or
> whatever), so this should generate some use experience for 1.2, like
> whether we actually need a RM without trousers and also what the safety
> issues might be.  My main laptop is now TPM2, but I have a backup one
> that's TPM1.2 and also has all the TPM key stuff as well.

I would like to add basic 1.2 support to Jarkko's RM. I'm not sure if
my 5mth old will ever let me..

If we do that we write a simple broker as I described that works on
both 1.2 and 2.0 and trousers can go away.

Jason

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jarkko Sakkinen]

On Thu, Jan 05, 2017 at 03:52:02PM +0000, Fuchs, Andreas wrote:
> Great to see this coming along so well. Thanks a lot to Jarkko !
> I just wanted to point out a few things I deem important at this point:
> 
> - Number of virtual handles:
> From what I see there are currently 14 slots for virtual objects in the RFC (if I'm mistaking, please correct me).
> I'd advice to ask the TPM2_GetCapabilities(TPM_CAP_TPM_PROPERTIES, TPM_PT_HR_TRANSIENT_MIN or TPM_PT_HR_TRANSIENT_AVAIL)
> [Note: there is no actual max, i.e. the TPM will allow more transient objects that e.g. 3 if they are small] 
> and provide each TPM space with the same amount as the TPM will tell them is available.
> If an application needs more objects, I'd see a per-fd mini-RM module inside the TSS-libraries handling that job quite well.
> Same would apply for Session with TPM_PT_HR_LOADED_MIN and TPM_PT_HR_LOADED_AVAIL.
> This will reduce the memory consumption inside the kernel and provide userspace with a consistent view on the GetCapabilities vs its actual Allocations.

I rather have a fixed size object. It keeps the implementation simple
compact and stupid and that is what we want at this point. 

Even if I did what you proposed there would not be 1:1 match with
GetCapability provided data because we need to virtual handle values.

Leaving the virtualization of message bodies in the user space is a
design choice from my side. The kernel will provide only basic mechanism
for implementing easily an RM, not a full fledged implementation.

> - Enumeration of loaded (virtual) handles:
> The TPM allows an application to get the list of currently loaded handles TPM2_GetCapabilities(TPM_CAP_HANDLES).
> It would be great to have the RM be as transparent to userspace as possible. The RM spec of TCG therefore says that you need to intercept and override this command (unless it is run in an authentication session where you cannot override it, which is disadviced). It's a design choice, but I'd advice for it after long discussions.

I don't buy this because it doesn't scale (new commands in the standard,
vendor specific commands). It's just something that is factors easier
to do in the user space.

It's not an uncommon design in the Linux kernel to have basic mechanism
in the kernel and do some of the  heavy lifting in the user space. For example,
graphics drivers are like that.

> - Session Limits (here it gets ugly):
> Even thought the TPM supports the same swapping-scheme for sessions as it does for transient objects, it only allows for a limited number of session to be opened (64 in case of PC-Client), called active sessions.
> This means that a single process can still DoS the TPM if it allocates 64 sessions, or 64 processes can DoS the TPM if they allocate 1 session each.
> There are two principle solutions:
> a) Limit the number of active sessions per fd, process, user and hope for the best. Of course this will not really protect you from DoS'ed TPMs.
> b) Kick out old sessions whenever new sessions are requested and TPM is currently full (the TCG RM spec approach). Of course applications need to handle "randomly vanishing" hmac sessions in this case.

I'll think about this. The next patch set version will include
session isolation.

> - Session ungaping (here it gets REALLY ugly):
> The TPM has some scheme for handling sessions that are swapped (contextSaved) out. In this scheme, it can run into the case where it will deny actions on a session handle with a TPM2_RC_GAP error.
> This error means that the time between last usage of the oldest session and the current session is too far apart.
> The reaction needs to be that the RM loads this oldest sesssion (or in my implementation all swaped sessions) into the TPM and contextsave them back right away.
> This becomes especially ugly, when enabling the ability of userspace to contextsave a session on one fd and contextload this session on another fd (or even from another process).

This something we are not going to support in the first production
version. I'm happy review patches that try to do this nicely after
the first version of the feature has landed. I don't care about this
feature all that much.

/Jarkko

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jarkko Sakkinen]

On Wed, Jan 04, 2017 at 10:12:41AM -0600, Dr. Greg Wettstein wrote:
> The kernel needs a resource manager.  Everyone needs to think VERY
> hard and VERY, VERY carefully about what gets put into the kernel.  In
> making a decision, put the ABSOLUTE smallest amount of code into the
> kernel which allows various 'TPM2 personalities' to be implemented in
> userspace and functionally verified and protected by the physical
> instance.  The emergence of commodity TEE's (SGX, et.al) should be in
> the back of everyone's mind as a factor in the roadmap.

Here's my cuts for the kernel:

- Kernel virtualizes handle areas. It's mechanical.
- Kernel does not virtualize bodies. It's not mechanical.
- At least the first version of the RM will not do other than session
  isolation for sessions.

This keeps the core for RM inside the kernel small and tight.

If we start to do some weird shit to the bodies that we think is 
good after long hours over engineering, the implementation will be
a failure. In the user space the way bodies are virtualizes is easier
to fine-tune because it doesn't break every possible app using the
in-kernel RM.

/Jarkko

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/9/2017 6:16 PM, Jarkko Sakkinen wrote:
>
> Here's my cuts for the kernel:
>
> - Kernel virtualizes handle areas. It's mechanical.
> - Kernel does not virtualize bodies. It's not mechanical.
> - At least the first version of the RM will not do other than session
>   isolation for sessions.

Is it correct that "bodies" are the parameter area of the commands and 
responses?

if so, eventually something should virtualize getcapability.  It may be 
safer in user space, but it can mask RM issues.

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Tue, Jan 10, 2017 at 01:16:35AM +0200, Jarkko Sakkinen wrote:
> On Wed, Jan 04, 2017 at 10:12:41AM -0600, Dr. Greg Wettstein wrote:
> > The kernel needs a resource manager.  Everyone needs to think VERY
> > hard and VERY, VERY carefully about what gets put into the kernel.  In
> > making a decision, put the ABSOLUTE smallest amount of code into the
> > kernel which allows various 'TPM2 personalities' to be implemented in
> > userspace and functionally verified and protected by the physical
> > instance.  The emergence of commodity TEE's (SGX, et.al) should be in
> > the back of everyone's mind as a factor in the roadmap.
> 
> Here's my cuts for the kernel:
> 
> - Kernel virtualizes handle areas. It's mechanical.
> - Kernel does not virtualize bodies. It's not mechanical.
> - At least the first version of the RM will not do other than session
>   isolation for sessions.
> 
> This keeps the core for RM inside the kernel small and tight.

I think this makes sense.

In addition the kernel should only permit RM operations that are known
to be 100% correct with the RM.

I think you should stick with your original design basic design,
except instead of using an ioctl to switch modes, use an ioctl to
execute the operation:

struct tpm_ioctl_operation {
   u16 mode;  // == TPM1_RAW,TPM2_RAW,TPM1_RM,TPM2_RM
   u16 locality;
   u32 txlen;
   u32 rxlen;
   const void *txbuf;
   void *rxbuf;
};

The userspace broker would be expected to use a mixture of RM and RAW
operations.

Let's deal with the idea of another cdev some other day when someone
can figure out a comprehensive way to do that securely for unpriv..

Jason

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Andreas Fuchs]

Am 10.01.2017 um 21:05 schrieb Jason Gunthorpe:
> On Tue, Jan 10, 2017 at 01:16:35AM +0200, Jarkko Sakkinen wrote:
>> On Wed, Jan 04, 2017 at 10:12:41AM -0600, Dr. Greg Wettstein wrote:
>>> The kernel needs a resource manager.  Everyone needs to think VERY
>>> hard and VERY, VERY carefully about what gets put into the kernel.  In
>>> making a decision, put the ABSOLUTE smallest amount of code into the
>>> kernel which allows various 'TPM2 personalities' to be implemented in
>>> userspace and functionally verified and protected by the physical
>>> instance.  The emergence of commodity TEE's (SGX, et.al) should be in
>>> the back of everyone's mind as a factor in the roadmap.
>> Here's my cuts for the kernel:
>>
>> - Kernel virtualizes handle areas. It's mechanical.
>> - Kernel does not virtualize bodies. It's not mechanical.
>> - At least the first version of the RM will not do other than session
>>    isolation for sessions.
>>
>> This keeps the core for RM inside the kernel small and tight.

You need to do virtualization inside bodies, because TPM2_FlushContext 
carries it's handles inside the parameter body.
Yep, huge blunder in the TPM spec, but hey, time for quirks... ;-)

> I think this makes sense.
>
> In addition the kernel should only permit RM operations that are known
> to be 100% correct with the RM.
>
> I think you should stick with your original design basic design,
> except instead of using an ioctl to switch modes, use an ioctl to
> execute the operation:
>
> struct tpm_ioctl_operation {
>     u16 mode;  // == TPM1_RAW,TPM2_RAW,TPM1_RM,TPM2_RM
>     u16 locality;
>     u32 txlen;
>     u32 rxlen;
>     const void *txbuf;
>     void *rxbuf;
> };

could we please get an ioctl, that switches the "mode" of the fd entirely.
I'd like to see the write()/read() support still intact.
All my current code uses main-loop based poll on the fd and I don't want
to be force to start using threads...

Thanks,
Andreas

>
> The userspace broker would be expected to use a mixture of RM and RAW
> operations.
>
> Let's deal with the idea of another cdev some other day when someone
> can figure out a comprehensive way to do that securely for unpriv..
>
> Jason
>
> ------------------------------------------------------------------------------
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today. http://sdm.link/xeonphi
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Andreas Fuchs]

Am 09.01.2017 um 23:39 schrieb Jarkko Sakkinen:
> On Thu, Jan 05, 2017 at 03:52:02PM +0000, Fuchs, Andreas wrote:
>> Great to see this coming along so well. Thanks a lot to Jarkko !
>> I just wanted to point out a few things I deem important at this point:
>>
>> - Number of virtual handles:
>>  From what I see there are currently 14 slots for virtual objects in the RFC (if I'm mistaking, please correct me).
>> I'd advice to ask the TPM2_GetCapabilities(TPM_CAP_TPM_PROPERTIES, TPM_PT_HR_TRANSIENT_MIN or TPM_PT_HR_TRANSIENT_AVAIL)
>> [Note: there is no actual max, i.e. the TPM will allow more transient objects that e.g. 3 if they are small]
>> and provide each TPM space with the same amount as the TPM will tell them is available.
>> If an application needs more objects, I'd see a per-fd mini-RM module inside the TSS-libraries handling that job quite well.
>> Same would apply for Session with TPM_PT_HR_LOADED_MIN and TPM_PT_HR_LOADED_AVAIL.
>> This will reduce the memory consumption inside the kernel and provide userspace with a consistent view on the GetCapabilities vs its actual Allocations.
> I rather have a fixed size object. It keeps the implementation simple
> compact and stupid and that is what we want at this point.
>
> Even if I did what you proposed there would not be 1:1 match with
> GetCapability provided data because we need to virtual handle values.
>
> Leaving the virtualization of message bodies in the user space is a
> design choice from my side. The kernel will provide only basic mechanism
> for implementing easily an RM, not a full fledged implementation.
>
>> - Enumeration of loaded (virtual) handles:
>> The TPM allows an application to get the list of currently loaded handles TPM2_GetCapabilities(TPM_CAP_HANDLES).
>> It would be great to have the RM be as transparent to userspace as possible. The RM spec of TCG therefore says that you need to intercept and override this command (unless it is run in an authentication session where you cannot override it, which is disadviced). It's a design choice, but I'd advice for it after long discussions.
> I don't buy this because it doesn't scale (new commands in the standard,
> vendor specific commands). It's just something that is factors easier
> to do in the user space.
>
> It's not an uncommon design in the Linux kernel to have basic mechanism
> in the kernel and do some of the  heavy lifting in the user space. For example,
> graphics drivers are like that.
>
>> - Session Limits (here it gets ugly):
>> Even thought the TPM supports the same swapping-scheme for sessions as it does for transient objects, it only allows for a limited number of session to be opened (64 in case of PC-Client), called active sessions.
>> This means that a single process can still DoS the TPM if it allocates 64 sessions, or 64 processes can DoS the TPM if they allocate 1 session each.
>> There are two principle solutions:
>> a) Limit the number of active sessions per fd, process, user and hope for the best. Of course this will not really protect you from DoS'ed TPMs.
>> b) Kick out old sessions whenever new sessions are requested and TPM is currently full (the TCG RM spec approach). Of course applications need to handle "randomly vanishing" hmac sessions in this case.
> I'll think about this. The next patch set version will include
> session isolation.
>
>> - Session ungaping (here it gets REALLY ugly):
>> The TPM has some scheme for handling sessions that are swapped (contextSaved) out. In this scheme, it can run into the case where it will deny actions on a session handle with a TPM2_RC_GAP error.
>> This error means that the time between last usage of the oldest session and the current session is too far apart.
>> The reaction needs to be that the RM loads this oldest sesssion (or in my implementation all swaped sessions) into the TPM and contextsave them back right away.
>> This becomes especially ugly, when enabling the ability of userspace to contextsave a session on one fd and contextload this session on another fd (or even from another process).
> This something we are not going to support in the first production
> version. I'm happy review patches that try to do this nicely after
> the first version of the feature has landed. I don't care about this
> feature all that much.

I guess all of the above is more or less debatable.

This point here however is not. It's a necessity, otherwise you
will get weird machines that (seemingly) randomly stop working
after a long time; i.e. months. So nobody will find this in tests
or be able to reproduce.

These are the kind of bugs nobody will ever track down.
So please, do not release session-RM-support without taking
care of ungaping...
This would be highly irresponsible !

Thanks,
Andreas

>
> /Jarkko

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jarkko Sakkinen]

On Tue, Jan 10, 2017 at 01:05:58PM -0700, Jason Gunthorpe wrote:
> On Tue, Jan 10, 2017 at 01:16:35AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Jan 04, 2017 at 10:12:41AM -0600, Dr. Greg Wettstein wrote:
> > > The kernel needs a resource manager.  Everyone needs to think VERY
> > > hard and VERY, VERY carefully about what gets put into the kernel.  In
> > > making a decision, put the ABSOLUTE smallest amount of code into the
> > > kernel which allows various 'TPM2 personalities' to be implemented in
> > > userspace and functionally verified and protected by the physical
> > > instance.  The emergence of commodity TEE's (SGX, et.al) should be in
> > > the back of everyone's mind as a factor in the roadmap.
> > 
> > Here's my cuts for the kernel:
> > 
> > - Kernel virtualizes handle areas. It's mechanical.
> > - Kernel does not virtualize bodies. It's not mechanical.
> > - At least the first version of the RM will not do other than session
> >   isolation for sessions.
> > 
> > This keeps the core for RM inside the kernel small and tight.
> 
> I think this makes sense.
> 
> In addition the kernel should only permit RM operations that are known
> to be 100% correct with the RM.
> 
> I think you should stick with your original design basic design,
> except instead of using an ioctl to switch modes, use an ioctl to
> execute the operation:
> 
> struct tpm_ioctl_operation {
>    u16 mode;  // == TPM1_RAW,TPM2_RAW,TPM1_RM,TPM2_RM
>    u16 locality;
>    u32 txlen;
>    u32 rxlen;
>    const void *txbuf;
>    void *rxbuf;
> };
> 
> The userspace broker would be expected to use a mixture of RM and RAW
> operations.
> 
> Let's deal with the idea of another cdev some other day when someone
> can figure out a comprehensive way to do that securely for unpriv..

James, what do you think about this proposal?

/Jarkko


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jarkko Sakkinen]

On Tue, Jan 10, 2017 at 02:29:08PM -0500, Ken Goldman wrote:
> On 1/9/2017 6:16 PM, Jarkko Sakkinen wrote:
> > 
> > Here's my cuts for the kernel:
> > 
> > - Kernel virtualizes handle areas. It's mechanical.
> > - Kernel does not virtualize bodies. It's not mechanical.
> > - At least the first version of the RM will not do other than session
> >   isolation for sessions.
> 
> Is it correct that "bodies" are the parameter area of the commands and
> responses?
> 
> if so, eventually something should virtualize getcapability.  It may be
> safer in user space, but it can mask RM issues.

body == command / response - (header + handle area)

/Jarkko

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

On Wed, 2017-01-11 at 13:34 +0200, Jarkko Sakkinen wrote:
> On Tue, Jan 10, 2017 at 01:05:58PM -0700, Jason Gunthorpe wrote:
> > On Tue, Jan 10, 2017 at 01:16:35AM +0200, Jarkko Sakkinen wrote:
> > > On Wed, Jan 04, 2017 at 10:12:41AM -0600, Dr. Greg Wettstein
> > > wrote:
> > > > The kernel needs a resource manager.  Everyone needs to think 
> > > > VERY hard and VERY, VERY carefully about what gets put into the
> > > > kernel.  In making a decision, put the ABSOLUTE smallest amount 
> > > > of code into the kernel which allows various 'TPM2 
> > > > personalities' to be implemented in userspace and functionally 
> > > > verified and protected by the physical instance.  The emergence 
> > > > of commodity TEE's (SGX, et.al) should be in the back of
> > > > everyone's mind as a factor in the roadmap.
> > > 
> > > Here's my cuts for the kernel:
> > > 
> > > - Kernel virtualizes handle areas. It's mechanical.
> > > - Kernel does not virtualize bodies. It's not mechanical.
> > > - At least the first version of the RM will not do other than
> > > session
> > >   isolation for sessions.
> > > 
> > > This keeps the core for RM inside the kernel small and tight.
> > 
> > I think this makes sense.
> > 
> > In addition the kernel should only permit RM operations that are 
> > known to be 100% correct with the RM.
> > 
> > I think you should stick with your original design basic design,
> > except instead of using an ioctl to switch modes, use an ioctl to
> > execute the operation:
> > 
> > struct tpm_ioctl_operation {
> >    u16 mode;  // == TPM1_RAW,TPM2_RAW,TPM1_RM,TPM2_RM
> >    u16 locality;
> >    u32 txlen;
> >    u32 rxlen;
> >    const void *txbuf;
> >    void *rxbuf;
> > };
> > 
> > The userspace broker would be expected to use a mixture of RM and 
> > RAW operations.
> > 
> > Let's deal with the idea of another cdev some other day when 
> > someone can figure out a comprehensive way to do that securely for
> > unpriv..
> 
> James, what do you think about this proposal?

I don't think it's a good idea for the reasons stated before:

RAW access means the ability to DoS the TPM simply by exhausting
handles.  Therefore, I think most applications only get RM access.  If
you adopt this proposal, then, if you're only opening a single fd, we
can't go by the unix permissions (even the RM only applications need to
open it), so we'd need some permissions infrastructure within the
kernel to reject RAW access from RM only users.  That's why having
multiple devices is much better because we can use the usual UNIX
mechanism to separate access from raw and rm.  I'm less sure about
localities, but there may be the same issue at some point (not all
applications get access to all localities).  It would be good to get
the localities use case sorted out before adding it to an ioctl based
interfaces.

James



------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/11/2017 5:00 AM, Andreas Fuchs wrote:
>
> You need to do virtualization inside bodies, because TPM2_FlushContext
> carries it's handles inside the parameter body.
> Yep, huge blunder in the TPM spec, but hey, time for quirks... ;-)

It's not huge, not even a blunder.  The TPM spec Part 3 has a note 
explaining why the TPM side does it that way.

For the TSS and resource manager, simply treat flushHandle as a handle 
(in the handle area rather than in the parameter/body area).  It all 
just works.  I modified the TSS as a test, and there's no issue at all.



------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Wed, Jan 11, 2017 at 07:39:53AM -0800, James Bottomley wrote:
 
> RAW access means the ability to DoS the TPM simply by exhausting
> handles.  Therefore, I think most applications only get RM access.

Re-read what Jarkko is proposing. He is not making a complete safe &
secure RM in the kernel. He is making a tool to allow userspace and
the kernel to share the TPM sanely.

It is not an access control tool, it is not a security tool, it is not
intended to support safe unpriv userspace access.

So there is no reason to have a different access control model in
userspace, it is not a fundamentally different security environment
from the existing raw device.

A future project to provide an unpriv safe cdev from the kernel is
something different.

Jason

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Wed, Jan 11, 2017 at 11:00:43AM +0100, Andreas Fuchs wrote:

> could we please get an ioctl, that switches the "mode" of the fd entirely.
> I'd like to see the write()/read() support still intact.
> All my current code uses main-loop based poll on the fd and I don't want
> to be force to start using threads...

We currently do not support poll in the kernel for /dev/tpmX.

ie we do not supply a poll method for 'struct file_operations'.

Even worse, the current implementation blocks returning from write()
until the TPM has completed its work, so it doesn't even make sense to
combine it with poll.

Jason

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[James Bottomley]

On Wed, 2017-01-11 at 10:56 -0700, Jason Gunthorpe wrote:
> On Wed, Jan 11, 2017 at 07:39:53AM -0800, James Bottomley wrote:
> 
> > RAW access means the ability to DoS the TPM simply by exhausting
> > handles.  Therefore, I think most applications only get RM access.
> 
> Re-read what Jarkko is proposing. He is not making a complete safe &
> secure RM in the kernel. He is making a tool to allow userspace and
> the kernel to share the TPM sanely.
> 
> It is not an access control tool, it is not a security tool, it is 
> not intended to support safe unpriv userspace access.
> 
> So there is no reason to have a different access control model in
> userspace, it is not a fundamentally different security environment
> from the existing raw device.
> 
> A future project to provide an unpriv safe cdev from the kernel is
> something different.

Right, but we're going around in circles.  I'm currently researching
what it would take to be daemonless, so an ioctl which requires an
access broker daemon would obviously be something I'd object to.

Basically, though, I think you can do both: we can add an ioctl and the
differing device hooks.  I just think for that case RAW vs RM would be
redundant.

James

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Stefan Berger]

On 01/11/2017 01:03 PM, Jason Gunthorpe wrote:
> On Wed, Jan 11, 2017 at 11:00:43AM +0100, Andreas Fuchs wrote:
>
>> could we please get an ioctl, that switches the "mode" of the fd entirely.
>> I'd like to see the write()/read() support still intact.
>> All my current code uses main-loop based poll on the fd and I don't want
>> to be force to start using threads...
> We currently do not support poll in the kernel for /dev/tpmX.
>
> ie we do not supply a poll method for 'struct file_operations'.
>
> Even worse, the current implementation blocks returning from write()
> until the TPM has completed its work, so it doesn't even make sense to
> combine it with poll.

Newer applications could issue an ioctl() after the open() to unblock 
the write().

     Stefan


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Wed, Jan 11, 2017 at 10:25:57AM -0800, James Bottomley wrote:

> Right, but we're going around in circles.  I'm currently researching
> what it would take to be daemonless, so an ioctl which requires an
> access broker daemon would obviously be something I'd object to.

Well, when we figure out a security model that works for that and can
be implemented in the kernel then lets add the new cdev.

But that is *explicitly* not what Jarkko is doing, no reason to jump
the gun.

> Basically, though, I think you can do both: we can add an ioctl and the
> differing device hooks.  I just think for that case RAW vs RM would be
> redundant.

Right, some future new cdev would only support ioctl and only the RM
path, but for priv use having both concurrently available makes sense
as a userspace broker producing a full RM will need to using both paths.

Jason

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Jason Gunthorpe]

On Wed, Jan 11, 2017 at 01:27:30PM -0500, Stefan Berger wrote:
> On 01/11/2017 01:03 PM, Jason Gunthorpe wrote:
> >On Wed, Jan 11, 2017 at 11:00:43AM +0100, Andreas Fuchs wrote:
> >
> >>could we please get an ioctl, that switches the "mode" of the fd entirely.
> >>I'd like to see the write()/read() support still intact.
> >>All my current code uses main-loop based poll on the fd and I don't want
> >>to be force to start using threads...
> >We currently do not support poll in the kernel for /dev/tpmX.
> >
> >ie we do not supply a poll method for 'struct file_operations'.
> >
> >Even worse, the current implementation blocks returning from write()
> >until the TPM has completed its work, so it doesn't even make sense to
> >combine it with poll.
> 
> Newer applications could issue an ioctl() after the open() to unblock the
> write().

The ioctl api I outlined could support poll by having userspace set
the rxbuf = NULL.

The kernel would then launch the tx async and provide poll support to
allow read() to return the result once the tpm has finished.

This can be added as a new capability down the road..

Jason