More style fixes to Curve448 code based on review feedback

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5105)

diff --git a/crypto/ec/curve448/arch_32/arch_intrinsics.h b/crypto/ec/curve448/arch_32/arch_intrinsics.h
index 1f5d2d7751d07af63f68e5a136eb162a2181bcc1..48081c77170b089e3672e428611e6a0dfa0fddf3 100644 (file)
--- a/crypto/ec/curve448/arch_32/arch_intrinsics.h
+++ b/crypto/ec/curve448/arch_32/arch_intrinsics.h
@@ -10,10 +10,10 @@
  * Originally written by Mike Hamburg
  */
 
-#include "internal/constant_time_locl.h"
+#ifndef HEADER_ARCH_32_ARCH_INTRINSICS_H
+# define HEADER_ARCH_32_ARCH_INTRINSICS_H
 
-#ifndef __ARCH_ARCH_32_ARCH_INTRINSICS_H__
-# define __ARCH_ARCH_32_ARCH_INTRINSICS_H__
+#include "internal/constant_time_locl.h"
 
 # define ARCH_WORD_BITS 32
 
@@ -24,4 +24,4 @@ static ossl_inline uint64_t widemul(uint32_t a, uint32_t b)
     return ((uint64_t)a) * b;
 }
 
-#endif                          /* __ARCH_ARCH_32_ARCH_INTRINSICS_H__ */
+#endif                          /* HEADER_ARCH_32_ARCH_INTRINSICS_H */

diff --git a/crypto/ec/curve448/arch_32/f_impl.h b/crypto/ec/curve448/arch_32/f_impl.h
index 7c87e972e2b584ca5147c63bb12fab62f952704d..217f9947e9b5f797f341a469e6e266b75b7b807e 100644 (file)
--- a/crypto/ec/curve448/arch_32/f_impl.h
+++ b/crypto/ec/curve448/arch_32/f_impl.h
@@ -9,12 +9,16 @@
  *
  * Originally written by Mike Hamburg
  */
-#define GF_HEADROOM 2
-#define LIMB(x) (x)&((1<<28)-1), (x)>>28
-#define FIELD_LITERAL(a,b,c,d,e,f,g,h) \
+
+#ifndef HEADER_ARCH_32_F_IMPL_H
+# define HEADER_ARCH_32_F_IMPL_H
+
+# define GF_HEADROOM 2
+# define LIMB(x) (x)&((1<<28)-1), (x)>>28
+# define FIELD_LITERAL(a,b,c,d,e,f,g,h) \
     {{LIMB(a),LIMB(b),LIMB(c),LIMB(d),LIMB(e),LIMB(f),LIMB(g),LIMB(h)}}
 
-#define LIMB_PLACE_VALUE(i) 28
+# define LIMB_PLACE_VALUE(i) 28
 
 void gf_add_RAW(gf out, const gf a, const gf b)
 {
@@ -54,3 +58,5 @@ void gf_weak_reduce(gf a)
         a->limb[i] = (a->limb[i] & mask) + (a->limb[i - 1] >> 28);
     a->limb[0] = (a->limb[0] & mask) + tmp;
 }
+
+#endif                  /* HEADER_ARCH_32_F_IMPL_H */

