RE: [PATCH 10/10] crypto: caam - add support for RSA algorithm

2016-03-21 Thread Tudor-Dan Ambarus 
Hi Stephan,

> -Original Message-
> From: Stephan Mueller [mailto:smuel...@chronox.de]
> Sent: Saturday, March 19, 2016 6:59 PM
> To: Tudor-Dan Ambarus
> Cc: herb...@gondor.apana.org.au; tadeusz.st...@intel.com; linux-
> cry...@vger.kernel.org; Horia Ioan Geanta Neag
> Subject: Re: [PATCH 10/10] crypto: caam - add support for RSA algorithm
> 
> > +void rsa_free_key(struct rsa_raw_key *key)
> > +{
> > +   kzfree(key->d);
> > +   key->d = NULL;
> > +
> > +   kfree(key->e);
> > +   key->e = NULL;
> > +
> > +   kfree(key->n);
> > +   key->n = NULL;
> > +
> > +   key->n_sz = 0;
> > +   key->e_sz = 0;
> > +}
> 
> As you implement raw key support for use in other drivers, shouldn't that
> function go into some helper file like free_mpis().
> > +

[ta] I should also add an rsa_free_coherent_key(struct device *dev, struct 
rsa_raw_key *key), for those implementations that use the DMA-coherent API. 


> > +static int caam_rsa_dec(struct akcipher_request *req)
> > +{
> > +   struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> > +   struct rsa_raw_ctx *ctx = akcipher_tfm_ctx(tfm);
> > +   struct rsa_raw_key *key = >key;
> > +   struct device *jrdev = ctx->dev;
> > +   struct rsa_edesc *edesc = NULL;
> > +   size_t desclen = sizeof(struct rsa_priv_f1_desc);
> > +   int ret;
> > +
> > +   if (unlikely(!key->n || !key->d))
> > +   return -EINVAL;
> > +
> > +   if (req->dst_len < key->n_sz) {
> > +   req->dst_len = key->n_sz;
> > +   return -EOVERFLOW;
> > +   }
> > +
> > +   /* Allocate extended descriptor. */
> > +   edesc = rsa_edesc_alloc(req, desclen);
> > +   if (IS_ERR(edesc))
> > +   return PTR_ERR(edesc);
> > +
> > +   /* Initialize Job Descriptor. */
> > +   ret = init_rsa_priv_f1_desc(req, edesc);
> > +   if (ret)
> > +   return ret;
> 
> Same here, kfree?

[ta] Sure, thanks.

> > +
> > +   ret = caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f1_done, req);
> > +   if (!ret) {
> > +   ret = -EINPROGRESS;
> 
> dto
>

[ta] resources are freed on rsa_priv_f1_done callback.
 
> > +static struct akcipher_alg rsa = {
> 
> can you please name that something else, like caam_rsa? It is a real hassle
> when searching some symbol and I get such a generic name.

[ta] ok.

Thank you for the review!
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

[PATCH 10/10] crypto: caam - add support for RSA algorithm

2016-03-19 Thread Tudor Ambarus 
Add RSA support to caam driver.

Coauthored-by: Yashpal Dutta 

Signed-off-by: Tudor Ambarus 
---
 drivers/crypto/caam/Kconfig|  12 +
 drivers/crypto/caam/Makefile   |   4 +
 drivers/crypto/caam/caampkc.c  | 513 +
 drivers/crypto/caam/caampkc.h  |  84 +++
 drivers/crypto/caam/desc.h |   2 +
 drivers/crypto/caam/pdb.h  |  16 +-
 drivers/crypto/caam/pkc_desc.c | 138 +++
 7 files changed, 768 insertions(+), 1 deletion(-)
 create mode 100644 drivers/crypto/caam/caampkc.c
 create mode 100644 drivers/crypto/caam/caampkc.h
 create mode 100644 drivers/crypto/caam/pkc_desc.c

diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index 5652a53..5427e63 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -99,6 +99,18 @@ config CRYPTO_DEV_FSL_CAAM_AHASH_API
  To compile this as a module, choose M here: the module
  will be called caamhash.
 
+config CRYPTO_DEV_FSL_CAAM_PKC_API
+tristate "Register public key cryptography implementations with Crypto 
API"
+depends on CRYPTO_DEV_FSL_CAAM && CRYPTO_DEV_FSL_CAAM_JR
+default y
+select CRYPTO_RSA_HELPER
+help
+  Selecting this will allow SEC Public key support for RSA.
+  Supported cryptographic primitives: encryption, decryption,
+  signature and verification.
+  To compile this as a module, choose M here: the module
+  will be called caam_pkc.
+
 config CRYPTO_DEV_FSL_CAAM_RNG_API
tristate "Register caam device for hwrng API"
depends on CRYPTO_DEV_FSL_CAAM && CRYPTO_DEV_FSL_CAAM_JR
diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
index 550758a..399ad55 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -5,11 +5,15 @@ ifeq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG), y)
EXTRA_CFLAGS := -DDEBUG
 endif
 
