Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-10-24 Thread James Bottomley
On Wed, 2018-10-24 at 02:48 +0300, Jarkko Sakkinen wrote:
> On Mon, 22 Oct 2018, James Bottomley wrote:
> > [...]

I'll tidy up the descriptions.

> These all sould be combined with the existing session stuff inside
> tpm2-cmd.c and not have duplicate infrastructures. The file name
> should be tpm2-session.c (we neither have tpm2-cmds.c).

You mean move tpm2_buf_append_auth() into the new sessions file as well
... sure, I can do that.

[...]
> > +
> > +/*
> > + * assume hash sha256 and nonces u, v of size SHA256_DIGEST_SIZE
> > but
> > + * otherwise standard KDFa.  Note output is in bytes not bits.
> > + */
> > +static void KDFa(u8 *key, int keylen, const char *label, u8 *u,
> > +u8 *v, int bytes, u8 *out)
> 
> Should this be in lower case? I would rename it as tpm_kdfa().

This one is defined as KDFa() in the standards and it's not TPM
specific (although some standards refer to it as KDFA).  I'm not averse
to making them tpm_kdfe() and tpm_kdfa() but I was hoping that one day
the crypto subsystem would need them and we could move them in there
because KDFs are the new shiny in crypto primitives (TLS 1.2 started
using them, for instance).

> > +{
> > +   u32 counter;
> > +   const __be32 bits = cpu_to_be32(bytes * 8);
> > +
> > +   for (counter = 1; bytes > 0; bytes -= SHA256_DIGEST_SIZE,
> > counter++,
> > +out += SHA256_DIGEST_SIZE) {
> 
> Only one counter is actually used for anything so this is overly
> complicated and IMHO it is ok to call the counter just 'i'. Maybe
> just:
> 
> for (i = 1; (bytes - (i - 1) * SHA256_DIGEST_SIZE) > 0; i++) {
> 
> > +   SHASH_DESC_ON_STACK(desc, sha256_hash);
> > +   __be32 c = cpu_to_be32(counter);
> > +
> > +   hmac_init(desc, key, keylen);
> > +   crypto_shash_update(desc, (u8 *), sizeof(c));
> > +   crypto_shash_update(desc, label, strlen(label)+1);
> > +   crypto_shash_update(desc, u, SHA256_DIGEST_SIZE);
> > +   crypto_shash_update(desc, v, SHA256_DIGEST_SIZE);
> > +   crypto_shash_update(desc, (u8 *),
> > sizeof(bits));
> > +   hmac_final(desc, key, keylen, out);
> > +   }
> > +}
> > +
> > +/*
> > + * Somewhat of a bastardization of the real KDFe.  We're assuming
> > + * we're working with known point sizes for the input parameters
> > and
> > + * the hash algorithm is fixed at sha256.  Because we know that
> > the
> > + * point size is 32 bytes like the hash size, there's no need to
> > loop
> > + * in this KDF.
> > + */
> > +static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8
> > *pt_v,
> > +u8 *keyout)
> > +{
> > +   SHASH_DESC_ON_STACK(desc, sha256_hash);
> > +   /*
> > +* this should be an iterative counter, but because we
> > know
> > +*  we're only taking 32 bytes for the point using a
> > sha256
> > +*  hash which is also 32 bytes, there's only one loop
> > +*/
> > +   __be32 c = cpu_to_be32(1);
> > +
> > +   desc->tfm = sha256_hash;
> > +   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> > +
> > +   crypto_shash_init(desc);
> > +   /* counter (BE) */
> > +   crypto_shash_update(desc, (u8 *), sizeof(c));
> > +   /* secret value */
> > +   crypto_shash_update(desc, z, EC_PT_SZ);
> > +   /* string including trailing zero */
> > +   crypto_shash_update(desc, str, strlen(str)+1);
> > +   crypto_shash_update(desc, pt_u, EC_PT_SZ);
> > +   crypto_shash_update(desc, pt_v, EC_PT_SZ);
> > +   crypto_shash_final(desc, keyout);
> > +}
> > +
> > +static void tpm_buf_append_salt(struct tpm_buf *buf, struct
> > tpm_chip *chip,
> > +   struct tpm2_auth *auth)
> 
> Given the complexity of this function and some not that obvious
> choices in the implementation (coordinates), it would make sense to
> document this function.

I'll try to beef up the salting description

> > +{
> > +   struct crypto_kpp *kpp;
> > +   struct kpp_request *req;
> > +   struct scatterlist s[2], d[1];
> > +   struct ecdh p = {0};
> > +   u8 encoded_key[EC_PT_SZ], *x, *y;
> 
> Why you use one character variable name 'p' and longer name
> 'encoded_key'?
> 
> > +   unsigned int buf_len;
> > +   u8 *secret;
> > +
> > +   secret = kmalloc(EC_PT_SZ, GFP_KERNEL);
> > +   if (!secret)
> > +   return;
> > +
> > +   p.curve_id = ECC_CURVE_NIST_P256;
> 
> Could this be set already in the initialization?

I'm never sure about designated

Re: [PATCH v4 0/7] add integrity and security to TPM2 transactions

2018-10-24 Thread James Bottomley
On Wed, 2018-10-24 at 02:51 +0300, Jarkko Sakkinen wrote:
> I would consider sending first a patch set that would iterate the
> existing session stuff to be ready for this i.e. merge in two
> iterations (emphasis on the word "consider"). We can probably merge
> the groundwork quite fast.

I realise we're going to have merge conflicts on the later ones, so why
don't we do this: I'll still send as one series, but you apply the ones
you think are precursors and I'll rebase and resend the rest?

James



Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-10-23 Thread James Bottomley
On Mon, 2018-10-22 at 19:19 -0300, Ard Biesheuvel wrote:
[...]
> > +static void hmac_init(struct shash_desc *desc, u8 *key, int
> > keylen)
> > +{
> > +   u8 pad[SHA256_BLOCK_SIZE];
> > +   int i;
> > +
> > +   desc->tfm = sha256_hash;
> > +   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> 
> I don't think this actually does anything in the shash API
> implementation, so you can drop this.

OK, I find crypto somewhat hard to follow.  There were bits I had to
understand, like when I wrote the CFB implementation or when I fixed
the ECDH scatterlist handling, but I've got to confess, in time
honoured tradition I simply copied this from EVM crypto without
actually digging into the code to understand why.

>  However, I take it this means that hmac_init() is never called in
> contexts where sleeping is not allowed? For the relevance of that,
> please see below.

Right, these routines are always called as an adjunct to a TPM
operation and TPM operations can sleep, so we must at least have kernel
thread context.

[...]
> > +   /* encrypt before HMAC */
> > +   if (auth->attrs & TPM2_SA_DECRYPT) {
> > +   struct scatterlist sg[1];
> > +   u16 len;
> > +   SKCIPHER_REQUEST_ON_STACK(req, auth->aes);
> > +
> > +   skcipher_request_set_tfm(req, auth->aes);
> > +   skcipher_request_set_callback(req,
> > CRYPTO_TFM_REQ_MAY_SLEEP,
> > + NULL, NULL);
> > +
> 
> Your crypto_alloc_skcipher() call [further down] does not mask out
> CRYPTO_ALG_ASYNC, and so it permits asynchronous implementations to
> be selected. Your generic cfb template only permits synchronous
> implementations, since it wraps the cipher directly (which is always
> synchronous). However, we have support in the tree for some
> accelerators that implement cfb(aes), and those will return
> -EINPROGRESS when invoking crypto_skcipher_en/decrypt(req), which you
> are not set up to handle.
> 
> So the simple solution is to call 'crypto_alloc_skcipher("cfb(aes)",
> 0, CRYPTO_ALG_ASYNC)' below instead.
> 
> However, I would prefer it if we permit asynchronous skciphers here.
> The reason is that, on a given platform, the accelerator may be the
> only truly time invariant AES implementation that is available, and
> since we are dealing with the TPM, a bit of paranoia does not hurt.
> It also makes it easier to implement it as a [time invariant] SIMD
> based asynchronous skcipher, which are simpler than synchronous ones
> since they don't require a non-SIMD fallback path for calls from
> contexts where the SIMD unit may not be used.
> 
> I have already implemented cfb(aes) for arm64/NEON. I will post the
> patch after the merge window closes.
> 
> > +   /* need key and IV */
> > +   KDFa(auth->session_key, SHA256_DIGEST_SIZE
> > ++ auth->passphraselen, "CFB", auth->our_nonce,
> > +auth->tpm_nonce, AES_KEYBYTES +
> > AES_BLOCK_SIZE,
> > +auth->scratch);
> > +   crypto_skcipher_setkey(auth->aes, auth->scratch,
> > AES_KEYBYTES);
> > +   len = tpm_get_inc_u16();
> > +   sg_init_one(sg, p, len);
> > +   skcipher_request_set_crypt(req, sg, sg, len,
> > +  auth->scratch +
> > AES_KEYBYTES);
> > +   crypto_skcipher_encrypt(req);
> 
> So please consider replacing this with something like.
> 
> DECLARE_CRYPTO_WAIT(wait); [further up]
> skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
>   crypto_req_done, );
> crypto_wait_req(crypto_skcipher_encrypt(req), );

Sure, I can do this ... the crypto skcipher handling was also cut and
paste, but I forget where from this time.  So what I think you're
asking for is below as the incremental diff?  I've tested this out and
it all works fine in my session testing environment (and on my real
laptop) ... although since I'm fully sync, I won't really have tested
the -EINPROGRESS do the wait case.

James

---

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 422c3ec64f8c..bbd3e8580954 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -165,7 +165,6 @@ static void hmac_init(struct shash_desc *desc, u8 *key, int 
keylen)
int i;
 
desc->tfm = sha256_hash;
-   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
crypto_shash_init(desc);
for (i = 0; i < sizeof(pad); i++) {
if (i < keylen)
@@ -244,7 +243,6 @@ static void KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, 
u8 *pt_v,
__be32 c = cpu_to_be32(1);
 
desc->tfm = sha256_hash;
-   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
 
crypto_shash_init(desc);
/* counter (BE) */
@@ -445,7 +443,6 @@ void tpm_buf_fill_hmac_session(struct tpm_buf *buf, struct 
tpm2_auth *auth)
auth->ordinal = head->ordinal;
 

[PATCH v4 7/7] tpm2-sessions: NOT FOR COMMITTING add sessions testing

2018-10-22 Thread James Bottomley
This runs through a preset sequence using sessions to demonstrate that
the session handling code functions.  It does both HMAC, encryption
and decryption by testing an encrypted sealing operation with
authority and proving that the same sealed data comes back again via
an HMAC and response encryption.  It also does policy unsealing which
mimics the more complex of the trusted key scenarios.

Signed-off-by: James Bottomley 

---
v3: add policy unseal testing with two sessions
---
 drivers/char/tpm/Makefile |   1 +
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm.h|   1 +
 drivers/char/tpm/tpm2-cmd.c   |   2 +
 drivers/char/tpm/tpm2-sessions-test.c | 360 ++
 5 files changed, 365 insertions(+)
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 1b5f6ccbb86d..b3f5e95679f9 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
 eventlog/tpm2.o tpm2-space.o tpm-buf.o tpm2-sessions.o
+obj-m +=  tpm2-sessions-test.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 46caadca916a..ef7ff32bf827 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -142,6 +142,7 @@ struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip)
return NULL;
return chip;
 }
