diff mbox series

[09/18] common: Move host-only logic in image-sig.c to separate file

Message ID 20210517163840.839097-10-mr.nuke.me@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series image: Reduce #ifdef abuse in image code | expand

Commit Message

Alexandru Gagniuc May 17, 2021, 4:38 p.m. UTC
image-sig.c is used to map a hash or crypto algorithm name to a
handler of that algorithm. There is some similarity between the host
and target variants, with the differences worked out by #ifdefs. The
purpose of this change is to remove those ifdefs.

First, copy the file to a host-only version, and remove target
specific code. Although it looks like we are duplicating code,
subsequent patches will change the way target algorithms are searched.
Besides we are only duplicating three string to struct mapping
functions. This isn't something to fuss about.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 tools/Makefile         |   5 +-
 tools/image-sig-host.c | 133 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 2 deletions(-)
 create mode 100644 tools/image-sig-host.c

Comments

Alexandru Gagniuc May 17, 2021, 7:47 p.m. UTC | #1
On 5/17/21 11:38 AM, Alexandru Gagniuc wrote:
> image-sig.c is used to map a hash or crypto algorithm name to a
> handler of that algorithm. There is some similarity between the host
> and target variants, with the differences worked out by #ifdefs. The
> purpose of this change is to remove those ifdefs.
> 
> First, copy the file to a host-only version, and remove target
> specific code. Although it looks like we are duplicating code,
> subsequent patches will change the way target algorithms are searched.
> Besides we are only duplicating three string to struct mapping
> functions. This isn't something to fuss about.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>   tools/Makefile         |   5 +-
>   tools/image-sig-host.c | 133 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 136 insertions(+), 2 deletions(-)
>   create mode 100644 tools/image-sig-host.c
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index d020c55d66..e39006b6f6 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -57,8 +57,9 @@ hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
>   
>   hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
>   
> -FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o common/image-fit-sig.o
> +FIT_OBJS-$(CONFIG_FIT) := image-sig-host.o fit_common.o fit_image.o \
> +			image-host.o common/image-fit.o
> +FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-fit-sig.o
>   FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o

This may cause a build failure with FIT_SIGNATURE disabled. I will have 
this fixed in v2. The correction is trivial.

Correct diff below for reference:

  FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o 
common/image-fit.o
-FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o 
common/image-fit-sig.o
+FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o 
common/image-fit-sig.o
Simon Glass May 19, 2021, 4:36 p.m. UTC | #2
On Mon, 17 May 2021 at 13:47, Alex G. <mr.nuke.me@gmail.com> wrote:
>
>
>
> On 5/17/21 11:38 AM, Alexandru Gagniuc wrote:
> > image-sig.c is used to map a hash or crypto algorithm name to a
> > handler of that algorithm. There is some similarity between the host
> > and target variants, with the differences worked out by #ifdefs. The
> > purpose of this change is to remove those ifdefs.
> >
> > First, copy the file to a host-only version, and remove target
> > specific code. Although it looks like we are duplicating code,
> > subsequent patches will change the way target algorithms are searched.
> > Besides we are only duplicating three string to struct mapping
> > functions. This isn't something to fuss about.
> >
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > ---
> >   tools/Makefile         |   5 +-
> >   tools/image-sig-host.c | 133 +++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 136 insertions(+), 2 deletions(-)
> >   create mode 100644 tools/image-sig-host.c
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index d020c55d66..e39006b6f6 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -57,8 +57,9 @@ hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
> >
> >   hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
> >
> > -FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
> > -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o common/image-fit-sig.o
> > +FIT_OBJS-$(CONFIG_FIT) := image-sig-host.o fit_common.o fit_image.o \
> > +                     image-host.o common/image-fit.o
> > +FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-fit-sig.o
> >   FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
>
> This may cause a build failure with FIT_SIGNATURE disabled. I will have
> this fixed in v2. The correction is trivial.

I see a build warning for an unused variable 'i', if that is what you mean.

>
> Correct diff below for reference:
>
>   FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o
> common/image-fit.o
> -FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o
> common/image-fit-sig.o
> +FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := image-sig-host.o
> common/image-fit-sig.o
>

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox series

Patch

