Fix some undefined behaviour in the Curve448 code

[openssl.git] / crypto / ec / curve448 / eddsa.c

diff --git a/crypto/ec/curve448/eddsa.c b/crypto/ec/curve448/eddsa.c
index f2d9839c639e3822f9239930ba118f9b2fe95cd7..d31887638c37d777be4f3d981bdaf926c79e312e 100644 (file)
--- a/crypto/ec/curve448/eddsa.c
+++ b/crypto/ec/curve448/eddsa.c
@@ -9,13 +9,12 @@
  *
  * Originally written by Mike Hamburg
  */
+#include <string.h>
 #include <openssl/crypto.h>
 #include <openssl/evp.h>
-
 #include "curve448_lcl.h"
 #include "word.h"
 #include "ed448.h"
-#include <string.h>
 #include "internal/numbers.h"
 
 #define COFACTOR 4
@@ -29,8 +28,8 @@ static c448_error_t oneshot_hash(uint8_t *out, size_t outlen,
         return C448_FAILURE;
 
     if (!EVP_DigestInit_ex(hashctx, EVP_shake256(), NULL)
-        || !EVP_DigestUpdate(hashctx, in, inlen)
-        || !EVP_DigestFinalXOF(hashctx, out, outlen)) {
+            || !EVP_DigestUpdate(hashctx, in, inlen)
+            || !EVP_DigestFinalXOF(hashctx, out, outlen)) {
         EVP_MD_CTX_free(hashctx);
         return C448_FAILURE;
     }
@@ -41,17 +40,9 @@ static c448_error_t oneshot_hash(uint8_t *out, size_t outlen,
 
 static void clamp(uint8_t secret_scalar_ser[EDDSA_448_PRIVATE_BYTES])
 {
-    uint8_t hibit = (1 << 0) >> 1;
-
-    /* Blarg */
     secret_scalar_ser[0] &= -COFACTOR;
-    if (hibit == 0) {
-        secret_scalar_ser[EDDSA_448_PRIVATE_BYTES - 1] = 0;
-        secret_scalar_ser[EDDSA_448_PRIVATE_BYTES - 2] |= 0x80;
-    } else {
-        secret_scalar_ser[EDDSA_448_PRIVATE_BYTES - 1] &= hibit - 1;
-        secret_scalar_ser[EDDSA_448_PRIVATE_BYTES - 1] |= hibit;
-    }
+    secret_scalar_ser[EDDSA_448_PRIVATE_BYTES - 1] = 0;
+    secret_scalar_ser[EDDSA_448_PRIVATE_BYTES - 2] |= 0x80;
 }
 
 static c448_error_t hash_init_with_dom(EVP_MD_CTX *hashctx, uint8_t prehashed,
@@ -62,16 +53,18 @@ static c448_error_t hash_init_with_dom(EVP_MD_CTX *hashctx, uint8_t prehashed,
     const char *dom_s = "SigEd448";
     uint8_t dom[2];
 
-    dom[0] = 2 + word_is_zero(prehashed) + word_is_zero(for_prehash);
-    dom[1] = (uint8_t)context_len;
-
     if (context_len > UINT8_MAX)
         return C448_FAILURE;
 
+    dom[0] = (uint8_t)(2 - (prehashed == 0 ? 1 : 0)
+                       - (for_prehash == 0 ? 1 : 0));
+    dom[1] = (uint8_t)context_len;
+
     if (!EVP_DigestInit_ex(hashctx, EVP_shake256(), NULL)
-        || !EVP_DigestUpdate(hashctx, dom_s, strlen(dom_s))
-        || !EVP_DigestUpdate(hashctx, dom, sizeof(dom))
-        || !EVP_DigestUpdate(hashctx, context, context_len))
+            || !EVP_DigestUpdate(hashctx, dom_s, strlen(dom_s))
+            || !EVP_DigestUpdate(hashctx, dom, sizeof(dom))
+            || (context_len > 0
+                && !EVP_DigestUpdate(hashctx, context, context_len)))
         return C448_FAILURE;
 
     return C448_SUCCESS;
@@ -165,9 +158,11 @@ c448_error_t c448_ed448_sign(
 
         /* Hash to create the nonce */
         if (!hash_init_with_dom(hashctx, prehashed, 0, context, context_len)
-            || !EVP_DigestUpdate(hashctx, expanded + EDDSA_448_PRIVATE_BYTES,
-                                 EDDSA_448_PRIVATE_BYTES)
-            || !EVP_DigestUpdate(hashctx, message, message_len)) {
+                || !EVP_DigestUpdate(hashctx,
+                                     expanded + EDDSA_448_PRIVATE_BYTES,
+                                     EDDSA_448_PRIVATE_BYTES)
+                || (message_len > 0
+                    && !EVP_DigestUpdate(hashctx, message, message_len))) {
             OPENSSL_cleanse(expanded, sizeof(expanded));
             goto err;
         }
@@ -190,9 +185,8 @@ c448_error_t c448_ed448_sign(
         curve448_point_t p;
 
         curve448_scalar_halve(nonce_scalar_2, nonce_scalar);
-        for (c = 2; c < C448_EDDSA_ENCODE_RATIO; c <<= 1) {
+        for (c = 2; c < C448_EDDSA_ENCODE_RATIO; c <<= 1)
             curve448_scalar_halve(nonce_scalar_2, nonce_scalar_2);
-        }
 
         curve448_precomputed_scalarmul(p, curve448_precomputed_base,
                                        nonce_scalar_2);
@@ -206,10 +200,11 @@ c448_error_t c448_ed448_sign(
 
         /* Compute the challenge */
         if (!hash_init_with_dom(hashctx, prehashed, 0, context, context_len)
-            || !EVP_DigestUpdate(hashctx, nonce_point, sizeof(nonce_point))
-            || !EVP_DigestUpdate(hashctx, pubkey, EDDSA_448_PUBLIC_BYTES)
-            || !EVP_DigestUpdate(hashctx, message, message_len)
-            || !EVP_DigestFinalXOF(hashctx, challenge, sizeof(challenge)))
+                || !EVP_DigestUpdate(hashctx, nonce_point, sizeof(nonce_point))
+                || !EVP_DigestUpdate(hashctx, pubkey, EDDSA_448_PUBLIC_BYTES)
+                || (message_len > 0
+                    && !EVP_DigestUpdate(hashctx, message, message_len))
+                || !EVP_DigestFinalXOF(hashctx, challenge, sizeof(challenge)))
             goto err;
 
         curve448_scalar_decode_long(challenge_scalar, challenge,
@@ -274,12 +269,12 @@ c448_error_t c448_ed448_verify(
         uint8_t challenge[2 * EDDSA_448_PRIVATE_BYTES];
 
         if (hashctx == NULL
-            || !hash_init_with_dom(hashctx, prehashed, 0, context, context_len)
-            || !EVP_DigestUpdate(hashctx, signature,
-                                 EDDSA_448_PUBLIC_BYTES)
-            || !EVP_DigestUpdate(hashctx, pubkey, EDDSA_448_PUBLIC_BYTES)
-            || !EVP_DigestUpdate(hashctx, message, message_len)
-            || !EVP_DigestFinalXOF(hashctx, challenge, sizeof(challenge))) {
+                || !hash_init_with_dom(hashctx, prehashed, 0, context,
+                                       context_len)
+                || !EVP_DigestUpdate(hashctx, signature, EDDSA_448_PUBLIC_BYTES)
+                || !EVP_DigestUpdate(hashctx, pubkey, EDDSA_448_PUBLIC_BYTES)
+                || !EVP_DigestUpdate(hashctx, message, message_len)
+                || !EVP_DigestFinalXOF(hashctx, challenge, sizeof(challenge))) {
             EVP_MD_CTX_free(hashctx);
             return C448_FAILURE;
         }
@@ -312,19 +307,14 @@ c448_error_t c448_ed448_verify_prehash(
                     const uint8_t hash[64], const uint8_t *context,
                     uint8_t context_len)
 {
-    c448_error_t ret;
-
-    ret = c448_ed448_verify(signature, pubkey, hash, 64, 1, context,
-                            context_len);
-
-    return ret;
+    return c448_ed448_verify(signature, pubkey, hash, 64, 1, context,
+                             context_len);
 }
 
 int ED448_sign(uint8_t *out_sig, const uint8_t *message, size_t message_len,
                const uint8_t public_key[57], const uint8_t private_key[57],
                const uint8_t *context, size_t context_len)
 {
-
     return c448_ed448_sign(out_sig, private_key, public_key, message,
                            message_len, 0, context, context_len)
         == C448_SUCCESS;