Skip to contentSkip to conversation

move TPM-specific fields into trusted_tpm_options

PATCH Applied Srish Srinivasan 3 participants 10 messages Feb 20 – Mar 23

Subsystems (3)

v2 of 5 Applied Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v2 cover move TPM-specific fields into trusted_tpm_options

The backend-agnostic trusted_key_options structure carries TPM-specific fields. With the recent addition of a backend-private pointer, these fields can be moved out of the generic options structure.

This patch series intends to reloacte all TPM-spcific fields into a newly defined trusted_tpm_options structure. A pointer to the trusted_tpm_options struct is stored in trusted_key_option's private.

Along with the migration of TPM-specific fields, this patch series includes a preparatory clean-up patch: replacing pr_info() with pr_debug() and using KERN_DEBUG for print_hex_dump() for logging debug information.

Testing covered both TPM 1.2 and TPM 2.0 backends (virtual environment), including trusted key creation, revocation, unlinking, invalidation, and loading keys from encrypted blobs. I would welcome any additional testing from upstream to further strengthen the validation.

Changelog:

v2:
* Exclude the bug-fix patch as it has already been applied to 6.19-rc7
* Rename instances of trusted_tpm_options from tpm_opts to private
* Use pr_debug and KERN_DEBUG for logging debug messages (preparatory clean up patch)
* Address comments from Jarkko

Srish Srinivasan (2):
keys/trusted_keys: clean up debug message logging in the tpm backend keys/trusted_keys: move TPM-specific fields into trusted_tpm_options

 include/keys/trusted-type.h               |  11 --
 include/keys/trusted_tpm.h                |  14 +++
 security/keys/trusted-keys/trusted_tpm1.c | 129 ++++++++++++----------
 security/keys/trusted-keys/trusted_tpm2.c |  51 +++++----
 4 files changed, 111 insertions(+), 94 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v2 1/2 keys/trusted_keys: clean up debug message logging in the tpm backend

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump(). Remove TPM_DEBUG.

No functional change intended.

---
 security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
 1 file changed, 12 insertions(+), 28 deletions(-)
security/keys/trusted-keys/trusted_tpm1.c +12 -28
index c865c97aa1b4..216caef97ffc 100644
@@ -46,28 +46,25 @@ enum {
 	SRK_keytype = 4
 };
 
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
 static inline void dump_options(struct trusted_key_options *o)
 {
-	pr_info("sealing key type %d\n", o->keytype);
-	pr_info("sealing key handle %0X\n", o->keyhandle);
-	pr_info("pcrlock %d\n", o->pcrlock);
-	pr_info("pcrinfo %d\n", o->pcrinfo_len);
-	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
+	pr_debug("sealing key type %d\n", o->keytype);
+	pr_debug("sealing key handle %0X\n", o->keyhandle);
+	pr_debug("pcrlock %d\n", o->pcrlock);
+	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
 		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
 }
 
 static inline void dump_sess(struct osapsess *s)
 {
-	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
+	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
 		       16, 1, &s->handle, 4, 0);
-	pr_info("secret:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("secret:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
 		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-	pr_info("trusted-key: enonce:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("trusted-key: enonce:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
 		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
 }
 
@@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
 {
 	int len;
 
-	pr_info("\ntpm buffer\n");
+	pr_debug("\ntpm buffer\n");
 	len = LOAD32(buf, TPM_SIZE_OFFSET);
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
 }
-#endif
 
 static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 		       unsigned int keylen, ...)
Signature · 1 line
2.43.0
maintainer Reviewed-by on v2 1/2 Applied
#2 lore raw

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

Quoted 86 lines · Srish Srinivasan

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump(). Remove TPM_DEBUG.

No functional change intended.

---
 security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
 1 file changed, 12 insertions(+), 28 deletions(-)
security/keys/trusted-keys/trusted_tpm1.c +12 -28
index c865c97aa1b4..216caef97ffc 100644
@@ -46,28 +46,25 @@ enum {
 	SRK_keytype = 4
 };
 
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
 static inline void dump_options(struct trusted_key_options *o)
 {
-	pr_info("sealing key type %d\n", o->keytype);
-	pr_info("sealing key handle %0X\n", o->keyhandle);
-	pr_info("pcrlock %d\n", o->pcrlock);
-	pr_info("pcrinfo %d\n", o->pcrinfo_len);
-	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
+	pr_debug("sealing key type %d\n", o->keytype);
+	pr_debug("sealing key handle %0X\n", o->keyhandle);
+	pr_debug("pcrlock %d\n", o->pcrlock);
+	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
 		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
 }
 
 static inline void dump_sess(struct osapsess *s)
 {
-	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
+	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
 		       16, 1, &s->handle, 4, 0);