+EXPORT_SYMBOL(tpm_find_get_ops);
 
 /**
  * tpm_dev_release() - free chip memory and the device number
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d39065d9995d..8cb8b32a8418 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -163,6 +163,7 @@ enum tpm2_command_codes {
TPM2_CC_CONTEXT_LOAD= 0x0161,
TPM2_CC_CONTEXT_SAVE= 0x0162,
TPM2_CC_FLUSH_CONTEXT   = 0x0165,
+   TPM2_CC_POLICY_COMMAND_CODE = 0x16c,
TPM2_CC_READ_PUBLIC = 0x0173,
TPM2_CC_START_AUTH_SESS = 0x0176,
TPM2_CC_GET_CAPABILITY  = 0x017A,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a8655cd535d1..7afe0f50dcce 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -379,6 +379,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 
handle,
 
tpm_buf_destroy();
 }
+EXPORT_SYMBOL_GPL(tpm2_flush_context_cmd);
 
 /**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
@@ -409,6 +410,7 @@ void tpm2_buf_append_auth(struct tpm_buf *buf, u32 
session_handle,
if (hmac && hmac_len)
tpm_buf_append(buf, hmac, hmac_len);
 }
+EXPORT_SYMBOL_GPL(tpm2_buf_append_auth);
 
 /**
  * tpm2_seal_trusted() - seal the payload of a trusted key
diff --git a/drivers/char/tpm/tpm2-sessions-test.c 
b/drivers/char/tpm/tpm2-sessions-test.c
new file mode 100644
index ..7b4a01ad3745
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions-test.c
@@ -0,0 +1,360 @@
+/* run a set of tests of the sessions code */
+#include "tpm.h"
+#include "tpm2-sessions.h"
+
+#include 
+
+#include 
+
+/* simple policy: command code must be TPM2_CC_UNSEAL */
+static u8 policy[] = {
+   0xe6, 0x13, 0x13, 0x70, 0x76, 0x52, 0x4b, 0xde,
+   0x48, 0x75, 0x33, 0x86, 0x58, 0x84, 0xe9, 0x73,
+   0x2e, 0xbe, 0xe3, 0xaa, 0xcb, 0x09, 0x5d, 0x94,
+   0xa6, 0xde, 0x49, 0x2e, 0xc0, 0x6c, 0x46, 0xfa,
+};
+
+static u32 get_policy(struct tpm_chip *chip)
+{
+   struct tpm_buf buf;
+   u8 nonce[SHA256_DIGEST_SIZE];
+   u32 h;
+   int rc;
+
+   rc = tpm_buf_init(, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
+   if (rc)
+   return 0;
+
+   /* salt key */
+   tpm_buf_append_u32(, TPM2_RH_NULL);
+   /* bind key */
+   tpm_buf_append_u32(, TPM2_RH_NULL);
+   /* zero nonce */
+   memset(nonce, 0, sizeof(nonce));
+   tpm_buf_append_u16(, sizeof(nonce));
+   tpm_buf_append(, nonce, sizeof(nonce));
+   /* encrypted salt (empty) */
+   tpm_buf_append_u16(, 0);
+   /* session type (HMAC, audit or policy) */
+   tpm_buf_append_u8(, TPM2_SE_POLICY);
+   /* symmetric encryption parameters */
+   /* symmetric algorithm */
+   tpm_buf_append_u16(, TPM2_ALG_NULL);
+
+   /* hash algorithm for session */
+   tpm_buf_append_u16(, TPM2_ALG_SHA256);
+
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "start policy session");
+
+   h = get_unaligned_be32([TPM_HEADER_SIZE]);
+
+   tpm_buf_reset(, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_POLICY_COMMAND_CODE);
+   tpm_buf_ap

[PATCH v4 6/7] tpm: add the null key name as a tpm2 sysfs variable

2018-10-22 Thread James Bottomley
This is the last component of encrypted tpm2 session handling that
allows us to verify from userspace that the key derived from the NULL
seed genuinely belongs to the TPM and has not been spoofed.

The procedure for doing this involves creating an attestation identity
key (which requires verification of the TPM EK certificate) and then
using that AIK to sign a certification of the Elliptic Curve key over
the NULL seed.  Userspace must create this EC Key using the parameters
prescribed in TCG TPM v2.0 Provisioning Guidance for the SRK ECC; if
this is done correctly the names will match and the TPM can then run a
TPM2_Certify operation on this derived primary key using the newly
created AIK.

Signed-off-by: James Bottomley 
---
 drivers/char/tpm/tpm-sysfs.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 83a77a445538..7e901d9e893e 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -284,6 +284,19 @@ static ssize_t timeouts_show(struct device *dev, struct 
device_attribute *attr,
 }
 static DEVICE_ATTR_RO(timeouts);
 
+static ssize_t null_name_show(struct device *dev, struct device_attribute 
*attr,
+ char *buf)
+{
+   struct tpm_chip *chip = to_tpm_chip(dev);
+   int size = TPM2_NAME_SIZE;
+
+   bin2hex(buf, chip->tpmkeyname, size);
+   size *= 2;
+   buf[size++] = '\n';
+   return size;
+}
+static DEVICE_ATTR_RO(null_name);
+
 static struct attribute *tpm_dev_attrs[] = {
_attr_pubek.attr,
_attr_pcrs.attr,
@@ -298,17 +311,29 @@ static struct attribute *tpm_dev_attrs[] = {
NULL,
 };
 
+static struct attribute *tpm2_dev_attrs[] = {
+   _attr_null_name.attr,
+   NULL,
+};
+
 static const struct attribute_group tpm_dev_group = {
.attrs = tpm_dev_attrs,
 };
 
+static const struct attribute_group tpm2_dev_group = {
+   .attrs = tpm2_dev_attrs,
+};
+
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
/* XXX: If you wish to remove this restriction, you must first update
 * tpm_sysfs to explicitly lock chip->ops.
 */
-   if (chip->flags & TPM_CHIP_FLAG_TPM2)
+   if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+   WARN_ON(chip->groups_cnt != 0);
+   chip->groups[chip->groups_cnt++] = _dev_group;
return;
+   }
 
/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
 * is called before ops is null'd and the sysfs core synchronizes this
-- 
2.16.4



[PATCH v4 5/7] trusted keys: Add session encryption protection to the seal/unseal path

2018-10-22 Thread James Bottomley
If some entity is snooping the TPM bus, the can see the data going in
to be sealed and the data coming out as it is unsealed.  Add parameter
and response encryption to these cases to ensure that no secrets are
leaked even if the bus is snooped.

As part of doing this conversion it was discovered that policy
sessions can't work with HMAC protected authority because of missing
pieces (the tpm Nonce).  I've added code to work the same way as
before, which will result in potential authority exposure (while still
adding security for the command and the returned blob), and a fixme to
redo the API to get rid of this security hole.

Signed-off-by: James Bottomley 
---
 drivers/char/tpm/tpm2-cmd.c | 155 
 1 file changed, 98 insertions(+), 57 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 22f1c7bee173..a8655cd535d1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -425,7 +425,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
unsigned int blob_len;
struct tpm_buf buf;
+   struct tpm_buf t2b;
u32 hash;
+   struct tpm2_auth *auth;
int i;
int rc;
 
@@ -439,45 +441,56 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (i == ARRAY_SIZE(tpm2_hash_map))
return -EINVAL;
 
-   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+   rc = tpm2_start_auth_session(chip, );
if (rc)
return rc;
 
-   tpm_buf_append_u32(, options->keyhandle);
-   tpm2_buf_append_auth(, TPM2_RS_PW,
-NULL /* nonce */, 0,
-0 /* session_attributes */,
-options->keyauth /* hmac */,
-TPM_DIGEST_SIZE);
+   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+   if (rc) {
+   tpm2_end_auth_session(auth);
+   return rc;
+   }
+
+   rc = tpm_buf_init_2b();
+   if (rc) {
+   tpm_buf_destroy();
+   tpm2_end_auth_session(auth);
+   return rc;
+   }
 
+   tpm_buf_append_name(, auth, options->keyhandle, NULL);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_DECRYPT,
+   options->keyauth, TPM_DIGEST_SIZE);
/* sensitive */
-   tpm_buf_append_u16(, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+   tpm_buf_append_u16(, TPM_DIGEST_SIZE);
+   tpm_buf_append(, options->blobauth, TPM_DIGEST_SIZE);
+   tpm_buf_append_u16(, payload->key_len + 1);
+   tpm_buf_append(, payload->key, payload->key_len);
+   tpm_buf_append_u8(, payload->migratable);
 
-   tpm_buf_append_u16(, TPM_DIGEST_SIZE);
-   tpm_buf_append(, options->blobauth, TPM_DIGEST_SIZE);
-   tpm_buf_append_u16(, payload->key_len + 1);
-   tpm_buf_append(, payload->key, payload->key_len);
-   tpm_buf_append_u8(, payload->migratable);
+   tpm_buf_append_2b(, );
 
/* public */
-   tpm_buf_append_u16(, 14 + options->policydigest_len);
-   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
-   tpm_buf_append_u16(, hash);
+   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
+   tpm_buf_append_u16(, hash);
 
/* policy */
if (options->policydigest_len) {
-   tpm_buf_append_u32(, 0);
-   tpm_buf_append_u16(, options->policydigest_len);
-   tpm_buf_append(, options->policydigest,
+   tpm_buf_append_u32(, 0);
+   tpm_buf_append_u16(, options->policydigest_len);
+   tpm_buf_append(, options->policydigest,
   options->policydigest_len);
} else {
-   tpm_buf_append_u32(, TPM2_OA_USER_WITH_AUTH);
-   tpm_buf_append_u16(, 0);
+   tpm_buf_append_u32(, TPM2_OA_USER_WITH_AUTH);
+   tpm_buf_append_u16(, 0);
}
 
/* public parameters */
-   tpm_buf_append_u16(, TPM2_ALG_NULL);
-   tpm_buf_append_u16(, 0);
+   tpm_buf_append_u16(, TPM2_ALG_NULL);
+   /* unique (zero) */
+   tpm_buf_append_u16(, 0);
+
+   tpm_buf_append_2b(, );
 
/* outside info */
tpm_buf_append_u16(, 0);
@@ -490,8 +503,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
 
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
- "sealing data");
+   tpm_buf_fill_hmac_session(, auth);
+
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data,
+ PAGE_SIZE, 4, 0, "sealing data");
+   rc = tpm_buf_check_hmac_response(, auth, rc);
if (rc)
goto out;
 
@@ -509,6 +525,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
payload->blob_len = blob_len;
 
 out:
+   tpm_buf_destroy();
   

[PATCH v4 4/7] tpm2: add session encryption protection to tpm2_get_random()

2018-10-22 Thread James Bottomley
If some entity is snooping the TPM bus, they can see the random
numbers we're extracting from the TPM and do prediction attacks
against their consumers.  Foil this attack by using response
encryption to prevent the attacker from seeing the random sequence.

Signed-off-by: James Bottomley 

---

v3: add error handling to sessions and redo to be outside loop
---
 drivers/char/tpm/tpm2-cmd.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 332b34b347f1..22f1c7bee173 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -266,7 +266,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 
count,
return rc;
 }
 
-
 struct tpm2_get_random_out {
__be16 size;
u8 buffer[TPM_MAX_RNG_DATA];
@@ -293,21 +292,32 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, 
size_t max)
int total = 0;
int retries = 5;
u8 *dest_ptr = dest;
+   struct tpm2_auth *auth;
 
if (!num_bytes || max > TPM_MAX_RNG_DATA)
return -EINVAL;
 
-   err = tpm_buf_init(, 0, 0);
+   err = tpm2_start_auth_session(chip, );
if (err)
return err;
 
+   err = tpm_buf_init(, 0, 0);
+   if (err) {
+   tpm2_end_auth_session(auth);
+   return err;
+   }
+
do {
-   tpm_buf_reset(, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
+   tpm_buf_reset(, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_ENCRYPT
+   | TPM2_SA_CONTINUE_SESSION,
+   NULL, 0);
tpm_buf_append_u16(, num_bytes);
-   err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-  offsetof(struct tpm2_get_random_out,
-   buffer),
+   tpm_buf_fill_hmac_session(, auth);
+   err = tpm_transmit_cmd(chip, >kernel_space, buf.data,
+  PAGE_SIZE, TPM_HEADER_SIZE + 2,
   0, "attempting get random");
+   err = tpm_buf_check_hmac_response(, auth, err);
if (err)
goto out;
 
@@ -327,6 +337,8 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t 
max)
} while (retries-- && total < max);
 
tpm_buf_destroy();
+   tpm2_end_auth_session(auth);
+
return total ? total : -EIO;
 out:
tpm_buf_destroy();
-- 
2.16.4



[PATCH v4 3/7] tpm2: add hmac checks to tpm2_pcr_extend()

2018-10-22 Thread James Bottomley
We use tpm2_pcr_extend() in trusted keys to extend a PCR to prevent a
key from being re-loaded until the next reboot.  To use this
functionality securely, that extend must be protected by a session
hmac.

Signed-off-by: James Bottomley 

---

v3: add error handling to sessions
---
 drivers/char/tpm/tpm2-cmd.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a17e5c573c4e..332b34b347f1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -209,13 +209,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
return rc;
 }
 
-struct tpm2_null_auth_area {
-   __be32  handle;
-   __be16  nonce_size;
-   u8  attributes;
-   __be16  auth_size;
-} __packed;
-
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
@@ -230,7 +223,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 
count,
struct tpm2_digest *digests)
 {
struct tpm_buf buf;
-   struct tpm2_null_auth_area auth_area;
+   struct tpm2_auth *auth;
int rc;
int i;
int j;
@@ -238,20 +231,19 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
if (count > ARRAY_SIZE(chip->active_banks))
return -EINVAL;
 
-   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+   rc = tpm2_start_auth_session(chip, );
if (rc)
return rc;
 
-   tpm_buf_append_u32(, pcr_idx);
+   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+   if (rc) {
+   tpm2_end_auth_session(auth);
+   return rc;
+   }
 
-   auth_area.handle = cpu_to_be32(TPM2_RS_PW);
-   auth_area.nonce_size = 0;
-   auth_area.attributes = 0;
-   auth_area.auth_size = 0;
+   tpm_buf_append_name(, auth, pcr_idx, NULL);
+   tpm_buf_append_hmac_session(, auth, 0, NULL, 0);
 
-   tpm_buf_append_u32(, sizeof(struct tpm2_null_auth_area));
-   tpm_buf_append(, (const unsigned char *)_area,
-  sizeof(auth_area));
tpm_buf_append_u32(, count);
 
for (i = 0; i < count; i++) {
@@ -264,9 +256,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
   hash_digest_size[tpm2_hash_map[j].crypto_id]);
}
}
-
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
- "attempting extend a PCR value");
+   tpm_buf_fill_hmac_session(, auth);
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "attempting extend a PCR value");
+   rc = tpm_buf_check_hmac_response(, auth, rc);
 
tpm_buf_destroy();
 
-- 
2.16.4



[PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-10-22 Thread James Bottomley
This code adds true session based HMAC authentication plus parameter
decryption and response encryption using AES.

The basic design of this code is to segregate all the nasty crypto,
hash and hmac code into tpm2-sessions.c and export a usable API.

The API first of all starts off by gaining a session with

tpm2_start_auth_session()

Which initiates a session with the TPM and allocates an opaque
tpm2_auth structure to handle the session parameters.  Then the use is
simply:

* tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
  handles

* tpm_buf_append_hmac_session() where tpm2_append_auth() would go

* tpm_buf_fill_hmac_session() called after the entire command buffer
  is finished but before tpm_transmit_cmd() is called which computes
  the correct HMAC and places it in the command at the correct
  location.

Finally, after tpm_transmit_cmd() is called,
tpm_buf_check_hmac_response() is called to check that the returned
HMAC matched and collect the new state for the next use of the
session, if any.

The features of the session is controlled by the session attributes
set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
not specified, the session will be flushed and the tpm2_auth structure
freed in tpm_buf_check_hmac_response(); otherwise the session may be
used again.  Parameter encryption is specified by or'ing the flag
TPM2_SA_DECRYPT and response encryption by or'ing the flag
TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
respectively.

To get all of this to work securely, the Kernel now needs a primary
key to encrypt the session salt to, so we derive an EC key from the
NULL seed and store it in the tpm_chip structure.  We also make sure
that this seed remains for the kernel by using a kernel space to take
it out of the TPM when userspace wants to use it.

Signed-off-by: James Bottomley 

---

v2: Added docbook and improved response check API
v3: Add readpublic, fix hmac length, add API for close on error
allow for the hmac session not being first in the sessions
v4: Make NULL seed template exactly match the SRK ECC template.
Also check the NULL primary key name is what getpublic returns
to prevent spoofing.  Also parametrise the name size for reuse
---
 drivers/char/tpm/Kconfig |3 +
 drivers/char/tpm/Makefile|2 +-
 drivers/char/tpm/tpm.h   |   32 +
 drivers/char/tpm/tpm2-cmd.c  |   34 +-
 drivers/char/tpm/tpm2-sessions.c | 1188 ++
 drivers/char/tpm/tpm2-sessions.h |   57 ++
 6 files changed, 1300 insertions(+), 16 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0aee88df98d1..8c714d8550c4 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -8,6 +8,9 @@ menuconfig TCG_TPM
select SECURITYFS
select CRYPTO
select CRYPTO_HASH_INFO
+   select CRYPTO_ECDH
+   select CRYPTO_AES
+   select CRYPTO_CFB
---help---
  If you have a TPM security chip in your system, which
  implements the Trusted Computing Group's specification,
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 65d165cce530..1b5f6ccbb86d 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
-eventlog/tpm2.o tpm2-space.o tpm-buf.o
+eventlog/tpm2.o tpm2-space.o tpm-buf.o tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d73701e36eba..d39065d9995d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -42,6 +42,15 @@
 #include 
 #endif
 
+/* fixed define for the curve we use which is NIST_P256 */
+#define EC_PT_SZ   32
+
+/*
+ * fixed define for the size of a name.  This is actually HASHALG size
+ * plus 2, so 32 for SHA256
+ */
+#define TPM2_NAME_SIZE 34
+
 enum tpm_const {
TPM_MINOR = 224,/* officially assigned */
TPM_BUFSIZE = 4096,
@@ -103,6 +112,7 @@ enum tpm2_timeouts {
 enum tpm2_structures {
TPM2_ST_NO_SESSIONS = 0x8001,
TPM2_ST_SESSIONS= 0x8002,
+   TPM2_ST_CREATION= 0x8021,
 };
 
 /* Indicates from what layer of the software stack the error comes from */
@@ -125,12 +135,20 @@ enum tpm2_return_codes {
 enum tpm2_algorithms {
TPM2_ALG_ERROR  = 0x,
TPM2_ALG_SHA1   = 0x0004,
+   TPM2_ALG_AES= 0x0006,
TPM2_ALG_KEYEDHASH  = 0x0008,
TPM2_ALG_SHA256 = 0x000B,
TPM2_ALG_SHA384

[PATCH v4 1/7] tpm-buf: create new functions for handling TPM buffers

2018-10-22 Thread James Bottomley
This separates out the old tpm_buf_... handling functions from static
inlines into tpm.h and makes them their own tpm-buf.c file.  It also
adds handling for tpm2b structures and also incremental pointer
advancing parsers.

Signed-off-by: James Bottomley 

---

v2: added this patch to separate out the API changes
v3: added tpm_buf_reset_cmd()
v4: renamed tpm_buf_reset_cmd() to tpm_buf_reset()
---
 drivers/char/tpm/Makefile  |   2 +-
 drivers/char/tpm/tpm-buf.c | 191 +
 drivers/char/tpm/tpm.h |  96 ---
 3 files changed, 208 insertions(+), 81 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 3655258bee73..65d165cce530 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o eventlog/common.o eventlog/tpm1.o \
-eventlog/tpm2.o tpm2-space.o
+eventlog/tpm2.o tpm2-space.o tpm-buf.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o
 tpm-$(CONFIG_EFI) += eventlog/efi.o
 tpm-$(CONFIG_OF) += eventlog/of.o
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
new file mode 100644
index ..faa22bb09d99
--- /dev/null
+++ b/drivers/char/tpm/tpm-buf.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Handing for tpm2b structures to facilitate the building of commands
+ */
+
+#include "tpm.h"
+
+#include 
+
+#include 
+
+static int __tpm_buf_init(struct tpm_buf *buf)
+{
+   buf->data_page = alloc_page(GFP_HIGHUSER);
+   if (!buf->data_page)
+   return -ENOMEM;
+
+   buf->flags = 0;
+   buf->data = kmap(buf->data_page);
+
+   return 0;
+}
+
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+   struct tpm_input_header *head;
+
+   head = (struct tpm_input_header *) buf->data;
+
+   head->tag = cpu_to_be16(tag);
+   head->length = cpu_to_be32(sizeof(*head));
+   head->ordinal = cpu_to_be32(ordinal);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_reset);
+
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+   int rc;
+
+   rc = __tpm_buf_init(buf);
+   if (rc)
+   return rc;
+
+   tpm_buf_reset(buf, tag, ordinal);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
+
+int tpm_buf_init_2b(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head;
+   int rc;
+
+   rc = __tpm_buf_init(buf);
+   if (rc)
+   return rc;
+
+   head = (struct tpm_input_header *) buf->data;
+
+   head->length = cpu_to_be32(sizeof(*head));
+
+   buf->flags = TPM_BUF_2B;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init_2b);
+
+void tpm_buf_destroy(struct tpm_buf *buf)
+{
+   kunmap(buf->data_page);
+   __free_page(buf->data_page);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+
+static void *tpm_buf_data(struct tpm_buf *buf)
+{
+   if (buf->flags & TPM_BUF_2B)
+   return buf->data + TPM_HEADER_SIZE;
+   return buf->data;
+}
+
+u32 tpm_buf_length(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+   u32 len;
+
+   len = be32_to_cpu(head->length);
+   if (buf->flags & TPM_BUF_2B)
+   len -= sizeof(*head);
+   return len;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_length);
+
+u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+
+   return be16_to_cpu(head->tag);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_tag);
+
+void tpm_buf_append(struct tpm_buf *buf,
+   const unsigned char *new_data,
+   unsigned int new_len)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+   u32 len = be32_to_cpu(head->length);
+
+   /* Return silently if overflow has already happened. */
+   if (buf->flags & TPM_BUF_OVERFLOW)
+   return;
+
+   if ((len + new_len) > PAGE_SIZE) {
+   WARN(1, "tpm_buf: overflow\n");
+   buf->flags |= TPM_BUF_OVERFLOW;
+   return;
+   }
+
+   memcpy(>data[len], new_data, new_len);
+   head->length = cpu_to_be32(len + new_len);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append);
+
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+   tpm_buf_append(buf, , 1);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u8);
+
+void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+   __be16 value2 = cpu_to_be16(value);
+
+   tpm_buf_append(buf, (u8 *) , 2);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u16);
+
+void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+   __be32 value2 = cpu_to_be32(value);
+
+   tpm_buf_a

[PATCH v4 0/7] add integrity and security to TPM2 transactions

2018-10-22 Thread James Bottomley
By now, everybody knows we have a problem with the TPM2_RS_PW easy
button on TPM2 in that transactions on the TPM bus can be intercepted
and altered.  The way to fix this is to use real sessions for HMAC
capabilities to ensure integrity and to use parameter and response
encryption to ensure confidentiality of the data flowing over the TPM
bus.

This patch series is about adding a simple API which can ensure the
above properties as a layered addition to the existing TPM handling
code.  This series now includes protections for PCR extend, getting
random numbers from the TPM and data sealing and unsealing.  It
therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
encryption protection to sensitive data flowing into and out of the
TPM.

In the third version I added data sealing and unsealing protection,
apart from one API based problem which means that the way trusted keys
were protected it's not currently possible to HMAC protect an authority
that comes with a policy, so the API will have to be extended to fix
that case

In this fourth version, I tidy up some of the code and add more
security features, the most notable is that we now calculate the NULL
seed name and compare our calculation to the value returned in
TPM2_ReadPublic, which means we now can't be spoofed.  This version
also gives a sysfs variable for the null seed which userspace can use
to run a key certification operation to prove that the TPM was always
secure when communicating with the kernel.

I've verified this using the test suite in the last patch on a VM
connected to a tpm2 emulator.  I also instrumented the emulator to make
sure the sensitive data was properly encrypted.

James

---


James Bottomley (7):
  tpm-buf: create new functions for handling TPM buffers
  tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
  tpm2: add hmac checks to tpm2_pcr_extend()
  tpm2: add session encryption protection to tpm2_get_random()
  trusted keys: Add session encryption protection to the seal/unseal
path
  tpm: add the null key name as a tpm2 sysfs variable
  tpm2-sessions: NOT FOR COMMITTING add sessions testing

 drivers/char/tpm/Kconfig  |3 +
 drivers/char/tpm/Makefile |3 +-
 drivers/char/tpm/tpm-buf.c|  191 ++
 drivers/char/tpm/tpm-chip.c   |1 +
 drivers/char/tpm/tpm-sysfs.c  |   27 +-
 drivers/char/tpm/tpm.h|  129 ++--
 drivers/char/tpm/tpm2-cmd.c   |  248 ---
 drivers/char/tpm/tpm2-sessions-test.c |  360 ++
 drivers/char/tpm/tpm2-sessions.c  | 1188 +
 drivers/char/tpm/tpm2-sessions.h  |   57 ++
 10 files changed, 2027 insertions(+), 180 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

-- 
2.16.4



Re: [PATCH 1/2] crypto: fix cfb mode decryption

2018-10-21 Thread James Bottomley
On October 21, 2018 9:58:04 AM GMT, Ard Biesheuvel  
wrote:
>On 21 October 2018 at 10:07, James Bottomley
> wrote:
>> On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
>>> (+ James)
>>
>> Thanks!
>>
>>> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
>>>  wrote:
>>> > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated
>keystream
>>> > with
>>> > IV, rather than with data stream, resulting in incorrect
>>> > decryption.
>>> > Test vectors will be added in the next patch.
>>> >
>>> > Signed-off-by: Dmitry Eremin-Solenikov 
>>> > Cc: sta...@vger.kernel.org
>>> > ---
>>> >  crypto/cfb.c | 2 +-
>>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>>> >
>>> > diff --git a/crypto/cfb.c b/crypto/cfb.c
>>> > index a0d68c09e1b9..fd4e8500e121 100644
>>> > --- a/crypto/cfb.c
>>> > +++ b/crypto/cfb.c
>>> > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
>>> > skcipher_walk *walk,
>>> >
>>> > do {
>>> > crypto_cfb_encrypt_one(tfm, iv, dst);
>>> > -   crypto_xor(dst, iv, bsize);
>>> > +   crypto_xor(dst, src, bsize);
>>
>> This does look right.  I think the reason the TPM code works is that
>it
>> always does encrypt/decrypt in-place, which is a separate piece of
>the
>> code which appears to be correct.
>>
>
>Yeah I figured that.
>
>So where is the TPM code that actually uses this code?

It was posted to the integrity list a while ago.  I'm planning a repost  
shortly.

James


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


Re: [PATCH 1/2] crypto: fix cfb mode decryption

2018-10-21 Thread James Bottomley
On Sun, 2018-10-21 at 09:05 +0200, Ard Biesheuvel wrote:
> (+ James)

Thanks!

> On 20 October 2018 at 01:01, Dmitry Eremin-Solenikov
>  wrote:
> > crypto_cfb_decrypt_segment() incorrectly XOR'ed generated keystream
> > with
> > IV, rather than with data stream, resulting in incorrect
> > decryption.
> > Test vectors will be added in the next patch.
> > 
> > Signed-off-by: Dmitry Eremin-Solenikov 
> > Cc: sta...@vger.kernel.org
> > ---
> >  crypto/cfb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/crypto/cfb.c b/crypto/cfb.c
> > index a0d68c09e1b9..fd4e8500e121 100644
> > --- a/crypto/cfb.c
> > +++ b/crypto/cfb.c
> > @@ -144,7 +144,7 @@ static int crypto_cfb_decrypt_segment(struct
> > skcipher_walk *walk,
> > 
> > do {
> > crypto_cfb_encrypt_one(tfm, iv, dst);
> > -   crypto_xor(dst, iv, bsize);
> > +   crypto_xor(dst, src, bsize);

This does look right.  I think the reason the TPM code works is that it
always does encrypt/decrypt in-place, which is a separate piece of the
code which appears to be correct.

James



Re: CAAM and IMA/EVM : caam_rsa_enc: DECO: desc idx 7: Protocol Size Error

2018-04-10 Thread James Bottomley
On Tue, 2018-04-10 at 23:01 +0100, Martin Townsend wrote:
> Using openssl to get the signature in my x509 cert
> 
>    Signature Algorithm: sha256WithRSAEncryption
>  68:82:cc:5d:f9:ee:fb:1a:77:72:a6:a9:c6:4c:cc:d7:f6:2a:
>  17:a5:db:bf:5a:2b:8d:39:60:dc:a0:93:39:45:0f:bc:a7:e8:
>  7f:6c:06:84:2d:f3:c1:94:0a:60:56:1c:50:78:dc:34:d1:87:
> 
> So there's an extra 0x00 and the signature is 257 bytes so I guess
> this is upsetting CAAM, just need to work out where it's coming from,
> or whether it's valid and CAAM should be handling it.

A signature is just a bignum so leading zeros are permitted because
it's the same numeric value; however, there are normalization
procedures that require stripping the leading zeros, say before doing a
hash or other operation which would be affected by them.

CAAM should definitely handle it on the "be liberal in what you accept"
 principle.  The kernel should probably remove the leading zeros on the
"be conservative in what you do" part of the same principle. 

>   I notice that in my stack trace I have pkcs1pad_verify which
> suggests some sort of padding?

Yes, RSA has various forms of padding because the information being
encrypted is usually much smaller than the encryption unit; PKCS1 is
the most common (although its now deprecated in favour of OAEP because
of all the padding oracle problems).

James



Re: [tpmdd-devel] in-kernel user of ecdsa

2018-03-12 Thread James Bottomley
On Mon, 2018-03-12 at 20:56 +0100, Stephan Mueller wrote:
> Am Montag, 12. März 2018, 19:09:18 CET schrieb James Bottomley:
> 
> Hi James,
> 
> > 
> > On Mon, 2018-03-12 at 19:07 +0200, Tudor Ambarus wrote:
> > > 
> > > Hi,
> > > 
> > > Would you consider using ECDSA in the kernel module signing
> > > facility? When compared with RSA, ECDSA has shorter keys, the key
> > > generation process is faster, the sign operation is faster, but
> > > the verify operation is slower than with RSA.
> > 
> > You missed the keyrings list, which is where the module signing
> > utility is discussed.
> > 
> > First question is, have you actually tried?  It looks like sign-
> > file doesn't do anything RSA specific so if you give it an EC X.509
> > certificate it will produce an ECDSA signature.
> > 
> > I think our kernel internal x509 parsers don't have the EC OIDs, so
> > signature verification will fail; but, especially since we have the
> > rest of the EC machinery in the crypto subsystem, that looks to be
> > simply fixable.
> 
> ECDSA is not implemented currently in the kernel crypto API.

an ECDSA signature is produced as a ECDH operation using the DSA
algorithm instead of KDFe, so it's trivial with what we have; signature
verification involves a separate point addition but we have all the
primitives for this in crypto/ecc.c so adding it isn't really
difficult, is it?

James



Re: [tpmdd-devel] in-kernel user of ecdsa

2018-03-12 Thread James Bottomley
On Mon, 2018-03-12 at 19:07 +0200, Tudor Ambarus wrote:
> Hi,
> 
> Would you consider using ECDSA in the kernel module signing facility?
> When compared with RSA, ECDSA has shorter keys, the key generation
> process is faster, the sign operation is faster, but the verify
> operation is slower than with RSA.

You missed the keyrings list, which is where the module signing utility
is discussed.

First question is, have you actually tried?  It looks like sign-file
doesn't do anything RSA specific so if you give it an EC X.509
certificate it will produce an ECDSA signature.

I think our kernel internal x509 parsers don't have the EC OIDs, so
signature verification will fail; but, especially since we have the
rest of the EC machinery in the crypto subsystem, that looks to be
simply fixable.

James



Re: [PATCH v3 1/6] tpm-buf: create new functions for handling TPM buffers

2018-03-12 Thread James Bottomley
On Mon, 2018-03-12 at 09:00 -0700, J Freyensee wrote:
> > 
> > +int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
> > +{
> > +   int rc;
> > +
> > +   rc = __tpm_buf_init(buf);
> 
> 
> Assuming that functions like tpm_buf_init() are the top-level API
> being defined in this patch, shouldn't it check if buf is valid
> before passing into the internal functions like __tpm_buf_init(buf)
> (maybe WARN()/BUG_ON()?).  Or does __tpm_buf_init(buf) do this check?

These are kernel internal APIs designed for on stack struct tpm_buf
usage, so I can't think of a viable threat model that would require
this type of checking ... do you have one?

James



Re: [PATCH v3 0/6] add integrity and security to TPM2 transactions

2018-03-12 Thread James Bottomley
On Mon, 2018-03-12 at 12:58 +0200, Jarkko Sakkinen wrote:
> On Sat, 2018-03-10 at 14:13 -0800, James Bottomley wrote:
> > 
> > By now, everybody knows we have a problem with the TPM2_RS_PW easy
> > button on TPM2 in that transactions on the TPM bus can be
> > intercepted
> > and altered.  The way to fix this is to use real sessions for HMAC
> > capabilities to ensure integrity and to use parameter and response
> > encryption to ensure confidentiality of the data flowing over the
> > TPM
> > bus.
> > 
> > This patch series is about adding a simple API which can ensure the
> > above properties as a layered addition to the existing TPM handling
> > code.  This series now includes protections for PCR extend, getting
> > random numbers from the TPM and data sealing and unsealing.  It
> > therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
> > encryption protection to sensitive data flowing into and out of the
> > TPM.
> > 
> > This series is also dependent on additions to the crypto subsystem
> > to
> > fix problems in the elliptic curve key handling and add the Cipher
> > FeedBack encryption scheme:
> > 
> > https://marc.info/?l=linux-crypto-vger=151994371015475
> > 
> > In the third version I've added data sealing and unsealing
> > protection,
> > apart from one API based problem which means that the way trusted
> > keys
> > were protected it's not currently possible to HMAC protect an
> > authority
> > that comes with a policy, so the API will have to be extended to
> > fix
> > that case
> > 
> > I've verified this using the test suite in the last patch on a VM
> > connected to a tpm2 emulator.  I also instrumented the emulator to
> > make
> > sure the sensitive data was properly encrypted.
> > 
> > James
> 
> 1. Can I ignore v2 and just review/test this version? I haven't even
>    peeked into v2 yet.

Yes, v3 is a more complete version of v2 with a couple of sessions API
additions.

I think the way I'm going to fix the trusted key policy problem is to
move it back into the kernel for the simple PCR lock policy (which will
make changing from 1.2 to 2.0 seamless because the external Key API
will then become the same) so the kernel gets the missing TPM nonce and
can then do TPM2_PolicyAuthValue.

User generated policy sessions for trusted keys are very flexible but
also a hugely bad idea for consumers because it's so different from the
way 1.2 works and it means now the user has to exercise a TPM API to
produce the policy sessions.

Longer term, I think having a particular trusted key represent a policy
session which can then be attached to a different trusted key
representing the blob is the best idea because we can expose the policy
build up via the trusted key API and keep all the TPM nastiness inside
the kernel.

> 2. Do you know in which kernel version will the crypto additions
> land?

They're here:

https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/log/

So I'd guess next merge window.  You can do what we do in SCSI and
create a "postmerge" branch based on the cryptodev one (we often have
SCSI stuff with block tree precursors).  The way I run it is that I
don't send the merge window pull request until I see the merge-base
against Linus master move to the base of the patches (meaning all the
precursors are upstream).

> /Jarkko
> 



[PATCH v3 6/6] tpm2-sessions: NOT FOR COMMITTING add sessions testing

2018-03-10 Thread James Bottomley
This runs through a preset sequence using sessions to demonstrate that
the session handling code functions.  It does both HMAC, encryption
and decryption by testing an encrypted sealing operation with
authority and proving that the same sealed data comes back again via
an HMAC and response encryption.  It also does policy unsealing which
mimics the more complex of the trusted key scenarios.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---
v3: add policy unseal testing with two sessions
---
 drivers/char/tpm/Makefile |   1 +
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm.h|   1 +
 drivers/char/tpm/tpm2-cmd.c   |   2 +
 drivers/char/tpm/tpm2-sessions-test.c | 359 ++
 5 files changed, 364 insertions(+)
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index b83737ccaa81..1ac7a4046630 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
  tpm2-space.o tpm-buf.o tpm2-sessions.o
+obj-m +=  tpm2-sessions-test.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 0a62c19937b6..ca174ee1e670 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -118,6 +118,7 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
 
return res;
 }
+EXPORT_SYMBOL(tpm_chip_find_get);
 
 /**
  * tpm_dev_release() - free chip memory and the device number
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index b1eee56cbbb5..8a652d36939d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -146,6 +146,7 @@ enum tpm2_command_codes {
TPM2_CC_CONTEXT_LOAD= 0x0161,
TPM2_CC_CONTEXT_SAVE= 0x0162,
TPM2_CC_FLUSH_CONTEXT   = 0x0165,
+   TPM2_CC_POLICY_COMMAND_CODE = 0x16c,
TPM2_CC_READ_PUBLIC = 0x0173,
TPM2_CC_START_AUTH_SESS = 0x0176,
TPM2_CC_GET_CAPABILITY  = 0x017A,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 8b164b7347de..3f47d8b3d361 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -418,6 +418,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 
handle,
 
tpm_buf_destroy();
 }
+EXPORT_SYMBOL_GPL(tpm2_flush_context_cmd);
 
 /**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
@@ -448,6 +449,7 @@ void tpm2_buf_append_auth(struct tpm_buf *buf, u32 
session_handle,
if (hmac && hmac_len)
tpm_buf_append(buf, hmac, hmac_len);
 }
+EXPORT_SYMBOL_GPL(tpm2_buf_append_auth);
 
 /**
  * tpm2_seal_trusted() - seal the payload of a trusted key
diff --git a/drivers/char/tpm/tpm2-sessions-test.c 
b/drivers/char/tpm/tpm2-sessions-test.c
new file mode 100644
index ..4559e1a5f4d8
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions-test.c
@@ -0,0 +1,359 @@
+/* run a set of tests of the sessions code */
+#include "tpm.h"
+#include "tpm2-sessions.h"
+
+#include 
+
+#include 
+
+/* simple policy: command code must be TPM2_CC_UNSEAL */
+static u8 policy[] = {
+   0xe6, 0x13, 0x13, 0x70, 0x76, 0x52, 0x4b, 0xde,
+   0x48, 0x75, 0x33, 0x86, 0x58, 0x84, 0xe9, 0x73,
+   0x2e, 0xbe, 0xe3, 0xaa, 0xcb, 0x09, 0x5d, 0x94,
+   0xa6, 0xde, 0x49, 0x2e, 0xc0, 0x6c, 0x46, 0xfa,
+};
+
+static u32 get_policy(struct tpm_chip *chip)
+{
+   struct tpm_buf buf;
+   u8 nonce[SHA256_DIGEST_SIZE];
+   u32 h;
+   int rc;
+
+   rc = tpm_buf_init(, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
+   if (rc)
+   return 0;
+
+   /* salt key */
+   tpm_buf_append_u32(, TPM2_RH_NULL);
+   /* bind key */
+   tpm_buf_append_u32(, TPM2_RH_NULL);
+   /* zero nonce */
+   memset(nonce, 0, sizeof(nonce));
+   tpm_buf_append_u16(, sizeof(nonce));
+   tpm_buf_append(, nonce, sizeof(nonce));
+   /* encrypted salt (empty) */
+   tpm_buf_append_u16(, 0);
+   /* session type (HMAC, audit or policy) */
+   tpm_buf_append_u8(, TPM2_SE_POLICY);
+   /* symmetric encryption parameters */
+   /* symmetric algorithm */
+   tpm_buf_append_u16(, TPM2_ALG_NULL);
+
+   /* hash algorithm for session */
+   tpm_buf_append_u16(, TPM2_ALG_SHA256);
+
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "start policy session");
+
+   h = get_unaligned_be32([TPM_HEADER_SIZE]);
+
+   tpm_buf_reset_cmd(, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_POLICY_COMMAND_CODE);
+   tpm_

[PATCH v3 5/6] trusted keys: Add session encryption protection to the seal/unseal path

2018-03-10 Thread James Bottomley
If some entity is snooping the TPM bus, the can see the data going in
to be sealed and the data coming out as it is unsealed.  Add parameter
and response encryption to these cases to ensure that no secrets are
leaked even if the bus is snooped.

As part of doing this conversion it was discovered that policy
sessions can't work with HMAC protected authority because of missing
pieces (the tpm Nonce).  I've added code to work the same way as
before, which will result in potential authority exposure (while still
adding security for the command and the returned blob), and a fixme to
redo the API to get rid of this security hole.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 drivers/char/tpm/tpm2-cmd.c | 156 
 1 file changed, 98 insertions(+), 58 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 47395c455ae1..8b164b7347de 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -463,8 +463,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
  struct trusted_key_options *options)
 {
unsigned int blob_len;
-   struct tpm_buf buf;
+   struct tpm_buf buf, t2b;
u32 hash, rlength;
+   struct tpm2_auth *auth;
int i;
int rc;
 
@@ -478,45 +479,56 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (i == ARRAY_SIZE(tpm2_hash_map))
return -EINVAL;
 
-   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+   rc = tpm2_start_auth_session(chip, );
if (rc)
return rc;
 
-   tpm_buf_append_u32(, options->keyhandle);
-   tpm2_buf_append_auth(, TPM2_RS_PW,
-NULL /* nonce */, 0,
-0 /* session_attributes */,
-options->keyauth /* hmac */,
-TPM_DIGEST_SIZE);
+   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+   if (rc) {
+   tpm2_end_auth_session(auth);
+   return rc;
+   }
 
+   rc = tpm_buf_init_2b();
+   if (rc) {
+   tpm_buf_destroy();
+   tpm2_end_auth_session(auth);
+   return rc;
+   }
+
+   tpm_buf_append_name(, auth, options->keyhandle, NULL);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_DECRYPT,
+   options->keyauth, TPM_DIGEST_SIZE);
/* sensitive */
-   tpm_buf_append_u16(, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+   tpm_buf_append_u16(, TPM_DIGEST_SIZE);
+   tpm_buf_append(, options->blobauth, TPM_DIGEST_SIZE);
+   tpm_buf_append_u16(, payload->key_len + 1);
+   tpm_buf_append(, payload->key, payload->key_len);
+   tpm_buf_append_u8(, payload->migratable);
 
-   tpm_buf_append_u16(, TPM_DIGEST_SIZE);
-   tpm_buf_append(, options->blobauth, TPM_DIGEST_SIZE);
-   tpm_buf_append_u16(, payload->key_len + 1);
-   tpm_buf_append(, payload->key, payload->key_len);
-   tpm_buf_append_u8(, payload->migratable);
+   tpm_buf_append_2b(, );
 
/* public */
-   tpm_buf_append_u16(, 14 + options->policydigest_len);
-   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
-   tpm_buf_append_u16(, hash);
+   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
+   tpm_buf_append_u16(, hash);
 
/* policy */
if (options->policydigest_len) {
-   tpm_buf_append_u32(, 0);
-   tpm_buf_append_u16(, options->policydigest_len);
-   tpm_buf_append(, options->policydigest,
+   tpm_buf_append_u32(, 0);
+   tpm_buf_append_u16(, options->policydigest_len);
+   tpm_buf_append(, options->policydigest,
   options->policydigest_len);
} else {
-   tpm_buf_append_u32(, TPM2_OA_USER_WITH_AUTH);
-   tpm_buf_append_u16(, 0);
+   tpm_buf_append_u32(, TPM2_OA_USER_WITH_AUTH);
+   tpm_buf_append_u16(, 0);
}
 
/* public parameters */
-   tpm_buf_append_u16(, TPM2_ALG_NULL);
-   tpm_buf_append_u16(, 0);
+   tpm_buf_append_u16(, TPM2_ALG_NULL);
+   /* unique (zero) */
+   tpm_buf_append_u16(, 0);
+
+   tpm_buf_append_2b(, );
 
/* outside info */
tpm_buf_append_u16(, 0);
@@ -529,8 +541,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
 
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
- "sealing data");
+   tpm_buf_fill_hmac_session(, auth);
+
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data,
+ PAGE_SIZE, 4, 0, "sealing data");
+   rc = tpm_buf_check_hmac_response(, auth, rc);
if (rc)
goto out;
 
@@ -549,6 +564

[PATCH v3 4/6] tpm2: add session encryption protection to tpm2_get_random()

2018-03-10 Thread James Bottomley
If some entity is snooping the TPM bus, they can see the random
numbers we're extracting from the TPM and do prediction attacks
against their consumers.  Foil this attack by using response
encryption to prevent the attacker from seeing the random sequence.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---

v3: add error handling to sessions and redo to be outside loop
---
 drivers/char/tpm/tpm2-cmd.c | 73 +++--
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6ed07ca4a5e8..47395c455ae1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -38,10 +38,6 @@ struct tpm2_get_tpm_pt_out {
__be32  value;
 } __packed;
 
-struct tpm2_get_random_in {
-   __be16  size;
-} __packed;
-
 struct tpm2_get_random_out {
__be16  size;
u8  buffer[TPM_MAX_RNG_DATA];
@@ -51,8 +47,6 @@ union tpm2_cmd_params {
struct  tpm2_startup_in startup_in;
struct  tpm2_get_tpm_pt_in  get_tpm_pt_in;
struct  tpm2_get_tpm_pt_out get_tpm_pt_out;
-   struct  tpm2_get_random_in  getrandom_in;
-   struct  tpm2_get_random_out getrandom_out;
 };
 
 struct tpm2_cmd {
@@ -304,17 +298,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
return rc;
 }
 
-
-#define TPM2_GETRANDOM_IN_SIZE \
-   (sizeof(struct tpm_input_header) + \
-sizeof(struct tpm2_get_random_in))
-
-static const struct tpm_input_header tpm2_getrandom_header = {
-   .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
-   .length = cpu_to_be32(TPM2_GETRANDOM_IN_SIZE),
-   .ordinal = cpu_to_be32(TPM2_CC_GET_RANDOM)
-};
-
 /**
  * tpm2_get_random() - get random bytes from the TPM RNG
  *
@@ -327,44 +310,64 @@ static const struct tpm_input_header 
tpm2_getrandom_header = {
  */
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 {
-   struct tpm2_cmd cmd;
-   u32 recd, rlength;
+   u32 recd;
u32 num_bytes;
int err;
int total = 0;
int retries = 5;
u8 *dest = out;
+   struct tpm_buf buf;
+   struct tpm2_get_random_out *rout;
+   struct tpm2_auth *auth;
 
-   num_bytes = min_t(u32, max, sizeof(cmd.params.getrandom_out.buffer));
+   num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
 
-   if (!out || !num_bytes ||
-   max > sizeof(cmd.params.getrandom_out.buffer))
+   if (!out || !num_bytes
+   || max > TPM_MAX_RNG_DATA)
return -EINVAL;
 
-   do {
-   cmd.header.in = tpm2_getrandom_header;
-   cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
+   err = tpm2_start_auth_session(chip, );
+   if (err)
+   return err;
+
+   err = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+   if (err) {
+   tpm2_end_auth_session(auth);
+   return err;
+   }
 
-   err = tpm_transmit_cmd(chip, NULL, , sizeof(cmd),
-  offsetof(struct tpm2_get_random_out,
-   buffer),
+   do {
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_ENCRYPT
+   | TPM2_SA_CONTINUE_SESSION,
+   NULL, 0);
+   tpm_buf_append_u16(, num_bytes);
+   tpm_buf_fill_hmac_session(, auth);
+   err = tpm_transmit_cmd(chip, >kernel_space, buf.data,
+  PAGE_SIZE, TPM_HEADER_SIZE + 2,
   0, "attempting get random");
+   err = tpm_buf_check_hmac_response(, auth, err);
if (err)
break;
 
-   recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
-num_bytes);
-   rlength = be32_to_cpu(cmd.header.out.length);
-   if (rlength < offsetof(struct tpm2_get_random_out, buffer) +
- recd)
-   return -EFAULT;
-   memcpy(dest, cmd.params.getrandom_out.buffer, recd);
+   rout = (struct tpm2_get_random_out *)[TPM_HEADER_SIZE 
+ 4];
+   recd = be16_to_cpu(rout->size);
+   recd = min_t(u32, recd, num_bytes);
+   if (tpm_buf_length() < TPM_HEADER_SIZE + 4
+   + 2 + recd) {
+   total = -EFAULT;
+   break;
+   }
+   memcpy(dest, rout->buffer, recd);
 
dest += recd;
total += recd;
num_bytes -= recd;
+   tpm_buf_reset_cmd(, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
} while (retries-- && total < max);
 
+   tpm_buf_destroy();
+   tpm2_end_auth_sess

[PATCH v3 3/6] tpm2: add hmac checks to tpm2_pcr_extend()

2018-03-10 Thread James Bottomley
We use tpm2_pcr_extend() in trusted keys to extend a PCR to prevent a
key from being re-loaded until the next reboot.  To use this
functionality securely, that extend must be protected by a session
hmac.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---

v3: add error handling to sessions
---
 drivers/char/tpm/tpm2-cmd.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index c0ebfc4efd4d..6ed07ca4a5e8 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -247,13 +247,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
return rc;
 }
 
-struct tpm2_null_auth_area {
-   __be32  handle;
-   __be16  nonce_size;
-   u8  attributes;
-   __be16  auth_size;
-} __packed;
-
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
@@ -268,7 +261,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 
count,
struct tpm2_digest *digests)
 {
struct tpm_buf buf;
-   struct tpm2_null_auth_area auth_area;
+   struct tpm2_auth *auth;
int rc;
int i;
int j;
@@ -276,20 +269,19 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
if (count > ARRAY_SIZE(chip->active_banks))
return -EINVAL;
 
-   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+   rc = tpm2_start_auth_session(chip, );
if (rc)
return rc;
 
-   tpm_buf_append_u32(, pcr_idx);
+   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+   if (rc) {
+   tpm2_end_auth_session(auth);
+   return rc;
+   }
 
-   auth_area.handle = cpu_to_be32(TPM2_RS_PW);
-   auth_area.nonce_size = 0;
-   auth_area.attributes = 0;
-   auth_area.auth_size = 0;
+   tpm_buf_append_name(, auth, pcr_idx, NULL);
+   tpm_buf_append_hmac_session(, auth, 0, NULL, 0);
 
-   tpm_buf_append_u32(, sizeof(struct tpm2_null_auth_area));
-   tpm_buf_append(, (const unsigned char *)_area,
-  sizeof(auth_area));
tpm_buf_append_u32(, count);
 
for (i = 0; i < count; i++) {
@@ -302,9 +294,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
   hash_digest_size[tpm2_hash_map[j].crypto_id]);
}
}
-
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
- "attempting extend a PCR value");
+   tpm_buf_fill_hmac_session(, auth);
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "attempting extend a PCR value");
+   rc = tpm_buf_check_hmac_response(, auth, rc);
 
tpm_buf_destroy();
 
-- 
2.12.3


[PATCH v3 2/6] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-03-10 Thread James Bottomley
This code adds true session based HMAC authentication plus parameter
decryption and response encryption using AES.

The basic design of this code is to segregate all the nasty crypto,
hash and hmac code into tpm2-sessions.c and export a usable API.

The API first of all starts off by gaining a session with

tpm2_start_auth_session()

Which initiates a session with the TPM and allocates an opaque
tpm2_auth structure to handle the session parameters.  Then the use is
simply:

* tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
  handles

* tpm_buf_append_hmac_session() where tpm2_append_auth() would go

* tpm_buf_fill_hmac_session() called after the entire command buffer
  is finished but before tpm_transmit_cmd() is called which computes
  the correct HMAC and places it in the command at the correct
  location.

Finally, after tpm_transmit_cmd() is called,
tpm_buf_check_hmac_response() is called to check that the returned
HMAC matched and collect the new state for the next use of the
session, if any.

The features of the session is controlled by the session attributes
set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
not specified, the session will be flushed and the tpm2_auth structure
freed in tpm_buf_check_hmac_response(); otherwise the session may be
used again.  Parameter encryption is specified by or'ing the flag
TPM2_SA_DECRYPT and response encryption by or'ing the flag
TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
respectively.

To get all of this to work securely, the Kernel now needs a primary
key to encrypt the session salt to, so we derive an EC key from the
NULL seed and store it in the tpm_chip structure.  We also make sure
that this seed remains for the kernel by using a kernel space to take
it out of the TPM when userspace wants to use it.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---

v2: Added docbook and improved response check API
v3: Add readpublic, fix hmac length, add API for close on error
allow for the hmac session not being first in the sessions
---
 drivers/char/tpm/Kconfig |3 +
 drivers/char/tpm/Makefile|2 +-
 drivers/char/tpm/tpm.h   |   27 +
 drivers/char/tpm/tpm2-cmd.c  |   34 +-
 drivers/char/tpm/tpm2-sessions.c | 1166 ++
 drivers/char/tpm/tpm2-sessions.h |   57 ++
 6 files changed, 1273 insertions(+), 16 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0aee88df98d1..8c714d8550c4 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -8,6 +8,9 @@ menuconfig TCG_TPM
select SECURITYFS
select CRYPTO
select CRYPTO_HASH_INFO
+   select CRYPTO_ECDH
+   select CRYPTO_AES
+   select CRYPTO_CFB
---help---
  If you have a TPM security chip in your system, which
  implements the Trusted Computing Group's specification,
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 41b2482b97c3..b83737ccaa81 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
- tpm2-space.o tpm-buf.o
+ tpm2-space.o tpm-buf.o tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 2fca263d4ca3..b1eee56cbbb5 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -42,6 +42,9 @@
 #include 
 #endif
 
+/* fixed define for the curve we use which is NIST_P256 */
+#define EC_PT_SZ   32
+
 enum tpm_const {
TPM_MINOR = 224,/* officially assigned */
TPM_BUFSIZE = 4096,
@@ -93,6 +96,7 @@ enum tpm2_const {
 enum tpm2_structures {
TPM2_ST_NO_SESSIONS = 0x8001,
TPM2_ST_SESSIONS= 0x8002,
+   TPM2_ST_CREATION= 0x8021,
 };
 
 /* Indicates from what layer of the software stack the error comes from */
@@ -114,16 +118,25 @@ enum tpm2_return_codes {
 enum tpm2_algorithms {
TPM2_ALG_ERROR  = 0x,
TPM2_ALG_SHA1   = 0x0004,
+   TPM2_ALG_AES= 0x0006,
TPM2_ALG_KEYEDHASH  = 0x0008,
TPM2_ALG_SHA256 = 0x000B,
TPM2_ALG_SHA384 = 0x000C,
TPM2_ALG_SHA512 = 0x000D,
TPM2_ALG_NULL   = 0x0010,
TPM2_ALG_SM3_256= 0x0012,
+   TPM2_ALG_ECC= 0x0023,
+   TPM2_ALG_CFB= 0x0043,
+};
+
+enum tpm2_curves {
+   TPM2_ECC_NONE   = 0x,
+   TPM2_ECC_NIS

[PATCH v3 1/6] tpm-buf: create new functions for handling TPM buffers

2018-03-10 Thread James Bottomley
This separates out the old tpm_buf_... handling functions from static
inlines into tpm.h and makes them their own tpm-buf.c file.  It also
adds handling for tpm2b structures and also incremental pointer
advancing parsers.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---

v2: added this patch to separate out the API changes
v3: added tpm_buf_reset_cmd()
---
 drivers/char/tpm/Makefile  |   2 +-
 drivers/char/tpm/tpm-buf.c | 191 +
 drivers/char/tpm/tpm.h |  95 --
 3 files changed, 208 insertions(+), 80 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index d37c4a1748f5..41b2482b97c3 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
- tpm2-space.o
+ tpm2-space.o tpm-buf.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
new file mode 100644
index ..146a71cec067
--- /dev/null
+++ b/drivers/char/tpm/tpm-buf.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Handing for tpm2b structures to facilitate the building of commands
+ */
+
+#include "tpm.h"
+
+#include 
+
+#include 
+
+static int __tpm_buf_init(struct tpm_buf *buf)
+{
+   buf->data_page = alloc_page(GFP_HIGHUSER);
+   if (!buf->data_page)
+   return -ENOMEM;
+
+   buf->flags = 0;
+   buf->data = kmap(buf->data_page);
+
+   return 0;
+}
+
+void tpm_buf_reset_cmd(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+   struct tpm_input_header *head;
+
+   head = (struct tpm_input_header *) buf->data;
+
+   head->tag = cpu_to_be16(tag);
+   head->length = cpu_to_be32(sizeof(*head));
+   head->ordinal = cpu_to_be32(ordinal);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_reset_cmd);
+
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+   int rc;
+
+   rc = __tpm_buf_init(buf);
+   if (rc)
+   return rc;
+
+   tpm_buf_reset_cmd(buf, tag, ordinal);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
+
+int tpm_buf_init_2b(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head;
+   int rc;
+
+   rc = __tpm_buf_init(buf);
+   if (rc)
+   return rc;
+
+   head = (struct tpm_input_header *) buf->data;
+
+   head->length = cpu_to_be32(sizeof(*head));
+
+   buf->flags = TPM_BUF_2B;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init_2b);
+
+void tpm_buf_destroy(struct tpm_buf *buf)
+{
+   kunmap(buf->data_page);
+   __free_page(buf->data_page);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+
+static void *tpm_buf_data(struct tpm_buf *buf)
+{
+   if (buf->flags & TPM_BUF_2B)
+   return buf->data + TPM_HEADER_SIZE;
+   return buf->data;
+}
+
+u32 tpm_buf_length(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+   u32 len;
+
+   len = be32_to_cpu(head->length);
+   if (buf->flags & TPM_BUF_2B)
+   len -= sizeof(*head);
+   return len;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_length);
+
+u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+
+   return be16_to_cpu(head->tag);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_tag);
+
+void tpm_buf_append(struct tpm_buf *buf,
+   const unsigned char *new_data,
+   unsigned int new_len)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+   u32 len = be32_to_cpu(head->length);
+
+   /* Return silently if overflow has already happened. */
+   if (buf->flags & TPM_BUF_OVERFLOW)
+   return;
+
+   if ((len + new_len) > PAGE_SIZE) {
+   WARN(1, "tpm_buf: overflow\n");
+   buf->flags |= TPM_BUF_OVERFLOW;
+   return;
+   }
+
+   memcpy(>data[len], new_data, new_len);
+   head->length = cpu_to_be32(len + new_len);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append);
+
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+   tpm_buf_append(buf, , 1);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u8);
+
+void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+   __be16 value2 = cpu_to_be16(value);
+
+   tpm_buf_append(buf, (u8 *) , 2);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u16);
+
+void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+   __be32 value2 = cpu_to_be32(value);
+
+   tpm_buf_append(buf, (u8 *) , 4);
+}

[PATCH v3 0/6] add integrity and security to TPM2 transactions

2018-03-10 Thread James Bottomley
By now, everybody knows we have a problem with the TPM2_RS_PW easy
button on TPM2 in that transactions on the TPM bus can be intercepted
and altered.  The way to fix this is to use real sessions for HMAC
capabilities to ensure integrity and to use parameter and response
encryption to ensure confidentiality of the data flowing over the TPM
bus.

This patch series is about adding a simple API which can ensure the
above properties as a layered addition to the existing TPM handling
code.  This series now includes protections for PCR extend, getting
random numbers from the TPM and data sealing and unsealing.  It
therefore eliminates all uses of TPM2_RS_PW in the kernel and adds
encryption protection to sensitive data flowing into and out of the
TPM.

This series is also dependent on additions to the crypto subsystem to
fix problems in the elliptic curve key handling and add the Cipher
FeedBack encryption scheme:

https://marc.info/?l=linux-crypto-vger=151994371015475

In the third version I've added data sealing and unsealing protection,
apart from one API based problem which means that the way trusted keys
were protected it's not currently possible to HMAC protect an authority
that comes with a policy, so the API will have to be extended to fix
that case

I've verified this using the test suite in the last patch on a VM
connected to a tpm2 emulator.  I also instrumented the emulator to make
sure the sensitive data was properly encrypted.

James

---

James Bottomley (6):
  tpm-buf: create new functions for handling TPM buffers
  tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
  tpm2: add hmac checks to tpm2_pcr_extend()
  tpm2: add session encryption protection to tpm2_get_random()
  trusted keys: Add session encryption protection to the seal/unseal path
  tpm2-sessions: NOT FOR COMMITTING add sessions testing

 drivers/char/tpm/Kconfig  |3 +
 drivers/char/tpm/Makefile |3 +-
 drivers/char/tpm/tpm-buf.c|  191 ++
 drivers/char/tpm/tpm-chip.c   |1 +
 drivers/char/tpm/tpm.h|  123 ++--
 drivers/char/tpm/tpm2-cmd.c   |  298 +
 drivers/char/tpm/tpm2-sessions-test.c |  359 ++
 drivers/char/tpm/tpm2-sessions.c  | 1166 +
 drivers/char/tpm/tpm2-sessions.h  |   57 ++
 9 files changed, 1993 insertions(+), 208 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

-- 
2.12.3


Re: [RFC 0/5] add integrity and security to TPM2 transactions

2018-03-10 Thread James Bottomley
On Sat, 2018-03-10 at 14:49 +0200, Jarkko Sakkinen wrote:
> On Wed, 2018-03-07 at 15:29 -0800, James Bottomley wrote:
> > 
> > By now, everybody knows we have a problem with the TPM2_RS_PW easy
> > button on TPM2 in that transactions on the TPM bus can be
> > intercepted
> > and altered.  The way to fix this is to use real sessions for HMAC
> > capabilities to ensure integrity and to use parameter and response
> > encryption to ensure confidentiality of the data flowing over the
> > TPM
> > bus.
> > 
> > This RFC is about adding a simple API which can ensure the above
> > properties as a layered addition to the existing TPM handling code.
> >  Eventually we can add this to the random number generator, the PCR
> > extensions and the trusted key handling, but this all depends on
> > the
> > conversion to tpm_buf which is not yet upstream, so I've
> > constructed a
> > second patch which demonstrates the new API in a test module for
> > those
> > who wish to play with it.
> > 
> > This series is also dependent on additions to the crypto subsystem
> > to
> > fix problems in the elliptic curve key handling and add the Cipher
> > FeedBack encryption scheme:
> > 
> > https://marc.info/?l=linux-crypto-vger=151994371015475
> > 
> > In the second version, I added security HMAC to our PCR extend and
> > encryption to the returned random number generators and also
> > extracted
> > the parsing and tpm2b construction API into a new file.
> > 
> > James
> 
> Might take up until end of next week before I have time to try this
> out.Anyway, I'll see if I get this running on my systems before at
> the code that much.

OK, you might want to wait for v3 then.  I've got it working with
sealed (trusted) keys, well except for a problem with the trusted keys
API that means we can't protect the password for policy based keys.  I
think the API is finally complete, so I'll send v3 as a PATCH not an
RFC.

The point of the last patch is to show the test rig for this I'm
running in a VM using an instrumented tpm2 emulator to prove we're
getting all the correct data in and out (and that the encryption and
hmac are working); more physical TPM testing would be useful ..

Thanks,

James



[RFC v2 5/5] tpm2-sessions: NOT FOR COMMITTING add sessions testing

2018-03-07 Thread James Bottomley
>From f69d2ec1bdddefa87c7130699c797cd5e24fcaf2 Mon Sep 17 00:00:00 2001
This runs through a preset sequence using sessions to demonstrate that
the session handling code functions.  It does both HMAC, encryption
and decryption by testing an encrypted sealing operation with
authority and proving that the same sealed data comes back again via
an HMAC and response encryption.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 drivers/char/tpm/Makefile |   1 +
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm2-sessions-test.c | 177 ++
 3 files changed, 179 insertions(+)
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index b83737ccaa81..1ac7a4046630 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
  tpm2-space.o tpm-buf.o tpm2-sessions.o
+obj-m +=  tpm2-sessions-test.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 0a62c19937b6..ca174ee1e670 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -118,6 +118,7 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
 
return res;
 }
+EXPORT_SYMBOL(tpm_chip_find_get);
 
 /**
  * tpm_dev_release() - free chip memory and the device number
diff --git a/drivers/char/tpm/tpm2-sessions-test.c 
b/drivers/char/tpm/tpm2-sessions-test.c
new file mode 100644
index ..bd599648c971
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions-test.c
@@ -0,0 +1,177 @@
+/* run a set of tests of the sessions code */
+#include "tpm.h"
+#include "tpm2-sessions.h"
+
+#include 
+
+int tpm2_sessions_test(void)
+{
+   struct tpm2_auth *auth;
+   struct tpm_buf buf, b1;
+   struct tpm_buf t2b;
+   struct tpm_chip *chip;
+   int rc;
+   char payload[29];
+   char *password = "Passw0Rd";
+   const u8 *p;
+   u32 h;
+   u8 name[34];
+   u16 len;
+   int ret = -EINVAL;
+
+   chip = tpm_chip_find_get(NULL);
+   if (!chip)
+   return -ENODEV;
+
+   if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+   return -ENODEV;
+
+   get_random_bytes(payload, sizeof(payload));
+
+   /* precursor: get a session */
+   rc = tpm2_start_auth_session(chip, );
+   dev_info(>dev, "TPM: start auth session returned %d\n", rc);
+   if (rc)
+   goto out;
+
+   /* first test: get random bytes from TPM */
+   tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_ENCRYPT
+   | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+   tpm_buf_append_u16(, 29);
+   tpm_buf_fill_hmac_session(, auth);
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "get random");
+   rc = tpm_buf_check_hmac_response(, auth, rc);
+   dev_info(>dev, "TPM: check hmac response returned %d\n", rc);
+   tpm_buf_destroy();
+
+   /*
+* second test, seal random data protecting sensitive by
+* encryption and also doing response encryption (not
+* necessary) The encrypted payload has two components: an
+* authorization password which must be presented on useal and
+* the actual data (the random payload)
+*/
+   tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+   tpm_buf_append_name(, auth, chip->tpmkey, chip->tpmkeyname);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_DECRYPT
+   | TPM2_SA_ENCRYPT
+   | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+   /* sensitive */
+   tpm_buf_init_2b();
+   /* the authorization */
+   tpm_buf_append_u16(, strlen(password));
+   tpm_buf_append(, password, strlen(password));
+   /* the payload */
+   tpm_buf_append_u16(, sizeof(payload));
+   tpm_buf_append(, payload, sizeof(payload));
+   tpm_buf_append_2b(, );
+   /* the public */
+   /* type */
+   tpm_buf_append_u16(, TPM2_ALG_KEYEDHASH);
+   /* name hash */
+   tpm_buf_append_u16(, TPM2_ALG_SHA256);
+   /* object properties */
+   tpm_buf_append_u32(, TPM2_OA_USER_WITH_AUTH | TPM2_OA_NO_DA);
+   /* auth policy (empty) */
+   tpm_buf_append_u16(, 0);
+   /* keyed hash parameters (we're null for a non-HMAC data blob) */
+   tpm_buf_append_u16(, TPM2_ALG_NULL);
+   /* unique */
+   tpm_buf_append_u16(, 0);
+   tp

[RFC v2 3/5] tpm2: add hmac checks to tpm2_pcr_extend()

2018-03-07 Thread James Bottomley
We use tpm2_pcr_extend() in trusted keys to extend a PCR to prevent a
key from being re-loaded until the next reboot.  To use this
functionality securely, that extend must be protected by a session
hmac.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 drivers/char/tpm/tpm2-cmd.c | 31 +++
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 2042d4008b9c..a56cdd5d55ff 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -247,13 +247,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
return rc;
 }
 
