diff mbox series

[v4,2/6] lib/rsa: Make fdt_add_bignum() available outside of RSA code

Message ID 20210108191737.615022-3-mr.nuke.me@gmail.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series Add support for ECDSA image signing (with test) | expand

Commit Message

Alexandru Gagniuc Jan. 8, 2021, 7:17 p.m. UTC
fdt_add_bignum() is useful for algorithms other than just RSA. To
allow its use for ECDSA, move it to a common file under lib/.

The new file is suffixed with '-libcrypto' because it has a direct
dependency on openssl. This is due to the use of the "BIGNUM *" type.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 include/u-boot/fdt-libcrypto.h | 27 +++++++++++++
 lib/fdt-libcrypto.c            | 72 ++++++++++++++++++++++++++++++++++
 lib/rsa/rsa-sign.c             | 65 +-----------------------------
 tools/Makefile                 |  1 +
 4 files changed, 101 insertions(+), 64 deletions(-)
 create mode 100644 include/u-boot/fdt-libcrypto.h
 create mode 100644 lib/fdt-libcrypto.c

Comments

Patrick DELAUNAY Jan. 27, 2021, 7:10 p.m. UTC | #1
Hi,

On 1/8/21 8:17 PM, Alexandru Gagniuc wrote:
> fdt_add_bignum() is useful for algorithms other than just RSA. To
> allow its use for ECDSA, move it to a common file under lib/.
>
> The new file is suffixed with '-libcrypto' because it has a direct
> dependency on openssl. This is due to the use of the "BIGNUM *" type.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>   include/u-boot/fdt-libcrypto.h | 27 +++++++++++++
>   lib/fdt-libcrypto.c            | 72 ++++++++++++++++++++++++++++++++++
>   lib/rsa/rsa-sign.c             | 65 +-----------------------------
>   tools/Makefile                 |  1 +
>   4 files changed, 101 insertions(+), 64 deletions(-)
>   create mode 100644 include/u-boot/fdt-libcrypto.h
>   create mode 100644 lib/fdt-libcrypto.c
(...)
> diff --git a/tools/Makefile b/tools/Makefile
> index b1595ad814..af7698fd01 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -106,6 +106,7 @@ dumpimage-mkimage-objs := aisimage.o \
>   			socfpgaimage.o \
>   			lib/crc16.o \
>   			lib/hash-checksum.o \
> +			lib/fdt-libcrypto.o \
>   			lib/sha1.o \
>   			lib/sha256.o \
>   			lib/sha512.o \


This part cause compilation issue when CONFIG_FIT_SIGNATURE is not 
defined / when openssl is not used