-	pr_info("secret:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("secret:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
 		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-	pr_info("trusted-key: enonce:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("trusted-key: enonce:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
 		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
 }
 
@@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
 {
 	int len;
 
-	pr_info("\ntpm buffer\n");
+	pr_debug("\ntpm buffer\n");
 	len = LOAD32(buf, TPM_SIZE_OFFSET);
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
 }
-#endif
 
 static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
 		       unsigned int keylen, ...)
Signature · 1 line
2.43.0

Applied.

BR, Jarkko

#3 lore raw

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

In reply to Srish Srinivasan
Quoted 7 lines · Srish Srinivasan

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump(). Remove TPM_DEBUG.

No functional change intended.

There is functional change here.  This change allows secret and nonce in the function dump_sess() to be logged to kernel logs when dynamic debug is enabled. Previously, it was possible only in the debug builds and not the production builds at runtime. With this change, it is always there in production build. This can result in possible attack.

Instead of doing this change, I think add a comment to prevent this sort of change in the future.

Thanks & Regards,

    - Nayna

Quoted 76 lines
---
  security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
  1 file changed, 12 insertions(+), 28 deletions(-)
security/keys/trusted-keys/trusted_tpm1.c +12 -28
index c865c97aa1b4..216caef97ffc 100644
@@ -46,28 +46,25 @@ enum {
  	SRK_keytype = 4
  };
 
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
  static inline void dump_options(struct trusted_key_options *o)
  {
-	pr_info("sealing key type %d\n", o->keytype);
-	pr_info("sealing key handle %0X\n", o->keyhandle);
-	pr_info("pcrlock %d\n", o->pcrlock);
-	pr_info("pcrinfo %d\n", o->pcrinfo_len);
-	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
+	pr_debug("sealing key type %d\n", o->keytype);
+	pr_debug("sealing key handle %0X\n", o->keyhandle);
+	pr_debug("pcrlock %d\n", o->pcrlock);
+	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
  		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
  }
 
  static inline void dump_sess(struct osapsess *s)
  {
-	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
+	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
  		       16, 1, &s->handle, 4, 0);
-	pr_info("secret:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("secret:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
  		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-	pr_info("trusted-key: enonce:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("trusted-key: enonce:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
  		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
  }
 
@@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
  {
  	int len;
 
-	pr_info("\ntpm buffer\n");
+	pr_debug("\ntpm buffer\n");
  	len = LOAD32(buf, TPM_SIZE_OFFSET);
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
  }
-#endif
 
  static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
  		       unsigned int keylen, ...)
#4 lore raw

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

Quoted 10 lines · Nayna Jain
Quoted 7 lines · Srish Srinivasan

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump(). Remove TPM_DEBUG.

No functional change intended.

There is functional change here.  This change allows secret and nonce in the function dump_sess() to be logged to kernel logs when dynamic debug is enabled. Previously, it was possible only in the debug builds and not the production builds at runtime. With this change, it is always there in production build. This can result in possible attack.

Hi Jarkko,
Could you please let us know your thoughts on this one?

And Nayna,
thanks for bringing this up.

thanks,
Srish.

Quoted 81 lines

Instead of doing this change, I think add a comment to prevent this sort of change in the future.

Thanks & Regards,

    - Nayna

Quoted 74 lines

---
  security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------

  1 file changed, 12 insertions(+), 28 deletions(-)
security/keys/trusted-keys/trusted_tpm1.c +0 -0
index c865c97aa1b4..216caef97ffc 100644
@@ -46,28 +46,25 @@ enum {

      SRK_keytype = 4
  };
  -#define TPM_DEBUG 0