-struct tpm2_null_auth_area {
-   __be32  handle;
-   __be16  nonce_size;
-   u8  attributes;
-   __be16  auth_size;
-} __packed;
-
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
@@ -268,7 +261,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 
count,
struct tpm2_digest *digests)
 {
struct tpm_buf buf;
-   struct tpm2_null_auth_area auth_area;
+   struct tpm2_auth *auth;
int rc;
int i;
int j;
@@ -276,20 +269,17 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
if (count > ARRAY_SIZE(chip->active_banks))
return -EINVAL;
 
-   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+   rc = tpm2_start_auth_session(chip, );
if (rc)
return rc;
 
-   tpm_buf_append_u32(, pcr_idx);
+   rc = tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
+   if (rc)
+   return rc;
 
-   auth_area.handle = cpu_to_be32(TPM2_RS_PW);
-   auth_area.nonce_size = 0;
-   auth_area.attributes = 0;
-   auth_area.auth_size = 0;
+   tpm_buf_append_name(, auth, pcr_idx, NULL);
+   tpm_buf_append_hmac_session(, auth, 0, NULL, 0);
 
-   tpm_buf_append_u32(, sizeof(struct tpm2_null_auth_area));
-   tpm_buf_append(, (const unsigned char *)_area,
-  sizeof(auth_area));
tpm_buf_append_u32(, count);
 
for (i = 0; i < count; i++) {
@@ -302,9 +292,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, 
u32 count,
   hash_digest_size[tpm2_hash_map[j].crypto_id]);
}
}
-
-   rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
- "attempting extend a PCR value");
+   tpm_buf_fill_hmac_session(, auth);
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "attempting extend a PCR value");
+   rc = tpm_buf_check_hmac_response(, auth, rc);
 
