* [PATCH 0/3] KEYS: fixes for asym_tpm keys
@ 2022-01-13 23:54 Eric Biggers
2022-01-13 23:54 ` [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters() Eric Biggers
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Eric Biggers @ 2022-01-13 23:54 UTC (permalink / raw)
To: keyrings, David Howells, Jarkko Sakkinen
Cc: Denis Kenzior, Marcel Holtmann, James Morris, linux-crypto
This series fixes some bugs in asym_tpm.c.
Eric Biggers (3):
KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
KEYS: asym_tpm: fix incorrect comment
KEYS: asym_tpm: rename derive_pub_key()
crypto/asymmetric_keys/asym_tpm.c | 44 +++++++++++++++++++------------
1 file changed, 27 insertions(+), 17 deletions(-)
base-commit: feb7a43de5ef625ad74097d8fd3481d5dbc06a59
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-13 23:54 [PATCH 0/3] KEYS: fixes for asym_tpm keys Eric Biggers
@ 2022-01-13 23:54 ` Eric Biggers
2022-01-15 21:40 ` Jarkko Sakkinen
2022-01-13 23:54 ` [PATCH 2/3] KEYS: asym_tpm: fix incorrect comment Eric Biggers
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2022-01-13 23:54 UTC (permalink / raw)
To: keyrings, David Howells, Jarkko Sakkinen
Cc: Denis Kenzior, Marcel Holtmann, James Morris, linux-crypto, stable
From: Eric Biggers <ebiggers@google.com>
extract_key_parameters() can read past the end of the input buffer due
to buggy and missing bounds checks. Fix it as follows:
- Before reading each key length field, verify that there are at least 4
bytes remaining.
- Avoid integer overflows when validating size fields; 'sz + 12' and
'4 + sz' overflowed if 'sz' is near U32_MAX.
- Before saving the pointer to the public key, check that it doesn't run
past the end of the buffer.
Fixes: f8c54e1ac4b8 ("KEYS: asym_tpm: extract key size & public key [ver #2]")
Cc: <stable@vger.kernel.org> # v4.20+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/asymmetric_keys/asym_tpm.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c
index 0959613560b9..60d20d44c885 100644
--- a/crypto/asymmetric_keys/asym_tpm.c
+++ b/crypto/asymmetric_keys/asym_tpm.c
@@ -814,7 +814,6 @@ static int extract_key_parameters(struct tpm_key *tk)
{
const void *cur = tk->blob;
uint32_t len = tk->blob_len;
- const void *pub_key;
uint32_t sz;
uint32_t key_len;
@@ -845,14 +844,14 @@ static int extract_key_parameters(struct tpm_key *tk)
return -EBADMSG;
sz = get_unaligned_be32(cur + 8);
- if (len < sz + 12)
- return -EBADMSG;
/* Move to TPM_RSA_KEY_PARMS */
- len -= 12;
cur += 12;
+ len -= 12;
/* Grab the RSA key length */
+ if (len < 4)
+ return -EBADMSG;
key_len = get_unaligned_be32(cur);
switch (key_len) {
@@ -866,29 +865,36 @@ static int extract_key_parameters(struct tpm_key *tk)
}
/* Move just past TPM_KEY_PARMS */
+ if (len < sz)
+ return -EBADMSG;
cur += sz;
len -= sz;
if (len < 4)
return -EBADMSG;
-
sz = get_unaligned_be32(cur);
- if (len < 4 + sz)
- return -EBADMSG;
+ cur += 4;
+ len -= 4;
/* Move to TPM_STORE_PUBKEY */
- cur += 4 + sz;
- len -= 4 + sz;
+ if (len < sz)
+ return -EBADMSG;
+ cur += sz;
+ len -= sz;
/* Grab the size of the public key, it should jive with the key size */
+ if (len < 4)
+ return -EBADMSG;
sz = get_unaligned_be32(cur);
+ cur += 4;
+ len -= 4;
if (sz > 256)
return -EINVAL;
-
- pub_key = cur + 4;
+ if (len < sz)
+ return -EBADMSG;
tk->key_len = key_len;
- tk->pub_key = pub_key;
+ tk->pub_key = cur;
tk->pub_key_len = sz;
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] KEYS: asym_tpm: fix incorrect comment
2022-01-13 23:54 [PATCH 0/3] KEYS: fixes for asym_tpm keys Eric Biggers
2022-01-13 23:54 ` [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters() Eric Biggers
@ 2022-01-13 23:54 ` Eric Biggers
2022-01-15 19:12 ` Jarkko Sakkinen
2022-01-13 23:54 ` [PATCH 3/3] KEYS: asym_tpm: rename derive_pub_key() Eric Biggers
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2022-01-13 23:54 UTC (permalink / raw)
To: keyrings, David Howells, Jarkko Sakkinen
Cc: Denis Kenzior, Marcel Holtmann, James Morris, linux-crypto
From: Eric Biggers <ebiggers@google.com>
tpm_key_create() doesn't actually load the key into the TPM. Fix the
comment to describe what the function does.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/asymmetric_keys/asym_tpm.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c
index 60d20d44c885..2e365a221fbe 100644
--- a/crypto/asymmetric_keys/asym_tpm.c
+++ b/crypto/asymmetric_keys/asym_tpm.c
@@ -900,7 +900,11 @@ static int extract_key_parameters(struct tpm_key *tk)
return 0;
}
-/* Given the blob, parse it and load it into the TPM */
+/*
+ * Verify that a supported TPM is present, then parse the key blob. We don't
+ * actually load the key into the TPM here; that happens only for the actual
+ * sign and decrypt operations.
+ */
struct tpm_key *tpm_key_create(const void *blob, uint32_t blob_len)
{
int r;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] KEYS: asym_tpm: rename derive_pub_key()
2022-01-13 23:54 [PATCH 0/3] KEYS: fixes for asym_tpm keys Eric Biggers
2022-01-13 23:54 ` [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters() Eric Biggers
2022-01-13 23:54 ` [PATCH 2/3] KEYS: asym_tpm: fix incorrect comment Eric Biggers
@ 2022-01-13 23:54 ` Eric Biggers
2022-01-15 19:09 ` Jarkko Sakkinen
2022-01-14 14:54 ` [PATCH 0/3] KEYS: fixes for asym_tpm keys Denis Kenzior
2022-01-15 21:42 ` Jarkko Sakkinen
4 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2022-01-13 23:54 UTC (permalink / raw)
To: keyrings, David Howells, Jarkko Sakkinen
Cc: Denis Kenzior, Marcel Holtmann, James Morris, linux-crypto
From: Eric Biggers <ebiggers@google.com>
derive_pub_key() doesn't actually derive a key; it just formats one.
Rename it accordingly.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/asymmetric_keys/asym_tpm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c
index 2e365a221fbe..d2e0036d34a5 100644
--- a/crypto/asymmetric_keys/asym_tpm.c
+++ b/crypto/asymmetric_keys/asym_tpm.c
@@ -336,7 +336,7 @@ static inline uint8_t *encode_tag_length(uint8_t *buf, uint8_t tag,
return buf + 3;
}
-static uint32_t derive_pub_key(const void *pub_key, uint32_t len, uint8_t *buf)
+static uint32_t format_pub_key(const void *pub_key, uint32_t len, uint8_t *buf)
{
uint8_t *cur = buf;
uint32_t n_len = definite_length(len) + 1 + len + 1;
@@ -409,7 +409,7 @@ static int tpm_key_query(const struct kernel_pkey_params *params,
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- der_pub_key_len = derive_pub_key(tk->pub_key, tk->pub_key_len,
+ der_pub_key_len = format_pub_key(tk->pub_key, tk->pub_key_len,
der_pub_key);
ret = crypto_akcipher_set_pub_key(tfm, der_pub_key, der_pub_key_len);
@@ -463,7 +463,7 @@ static int tpm_key_encrypt(struct tpm_key *tk,
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- der_pub_key_len = derive_pub_key(tk->pub_key, tk->pub_key_len,
+ der_pub_key_len = format_pub_key(tk->pub_key, tk->pub_key_len,
der_pub_key);
ret = crypto_akcipher_set_pub_key(tfm, der_pub_key, der_pub_key_len);
@@ -758,7 +758,7 @@ static int tpm_key_verify_signature(const struct key *key,
if (IS_ERR(tfm))
return PTR_ERR(tfm);
- der_pub_key_len = derive_pub_key(tk->pub_key, tk->pub_key_len,
+ der_pub_key_len = format_pub_key(tk->pub_key, tk->pub_key_len,
der_pub_key);
ret = crypto_akcipher_set_pub_key(tfm, der_pub_key, der_pub_key_len);
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] KEYS: fixes for asym_tpm keys
2022-01-13 23:54 [PATCH 0/3] KEYS: fixes for asym_tpm keys Eric Biggers
` (2 preceding siblings ...)
2022-01-13 23:54 ` [PATCH 3/3] KEYS: asym_tpm: rename derive_pub_key() Eric Biggers
@ 2022-01-14 14:54 ` Denis Kenzior
2022-01-15 21:42 ` Jarkko Sakkinen
4 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2022-01-14 14:54 UTC (permalink / raw)
To: Eric Biggers, keyrings, David Howells, Jarkko Sakkinen
Cc: Marcel Holtmann, James Morris, linux-crypto
Hi Eric,
On 1/13/22 17:54, Eric Biggers wrote:
> This series fixes some bugs in asym_tpm.c.
>
> Eric Biggers (3):
> KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
> KEYS: asym_tpm: fix incorrect comment
> KEYS: asym_tpm: rename derive_pub_key()
>
> crypto/asymmetric_keys/asym_tpm.c | 44 +++++++++++++++++++------------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
>
> base-commit: feb7a43de5ef625ad74097d8fd3481d5dbc06a59
>
For the series:
Reviewed-By: Denis Kenzior <denkenz@gmail.com>
Regards,
-Denis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] KEYS: asym_tpm: rename derive_pub_key()
2022-01-13 23:54 ` [PATCH 3/3] KEYS: asym_tpm: rename derive_pub_key() Eric Biggers
@ 2022-01-15 19:09 ` Jarkko Sakkinen
0 siblings, 0 replies; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-01-15 19:09 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto
On Thu, Jan 13, 2022 at 03:54:40PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> derive_pub_key() doesn't actually derive a key; it just formats one.
> Rename it accordingly.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
/Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] KEYS: asym_tpm: fix incorrect comment
2022-01-13 23:54 ` [PATCH 2/3] KEYS: asym_tpm: fix incorrect comment Eric Biggers
@ 2022-01-15 19:12 ` Jarkko Sakkinen
0 siblings, 0 replies; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-01-15 19:12 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto
On Thu, Jan 13, 2022 at 03:54:39PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> tpm_key_create() doesn't actually load the key into the TPM. Fix the
> comment to describe what the function does.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
This asym_tpm has not been properly reviewed as far as I can tell.
For starters, I do not get who needed new TPM 1.x features in 2018...
It's long after SHA1 was declared as insecure and world was mostly
settled with TPM2.
BR, Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-13 23:54 ` [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters() Eric Biggers
@ 2022-01-15 21:40 ` Jarkko Sakkinen
2022-01-19 0:59 ` Eric Biggers
0 siblings, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-01-15 21:40 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto, stable
On Thu, Jan 13, 2022 at 03:54:38PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> extract_key_parameters() can read past the end of the input buffer due
> to buggy and missing bounds checks. Fix it as follows:
>
> - Before reading each key length field, verify that there are at least 4
> bytes remaining.
Maybe start with a "Key length is described as an unsigned 32-bit integer
in the TPM header". Just for clarity.
>
> - Avoid integer overflows when validating size fields; 'sz + 12' and
> '4 + sz' overflowed if 'sz' is near U32_MAX.
So we have a struct tpm_header in include/linux/tpm.h. It would be way
more informative to use sizeof(struct tpm_header) than number 12, even
if the patch does not otherwise use the struct. It tells what it is, 12
does not.
> - Before saving the pointer to the public key, check that it doesn't run
> past the end of the buffer.
>
> Fixes: f8c54e1ac4b8 ("KEYS: asym_tpm: extract key size & public key [ver #2]")
> Cc: <stable@vger.kernel.org> # v4.20+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
BR, Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] KEYS: fixes for asym_tpm keys
2022-01-13 23:54 [PATCH 0/3] KEYS: fixes for asym_tpm keys Eric Biggers
` (3 preceding siblings ...)
2022-01-14 14:54 ` [PATCH 0/3] KEYS: fixes for asym_tpm keys Denis Kenzior
@ 2022-01-15 21:42 ` Jarkko Sakkinen
4 siblings, 0 replies; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-01-15 21:42 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto
On Thu, Jan 13, 2022 at 03:54:37PM -0800, Eric Biggers wrote:
> This series fixes some bugs in asym_tpm.c.
>
> Eric Biggers (3):
> KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
> KEYS: asym_tpm: fix incorrect comment
> KEYS: asym_tpm: rename derive_pub_key()
>
> crypto/asymmetric_keys/asym_tpm.c | 44 +++++++++++++++++++------------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
>
> base-commit: feb7a43de5ef625ad74097d8fd3481d5dbc06a59
> --
> 2.34.1
>
I really appreciate your effort to go and fix this key type. Thank you.
BR, Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-15 21:40 ` Jarkko Sakkinen
@ 2022-01-19 0:59 ` Eric Biggers
2022-01-26 14:21 ` Jarkko Sakkinen
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2022-01-19 0:59 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto, stable
On Sat, Jan 15, 2022 at 11:40:48PM +0200, Jarkko Sakkinen wrote:
> >
> > - Avoid integer overflows when validating size fields; 'sz + 12' and
> > '4 + sz' overflowed if 'sz' is near U32_MAX.
>
> So we have a struct tpm_header in include/linux/tpm.h. It would be way
> more informative to use sizeof(struct tpm_header) than number 12, even
> if the patch does not otherwise use the struct. It tells what it is, 12
> does not.
I don't think that would be an improvement, given that the code is using
hard-coded offsets. If it's reading 4 bytes from cur + 8, it's much easier to
understand that it needs 12 bytes than 'sizeof(struct tpm_header)' bytes.
I'd certainly encourage whoever is maintaining this code to change it to use
structs instead, but that's not what this patch is meant to do.
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-19 0:59 ` Eric Biggers
@ 2022-01-26 14:21 ` Jarkko Sakkinen
2022-01-26 14:22 ` Jarkko Sakkinen
0 siblings, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-01-26 14:21 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto, stable
On Tue, Jan 18, 2022 at 04:59:47PM -0800, Eric Biggers wrote:
> On Sat, Jan 15, 2022 at 11:40:48PM +0200, Jarkko Sakkinen wrote:
> > >
> > > - Avoid integer overflows when validating size fields; 'sz + 12' and
> > > '4 + sz' overflowed if 'sz' is near U32_MAX.
> >
> > So we have a struct tpm_header in include/linux/tpm.h. It would be way
> > more informative to use sizeof(struct tpm_header) than number 12, even
> > if the patch does not otherwise use the struct. It tells what it is, 12
> > does not.
>
> I don't think that would be an improvement, given that the code is using
> hard-coded offsets. If it's reading 4 bytes from cur + 8, it's much easier to
> understand that it needs 12 bytes than 'sizeof(struct tpm_header)' bytes.
>
> I'd certainly encourage whoever is maintaining this code to change it to use
> structs instead, but that's not what this patch is meant to do.
I would consider dropping asym_tpm as it has no practical use cases
existing.
/Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-26 14:21 ` Jarkko Sakkinen
@ 2022-01-26 14:22 ` Jarkko Sakkinen
2022-01-28 19:00 ` Eric Biggers
0 siblings, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-01-26 14:22 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, David Howells, Denis Kenzior, Marcel Holtmann,
James Morris, linux-crypto, stable
On Wed, Jan 26, 2022 at 04:21:53PM +0200, Jarkko Sakkinen wrote:
> On Tue, Jan 18, 2022 at 04:59:47PM -0800, Eric Biggers wrote:
> > On Sat, Jan 15, 2022 at 11:40:48PM +0200, Jarkko Sakkinen wrote:
> > > >
> > > > - Avoid integer overflows when validating size fields; 'sz + 12' and
> > > > '4 + sz' overflowed if 'sz' is near U32_MAX.
> > >
> > > So we have a struct tpm_header in include/linux/tpm.h. It would be way
> > > more informative to use sizeof(struct tpm_header) than number 12, even
> > > if the patch does not otherwise use the struct. It tells what it is, 12
> > > does not.
> >
> > I don't think that would be an improvement, given that the code is using
> > hard-coded offsets. If it's reading 4 bytes from cur + 8, it's much easier to
> > understand that it needs 12 bytes than 'sizeof(struct tpm_header)' bytes.
> >
> > I'd certainly encourage whoever is maintaining this code to change it to use
> > structs instead, but that's not what this patch is meant to do.
>
> I would consider dropping asym_tpm as it has no practical use cases
> existing.
At least I have zero motivation to maintain it as it does not meet
any quality standards and is based on insecure crypto algorithms.
I neither have participated to its review process.
/Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-26 14:22 ` Jarkko Sakkinen
@ 2022-01-28 19:00 ` Eric Biggers
2022-02-08 9:30 ` Jarkko Sakkinen
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2022-01-28 19:00 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: keyrings, Denis Kenzior, David Howells, Marcel Holtmann,
James Morris, linux-crypto, stable
On Wed, Jan 26, 2022 at 04:22:53PM +0200, Jarkko Sakkinen wrote:
> On Wed, Jan 26, 2022 at 04:21:53PM +0200, Jarkko Sakkinen wrote:
> > On Tue, Jan 18, 2022 at 04:59:47PM -0800, Eric Biggers wrote:
> > > On Sat, Jan 15, 2022 at 11:40:48PM +0200, Jarkko Sakkinen wrote:
> > > > >
> > > > > - Avoid integer overflows when validating size fields; 'sz + 12' and
> > > > > '4 + sz' overflowed if 'sz' is near U32_MAX.
> > > >
> > > > So we have a struct tpm_header in include/linux/tpm.h. It would be way
> > > > more informative to use sizeof(struct tpm_header) than number 12, even
> > > > if the patch does not otherwise use the struct. It tells what it is, 12
> > > > does not.
> > >
> > > I don't think that would be an improvement, given that the code is using
> > > hard-coded offsets. If it's reading 4 bytes from cur + 8, it's much easier to
> > > understand that it needs 12 bytes than 'sizeof(struct tpm_header)' bytes.
> > >
> > > I'd certainly encourage whoever is maintaining this code to change it to use
> > > structs instead, but that's not what this patch is meant to do.
> >
> > I would consider dropping asym_tpm as it has no practical use cases
> > existing.
>
> At least I have zero motivation to maintain it as it does not meet
> any quality standards and is based on insecure crypto algorithms.
> I neither have participated to its review process.
Fair enough, I'll send a patch to remove it then.
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters()
2022-01-28 19:00 ` Eric Biggers
@ 2022-02-08 9:30 ` Jarkko Sakkinen
0 siblings, 0 replies; 14+ messages in thread
From: Jarkko Sakkinen @ 2022-02-08 9:30 UTC (permalink / raw)
To: Eric Biggers
Cc: keyrings, Denis Kenzior, David Howells, Marcel Holtmann,
James Morris, linux-crypto, stable
On Fri, Jan 28, 2022 at 11:00:12AM -0800, Eric Biggers wrote:
> On Wed, Jan 26, 2022 at 04:22:53PM +0200, Jarkko Sakkinen wrote:
> > On Wed, Jan 26, 2022 at 04:21:53PM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Jan 18, 2022 at 04:59:47PM -0800, Eric Biggers wrote:
> > > > On Sat, Jan 15, 2022 at 11:40:48PM +0200, Jarkko Sakkinen wrote:
> > > > > >
> > > > > > - Avoid integer overflows when validating size fields; 'sz + 12' and
> > > > > > '4 + sz' overflowed if 'sz' is near U32_MAX.
> > > > >
> > > > > So we have a struct tpm_header in include/linux/tpm.h. It would be way
> > > > > more informative to use sizeof(struct tpm_header) than number 12, even
> > > > > if the patch does not otherwise use the struct. It tells what it is, 12
> > > > > does not.
> > > >
> > > > I don't think that would be an improvement, given that the code is using
> > > > hard-coded offsets. If it's reading 4 bytes from cur + 8, it's much easier to
> > > > understand that it needs 12 bytes than 'sizeof(struct tpm_header)' bytes.
> > > >
> > > > I'd certainly encourage whoever is maintaining this code to change it to use
> > > > structs instead, but that's not what this patch is meant to do.
> > >
> > > I would consider dropping asym_tpm as it has no practical use cases
> > > existing.
> >
> > At least I have zero motivation to maintain it as it does not meet
> > any quality standards and is based on insecure crypto algorithms.
> > I neither have participated to its review process.
>
> Fair enough, I'll send a patch to remove it then.
It is IMHO. I mean having this advertising insecure ways to to do crypto.
Thank you.
PS. My latency is because I've been moving to a new job. It is temporary.
/Jarkko
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-02-08 9:30 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 23:54 [PATCH 0/3] KEYS: fixes for asym_tpm keys Eric Biggers
2022-01-13 23:54 ` [PATCH 1/3] KEYS: asym_tpm: fix buffer overreads in extract_key_parameters() Eric Biggers
2022-01-15 21:40 ` Jarkko Sakkinen
2022-01-19 0:59 ` Eric Biggers
2022-01-26 14:21 ` Jarkko Sakkinen
2022-01-26 14:22 ` Jarkko Sakkinen
2022-01-28 19:00 ` Eric Biggers
2022-02-08 9:30 ` Jarkko Sakkinen
2022-01-13 23:54 ` [PATCH 2/3] KEYS: asym_tpm: fix incorrect comment Eric Biggers
2022-01-15 19:12 ` Jarkko Sakkinen
2022-01-13 23:54 ` [PATCH 3/3] KEYS: asym_tpm: rename derive_pub_key() Eric Biggers
2022-01-15 19:09 ` Jarkko Sakkinen
2022-01-14 14:54 ` [PATCH 0/3] KEYS: fixes for asym_tpm keys Denis Kenzior
2022-01-15 21:42 ` Jarkko Sakkinen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.