+ccflags-y += -I$(srctree)/crypto
+
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_JR) += caam_jr.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API) += caamalg.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
+obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caam_pkc.o
 
 caam-objs := ctrl.o
 caam_jr-objs := jr.o key_gen.o error.o
+caam_pkc-y := caampkc.o pkc_desc.o
diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
new file mode 100644
index 000..1c53158
--- /dev/null
+++ b/drivers/crypto/caam/caampkc.c
@@ -0,0 +1,513 @@
+/*
+ * caam - Freescale FSL CAAM support for Public Key Cryptography
+ *
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ *
+ * There is no Shared Descriptor for PKC so that the Job Descriptor must carry
+ * all the desired key parameters, input and output pointers.
+ */
+#include 
+#include 
+#include "compat.h"
+#include "caampkc.h"
+#include "sg_sw_sec4.h"
+#include "regs.h"
+#include "intern.h"
+#include "jr.h"
+#include "error.h"
+
+void rsa_free_key(struct rsa_raw_key *key)
+{
+   kzfree(key->d);
+   key->d = NULL;
+
+   kfree(key->e);
+   key->e = NULL;
+
+   kfree(key->n);
+   key->n = NULL;
+
+   key->n_sz = 0;
+   key->e_sz = 0;
+}
+
+static int caam_rsa_setpubkey(struct crypto_akcipher *tfm, const void *key,
+ unsigned int keylen)
+{
+   struct rsa_raw_ctx *ctx = akcipher_tfm_ctx(tfm);
+   struct rsa_raw_key *raw_key = >key;
+   int ret;
+
+   set_raw_rsa_pub_action(>action);
+
+   /* Free the old key if any */
+   rsa_free_key(raw_key);
+
+   ret = asn1_ber_decoder(_decoder, ctx, key, keylen);
+   if (ret < 0)
+   goto free;
+
+   if (!raw_key->n || !raw_key->e) {
+   /* Invalid key provided */
+   ret = -EINVAL;
+   goto free;
+   }
+
+   return 0;
+free:
+   rsa_free_key(raw_key);
+   return ret;
+}
+
+static int caam_rsa_setprivkey(struct crypto_akcipher *tfm, const void *key,
+  unsigned int keylen)
+{
+   struct rsa_raw_ctx *ctx = akcipher_tfm_ctx(tfm);
+   struct rsa_raw_key *raw_key = >key;
+   int ret;
+
+   set_raw_rsa_priv_action(>action);
+
+   /* Free the old key if any */
+   rsa_free_key(raw_key);
+
+   ret = asn1_ber_decoder(_decoder, ctx, key, keylen);
+   if (ret < 0)
+   goto free;
+
+   if (!raw_key->n || !raw_key->e || !raw_key->d) {
+   /* Invalid key provided */
+   ret = -EINVAL;
+   goto free;
+   }
+
+   return 0;
+free:
+   rsa_free_key(raw_key);
+   return ret;
+}
+
+static void rsa_pub_unmap(struct device *dev, struct rsa_edesc *edesc,
+ struct akcipher_request *req)
+{
+   struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
+   struct rsa_raw_ctx *ctx

Re: [PATCH 10/10] crypto: caam - add support for RSA algorithm

2016-03-19 Thread Stephan Mueller 
Am Freitag, 18. März 2016, 20:32:07 schrieb Tudor Ambarus:

Hi Tudor,

> Add RSA support to caam driver.
> 
> Coauthored-by: Yashpal Dutta 
> 
> Signed-off-by: Tudor Ambarus 
> ---
>  drivers/crypto/caam/Kconfig|  12 +
>  drivers/crypto/caam/Makefile   |   4 +
>  drivers/crypto/caam/caampkc.c  | 513
> + drivers/crypto/caam/caampkc.h  | 
> 84 +++
>  drivers/crypto/caam/desc.h |   2 +
>  drivers/crypto/caam/pdb.h  |  16 +-
>  drivers/crypto/caam/pkc_desc.c | 138 +++
>  7 files changed, 768 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/crypto/caam/caampkc.c
>  create mode 100644 drivers/crypto/caam/caampkc.h
>  create mode 100644 drivers/crypto/caam/pkc_desc.c
> 
> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
> index 5652a53..5427e63 100644
> --- a/drivers/crypto/caam/Kconfig
> +++ b/drivers/crypto/caam/Kconfig
> @@ -99,6 +99,18 @@ config CRYPTO_DEV_FSL_CAAM_AHASH_API
> To compile this as a module, choose M here: the module
> will be called caamhash.
> 
> +config CRYPTO_DEV_FSL_CAAM_PKC_API
> +tristate "Register public key cryptography implementations with
> Crypto API" +depends on CRYPTO_DEV_FSL_CAAM &&
> CRYPTO_DEV_FSL_CAAM_JR
> +default y
> +select CRYPTO_RSA_HELPER
> +help
> +  Selecting this will allow SEC Public key support for RSA.
> +  Supported cryptographic primitives: encryption, decryption,
> +  signature and verification.
> +  To compile this as a module, choose M here: the module
> +  will be called caam_pkc.
> +
>  config CRYPTO_DEV_FSL_CAAM_RNG_API
>   tristate "Register caam device for hwrng API"
>   depends on CRYPTO_DEV_FSL_CAAM && CRYPTO_DEV_FSL_CAAM_JR
> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 550758a..399ad55 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -5,11 +5,15 @@ ifeq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG), y)
>   EXTRA_CFLAGS := -DDEBUG
>  endif
> 
> +ccflags-y += -I$(srctree)/crypto
> +
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_JR) += caam_jr.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API) += caamalg.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o
>  obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o
> +obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caam_pkc.o
> 
>  caam-objs := ctrl.o
>  caam_jr-objs := jr.o key_gen.o error.o
> +caam_pkc-y := caampkc.o pkc_desc.o
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> new file mode 100644
> index 000..1c53158
> --- /dev/null
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -0,0 +1,513 @@
> +/*
> + * caam - Freescale FSL CAAM support for Public Key Cryptography
> + *
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * There is no Shared Descriptor for PKC so that the Job Descriptor must
> carry + * all the desired key parameters, input and output pointers.
> + */
> +#include 
> +#include 
> +#include "compat.h"
> +#include "caampkc.h"
> +#include "sg_sw_sec4.h"
> +#include "regs.h"
> +#include "intern.h"
> +#include "jr.h"
> +#include "error.h"
> +
> +void rsa_free_key(struct rsa_raw_key *key)
> +{
> + kzfree(key->d);
> + key->d = NULL;
> +
> + kfree(key->e);
> + key->e = NULL;
> +
> + kfree(key->n);
> + key->n = NULL;
> +
> + key->n_sz = 0;
> + key->e_sz = 0;
> +}

As you implement raw key support for use in other drivers, shouldn't that 
function go into some helper file like free_mpis().
> +
> +static int caam_rsa_setpubkey(struct crypto_akcipher *tfm, const void *key,
> +   unsigned int keylen)
> +{
> + struct rsa_raw_ctx *ctx = akcipher_tfm_ctx(tfm);
> + struct rsa_raw_key *raw_key = >key;
> + int ret;
> +
> + set_raw_rsa_pub_action(>action);
> +
> + /* Free the old key if any */
> + rsa_free_key(raw_key);
> +
> + ret = asn1_ber_decoder(_decoder, ctx, key, keylen);
> + if (ret < 0)
> + goto free;
> +
> + if (!raw_key->n || !raw_key->e) {
> + /* Invalid key provided */
> + ret = -EINVAL;
> + goto free;
> + }
> +
> + return 0;
> +free:
> + rsa_free_key(raw_key);
> + return ret;
> +}
> +
> +static int caam_rsa_setprivkey(struct crypto_akcipher *tfm, const void
> *key, +  unsigned int keylen)
> +{
> + struct rsa_raw_ctx *ctx = akcipher_tfm_ctx(tfm);
> + struct rsa_raw_key *raw_key = >key;
> + int ret;
> +
> + set_raw_rsa_priv_action(>action);
> +
> + /* Free the old key if any */
> + rsa_free_key(raw_key);
> +
> + ret = asn1_ber_decoder(_decoder, ctx, key, keylen);
> + if (ret < 0)
> + goto free;
> +
> + if (!raw_key->n || !raw_key->e || !raw_key->d) {
> + /*