Re: [PATCH v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-24 Thread Herbert Xu 
On Mon, Apr 25, 2016 at 05:54:44AM +, Tudor-Dan Ambarus wrote:
>
> Do you want me to address some suggestions, or do you intend to apply this 
> patch set?

Please ensure that we have exactly one parser for the RSA keys.
It should just return the raw integer with no other processing.

The software RSA implementation can then do the MPI conversion
on top.

Cheers,
-- 
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 v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-24 Thread Tudor-Dan Ambarus 
Hi Herbert,

> > On Fri, Apr 15, 2016 at 02:32:42PM +, Tudor-Dan Ambarus wrote:
> > > > On Wed, Apr 06, 2016 at 04:37:04PM +0300, Tudor Ambarus wrote:
> > > > > This approach has the advantage that users can select specific
> > > > > parser actions by using a general parser with function pointers
> > > > > to specific actions.
> > > >
> > > > I don't understand why we need different parsing functions in the
> > > > first place.  Can't they just return raw integers always?
> > > >
> > > > You can then trivially convert the raw integers to MPI, no?
> > >
> > > We need different parsing functions so that we don't allocate duplicate
> > buffers for the same data.
> > >
> > > You need to allocate buffers when getting the raw integers and you need
> > to allocate other (duplicate) buffers when converting the raw integers to
> > MPI.
> > >
> > > Using the proposed API each user can select the format of data he
> wants,
> > eliminating the need of a double conversion (with its drawbacks:
> duplicate
> > buffers, unnecessary cycles).
> >
> > The double allocation only happens with software RSA, right? I
> > don't really think that is going to matter considering how slow
> > it is, no?
> 
> The double allocation happens only with software RSA. I can't estimate the
> performance hit for the double conversion. Naturally is to parse the data
> only once. Plus, my suggestion is simple, faster and doesn't waste
> resources.

Do you want me to address some suggestions, or do you intend to apply this 
patch set?

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 v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-15 Thread Tudor-Dan Ambarus 
> On Fri, Apr 15, 2016 at 02:32:42PM +, Tudor-Dan Ambarus wrote:
> > > On Wed, Apr 06, 2016 at 04:37:04PM +0300, Tudor Ambarus wrote:
> > > > This approach has the advantage that users can select specific
> > > > parser actions by using a general parser with function pointers
> > > > to specific actions.
> > >
> > > I don't understand why we need different parsing functions in the
> > > first place.  Can't they just return raw integers always?
> > >
> > > You can then trivially convert the raw integers to MPI, no?
> >
> > We need different parsing functions so that we don't allocate duplicate
> buffers for the same data.
> >
> > You need to allocate buffers when getting the raw integers and you need
> to allocate other (duplicate) buffers when converting the raw integers to
> MPI.
> >
> > Using the proposed API each user can select the format of data he wants,
> eliminating the need of a double conversion (with its drawbacks: duplicate
> buffers, unnecessary cycles).
> 
> The double allocation only happens with software RSA, right? I
> don't really think that is going to matter considering how slow
> it is, no?

The double allocation happens only with software RSA. I can't estimate the 
performance hit for the double conversion. Naturally is to parse the data only 
once. Plus, my suggestion is simple, faster and doesn't waste resources.

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 v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-15 Thread Herbert Xu 
On Fri, Apr 15, 2016 at 02:32:42PM +, Tudor-Dan Ambarus wrote:
> > On Wed, Apr 06, 2016 at 04:37:04PM +0300, Tudor Ambarus wrote:
> > > This approach has the advantage that users can select specific
> > > parser actions by using a general parser with function pointers
> > > to specific actions.
> > 
> > I don't understand why we need different parsing functions in the
> > first place.  Can't they just return raw integers always?
> > 
> > You can then trivially convert the raw integers to MPI, no?
> 
> We need different parsing functions so that we don't allocate duplicate 
> buffers for the same data.
> 
> You need to allocate buffers when getting the raw integers and you need to 
> allocate other (duplicate) buffers when converting the raw integers to MPI.
> 
> Using the proposed API each user can select the format of data he wants, 
> eliminating the need of a double conversion (with its drawbacks: duplicate 
> buffers, unnecessary cycles).

The double allocation only happens with software RSA, right? I
don't really think that is going to matter considering how slow
it is, no?

Cheers,
-- 
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 v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-15 Thread Tudor-Dan Ambarus 
> On Wed, Apr 06, 2016 at 04:37:04PM +0300, Tudor Ambarus wrote:
> > This approach has the advantage that users can select specific
> > parser actions by using a general parser with function pointers
> > to specific actions.
> 
> I don't understand why we need different parsing functions in the
> first place.  Can't they just return raw integers always?
> 
> You can then trivially convert the raw integers to MPI, no?

We need different parsing functions so that we don't allocate duplicate buffers 
for the same data.

You need to allocate buffers when getting the raw integers and you need to 
allocate other (duplicate) buffers when converting the raw integers to MPI.

Using the proposed API each user can select the format of data he wants, 
eliminating the need of a double conversion (with its drawbacks: duplicate 
buffers, unnecessary cycles).

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 v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-15 Thread Herbert Xu 
On Wed, Apr 06, 2016 at 04:37:04PM +0300, Tudor Ambarus wrote:
> Use common ASN.1 sequences for all RSA implementations.
> 
> Give hardware RSA implementations the chance to use
> the RSA's software implementation parser even if they
> are likely to want to use raw integers.
> 
> The parser expects a context that contains at the first address
> a pointer to a struct rsa_asn1_action instance that has function
> pointers to specific parser actions (return MPI or raw integer keys),
> followed by a key representation structure (for MPI or raw integers).
> 
> This approach has the advantage that users can select specific
> parser actions by using a general parser with function pointers
> to specific actions.

I don't understand why we need different parsing functions in the
first place.  Can't they just return raw integers always?

You can then trivially convert the raw integers to MPI, no?

Cheers,
-- 
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

[PATCH v3 1/3] crypto: rsa - generalize ASN.1 sequences

2016-04-06 Thread Tudor Ambarus 
Use common ASN.1 sequences for all RSA implementations.

Give hardware RSA implementations the chance to use
the RSA's software implementation parser even if they
are likely to want to use raw integers.

The parser expects a context that contains at the first address
a pointer to a struct rsa_asn1_action instance that has function
pointers to specific parser actions (return MPI or raw integer keys),
followed by a key representation structure (for MPI or raw integers).

This approach has the advantage that users can select specific
parser actions by using a general parser with function pointers
to specific actions.

Signed-off-by: Tudor Ambarus 
---
 crypto/rsa.c  |  60 ++-
 crypto/rsa_helper.c   | 166 --
 include/crypto/internal/rsa.h |  31 ++--
 3 files changed, 194 insertions(+), 63 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index 77d737f..7cb0153 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -19,7 +19,7 @@
  * RSAEP function [RFC3447 sec 5.1.1]
  * c = m^e mod n;
  */
-static int _rsa_enc(const struct rsa_key *key, MPI c, MPI m)
+static int _rsa_enc(const struct rsa_mpi_key *key, MPI c, MPI m)
 {
/* (1) Validate 0 <= m < n */
if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
@@ -33,7 +33,7 @@ static int _rsa_enc(const struct rsa_key *key, MPI c, MPI m)
  * RSADP function [RFC3447 sec 5.1.2]
  * m = c^d mod n;
  */
-static int _rsa_dec(const struct rsa_key *key, MPI m, MPI c)
+static int _rsa_dec(const struct rsa_mpi_key *key, MPI m, MPI c)
 {
/* (1) Validate 0 <= c < n */
if (mpi_cmp_ui(c, 0) < 0 || mpi_cmp(c, key->n) >= 0)
@@ -47,7 +47,7 @@ static int _rsa_dec(const struct rsa_key *key, MPI m, MPI c)
  * RSASP1 function [RFC3447 sec 5.2.1]
  * s = m^d mod n
  */
-static int _rsa_sign(const struct rsa_key *key, MPI s, MPI m)
+static int _rsa_sign(const struct rsa_mpi_key *key, MPI s, MPI m)
 {
/* (1) Validate 0 <= m < n */
if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
@@ -61,7 +61,7 @@ static int _rsa_sign(const struct rsa_key *key, MPI s, MPI m)
  * RSAVP1 function [RFC3447 sec 5.2.2]
  * m = s^e mod n;
  */
-static int _rsa_verify(const struct rsa_key *key, MPI m, MPI s)
+static int _rsa_verify(const struct rsa_mpi_key *key, MPI m, MPI s)
 {
/* (1) Validate 0 <= s < n */
if (mpi_cmp_ui(s, 0) < 0 || mpi_cmp(s, key->n) >= 0)
@@ -71,15 +71,17 @@ static int _rsa_verify(const struct rsa_key *key, MPI m, 
MPI s)
return mpi_powm(m, s, key->e, key->n);
 }
 
-static inline struct rsa_key *rsa_get_key(struct crypto_akcipher *tfm)
+static inline struct rsa_mpi_key *rsa_get_key(struct crypto_akcipher *tfm)
 {
-   return akcipher_tfm_ctx(tfm);
+   struct rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
+
+   return >key;
 }
 
 static int rsa_enc(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI m, c = mpi_alloc(0);
int ret = 0;
int sign;
@@ -118,7 +120,7 @@ err_free_c:
 static int rsa_dec(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI c, m = mpi_alloc(0);
int ret = 0;
int sign;
@@ -156,7 +158,7 @@ err_free_m:
 static int rsa_sign(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI m, s = mpi_alloc(0);
int ret = 0;
int sign;
@@ -195,7 +197,7 @@ err_free_s:
 static int rsa_verify(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI s, m = mpi_alloc(0);
int ret = 0;
int sign;
@@ -251,15 +253,16 @@ static int rsa_check_key_length(unsigned int len)
 static int rsa_set_pub_key(struct crypto_akcipher *tfm, const void *key,
   unsigned int keylen)
 {
-   struct rsa_key *pkey = akcipher_tfm_ctx(tfm);
+   struct rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
+   struct rsa_mpi_key *pkey = >key;
int ret;
 
-   ret = rsa_parse_pub_key(pkey, key, keylen);
+   ret = rsa_parse_mpi_pub_key(ctx, key, keylen);
if (ret)
return ret;
 
if (rsa_check_key_length(mpi_get_size(pkey->n) << 3)) {
-   rsa_free_key(pkey);
+   rsa_free_mpi_key(pkey);
ret = -EINVAL;
}
return ret;
@@ -268,15 +271,16 @@ static int rsa_set_pub_key(struct crypto_akcipher *tfm, 
const