Merge pull request #7513 from julek-wolfssl/gh/7510

ed25519: check that the signature is smaller than the order
wolfSSL · May 13, 2024 · a916429 · a916429
2 parents 568fda0 + ab03324
commit a916429
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 33 deletions.
diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c
@@ -698,15 +698,14 @@ static int ed25519_verify_msg_update_with_sha(const byte* msgSegment,
  return ed25519_hash_update(key, sha, msgSegment, msgSegmentLen);
 }
 
-/* Low part of order in big endian. */
-static const byte ed25519_low_order[] = {
- 0x14, 0xde, 0xf9, 0xde, 0xa2, 0xf7, 0x9c, 0xd6,
- 0x58, 0x12, 0x63, 0x1a, 0x5c, 0xf5, 0xd3, 0xed
+/* ed25519 order in little endian. */
+static const byte ed25519_order[] = {
+ 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+ 0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10
 };
 
-#define ED25519_SIG_LOW_ORDER_IDX \
- ((int)(ED25519_SIG_SIZE/2 + sizeof(ed25519_low_order) - 1))
-
 /*
  sig is array of bytes containing the signature
  sigLen is the length of sig byte array
@@ -725,6 +724,7 @@ static int ed25519_verify_msg_final_with_sha(const byte* sig, word32 sigLen,
  ge_p2 R;
 #endif
  int ret;
+ int i;
 
  /* sanity check on arguments */
  if (sig == NULL || res == NULL || key == NULL)
@@ -740,33 +740,19 @@ static int ed25519_verify_msg_final_with_sha(const byte* sig, word32 sigLen,
  * 2^252 + 0x14def9dea2f79cd65812631a5cf5d3ed
  * = 0x1000000000000000000000000000000014def9dea2f79cd65812631a5cf5d3ed
  */
- if (sig[ED25519_SIG_SIZE-1] > 0x10)
- return BAD_FUNC_ARG;
- if (sig[ED25519_SIG_SIZE-1] == 0x10) {
- int i = ED25519_SIG_SIZE-1;
- int j;
-
- /* Check high zeros. */
- for (--i; i > ED25519_SIG_LOW_ORDER_IDX; i--) {
- if (sig[i] > 0x00)
- break;
- }
- /* Did we see all zeros up to lower order index? */
- if (i == ED25519_SIG_LOW_ORDER_IDX) {
- /* Check lower part. */
- for (j = 0; j < (int)sizeof(ed25519_low_order); j++, i--) {
- /* Check smaller. */
- if (sig[i] < ed25519_low_order[j])
- break;
- /* Check bigger. */
- if (sig[i] > ed25519_low_order[j])
- return BAD_FUNC_ARG;
- }
- /* Check equal - all bytes match. */
- if (i == ED25519_SIG_SIZE/2 - 1)
- return BAD_FUNC_ARG;
- }
+
+ /* Check S is not larger than or equal to order. */
+ for (i = (int)sizeof(ed25519_order) - 1; i >= 0; i--) {
+ /* Bigger than order. */
+ if (sig[ED25519_SIG_SIZE/2 + i] > ed25519_order[i])
+ return BAD_FUNC_ARG;
+ /* Less than order. */
+ if (sig[ED25519_SIG_SIZE/2 + i] < ed25519_order[i])
+ break;
  }
+ /* Check equal - all bytes match. */
+ if (i == -1)
+ return BAD_FUNC_ARG;
 
  /* uncompress A (public key), test if valid, and negate it */
 #ifndef FREESCALE_LTC_ECC

diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c
@@ -34016,6 +34016,79 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t ed25519_test(void)
 #endif /* HAVE_ED25519_VERIFY */
  }
 
+ {
+ /* Run tests for some rare code paths */
+ /* sig is exactly equal to the order */
+ const byte rareEd1[] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+ 0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10
+ };
+ /* sig is larger than the order before we get to the low part */
+ const byte rareEd2[] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+ 0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x10
+ };
+ /* sig is larger than the order in the low part */
+ const byte rareEd3[] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+ 0xd6, 0x9c, 0xf9, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10
+ };
+ /* sig is smaller than the order */
+ const byte rareEd4[] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+ 0xd6, 0x9c, 0xf1, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10
+ };
+
+ ret = wc_ed25519_import_private_key(sKeys[0], ED25519_KEY_SIZE,
+ pKeys[0], pKeySz[0], &key);
+ if (ret != 0)
+ return ret;
+
+ ret = wc_ed25519_verify_msg(rareEd1, sizeof(rareEd1), msgs[0], msgSz[0],
+ &verify, &key);
+ if (ret != BAD_FUNC_ARG)
+ return ret;
+
+ ret = wc_ed25519_verify_msg(rareEd2, sizeof(rareEd2), msgs[0], msgSz[0],
+ &verify, &key);
+ if (ret != BAD_FUNC_ARG)
+ return ret;
+
+ ret = wc_ed25519_verify_msg(rareEd3, sizeof(rareEd3), msgs[0], msgSz[0],
+ &verify, &key);
+ if (ret != BAD_FUNC_ARG)
+ return ret;
+
+ ret = wc_ed25519_verify_msg(rareEd4, sizeof(rareEd4), msgs[0], msgSz[0],
+ &verify, &key);
+ if (ret != SIG_VERIFY_E)
+ return ret;
+ }
+
  ret = ed25519ctx_test();
  if (ret != 0)
  return ret;