fdt-libcrypto.c:(.text+0x2f): undefined reference to `BN_new'
fdt-libcrypto.c:(.text+0x37): undefined reference to `BN_new'
fdt-libcrypto.c:(.text+0x44): undefined reference to `BN_new'
fdt-libcrypto.c:(.text+0x51): undefined reference to `BN_new'
fdt-libcrypto.c:(.text+0x7d): undefined reference to `BN_CTX_new'
fdt-libcrypto.c:(.text+0x9d): undefined reference to `BN_set_word'
fdt-libcrypto.c:(.text+0xaf): undefined reference to `BN_set_word'
fdt-libcrypto.c:(.text+0xc5): undefined reference to `BN_exp'
fdt-libcrypto.c:(.text+0x135): undefined reference to `BN_div'
fdt-libcrypto.c:(.text+0x13d): undefined reference to `BN_get_word'
fdt-libcrypto.c:(.text+0x172): undefined reference to `BN_rshift'
fdt-libcrypto.c:(.text+0x1a9): undefined reference to `BN_free'
fdt-libcrypto.c:(.text+0x1b3): undefined reference to `BN_free'
fdt-libcrypto.c:(.text+0x1bd): undefined reference to `BN_free'
fdt-libcrypto.c:(.text+0x1c5): undefined reference to `BN_free'

libssl not always include in mkimage, see the lines:

   # MXSImage needs LibSSL
   ifneq 
($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
   HOSTCFLAGS_kwbimage.o += \
       $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
   HOSTLDLIBS_mkimage += \
       $(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo 
"-lssl -lcrypto")


=> I propose to add the added lib only when CONFIG_FIT_SIGNATURE is

       activated by removing dumpimage-mkimage-objs udpate and with:


- FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o 
common/image-fit-sig.o

+ FIT_SIG_OBJS-$(CONFIG_FIT_SIGNATURE) := common/image-sig.o 
common/image-fit-sig.o lib/fdt-libcrypto.o


Regards

Patrick
Tom Rini Jan. 28, 2021, 12:57 a.m. UTC | #2
On Fri, Jan 08, 2021 at 01:17:33PM -0600, Alexandru Gagniuc wrote:

> fdt_add_bignum() is useful for algorithms other than just RSA. To
> allow its use for ECDSA, move it to a common file under lib/.
> 
> The new file is suffixed with '-libcrypto' because it has a direct
> dependency on openssl. This is due to the use of the "BIGNUM *" type.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

This makes a large number of platform tools fail to build such as
j7200_evm_a72.
diff mbox series

Patch

diff --git a/include/u-boot/fdt-libcrypto.h b/include/u-boot/fdt-libcrypto.h
new file mode 100644
index 0000000000..5142f37039
--- /dev/null
+++ b/include/u-boot/fdt-libcrypto.h
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>
+ * Copyright (c) 2013, Google Inc.
+ */
+
+#ifndef _FDT_LIBCRYPTO_H
+#define _FDT_LIBCRYPTO_H
+
+#include <openssl/bn.h>
+
+/**
+ * fdt_add_bignum() - Write a libcrypto BIGNUM as an FDT property
+ *
+ * Convert a libcrypto BIGNUM * into a big endian array of integers.
+ *
+ * @blob:	FDT blob to modify
+ * @noffset:	Offset of the FDT node
+ * @prop_name:	What to call the property in the FDT
+ * @num:	pointer to a libcrypto big number
+ * @num_bits:	How big is 'num' in bits?
+ * @return 0 if all good all working, -ve on horror
+ */
+int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
+		   BIGNUM *num, int num_bits);
+
+#endif /* _FDT_LIBCRYPTO_H */
diff --git a/lib/fdt-libcrypto.c b/lib/fdt-libcrypto.c
new file mode 100644
index 0000000000..ecb0344c8f
--- /dev/null
+++ b/lib/fdt-libcrypto.c
@@ -0,0 +1,72 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>
+ * Copyright (c) 2013, Google Inc.
+ */
+
+#include <libfdt.h>
+#include <u-boot/fdt-libcrypto.h>
+
+int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
+		   BIGNUM *num, int num_bits)
+{
+	int nwords = num_bits / 32;
+	int size;
+	uint32_t *buf, *ptr;
+	BIGNUM *tmp, *big2, *big32, *big2_32;
+	BN_CTX *ctx;
+	int ret;
+
+	tmp = BN_new();
+	big2 = BN_new();
+	big32 = BN_new();
+	big2_32 = BN_new();
+
+	/*
+	 * Note: This code assumes that all of the above succeed, or all fail.
+	 * In practice memory allocations generally do not fail (unless the
+	 * process is killed), so it does not seem worth handling each of these
+	 * as a separate case. Technicaly this could leak memory on failure,
+	 * but a) it won't happen in practice, and b) it doesn't matter as we
+	 * will immediately exit with a failure code.
+	 */
+	if (!tmp || !big2 || !big32 || !big2_32) {
+		fprintf(stderr, "Out of memory (bignum)\n");
+		return -ENOMEM;
+	}
+	ctx = BN_CTX_new();
+	if (!ctx) {
+		fprintf(stderr, "Out of memory (bignum context)\n");
+		return -ENOMEM;
+	}
+	BN_set_word(big2, 2L);
+	BN_set_word(big32, 32L);
+	BN_exp(big2_32, big2, big32, ctx); /* B = 2^32 */
+
+	size = nwords * sizeof(uint32_t);
+	buf = malloc(size);
+	if (!buf) {
+		fprintf(stderr, "Out of memory (%d bytes)\n", size);
+		return -ENOMEM;
+	}
+
+	/* Write out modulus as big endian array of integers */
+	for (ptr = buf + nwords - 1; ptr >= buf; ptr--) {
+		BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */
+		*ptr = cpu_to_fdt32(BN_get_word(tmp));
+		BN_rshift(num, num, 32); /*  N = N/B */
+	}
+
+	/*
+	 * We try signing with successively increasing size values, so this
+	 * might fail several times
+	 */
+	ret = fdt_setprop(blob, noffset, prop_name, buf, size);
+	free(buf);
+	BN_free(tmp);
+	BN_free(big2);
+	BN_free(big32);
+	BN_free(big2_32);
+
+	return ret ? -FDT_ERR_NOSPACE : 0;
+}
diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index 1f0d81bd7a..557c690a6d 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -9,6 +9,7 @@ 
 #include <string.h>
 #include <image.h>
 #include <time.h>
+#include <u-boot/fdt-libcrypto.h>
 #include <openssl/bn.h>
 #include <openssl/rsa.h>
 #include <openssl/pem.h>
@@ -680,70 +681,6 @@  int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
 	return ret;
 }
 
-static int fdt_add_bignum(void *blob, int noffset, const char *prop_name,
-			  BIGNUM *num, int num_bits)
-{
-	int nwords = num_bits / 32;
-	int size;
-	uint32_t *buf, *ptr;
-	BIGNUM *tmp, *big2, *big32, *big2_32;
-	BN_CTX *ctx;
-	int ret;
-
-	tmp = BN_new();
-	big2 = BN_new();
-	big32 = BN_new();
-	big2_32 = BN_new();
-
-	/*
-	 * Note: This code assumes that all of the above succeed, or all fail.
-	 * In practice memory allocations generally do not fail (unless the
-	 * process is killed), so it does not seem worth handling each of these
-	 * as a separate case. Technicaly this could leak memory on failure,
-	 * but a) it won't happen in practice, and b) it doesn't matter as we
-	 * will immediately exit with a failure code.
-	 */
-	if (!tmp || !big2 || !big32 || !big2_32) {
-		fprintf(stderr, "Out of memory (bignum)\n");
-		return -ENOMEM;
-	}
-	ctx = BN_CTX_new();
-	if (!ctx) {
-		fprintf(stderr, "Out of memory (bignum context)\n");
-		return -ENOMEM;
-	}
-	BN_set_word(big2, 2L);
-	BN_set_word(big32, 32L);
-	BN_exp(big2_32, big2, big32, ctx); /* B = 2^32 */
-
-	size = nwords * sizeof(uint32_t);
-	buf = malloc(size);
-	if (!buf) {
-		fprintf(stderr, "Out of memory (%d bytes)\n", size);
-		return -ENOMEM;
-	}
-
-	/* Write out modulus as big endian array of integers */
-	for (ptr = buf + nwords - 1; ptr >= buf; ptr--) {
-		BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */
-		*ptr = cpu_to_fdt32(BN_get_word(tmp));
-		BN_rshift(num, num, 32); /*  N = N/B */
-	}
-
-	/*
-	 * We try signing with successively increasing size values, so this
-	 * might fail several times
-	 */
-	ret = fdt_setprop(blob, noffset, prop_name, buf, size);
-	free(buf);
-	BN_free(tmp);
-	BN_free(big2);
-	BN_free(big32);
-	BN_free(big2_32);
-
-	return ret ? -FDT_ERR_NOSPACE : 0;
-}
-
 int rsa_add_verify_data(struct image_sign_info *info, void *keydest)
 {
 	BIGNUM *modulus, *r_squared;
diff --git a/tools/Makefile b/tools/Makefile
index b1595ad814..af7698fd01 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -106,6 +106,7 @@  dumpimage-mkimage-objs := aisimage.o \
 			socfpgaimage.o \
 			lib/crc16.o \
 			lib/hash-checksum.o \
+			lib/fdt-libcrypto.o \
 			lib/sha1.o \
 			lib/sha256.o \
 			lib/sha512.o \