tpm_buf_destroy();
 
-- 
2.12.3


[RFC v2 2/5] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-03-07 Thread James Bottomley
This code adds true session based HMAC authentication plus parameter
decryption and response encryption using AES.

The basic design of this code is to segregate all the nasty crypto,
hash and hmac code into tpm2-sessions.c and export a usable API.

The API first of all starts off by gaining a session with

tpm2_start_auth_session()

Which initiates a session with the TPM and allocates an opaque
tpm2_auth structure to handle the session parameters.  Then the use is
simply:

* tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
  handles

* tpm_buf_append_hmac_session() where tpm2_append_auth() would go

* tpm_buf_fill_hmac_session() called after the entire command buffer
  is finished but before tpm_transmit_cmd() is called which computes
  the correct HMAC and places it in the command at the correct
  location.

Finally, after tpm_transmit_cmd() is called,
tpm_buf_check_hmac_response() is called to check that the returned
HMAC matched and collect the new state for the next use of the
session, if any.

The features of the session is controlled by the session attributes
set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
not specified, the session will be flushed and the tpm2_auth structure
freed in tpm_buf_check_hmac_response(); otherwise the session may be
used again.  Parameter encryption is specified by or'ing the flag
TPM2_SA_DECRYPT and response encryption by or'ing the flag
TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
respectively.