security/keys/trusted-keys/trusted_tpm1.c +0 -2
-
-#if TPM_DEBUG

  static inline void dump_options(struct trusted_key_options *o) {
-    pr_info("sealing key type %d\n", o->keytype);
-    pr_info("sealing key handle %0X\n", o->keyhandle);
-    pr_info("pcrlock %d\n", o->pcrlock);
-    pr_info("pcrinfo %d\n", o->pcrinfo_len);
-    print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE, +    pr_debug("sealing key type %d\n", o->keytype);
+    pr_debug("sealing key handle %0X\n", o->keyhandle);
+    pr_debug("pcrlock %d\n", o->pcrlock);
+    pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+    print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE, 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
  }
    static inline void dump_sess(struct osapsess *s)
  {
-    print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE, +    print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
                 16, 1, &s->handle, 4, 0);
-    pr_info("secret:\n");
-    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+    pr_debug("secret:\n");
+    print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
                 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-    pr_info("trusted-key: enonce:\n");
-    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+    pr_debug("trusted-key: enonce:\n");
+    print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
                 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
  }
  @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf) {
      int len;
  -    pr_info("\ntpm buffer\n");
+    pr_debug("\ntpm buffer\n");
      len = LOAD32(buf, TPM_SIZE_OFFSET);
-    print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);

security/keys/trusted-keys/trusted_tpm1.c +1 -12
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
+    print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, 

len, 0);
  }
-#endif
    static int TSS_rawhmac(unsigned char *digest, const unsigned char *key, unsigned int keylen, ...)

maintainer
#5 lore raw

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

Quoted 19 lines · Srish Srinivasan
Quoted 9 lines · Nayna Jain
Quoted 7 lines · Srish Srinivasan

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
Remove TPM_DEBUG.

Quoted 6 lines
Quoted 1 line

No functional change intended.

There is functional change here.  This change allows secret and nonce in the function dump_sess() to be logged to kernel logs when dynamic debug is enabled. Previously, it was possible only in the debug builds and not the production builds at runtime. With this change, it is always there in production build. This can result in possible attack.

Quoted 5 lines

Hi Jarkko,
Could you please let us know your thoughts on this one?

And Nayna,
thanks for bringing this up.

Nayna is absolutely right so I dropped it.

Solution is debatable.

Quoted 3 lines

thanks,
Srish.

BR, Jarkko

maintainer
#6 lore raw

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

In reply to Nayna Jain
Quoted 8 lines · Nayna Jain
Quoted 6 lines · Srish Srinivasan

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump(). Remove TPM_DEBUG.

Quoted 6 lines
Quoted 1 line

No functional change intended.

There is functional change here.  This change allows secret and nonce in the function dump_sess() to be logged to kernel logs when dynamic debug is enabled. Previously, it was possible only in the debug builds and not the production builds at runtime. With this change, it is always there in production build. This can result in possible attack.

Good catch, thank you. It's in my master branch still (not in -next).

TPM_DEBUG should be removed in all cases. If you really want to read a secret, use tracing tools.

This only proves that the print should exist or should be a constant value, or overwritten same length value.

Quoted 70 lines

Instead of doing this change, I think add a comment to prevent this sort of change in the future.

Thanks & Regards,

    - Nayna

Quoted 63 lines
---
  security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
  1 file changed, 12 insertions(+), 28 deletions(-)
security/keys/trusted-keys/trusted_tpm1.c +11 -24
index c865c97aa1b4..216caef97ffc 100644
@@ -46,28 +46,25 @@ enum {
  	SRK_keytype = 4
  };
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
  static inline void dump_options(struct trusted_key_options *o)
  {
-	pr_info("sealing key type %d\n", o->keytype);
-	pr_info("sealing key handle %0X\n", o->keyhandle);
-	pr_info("pcrlock %d\n", o->pcrlock);
-	pr_info("pcrinfo %d\n", o->pcrinfo_len);
-	print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
+	pr_debug("sealing key type %d\n", o->keytype);
+	pr_debug("sealing key handle %0X\n", o->keyhandle);
+	pr_debug("pcrlock %d\n", o->pcrlock);
+	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
  		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
  }
  static inline void dump_sess(struct osapsess *s)
  {
-	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
+	print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
  		       16, 1, &s->handle, 4, 0);
-	pr_info("secret:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("secret:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
  		       16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
-	pr_info("trusted-key: enonce:\n");
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
+	pr_debug("trusted-key: enonce:\n");
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
  		       16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
  }
@@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
  {
  	int len;
-	pr_info("\ntpm buffer\n");
+	pr_debug("\ntpm buffer\n");
  	len = LOAD32(buf, TPM_SIZE_OFFSET);
-	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
-}
-#else
-static inline void dump_options(struct trusted_key_options *o)
-{
-}
-
-static inline void dump_sess(struct osapsess *s)
-{
-}
Quoted 8 lines

