From 53ef3252fa3f7008feaf5696d24a05bef0efa676 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 7 Feb 2018 11:47:41 +0000 Subject: [PATCH] More style fixes for the curve448 code Reviewed-by: Bernd Edlinger (Merged from https://github.com/openssl/openssl/pull/5105) --- crypto/ec/curve448/arch_32/f_impl.h | 12 +++++------- crypto/ec/curve448/curve448.c | 19 +++++++++---------- crypto/ec/curve448/ed448.h | 9 +++------ crypto/ec/curve448/eddsa.c | 10 ++++------ crypto/ec/curve448/field.h | 24 ++++++++++-------------- crypto/ec/curve448/point_448.h | 8 ++++---- crypto/ec/curve448/scalar.c | 1 + crypto/ec/curve448/word.h | 2 +- 8 files changed, 37 insertions(+), 48 deletions(-) diff --git a/crypto/ec/curve448/arch_32/f_impl.h b/crypto/ec/curve448/arch_32/f_impl.h index 217f9947e9..a8b2662182 100644 --- a/crypto/ec/curve448/arch_32/f_impl.h +++ b/crypto/ec/curve448/arch_32/f_impl.h @@ -14,9 +14,9 @@ # 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(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 @@ -24,18 +24,16 @@ void gf_add_RAW(gf out, const gf a, const gf b) { unsigned int i; - for (i = 0; i < NLIMBS; i++) { + for (i = 0; i < NLIMBS; i++) out->limb[i] = a->limb[i] + b->limb[i]; - } } void gf_sub_RAW(gf out, const gf a, const gf b) { unsigned int i; - for (i = 0; i < NLIMBS; i++) { + for (i = 0; i < NLIMBS; i++) out->limb[i] = a->limb[i] - b->limb[i]; - } } void gf_bias(gf a, int amt) diff --git a/crypto/ec/curve448/curve448.c b/crypto/ec/curve448/curve448.c index 2d91f9715d..dda3da4ffe 100644 --- a/crypto/ec/curve448/curve448.c +++ b/crypto/ec/curve448/curve448.c @@ -42,8 +42,8 @@ extern const struct curve448_precomputed_s *curve448_precomputed_base; static void gf_invert(gf y, const gf x, int assert_nonzero) { mask_t ret; - gf t1, t2; + gf_sqr(t1, x); /* o^2 */ ret = gf_isr(t2, t1); /* +-1/sqrt(o^2) = +-1/o */ (void)ret; @@ -248,10 +248,9 @@ void curve448_precomputed_scalarmul(curve448_point_t out, for (k = 0; k < t; k++) { unsigned int bit = (i - 1) + s * (k + j * t); - if (bit < C448_SCALAR_BITS) { + if (bit < C448_SCALAR_BITS) tab |= (scalar1x->limb[bit / WBITS] >> (bit % WBITS) & 1) << k; - } } invert = (tab >> (t - 1)) - 1; @@ -262,11 +261,10 @@ void curve448_precomputed_scalarmul(curve448_point_t out, 1 << (t - 1), tab); cond_neg_niels(ni, invert); - if ((i != s) || j != 0) { + if ((i != s) || j != 0) add_niels_to_pt(out, ni, j == n - 1 && i != 1); - } else { + else niels_to_pt(out, ni); - } } } @@ -485,9 +483,9 @@ void x448_derive_public_key(uint8_t out[X_PUBLIC_BYTES], curve448_scalar_decode_long(the_scalar, scalar2, sizeof(scalar2)); /* Compensate for the encoding ratio */ - for (i = 1; i < X448_ENCODE_RATIO; i <<= 1) { + for (i = 1; i < X448_ENCODE_RATIO; i <<= 1) curve448_scalar_halve(the_scalar, the_scalar); - } + curve448_precomputed_scalarmul(p, curve448_precomputed_base, the_scalar); curve448_point_mul_by_ratio_and_encode_like_x448(out, p); curve448_point_destroy(p); @@ -566,7 +564,7 @@ static int recode_wnaf(struct smvt_control *control, if (w < (C448_SCALAR_BITS - 1) / 16 + 1) { /* Refill the 16 high bits of current */ current += (uint32_t)((scalar->limb[w / B_OVER_16] - >> (16 * (w % B_OVER_16))) << 16); + >> (16 * (w % B_OVER_16))) << 16); } while (current & 0xFFFF) { @@ -646,7 +644,8 @@ void curve448_base_double_scalarmul_non_secret(curve448_point_t combo, if (i < 0) { curve448_point_copy(combo, curve448_point_identity); return; - } else if (i > control_pre[0].power) { + } + if (i > control_pre[0].power) { pniels_to_pt(combo, precmp_var[control_var[0].addend >> 1]); contv++; } else if (i == control_pre[0].power && i >= 0) { diff --git a/crypto/ec/curve448/ed448.h b/crypto/ec/curve448/ed448.h index 87c1aed1eb..5fe939e8e1 100644 --- a/crypto/ec/curve448/ed448.h +++ b/crypto/ec/curve448/ed448.h @@ -56,8 +56,7 @@ c448_error_t c448_ed448_derive_public_key( * * 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_sign( uint8_t signature[EDDSA_448_SIGNATURE_BYTES], @@ -81,8 +80,7 @@ c448_error_t c448_ed448_sign( * * 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_sign_prehash( uint8_t signature[EDDSA_448_SIGNATURE_BYTES], @@ -133,8 +131,7 @@ c448_error_t c448_ed448_verify(const uint8_t * * 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_prehash( const uint8_t signature[EDDSA_448_SIGNATURE_BYTES], diff --git a/crypto/ec/curve448/eddsa.c b/crypto/ec/curve448/eddsa.c index 556edf07b9..09304020c7 100644 --- a/crypto/ec/curve448/eddsa.c +++ b/crypto/ec/curve448/eddsa.c @@ -9,13 +9,12 @@ * * Originally written by Mike Hamburg */ +#include #include #include - #include "curve448_lcl.h" #include "word.h" #include "ed448.h" -#include #include "internal/numbers.h" #define COFACTOR 4 @@ -62,12 +61,12 @@ 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] = 2 + word_is_zero(prehashed) + word_is_zero(for_prehash); + 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)) @@ -320,7 +319,6 @@ 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; diff --git a/crypto/ec/curve448/field.h b/crypto/ec/curve448/field.h index 18d91f3600..c3bad40e3b 100644 --- a/crypto/ec/curve448/field.h +++ b/crypto/ec/curve448/field.h @@ -25,7 +25,7 @@ # if defined(__GNUC__) || defined(__clang__) # define INLINE_UNUSED __inline__ __attribute__((__unused__,__always_inline__)) # define RESTRICT __restrict__ -# define ALIGNED __attribute__((aligned(32))) +# define ALIGNED __attribute__((__aligned__(32))) # else # define INLINE_UNUSED ossl_inline # define RESTRICT @@ -68,9 +68,7 @@ mask_t gf_deserialize(gf x, const uint8_t serial[SER_BYTES], int with_hibit, # include "f_impl.h" /* Bring in the inline implementations */ -# ifndef LIMBPERM -# define LIMBPERM(i) (i) -# endif +# define LIMBPERM(i) (i) # define LIMB_MASK(i) (((1)< 0); if (n & 1) { gf_sqr(y, x); @@ -132,14 +131,12 @@ static ossl_inline void gf_cond_sel(gf x, const gf y, const gf z, mask_t is_z) for (i = 0; i < NLIMBS; i++) { #if ARCH_WORD_BITS == 32 - x[0].limb[i] = constant_time_select_32((uint32_t)is_z, - (uint32_t)(z[0].limb[i]), - (uint32_t)(y[0].limb[i])); + x[0].limb[i] = constant_time_select_32(is_z, z[0].limb[i], + y[0].limb[i]); #else /* Must be 64 bit */ - x[0].limb[i] = constant_time_select_64((uint64_t)is_z, - (uint64_t)(z[0].limb[i]), - (uint64_t)(y[0].limb[i])); + x[0].limb[i] = constant_time_select_64(is_z, z[0].limb[i], + y[0].limb[i]); #endif } } @@ -148,6 +145,7 @@ static ossl_inline void gf_cond_sel(gf x, const gf y, const gf z, mask_t is_z) static ossl_inline void gf_cond_neg(gf x, mask_t neg) { gf y; + gf_sub(y, ZERO, x); gf_cond_sel(x, x, y, neg); } @@ -159,12 +157,10 @@ static ossl_inline void gf_cond_swap(gf x, gf_s * RESTRICT y, mask_t swap) for (i = 0; i < NLIMBS; i++) { #if ARCH_WORD_BITS == 32 - constant_time_cond_swap_32((uint32_t)swap, (uint32_t *)&(x[0].limb[i]), - (uint32_t *)&(y->limb[i])); + constant_time_cond_swap_32(swap, &(x[0].limb[i]), &(y->limb[i])); #else /* Must be 64 bit */ - constant_time_cond_swap_64((uint64_t)swap, (uint64_t *)&(x[0].limb[i]), - (uint64_t *)&(y->limb[i])); + constant_time_cond_swap_64(swap, &(x[0].limb[i]), &(y->limb[i])); #endif } } diff --git a/crypto/ec/curve448/point_448.h b/crypto/ec/curve448/point_448.h index 30e5b780c0..03af43b6c6 100644 --- a/crypto/ec/curve448/point_448.h +++ b/crypto/ec/curve448/point_448.h @@ -115,7 +115,7 @@ void curve448_scalar_encode(unsigned char ser[C448_SCALAR_BYTES], const curve448_scalar_t s); /* - * Add two scalars. The scalars may use the same memory. + * Add two scalars. |a|, |b| and |out| may alias each other. * * a (in): One scalar. * b (in): Another scalar. @@ -125,7 +125,7 @@ void curve448_scalar_add(curve448_scalar_t out, const curve448_scalar_t a, const curve448_scalar_t b); /* - * Subtract two scalars. The scalars may use the same memory. + * Subtract two scalars. |a|, |b| and |out| may alias each other. * a (in): One scalar. * b (in): Another scalar. * out (out): a-b. @@ -134,7 +134,7 @@ void curve448_scalar_sub(curve448_scalar_t out, const curve448_scalar_t a, const curve448_scalar_t b); /* - * Multiply two scalars. The scalars may use the same memory. + * Multiply two scalars. |a|, |b| and |out| may alias each other. * * a (in): One scalar. * b (in): Another scalar. @@ -144,7 +144,7 @@ void curve448_scalar_mul(curve448_scalar_t out, const curve448_scalar_t a, const curve448_scalar_t b); /* -* Halve a scalar. The scalars may use the same memory. +* Halve a scalar. |a| and |out| may alias each other. * * a (in): A scalar. * out (out): a/2. diff --git a/crypto/ec/curve448/scalar.c b/crypto/ec/curve448/scalar.c index db4841e6bf..f899ce2ae6 100644 --- a/crypto/ec/curve448/scalar.c +++ b/crypto/ec/curve448/scalar.c @@ -223,6 +223,7 @@ void curve448_scalar_halve(curve448_scalar_t out, const curve448_scalar_t a) c448_word_t mask = 0 - (a->limb[0] & 1); c448_dword_t chain = 0; unsigned int i; + for (i = 0; i < C448_SCALAR_LIMBS; i++) { chain = (chain + a->limb[i]) + (sc_p->limb[i] & mask); out->limb[i] = (c448_word_t)chain; diff --git a/crypto/ec/curve448/word.h b/crypto/ec/curve448/word.h index ff85c4c458..8a45e1e56f 100644 --- a/crypto/ec/curve448/word.h +++ b/crypto/ec/curve448/word.h @@ -56,7 +56,7 @@ typedef int64_t dsword_t; # if C448_WORD_BITS == 64 # define SC_LIMB(x) (x) # elif C448_WORD_BITS == 32 -# define SC_LIMB(x) ((uint32_t)x),(x>>32) +# define SC_LIMB(x) ((uint32_t)x),(x >> 32) # else # error "For now we only support 32- and 64-bit architectures." # endif