[U-Boot] [PATCH 5/9] [v4] lib/rsa: Modify rsa to use DM driver

Simon Glass sjg at chromium.org
Fri Jan 2 23:24:19 CET 2015


Hi Ruchika,

On 30 December 2014 at 02:30, Ruchika Gupta <ruchika.gupta at freescale.com> wrote:
> Modify rsa_verify to use the rsa driver of DM library .The tools
> will continue to use the same RSA sw library.
>
> CONFIG_RSA is now dependent on CONFIG_DM. All configurations which
> enable FIT based signatures have been modified to enable CONFIG_DM
> by default.
>
> Signed-off-by: Ruchika Gupta <ruchika.gupta at freescale.com>
> CC: Simon Glass <sjg at chromium.org>
> ---
> Changes in v4:
> Make CONFIG_RSA always depenedent on Driver Model.
> Add CONFIG_DM in defconfigs of the platforms which enable CONFIG_FIT_SIGNATURE
>
> Changes in v3:
> New patch
>
>  README                             |  7 ++++++-
>  configs/ids8313_defconfig          |  1 +
>  configs/sandbox_defconfig          |  1 +
>  configs/zynq_microzed_defconfig    |  1 +
>  configs/zynq_zc70x_defconfig       |  1 +
>  configs/zynq_zc770_xm010_defconfig |  1 +
>  configs/zynq_zc770_xm012_defconfig |  1 +
>  configs/zynq_zc770_xm013_defconfig |  1 +
>  configs/zynq_zed_defconfig         |  1 +
>  configs/zynq_zybo_defconfig        |  1 +
>  include/configs/am335x_evm.h       |  3 +++
>  include/configs/sandbox.h          |  1 -
>  lib/Kconfig                        |  5 ++++-
>  lib/rsa/rsa-verify.c               | 19 +++++++++++++++++++
>  14 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/README b/README
> index 4ca04d0..1d7978f 100644
> --- a/README
> +++ b/README
> @@ -3173,8 +3173,13 @@ CBFS (Coreboot Filesystem) support
>                 This enables the RSA algorithm used for FIT image verification
>                 in U-Boot. See doc/uImage.FIT/signature.txt for more information.
>
> +               The Modular Exponentiation algorithm in RSA is implemented using
> +               driver model. So CONFIG_DM needs to be enabled by default for this
> +               library to function.
> +
>                 The signing part is build into mkimage regardless of this
> -               option.
> +               option. The software based modular exponentiation is built into
> +               mkimage irrespective of this option.

Just to avoid confusion, I think your addition here to the README is good.

>
>  - bootcount support:
>                 CONFIG_BOOTCOUNT_LIMIT
> diff --git a/configs/ids8313_defconfig b/configs/ids8313_defconfig
> index 8479cd4..0950ec8 100644
> --- a/configs/ids8313_defconfig
> +++ b/configs/ids8313_defconfig
> @@ -4,3 +4,4 @@ CONFIG_MPC83xx=y
>  CONFIG_FIT=y
>  CONFIG_FIT_SIGNATURE=y
>  CONFIG_TARGET_IDS8313=y
> +CONFIG_DM=y

Eek I suspect this might cause a problem, but by the time this merges,
DM support for PPC should be merged, so OK.

> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 0111f25..660063e 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -3,4 +3,5 @@ CONFIG_OF_HOSTFILE=y
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
>  CONFIG_DEFAULT_DEVICE_TREE="sandbox"
> diff --git a/configs/zynq_microzed_defconfig b/configs/zynq_microzed_defconfig
> index b9a6fe5..8b985fe 100644
> --- a/configs/zynq_microzed_defconfig
> +++ b/configs/zynq_microzed_defconfig
> @@ -6,4 +6,5 @@ CONFIG_OF_CONTROL=y
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
>  CONFIG_DEFAULT_DEVICE_TREE="zynq-microzed"
> diff --git a/configs/zynq_zc70x_defconfig b/configs/zynq_zc70x_defconfig
> index dc8aa84..cceb321 100644
> --- a/configs/zynq_zc70x_defconfig
> +++ b/configs/zynq_zc70x_defconfig
> @@ -7,3 +7,4 @@ CONFIG_DEFAULT_DEVICE_TREE="zynq-zc702"
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
> diff --git a/configs/zynq_zc770_xm010_defconfig b/configs/zynq_zc770_xm010_defconfig
> index 2f5fa8c..2935c0d 100644
> --- a/configs/zynq_zc770_xm010_defconfig
> +++ b/configs/zynq_zc770_xm010_defconfig
> @@ -8,3 +8,4 @@ CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm010"
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
> diff --git a/configs/zynq_zc770_xm012_defconfig b/configs/zynq_zc770_xm012_defconfig
> index a92d495..0401739 100644
> --- a/configs/zynq_zc770_xm012_defconfig
> +++ b/configs/zynq_zc770_xm012_defconfig
> @@ -8,3 +8,4 @@ CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm012"
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
> diff --git a/configs/zynq_zc770_xm013_defconfig b/configs/zynq_zc770_xm013_defconfig
> index 3a02f75..a95970a 100644
> --- a/configs/zynq_zc770_xm013_defconfig
> +++ b/configs/zynq_zc770_xm013_defconfig
> @@ -8,3 +8,4 @@ CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm013"
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
> diff --git a/configs/zynq_zed_defconfig b/configs/zynq_zed_defconfig
> index 1d816f6..0fbc41a 100644
> --- a/configs/zynq_zed_defconfig
> +++ b/configs/zynq_zed_defconfig
> @@ -7,3 +7,4 @@ CONFIG_DEFAULT_DEVICE_TREE="zynq-zed"
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
> diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
> index 9183629..4e66760 100644
> --- a/configs/zynq_zybo_defconfig
> +++ b/configs/zynq_zybo_defconfig
> @@ -7,3 +7,4 @@ CONFIG_DEFAULT_DEVICE_TREE="zynq-zybo"
>  CONFIG_FIT=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_FIT_SIGNATURE=y
> +CONFIG_DM=y
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index cc36985..1eb0ce7 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -24,6 +24,9 @@
>  # define CONFIG_LZO
>  # ifdef CONFIG_ENABLE_VBOOT
>  # define CONFIG_FIT_SIGNATURE
> +#ifndef CONFIG_DM
> +#define CONFIG_DM
> +#endif
>  # define CONFIG_RSA
>  # endif