security/keys/trusted-keys/trusted_tpm1.c static inline void dump_tpm_buf(unsigned char *buf) · line 81

Quoted 8 lines
security/keys/trusted-keys/trusted_tpm1.c +1 -4
-
-static inline void dump_tpm_buf(unsigned char *buf)
-{
+	print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
  }
-#endif
  static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
  		       unsigned int keylen, ...)

BR, Jarkko

maintainer
#7 lore raw

Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend

Quoted 16 lines · Jarkko Sakkinen
Quoted 8 lines · Nayna Jain
Quoted 6 lines · Srish Srinivasan

The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info() for logging debug information.

Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump(). Remove TPM_DEBUG.

Quoted 6 lines
Quoted 1 line

No functional change intended.

There is functional change here.  This change allows secret and nonce in the function dump_sess() to be logged to kernel logs when dynamic debug is enabled. Previously, it was possible only in the debug builds and not the production builds at runtime. With this change, it is always there in production build. This can result in possible attack.

Quoted 7 lines

Good catch, thank you. It's in my master branch still (not in -next).

TPM_DEBUG should be removed in all cases. If you really want to read a secret, use tracing tools.

This only proves that the print should exist or should be a constant value, or overwritten same length value.

I dropped the current patches but yeah, a comment "do not touch this, could be poisonous" won't be an acceptable way to address this.

If you want "some" debug information you can always put F-string or 0-string of same length, so there's options.

BR, Jarkko

#8 lore raw

PATCH v2 2/2 keys/trusted_keys: move TPM-specific fields into trusted_tpm_options

In reply to Srish Srinivasan

The trusted_key_options struct contains TPM-specific fields (keyhandle, keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash, policydigest_len, policydigest, and policyhandle). This leads to the accumulation of backend-specific fields in the generic options structure.

Define trusted_tpm_options structure and move the TPM-specific fields there. Store a pointer to trusted_tpm_options in trusted_key_options's private.

No functional change intended.

---
 include/keys/trusted-type.h               | 11 ---
 include/keys/trusted_tpm.h                | 14 ++++
 security/keys/trusted-keys/trusted_tpm1.c | 95 ++++++++++++++---------
 security/keys/trusted-keys/trusted_tpm2.c | 51 ++++++------
 4 files changed, 102 insertions(+), 69 deletions(-)
include/keys/trusted-type.h +0 -11
index 03527162613f..b80f250305b8 100644
@@ -39,17 +39,6 @@ struct trusted_key_payload {
 
 struct trusted_key_options {
 	uint16_t keytype;
-	uint32_t keyhandle;
-	unsigned char keyauth[TPM_DIGEST_SIZE];
-	uint32_t blobauth_len;
-	unsigned char blobauth[TPM_DIGEST_SIZE];
-	uint32_t pcrinfo_len;
-	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
-	int pcrlock;
-	uint32_t hash;
-	uint32_t policydigest_len;
-	unsigned char policydigest[MAX_DIGEST_SIZE];
-	uint32_t policyhandle;
 	void *private;
 };
 
include/keys/trusted_tpm.h +14 -0
index 0fadc6a4f166..355ebd36cbfd 100644
@@ -7,6 +7,20 @@
 
 extern struct trusted_key_ops trusted_key_tpm_ops;
 
+struct trusted_tpm_options {
+	uint32_t keyhandle;
+	unsigned char keyauth[TPM_DIGEST_SIZE];
+	uint32_t blobauth_len;
+	unsigned char blobauth[TPM_DIGEST_SIZE];
+	uint32_t pcrinfo_len;
+	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
+	int pcrlock;
+	uint32_t hash;
+	uint32_t policydigest_len;
+	unsigned char policydigest[MAX_DIGEST_SIZE];
+	uint32_t policyhandle;
+};
+
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options);
security/keys/trusted-keys/trusted_tpm1.c +59 -36

260 lines not on this page · raw message

security/keys/trusted-keys/trusted_tpm2.c +29 -22

160 lines not on this page · raw message

Signature · 1 line
2.43.0
maintainer Reviewed-by on v2 2/2 Applied
#9 lore raw

Re: [PATCH v2 2/2] keys/trusted_keys: move TPM-specific fields into trusted_tpm_options

