[U-Boot] [PATCH 3/9] [v3] DM: crypto/rsa: Add rsa Modular Exponentiation DM driver

* [U-Boot] [PATCH 3/9] [v3] DM: crypto/rsa: Add rsa Modular Exponentiation DM driver
@ 2014-12-29  3:53 Ruchika Gupta
  2014-12-29 20:24 ` Simon Glass
  0 siblings, 1 reply; 4+ messages in thread
From: Ruchika Gupta @ 2014-12-29  3:53 UTC (permalink / raw)
  To: u-boot

Re-sending as delivery to uboot mailing list failed last time...

Hi Simon,

Thanks for the review comments. 

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Wednesday, December 24, 2014 6:19 AM
> To: Gupta Ruchika-R66431
> Cc: U-Boot Mailing List; Sun York-R58495
> Subject: Re: [PATCH 3/9] [v3] DM: crypto/rsa: Add rsa Modular 
> Exponentiation DM driver
> 
> Hi Ruchika,
> 
> On 23 December 2014 at 04:32, Ruchika Gupta 
> <ruchika.gupta@freescale.com>
> wrote:
> > Add a new rsa uclass for performing modular exponentiation and 
> > implement the software driver basing on this uclass.
> >
> > Signed-off-by: Ruchika Gupta <ruchika.gupta@freescale.com>
> > CC: Simon Glass <sjg@chromium.org>
> > ---
> > Changes in v3:
> > New patch with driver model for RSA UCLASS
> >
> >  drivers/crypto/Kconfig          |  1 +
> >  drivers/crypto/Makefile         |  1 +
> >  drivers/crypto/rsa/Kconfig      |  5 +++++
> >  drivers/crypto/rsa/Makefile     |  8 ++++++++
> >  drivers/crypto/rsa/rsa_sw.c     | 39
> +++++++++++++++++++++++++++++++++++++++
> >  drivers/crypto/rsa/rsa_uclass.c | 31 +++++++++++++++++++++++++++++++
> >  include/dm/uclass-id.h          |  1 +
> >  include/u-boot/rsa-mod-exp.h    | 40
> ++++++++++++++++++++++++++++++++++++++++
> >  8 files changed, 126 insertions(+)
> >  create mode 100644 drivers/crypto/rsa/Kconfig  create mode 100644 
> > drivers/crypto/rsa/Makefile  create mode 100644 
> > drivers/crypto/rsa/rsa_sw.c  create mode 100644 
> > drivers/crypto/rsa/rsa_uclass.c
> >
> > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index
> > e69de29..75f3479 100644
> > --- a/drivers/crypto/Kconfig
> > +++ b/drivers/crypto/Kconfig
> > @@ -0,0 +1 @@
> > +source drivers/crypto/rsa/Kconfig
> > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile index 
> > 7b79237..a2f30fc 100644
> > --- a/drivers/crypto/Makefile
> > +++ b/drivers/crypto/Makefile
> > @@ -6,4 +6,5 @@
> >  #
> >
> >  obj-$(CONFIG_EXYNOS_ACE_SHA)   += ace_sha.o
> > +obj-y += rsa/
> >  obj-y += fsl/
> > diff --git a/drivers/crypto/rsa/Kconfig b/drivers/crypto/rsa/Kconfig 
> > new file mode 100644 index 0000000..7eb90a1
> > --- /dev/null
> > +++ b/drivers/crypto/rsa/Kconfig
> > @@ -0,0 +1,5 @@
> > +config DM_RSA
> > +       bool "Enable Driver Model for RSA "
> > +       depends on DM
> > +       help
> > +         If you want to use driver model for RSA Modular 
> > +Exponentiation,
> say Y.
> 
> Can you send a new patch (later if you prefer) which removes this 
> option altogether? It should be the default. In other words, RSA 
> should always use driver model.

If we make it mandatory for RSA to use driver model, all platforms would need to use DM by default in their defconfig. Would that be acceptable ?
Secondly if I force select DM and DM_RSA when CONFIG_RSA is selected, this may result in duplication of CONFIG_DM with platforms having it defined already.
Please suggest.

> 
> > diff --git a/drivers/crypto/rsa/Makefile 
> > b/drivers/crypto/rsa/Makefile new file mode 100644 index 
> > 0000000..fae4f8c
> > --- /dev/null
> > +++ b/drivers/crypto/rsa/Makefile
> > @@ -0,0 +1,8 @@
> > +#
> > +# (C) Copyright 2014 Freescale Semiconductor, Inc.
> > +#
> > +# SPDX-License-Identifier:     GPL-2.0+
> > +#
> > +
> > +obj-$(CONFIG_DM_RSA) += rsa_uclass.o
> > +obj-$(CONFIG_RSA_SW) += rsa_sw.o
> > diff --git a/drivers/crypto/rsa/rsa_sw.c 
> > b/drivers/crypto/rsa/rsa_sw.c new file mode 100644 index 
> > 0000000..5d94754
> > --- /dev/null
> > +++ b/drivers/crypto/rsa/rsa_sw.c
> > @@ -0,0 +1,39 @@
> > +/*
> > + * (C) Copyright 2014 Freescale Semiconductor, Inc.
> > + * Author: Ruchika Gupta <ruchika.gupta@freescale.com>
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <config.h>
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <u-boot/rsa-mod-exp.h>
> > +
> > +int mod_exp_sw(struct udevice *dev, const uint8_t *sig, uint32_t sig_len,
> > +               struct key_prop *prop, uint8_t *out) {
> > +       int ret = 0;
> > +
> > +       ret = rsa_mod_exp_sw(sig, sig_len, prop, out);
> > +
> > +       if (ret) {
> > +               debug("%s: RSA failed to verify: %d\n", __func__, ret);
> > +               return ret;
> > +       }
> > +       return 0;
> > +}
> > +
> > +static const struct rsa_ops rsa_ops_sw = {
> > +       .get_mod_exp    = mod_exp_sw,
> > +};
> > +
> > +U_BOOT_DRIVER(fsl_rsa) = {
> > +       .name   = "rsa_sw",
> > +       .id     = UCLASS_RSA,
> > +       .ops    = &rsa_ops_sw,
> > +};
> > +
> > +U_BOOT_DEVICE(rsa_sw) = {
> > +       .name = "rsa_sw",
> > +};
> > diff --git a/drivers/crypto/rsa/rsa_uclass.c 
> > b/drivers/crypto/rsa/rsa_uclass.c new file mode 100644 index
> > 0000000..f4f4f39
> > --- /dev/null
> > +++ b/drivers/crypto/rsa/rsa_uclass.c
> > @@ -0,0 +1,31 @@
> > +/*
> > + * (C) Copyright 2014 Freescale Semiconductor, Inc
> > + * Author: Ruchika Gupta <ruchika.gupta@freescale.com>
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <u-boot/rsa-mod-exp.h>
> > +#include <errno.h>
> > +#include <fdtdec.h>
> > +#include <malloc.h>
> > +#include <asm/io.h>
> > +#include <linux/list.h>
> > +
> > +int rsa_mod_exp(struct udevice *dev, const uint8_t *sig, uint32_t sig_len,
> > +               struct key_prop *node, uint8_t *out) {
> > +       const struct rsa_ops *ops = device_get_ops(dev);
> > +
> > +       if (!ops->get_mod_exp)
> > +               return -ENOSYS;
> > +
> > +       return ops->get_mod_exp(dev, sig, sig_len, node, out); }
> > +
> > +UCLASS_DRIVER(rsa) = {
> > +       .id             = UCLASS_RSA,
> > +       .name           = "rsa",
> > +};
> > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 
> > f17c3c2..659369e 100644
> > --- a/include/dm/uclass-id.h
> > +++ b/include/dm/uclass-id.h
> > @@ -33,6 +33,7 @@ enum uclass_id {
> >         UCLASS_I2C,             /* I2C bus */
> >         UCLASS_I2C_GENERIC,     /* Generic I2C device */
> >         UCLASS_I2C_EEPROM,      /* I2C EEPROM device */
> > +       UCLASS_RSA      ,       /* RSA Mod Exp device */
> 
> Funny spacing here.
Sorry, I will correct that.
> 
> >
> >         UCLASS_COUNT,
> >         UCLASS_INVALID = -1,
> > diff --git a/include/u-boot/rsa-mod-exp.h 
> > b/include/u-boot/rsa-mod-exp.h index 59cd9ea..7f7e196 100644
> > --- a/include/u-boot/rsa-mod-exp.h
> > +++ b/include/u-boot/rsa-mod-exp.h
> > @@ -40,4 +40,44 @@ struct key_prop {  int rsa_mod_exp_sw(const 
> > uint8_t *sig, uint32_t sig_len,
> >                 struct key_prop *node, uint8_t *out);
> >
> > +/**
> > + * rsa_mod_exp - Perform RSA Modular Exponentiation
> > + *
> > + * Operation: out[] = sig ^ exponent % modulus
> > + *
> > + * @udev:      RSA Device
> > + * @sig:       RSA PKCS1.5 signature
> > + * @sig_len:   Length of signature in number of bytes
> > + * @node:      Node with RSA key elements like modulus, exponent, R^2,
> n0inv
> > + * @out:       Result in form of byte array
> 
> How big is this array?
Thanks for pointing out. In next set of patch, I will change it to uint8_t **out and introduce a uin32_t *out_len also. Ideally the implemntor of this function should allocate out as per length required and return back the pointer and the allocated length. This will also require change in patch 1 of this series. I will change and float next version.

> 
> > + */
> > +int rsa_mod_exp(struct udevice *dev, const uint8_t *sig, uint32_t sig_len,
> > +               struct key_prop *node, uint8_t *out);
> > +
> > +/**
> > + * struct struct rsa_ops - Driver model for RSA operations
> > + *
> > + * The uclass interface is implemented by all crypto devices which 
> > +use
> > + * driver model.
> > + */
> > +struct rsa_ops {
> > +       /**
> > +        * Perform Modular Exponentiation
> > +        *
> > +        * Operation: out[] = sig ^ exponent % modulus
> > +        *
> > +        * @dev:        RSA Device
> > +        * @sig:        RSA PKCS1.5 signature
> > +        * @sig_len:    Length of signature in number of bytes
> > +        * @node:       Node with RSA key elements like modulus, exponent,
> > +        *              R^2, n0inv
> > +        * @out:        Result in form of byte array
> 
> How big is this array?
Same as above, I will change it to **outp and add a pointer for length
> 
> > +        * Returns: 0 if exponentiation is succesful, or a negative 
> > + value
> 
> successful
Ok
> 
> > +        * if it wasn't.
> > +        */
> > +       int (*get_mod_exp)(struct udevice *dev, const uint8_t *sig,
> 
> mod_exp() is better I think, since it matches your function above.
Ok. I will change it.
> 
> > +                          uint32_t sig_len, struct key_prop *node,
> > +                          uint8_t *out); };
> > +
> >  #endif
> > --
> > 1.8.1.4
> >
> 
> Regards,
> Simon

Regards,
Ruchika

^ permalink raw reply	[flat|nested] 4+ messages in thread