Re: [PATCH v5] crypto: rsa - return raw integers for the ASN.1 parser

[Herbert Xu]

On Tue, Jun 14, 2016 at 04:14:58PM +0300, Tudor Ambarus wrote:
> Return the raw key with no other processing so that the caller
> can copy it or MPI parse it, etc.
> 
> The scope is to have only one ANS.1 parser for all RSA
> implementations.
> 
> Update the RSA software implementation so that it does
> the MPI conversion on top.
> 
> Signed-off-by: Tudor Ambarus 

Patch applied.  Thanks for persisting with this.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v5] crypto: rsa - return raw integers for the ASN.1 parser

[Tudor-Dan Ambarus]

Hi Stephan,

> But then I need to refine my question: isn't rsa_parse_priv_key allocating
> the
> MPIs (at least rsa_parse_priv_key seems to hint to that considering the
> error
> code path)? So, shouldn't the MPIs be freed here with free_mpis()? This
> would
> apply to parse_pub_key too.

rsa_parse_priv_key is not allocating the MPIs, mpi_read_raw_data is.

MPIs are freed on error path. On success, they are freed when exiting
the tfm or when setting a new key.

Thanks,
ta
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] crypto: rsa - return raw integers for the ASN.1 parser

[Stephan Mueller]

Am Dienstag, 14. Juni 2016, 21:38:06 schrieb Herbert Xu:

Hi Herbert,

> On Tue, Jun 14, 2016 at 03:20:06PM +0200, Stephan Mueller wrote:
> > memzero_explicit(raw_key) should be added here in success and failure code
> > paths.
> 
> The raw_key is just a bunch of pointers, do we really need to
> zero it?

You are correct.

But then I need to refine my question: isn't rsa_parse_priv_key allocating the 
MPIs (at least rsa_parse_priv_key seems to hint to that considering the error 
code path)? So, shouldn't the MPIs be freed here with free_mpis()? This would 
apply to parse_pub_key too.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] crypto: rsa - return raw integers for the ASN.1 parser

[Herbert Xu]

On Tue, Jun 14, 2016 at 03:20:06PM +0200, Stephan Mueller wrote:
>
> memzero_explicit(raw_key) should be added here in success and failure code 
> paths.

The raw_key is just a bunch of pointers, do we really need to
zero it?

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] crypto: rsa - return raw integers for the ASN.1 parser

[Stephan Mueller]

Am Dienstag, 14. Juni 2016, 16:14:58 schrieb Tudor Ambarus:

Hi Tudor,

>  static int rsa_set_priv_key(struct crypto_akcipher *tfm, const void *key,
>   unsigned int keylen)
>  {
> - struct rsa_key *pkey = akcipher_tfm_ctx(tfm);
> + struct rsa_mpi_key *mpi_key = akcipher_tfm_ctx(tfm);
> + struct rsa_key raw_key = {0};
>   int ret;
> 
> - ret = rsa_parse_priv_key(pkey, key, keylen);
> + /* Free the old MPI key if any */
> + rsa_free_mpi_key(mpi_key);
> +
> + ret = rsa_parse_priv_key(_key, key, keylen);
>   if (ret)
>   return ret;
> 
> - if (rsa_check_key_length(mpi_get_size(pkey->n) << 3)) {
> - rsa_free_key(pkey);
> - ret = -EINVAL;
> + mpi_key->d = mpi_read_raw_data(raw_key.d, raw_key.d_sz);
> + if (!mpi_key->d)
> + goto err;
> +
> + mpi_key->e = mpi_read_raw_data(raw_key.e, raw_key.e_sz);
> + if (!mpi_key->e)
> + goto err;
> +
> + mpi_key->n = mpi_read_raw_data(raw_key.n, raw_key.n_sz);
> + if (!mpi_key->n)
> + goto err;
> +
> + if (rsa_check_key_length(mpi_get_size(mpi_key->n) << 3)) {
> + rsa_free_mpi_key(mpi_key);
> + return -EINVAL;
>   }
> - return ret;
> +
> + return 0;
> +
> +err:
> + rsa_free_mpi_key(mpi_key);
> + return -ENOMEM;
>  }

memzero_explicit(raw_key) should be added here in success and failure code 
paths.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html