Quoted 532 lines · Srish Srinivasan

The trusted_key_options struct contains TPM-specific fields (keyhandle, keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash, policydigest_len, policydigest, and policyhandle). This leads to the accumulation of backend-specific fields in the generic options structure.

Define trusted_tpm_options structure and move the TPM-specific fields there. Store a pointer to trusted_tpm_options in trusted_key_options's private.

No functional change intended.

---
 include/keys/trusted-type.h               | 11 ---
 include/keys/trusted_tpm.h                | 14 ++++
 security/keys/trusted-keys/trusted_tpm1.c | 95 ++++++++++++++---------
 security/keys/trusted-keys/trusted_tpm2.c | 51 ++++++------
 4 files changed, 102 insertions(+), 69 deletions(-)
include/keys/trusted-type.h +0 -11
index 03527162613f..b80f250305b8 100644
@@ -39,17 +39,6 @@ struct trusted_key_payload {
 
 struct trusted_key_options {
 	uint16_t keytype;
-	uint32_t keyhandle;
-	unsigned char keyauth[TPM_DIGEST_SIZE];
-	uint32_t blobauth_len;
-	unsigned char blobauth[TPM_DIGEST_SIZE];
-	uint32_t pcrinfo_len;
-	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
-	int pcrlock;
-	uint32_t hash;
-	uint32_t policydigest_len;
-	unsigned char policydigest[MAX_DIGEST_SIZE];
-	uint32_t policyhandle;
 	void *private;
 };
 
include/keys/trusted_tpm.h +14 -0
index 0fadc6a4f166..355ebd36cbfd 100644
@@ -7,6 +7,20 @@
 
 extern struct trusted_key_ops trusted_key_tpm_ops;
 
+struct trusted_tpm_options {
+	uint32_t keyhandle;
+	unsigned char keyauth[TPM_DIGEST_SIZE];
+	uint32_t blobauth_len;
+	unsigned char blobauth[TPM_DIGEST_SIZE];
+	uint32_t pcrinfo_len;
+	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
+	int pcrlock;
+	uint32_t hash;
+	uint32_t policydigest_len;
+	unsigned char policydigest[MAX_DIGEST_SIZE];
+	uint32_t policyhandle;
+};
+
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options);
security/keys/trusted-keys/trusted_tpm1.c +59 -36
index 216caef97ffc..741b1d47d9f8 100644
@@ -48,12 +48,14 @@ enum {
 
 static inline void dump_options(struct trusted_key_options *o)
 {
+	struct trusted_tpm_options *private = o->private;
+
 	pr_debug("sealing key type %d\n", o->keytype);
-	pr_debug("sealing key handle %0X\n", o->keyhandle);
-	pr_debug("pcrlock %d\n", o->pcrlock);
-	pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+	pr_debug("sealing key handle %0X\n", private->keyhandle);
+	pr_debug("pcrlock %d\n", private->pcrlock);
+	pr_debug("pcrinfo %d\n", private->pcrinfo_len);
 	print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
-		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
+		       16, 1, private->pcrinfo, private->pcrinfo_len, 0);
 }
 
 static inline void dump_sess(struct osapsess *s)
