From 722961f55c68859c9f3a2a67e22105a2fa8e9964 Mon Sep 17 00:00:00 2001 From: kaleb-himes Date: Tue, 23 Jun 2020 17:32:00 -0600 Subject: [PATCH 1/2] ed25519 and ed448 check sigLen against expected --- tests/api.c | 41 +++++++++++++++++++++++++++++++++++++++++ wolfcrypt/src/ed25519.c | 2 +- wolfcrypt/src/ed448.c | 2 +- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/tests/api.c b/tests/api.c index d3a29182c..15b0faee0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -14669,6 +14669,8 @@ static int test_wc_ed25519_sign_msg (void) ed25519_key key; byte msg[] = "Everybody gets Friday off.\n"; byte sig[ED25519_SIG_SIZE]; + byte sigTooShort[ED25519_SIG_SIZE - 1]; + byte sigTooLong[ED25519_SIG_SIZE + 1]; word32 msglen = sizeof(msg); word32 siglen = sizeof(sig); word32 badSigLen = sizeof(sig) - 1; @@ -14676,6 +14678,8 @@ static int test_wc_ed25519_sign_msg (void) /* Initialize stack variables. */ XMEMSET(sig, 0, siglen); + XMEMSET(sigTooShort, 0, siglen-1); + XMEMSET(sigTooLong, 0, siglen+1); /* Initialize key. */ ret = wc_InitRng(&rng); @@ -14690,6 +14694,9 @@ static int test_wc_ed25519_sign_msg (void) if (ret == 0) { ret = wc_ed25519_sign_msg(msg, msglen, sig, &siglen, &key); + XMEMCPY(sigTooShort, sig, siglen-1); + XMEMCPY(sigTooLong, sig, siglen); + sigTooLong[ED25519_SIG_SIZE] = 0x01; /* add byte to end of sig */ } /* Test bad args. */ if (ret == 0 && siglen == ED25519_SIG_SIZE) { @@ -14729,6 +14736,18 @@ static int test_wc_ed25519_sign_msg (void) /* Test bad args. */ if (ret == 0) { + AssertIntEQ(wc_ed25519_verify_msg(sigTooShort, siglen - 1, msg, + msglen, &verify_ok, &key), + BAD_FUNC_ARG); + /* This should verify even though sig is modified, only siglen + * bytes are checked */ + AssertIntEQ(wc_ed25519_verify_msg(sigTooLong, siglen, msg, + msglen, &verify_ok, &key), + 0); + AssertIntEQ(wc_ed25519_verify_msg(sigTooLong, siglen + 1, msg, + msglen, &verify_ok, &key), + BAD_FUNC_ARG); + ret = wc_ed25519_verify_msg(NULL, siglen, msg, msglen, &verify_ok, &key); if (ret == BAD_FUNC_ARG) { @@ -15416,6 +15435,8 @@ static int test_wc_ed448_sign_msg (void) ed448_key key; byte msg[] = "Everybody gets Friday off.\n"; byte sig[ED448_SIG_SIZE]; + byte sigTooShort[ED448_SIG_SIZE - 1]; + byte sigTooLong[ED448_SIG_SIZE + 1]; word32 msglen = sizeof(msg); word32 siglen = sizeof(sig); word32 badSigLen = sizeof(sig) - 1; @@ -15423,6 +15444,8 @@ static int test_wc_ed448_sign_msg (void) /* Initialize stack variables. */ XMEMSET(sig, 0, siglen); + XMEMSET(sigTooShort, 0, siglen - 1); + XMEMSET(sigTooLong, 0, siglen + 1); /* Initialize key. */ ret = wc_InitRng(&rng); @@ -15437,6 +15460,9 @@ static int test_wc_ed448_sign_msg (void) if (ret == 0) { ret = wc_ed448_sign_msg(msg, msglen, sig, &siglen, &key, NULL, 0); + XMEMCPY(sigTooShort, sig, siglen - 1); + XMEMCPY(sigTooLong, sig, siglen); + sigTooLong[ED448_SIG_SIZE] = 0x01; /* add byte to end of sig */ } /* Test bad args. */ if (ret == 0 && siglen == ED448_SIG_SIZE) { @@ -15478,6 +15504,21 @@ static int test_wc_ed448_sign_msg (void) /* Test bad args. */ if (ret == 0) { + AssertIntEQ(wc_ed448_verify_msg(sigTooShort, siglen - 1, msg, + msglen, &verify_ok, &key, + NULL, 0), + BAD_FUNC_ARG); + /* This should verify even though sig is modified, only siglen + * bytes are checked */ + AssertIntEQ(wc_ed448_verify_msg(sigTooLong, siglen, msg, + msglen, &verify_ok, &key, + NULL, 0), + 0); + AssertIntEQ(wc_ed448_verify_msg(sigTooLong, siglen + 1, msg, + msglen, &verify_ok, &key, + NULL, 0), + BAD_FUNC_ARG); + ret = wc_ed448_verify_msg(NULL, siglen, msg, msglen, &verify_ok, &key, NULL, 0); if (ret == BAD_FUNC_ARG) { diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index b78732d5e..64aee3389 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -365,7 +365,7 @@ static int ed25519_verify_msg(const byte* sig, word32 sigLen, const byte* msg, *res = 0; /* check on basics needed to verify signature */ - if (sigLen < ED25519_SIG_SIZE || (sig[ED25519_SIG_SIZE-1] & 224)) + if (sigLen != ED25519_SIG_SIZE || (sig[ED25519_SIG_SIZE-1] & 224)) return BAD_FUNC_ARG; /* uncompress A (public key), test if valid, and negate it */ diff --git a/wolfcrypt/src/ed448.c b/wolfcrypt/src/ed448.c index edeec0707..a25fc5bcc 100644 --- a/wolfcrypt/src/ed448.c +++ b/wolfcrypt/src/ed448.c @@ -379,7 +379,7 @@ static int ed448_verify_msg(const byte* sig, word32 sigLen, const byte* msg, *res = 0; /* check on basics needed to verify signature */ - if (sigLen < ED448_SIG_SIZE) { + if (sigLen != ED448_SIG_SIZE) { ret = BAD_FUNC_ARG; } } From 17466727b256716044b4250dd04e72feb315c6b4 Mon Sep 17 00:00:00 2001 From: kaleb-himes Date: Thu, 25 Jun 2020 09:43:22 -0600 Subject: [PATCH 2/2] Implement peer review feedback --- tests/api.c | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/tests/api.c b/tests/api.c index 15b0faee0..5878bfafd 100644 --- a/tests/api.c +++ b/tests/api.c @@ -14669,8 +14669,6 @@ static int test_wc_ed25519_sign_msg (void) ed25519_key key; byte msg[] = "Everybody gets Friday off.\n"; byte sig[ED25519_SIG_SIZE]; - byte sigTooShort[ED25519_SIG_SIZE - 1]; - byte sigTooLong[ED25519_SIG_SIZE + 1]; word32 msglen = sizeof(msg); word32 siglen = sizeof(sig); word32 badSigLen = sizeof(sig) - 1; @@ -14678,8 +14676,6 @@ static int test_wc_ed25519_sign_msg (void) /* Initialize stack variables. */ XMEMSET(sig, 0, siglen); - XMEMSET(sigTooShort, 0, siglen-1); - XMEMSET(sigTooLong, 0, siglen+1); /* Initialize key. */ ret = wc_InitRng(&rng); @@ -14694,9 +14690,6 @@ static int test_wc_ed25519_sign_msg (void) if (ret == 0) { ret = wc_ed25519_sign_msg(msg, msglen, sig, &siglen, &key); - XMEMCPY(sigTooShort, sig, siglen-1); - XMEMCPY(sigTooLong, sig, siglen); - sigTooLong[ED25519_SIG_SIZE] = 0x01; /* add byte to end of sig */ } /* Test bad args. */ if (ret == 0 && siglen == ED25519_SIG_SIZE) { @@ -14736,15 +14729,10 @@ static int test_wc_ed25519_sign_msg (void) /* Test bad args. */ if (ret == 0) { - AssertIntEQ(wc_ed25519_verify_msg(sigTooShort, siglen - 1, msg, + AssertIntEQ(wc_ed25519_verify_msg(sig, siglen - 1, msg, msglen, &verify_ok, &key), BAD_FUNC_ARG); - /* This should verify even though sig is modified, only siglen - * bytes are checked */ - AssertIntEQ(wc_ed25519_verify_msg(sigTooLong, siglen, msg, - msglen, &verify_ok, &key), - 0); - AssertIntEQ(wc_ed25519_verify_msg(sigTooLong, siglen + 1, msg, + AssertIntEQ(wc_ed25519_verify_msg(sig, siglen + 1, msg, msglen, &verify_ok, &key), BAD_FUNC_ARG); @@ -15435,8 +15423,6 @@ static int test_wc_ed448_sign_msg (void) ed448_key key; byte msg[] = "Everybody gets Friday off.\n"; byte sig[ED448_SIG_SIZE]; - byte sigTooShort[ED448_SIG_SIZE - 1]; - byte sigTooLong[ED448_SIG_SIZE + 1]; word32 msglen = sizeof(msg); word32 siglen = sizeof(sig); word32 badSigLen = sizeof(sig) - 1; @@ -15444,8 +15430,6 @@ static int test_wc_ed448_sign_msg (void) /* Initialize stack variables. */ XMEMSET(sig, 0, siglen); - XMEMSET(sigTooShort, 0, siglen - 1); - XMEMSET(sigTooLong, 0, siglen + 1); /* Initialize key. */ ret = wc_InitRng(&rng); @@ -15460,9 +15444,6 @@ static int test_wc_ed448_sign_msg (void) if (ret == 0) { ret = wc_ed448_sign_msg(msg, msglen, sig, &siglen, &key, NULL, 0); - XMEMCPY(sigTooShort, sig, siglen - 1); - XMEMCPY(sigTooLong, sig, siglen); - sigTooLong[ED448_SIG_SIZE] = 0x01; /* add byte to end of sig */ } /* Test bad args. */ if (ret == 0 && siglen == ED448_SIG_SIZE) { @@ -15504,17 +15485,11 @@ static int test_wc_ed448_sign_msg (void) /* Test bad args. */ if (ret == 0) { - AssertIntEQ(wc_ed448_verify_msg(sigTooShort, siglen - 1, msg, + AssertIntEQ(wc_ed448_verify_msg(sig, siglen - 1, msg, msglen, &verify_ok, &key, NULL, 0), BAD_FUNC_ARG); - /* This should verify even though sig is modified, only siglen - * bytes are checked */ - AssertIntEQ(wc_ed448_verify_msg(sigTooLong, siglen, msg, - msglen, &verify_ok, &key, - NULL, 0), - 0); - AssertIntEQ(wc_ed448_verify_msg(sigTooLong, siglen + 1, msg, + AssertIntEQ(wc_ed448_verify_msg(sig, siglen + 1, msg, msglen, &verify_ok, &key, NULL, 0), BAD_FUNC_ARG);