Keyrings Archive mirror
 help / color / mirror / Atom feed
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "David Howells" <dhowells@redhat.com>
Cc: "James Bottomley" <James.Bottomley@HansenPartnership.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	<linux-integrity@vger.kernel.org>, <keyrings@vger.kernel.org>,
	<Andreas.Fuchs@infineon.com>,
	"James Prestwood" <prestwoj@gmail.com>,
	"David Woodhouse" <dwmw2@infradead.org>,
	"Eric Biggers" <ebiggers@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"open list:CRYPTO API" <linux-crypto@vger.kernel.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"Peter Huewe" <peterhuewe@gmx.de>,
	"Jason Gunthorpe" <jgg@ziepe.ca>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"James Morris" <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"open list:SECURITY SUBSYSTEM"
	<linux-security-module@vger.kernel.org>
Subject: Re: [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver
Date: Wed, 22 May 2024 01:42:14 +0300	[thread overview]
Message-ID: <D1FOOI70OW9N.Y2ATVMAK2QR@kernel.org> (raw)
In-Reply-To: <336755.1716327854@warthog.procyon.org.uk>

On Wed May 22, 2024 at 12:44 AM EEST, David Howells wrote:
> Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> > On Tue May 21, 2024 at 9:18 PM EEST, James Bottomley wrote:
> > ...
> > You don't save a single byte of memory with any constant that dictates
> > the size requirements for multiple modules in two disjoint subsystems.
>
> I think James is just suggesting you replace your limit argument with a
> constant not that you always allocate that amount of memory.  What the limit
> should be, OTOH, is up for discussion, but PAGE_SIZE seems not unreasonable.

When the decoder for ASN.1 was part of trusted keys, the check used to
be:

	if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
		return -EINVAL;

And MAX_BLOB_SIZE is only 512 bytes, which does not fit event 2048 bit
RSA key but that 512 bytes cap seems to be just fine for trusted keys.

So the new check is:

	if (blob_len > max_key_len)
		return -E2BIG;

1. Too big value is not invalid value, thus -E2BIG. It is has also
   shown to be practically useful while testing this key type.
2. tpm2_key_rsa needs up to 8192 bytes for a blob to fit 4096-bit
   RSA key. 

Just saying but there is also primary null key allocated by the driver.
And neither driver uses MAX_BLOB_SiZE. It uses value 8x MAX_BLOB_SIZE
i.e. 4096 bytes so not really following the idea suggested.

Finaly, there is three completely separate algorithms:

- KEYEDHASH (trusted_keys)
- RSA (tpm2_key_rsa)
- ECDSA (driver)§
	
With all this put together it is just common sense to have parametrized
cap value, and it would have no logic at all to treat them unified way.

For tpm2_key_rsa I will define for clarity:

#define TPM2_KEY_RSA_MAX_SIZE 8192

For tpm2_key_ecdsa you would define

#define TPM2_KEY_ECDSA_MAX_SIZE 4096

So yeah, this is how I will proceed because it is really the only
senseful way to proceed.

>
> David

BR, Jarkko

  parent reply	other threads:[~2024-05-21 22:42 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-21  3:16 [PATCH v2 0/6] KEYS: asymmetric: tpm2_key_rsa Jarkko Sakkinen
2024-05-21  3:16 ` [PATCH v2 1/6] crypto: rsa-pkcs1pad: export rsa1_asn_lookup() Jarkko Sakkinen
2024-05-21  3:16 ` [PATCH v2 2/6] lib: Expand asn1_encode_integer() to variable size integers Jarkko Sakkinen
2024-05-21  5:36   ` [EXTERNAL] " Bharat Bhushan
     [not found]     ` < <SN7PR18MB5314CFBD18B011F292809EBFE3EA2@SN7PR18MB5314.namprd18.prod.outlook.com>
2024-05-21  6:21       ` Jarkko Sakkinen
2024-05-21  3:16 ` [PATCH v2 3/6] tpm: Export tpm2_load_context() Jarkko Sakkinen
2024-05-21  3:16 ` [PATCH v2 4/6] KEYS: trusted: Move tpm2_key_decode() to the TPM driver Jarkko Sakkinen
2024-05-21 18:18   ` James Bottomley
2024-05-21 21:17     ` Jarkko Sakkinen
2024-05-21 21:44     ` David Howells
2024-05-21 21:59       ` James Bottomley
2024-05-21 22:45         ` Jarkko Sakkinen
2024-05-21 22:59           ` Jarkko Sakkinen
2024-05-21 22:42       ` Jarkko Sakkinen [this message]
2024-05-21  3:16 ` [PATCH v2 5/6] tpm: tpm2_key: Extend parser to TPM_LoadableKey Jarkko Sakkinen
2024-05-21  5:47   ` [EXTERNAL] " Bharat Bhushan
     [not found]     ` < <SN7PR18MB53140F4341BC441C1C11586EE3EA2@SN7PR18MB5314.namprd18.prod.outlook.com>
2024-05-21  7:13       ` Jarkko Sakkinen
2024-05-21  3:16 ` [PATCH v2 6/6] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE Jarkko Sakkinen
2024-05-21  7:25   ` [EXTERNAL] " Bharat Bhushan
     [not found]     ` < <SN7PR18MB531494159D3996799475209DE3EA2@SN7PR18MB5314.namprd18.prod.outlook.com>
2024-05-21  7:38       ` Jarkko Sakkinen

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=D1FOOI70OW9N.Y2ATVMAK2QR@kernel.org \
    --to=jarkko@kernel.org \
    --cc=Andreas.Fuchs@infineon.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=ebiggers@kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=jgg@ziepe.ca \
    --cc=jmorris@namei.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=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=peterhuewe@gmx.de \
    --cc=prestwoj@gmail.com \
    --cc=serge@hallyn.com \
    --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 read-only IMAP folder(s) and NNTP newsgroup(s).