diff mbox series

[v6a,2/4] secboot_tpm.c: increase tpmnv vars index size

Message ID 20200928220609.10479-3-erichte@linux.ibm.com
State Accepted
Headers show
Series Initial secure variable drivers addendum | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch
snowpatch_ozlabs/apply_patch warning Failed to apply on branch master (d362ae4f4c521a7faffb1befe2fbba467f2c4d18)

Commit Message

Eric Richter Sept. 28, 2020, 10:06 p.m. UTC
The TPM NV index size for storing the PK was originally set to 1024,
which was determined to be a "smallest maximum" size that we
determined to be enough to store the PK. However with overhead, this
only allowed for about ~912 bytes, which is far too small to store a
certificate, as it only permits about ~10 characters in the x509
subject field.

This patch increases the TPM NV Vars index to 2048 bytes, which is the
largest size a single NV index can be on the Nuvoton npct650 chip.

Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
 libstb/secvar/storage/fakenv_ops.c            | 2 +-
 libstb/secvar/storage/gen_tpmnv_public_name.c | 2 +-
 libstb/secvar/storage/secboot_tpm.c           | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

Comments

Kenneth Goldman Sept. 29, 2020, 1:13 p.m. UTC | #1
I understand wanting some margin, but a 1024-byte subject seems excessive.
Don't we have control over the subject field?  Can you find a safe but
smaller value.

The TPM is resource limited and NV is so useful for applications that
every developer should try to minimize the amount used.

> From: Eric Richter <erichte@linux.ibm.com>
> To: skiboot@lists.ozlabs.org
> Cc: klaus@linux.ibm.com, nayna@linux.ibm.com
> Date: 09/28/2020 06:07 PM
> Subject: [EXTERNAL] [Skiboot] [PATCH v6a 2/4] secboot_tpm.c:
> increase tpmnv vars index size
> Sent by: "Skiboot" <skiboot-bounces+kgoldman=us.ibm.com@lists.ozlabs.org>
>
> The TPM NV index size for storing the PK was originally set to 1024,
> which was determined to be a "smallest maximum" size that we
> determined to be enough to store the PK. However with overhead, this
> only allowed for about ~912 bytes, which is far too small to store a
> certificate, as it only permits about ~10 characters in the x509
> subject field.
>
> This patch increases the TPM NV Vars index to 2048 bytes, which is the
> largest size a single NV index can be on the Nuvoton npct650 chip.
diff mbox series

Patch

diff --git a/libstb/secvar/storage/fakenv_ops.c b/libstb/secvar/storage/fakenv_ops.c
index 64d5d51e..77ae6c3e 100644
--- a/libstb/secvar/storage/fakenv_ops.c
+++ b/libstb/secvar/storage/fakenv_ops.c
@@ -9,7 +9,7 @@  static size_t fakenv_offset = sizeof(struct secboot);
 struct fake_tpmnv {
 	struct {
 		struct secboot_header header;
-		char vars[1024]; // Hardcode the size to 1024 for now
+		char vars[2048]; // Hardcode the size to 2048 for now
 	} vars;
 	struct tpmnv_control control;
 	int defined[2];
diff --git a/libstb/secvar/storage/gen_tpmnv_public_name.c b/libstb/secvar/storage/gen_tpmnv_public_name.c
index bfeb9743..7af51312 100644
--- a/libstb/secvar/storage/gen_tpmnv_public_name.c
+++ b/libstb/secvar/storage/gen_tpmnv_public_name.c
@@ -21,7 +21,7 @@  int verbose;
 TPMS_NV_PUBLIC vars = {
 	.nvIndex = 0x01c10190,
 	.nameAlg = TPM_ALG_SHA256,
-	.dataSize = 1024,
+	.dataSize = 2048,
 	.attributes.val = TPMA_NVA_PPWRITE		|
 			  TPMA_NVA_ORDINARY             |
 			  TPMA_NVA_WRITE_STCLEAR        |
diff --git a/libstb/secvar/storage/secboot_tpm.c b/libstb/secvar/storage/secboot_tpm.c
index b6a294b0..129f674a 100644
--- a/libstb/secvar/storage/secboot_tpm.c
+++ b/libstb/secvar/storage/secboot_tpm.c
@@ -22,7 +22,7 @@  struct secboot *secboot_image = NULL;
 struct tpmnv_vars *tpmnv_vars_image = NULL;
 struct tpmnv_control *tpmnv_control_image = NULL;
 
-const size_t tpmnv_vars_size = 1024;
+const size_t tpmnv_vars_size = 2048;
 
 /* Expected TPM NV index name field from NV_ReadPublic given our known
  * set of attributes (see tss_nv_define_space).
@@ -33,9 +33,9 @@  const size_t tpmnv_vars_size = 1024;
  * which alters the hash slightly as it sets TPMA_NV_WRITELOCKED
  */
 const uint8_t tpmnv_vars_name[] = {
-	0x00, 0x0b, 0x94, 0x64, 0x36, 0x25, 0xfc, 0xc1, 0x1d, 0xc1, 0x0e, 0x28, 0xe7,
-	0xac, 0xaf, 0xc6, 0x08, 0x8e, 0xda, 0x21, 0xd6, 0x43, 0xd2, 0x77, 0xe7, 0x2d,
-	0x83, 0x39, 0x0f, 0xa6, 0xdf, 0xc0, 0x59, 0x37,
+        0x00, 0x0b, 0x7a, 0xdb, 0x70, 0xdd, 0x27, 0x94, 0x93, 0x26, 0x11, 0xe2, 0x97,
+        0x00, 0x77, 0x22, 0x4d, 0x5a, 0x74, 0xf8, 0x91, 0x6f, 0xbf, 0xf8, 0x51, 0x4a,
+        0x67, 0x6f, 0xd9, 0xa8, 0xc3, 0xfc, 0x39, 0xed,
 };
 
 const uint8_t tpmnv_control_name[] = {