move TPM-specific fields into trusted_tpm_options
Subsystems (3)
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
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
@@ -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
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_DEBUGguard andpr_info()for logging debug information.Replace
pr_info()withpr_debug(), and useKERN_DEBUGforprint_hex_dump(). RemoveTPM_DEBUG.No functional change intended.
Signed-off-by Srish Srinivasan <[email protected]>Reviewed-by Stefan Berger <[email protected]>--- security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++---------------- 1 file changed, 12 insertions(+), 28 deletions(-)security/keys/trusted-keys/trusted_tpm1.c +12
-28@@ -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
Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
In reply to Srish SrinivasanQuoted 7 lines · Srish Srinivasan
The TPM trusted-keys backend uses a local
TPM_DEBUGguard andpr_info()for logging debug information.Replace
pr_info()withpr_debug(), and useKERN_DEBUGforprint_hex_dump(). RemoveTPM_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
Signed-off-by Srish Srinivasan <[email protected]>Reviewed-by Stefan Berger <[email protected]>--- security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++---------------- 1 file changed, 12 insertions(+), 28 deletions(-)security/keys/trusted-keys/trusted_tpm1.c +12
-28@@ -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, ...)
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_DEBUGguard andpr_info()for logging debug information.Replace
pr_info()withpr_debug(), and useKERN_DEBUGforprint_hex_dump(). RemoveTPM_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
Signed-off-by Srish Srinivasan <[email protected]>Reviewed-by Stefan Berger <[email protected]>---
security/keys/trusted-keys/trusted_tpm1.c| 40 +++++++----------------1 file changed, 12 insertions(+), 28 deletions(-)security/keys/trusted-keys/trusted_tpm1.c +0
-0@@ -46,28 +46,25 @@ enum {
SRK_keytype= 4
};
-#defineTPM_DEBUG0security/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 voiddump_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 voiddump_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 intTSS_rawhmac(unsigned char *digest, const unsigned char *key, unsigned int keylen, ...)
Jarkko Sakkinen
maintainerRe: [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_DEBUGguard andpr_info()for logging debug information.Replace
pr_info()withpr_debug(), and useKERN_DEBUGforprint_hex_dump().
RemoveTPM_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
Jarkko Sakkinen
maintainerRe: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
In reply to Nayna JainQuoted 8 lines · Nayna Jain
Quoted 6 lines · Srish Srinivasan
The TPM trusted-keys backend uses a local
TPM_DEBUGguard andpr_info()for logging debug information.Replace
pr_info()withpr_debug(), and useKERN_DEBUGforprint_hex_dump(). RemoveTPM_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
Signed-off-by Srish Srinivasan <[email protected]>Reviewed-by Stefan Berger <[email protected]>--- security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++---------------- 1 file changed, 12 insertions(+), 28 deletions(-)security/keys/trusted-keys/trusted_tpm1.c +11
-24@@ -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
Jarkko Sakkinen
maintainerRe: [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_DEBUGguard andpr_info()for logging debug information.Replace
pr_info()withpr_debug(), and useKERN_DEBUGforprint_hex_dump(). RemoveTPM_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_DEBUGshould 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
PATCH v2 2/2 keys/trusted_keys: move TPM-specific fields into trusted_tpm_options
In reply to Srish SrinivasanThe 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
@@ -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
@@ -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
Re: [PATCH v2 2/2] keys/trusted_keys: move TPM-specific fields into trusted_tpm_options
Quoted 532 lines · Srish Srinivasan
The
trusted_key_optionsstruct 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_optionsstructure and move the TPM-specific fields there. Store a pointer totrusted_tpm_optionsin trusted_key_options's private.No functional change intended.
Signed-off-by Srish Srinivasan <[email protected]>Reviewed-by Stefan Berger <[email protected]>--- 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@@ -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@@ -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@@ -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@@ -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