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

2018-10-23 Thread Jarkko Sakkinen

The tag in the short description does not look at all. Should be either
"tpm:" or "keys, trusted:".

On Mon, 22 Oct 2018, James Bottomley wrote:

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;


[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();
tpm_buf_destroy();
 
if (rc > 0) {
@@ -528,7 +545,6 @@ int tpm2_seal_trusted(struct