To get all of this to work securely, the Kernel now needs a primary
key to encrypt the session salt to, so we derive an EC key from the
NULL seed and store it in the tpm_chip structure.  We also make sure
that this seed remains for the kernel by using a kernel space to take
it out of the TPM when userspace wants to use it.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---

v2: Added docbook and improved response check API
---
 drivers/char/tpm/Kconfig |3 +
 drivers/char/tpm/Makefile|2 +-
 drivers/char/tpm/tpm.h   |   26 +
 drivers/char/tpm/tpm2-cmd.c  |   22 +-
 drivers/char/tpm/tpm2-sessions.c | 1030 ++
 drivers/char/tpm/tpm2-sessions.h |   56 +++
 6 files changed, 1126 insertions(+), 13 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0aee88df98d1..8c714d8550c4 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -8,6 +8,9 @@ menuconfig TCG_TPM
select SECURITYFS
select CRYPTO
select CRYPTO_HASH_INFO
+   select CRYPTO_ECDH
+   select CRYPTO_AES
+   select CRYPTO_CFB
---help---
  If you have a TPM security chip in your system, which
  implements the Trusted Computing Group's specification,
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 41b2482b97c3..b83737ccaa81 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
- tpm2-space.o tpm-buf.o
+ tpm2-space.o tpm-buf.o tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 51c1fab2354c..1b495d748f90 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -42,6 +42,9 @@
 #include 
 #endif
 