Since you have moved CONFIG_FIT_SIGNATURE to Kconfig you can remove
all of this and just add your options to
configs/am335x_boneblack_vboot_defconfig instead. That is the only
config which enables vboot.

>  #endif
> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> index 6fd29b9..e9d3f32 100644
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> @@ -23,7 +23,6 @@
>
>  #define CONFIG_BOOTSTAGE
>  #define CONFIG_BOOTSTAGE_REPORT
> -#define CONFIG_DM
>  #define CONFIG_CMD_DEMO
>  #define CONFIG_CMD_DM
>  #define CONFIG_DM_DEMO
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 2455f7a..f317f81 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -29,9 +29,12 @@ config SYS_HZ
>
>  config RSA
>         bool "Use RSA Library"
> +       depends on DM
>         help
>           RSA support.This enables the RSA algorithm used for FIT image
> -         verification in U-Boot.
> +         verification in U-Boot. RSA support for Modular exponentiation
> +         is implemented as a driver model. Driver Model should be enabled
> +         to select this option.

I think you should drop this change. The fact that it depends on DM
will be obvious from the depends line, and you won't see the option
unless DM is enabled anyway.

>           See doc/uImage.FIT/signature.txt for more details.
>
>  endmenu
> diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
> index 1d2e707..af915d3 100644
> --- a/lib/rsa/rsa-verify.c
> +++ b/lib/rsa/rsa-verify.c
> @@ -12,6 +12,7 @@
>  #include <asm/errno.h>
>  #include <asm/types.h>
>  #include <asm/unaligned.h>
> +#include <dm.h>
>  #else
>  #include "fdt_host.h"
>  #include "mkimage.h"
> @@ -44,6 +45,9 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig,
>         const uint8_t *padding;
>         int pad_len;
>         int ret;
> +#if !defined(USE_HOSTCC)
> +       struct udevice *rsa_dev;
> +#endif
>
>         if (!prop || !sig || !hash || !algo)
>                 return -EIO;
> @@ -65,11 +69,26 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig,
>         uint8_t *buf;
>         uint32_t buf_len;
>
> +#if !defined(USE_HOSTCC)
> +       ret = uclass_get_device(UCLASS_RSA, 0, &rsa_dev);
> +       if (!ret) {
> +               ret = rsa_mod_exp(rsa_dev, sig, sig_len, prop, &buf,
> +                                 &buf_len);
> +               if (ret) {
> +                       debug("Error in Modular exponentation\n");
> +                       return ret;
> +               }
> +       } else {
> +               printf("RSA: Can't find Mod Exp implemnetation\n");
> +               return -EINVAL;
> +       }
> +#else
>         ret = rsa_mod_exp_sw(sig, sig_len, prop, &buf, &buf_len);
>         if (ret) {
>                 debug("Error in Modular exponentation\n");
>                 return ret;
>         }
> +#endif

Please can you refactor to reduce duplication? Something like this
(excuse formatting):

> +#if !defined(USE_HOSTCC)
> +       ret = uclass_get_device(UCLASS_RSA, 0, &rsa_dev);
> +       if (ret)
> +               printf("RSA: Can't find Mod Exp implemnetation\n");          - fix typo here!
> +               return -EINVAL;
> +       }
   +       ret = rsa_mod_exp(rsa_dev, sig, sig_len, prop, &buf,
> +                                 &buf_len);
> +#else
>         ret = rsa_mod_exp_sw(sig, sig_len, prop, &buf, &buf_len);
#endif
>         if (ret) {
>                 debug("Error in Modular exponentation\n");
>                 return ret;
>         }


>
>         if (buf_len != sig_len) {
>                 debug("In RSAVerify(): hash length not same as sig len\n");
> --
> 1.8.1.4
>

Regards,
Simon


More information about the U-Boot mailing list