2024-05-23 21:39:33

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

On Fri May 24, 2024 at 12:25 AM EEST, Jarkko Sakkinen wrote:
> + /*
> + * ABI requires this according include/crypto/akcipher.h, which says
> + * that there is epilogue with algorithm OID and parameters length.
> + * Neither size nor semantics is documented *anywhere*, and there's no
> + * struct to hold them.
> + *
> + * So zeroing out the last eight bytes after the key blob seems like the
> + * best bet, given no better (or any) information. The size of the
> + * parameters (two u32's) was found from crypto/asymmetric/public_key.c.
> + */
> + memset(work, 0, 8);

This is a mystery (or nightmare).

BR, Jarkko


2024-05-23 21:53:31

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

On Fri May 24, 2024 at 12:39 AM EEST, Jarkko Sakkinen wrote:
> On Fri May 24, 2024 at 12:25 AM EEST, Jarkko Sakkinen wrote:
> > + /*
> > + * ABI requires this according include/crypto/akcipher.h, which says
> > + * that there is epilogue with algorithm OID and parameters length.
> > + * Neither size nor semantics is documented *anywhere*, and there's no
> > + * struct to hold them.
> > + *
> > + * So zeroing out the last eight bytes after the key blob seems like the
> > + * best bet, given no better (or any) information. The size of the
> > + * parameters (two u32's) was found from crypto/asymmetric/public_key.c.
> > + */
> > + memset(work, 0, 8);
>
> This is a mystery (or nightmare).

This is from akchiper_alg documentation:

* @set_pub_key: Function invokes the algorithm specific set public key
* function, which knows how to decode and interpret
* the BER encoded public key and parameters

No struct, no size information and no description what they are used for.

Can we get these properly documented? My documentation at the moment
is grep and kprobes, literally.

BR, Jarkko

2024-05-23 22:03:29

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] keys: asymmetric: ASYMMETRIC_TPM2_KEY_RSA_SUBTYPE

On Fri May 24, 2024 at 12:52 AM EEST, Jarkko Sakkinen wrote:
> On Fri May 24, 2024 at 12:39 AM EEST, Jarkko Sakkinen wrote:
> > On Fri May 24, 2024 at 12:25 AM EEST, Jarkko Sakkinen wrote:
> > > + /*
> > > + * ABI requires this according include/crypto/akcipher.h, which says
> > > + * that there is epilogue with algorithm OID and parameters length.
> > > + * Neither size nor semantics is documented *anywhere*, and there's no
> > > + * struct to hold them.
> > > + *
> > > + * So zeroing out the last eight bytes after the key blob seems like the
> > > + * best bet, given no better (or any) information. The size of the
> > > + * parameters (two u32's) was found from crypto/asymmetric/public_key.c.
> > > + */
> > > + memset(work, 0, 8);
> >
> > This is a mystery (or nightmare).
>
> This is from akchiper_alg documentation:
>
> * @set_pub_key: Function invokes the algorithm specific set public key
> * function, which knows how to decode and interpret
> * the BER encoded public key and parameters
>
> No struct, no size information and no description what they are used for.
>
> Can we get these properly documented? My documentation at the moment
> is grep and kprobes, literally.

That said, zero issues with the interface, just pointing out the
part that is not right, and should be fixed.

I mean I have three layers: this, rsa-pcks1 and rsa. How I can be
sure that either of two layers below never ever up until sun melts
will do any changes that would break, with the data that I put
there? Is this a contract that will hold forever?

This is concerning so I have to point this out.

BR, Jarkko