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@xxxxxxxxxxxxxxxxxxxxx>
> ---
> 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
