From: Jarkko Sakkinen <jarkko@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>,
Mimi Zohar <zohar@linux.ibm.com>,
keyrings@vger.kernel.org, David Howells <dhowells@redhat.com>,
linux-integrity@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] KEYS: trusted_tpm1: Move private functionality out of public header
Date: Sat, 9 Aug 2025 13:38:29 +0300 [thread overview]
Message-ID: <aJclJaSDuF8aIdrx@kernel.org> (raw)
In-Reply-To: <20250805173347.GE1286@sol>
On Tue, Aug 05, 2025 at 10:33:47AM -0700, Eric Biggers wrote:
> On Tue, Aug 05, 2025 at 04:48:27PM +0300, Jarkko Sakkinen wrote:
> > On Thu, Jul 31, 2025 at 02:23:54PM -0700, Eric Biggers wrote:
> > > Move functionality used only by trusted_tpm1.c out of the public header
> > > <keys/trusted_tpm.h>. Specifically, change the exported functions into
> > > static functions, since they are not used outside trusted_tpm1.c, and
> > > move various other definitions and inline functions to trusted_tpm1.c.
> > >
> > > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > > ---
> > > include/keys/trusted_tpm.h | 79 ----------------------
> > > security/keys/trusted-keys/trusted_tpm1.c | 80 ++++++++++++++++++++---
> > > 2 files changed, 72 insertions(+), 87 deletions(-)
> > >
> > > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> > > index a088b33fd0e3b..0fadc6a4f1663 100644
> > > --- a/include/keys/trusted_tpm.h
> > > +++ b/include/keys/trusted_tpm.h
> > > @@ -3,94 +3,15 @@
> > > #define __TRUSTED_TPM_H
> > >
> > > #include <keys/trusted-type.h>
> > > #include <linux/tpm_command.h>
> > >
> > > -/* implementation specific TPM constants */
> > > -#define TPM_SIZE_OFFSET 2
> > > -#define TPM_RETURN_OFFSET 6
> > > -#define TPM_DATA_OFFSET 10
> > > -
> > > -#define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset]))
> > > -#define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset])
> > > -#define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset]))
> > > -
> > > extern struct trusted_key_ops trusted_key_tpm_ops;
> > >
> > > -struct osapsess {
> > > - uint32_t handle;
> > > - unsigned char secret[SHA1_DIGEST_SIZE];
> > > - unsigned char enonce[TPM_NONCE_SIZE];
> > > -};
> > > -
> > > -/* discrete values, but have to store in uint16_t for TPM use */
> > > -enum {
> > > - SEAL_keytype = 1,
> > > - SRK_keytype = 4
> > > -};
> > > -
> > > -int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> > > - unsigned int keylen, unsigned char *h1,
> > > - unsigned char *h2, unsigned int h3, ...);
> > > -int TSS_checkhmac1(unsigned char *buffer,
> > > - const uint32_t command,
> > > - const unsigned char *ononce,
> > > - const unsigned char *key,
> > > - unsigned int keylen, ...);
> > > -
> > > -int trusted_tpm_send(unsigned char *cmd, size_t buflen);
> > > -int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
> > > -
> > > int tpm2_seal_trusted(struct tpm_chip *chip,
> > > struct trusted_key_payload *payload,
> > > struct trusted_key_options *options);
> > > int tpm2_unseal_trusted(struct tpm_chip *chip,
> > > struct trusted_key_payload *payload,
> > > struct trusted_key_options *options);
> > >
> > > -#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,
> > > - 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,
> > > - 16, 1, &s->handle, 4, 0);
> > > - pr_info("secret:\n");
> > > - print_hex_dump(KERN_INFO, "", 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,
> > > - 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> > > -}
> > > -
> > > -static inline void dump_tpm_buf(unsigned char *buf)
> > > -{
> > > - int len;
> > > -
> > > - pr_info("\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)
> > > -{
> > > -}
> > > -#endif
> > > #endif
> > > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > > index 126437459a74d..636acb66a4f69 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > > @@ -22,10 +22,78 @@
> > > #include <keys/trusted_tpm.h>
> > >
> > > static struct tpm_chip *chip;
> > > static struct tpm_digest *digests;
> > >
> > > +/* implementation specific TPM constants */
> > > +#define TPM_SIZE_OFFSET 2
> > > +#define TPM_RETURN_OFFSET 6
> > > +#define TPM_DATA_OFFSET 10
> > > +
> > > +#define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset]))
> > > +#define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset])
> > > +#define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset]))
> > > +
> > > +struct osapsess {
> > > + uint32_t handle;
> > > + unsigned char secret[SHA1_DIGEST_SIZE];
> > > + unsigned char enonce[TPM_NONCE_SIZE];
> > > +};
> > > +
> > > +/* discrete values, but have to store in uint16_t for TPM use */
> > > +enum {
> > > + SEAL_keytype = 1,
> > > + 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,
> > > + 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,
> > > + 16, 1, &s->handle, 4, 0);
> > > + pr_info("secret:\n");
> > > + print_hex_dump(KERN_INFO, "", 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,
> > > + 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> > > +}
> > > +
> > > +static inline void dump_tpm_buf(unsigned char *buf)
> > > +{
> > > + int len;
> > > +
> > > + pr_info("\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)
> > > +{
> > > +}
> > > +#endif
> > > +
> > > static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> > > unsigned int keylen, ...)
> > > {
> > > struct hmac_sha1_ctx hmac_ctx;
> > > va_list argp;
> > > @@ -54,11 +122,11 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> > > }
> > >
> > > /*
> > > * calculate authorization info fields to send to TPM
> > > */
> > > -int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> > > +static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> > > unsigned int keylen, unsigned char *h1,
> > > unsigned char *h2, unsigned int h3, ...)
> > > {
> > > unsigned char paramdigest[SHA1_DIGEST_SIZE];
> > > struct sha1_ctx sha_ctx;
> > > @@ -92,16 +160,15 @@ int TSS_authhmac(unsigned char *digest, const unsigned char *key,
> > > ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
> > > paramdigest, TPM_NONCE_SIZE, h1,
> > > TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
> > > return ret;
> > > }
> > > -EXPORT_SYMBOL_GPL(TSS_authhmac);
> > >
> > > /*
> > > * verify the AUTH1_COMMAND (Seal) result from TPM
> > > */
> > > -int TSS_checkhmac1(unsigned char *buffer,
> > > +static int TSS_checkhmac1(unsigned char *buffer,
> > > const uint32_t command,
> > > const unsigned char *ononce,
> > > const unsigned char *key,
> > > unsigned int keylen, ...)
> > > {
> > > @@ -157,11 +224,10 @@ int TSS_checkhmac1(unsigned char *buffer,
> > >
> > > if (crypto_memneq(testhmac, authdata, SHA1_DIGEST_SIZE))
> > > return -EINVAL;
> > > return 0;
> > > }
> > > -EXPORT_SYMBOL_GPL(TSS_checkhmac1);
> > >
> > > /*
> > > * verify the AUTH2_COMMAND (unseal) result from TPM
> > > */
> > > static int TSS_checkhmac2(unsigned char *buffer,
> > > @@ -242,11 +308,11 @@ static int TSS_checkhmac2(unsigned char *buffer,
> > >
> > > /*
> > > * For key specific tpm requests, we will generate and send our
> > > * own TPM command packets using the drivers send function.
> > > */
> > > -int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > > +static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > > {
> > > struct tpm_buf buf;
> > > int rc;
> > >
> > > if (!chip)
> > > @@ -268,11 +334,10 @@ int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> > > rc = -EPERM;
> > >
> > > tpm_put_ops(chip);
> > > return rc;
> > > }
> > > -EXPORT_SYMBOL_GPL(trusted_tpm_send);
> > >
> > > /*
> > > * Lock a trusted key, by extending a selected PCR.
> > > *
> > > * Prevents a trusted key that is sealed to PCRs from being accessed.
> > > @@ -322,11 +387,11 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> > > }
> > >
> > > /*
> > > * Create an object independent authorisation protocol (oiap) session
> > > */
> > > -int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> > > +static int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> > > {
> > > int ret;
> > >
> > > if (!chip)
> > > return -ENODEV;
> > > @@ -339,11 +404,10 @@ int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
> > > *handle = LOAD32(tb->data, TPM_DATA_OFFSET);
> > > memcpy(nonce, &tb->data[TPM_DATA_OFFSET + sizeof(uint32_t)],
> > > TPM_NONCE_SIZE);
> > > return 0;
> > > }
> > > -EXPORT_SYMBOL_GPL(oiap);
> > >
> > > struct tpm_digests {
> > > unsigned char encauth[SHA1_DIGEST_SIZE];
> > > unsigned char pubauth[SHA1_DIGEST_SIZE];
> > > unsigned char xorwork[SHA1_DIGEST_SIZE * 2];
> > > --
> > > 2.50.1
> > >
> >
> > IMHO, this could followed (as next logical steps):
> >
> > 1. Get rid of LOAD*() (tpm_buf_read*)
> > 2. I think we should delete dump_* given modern times and countless ways
> > to acquire that data (e.g., with eBPF).
> >
>
> Sure, would you be interested in sending a follow-up patch? I don't
> have much interest in this driver myself.
OK fine I'll work on that later!
>
> - Eric
BR, Jarkko
prev parent reply other threads:[~2025-08-09 10:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-31 21:23 [PATCH 0/3] KEYS: trusted_tpm1: HMAC fix and cleanup Eric Biggers
2025-07-31 21:23 ` [PATCH 1/3] KEYS: trusted_tpm1: Compare HMAC values in constant time Eric Biggers
2025-08-05 13:44 ` Jarkko Sakkinen
2025-08-05 17:32 ` Eric Biggers
2025-08-09 10:37 ` Jarkko Sakkinen
2025-08-09 17:21 ` Eric Biggers
2025-07-31 21:23 ` [PATCH 2/3] KEYS: trusted_tpm1: Use SHA-1 library instead of crypto_shash Eric Biggers
2025-08-05 13:45 ` Jarkko Sakkinen
2025-07-31 21:23 ` [PATCH 3/3] KEYS: trusted_tpm1: Move private functionality out of public header Eric Biggers
2025-08-05 13:48 ` Jarkko Sakkinen
2025-08-05 17:33 ` Eric Biggers
2025-08-09 10:38 ` Jarkko Sakkinen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aJclJaSDuF8aIdrx@kernel.org \
--to=jarkko@kernel.org \
--cc=James.Bottomley@hansenpartnership.com \
--cc=dhowells@redhat.com \
--cc=ebiggers@kernel.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).