| Message ID | 20240524130459.21510-2-James.Bottomley@HansenPartnership.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add policy to sealed keys | expand |
On Fri May 24, 2024 at 4:04 PM EEST, James Bottomley wrote: > linux crypto and the TPM use different numeric algorithm identifiers > for hash (and other) functions. The conversion array for this already > exists in two separate places. The new policy sessions code would > have to add a third copy, so instead of increasing the duplication, > move the definition to a single consolidated place in tpm.h so the > policy code can use it as is. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > --- > drivers/char/tpm/tpm2-cmd.c | 8 ---- > include/linux/tpm.h | 52 ++++++++++++++++++++++- > security/keys/trusted-keys/trusted_tpm2.c | 20 +-------- > 3 files changed, 53 insertions(+), 27 deletions(-) > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index 0cdf892ec2a7..f4428e715dd8 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -14,14 +14,6 @@ > #include "tpm.h" > #include <crypto/hash_info.h> > > -static struct tpm2_hash tpm2_hash_map[] = { > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > -}; > - > int tpm2_get_timeouts(struct tpm_chip *chip) > { > /* Fixed timeouts for TPM2 */ > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index c17e4efbb2e5..07f532456a0c 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -418,11 +418,61 @@ enum tpm2_session_attributes { > TPM2_SA_AUDIT = BIT(7), > }; > > -struct tpm2_hash { > +static const struct { > unsigned int crypto_id; > unsigned int tpm_id; > +} tpm2_hash_map[] = { > + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > }; > > +/** > + * tpm2_crypto_to_alg() - convert a crypto hash to a TPM alg id > + * > + * @hash: the crypto subsystem view of the hash > + * > + * Return: TPM algorithm id or -1 if no mapping was found. > + */ > +static inline int tpm2_crypto_to_alg(int hash) > +{ > + int i; > + int tpm_alg = -1; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + if (hash == tpm2_hash_map[i].crypto_id) { > + tpm_alg = tpm2_hash_map[i].tpm_id; > + break; > + } > + } > + > + return tpm_alg; > +} > + > +/** > + * tpm2_alg_to_crypto() - convert a TPM alg id to a crypto hash > + * > + * @hash: the TPM alg id view of the hash > + * > + * Return: TPM algorithm id or -1 if no mapping was found. > + */ > +static inline int tpm2_alg_to_crypto(int hash) > +{ > + int i; > + int crypto_hash = -1; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + if (hash == tpm2_hash_map[i].tpm_id) { > + crypto_hash = tpm2_hash_map[i].crypto_id; > + break; > + } > + } > + > + return crypto_hash; > +} > + > int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); > void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); > int tpm_buf_init_sized(struct tpm_buf *buf); > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > index dfeec06301ce..94ff9ccae66e 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -18,14 +18,6 @@ > > #include "tpm2key.asn1.h" > > -static struct tpm2_hash tpm2_hash_map[] = { > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > -}; > - > static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 }; > > static int tpm2_key_encode(struct trusted_key_payload *payload, > @@ -231,19 +223,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > off_t offset = TPM_HEADER_SIZE; > struct tpm_buf buf, sized; > int blob_len = 0; > - u32 hash; > + int hash = tpm2_crypto_to_alg(options->hash); > u32 flags; > - int i; > int rc; > > - for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > - if (options->hash == tpm2_hash_map[i].crypto_id) { > - hash = tpm2_hash_map[i].tpm_id; > - break; > - } > - } > - > - if (i == ARRAY_SIZE(tpm2_hash_map)) > + if (hash < 0) > return -EINVAL; > > if (!options->keyhandle) I want a patch set that renders out the WARN's before any other modification to this code. I've spent fixing one myself plus fixing totally trivial memory leak. That happens everyone but still focus in now all wrong. I.e. adding new stuff without polishing old first and let others take care cleaning up the mess... Also, HMAC still needs attention. And this patch set is totally conflicting getting asymmetric keys landed, which came first and if already maturing quite well. No issues reviewing after so this is not like rejecting the idea but doing right things right and in right order. BR, Jarkko
On Fri May 24, 2024 at 4:40 PM EEST, Jarkko Sakkinen wrote: > On Fri May 24, 2024 at 4:04 PM EEST, James Bottomley wrote: > > linux crypto and the TPM use different numeric algorithm identifiers > > for hash (and other) functions. The conversion array for this already > > exists in two separate places. The new policy sessions code would > > have to add a third copy, so instead of increasing the duplication, > > move the definition to a single consolidated place in tpm.h so the > > policy code can use it as is. > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > --- > > drivers/char/tpm/tpm2-cmd.c | 8 ---- > > include/linux/tpm.h | 52 ++++++++++++++++++++++- > > security/keys/trusted-keys/trusted_tpm2.c | 20 +-------- > > 3 files changed, 53 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > > index 0cdf892ec2a7..f4428e715dd8 100644 > > --- a/drivers/char/tpm/tpm2-cmd.c > > +++ b/drivers/char/tpm/tpm2-cmd.c > > @@ -14,14 +14,6 @@ > > #include "tpm.h" > > #include <crypto/hash_info.h> > > > > -static struct tpm2_hash tpm2_hash_map[] = { > > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > > -}; > > - > > int tpm2_get_timeouts(struct tpm_chip *chip) > > { > > /* Fixed timeouts for TPM2 */ > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index c17e4efbb2e5..07f532456a0c 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -418,11 +418,61 @@ enum tpm2_session_attributes { > > TPM2_SA_AUDIT = BIT(7), > > }; > > > > -struct tpm2_hash { > > +static const struct { > > unsigned int crypto_id; > > unsigned int tpm_id; > > +} tpm2_hash_map[] = { > > + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > > + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > > + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > > + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > > + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > > }; > > > > +/** > > + * tpm2_crypto_to_alg() - convert a crypto hash to a TPM alg id > > + * > > + * @hash: the crypto subsystem view of the hash > > + * > > + * Return: TPM algorithm id or -1 if no mapping was found. > > + */ > > +static inline int tpm2_crypto_to_alg(int hash) > > +{ > > + int i; > > + int tpm_alg = -1; > > + > > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > > + if (hash == tpm2_hash_map[i].crypto_id) { > > + tpm_alg = tpm2_hash_map[i].tpm_id; > > + break; > > + } > > + } > > + > > + return tpm_alg; > > +} > > + > > +/** > > + * tpm2_alg_to_crypto() - convert a TPM alg id to a crypto hash > > + * > > + * @hash: the TPM alg id view of the hash > > + * > > + * Return: TPM algorithm id or -1 if no mapping was found. > > + */ > > +static inline int tpm2_alg_to_crypto(int hash) > > +{ > > + int i; > > + int crypto_hash = -1; > > + > > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > > + if (hash == tpm2_hash_map[i].tpm_id) { > > + crypto_hash = tpm2_hash_map[i].crypto_id; > > + break; > > + } > > + } > > + > > + return crypto_hash; > > +} > > + > > int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); > > void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); > > int tpm_buf_init_sized(struct tpm_buf *buf); > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > > index dfeec06301ce..94ff9ccae66e 100644 > > --- a/security/keys/trusted-keys/trusted_tpm2.c > > +++ b/security/keys/trusted-keys/trusted_tpm2.c > > @@ -18,14 +18,6 @@ > > > > #include "tpm2key.asn1.h" > > > > -static struct tpm2_hash tpm2_hash_map[] = { > > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > > -}; > > - > > static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 }; > > > > static int tpm2_key_encode(struct trusted_key_payload *payload, > > @@ -231,19 +223,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > > off_t offset = TPM_HEADER_SIZE; > > struct tpm_buf buf, sized; > > int blob_len = 0; > > - u32 hash; > > + int hash = tpm2_crypto_to_alg(options->hash); > > u32 flags; > > - int i; > > int rc; > > > > - for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > > - if (options->hash == tpm2_hash_map[i].crypto_id) { > > - hash = tpm2_hash_map[i].tpm_id; > > - break; > > - } > > - } > > - > > - if (i == ARRAY_SIZE(tpm2_hash_map)) > > + if (hash < 0) > > return -EINVAL; > > > > if (!options->keyhandle) > > I want a patch set that renders out the WARN's before any other > modification to this code. I've spent fixing one myself plus > fixing totally trivial memory leak. That happens everyone but > still focus in now all wrong. I.e. adding new stuff without > polishing old first and let others take care cleaning up the > mess... > > Also, HMAC still needs attention. > > And this patch set is totally conflicting getting asymmetric > keys landed, which came first and if already maturing quite > well. > > No issues reviewing after so this is not like rejecting the > idea but doing right things right and in right order. Not conflicting in the sense that this would somehow fight against that code. It is anyway for different subsystem. I think this is probably legit work but does bunch of conflicting changes so not caring about this before asymmetric keys have been landed. I rarely implement anything and it is feature that I checked from various parties that it is still relevant so it is best to land it first. I did all the trouble and time and effort reviewing and even rewriting some of the buffer code in HMAC to better form exactly because I saw it more priority than landing asymmetric keys. So it is not like I prioritize my patch sets over others but these mess in same areas so thus the decision. BR, Jarkko
On Fri, May 24, 2024 at 09:04:54 -0400, James Bottomley wrote: > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index c17e4efbb2e5..07f532456a0c 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -418,11 +418,61 @@ enum tpm2_session_attributes { > TPM2_SA_AUDIT = BIT(7), > }; > > -struct tpm2_hash { > +static const struct { > unsigned int crypto_id; > unsigned int tpm_id; > +} tpm2_hash_map[] = { > + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > }; > > +/** > + * tpm2_crypto_to_alg() - convert a crypto hash to a TPM alg id Should "alg id" be "algorithm id" everwhere in the docs? > + * > + * @hash: the crypto subsystem view of the hash > + * > + * Return: TPM algorithm id or -1 if no mapping was found. > + */ > +static inline int tpm2_crypto_to_alg(int hash) How about naming this `crypto_id`? > +{ > + int i; > + int tpm_alg = -1; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + if (hash == tpm2_hash_map[i].crypto_id) { > + tpm_alg = tpm2_hash_map[i].tpm_id; > + break; > + } > + } > + > + return tpm_alg; > +} > + > +/** > + * tpm2_alg_to_crypto() - convert a TPM alg id to a crypto hash > + * > + * @hash: the TPM alg id view of the hash > + * > + * Return: TPM algorithm id or -1 if no mapping was found. Copy pasta? Should be "crypto hash" I think. > + */ > +static inline int tpm2_alg_to_crypto(int hash) Maybe name this `alg_id`? > +{ > + int i; > + int crypto_hash = -1; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + if (hash == tpm2_hash_map[i].tpm_id) { > + crypto_hash = tpm2_hash_map[i].crypto_id; > + break; > + } > + } > + > + return crypto_hash; > +} There seems to be some level of confusion with variable naming here. Are these "hashes" or "ids" being passed around? The structure calls them ids, the docs call them both, the variables call one a hash and the other unadorned. --Ben > + > int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); > void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); > int tpm_buf_init_sized(struct tpm_buf *buf); > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > index dfeec06301ce..94ff9ccae66e 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -18,14 +18,6 @@ > > #include "tpm2key.asn1.h" > > -static struct tpm2_hash tpm2_hash_map[] = { > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > -}; > - > static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 }; > > static int tpm2_key_encode(struct trusted_key_payload *payload, > @@ -231,19 +223,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > off_t offset = TPM_HEADER_SIZE; > struct tpm_buf buf, sized; > int blob_len = 0; > - u32 hash; > + int hash = tpm2_crypto_to_alg(options->hash); > u32 flags; > - int i; > int rc; > > - for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > - if (options->hash == tpm2_hash_map[i].crypto_id) { > - hash = tpm2_hash_map[i].tpm_id; > - break; > - } > - } > - > - if (i == ARRAY_SIZE(tpm2_hash_map)) > + if (hash < 0) > return -EINVAL; > > if (!options->keyhandle) > -- > 2.35.3 > >
On Mon May 27, 2024 at 6:45 AM EEST, Ben Boeckel wrote: > On Fri, May 24, 2024 at 09:04:54 -0400, James Bottomley wrote: > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index c17e4efbb2e5..07f532456a0c 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -418,11 +418,61 @@ enum tpm2_session_attributes { > > TPM2_SA_AUDIT = BIT(7), > > }; > > > > -struct tpm2_hash { > > +static const struct { > > unsigned int crypto_id; > > unsigned int tpm_id; > > +} tpm2_hash_map[] = { > > + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > > + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > > + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > > + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > > + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > > }; > > > > +/** > > + * tpm2_crypto_to_alg() - convert a crypto hash to a TPM alg id > > Should "alg id" be "algorithm id" everwhere in the docs? tpm2_hash_algorithm_from() would work for me. > > + * > > + * @hash: the crypto subsystem view of the hash It is an instance of &hash_algo not "crypto subsystem view of the hash". > > + * > > + * Return: TPM algorithm id or -1 if no mapping was found. > > + */ > > +static inline int tpm2_crypto_to_alg(int hash) > > How about naming this `crypto_id`? It really should be @hash_info, which an instance of &hash_info. Despite comments, this patch set will be ignored up until hmac encryption needs no active attention and asymmetric keys have been landed. BR, Jarkko
Now I bandwidth to give this the first round. On Fri May 24, 2024 at 4:04 PM EEST, James Bottomley wrote: > linux crypto and the TPM use different numeric algorithm identifiers > for hash (and other) functions. The conversion array for this already Please use exact names i.e. conversion is between enum tpm2_algorithms and enum hash_info. Much easier to lookup later on. > exists in two separate places. The new policy sessions code would > have to add a third copy, so instead of increasing the duplication, > move the definition to a single consolidated place in tpm.h so the > policy code can use it as is. "the new policy session code" is not an artifact. Instead, please say what needs the third instance and why. I don't consume white paper text. I'm fine if you just use merging two redundant copies together, with no reference to the third copy. In this form this unconditional NAK given that I have zero idea what the third copy is and where it is located. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > --- > drivers/char/tpm/tpm2-cmd.c | 8 ---- > include/linux/tpm.h | 52 ++++++++++++++++++++++- > security/keys/trusted-keys/trusted_tpm2.c | 20 +-------- > 3 files changed, 53 insertions(+), 27 deletions(-) > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index 0cdf892ec2a7..f4428e715dd8 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -14,14 +14,6 @@ > #include "tpm.h" > #include <crypto/hash_info.h> > > -static struct tpm2_hash tpm2_hash_map[] = { > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > -}; > - > int tpm2_get_timeouts(struct tpm_chip *chip) > { > /* Fixed timeouts for TPM2 */ > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index c17e4efbb2e5..07f532456a0c 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -418,11 +418,61 @@ enum tpm2_session_attributes { > TPM2_SA_AUDIT = BIT(7), > }; > > -struct tpm2_hash { > +static const struct { > unsigned int crypto_id; > unsigned int tpm_id; > +} tpm2_hash_map[] = { > + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > }; > > +/** > + * tpm2_crypto_to_alg() - convert a crypto hash to a TPM alg id > + * > + * @hash: the crypto subsystem view of the hash > + * > + * Return: TPM algorithm id or -1 if no mapping was found. > + */ > +static inline int tpm2_crypto_to_alg(int hash) > +{ > + int i; > + int tpm_alg = -1; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + if (hash == tpm2_hash_map[i].crypto_id) { > + tpm_alg = tpm2_hash_map[i].tpm_id; > + break; > + } > + } > + > + return tpm_alg; > +} > + > +/** > + * tpm2_alg_to_crypto() - convert a TPM alg id to a crypto hash > + * > + * @hash: the TPM alg id view of the hash > + * > + * Return: TPM algorithm id or -1 if no mapping was found. > + */ > +static inline int tpm2_alg_to_crypto(int hash) > +{ > + int i; > + int crypto_hash = -1; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + if (hash == tpm2_hash_map[i].tpm_id) { > + crypto_hash = tpm2_hash_map[i].crypto_id; > + break; > + } > + } > + > + return crypto_hash; > +} > + > int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); > void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); > int tpm_buf_init_sized(struct tpm_buf *buf); > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > index dfeec06301ce..94ff9ccae66e 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -18,14 +18,6 @@ > > #include "tpm2key.asn1.h" > > -static struct tpm2_hash tpm2_hash_map[] = { > - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, > - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, > - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, > - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, > - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, > -}; > - > static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 }; > > static int tpm2_key_encode(struct trusted_key_payload *payload, > @@ -231,19 +223,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > off_t offset = TPM_HEADER_SIZE; > struct tpm_buf buf, sized; > int blob_len = 0; > - u32 hash; > + int hash = tpm2_crypto_to_alg(options->hash); Please use reverse christmas tree order. > u32 flags; > - int i; > int rc; > > - for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > - if (options->hash == tpm2_hash_map[i].crypto_id) { > - hash = tpm2_hash_map[i].tpm_id; > - break; > - } > - } > - > - if (i == ARRAY_SIZE(tpm2_hash_map)) > + if (hash < 0) > return -EINVAL; > > if (!options->keyhandle) BR, Jarkko
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 0cdf892ec2a7..f4428e715dd8 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -14,14 +14,6 @@ #include "tpm.h" #include <crypto/hash_info.h> -static struct tpm2_hash tpm2_hash_map[] = { - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, -}; - int tpm2_get_timeouts(struct tpm_chip *chip) { /* Fixed timeouts for TPM2 */ diff --git a/include/linux/tpm.h b/include/linux/tpm.h index c17e4efbb2e5..07f532456a0c 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -418,11 +418,61 @@ enum tpm2_session_attributes { TPM2_SA_AUDIT = BIT(7), }; -struct tpm2_hash { +static const struct { unsigned int crypto_id; unsigned int tpm_id; +} tpm2_hash_map[] = { + {HASH_ALGO_SHA1, TPM_ALG_SHA1}, + {HASH_ALGO_SHA256, TPM_ALG_SHA256}, + {HASH_ALGO_SHA384, TPM_ALG_SHA384}, + {HASH_ALGO_SHA512, TPM_ALG_SHA512}, + {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, }; +/** + * tpm2_crypto_to_alg() - convert a crypto hash to a TPM alg id + * + * @hash: the crypto subsystem view of the hash + * + * Return: TPM algorithm id or -1 if no mapping was found. + */ +static inline int tpm2_crypto_to_alg(int hash) +{ + int i; + int tpm_alg = -1; + + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { + if (hash == tpm2_hash_map[i].crypto_id) { + tpm_alg = tpm2_hash_map[i].tpm_id; + break; + } + } + + return tpm_alg; +} + +/** + * tpm2_alg_to_crypto() - convert a TPM alg id to a crypto hash + * + * @hash: the TPM alg id view of the hash + * + * Return: TPM algorithm id or -1 if no mapping was found. + */ +static inline int tpm2_alg_to_crypto(int hash) +{ + int i; + int crypto_hash = -1; + + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { + if (hash == tpm2_hash_map[i].tpm_id) { + crypto_hash = tpm2_hash_map[i].crypto_id; + break; + } + } + + return crypto_hash; +} + int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); int tpm_buf_init_sized(struct tpm_buf *buf); diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index dfeec06301ce..94ff9ccae66e 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -18,14 +18,6 @@ #include "tpm2key.asn1.h" -static struct tpm2_hash tpm2_hash_map[] = { - {HASH_ALGO_SHA1, TPM_ALG_SHA1}, - {HASH_ALGO_SHA256, TPM_ALG_SHA256}, - {HASH_ALGO_SHA384, TPM_ALG_SHA384}, - {HASH_ALGO_SHA512, TPM_ALG_SHA512}, - {HASH_ALGO_SM3_256, TPM_ALG_SM3_256}, -}; - static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 }; static int tpm2_key_encode(struct trusted_key_payload *payload, @@ -231,19 +223,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip, off_t offset = TPM_HEADER_SIZE; struct tpm_buf buf, sized; int blob_len = 0; - u32 hash; + int hash = tpm2_crypto_to_alg(options->hash); u32 flags; - int i; int rc; - for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { - if (options->hash == tpm2_hash_map[i].crypto_id) { - hash = tpm2_hash_map[i].tpm_id; - break; - } - } - - if (i == ARRAY_SIZE(tpm2_hash_map)) + if (hash < 0) return -EINVAL; if (!options->keyhandle)
linux crypto and the TPM use different numeric algorithm identifiers for hash (and other) functions. The conversion array for this already exists in two separate places. The new policy sessions code would have to add a third copy, so instead of increasing the duplication, move the definition to a single consolidated place in tpm.h so the policy code can use it as is. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- drivers/char/tpm/tpm2-cmd.c | 8 ---- include/linux/tpm.h | 52 ++++++++++++++++++++++- security/keys/trusted-keys/trusted_tpm2.c | 20 +-------- 3 files changed, 53 insertions(+), 27 deletions(-)