diff --git a/crypto/ec/curve448/curve448.c b/crypto/ec/curve448/curve448.c
index bfeaf51bd4187cdc65133e174ee4f75bd7000530..eb8752db38522286e88bb44e841a9ab6a1b08abb 100644 (file)
--- a/crypto/ec/curve448/curve448.c
+++ b/crypto/ec/curve448/curve448.c
@@ -167,6 +167,7 @@ static void sub_niels_from_pt(curve448_point_t d, const niels_t e,
                               int before_double)
 {
     gf a, b, c;
+
     gf_sub_nr(b, d->y, d->x);   /* 3+e */
     gf_mul(a, e->b, b);
     gf_add_nr(b, d->x, d->y);   /* 2+e */
@@ -207,9 +208,9 @@ c448_bool_t curve448_point_eq(const curve448_point_t p,
                               const curve448_point_t q)
 {
     mask_t succ;
+    gf a, b;
 
     /* equality mod 2-torsion compares x/y */
-    gf a, b;
     gf_mul(a, p->y, q->x);
     gf_mul(b, q->y, p->x);
     succ = gf_eq(a, b);
@@ -220,8 +221,8 @@ c448_bool_t curve448_point_eq(const curve448_point_t p,
 c448_bool_t curve448_point_valid(const curve448_point_t p)
 {
     mask_t out;
-
     gf a, b, c;
+
     gf_mul(a, p->x, p->y);
     gf_mul(b, p->z, p->t);
     out = gf_eq(a, b);
@@ -248,17 +249,16 @@ void curve448_precomputed_scalarmul(curve448_point_t out,
                                     const curve448_precomputed_s * table,
                                     const curve448_scalar_t scalar)
 {
-    int i;
-    unsigned j, k;
+    unsigned int i, j, k;
     const unsigned int n = COMBS_N, t = COMBS_T, s = COMBS_S;
     niels_t ni;
-
     curve448_scalar_t scalar1x;
+
     curve448_scalar_add(scalar1x, scalar, precomputed_scalarmul_adjustment);
     curve448_scalar_halve(scalar1x, scalar1x);
 
-    for (i = s - 1; i >= 0; i--) {
-        if (i != (int)s - 1)
+    for (i = s; i > 0; i--) {
+        if (i != s)
             point_double_internal(out, out, 0);
 
         for (j = 0; j < n; j++) {
@@ -266,7 +266,8 @@ void curve448_precomputed_scalarmul(curve448_point_t out,
             mask_t invert;
 
             for (k = 0; k < t; k++) {
-                unsigned int bit = i + s * (k + j * t);
+                unsigned int bit = (i - 1) + s * (k + j * t);
+
                 if (bit < C448_SCALAR_BITS) {
                     tab |=
                         (scalar1x->limb[bit / WBITS] >> (bit % WBITS) & 1) << k;
@@ -281,8 +282,8 @@ void curve448_precomputed_scalarmul(curve448_point_t out,
                                        1 << (t - 1), tab);
 
             cond_neg_niels(ni, invert);
-            if ((i != (int)s - 1) || j) {
-                add_niels_to_pt(out, ni, j == n - 1 && i);
+            if ((i != s) || j != 0) {
+                add_niels_to_pt(out, ni, j == n - 1 && i != 1);
             } else {
                 niels_to_pt(out, ni);
             }
@@ -297,10 +298,10 @@ void curve448_point_mul_by_ratio_and_encode_like_eddsa(
                                     uint8_t enc[EDDSA_448_PUBLIC_BYTES],
                                     const curve448_point_t p)
 {
-
-    /* The point is now on the twisted curve.  Move it to untwisted. */
     gf x, y, z, t;
     curve448_point_t q;
+
+    /* The point is now on the twisted curve.  Move it to untwisted. */
     curve448_point_copy(q, p);
 
     {
@@ -354,9 +355,7 @@ c448_error_t curve448_point_decode_like_eddsa_and_mul_by_ratio(
     enc2[EDDSA_448_PRIVATE_BYTES - 1] &= ~0x80;
 
     succ = gf_deserialize(p->y, enc2, 1, 0);
-#if 0 == 0
     succ &= word_is_zero(enc2[EDDSA_448_PRIVATE_BYTES - 1]);
-#endif
 
     gf_sqr(p->x, p->y);
     gf_sub(p->z, ONE, p->x);    /* num = 1-y^2 */
@@ -371,8 +370,9 @@ c448_error_t curve448_point_decode_like_eddsa_and_mul_by_ratio(
     gf_copy(p->z, ONE);
 
     {
-        /* 4-isogeny 2xy/(y^2-ax^2), (y^2+ax^2)/(2-y^2-ax^2) */
         gf a, b, c, d;
+
+        /* 4-isogeny 2xy/(y^2-ax^2), (y^2+ax^2)/(2-y^2-ax^2) */
         gf_sqr(c, p->x);
         gf_sqr(a, p->y);
         gf_add(d, c, a);
@@ -478,6 +478,7 @@ void curve448_point_mul_by_ratio_and_encode_like_x448(uint8_t
                                                       const curve448_point_t p)
 {
     curve448_point_t q;
+
     curve448_point_copy(q, p);
     gf_invert(q->t, q->x, 0);   /* 1/x */
     gf_mul(q->z, q->t, q->y);   /* y/x */
@@ -523,7 +524,7 @@ struct smvt_control {
 # define NUMTRAILINGZEROS      numtrailingzeros
 static uint32_t numtrailingzeros(uint32_t i)
 {
-    unsigned int tmp;
+    uint32_t tmp;
     uint32_t num = 31;
 
     if (i == 0)
@@ -549,7 +550,8 @@ static uint32_t numtrailingzeros(uint32_t i)
         i = tmp;
         num -= 2;
     }
-    if ((i << 1) != 0)
+    tmp = i << 1;
+    if (tmp != 0)
         num--;
 
     return num;
@@ -593,7 +595,7 @@ static int recode_wnaf(struct smvt_control *control,
             int32_t delta = odd & mask;
 
             assert(position >= 0);
-            if (odd & 1 << (table_bits + 1))
+            if (odd & (1 << (table_bits + 1)))
                 delta -= (1 << (table_bits + 1));
             current -= delta << pos;
             control[position].power = pos + 16 * (w - 1);
@@ -606,9 +608,9 @@ static int recode_wnaf(struct smvt_control *control,
 
     position++;
     n = table_size - position;
-    for (i = 0; i < n; i++) {
+    for (i = 0; i < n; i++)
         control[i] = control[i + position];
-    }
+
     return n - 1;
 }
 
@@ -649,8 +651,8 @@ void curve448_base_double_scalarmul_non_secret(curve448_point_t combo,
                                                const curve448_point_t base2,
                                                const curve448_scalar_t scalar2)
 {
-    const int table_bits_var = C448_WNAF_VAR_TABLE_BITS,
-        table_bits_pre = C448_WNAF_FIXED_TABLE_BITS;
+    const int table_bits_var = C448_WNAF_VAR_TABLE_BITS;
+    const int table_bits_pre = C448_WNAF_FIXED_TABLE_BITS;
     struct smvt_control control_var[C448_SCALAR_BITS /
                                     (C448_WNAF_VAR_TABLE_BITS + 1) + 3];
     struct smvt_control control_pre[C448_SCALAR_BITS /
@@ -682,37 +684,36 @@ void curve448_base_double_scalarmul_non_secret(curve448_point_t combo,
     }
 
     for (i--; i >= 0; i--) {
-        int cv = (i == control_var[contv].power), cp =
-            (i == control_pre[contp].power);
+        int cv = (i == control_var[contv].power);
+        int cp = (i == control_pre[contp].power);
+
         point_double_internal(combo, combo, i && !(cv || cp));
 
         if (cv) {
             assert(control_var[contv].addend);
 
-            if (control_var[contv].addend > 0) {
+            if (control_var[contv].addend > 0)
                 add_pniels_to_pt(combo,
                                  precmp_var[control_var[contv].addend >> 1],
                                  i && !cp);
-            } else {
+            else
                 sub_pniels_from_pt(combo,
                                    precmp_var[(-control_var[contv].addend)
                                               >> 1], i && !cp);
-            }
             contv++;
         }
 
         if (cp) {
             assert(control_pre[contp].addend);
 
-            if (control_pre[contp].addend > 0) {
+            if (control_pre[contp].addend > 0)
                 add_niels_to_pt(combo,
                                 curve448_wnaf_base[control_pre[contp].addend
                                                    >> 1], i);
-            } else {
+            else
                 sub_niels_from_pt(combo,
                                   curve448_wnaf_base[(-control_pre
                                                       [contp].addend) >> 1], i);
-            }
             contp++;
         }
     }

diff --git a/crypto/ec/curve448/curve448_lcl.h b/crypto/ec/curve448/curve448_lcl.h
index 8513d93a2d39b3f2efa47f9b980a4860ac9cd9dc..2bc3bd84c86d02fc16744e38ab46a4b6f25de2d1 100644 (file)
--- a/crypto/ec/curve448/curve448_lcl.h
+++ b/crypto/ec/curve448/curve448_lcl.h
@@ -6,7 +6,9 @@
  * in the file LICENSE in the source distribution or at
  * https://www.openssl.org/source/license.html
  */
-#include "curve448utils.h"
+#ifndef HEADER_CURVE448_LCL_H
+# define HEADER_CURVE448_LCL_H
+# include "curve448utils.h"
 
 int X448(uint8_t out_shared_key[56], const uint8_t private_key[56],
          const uint8_t peer_public_value[56]);
@@ -32,3 +34,5 @@ int ED448ph_verify(const uint8_t hash[64], const uint8_t signature[114],
 
 int ED448_public_from_private(uint8_t out_public_key[57],
                               const uint8_t private_key[57]);
+
+#endif              /* HEADER_CURVE448_LCL_H */

diff --git a/crypto/ec/curve448/curve448utils.h b/crypto/ec/curve448/curve448utils.h
index 22438895948e39944b2eaf48c044c893b9f0d86f..95b8c26da80b9dd750b41956e203f3e25c1102ea 100644 (file)
--- a/crypto/ec/curve448/curve448utils.h
+++ b/crypto/ec/curve448/curve448utils.h
@@ -10,8 +10,8 @@
  * Originally written by Mike Hamburg
  */
 
-#ifndef __C448_COMMON_H__
-# define __C448_COMMON_H__ 1
+#ifndef HEADER_CURVE448UTILS_H
+# define HEADER_CURVE448UTILS_H
 
 # include <openssl/e_os2.h>
 
@@ -58,10 +58,10 @@ typedef int64_t c448_dsword_t;
 # endif
 
 /* C448_TRUE = -1 so that C448_TRUE & x = x */
-static const c448_bool_t C448_TRUE = 0 - (c448_bool_t)1;
+# define C448_TRUE      (0 - (c448_bool_t)1)
 
 /* C448_FALSE = 0 so that C448_FALSE & x = 0 */
-static const c448_bool_t C448_FALSE = 0;
+# define C448_FALSE     0
 
 /* Another boolean type used to indicate success or failure. */
 typedef enum {

diff --git a/crypto/ec/curve448/ed448.h b/crypto/ec/curve448/ed448.h
index eec66296fe5e47c819135f5b514bf64b55d6b660..87c1aed1eb6cc6e279bc645a784008b61fb0e67f 100644 (file)
--- a/crypto/ec/curve448/ed448.h
+++ b/crypto/ec/curve448/ed448.h
@@ -10,8 +10,8 @@
  * Originally written by Mike Hamburg
  */
 
-#ifndef __C448_ED448_H__
-# define __C448_ED448_H__ 1
+#ifndef HEADER_ED448_H
+# define HEADER_ED448_H
 
 # include "point_448.h"
 
@@ -108,8 +108,7 @@ c448_error_t c448_ed448_sign_prehash(
  *
  * For Ed25519, it is unsafe to use the same key for both prehashed and
  * non-prehashed messages, at least without some very careful protocol-level
- * disambiguation.  For Ed448 it is safe.  The C++ wrapper is designed to make
- * it harder to screw this up, but this C code gives you no seat belt.
+ * disambiguation.  For Ed448 it is safe.
  */
 c448_error_t c448_ed448_verify(const uint8_t
                                  signature[EDDSA_448_SIGNATURE_BYTES],
@@ -196,4 +195,4 @@ c448_error_t c448_ed448_convert_private_key_to_x448(
                             uint8_t x[X448_PRIVATE_BYTES],
                             const uint8_t ed[EDDSA_448_PRIVATE_BYTES]);
 
-#endif                          /* __C448_ED448_H__ */
+#endif                          /* HEADER_ED448_H */

diff --git a/crypto/ec/curve448/eddsa.c b/crypto/ec/curve448/eddsa.c
index aac8a9e728669908b92d48b2bef61bfa8b2eb308..556edf07b91cbf2b3d29e456ea383fbeb2a36e38 100644 (file)
--- a/crypto/ec/curve448/eddsa.c
+++ b/crypto/ec/curve448/eddsa.c
@@ -169,7 +169,7 @@ c448_error_t c448_ed448_sign(
                                      expanded + EDDSA_448_PRIVATE_BYTES,
                                      EDDSA_448_PRIVATE_BYTES)
                 || !EVP_DigestUpdate(hashctx, message, message_len)) {
-                OPENSSL_cleanse(expanded, sizeof(expanded));
+            OPENSSL_cleanse(expanded, sizeof(expanded));
             goto err;
         }
         OPENSSL_cleanse(expanded, sizeof(expanded));
@@ -191,9 +191,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);
@@ -207,10 +206,10 @@ 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)
+                || !EVP_DigestUpdate(hashctx, message, message_len)
+                || !EVP_DigestFinalXOF(hashctx, challenge, sizeof(challenge)))
             goto err;
 
         curve448_scalar_decode_long(challenge_scalar, challenge,
@@ -313,12 +312,8 @@ 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,

diff --git a/crypto/ec/curve448/f_generic.c b/crypto/ec/curve448/f_generic.c
index 81c75761a8fddc34d19243b5bf34f8db8653c36c..341eb3f3b04940df958a1f939fab528b1e60ab0a 100644 (file)
--- a/crypto/ec/curve448/f_generic.c
+++ b/crypto/ec/curve448/f_generic.c
@@ -46,6 +46,7 @@ void gf_serialize(uint8_t serial[SER_BYTES], const gf x, int with_hibit)
 mask_t gf_hibit(const gf x)
 {
     gf y;
+
     gf_add(y, x, x);
     gf_strong_reduce(y);
     return 0 - (y->limb[0] & 1);
@@ -55,6 +56,7 @@ mask_t gf_hibit(const gf x)
 mask_t gf_lobit(const gf x)
 {
     gf y;
+
     gf_copy(y, x);
     gf_strong_reduce(y);
     return 0 - (y->limb[0] & 1);
@@ -168,6 +170,7 @@ mask_t gf_eq(const gf a, const gf b)
 mask_t gf_isr(gf a, const gf x)
 {
     gf L0, L1, L2;
+
     gf_sqr(L1, x);
     gf_mul(L2, x, L1);
     gf_sqr(L1, L2);

diff --git a/crypto/ec/curve448/field.h b/crypto/ec/curve448/field.h
index 5bc16bc2beed2e31520d91f50d71d1a91a10482f..18d91f36001d79846dff8f1c9b091c22d220994a 100644 (file)
--- a/crypto/ec/curve448/field.h
+++ b/crypto/ec/curve448/field.h
@@ -10,8 +10,8 @@
  * Originally written by Mike Hamburg
  */
 
-#ifndef __GF_H__
-# define __GF_H__
+#ifndef HEADER_FIELD_H
+# define HEADER_FIELD_H
 
 # include "internal/constant_time_locl.h"
 # include <string.h>
@@ -23,7 +23,7 @@
 # define SER_BYTES 56
 
 # if defined(__GNUC__) || defined(__clang__)
-#  define INLINE_UNUSED __inline__ __attribute__((unused,always_inline))
+#  define INLINE_UNUSED __inline__ __attribute__((__unused__,__always_inline__))
 #  define RESTRICT __restrict__
 #  define ALIGNED __attribute__((aligned(32)))
 # else
@@ -169,4 +169,4 @@ static ossl_inline void gf_cond_swap(gf x, gf_s * RESTRICT y, mask_t swap)
     }
 }
 
-#endif                          /* __GF_H__ */
+#endif                          /* HEADER_FIELD_H */

diff --git a/crypto/ec/curve448/point_448.h b/crypto/ec/curve448/point_448.h
index 90a7c3ba05716f51eab6ed517e9410a71b551070..33e4001020a48a20972474d428c4d88669d76d56 100644 (file)
--- a/crypto/ec/curve448/point_448.h
+++ b/crypto/ec/curve448/point_448.h
@@ -10,8 +10,8 @@
  * Originally written by Mike Hamburg
  */
 
-#ifndef __C448_POINT_448_H__
-# define __C448_POINT_448_H__ 1
+#ifndef HEADER_POINT_448_H
+# define HEADER_POINT_448_H
 
 # include "curve448utils.h"
 # include "field.h"
@@ -280,4 +280,4 @@ void curve448_scalar_destroy(curve448_scalar_t scalar);
 /* Overwrite point with zeros. */
 void curve448_point_destroy(curve448_point_t point);
 
-#endif                          /* __C448_POINT_448_H__ */
+#endif                          /* HEADER_POINT_448_H */

diff --git a/crypto/ec/curve448/word.h b/crypto/ec/curve448/word.h
index a180850df8d3af97dbc8bf27cd7e52c3e185ef49..ff85c4c458a334c3c2bee4eb751c56362aa1d921 100644 (file)
--- a/crypto/ec/curve448/word.h
+++ b/crypto/ec/curve448/word.h
@@ -10,8 +10,8 @@
  * Originally written by Mike Hamburg
  */
 
-#ifndef __WORD_H__
-# define __WORD_H__
+#ifndef HEADER_WORD_H
+# define HEADER_WORD_H
 
 # include <string.h>
 
@@ -168,4 +168,4 @@ static ossl_inline void ignore_result(c448_bool_t boo)
     (void)boo;
 }
 
-#endif                          /* __WORD_H__ */
+#endif                          /* HEADER_WORD_H */

diff --git a/test/curve448_internal_test.c b/test/curve448_internal_test.c
index f01d81bbd141fcf7366f54b1c38e5571a46da8b5..aac4818965a840c37a963ec8d1558f3114978ec3 100644 (file)
--- a/test/curve448_internal_test.c
+++ b/test/curve448_internal_test.c
@@ -587,8 +587,8 @@ static const uint8_t *dohash(EVP_MD_CTX *hashctx, const uint8_t *msg,
     static uint8_t hashout[64];
 
     if (!EVP_DigestInit_ex(hashctx, EVP_shake256(), NULL)
-        || !EVP_DigestUpdate(hashctx, msg, msglen)
-        || !EVP_DigestFinalXOF(hashctx, hashout, sizeof(hashout)))
+            || !EVP_DigestUpdate(hashctx, msg, msglen)
+            || !EVP_DigestFinalXOF(hashctx, hashout, sizeof(hashout)))
         return NULL;
 
     return hashout;