diff --git a/tools/Makefile b/tools/Makefile
index d020c55d66..e39006b6f6 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -57,8 +57,9 @@  hostprogs-$(CONFIG_FIT_SIGNATURE) += fit_info fit_check_sign
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
-FIT_OBJS-$(CONFIG_FIT) := fit_common.o fit_image.o image-host.o common/image-fit.o
-FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o common/image-fit-sig.o
+FIT_OBJS-$(CONFIG_FIT) := image-sig-host.o fit_common.o fit_image.o \
+			image-host.o common/image-fit.o
+FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-fit-sig.o
 FIT_CIPHER_OBJS-$(CONFIG_FIT_CIPHER) := common/image-cipher.o
 
 # The following files are synced with upstream DTC.
diff --git a/tools/image-sig-host.c b/tools/image-sig-host.c
new file mode 100644
index 0000000000..8ed6998dab
--- /dev/null
+++ b/tools/image-sig-host.c
@@ -0,0 +1,133 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2013, Google Inc.
+ */
+
+#include "mkimage.h"
+#include <fdt_support.h>
+#include <time.h>
+#include <linux/libfdt.h>
+#include <image.h>
+#include <u-boot/ecdsa.h>
+#include <u-boot/rsa.h>
+#include <u-boot/hash-checksum.h>
+
+struct checksum_algo checksum_algos[] = {
+	{
+		.name = "sha1",
+		.checksum_len = SHA1_SUM_LEN,
+		.der_len = SHA1_DER_LEN,
+		.der_prefix = sha1_der_prefix,
+		.calculate_sign = EVP_sha1,
+		.calculate = hash_calculate,
+	},
+	{
+		.name = "sha256",
+		.checksum_len = SHA256_SUM_LEN,
+		.der_len = SHA256_DER_LEN,
+		.der_prefix = sha256_der_prefix,
+		.calculate_sign = EVP_sha256,
+		.calculate = hash_calculate,
+	},
+	{
+		.name = "sha384",
+		.checksum_len = SHA384_SUM_LEN,
+		.der_len = SHA384_DER_LEN,
+		.der_prefix = sha384_der_prefix,
+		.calculate_sign = EVP_sha384,
+		.calculate = hash_calculate,
+	},
+	{
+		.name = "sha512",
+		.checksum_len = SHA512_SUM_LEN,
+		.der_len = SHA512_DER_LEN,
+		.der_prefix = sha512_der_prefix,
+		.calculate_sign = EVP_sha512,
+		.calculate = hash_calculate,
+	},
+};
+
+struct crypto_algo crypto_algos[] = {
+	{
+		.name = "rsa2048",
+		.key_len = RSA2048_BYTES,
+		.sign = rsa_sign,
+		.add_verify_data = rsa_add_verify_data,
+		.verify = rsa_verify,
+	},
+	{
+		.name = "rsa4096",
+		.key_len = RSA4096_BYTES,
+		.sign = rsa_sign,
+		.add_verify_data = rsa_add_verify_data,
+		.verify = rsa_verify,
+	},
+	{
+		.name = "ecdsa256",
+		.key_len = ECDSA256_BYTES,
+		.sign = ecdsa_sign,
+		.add_verify_data = ecdsa_add_verify_data,
+		.verify = ecdsa_verify,
+	},
+};
+
+struct padding_algo padding_algos[] = {
+	{
+		.name = "pkcs-1.5",
+		.verify = padding_pkcs_15_verify,
+	},
+	{
+		.name = "pss",
+		.verify = padding_pss_verify,
+	}
+};
+
+struct checksum_algo *image_get_checksum_algo(const char *full_name)
+{
+	int i;
+	const char *name;
+
+	for (i = 0; i < ARRAY_SIZE(checksum_algos); i++) {
+		name = checksum_algos[i].name;
+		/* Make sure names match and next char is a comma */
+		if (!strncmp(name, full_name, strlen(name)) &&
+		    full_name[strlen(name)] == ',')
+			return &checksum_algos[i];
+	}
+
+	return NULL;
+}
+
+struct crypto_algo *image_get_crypto_algo(const char *full_name)
+{
+	int i;
+	const char *name;
+
+	/* Move name to after the comma */
+	name = strchr(full_name, ',');
+	if (!name)
+		return NULL;
+	name += 1;
+
+	for (i = 0; i < ARRAY_SIZE(crypto_algos); i++) {
+		if (!strcmp(crypto_algos[i].name, name))
+			return &crypto_algos[i];
+	}
+
+	return NULL;
+}
+
+struct padding_algo *image_get_padding_algo(const char *name)
+{
+	int i;
+
+	if (!name)
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(padding_algos); i++) {
+		if (!strcmp(padding_algos[i].name, name))
+			return &padding_algos[i];
+	}
+
+	return NULL;
+}