+/* fixed define for the curve we use which is NIST_P256 */
+#define EC_PT_SZ   32
+
 enum tpm_const {
TPM_MINOR = 224,/* officially assigned */
TPM_BUFSIZE = 4096,
@@ -93,6 +96,7 @@ enum tpm2_const {
 enum tpm2_structures {
TPM2_ST_NO_SESSIONS = 0x8001,
TPM2_ST_SESSIONS= 0x8002,
+   TPM2_ST_CREATION= 0x8021,
 };
 
 /* Indicates from what layer of the software stack the error comes from */
@@ -114,16 +118,25 @@ enum tpm2_return_codes {
 enum tpm2_algorithms {
TPM2_ALG_ERROR  = 0x,
TPM2_ALG_SHA1   = 0x0004,
+   TPM2_ALG_AES= 0x0006,
TPM2_ALG_KEYEDHASH  = 0x0008,
TPM2_ALG_SHA256 = 0x000B,
TPM2_ALG_SHA384 = 0x000C,
TPM2_ALG_SHA512 = 0x000D,
TPM2_ALG_NULL   = 0x0010,
TPM2_ALG_SM3_256= 0x0012,
+   TPM2_ALG_ECC= 0x0023,
+   TPM2_ALG_CFB= 0x0043,
+};
+
+enum tpm2_curves {
+   TPM2_ECC_NONE   = 0x,
+   TPM2_ECC_NIST_P256  = 0x0003,
 };
 
 enum tpm2_command_codes {
TPM2_CC_FIRST   = 0x011F,
+   TPM2_CC_CREATE_P

[RFC v2 1/5] tpm-buf: create new functions for handling TPM buffers

2018-03-07 Thread James Bottomley
This separates out the old tpm_buf_... handling functions from static
inlines into tpm.h and makes them their own tpm-buf.c file.  It also
adds handling for tpm2b structures and also incremental pointer
advancing parsers.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>

---

v2: added this patch to separate out the API changes
---
 drivers/char/tpm/Makefile  |   2 +-
 drivers/char/tpm/tpm-buf.c | 184 +
 drivers/char/tpm/tpm.h |  94 ---
 3 files changed, 200 insertions(+), 80 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index d37c4a1748f5..41b2482b97c3 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
- tpm2-space.o
+ tpm2-space.o tpm-buf.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
new file mode 100644
index ..8e674f410823
--- /dev/null
+++ b/drivers/char/tpm/tpm-buf.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Handing for tpm2b structures to facilitate the building of commands
+ */
+
+#include "tpm.h"
+
+#include 
+
+#include 
+
+static int __tpm_buf_init(struct tpm_buf *buf)
+{
+   buf->data_page = alloc_page(GFP_HIGHUSER);
+   if (!buf->data_page)
+   return -ENOMEM;
+
+   buf->flags = 0;
+   buf->data = kmap(buf->data_page);
+
+   return 0;
+}
+
+int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+   struct tpm_input_header *head;
+   int rc;
+
+   rc = __tpm_buf_init(buf);
+   if (rc)
+   return rc;
+
+   head = (struct tpm_input_header *) buf->data;
+
+   head->tag = cpu_to_be16(tag);
+   head->length = cpu_to_be32(sizeof(*head));
+   head->ordinal = cpu_to_be32(ordinal);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
+
+int tpm_buf_init_2b(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head;
+   int rc;
+
+   rc = __tpm_buf_init(buf);
+   if (rc)
+   return rc;
+
+   head = (struct tpm_input_header *) buf->data;
+
+   head->length = cpu_to_be32(sizeof(*head));
+
+   buf->flags = TPM_BUF_2B;
+   return 0;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init_2b);
+
+void tpm_buf_destroy(struct tpm_buf *buf)
+{
+   kunmap(buf->data_page);
+   __free_page(buf->data_page);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+
+static void *tpm_buf_data(struct tpm_buf *buf)
+{
+   if (buf->flags & TPM_BUF_2B)
+   return buf->data + TPM_HEADER_SIZE;
+   return buf->data;
+}
+
+u32 tpm_buf_length(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+   u32 len;
+
+   len = be32_to_cpu(head->length);
+   if (buf->flags & TPM_BUF_2B)
+   len -= sizeof(*head);
+   return len;
+}
+EXPORT_SYMBOL_GPL(tpm_buf_length);
+
+u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+
+   return be16_to_cpu(head->tag);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_tag);
+
+void tpm_buf_append(struct tpm_buf *buf,
+   const unsigned char *new_data,
+   unsigned int new_len)
+{
+   struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+   u32 len = be32_to_cpu(head->length);
+
+   /* Return silently if overflow has already happened. */
+   if (buf->flags & TPM_BUF_OVERFLOW)
+   return;
+
+   if ((len + new_len) > PAGE_SIZE) {
+   WARN(1, "tpm_buf: overflow\n");
+   buf->flags |= TPM_BUF_OVERFLOW;
+   return;
+   }
+
+   memcpy(>data[len], new_data, new_len);
+   head->length = cpu_to_be32(len + new_len);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append);
+
+void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+   tpm_buf_append(buf, , 1);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u8);
+
+void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+   __be16 value2 = cpu_to_be16(value);
+
+   tpm_buf_append(buf, (u8 *) , 2);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u16);
+
+void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+   __be32 value2 = cpu_to_be32(value);
+
+   tpm_buf_append(buf, (u8 *) , 4);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
+
+static void tpm_buf_reset(struct tpm_buf *buf)
+{
+   struct tpm_input_header *head;
+
+   head = (struct tpm_input_header *)buf->

[RFC 0/5] add integrity and security to TPM2 transactions

2018-03-07 Thread James Bottomley
By now, everybody knows we have a problem with the TPM2_RS_PW easy
button on TPM2 in that transactions on the TPM bus can be intercepted
and altered.  The way to fix this is to use real sessions for HMAC
capabilities to ensure integrity and to use parameter and response
encryption to ensure confidentiality of the data flowing over the TPM
bus.

This RFC is about adding a simple API which can ensure the above
properties as a layered addition to the existing TPM handling code.
 Eventually we can add this to the random number generator, the PCR
extensions and the trusted key handling, but this all depends on the
conversion to tpm_buf which is not yet upstream, so I've constructed a
second patch which demonstrates the new API in a test module for those
who wish to play with it.

This series is also dependent on additions to the crypto subsystem to
fix problems in the elliptic curve key handling and add the Cipher
FeedBack encryption scheme:

https://marc.info/?l=linux-crypto-vger=151994371015475

In the second version, I added security HMAC to our PCR extend and
encryption to the returned random number generators and also extracted
the parsing and tpm2b construction API into a new file.

James

---

James Bottomley (5):
  tpm-buf: create new functions for handling TPM buffers
  tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
  tpm2: add hmac checks to tpm2_pcr_extend()
  tpm2: add session encryption protection to tpm2_get_random()
  tpm2-sessions: NOT FOR COMMITTING add sessions testing

 drivers/char/tpm/Kconfig  |3 +
 drivers/char/tpm/Makefile |3 +-
 drivers/char/tpm/tpm-buf.c|  184 ++
 drivers/char/tpm/tpm-chip.c   |1 +
 drivers/char/tpm/tpm.h|  116 ++--
 drivers/char/tpm/tpm2-cmd.c   |  129 ++---
 drivers/char/tpm/tpm2-sessions-test.c |  177 ++
 drivers/char/tpm/tpm2-sessions.c  | 1030 +
 drivers/char/tpm/tpm2-sessions.h  |   56 ++
 9 files changed, 1548 insertions(+), 151 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-buf.c
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h

-- 
2.12.3


Re: [RFC 0/2] add integrity and security to TPM2 transactions

2018-03-05 Thread James Bottomley
On Mon, 2018-03-05 at 07:04 -0700, Jason Gunthorpe wrote:
> On Fri, Mar 02, 2018 at 10:04:54PM -0800, James Bottomley wrote:
> > 
> > By now, everybody knows we have a problem with the TPM2_RS_PW easy
> > button on TPM2 in that transactions on the TPM bus can be
> > intercepted and altered.  The way to fix this is to use real
> > sessions for HMAC capabilities to ensure integrity and to use
> > parameter and response encryption to ensure confidentiality of the
> > data flowing over the TPM bus.
> 
> We have the same issue for TPM1 then right?

Sort of.  HMAC authentication isn't optional in TPM1 like it is in
TPM2, so we do already use it (in the trusted keys code, for instance),
so we have less of a problem becasuse it doesn't have the insecure
TPM_RS_PW option.

However, TPM1 also has a specific weakness here in that if you don't
have authority in the object (i.e. no shared secret), the HMAC provides
no protection against an intelligent attacker.  The only way to get the
same security as we have with TPM2 in this situation is to use
transport encryption.

Given that sha1 is already compromised, TPM1 has a strictly limited
shelf life, especially as all laptops are being shipped with TPM2 now,
so I don't think it's unreasonable to say if you're worried about this
compromise you should use TPM2.

James



Re: [PATCH 1/2] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-03-05 Thread James Bottomley
On Mon, 2018-03-05 at 13:35 +0200, Jarkko Sakkinen wrote:
> On Fri, Mar 02, 2018 at 10:06:15PM -0800, James Bottomley wrote:
> > 
> > diff --git a/drivers/char/tpm/tpm2b.h b/drivers/char/tpm/tpm2b.h
> > new file mode 100644
> > index ..c7726f2895aa
> > --- /dev/null
> > +++ b/drivers/char/tpm/tpm2b.h
> > @@ -0,0 +1,82 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _TPM2_TPM2B_H
> > +#define _TPM2_TPM2B_H
> > +/*
> > + * Handing for tpm2b structures to facilitate the building of
> > commands
> > + */
> > +
> > +#include "tpm.h"
> > +
> > +#include 
> > +
> > +struct tpm2b {
> > +   struct tpm_buf buf;
> > +};
> > +
> > +/* opaque structure, holds auth session parameters like the
> > session key */
> > +struct tpm2_auth;
> > +
> > +static inline int tpm2b_init(struct tpm2b *buf)
> > +{
> > +   return tpm_buf_init(>buf, 0, 0);
> > +}
> > +
> > +static inline void tpm2b_reset(struct tpm2b *buf)
> > +{
> > +   struct tpm_input_header *head;
> > +
> > +   head = (struct tpm_input_header *)buf->buf.data;
> > +   head->length = cpu_to_be32(sizeof(*head));
> > +}
> > +
> > +static inline void tpm2b_append(struct tpm2b *buf, const unsigned
> > char *data,
> > +   unsigned int len)
> > +{
> > +   tpm_buf_append(>buf, data, len);
> > +}
> > +
> > +#define TPM2B_APPEND(type) \
> > +   static inline void tpm2b_append_##type(struct tpm2b *buf,
> > const type value) { tpm_buf_append_##type(>buf, value); }
> > +
> > +TPM2B_APPEND(u8)
> > +TPM2B_APPEND(u16)
> > +TPM2B_APPEND(u32)
> > +
> > +static inline void *tpm2b_buffer(const struct tpm2b *buf)
> > +{
> > +   return buf->buf.data + sizeof(struct tpm_input_header);
> > +}
> > +
> > +static inline u16 tpm2b_len(struct tpm2b *buf)
> > +{
> > +   return tpm_buf_length(>buf) - sizeof(struct
> > tpm_input_header);
> > +}
> > +
> > +static inline void tpm2b_destroy(struct tpm2b *buf)
> > +{
> > +   tpm_buf_destroy(>buf);
> > +}
> > +
> > +static inline void tpm_buf_append_2b(struct tpm_buf *buf, struct
> > tpm2b *tpm2b)
> > +{
> > +   u16 len = tpm2b_len(tpm2b);
> > +
> > +   tpm_buf_append_u16(buf, len);
> > +   tpm_buf_append(buf, tpm2b_buffer(tpm2b), len);
> > +   /* clear the buf for reuse */
> > +   tpm2b_reset(tpm2b);
> > +}
> > +
> > +/* Macros for unmarshalling known size BE data */
> > +#define GET_INC(type)  \
> > +static inline u##type get_inc_##type(const u8 **ptr) { \
> > +   u##type val;\
> > +   val = get_unaligned_be##type(*ptr); \
> > +   *ptr += sizeof(val);\
> > +   return val; \
> > +}
> > +
> > +GET_INC(16)
> > +GET_INC(32)
> > +
> > +#endif
> > -- 
> > 2.12.3
> > 
> 
> Some meta stuff:
> 
> * Add me to TO-field because I should probably review and eventually
>   test these, right?

Eventually; they're an RFC because we need to get the API right first
(I've already got a couple of fixes to it).

> * CC to linux-security-module

There's no change to anything in security module, so why?  All security
module people who care about the TPM should be on linux-integrity and
those who don't likely don't want to see the changes.  The reason
linux-crypto is on the cc is because I want them to make sure I'm using
their crypto system correctly.

> * Why there is no RFC tag given that these are so quite large
> changes?

There is an RFC tag on 0/2

> * Why in hell tpm2b.h?

Because all sized TPM structures are in 2B form and manipulating them
can be made a lot easier with helper routines.

James



[PATCH 2/2] tpm2-sessions: NOT FOR COMMITTING add sessions testing

2018-03-02 Thread James Bottomley
This runs through a preset sequence using sessions to demonstrate that
the session handling code functions.  It does both HMAC, encryption
and decryption by testing an encrypted sealing operation with
authority and proving that the same sealed data comes back again via
an HMAC and response encryption.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 drivers/char/tpm/Makefile |   1 +
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm2-sessions-test.c | 178 ++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 95ef2b10cc8d..b9eb70f1aee6 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
  tpm2-space.o tpm2-sessions.o
+obj-m +=  tpm2-sessions-test.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 0a62c19937b6..ca174ee1e670 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -118,6 +118,7 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
 
return res;
 }
+EXPORT_SYMBOL(tpm_chip_find_get);
 
 /**
  * tpm_dev_release() - free chip memory and the device number
diff --git a/drivers/char/tpm/tpm2-sessions-test.c 
b/drivers/char/tpm/tpm2-sessions-test.c
new file mode 100644
index ..a0e8a8b62cf1
--- /dev/null
+++ b/drivers/char/tpm/tpm2-sessions-test.c
@@ -0,0 +1,178 @@
+/* run a set of tests of the sessions code */
+#include "tpm.h"
+#include "tpm2b.h"
+#include "tpm2-sessions.h"
+
+#include 
+
+int tpm2_sessions_test(void)
+{
+   struct tpm2_auth *auth;
+   struct tpm_buf buf, b1;
+   struct tpm2b t2b;
+   struct tpm_chip *chip;
+   int rc;
+   char payload[29];
+   char *password = "Passw0Rd";
+   const u8 *p;
+   u32 h;
+   u8 name[34];
+   u16 len;
+   int ret = -EINVAL;
+
+   chip = tpm_chip_find_get(NULL);
+   if (!chip)
+   return -ENODEV;
+
+   if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+   return -ENODEV;
+
+   get_random_bytes(payload, sizeof(payload));
+
+   /* precursor: get a session */
+   rc = tpm2_start_auth_session(chip, );
+   dev_info(>dev, "TPM: start auth session returned %d\n", rc);
+   if (rc)
+   goto out;
+
+   /* first test: get random bytes from TPM */
+   tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_ENCRYPT
+   | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+   tpm_buf_append_u16(, 29);
+   tpm_buf_fill_hmac_session(, auth);
+   rc = tpm_transmit_cmd(chip, >kernel_space, buf.data, PAGE_SIZE,
+ 0, 0, "get random");
+   rc = tpm_buf_check_hmac_response(, auth);
+   dev_info(>dev, "TPM: check hmac response returned %d\n", rc);
+   tpm_buf_destroy();
+
+   /*
+* second test, seal random data protecting sensitive by
+* encryption and also doing response encryption (not
+* necessary) The encrypted payload has two components: an
+* authorization password which must be presented on useal and
+* the actual data (the random payload)
+*/
+   tpm_buf_init(, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+   tpm_buf_append_name(, auth, chip->tpmkey, chip->tpmkeyname);
+   tpm_buf_append_hmac_session(, auth, TPM2_SA_DECRYPT
+   | TPM2_SA_ENCRYPT
+   | TPM2_SA_CONTINUE_SESSION, NULL, 0);
+   /* sensitive */
+   tpm2b_init();
+   /* the authorization */
+   tpm2b_append_u16(, strlen(password));
+   tpm2b_append(, password, strlen(password));
+   /* the payload */
+   tpm2b_append_u16(, sizeof(payload));
+   tpm2b_append(, payload, sizeof(payload));
+   tpm_buf_append_2b(, );
+   /* the public */
+   /* type */
+   tpm2b_append_u16(, TPM2_ALG_KEYEDHASH);
+   /* name hash */
+   tpm2b_append_u16(, TPM2_ALG_SHA256);
+   /* object properties */
+   tpm2b_append_u32(, TPM2_OA_USER_WITH_AUTH | TPM2_OA_NO_DA);
+   /* auth policy (empty) */
+   tpm2b_append_u16(, 0);
+   /* keyed hash parameters (we're null for a non-HMAC data blob) */
+   tpm2b_append_u16(, TPM2_ALG_NULL);
+   /* unique */
+   tpm2b_append_u16(, 0);
+   tpm_buf_append_2b(, );
+   /* outside info (also empty) */
+   tpm_buf_append_u16(, 0);
+   /*

[PATCH 1/2] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

2018-03-02 Thread James Bottomley
This code adds true session based HMAC authentication plus parameter
decryption and response encryption using AES.

The basic design of this code is to segregate all the nasty crypto,
hash and hmac code into tpm2-sessions.c and export a usable API.

The API first of all starts off by gaining a session with

tpm2_start_auth_session()

Which initiates a session with the TPM and allocates an opaque
tpm2_auth structure to handle the session parameters.  Then the use is
simply:

* tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
  handles

* tpm_buf_append_hmac_session() where tpm2_append_auth() would go

* tpm_buf_fill_hmac_session() called after the entire command buffer
  is finished but before tpm_transmit_cmd() is called which computes
  the correct HMAC and places it in the command at the correct
  location.

Finally, after tpm_transmit_cmd() is called,
tpm_buf_check_hmac_response() is called to check that the returned
HMAC matched and collect the new state for the next use of the
session, if any.

The features of the session is controlled by the session attributes
set in tpm_buf_append_hmac_session().  If TPM2_SA_CONTINUE_SESSION is
not specified, the session will be flushed and the tpm2_auth structure
freed in tpm_buf_check_hmac_response(); otherwise the session may be
used again.  Parameter encryption is specified by or'ing the flag
TPM2_SA_DECRYPT and response encryption by or'ing the flag
TPM2_SA_ENCRYPT.  the various encryptions will be taken care of by
tpm_buf_fill_hmac_session() and tpm_buf_check_hmac_response()
respectively.

To get all of this to work securely, the Kernel now needs a primary
key to encrypt the session salt to, so we derive an EC key from the
NULL seed and store it in the tpm_chip structure.  We also make sure
that this seed remains for the kernel by using a kernel space to take
it out of the TPM when userspace wants to use it.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 drivers/char/tpm/Kconfig |   3 +
 drivers/char/tpm/Makefile|   2 +-
 drivers/char/tpm/tpm.h   |  22 +
 drivers/char/tpm/tpm2-cmd.c  |  22 +-
 drivers/char/tpm/tpm2-sessions.c | 907 +++
 drivers/char/tpm/tpm2-sessions.h |  55 +++
 drivers/char/tpm/tpm2b.h |  82 
 7 files changed, 1080 insertions(+), 13 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h
 create mode 100644 drivers/char/tpm/tpm2b.h

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 0aee88df98d1..8c714d8550c4 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -8,6 +8,9 @@ menuconfig TCG_TPM
select SECURITYFS
select CRYPTO
select CRYPTO_HASH_INFO
+   select CRYPTO_ECDH
+   select CRYPTO_AES
+   select CRYPTO_CFB
---help---
  If you have a TPM security chip in your system, which
  implements the Trusted Computing Group's specification,
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index d37c4a1748f5..95ef2b10cc8d 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -5,7 +5,7 @@
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
 tpm-dev-common.o tpmrm-dev.o tpm1_eventlog.o tpm2_eventlog.o \
- tpm2-space.o
+ tpm2-space.o tpm2-sessions.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_eventlog_acpi.o
 tpm-$(CONFIG_EFI) += tpm_eventlog_efi.o
 tpm-$(CONFIG_OF) += tpm_eventlog_of.o
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3e083a30a108..95a0d5288d6a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -42,6 +42,9 @@
 #include 
 #endif
 
+/* fixed define for the curve we use which is NIST_P256 */
+#define EC_PT_SZ   32
+
 enum tpm_const {
TPM_MINOR = 224,/* officially assigned */
TPM_BUFSIZE = 4096,
@@ -114,16 +117,25 @@ enum tpm2_return_codes {
 enum tpm2_algorithms {
TPM2_ALG_ERROR  = 0x,
TPM2_ALG_SHA1   = 0x0004,
+   TPM2_ALG_AES= 0x0006,
TPM2_ALG_KEYEDHASH  = 0x0008,
TPM2_ALG_SHA256 = 0x000B,
TPM2_ALG_SHA384 = 0x000C,
TPM2_ALG_SHA512 = 0x000D,
TPM2_ALG_NULL   = 0x0010,
TPM2_ALG_SM3_256= 0x0012,
+   TPM2_ALG_ECC= 0x0023,
+   TPM2_ALG_CFB= 0x0043,
+};
+
+enum tpm2_curves {
+   TPM2_ECC_NONE   = 0x,
+   TPM2_ECC_NIST_P256  = 0x0003,
 };
 
 enum tpm2_command_codes {
TPM2_CC_FIRST   = 0x011F,
+   TPM2_CC_CREATE_PRIMARY  = 0x0131,
TPM2_CC_SELF_TEST   = 0x0143,
TPM2_CC_STARTUP = 0x0144,
TPM2_CC_SHUTDOWN= 0x0145,
@@ -133,6 +145,7 @@ enum tpm2_command_codes {
TPM2_CC_CONTEXT_LOAD= 0x0161,
TPM2_CC_CONTEXT_SAVE= 

[RFC 0/2] add integrity and security to TPM2 transactions

2018-03-02 Thread James Bottomley
By now, everybody knows we have a problem with the TPM2_RS_PW easy
button on TPM2 in that transactions on the TPM bus can be intercepted
and altered.  The way to fix this is to use real sessions for HMAC
capabilities to ensure integrity and to use parameter and response
encryption to ensure confidentiality of the data flowing over the TPM
bus.

This RFC is about adding a simple API which can ensure the above
properties as a layered addition to the existing TPM handling code.
 Eventually we can add this to the random number generator, the PCR
extensions and the trusted key handling, but this all depends on the
conversion to tpm_buf which is not yet upstream, so I've constructed a
second patch which demonstrates the new API in a test module for those
who wish to play with it.

This series is also dependent on additions to the crypto subsystem to
fix problems in the elliptic curve key handling and add the Cipher
FeedBack encryption scheme:

https://marc.info/?l=linux-crypto-vger=151994371015475

---

James Bottomley (2):
  tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
  tpm2-sessions: NOT FOR COMMITTING add sessions testing

 drivers/char/tpm/Kconfig  |   3 +
 drivers/char/tpm/Makefile |   3 +-
 drivers/char/tpm/tpm-chip.c   |   1 +
 drivers/char/tpm/tpm.h|  22 +
 drivers/char/tpm/tpm2-cmd.c   |  22 +-
 drivers/char/tpm/tpm2-sessions-test.c | 178 +++
 drivers/char/tpm/tpm2-sessions.c  | 907 ++
 drivers/char/tpm/tpm2-sessions.h  |  55 +++
 drivers/char/tpm/tpm2b.h  |  82 +++
 9 files changed, 1260 insertions(+), 13 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-sessions-test.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.h
 create mode 100644 drivers/char/tpm/tpm2b.h

-- 
2.12.3


[PATCH 2/2] crypto: ecdh: fix to allow multi segment scatterlists

2018-03-01 Thread James Bottomley
Apparently the ecdh use case was in bluetooth which always has single
element scatterlists, so the ecdh module was hard coded to expect
them.  Now we're using this in TPM, we need multi-element
scatterlists, so remove this limitation.

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 crypto/ecdh.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index 3aca0933ec44..d2ec33f0e098 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -89,12 +89,19 @@ static int ecdh_compute_value(struct kpp_request *req)
if (!shared_secret)
goto free_pubkey;
 
-   copied = sg_copy_to_buffer(req->src, 1, public_key,
-  public_key_sz);
-   if (copied != public_key_sz) {
-   ret = -EINVAL;
+   /* from here on it's invalid parameters */
+   ret = -EINVAL;
+
+   /* must have exactly two points to be on the curve */
+   if (public_key_sz != req->src_len)
+   goto free_all;
+
+   copied = sg_copy_to_buffer(req->src,
+  sg_nents_for_len(req->src,
+   public_key_sz),
+  public_key, public_key_sz);
+   if (copied != public_key_sz)
goto free_all;
-   }
 
ret = crypto_ecdh_shared_secret(ctx->curve_id, ctx->ndigits,
ctx->private_key, public_key,
@@ -111,7 +118,11 @@ static int ecdh_compute_value(struct kpp_request *req)
if (ret < 0)
goto free_all;
 
-   copied = sg_copy_from_buffer(req->dst, 1, buf, nbytes);
+   /* might want less than we've got */
+   nbytes = min_t(size_t, nbytes, req->dst_len);
+   copied = sg_copy_from_buffer(req->dst, sg_nents_for_len(req->dst,
+   nbytes),
+buf, nbytes);
if (copied != nbytes)
ret = -EINVAL;
 
-- 
2.12.3


[PATCH 1/2] crypto: cfb: add support for Cipher FeedBack mode

2018-03-01 Thread James Bottomley
TPM security routines require encryption and decryption with AES in
CFB mode, so add it to the Linux Crypto schemes.  CFB is basically a
one time pad where the pad is generated initially from the encrypted
IV and then subsequently from the encrypted previous block of
ciphertext.  The pad is XOR'd into the plain text to get the final
ciphertext.

https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#CFB

Signed-off-by: James Bottomley <james.bottom...@hansenpartnership.com>
---
 crypto/Kconfig  |   8 ++
 crypto/Makefile |   1 +
 crypto/cfb.c| 353 
 3 files changed, 362 insertions(+)
 create mode 100644 crypto/cfb.c

diff --git a/crypto/Kconfig b/crypto/Kconfig
index b75264b09a46..d43f2f677a10 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -324,6 +324,14 @@ config CRYPTO_CBC
  CBC: Cipher Block Chaining mode
  This block cipher algorithm is required for IPSec.
 
+config CRYPTO_CFB
+   tristate "CFB support"
+   select CRYPTO_BLKCIPHER
+   select CRYPTO_MANAGER
+   help
+ CFB: Cipher FeedBack mode
+ This block cipher algorithm is required for TPM2 Cryptography.
+
 config CRYPTO_CTR
tristate "CTR support"
select CRYPTO_BLKCIPHER
diff --git a/crypto/Makefile b/crypto/Makefile
index cdbc03b35510..0dcad117532e 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
 obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
 obj-$(CONFIG_CRYPTO_ECB) += ecb.o
 obj-$(CONFIG_CRYPTO_CBC) += cbc.o
+obj-$(CONFIG_CRYPTO_CFB) += cfb.o
 obj-$(CONFIG_CRYPTO_PCBC) += pcbc.o
 obj-$(CONFIG_CRYPTO_CTS) += cts.o
 obj-$(CONFIG_CRYPTO_LRW) += lrw.o
diff --git a/crypto/cfb.c b/crypto/cfb.c
new file mode 100644
index ..94ee39bed758
--- /dev/null
+++ b/crypto/cfb.c
@@ -0,0 +1,353 @@
+//SPDX-License-Identifier: GPL-2.0
+/*
+ * CFB: Cipher FeedBack mode
+ *
+ * Copyright (c) 2018 james.bottom...@hansenpartnership.com
+ *
+ * CFB is a stream cipher mode which is layered on to a block
+ * encryption scheme.  It works very much like a one time pad where
+ * the pad is generated initially from the encrypted IV and then
+ * subsequently from the encrypted previous block of ciphertext.  The
+ * pad is XOR'd into the plain text to get the final ciphertext.
+ *
+ * The scheme of CFB is best described by wikipedia:
+ *
+ * https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#CFB
+ *
+ * Note that since the pad for both encryption and decryption is
+ * generated by an encryption operation, CFB never uses the block
+ * decryption function.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct crypto_cfb_ctx {
+   struct crypto_cipher *child;
+};
+
+static unsigned int crypto_cfb_bsize(struct crypto_skcipher *tfm)
+{
+   struct crypto_cfb_ctx *ctx = crypto_skcipher_ctx(tfm);
+   struct crypto_cipher *child = ctx->child;
+
+   return crypto_cipher_blocksize(child);
+}
+
+static void crypto_cfb_encrypt_one(struct crypto_skcipher *tfm,
+ const u8 *src, u8 *dst)
+{
+   struct crypto_cfb_ctx *ctx = crypto_skcipher_ctx(tfm);
+
+   crypto_cipher_encrypt_one(ctx->child, dst, src);
+}
+
+/* final encrypt and decrypt is the same */
+static void crypto_cfb_final(struct skcipher_walk *walk,
+struct crypto_skcipher *tfm)
+{
+   const unsigned int bsize = crypto_cfb_bsize(tfm);
+   const unsigned long alignmask = crypto_skcipher_alignmask(tfm);
+   u8 tmp[bsize + alignmask];
+   u8 *stream = PTR_ALIGN(tmp + 0, alignmask + 1);
+   u8 *src = walk->src.virt.addr;
+   u8 *dst = walk->dst.virt.addr;
+   u8 *iv = walk->iv;
+   unsigned int nbytes = walk->nbytes;
+
+   crypto_cfb_encrypt_one(tfm, iv, stream);
+   crypto_xor_cpy(dst, stream, src, nbytes);
+}
+
+static int crypto_cfb_encrypt_segment(struct skcipher_walk *walk,
+ struct crypto_skcipher *tfm)
+{
+   const unsigned int bsize = crypto_cfb_bsize(tfm);
+   unsigned int nbytes = walk->nbytes;
+   u8 *src = walk->src.virt.addr;
+   u8 *dst = walk->dst.virt.addr;
+   u8 *iv = walk->iv;
+
+   do {
+   crypto_cfb_encrypt_one(tfm, iv, dst);
+   crypto_xor(dst, src, bsize);
+   memcpy(iv, dst, bsize);
+
+   src += bsize;
+   dst += bsize;
+   } while ((nbytes -= bsize) >= bsize);
+
+   return nbytes;
+}
+
+static int crypto_cfb_encrypt_inplace(struct skcipher_walk *walk,
+ struct crypto_skcipher *tfm)
+{
+   const unsigned int bsize = crypto_cfb_bsize(tfm);
+   unsigned int nbytes = walk->nbytes;
+   u8 *src = walk->src.virt.addr;
+   u8 *iv = walk->iv;
+   u8 tmp[bsize]

[PATCH 0/2] add crypto support for TPM communication

2018-03-01 Thread James Bottomley
To support cryptographic communication with the TPM, we need to add
Cipher FeedBack (CFB) mode for stream encryption because this is the
mandated encryption scheme for all encrypted parameters and responses.
 Additionally, we ran across a problem in the elliptic curve routines
in that the size of the scatterlist is hard coded to 1 which causes a
kernel BUG if you use a longer scatterlist.  Since all the current
kernel consumers use a single element scatterlist, this bug won't
manifest until we add the TPM routines to use crypto, so I didn't mark
it for stable.

James Bottomley (2):
  crypto: cfb: add support for Cipher FeedBack mode
  crypto: ecdh: fix to allow multi segment scatterlists

 crypto/Kconfig  |   8 ++
 crypto/Makefile |   1 +
 crypto/cfb.c| 353 
 crypto/ecdh.c   |  23 +++-
 4 files changed, 379 insertions(+), 6 deletions(-)
 create mode 100644 crypto/cfb.c

-- 
2.12.3


Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

2013-09-26 Thread James Bottomley
On Thu, 2013-09-26 at 08:24 +0200, Jiri Kosina wrote:
 On Wed, 25 Sep 2013, James Bottomley wrote:
 
   I don't get this. Why is it important that current kernel can't
   recreate the signature?
  
  The thread model is an attack on the saved information (i.e. the suspend
  image) between it being saved by the old kernel and used by the new one.
  The important point isn't that the new kernel doesn't have access to
  K_{N-1} it's that no-one does: the key is destroyed as soon as the old
  kernel terminates however the verification public part P_{N-1} survives.
 
 James,
 
 could you please describe the exact scenario you think that the symmetric 
 keys aproach doesn't protect against, while the assymetric key aproach 
 does?
 
 The crucial points, which I believe make the symmetric key aproach work 
 (and I feel quite embarassed by the fact that I haven't realized this 
 initially when coming up with the assymetric keys aproach) are:
 
 - the kernel that is performing the actual resumption is trusted in the 
   secure boot model, i.e. you trust it to perform proper verification
 
 - potentially malicious userspace (which is what we are protecting against 
   -- malicious root creating fake hibernation image and issuing reboot) 
   doesn't have access to the symmetric key

OK, so the scheme is to keep a symmetric key in BS that is passed into
the kernel each time (effectively a secret key) for signing and
validation?

The only two problems I see are

 1. The key isn't generational (any compromise obtains it).  This
can be fixed by using a set of keys generated on each boot and
passing in both K_{N-1} and K_N
 2. No external agency other than the next kernel can do the
validation since the validating key has to be secret

The importance of 2 is just tripwire like detection ... perhaps it
doesn't really matter in a personal computer situation.  It would matter
in an enterprise where images are stored and re-used but until servers
have UEFI secure boot, that's not going to be an issue.

James



--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

2013-09-25 Thread James Bottomley
On Wed, 2013-09-25 at 17:25 -0400, Alan Stern wrote:
 On Wed, 25 Sep 2013, David Howells wrote:
 
  I have pushed some keyrings patches that will likely affect this to:
  
  
  http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
  
  I intend to ask James to pull these into his next branch.  If he's happy to 
  do
  so, I can look at pulling at least your asymmetric keys patch on top of 
  them.
 
 This suggests a point that I raised at the Linux Plumbers conference:
 
 Why are asymmetric keys used for verifying the hibernation image?  It
 seems that a symmetric key would work just as well.  And it would be a
 lot quicker to generate, because it wouldn't need any high-precision
 integer computations.

The reason is the desire to validate that the previous kernel created
something which it passed on to the current kernel (in this case, the
hibernation image) untampered with.  To do that, something must be
passed to the prior kernel that can be validated but *not* recreated by
the current kernel.

The scheme for doing this is a public/private key pair generated for
each boot incarnation N as a pair P_N (public key) and K_N (private
key).  Then the Nth boot incarnation gets P_{N-1} and K_N (the boot
environment holds P_N in inaccessible BS variables for passing into the
next kernel) so the Nth kernel can validate information from the N-1th
kernel using P_{N-1} and create information for passing on in a
validated fashion to the next kernel using K_N.

This scheme doesn't work with symmetric keys unless you have a
modification I haven't seen?

James



--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

2013-09-25 Thread James Bottomley
On Thu, 2013-09-26 at 02:27 +0200, Pavel Machek wrote:
 On Wed 2013-09-25 15:16:54, James Bottomley wrote:
  On Wed, 2013-09-25 at 17:25 -0400, Alan Stern wrote:
   On Wed, 25 Sep 2013, David Howells wrote:
   
I have pushed some keyrings patches that will likely affect this to:


http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel

I intend to ask James to pull these into his next branch.  If he's 
happy to do
so, I can look at pulling at least your asymmetric keys patch on top of 
them.
   
   This suggests a point that I raised at the Linux Plumbers conference:
   
   Why are asymmetric keys used for verifying the hibernation image?  It
   seems that a symmetric key would work just as well.  And it would be a
   lot quicker to generate, because it wouldn't need any high-precision
   integer computations.
  
  The reason is the desire to validate that the previous kernel created
  something which it passed on to the current kernel (in this case, the
  hibernation image) untampered with.  To do that, something must be
  passed to the prior kernel that can be validated but *not* recreated by
  the current kernel.
 
 I don't get this. Why is it important that current kernel can't
 recreate the signature?

The thread model is an attack on the saved information (i.e. the suspend
image) between it being saved by the old kernel and used by the new one.
The important point isn't that the new kernel doesn't have access to
K_{N-1} it's that no-one does: the key is destroyed as soon as the old
kernel terminates however the verification public part P_{N-1} survives.

James

 Current kernel is not considered malicious (if it were, you have worse
 problems).
 
   Pavel
 
 PS: And yes, it would be nice to have
 Documentation/power/swsusp-uefi.txt (or something) explaining the
 design.
 



--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html