@@ -609,6 +611,7 @@ static int tpm_unseal(struct tpm_buf *tb,
 static int key_seal(struct trusted_key_payload *p,
 		    struct trusted_key_options *o)
 {
+	struct trusted_tpm_options *private = o->private;
 	struct tpm_buf tb;
 	int ret;
 
@@ -619,9 +622,10 @@ static int key_seal(struct trusted_key_payload *p,
 	/* include migratable flag at end of sealed key */
 	p->key[p->key_len] = p->migratable;
 
-	ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
+	ret = tpm_seal(&tb, o->keytype, private->keyhandle, private->keyauth,
 		       p->key, p->key_len + 1, p->blob, &p->blob_len,
-		       o->blobauth, o->pcrinfo, o->pcrinfo_len);
+		       private->blobauth, private->pcrinfo,
+		       private->pcrinfo_len);
 	if (ret < 0)
 		pr_info("srkseal failed (%d)\n", ret);
 
@@ -635,6 +639,7 @@ static int key_seal(struct trusted_key_payload *p,
 static int key_unseal(struct trusted_key_payload *p,
 		      struct trusted_key_options *o)
 {
+	struct trusted_tpm_options *private = o->private;
 	struct tpm_buf tb;
 	int ret;
 
@@ -642,8 +647,8 @@ static int key_unseal(struct trusted_key_payload *p,
 	if (ret)
 		return ret;
 
-	ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
-			 o->blobauth, p->key, &p->key_len);
+	ret = tpm_unseal(&tb, private->keyhandle, private->keyauth, p->blob,
+			 p->blob_len, private->blobauth, p->key, &p->key_len);
 	if (ret < 0)
 		pr_info("srkunseal failed (%d)\n", ret);
 	else
@@ -680,6 +685,7 @@ static const match_table_t key_tokens = {
 static int getoptions(char *c, struct trusted_key_payload *pay,
 		      struct trusted_key_options *opt)
 {
+	struct trusted_tpm_options *private = opt->private;
 	substring_t args[MAX_OPT_ARGS];
 	char *p = c;
 	int token;
@@ -695,7 +701,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	if (tpm2 < 0)
 		return tpm2;
 
-	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
+	private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
 
 	if (!c)
 		return 0;
@@ -709,11 +715,11 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 
 		switch (token) {
 		case Opt_pcrinfo:
-			opt->pcrinfo_len = strlen(args[0].from) / 2;
-			if (opt->pcrinfo_len > MAX_PCRINFO_SIZE)
+			private->pcrinfo_len = strlen(args[0].from) / 2;
+			if (private->pcrinfo_len > MAX_PCRINFO_SIZE)
 				return -EINVAL;
-			res = hex2bin(opt->pcrinfo, args[0].from,
-				      opt->pcrinfo_len);
+			res = hex2bin(private->pcrinfo, args[0].from,
+				      private->pcrinfo_len);
 			if (res < 0)
 				return -EINVAL;
 			break;
@@ -722,12 +728,12 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			if (res < 0)
 				return -EINVAL;
 			opt->keytype = SEAL_keytype;
-			opt->keyhandle = handle;
+			private->keyhandle = handle;
 			break;
 		case Opt_keyauth:
 			if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
 				return -EINVAL;
-			res = hex2bin(opt->keyauth, args[0].from,
+			res = hex2bin(private->keyauth, args[0].from,
 				      SHA1_DIGEST_SIZE);
 			if (res < 0)
 				return -EINVAL;
@@ -738,21 +744,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			 * hex strings.  TPM 2.0 authorizations are simple
 			 * passwords (although it can take a hash as well)
 			 */
-			opt->blobauth_len = strlen(args[0].from);
+			private->blobauth_len = strlen(args[0].from);
 
-			if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) {
-				res = hex2bin(opt->blobauth, args[0].from,
+			if (private->blobauth_len == 2 * TPM_DIGEST_SIZE) {
+				res = hex2bin(private->blobauth, args[0].from,
 					      TPM_DIGEST_SIZE);
 				if (res < 0)
 					return -EINVAL;
 
-				opt->blobauth_len = TPM_DIGEST_SIZE;
+				private->blobauth_len = TPM_DIGEST_SIZE;
 				break;
 			}
 
-			if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
-				memcpy(opt->blobauth, args[0].from,
-				       opt->blobauth_len);
+			if (tpm2 &&
+			    private->blobauth_len <=
+			    sizeof(private->blobauth)) {
+				memcpy(private->blobauth, args[0].from,
+				       private->blobauth_len);
 				break;
 			}
 
@@ -770,14 +778,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			res = kstrtoul(args[0].from, 10, &lock);
 			if (res < 0)
 				return -EINVAL;
-			opt->pcrlock = lock;
+			private->pcrlock = lock;
 			break;
 		case Opt_hash:
 			if (test_bit(Opt_policydigest, &token_mask))
 				return -EINVAL;
 			for (i = 0; i < HASH_ALGO__LAST; i++) {
 				if (!strcmp(args[0].from, hash_algo_name[i])) {
-					opt->hash = i;
+					private->hash = i;
 					break;
 				}
 			}
@@ -789,14 +797,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			}
 			break;
 		case Opt_policydigest:
-			digest_len = hash_digest_size[opt->hash];
+			digest_len = hash_digest_size[private->hash];
 			if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
 				return -EINVAL;
-			res = hex2bin(opt->policydigest, args[0].from,
+			res = hex2bin(private->policydigest, args[0].from,
 				      digest_len);
 			if (res < 0)
 				return -EINVAL;
-			opt->policydigest_len = digest_len;
+			private->policydigest_len = digest_len;
 			break;
 		case Opt_policyhandle:
 			if (!tpm2)
@@ -804,7 +812,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			res = kstrtoul(args[0].from, 16, &handle);
 			if (res < 0)
 				return -EINVAL;
-			opt->policyhandle = handle;
+			private->policyhandle = handle;
 			break;
 		default:
 			return -EINVAL;
@@ -815,6 +823,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 
 static struct trusted_key_options *trusted_options_alloc(void)
 {
+	struct trusted_tpm_options *private;
 	struct trusted_key_options *options;
 	int tpm2;
 
@@ -827,14 +836,23 @@ static struct trusted_key_options *trusted_options_alloc(void)
 		/* set any non-zero defaults */
 		options->keytype = SRK_keytype;
 
-		if (!tpm2)
-			options->keyhandle = SRKHANDLE;
+		private = kzalloc(sizeof(*private), GFP_KERNEL);
+		if (!private) {
+			kfree_sensitive(options);
+			options = NULL;
+		} else {
+			if (!tpm2)
+				private->keyhandle = SRKHANDLE;
+
+			options->private = private;
+		}
 	}
 	return options;
 }
 
 static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
 {
+	struct trusted_tpm_options *private = NULL;
 	struct trusted_key_options *options = NULL;
 	int ret = 0;
 	int tpm2;
@@ -852,7 +870,8 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
 		goto out;
 	dump_options(options);
 
-	if (!options->keyhandle && !tpm2) {
+	private = options->private;
+	if (!private->keyhandle && !tpm2) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -866,20 +885,22 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
 		goto out;
 	}
 
-	if (options->pcrlock) {
-		ret = pcrlock(options->pcrlock);
+	if (private->pcrlock) {
+		ret = pcrlock(private->pcrlock);
 		if (ret < 0) {
 			pr_info("pcrlock failed (%d)\n", ret);
 			goto out;
 		}
 	}
 out:
+	kfree_sensitive(options->private);
 	kfree_sensitive(options);
 	return ret;
 }
 
 static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 {
+	struct trusted_tpm_options *private = NULL;
 	struct trusted_key_options *options = NULL;
 	int ret = 0;
 	int tpm2;
@@ -897,7 +918,8 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 		goto out;
 	dump_options(options);
 
-	if (!options->keyhandle && !tpm2) {
+	private = options->private;
+	if (!private->keyhandle && !tpm2) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -909,14 +931,15 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
 	if (ret < 0)
 		pr_info("key_unseal failed (%d)\n", ret);
 
-	if (options->pcrlock) {
-		ret = pcrlock(options->pcrlock);
+	if (private->pcrlock) {
+		ret = pcrlock(private->pcrlock);
 		if (ret < 0) {
 			pr_info("pcrlock failed (%d)\n", ret);
 			goto out;
 		}
 	}
 out:
+	kfree_sensitive(options->private);
 	kfree_sensitive(options);
 	return ret;
 }
security/keys/trusted-keys/trusted_tpm2.c +29 -22
index 6340823f8b53..94e01249b921 100644
@@ -24,6 +24,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
 			   u8 *src, u32 len)
 {
+	struct trusted_tpm_options *private = options->private;
 	const int SCRATCH_SIZE = PAGE_SIZE;
 	u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
 	u8 *work = scratch, *work1;
@@ -46,7 +47,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 	work = asn1_encode_oid(work, end_work, tpm2key_oid,
 			       asn1_oid_len(tpm2key_oid));
 
-	if (options->blobauth_len == 0) {
+	if (private->blobauth_len == 0) {
 		unsigned char bool[3], *w = bool;
 		/* tag 0 is emptyAuth */
 		w = asn1_encode_boolean(w, w + sizeof(bool), true);
@@ -69,7 +70,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
 		goto err;
 	}
 
-	work = asn1_encode_integer(work, end_work, options->keyhandle);
+	work = asn1_encode_integer(work, end_work, private->keyhandle);
 	work = asn1_encode_octet_string(work, end_work, pub, pub_len);
 	work = asn1_encode_octet_string(work, end_work, priv, priv_len);
 
@@ -102,6 +103,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
 			   u8 **buf)
 {
+	struct trusted_tpm_options *private = options->private;
 	int ret;
 	struct tpm2_key_context ctx;
 	u8 *blob;
@@ -121,7 +123,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
 		return -ENOMEM;
 
 	*buf = blob;
-	options->keyhandle = ctx.parent;
+	private->keyhandle = ctx.parent;
 
 	memcpy(blob, ctx.priv, ctx.priv_len);
 	blob += ctx.priv_len;
@@ -233,6 +235,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options)
 {
+	struct trusted_tpm_options *private = options->private;
 	off_t offset = TPM_HEADER_SIZE;
 	struct tpm_buf buf, sized;
 	int blob_len = 0;
@@ -240,11 +243,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	u32 flags;
 	int rc;
 
-	hash = tpm2_find_hash_alg(options->hash);
+	hash = tpm2_find_hash_alg(private->hash);
 	if (hash < 0)
 		return hash;
 
-	if (!options->keyhandle)
+	if (!private->keyhandle)
 		return -EINVAL;
 
 	rc = tpm_try_get_ops(chip);
@@ -268,18 +271,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out_put;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
 	if (rc)
 		goto out;
 
 	tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
-				    options->keyauth, TPM_DIGEST_SIZE);
+				    private->keyauth, TPM_DIGEST_SIZE);
 
 	/* sensitive */
-	tpm_buf_append_u16(&sized, options->blobauth_len);
+	tpm_buf_append_u16(&sized, private->blobauth_len);
 
-	if (options->blobauth_len)
-		tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
+	if (private->blobauth_len)
+		tpm_buf_append(&sized, private->blobauth,
+			       private->blobauth_len);
 
 	tpm_buf_append_u16(&sized, payload->key_len);
 	tpm_buf_append(&sized, payload->key, payload->key_len);
@@ -292,14 +296,15 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 
 	/* key properties */
 	flags = 0;
-	flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
+	flags |= private->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
 	flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
 	tpm_buf_append_u32(&sized, flags);
 
 	/* policy */
-	tpm_buf_append_u16(&sized, options->policydigest_len);
-	if (options->policydigest_len)
-		tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
+	tpm_buf_append_u16(&sized, private->policydigest_len);
+	if (private->policydigest_len)
+		tpm_buf_append(&sized, private->policydigest,
+			       private->policydigest_len);
 
 	/* public parameters */
 	tpm_buf_append_u16(&sized, TPM_ALG_NULL);
@@ -373,6 +378,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 			 u32 *blob_handle)
 {
 	u8 *blob_ref __free(kfree) = NULL;
+	struct trusted_tpm_options *private = options->private;
 	struct tpm_buf buf;
 	unsigned int private_len;
 	unsigned int public_len;
@@ -392,7 +398,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	}
 
 	/* new format carries keyhandle but old format doesn't */
-	if (!options->keyhandle)
+	if (!private->keyhandle)
 		return -EINVAL;
 
 	/* must be big enough for at least the two be16 size counts */
@@ -433,11 +439,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		return rc;
 	}
 
-	rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
+	rc = tpm_buf_append_name(chip, &buf, private->keyhandle, NULL);
 	if (rc)
 		goto out;
 
-	tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
+	tpm_buf_append_hmac_session(chip, &buf, 0, private->keyauth,
 				    TPM_DIGEST_SIZE);
 
 	tpm_buf_append(&buf, blob, blob_len);
@@ -481,6 +487,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_options *options,
 			   u32 blob_handle)
 {
+	struct trusted_tpm_options *private = options->private;
 	struct tpm_header *head;
 	struct tpm_buf buf;
 	u16 data_len;
@@ -502,10 +509,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	if (rc)
 		goto out;
 
-	if (!options->policyhandle) {
+	if (!private->policyhandle) {
 		tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
-					    options->blobauth,
-					    options->blobauth_len);
+					    private->blobauth,
+					    private->blobauth_len);
 	} else {
 		/*
 		 * FIXME: The policy session was generated outside the
@@ -518,9 +525,9 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 		 * could repeat our actions with the exfiltrated
 		 * password.
 		 */
-		tpm2_buf_append_auth(&buf, options->policyhandle,
+		tpm2_buf_append_auth(&buf, private->policyhandle,
 				     NULL /* nonce */, 0, 0,
-				     options->blobauth, options->blobauth_len);
+				     private->blobauth, private->blobauth_len);
 		if (tpm2_chip_auth(chip)) {
 			tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
 		} else  {
Signature · 1 line
2.43.0

Applied.

BR, Jarkko