[U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically

public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed

* [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically
       [not found] <20161108185329.56774-1-aduda@meraki.com>
@ 2016-11-08 18:53 ` aduda
  2016-11-11 16:17   ` Simon Glass
  2016-11-22  2:55   ` [U-Boot] [U-Boot,2/4] " Tom Rini
  2016-11-08 18:53 ` [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info aduda
  2016-11-08 18:53 ` [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info aduda
  2 siblings, 2 replies; 11+ messages in thread
From: aduda @ 2016-11-08 18:53 UTC (permalink / raw)
  To: u-boot

From: Andrew Duda <aduda@meraki.com>

Padding verification was done against static SHA/RSA pair arrays which
take up a lot of static memory, are mostly 0xff, and cannot be reused
for additional SHA/RSA pairings. The padding can be easily computed
according to PKCS#1v2.1 as:

  EM = 0x00 || 0x01 || PS || 0x00 || T

where PS is (emLen - tLen - 3) octets of 0xff and T is DER encoding
of the hash.

Store DER prefix in checksum_algo and create rsa_verify_padding
function to handle verification of a message for any SHA/RSA pairing.

Signed-off-by: Andrew Duda <aduda@meraki.com>
Signed-off-by: aduda <aduda@meraki.com>
---

 common/image-sig.c            |   9 ++--
 include/image.h               |   3 +-
 include/u-boot/rsa-checksum.h |   4 --
 include/u-boot/sha1.h         |   3 ++
 include/u-boot/sha256.h       |   3 ++
 lib/rsa/rsa-checksum.c        | 121 ------------------------------------------
 lib/rsa/rsa-verify.c          |  38 ++++++++++++-
 lib/sha1.c                    |   5 ++
 lib/sha256.c                  |   6 +++
 9 files changed, 61 insertions(+), 131 deletions(-)

diff --git a/common/image-sig.c b/common/image-sig.c
index 28f7a20..008d2c5 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -34,32 +34,35 @@ struct checksum_algo checksum_algos[] = {
 	{
 		"sha1",
 		SHA1_SUM_LEN,
+		SHA1_DER_LEN,
+		sha1_der_prefix,
 		RSA2048_BYTES,
 #if IMAGE_ENABLE_SIGN
 		EVP_sha1,
 #endif
 		hash_calculate,
-		padding_sha1_rsa2048,
 	},
 	{
 		"sha256",
 		SHA256_SUM_LEN,
+		SHA256_DER_LEN,
+		sha256_der_prefix,
 		RSA2048_BYTES,
 #if IMAGE_ENABLE_SIGN
 		EVP_sha256,
 #endif
 		hash_calculate,
-		padding_sha256_rsa2048,
 	},
 	{
 		"sha256",
 		SHA256_SUM_LEN,
+		SHA256_DER_LEN,
+		sha256_der_prefix,
 		RSA4096_BYTES,
 #if IMAGE_ENABLE_SIGN
 		EVP_sha256,
 #endif
 		hash_calculate,
-		padding_sha256_rsa4096,
 	}
 
 };
diff --git a/include/image.h b/include/image.h
index bfe10a0..de73a07 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1070,6 +1070,8 @@ struct image_region {
 struct checksum_algo {
 	const char *name;
 	const int checksum_len;
+	const int der_len;
+	const uint8_t *der_prefix;
 	const int key_len;
 #if IMAGE_ENABLE_SIGN
 	const EVP_MD *(*calculate_sign)(void);
@@ -1077,7 +1079,6 @@ struct checksum_algo {
 	int (*calculate)(const char *name,
 			 const struct image_region region[],
 			 int region_count, uint8_t *checksum);
-	const uint8_t *rsa_padding;
 };
 
 struct image_sig_algo {
diff --git a/include/u-boot/rsa-checksum.h b/include/u-boot/rsa-checksum.h
index 3c69d85..c240720 100644
--- a/include/u-boot/rsa-checksum.h
+++ b/include/u-boot/rsa-checksum.h
@@ -12,10 +12,6 @@
 #include <u-boot/sha1.h>
 #include <u-boot/sha256.h>
 
-extern const uint8_t padding_sha256_rsa4096[];
-extern const uint8_t padding_sha256_rsa2048[];
-extern const uint8_t padding_sha1_rsa2048[];
-
 /**
  * hash_calculate() - Calculate hash over the data
  *
diff --git a/include/u-boot/sha1.h b/include/u-boot/sha1.h
index b0d9ce9..2634a29 100644
--- a/include/u-boot/sha1.h
+++ b/include/u-boot/sha1.h
@@ -21,6 +21,9 @@ extern "C" {
 
 #define SHA1_SUM_POS	-0x20
 #define SHA1_SUM_LEN	20
+#define SHA1_DER_LEN	15
+
+extern const uint8_t sha1_der_prefix[];
 
 /**
  * \brief	   SHA-1 context structure
diff --git a/include/u-boot/sha256.h b/include/u-boot/sha256.h
index beadab3..9aa1251 100644
--- a/include/u-boot/sha256.h
+++ b/include/u-boot/sha256.h
@@ -2,6 +2,9 @@
 #define _SHA256_H
 
 #define SHA256_SUM_LEN	32
+#define SHA256_DER_LEN	19
+
+extern const uint8_t sha256_der_prefix[];
 
 /* Reset watchdog each time we process this many bytes */
 #define CHUNKSZ_SHA256	(64 * 1024)
diff --git a/lib/rsa/rsa-checksum.c b/lib/rsa/rsa-checksum.c
index db183ff..2bf28e2 100644
--- a/lib/rsa/rsa-checksum.c
+++ b/lib/rsa/rsa-checksum.c
@@ -13,130 +13,9 @@
 #include <hash.h>
 #else
 #include "fdt_host.h"
-#include <u-boot/sha1.h>
-#include <u-boot/sha256.h>
 #endif
 #include <u-boot/rsa.h>
 
-/* PKCS 1.5 paddings as described in the RSA PKCS#1 v2.1 standard. */
-
-const uint8_t padding_sha256_rsa2048[RSA2048_BYTES - SHA256_SUM_LEN] = {
-0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x30, 0x31, 0x30,
-0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05,
-0x00, 0x04, 0x20
-};
-
-const uint8_t padding_sha1_rsa2048[RSA2048_BYTES - SHA1_SUM_LEN] = {
-	0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0x00, 0x30, 0x21, 0x30,
-	0x09, 0x06, 0x05, 0x2b, 0x0e, 0x03, 0x02, 0x1a,
-	0x05, 0x00, 0x04, 0x14
-};
-
-const uint8_t padding_sha256_rsa4096[RSA4096_BYTES - SHA256_SUM_LEN] = {
-	0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
-	0xff, 0xff, 0xff, 0xff, 0x00, 0x30, 0x31, 0x30,
-	0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65,
-	0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20
-};
-
 int hash_calculate(const char *name,
 		    const struct image_region region[],
 		    int region_count, uint8_t *checksum)
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 5418f59..ee8988d 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -25,6 +25,40 @@
 #define RSA_DEFAULT_PUBEXP	65537
 
 /**
+ * rsa_verify_padding() - Verify RSA message padding is valid
+ *
+ * Verify a RSA message's padding is consistent with PKCS1.5
+ * padding as described in the RSA PKCS#1 v2.1 standard.
+ *
+ * @msg:	Padded message
+ * @pad_len:	Number of expected padding bytes
+ * @algo:	Checksum algo structure having information on DER encoding etc.
+ * @return 0 on success, != 0 on failure
+ */
+static int rsa_verify_padding(const uint8_t *msg, const int pad_len,
+			      struct checksum_algo *algo)
+{
+	int ff_len;
+	int ret;
+
+	/* first byte must be 0x00 */
+	ret = *msg++;
+	/* second byte must be 0x01 */
+	ret |= *msg++ ^ 0x01;
+	/* next ff_len bytes must be 0xff */
+	ff_len = pad_len - algo->der_len - 3;
+	ret |= *msg ^ 0xff;
+	ret |= memcmp(msg, msg+1, ff_len-1);
+	msg += ff_len;
+	/* next byte must be 0x00 */
+	ret |= *msg++;
+	/* next der_len bytes must match der_prefix */
+	ret |= memcmp(msg, algo->der_prefix, algo->der_len);
+
+	return ret;
+}
+
+/**
  * rsa_verify_key() - Verify a signature against some data using RSA Key
  *
  * Verify a RSA PKCS1.5 signature against an expected hash using
@@ -83,11 +117,11 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig,
 		return ret;
 	}
 
-	padding = algo->rsa_padding;
 	pad_len = algo->key_len - algo->checksum_len;
 
 	/* Check pkcs1.5 padding bytes. */
-	if (memcmp(buf, padding, pad_len)) {
+	ret = rsa_verify_padding(buf, pad_len, algo);
+	if (ret) {
 		debug("In RSAVerify(): Padding check failed!\n");
 		return -EINVAL;
 	}
diff --git a/lib/sha1.c b/lib/sha1.c
index 72c5dea..f54bb5b 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -26,6 +26,11 @@
 #include <watchdog.h>
 #include <u-boot/sha1.h>
 
+const uint8_t sha1_der_prefix[SHA1_DER_LEN] = {
+	0x30, 0x21, 0x30, 0x09, 0x06, 0x05, 0x2b, 0x0e,
+	0x03, 0x02, 0x1a, 0x05, 0x00, 0x04, 0x14
+};
+
 /*
  * 32-bit integer manipulation macros (big endian)
  */
diff --git a/lib/sha256.c b/lib/sha256.c
index bb338ba..7f5a361 100644
--- a/lib/sha256.c
+++ b/lib/sha256.c
@@ -15,6 +15,12 @@
 #include <watchdog.h>
 #include <u-boot/sha256.h>
 
+const uint8_t sha256_der_prefix[SHA256_DER_LEN] = {
+	0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86,
+	0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05,
+	0x00, 0x04, 0x20
+};
+
 /*
  * 32-bit integer manipulation macros (big endian)
  */
-- 
2.10.2

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

* [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically
  2016-11-08 18:53 ` [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically aduda
@ 2016-11-11 16:17   ` Simon Glass
  2016-11-11 21:22     ` Andrew Duda
  2016-11-22  2:55   ` [U-Boot] [U-Boot,2/4] " Tom Rini
  1 sibling, 1 reply; 11+ messages in thread
From: Simon Glass @ 2016-11-11 16:17 UTC (permalink / raw)
  To: u-boot

On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote:
> From: Andrew Duda <aduda@meraki.com>
>
> Padding verification was done against static SHA/RSA pair arrays which
> take up a lot of static memory, are mostly 0xff, and cannot be reused
> for additional SHA/RSA pairings. The padding can be easily computed
> according to PKCS#1v2.1 as:
>
>   EM = 0x00 || 0x01 || PS || 0x00 || T
>
> where PS is (emLen - tLen - 3) octets of 0xff and T is DER encoding
> of the hash.
>
> Store DER prefix in checksum_algo and create rsa_verify_padding
> function to handle verification of a message for any SHA/RSA pairing.
>
> Signed-off-by: Andrew Duda <aduda@meraki.com>
> Signed-off-by: aduda <aduda@meraki.com>
> ---
>
>  common/image-sig.c            |   9 ++--
>  include/image.h               |   3 +-
>  include/u-boot/rsa-checksum.h |   4 --
>  include/u-boot/sha1.h         |   3 ++
>  include/u-boot/sha256.h       |   3 ++
>  lib/rsa/rsa-checksum.c        | 121 ------------------------------------------
>  lib/rsa/rsa-verify.c          |  38 ++++++++++++-
>  lib/sha1.c                    |   5 ++
>  lib/sha256.c                  |   6 +++
>  9 files changed, 61 insertions(+), 131 deletions(-)

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

How much memory does this save?

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

* [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically
  2016-11-11 16:17   ` Simon Glass
@ 2016-11-11 21:22     ` Andrew Duda
  2016-11-14 19:04       ` Simon Glass
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Duda @ 2016-11-11 21:22 UTC (permalink / raw)
  To: u-boot

Simon,

So I looked into this more after you asked this, and it looks very
platform dependent. I tested on two builds: sandbox and a version of
x86-common. The before/after for sandbox image was
5486016-5486800(+784). The before/after for my x86 build was
3306100-3305908(-192). So memory saving is anywhere from a few bytes
to actually more space. But the big motivation is the next two patches
depend on this change.

Thanks,
Andrew

On Fri, Nov 11, 2016 at 8:17 AM, Simon Glass <sjg@chromium.org> wrote:
> On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote:
>> From: Andrew Duda <aduda@meraki.com>
>>
>> Padding verification was done against static SHA/RSA pair arrays which
>> take up a lot of static memory, are mostly 0xff, and cannot be reused
>> for additional SHA/RSA pairings. The padding can be easily computed
>> according to PKCS#1v2.1 as:
>>
>>   EM = 0x00 || 0x01 || PS || 0x00 || T
>>
>> where PS is (emLen - tLen - 3) octets of 0xff and T is DER encoding
>> of the hash.
>>
>> Store DER prefix in checksum_algo and create rsa_verify_padding
>> function to handle verification of a message for any SHA/RSA pairing.
>>
>> Signed-off-by: Andrew Duda <aduda@meraki.com>
>> Signed-off-by: aduda <aduda@meraki.com>
>> ---
>>
>>  common/image-sig.c            |   9 ++--
>>  include/image.h               |   3 +-
>>  include/u-boot/rsa-checksum.h |   4 --
>>  include/u-boot/sha1.h         |   3 ++
>>  include/u-boot/sha256.h       |   3 ++
>>  lib/rsa/rsa-checksum.c        | 121 ------------------------------------------
>>  lib/rsa/rsa-verify.c          |  38 ++++++++++++-
>>  lib/sha1.c                    |   5 ++
>>  lib/sha256.c                  |   6 +++
>>  9 files changed, 61 insertions(+), 131 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> How much memory does this save?

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

* [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically
  2016-11-11 21:22     ` Andrew Duda
@ 2016-11-14 19:04       ` Simon Glass
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2016-11-14 19:04 UTC (permalink / raw)
  To: u-boot

Hi Andrew,

On 11 November 2016 at 14:22, Andrew Duda <andrew.duda@meraki.net> wrote:
> Simon,
>
> So I looked into this more after you asked this, and it looks very
> platform dependent. I tested on two builds: sandbox and a version of
> x86-common. The before/after for sandbox image was
> 5486016-5486800(+784). The before/after for my x86 build was
> 3306100-3305908(-192). So memory saving is anywhere from a few bytes
> to actually more space. But the big motivation is the next two patches
> depend on this change.

OK, well I'm not worried about sandbox, and seeing a saving on a real
x86 board is comforting.

BTW please try not to top-post as it confuses the history.

Regards,
Simon

>
> Thanks,
> Andrew
>
> On Fri, Nov 11, 2016 at 8:17 AM, Simon Glass <sjg@chromium.org> wrote:
>> On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote:
>>> From: Andrew Duda <aduda@meraki.com>
>>>
>>> Padding verification was done against static SHA/RSA pair arrays which
>>> take up a lot of static memory, are mostly 0xff, and cannot be reused
>>> for additional SHA/RSA pairings. The padding can be easily computed
>>> according to PKCS#1v2.1 as:
>>>
>>>   EM = 0x00 || 0x01 || PS || 0x00 || T
>>>
>>> where PS is (emLen - tLen - 3) octets of 0xff and T is DER encoding
>>> of the hash.
>>>
>>> Store DER prefix in checksum_algo and create rsa_verify_padding
>>> function to handle verification of a message for any SHA/RSA pairing.
>>>
>>> Signed-off-by: Andrew Duda <aduda@meraki.com>
>>> Signed-off-by: aduda <aduda@meraki.com>
>>> ---
>>>
>>>  common/image-sig.c            |   9 ++--
>>>  include/image.h               |   3 +-
>>>  include/u-boot/rsa-checksum.h |   4 --
>>>  include/u-boot/sha1.h         |   3 ++
>>>  include/u-boot/sha256.h       |   3 ++
>>>  lib/rsa/rsa-checksum.c        | 121 ------------------------------------------
>>>  lib/rsa/rsa-verify.c          |  38 ++++++++++++-
>>>  lib/sha1.c                    |   5 ++
>>>  lib/sha256.c                  |   6 +++
>>>  9 files changed, 61 insertions(+), 131 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> How much memory does this save?

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

* [U-Boot] [U-Boot,2/4] rsa: Verify RSA padding programatically
  2016-11-08 18:53 ` [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically aduda
  2016-11-11 16:17   ` Simon Glass
@ 2016-11-22  2:55   ` Tom Rini
  1 sibling, 0 replies; 11+ messages in thread
From: Tom Rini @ 2016-11-22  2:55 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 08, 2016 at 06:53:40PM +0000, aduda wrote:

> From: Andrew Duda <aduda@meraki.com>
> 
> Padding verification was done against static SHA/RSA pair arrays which
> take up a lot of static memory, are mostly 0xff, and cannot be reused
> for additional SHA/RSA pairings. The padding can be easily computed
> according to PKCS#1v2.1 as:
> 
>   EM = 0x00 || 0x01 || PS || 0x00 || T
> 
> where PS is (emLen - tLen - 3) octets of 0xff and T is DER encoding
> of the hash.
> 
> Store DER prefix in checksum_algo and create rsa_verify_padding
> function to handle verification of a message for any SHA/RSA pairing.
> 
> Signed-off-by: Andrew Duda <aduda@meraki.com>
> Signed-off-by: aduda <aduda@meraki.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161121/84dc47cd/attachment.sig>

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

* [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info
       [not found] <20161108185329.56774-1-aduda@meraki.com>
  2016-11-08 18:53 ` [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically aduda
@ 2016-11-08 18:53 ` aduda
  2016-11-11 16:17   ` Simon Glass
  2016-11-22  2:56   ` [U-Boot] [U-Boot, " Tom Rini
  2016-11-08 18:53 ` [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info aduda
  2 siblings, 2 replies; 11+ messages in thread
From: aduda @ 2016-11-08 18:53 UTC (permalink / raw)
  To: u-boot

From: Andrew Duda <aduda@meraki.com>

Cut down on the repetition of algorithm information by defining separate
checksum and crypto structs. image_sig_algos are now simply pairs of
unique checksum and crypto algos.

Signed-off-by: Andrew Duda <aduda@meraki.com>
Signed-off-by: aduda <aduda@meraki.com>
---

 common/image-sig.c   | 46 +++++++++++++++++++++++-----------------------
 include/image.h      |  9 +++++++--
 lib/rsa/rsa-verify.c | 19 +++++++++++--------
 tools/image-host.c   |  9 +++++----
 4 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/common/image-sig.c b/common/image-sig.c
index 008d2c5..8b4314d 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -36,7 +36,6 @@ struct checksum_algo checksum_algos[] = {
 		SHA1_SUM_LEN,
 		SHA1_DER_LEN,
 		sha1_der_prefix,
-		RSA2048_BYTES,
 #if IMAGE_ENABLE_SIGN
 		EVP_sha1,
 #endif
@@ -47,22 +46,28 @@ struct checksum_algo checksum_algos[] = {
 		SHA256_SUM_LEN,
 		SHA256_DER_LEN,
 		sha256_der_prefix,
-		RSA2048_BYTES,
 #if IMAGE_ENABLE_SIGN
 		EVP_sha256,
 #endif
 		hash_calculate,
+	}
+
+};
+
+struct crypto_algo crypto_algos[] = {
+	{
+		"rsa2048",
+		RSA2048_BYTES,
+		rsa_sign,
+		rsa_add_verify_data,
+		rsa_verify,
 	},
 	{
-		"sha256",
-		SHA256_SUM_LEN,
-		SHA256_DER_LEN,
-		sha256_der_prefix,
+		"rsa4096",
 		RSA4096_BYTES,
-#if IMAGE_ENABLE_SIGN
-		EVP_sha256,
-#endif
-		hash_calculate,
+		rsa_sign,
+		rsa_add_verify_data,
+		rsa_verify,
 	}
 
 };
@@ -70,24 +75,18 @@ struct checksum_algo checksum_algos[] = {
 struct image_sig_algo image_sig_algos[] = {
 	{
 		"sha1,rsa2048",
-		rsa_sign,
-		rsa_add_verify_data,
-		rsa_verify,
+		&crypto_algos[0],
 		&checksum_algos[0],
 	},
 	{
 		"sha256,rsa2048",
-		rsa_sign,
-		rsa_add_verify_data,
-		rsa_verify,
+		&crypto_algos[0],
 		&checksum_algos[1],
 	},
 	{
 		"sha256,rsa4096",
-		rsa_sign,
-		rsa_add_verify_data,
-		rsa_verify,
-		&checksum_algos[2],
+		&crypto_algos[1],
+		&checksum_algos[1],
 	}
 
 };
@@ -197,7 +196,8 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data,
 	region.data = data;
 	region.size = size;
 
-	if (info.algo->verify(&info, &region, 1, fit_value, fit_value_len)) {
+	if (info.algo->crypto->verify(&info, &region, 1, fit_value,
+				      fit_value_len)) {
 		*err_msgp = "Verification failed";
 		return -1;
 	}
@@ -378,8 +378,8 @@ int fit_config_check_sig(const void *fit, int noffset, int required_keynode,
 	struct image_region region[count];
 
 	fit_region_make_list(fit, fdt_regions, count, region);
-	if (info.algo->verify(&info, region, count, fit_value,
-			      fit_value_len)) {
+	if (info.algo->crypto->verify(&info, region, count, fit_value,
+				      fit_value_len)) {
 		*err_msgp = "Verification failed";
 		return -1;
 	}
diff --git a/include/image.h b/include/image.h
index de73a07..c3c9866 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1072,7 +1072,6 @@ struct checksum_algo {
 	const int checksum_len;
 	const int der_len;
 	const uint8_t *der_prefix;
-	const int key_len;
 #if IMAGE_ENABLE_SIGN
 	const EVP_MD *(*calculate_sign)(void);
 #endif
@@ -1081,8 +1080,9 @@ struct checksum_algo {
 			 int region_count, uint8_t *checksum);
 };
 
-struct image_sig_algo {
+struct crypto_algo {
 	const char *name;		/* Name of algorithm */
+	const int key_len;
 
 	/**
 	 * sign() - calculate and return signature for given input data
@@ -1131,7 +1131,12 @@ struct image_sig_algo {
 	int (*verify)(struct image_sign_info *info,
 		      const struct image_region region[], int region_count,
 		      uint8_t *sig, uint sig_len);
+};
 
+struct image_sig_algo {
+	const char *name;
+	/* pointer to cryptosystem algorithm */
+	struct crypto_algo *crypto;
 	/* pointer to checksum algorithm */
 	struct checksum_algo *checksum;
 };
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index ee8988d..61dc4c2 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -68,14 +68,14 @@ static int rsa_verify_padding(const uint8_t *msg, const int pad_len,
  * @sig:	Signature
  * @sig_len:	Number of bytes in signature
  * @hash:	Pointer to the expected hash
- * @algo:	Checksum algo structure having information on RSA padding etc.
+ * @key_len:	Number of bytes in rsa key
+ * @algo:	Checksum algo structure having information on DER encoding etc.
  * @return 0 if verified, -ve on error
  */
 static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig,
 			  const uint32_t sig_len, const uint8_t *hash,
-			  struct checksum_algo *algo)
+			  const uint32_t key_len, struct checksum_algo *algo)
 {
-	const uint8_t *padding;
 	int pad_len;
 	int ret;
 #if !defined(USE_HOSTCC)
@@ -117,7 +117,7 @@ static int rsa_verify_key(struct key_prop *prop, const uint8_t *sig,
 		return ret;
 	}
 
-	pad_len = algo->key_len - algo->checksum_len;
+	pad_len = key_len - algo->checksum_len;
 
 	/* Check pkcs1.5 padding bytes. */
 	ret = rsa_verify_padding(buf, pad_len, algo);
@@ -183,7 +183,9 @@ static int rsa_verify_with_keynode(struct image_sign_info *info,
 		return -EFAULT;
 	}
 
-	ret = rsa_verify_key(&prop, sig, sig_len, hash, info->algo->checksum);
+	ret = rsa_verify_key(&prop, sig, sig_len, hash,
+			     info->algo->crypto->key_len,
+			     info->algo->checksum);
 
 	return ret;
 }
@@ -194,7 +196,7 @@ int rsa_verify(struct image_sign_info *info,
 {
 	const void *blob = info->fdt_blob;
 	/* Reserve memory for maximum checksum-length */
-	uint8_t hash[info->algo->checksum->key_len];
+	uint8_t hash[info->algo->crypto->key_len];
 	int ndepth, noffset;
 	int sig_node, node;
 	char name[100];
@@ -205,9 +207,10 @@ int rsa_verify(struct image_sign_info *info,
 	 * rsa-signature-length
 	 */
 	if (info->algo->checksum->checksum_len >
-	    info->algo->checksum->key_len) {
+	    info->algo->crypto->key_len) {
 		debug("%s: invlaid checksum-algorithm %s for %s\n",
-		      __func__, info->algo->checksum->name, info->algo->name);
+		      __func__, info->algo->checksum->name,
+		      info->algo->crypto->name);
 		return -EINVAL;
 	}
 
diff --git a/tools/image-host.c b/tools/image-host.c
index 1104695..dac85b4 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -213,7 +213,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
 	node_name = fit_get_name(fit, noffset, NULL);
 	region.data = data;
 	region.size = size;
-	ret = info.algo->sign(&info, &region, 1, &value, &value_len);
+	ret = info.algo->crypto->sign(&info, &region, 1, &value, &value_len);
 	if (ret) {
 		printf("Failed to sign '%s' signature node in '%s' image node: %d\n",
 		       node_name, image_name, ret);
@@ -239,7 +239,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
 	info.keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL);
 
 	if (keydest)
-		ret = info.algo->add_verify_data(&info, keydest);
+		ret = info.algo->crypto->add_verify_data(&info, keydest);
 	else
 		return -1;
 
@@ -588,7 +588,8 @@ static int fit_config_process_sig(const char *keydir, void *keydest,
 				require_keys ? "conf" : NULL))
 		return -1;
 
-	ret = info.algo->sign(&info, region, region_count, &value, &value_len);
+	ret = info.algo->crypto->sign(&info, region, region_count, &value,
+				      &value_len);
 	free(region);
 	if (ret) {
 		printf("Failed to sign '%s' signature node in '%s' conf node\n",
@@ -617,7 +618,7 @@ static int fit_config_process_sig(const char *keydir, void *keydest,
 
 	/* Write the public key into the supplied FDT file */
 	if (keydest) {
-		ret = info.algo->add_verify_data(&info, keydest);
+		ret = info.algo->crypto->add_verify_data(&info, keydest);
 		if (ret == -ENOSPC)
 			return -ENOSPC;
 		if (ret) {
-- 
2.10.2

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

* [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info
  2016-11-08 18:53 ` [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info aduda
@ 2016-11-11 16:17   ` Simon Glass
  2016-11-22  2:56   ` [U-Boot] [U-Boot, " Tom Rini
  1 sibling, 0 replies; 11+ messages in thread
From: Simon Glass @ 2016-11-11 16:17 UTC (permalink / raw)
  To: u-boot

On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote:
> From: Andrew Duda <aduda@meraki.com>
>
> Cut down on the repetition of algorithm information by defining separate
> checksum and crypto structs. image_sig_algos are now simply pairs of
> unique checksum and crypto algos.
>
> Signed-off-by: Andrew Duda <aduda@meraki.com>
> Signed-off-by: aduda <aduda@meraki.com>
> ---
>
>  common/image-sig.c   | 46 +++++++++++++++++++++++-----------------------
>  include/image.h      |  9 +++++++--
>  lib/rsa/rsa-verify.c | 19 +++++++++++--------
>  tools/image-host.c   |  9 +++++----
>  4 files changed, 46 insertions(+), 37 deletions(-)

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

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

* [U-Boot] [U-Boot, 3/4] image: Add crypto_algo struct for RSA info
  2016-11-08 18:53 ` [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info aduda
  2016-11-11 16:17   ` Simon Glass
@ 2016-11-22  2:56   ` Tom Rini
  1 sibling, 0 replies; 11+ messages in thread
From: Tom Rini @ 2016-11-22  2:56 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 08, 2016 at 06:53:41PM +0000, aduda wrote:

> From: Andrew Duda <aduda@meraki.com>
> 
> Cut down on the repetition of algorithm information by defining separate
> checksum and crypto structs. image_sig_algos are now simply pairs of
> unique checksum and crypto algos.
> 
> Signed-off-by: Andrew Duda <aduda@meraki.com>
> Signed-off-by: aduda <aduda@meraki.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161121/e6f10453/attachment.sig>

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

* [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info
       [not found] <20161108185329.56774-1-aduda@meraki.com>
  2016-11-08 18:53 ` [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically aduda
  2016-11-08 18:53 ` [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info aduda
@ 2016-11-08 18:53 ` aduda
  2016-11-11 16:17   ` Simon Glass
  2016-11-22  2:55   ` [U-Boot] [U-Boot, " Tom Rini
  2 siblings, 2 replies; 11+ messages in thread
From: aduda @ 2016-11-08 18:53 UTC (permalink / raw)
  To: u-boot

From: Andrew Duda <aduda@meraki.com>

Remove the need to explicitly add SHA/RSA pairings. Invalid SHA/RSA
pairings will still fail on verify operations when the hash length is
longer than the key length.

Follow the same naming scheme "checksum,crytpo" without explicitly
defining the string.

Indirectly adds support for "sha1,rsa4096" signing/verification.

Signed-off-by: Andrew Duda <aduda@meraki.com>
Signed-off-by: aduda <aduda@meraki.com>
---

 common/image-sig.c   | 57 ++++++++++++++++++++++++++++------------------------
 include/image.h      | 24 ++++++++++++----------
 lib/rsa/rsa-sign.c   |  4 ++--
 lib/rsa/rsa-verify.c | 14 ++++++-------
 tools/image-host.c   | 16 ++++++++-------
 5 files changed, 61 insertions(+), 54 deletions(-)

diff --git a/common/image-sig.c b/common/image-sig.c
index 8b4314d..455f2b9 100644
--- a/common/image-sig.c
+++ b/common/image-sig.c
@@ -72,32 +72,36 @@ struct crypto_algo crypto_algos[] = {
 
 };
 
-struct image_sig_algo image_sig_algos[] = {
-	{
-		"sha1,rsa2048",
-		&crypto_algos[0],
-		&checksum_algos[0],
-	},
-	{
-		"sha256,rsa2048",
-		&crypto_algos[0],
-		&checksum_algos[1],
-	},
-	{
-		"sha256,rsa4096",
-		&crypto_algos[1],
-		&checksum_algos[1],
+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 image_sig_algo *image_get_sig_algo(const char *name)
+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(image_sig_algos); i++) {
-		if (!strcmp(image_sig_algos[i].name, name))
-			return &image_sig_algos[i];
+	for (i = 0; i < ARRAY_SIZE(crypto_algos); i++) {
+		if (!strcmp(crypto_algos[i].name, name))
+			return &crypto_algos[i];
 	}
 
 	return NULL;
@@ -161,12 +165,14 @@ static int fit_image_setup_verify(struct image_sign_info *info,
 	info->keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL);
 	info->fit = (void *)fit;
 	info->node_offset = noffset;
-	info->algo = image_get_sig_algo(algo_name);
+	info->name = algo_name;
+	info->checksum = image_get_checksum_algo(algo_name);
+	info->crypto = image_get_crypto_algo(algo_name);
 	info->fdt_blob = gd_fdt_blob();
 	info->required_keynode = required_keynode;
 	printf("%s:%s", algo_name, info->keyname);
 
-	if (!info->algo) {
+	if (!info->checksum || !info->crypto) {
 		*err_msgp = "Unknown signature algorithm";
 		return -1;
 	}
@@ -196,8 +202,7 @@ int fit_image_check_sig(const void *fit, int noffset, const void *data,
 	region.data = data;
 	region.size = size;
 
-	if (info.algo->crypto->verify(&info, &region, 1, fit_value,
-				      fit_value_len)) {
+	if (info.crypto->verify(&info, &region, 1, fit_value, fit_value_len)) {
 		*err_msgp = "Verification failed";
 		return -1;
 	}
@@ -378,8 +383,8 @@ int fit_config_check_sig(const void *fit, int noffset, int required_keynode,
 	struct image_region region[count];
 
 	fit_region_make_list(fit, fdt_regions, count, region);
-	if (info.algo->crypto->verify(&info, region, count, fit_value,
-				      fit_value_len)) {
+	if (info.crypto->verify(&info, region, count, fit_value,
+				fit_value_len)) {
 		*err_msgp = "Verification failed";
 		return -1;
 	}
diff --git a/include/image.h b/include/image.h
index c3c9866..8131595 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1049,7 +1049,9 @@ struct image_sign_info {
 	const char *keyname;		/* Name of key to use */
 	void *fit;			/* Pointer to FIT blob */
 	int node_offset;		/* Offset of signature node */
-	struct image_sig_algo *algo;	/* Algorithm information */
+	const char *name;		/* Algorithm name */
+	struct checksum_algo *checksum;	/* Checksum algorithm information */
+	struct crypto_algo *crypto;	/* Crypto algorithm information */
 	const void *fdt_blob;		/* FDT containing public keys */
 	int required_keynode;		/* Node offset of key to use: -1=any */
 	const char *require_keys;	/* Value for 'required' property */
@@ -1133,21 +1135,21 @@ struct crypto_algo {
 		      uint8_t *sig, uint sig_len);
 };
 
-struct image_sig_algo {
-	const char *name;
-	/* pointer to cryptosystem algorithm */
-	struct crypto_algo *crypto;
-	/* pointer to checksum algorithm */
-	struct checksum_algo *checksum;
-};
+/**
+ * image_get_checksum_algo() - Look up a checksum algorithm
+ *
+ * @param full_name	Name of algorithm in the form "checksum,crypto"
+ * @return pointer to algorithm information, or NULL if not found
+ */
+struct checksum_algo *image_get_checksum_algo(const char *full_name);
 
 /**
- * image_get_sig_algo() - Look up a signature algortihm
+ * image_get_crypto_algo() - Look up a cryptosystem algorithm
  *
- * @param name		Name of algorithm
+ * @param full_name	Name of algorithm in the form "checksum,crypto"
  * @return pointer to algorithm information, or NULL if not found
  */
-struct image_sig_algo *image_get_sig_algo(const char *name);
+struct crypto_algo *image_get_crypto_algo(const char *full_name);
 
 /**
  * fit_image_verify_required_sigs() - Verify signatures marked as 'required'
diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index c26f741..9a09280 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -244,7 +244,7 @@ int rsa_sign(struct image_sign_info *info,
 	ret = rsa_get_priv_key(info->keydir, info->keyname, &rsa);
 	if (ret)
 		goto err_priv;
-	ret = rsa_sign_with_key(rsa, info->algo->checksum, region,
+	ret = rsa_sign_with_key(rsa, info->checksum, region,
 				region_count, sigp, sig_len);
 	if (ret)
 		goto err_sign;
@@ -508,7 +508,7 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest)
 	}
 	if (!ret) {
 		ret = fdt_setprop_string(keydest, node, FIT_ALGO_PROP,
-					 info->algo->name);
+					 info->name);
 	}
 	if (!ret && info->require_keys) {
 		ret = fdt_setprop_string(keydest, node, "required",
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 61dc4c2..0d548f8 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -184,8 +184,7 @@ static int rsa_verify_with_keynode(struct image_sign_info *info,
 	}
 
 	ret = rsa_verify_key(&prop, sig, sig_len, hash,
-			     info->algo->crypto->key_len,
-			     info->algo->checksum);
+			     info->crypto->key_len, info->checksum);
 
 	return ret;
 }
@@ -196,7 +195,7 @@ int rsa_verify(struct image_sign_info *info,
 {
 	const void *blob = info->fdt_blob;
 	/* Reserve memory for maximum checksum-length */
-	uint8_t hash[info->algo->crypto->key_len];
+	uint8_t hash[info->crypto->key_len];
 	int ndepth, noffset;
 	int sig_node, node;
 	char name[100];
@@ -206,11 +205,10 @@ int rsa_verify(struct image_sign_info *info,
 	 * Verify that the checksum-length does not exceed the
 	 * rsa-signature-length
 	 */
-	if (info->algo->checksum->checksum_len >
-	    info->algo->crypto->key_len) {
+	if (info->checksum->checksum_len >
+	    info->crypto->key_len) {
 		debug("%s: invlaid checksum-algorithm %s for %s\n",
-		      __func__, info->algo->checksum->name,
-		      info->algo->crypto->name);
+		      __func__, info->checksum->name, info->crypto->name);
 		return -EINVAL;
 	}
 
@@ -221,7 +219,7 @@ int rsa_verify(struct image_sign_info *info,
 	}
 
 	/* Calculate checksum with checksum-algorithm */
-	ret = info->algo->checksum->calculate(info->algo->checksum->name,
+	ret = info->checksum->calculate(info->checksum->name,
 					region, region_count, hash);
 	if (ret < 0) {
 		debug("%s: Error in checksum calculation\n", __func__);
diff --git a/tools/image-host.c b/tools/image-host.c
index dac85b4..c1a0122 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -166,9 +166,11 @@ static int fit_image_setup_sig(struct image_sign_info *info,
 	info->keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL);
 	info->fit = fit;
 	info->node_offset = noffset;
-	info->algo = image_get_sig_algo(algo_name);
+	info->name = algo_name;
+	info->checksum = image_get_checksum_algo(algo_name);
+	info->crypto = image_get_crypto_algo(algo_name);
 	info->require_keys = require_keys;
-	if (!info->algo) {
+	if (!info->checksum || !info->crypto) {
 		printf("Unsupported signature algorithm (%s) for '%s' signature node in '%s' image node\n",
 		       algo_name, node_name, image_name);
 		return -1;
@@ -213,7 +215,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
 	node_name = fit_get_name(fit, noffset, NULL);
 	region.data = data;
 	region.size = size;
-	ret = info.algo->crypto->sign(&info, &region, 1, &value, &value_len);
+	ret = info.crypto->sign(&info, &region, 1, &value, &value_len);
 	if (ret) {
 		printf("Failed to sign '%s' signature node in '%s' image node: %d\n",
 		       node_name, image_name, ret);
@@ -239,7 +241,7 @@ static int fit_image_process_sig(const char *keydir, void *keydest,
 	info.keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL);
 
 	if (keydest)
-		ret = info.algo->crypto->add_verify_data(&info, keydest);
+		ret = info.crypto->add_verify_data(&info, keydest);
 	else
 		return -1;
 
@@ -588,8 +590,8 @@ static int fit_config_process_sig(const char *keydir, void *keydest,
 				require_keys ? "conf" : NULL))
 		return -1;
 
-	ret = info.algo->crypto->sign(&info, region, region_count, &value,
-				      &value_len);
+	ret = info.crypto->sign(&info, region, region_count, &value,
+				&value_len);
 	free(region);
 	if (ret) {
 		printf("Failed to sign '%s' signature node in '%s' conf node\n",
@@ -618,7 +620,7 @@ static int fit_config_process_sig(const char *keydir, void *keydest,
 
 	/* Write the public key into the supplied FDT file */
 	if (keydest) {
-		ret = info.algo->crypto->add_verify_data(&info, keydest);
+		ret = info.crypto->add_verify_data(&info, keydest);
 		if (ret == -ENOSPC)
 			return -ENOSPC;
 		if (ret) {
-- 
2.10.2

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

* [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info
  2016-11-08 18:53 ` [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info aduda
@ 2016-11-11 16:17   ` Simon Glass
  2016-11-22  2:55   ` [U-Boot] [U-Boot, " Tom Rini
  1 sibling, 0 replies; 11+ messages in thread
From: Simon Glass @ 2016-11-11 16:17 UTC (permalink / raw)
  To: u-boot

On 8 November 2016 at 11:53, aduda <aduda@meraki.com> wrote:
> From: Andrew Duda <aduda@meraki.com>
>
> Remove the need to explicitly add SHA/RSA pairings. Invalid SHA/RSA
> pairings will still fail on verify operations when the hash length is
> longer than the key length.
>
> Follow the same naming scheme "checksum,crytpo" without explicitly
> defining the string.
>
> Indirectly adds support for "sha1,rsa4096" signing/verification.
>
> Signed-off-by: Andrew Duda <aduda@meraki.com>
> Signed-off-by: aduda <aduda@meraki.com>
> ---
>
>  common/image-sig.c   | 57 ++++++++++++++++++++++++++++------------------------
>  include/image.h      | 24 ++++++++++++----------
>  lib/rsa/rsa-sign.c   |  4 ++--
>  lib/rsa/rsa-verify.c | 14 ++++++-------
>  tools/image-host.c   | 16 ++++++++-------
>  5 files changed, 61 insertions(+), 54 deletions(-)

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

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

* [U-Boot] [U-Boot, 4/4] image: Combine image_sig_algo with image_sign_info
  2016-11-08 18:53 ` [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info aduda
  2016-11-11 16:17   ` Simon Glass
@ 2016-11-22  2:55   ` Tom Rini
  1 sibling, 0 replies; 11+ messages in thread
From: Tom Rini @ 2016-11-22  2:55 UTC (permalink / raw)
  To: u-boot

On Tue, Nov 08, 2016 at 06:53:41PM +0000, aduda wrote:

> From: Andrew Duda <aduda@meraki.com>
> 
> Remove the need to explicitly add SHA/RSA pairings. Invalid SHA/RSA
> pairings will still fail on verify operations when the hash length is
> longer than the key length.
> 
> Follow the same naming scheme "checksum,crytpo" without explicitly
> defining the string.
> 
> Indirectly adds support for "sha1,rsa4096" signing/verification.
> 
> Signed-off-by: Andrew Duda <aduda@meraki.com>
> Signed-off-by: aduda <aduda@meraki.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161121/ba501563/attachment.sig>

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

end of thread, other threads:[~2016-11-22  2:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20161108185329.56774-1-aduda@meraki.com>
2016-11-08 18:53 ` [U-Boot] [PATCH 2/4] rsa: Verify RSA padding programatically aduda
2016-11-11 16:17   ` Simon Glass
2016-11-11 21:22     ` Andrew Duda
2016-11-14 19:04       ` Simon Glass
2016-11-22  2:55   ` [U-Boot] [U-Boot,2/4] " Tom Rini
2016-11-08 18:53 ` [U-Boot] [PATCH 3/4] image: Add crypto_algo struct for RSA info aduda
2016-11-11 16:17   ` Simon Glass
2016-11-22  2:56   ` [U-Boot] [U-Boot, " Tom Rini
2016-11-08 18:53 ` [U-Boot] [PATCH 4/4] image: Combine image_sig_algo with image_sign_info aduda
2016-11-11 16:17   ` Simon Glass
2016-11-22  2:55   ` [U-Boot] [U-Boot, " Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox