From c30a1b3b33dad17db8a221bc1a2bc80b4f5f56a2 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Fri, 24 Apr 2015 16:53:30 +0200 Subject: [PATCH] Error checking and memory leak fixes in NISTZ256. Thanks to Brian Smith for reporting these issues. Reviewed-by: Richard Levitte --- crypto/ec/ec.h | 11 ++++--- crypto/ec/ec_err.c | 15 ++++----- crypto/ec/ecp_nistz256.c | 67 +++++++++++++++++++++++++--------------- 3 files changed, 56 insertions(+), 37 deletions(-) diff --git a/crypto/ec/ec.h b/crypto/ec/ec.h index 98edfdf8bc..6d3178f609 100644 --- a/crypto/ec/ec.h +++ b/crypto/ec/ec.h @@ -1097,6 +1097,12 @@ void ERR_load_EC_strings(void); # define EC_F_ECPARAMETERS_PRINT_FP 148 # define EC_F_ECPKPARAMETERS_PRINT 149 # define EC_F_ECPKPARAMETERS_PRINT_FP 150 +# define EC_F_ECP_NISTZ256_GET_AFFINE 240 +# define EC_F_ECP_NISTZ256_MULT_PRECOMPUTE 243 +# define EC_F_ECP_NISTZ256_POINTS_MUL 241 +# define EC_F_ECP_NISTZ256_PRE_COMP_NEW 244 +# define EC_F_ECP_NISTZ256_SET_WORDS 245 +# define EC_F_ECP_NISTZ256_WINDOWED_MUL 242 # define EC_F_ECP_NIST_MOD_192 203 # define EC_F_ECP_NIST_MOD_224 204 # define EC_F_ECP_NIST_MOD_256 205 @@ -1208,11 +1214,6 @@ void ERR_load_EC_strings(void); # define EC_F_NISTP224_PRE_COMP_NEW 227 # define EC_F_NISTP256_PRE_COMP_NEW 236 # define EC_F_NISTP521_PRE_COMP_NEW 237 -# define EC_F_ECP_NISTZ256_GET_AFFINE 240 -# define EC_F_ECP_NISTZ256_POINTS_MUL 241 -# define EC_F_ECP_NISTZ256_WINDOWED_MUL 242 -# define EC_F_ECP_NISTZ256_MULT_PRECOMPUTE 243 -# define EC_F_ECP_NISTZ256_PRE_COMP_NEW 244 # define EC_F_O2I_ECPUBLICKEY 152 # define EC_F_OLD_EC_PRIV_DECODE 222 # define EC_F_PKEY_EC_CTRL 197 diff --git a/crypto/ec/ec_err.c b/crypto/ec/ec_err.c index 13b32c78ac..6fe5baafd4 100644 --- a/crypto/ec/ec_err.c +++ b/crypto/ec/ec_err.c @@ -1,6 +1,6 @@ /* crypto/ec/ec_err.c */ /* ==================================================================== - * Copyright (c) 1999-2014 The OpenSSL Project. All rights reserved. + * Copyright (c) 1999-2015 The OpenSSL Project. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -89,6 +89,13 @@ static ERR_STRING_DATA EC_str_functs[] = { {ERR_FUNC(EC_F_ECPARAMETERS_PRINT_FP), "ECParameters_print_fp"}, {ERR_FUNC(EC_F_ECPKPARAMETERS_PRINT), "ECPKParameters_print"}, {ERR_FUNC(EC_F_ECPKPARAMETERS_PRINT_FP), "ECPKParameters_print_fp"}, + {ERR_FUNC(EC_F_ECP_NISTZ256_GET_AFFINE), "ecp_nistz256_get_affine"}, + {ERR_FUNC(EC_F_ECP_NISTZ256_MULT_PRECOMPUTE), + "ecp_nistz256_mult_precompute"}, + {ERR_FUNC(EC_F_ECP_NISTZ256_POINTS_MUL), "ecp_nistz256_points_mul"}, + {ERR_FUNC(EC_F_ECP_NISTZ256_PRE_COMP_NEW), "ecp_nistz256_pre_comp_new"}, + {ERR_FUNC(EC_F_ECP_NISTZ256_SET_WORDS), "ecp_nistz256_set_words"}, + {ERR_FUNC(EC_F_ECP_NISTZ256_WINDOWED_MUL), "ecp_nistz256_windowed_mul"}, {ERR_FUNC(EC_F_ECP_NIST_MOD_192), "ECP_NIST_MOD_192"}, {ERR_FUNC(EC_F_ECP_NIST_MOD_224), "ECP_NIST_MOD_224"}, {ERR_FUNC(EC_F_ECP_NIST_MOD_256), "ECP_NIST_MOD_256"}, @@ -239,12 +246,6 @@ static ERR_STRING_DATA EC_str_functs[] = { {ERR_FUNC(EC_F_NISTP224_PRE_COMP_NEW), "NISTP224_PRE_COMP_NEW"}, {ERR_FUNC(EC_F_NISTP256_PRE_COMP_NEW), "NISTP256_PRE_COMP_NEW"}, {ERR_FUNC(EC_F_NISTP521_PRE_COMP_NEW), "NISTP521_PRE_COMP_NEW"}, - {ERR_FUNC(EC_F_ECP_NISTZ256_GET_AFFINE), "ecp_nistz256_get_affine"}, - {ERR_FUNC(EC_F_ECP_NISTZ256_POINTS_MUL), "ecp_nistz256_points_mul"}, - {ERR_FUNC(EC_F_ECP_NISTZ256_WINDOWED_MUL), "ecp_nistz256_windowed_mul"}, - {ERR_FUNC(EC_F_ECP_NISTZ256_MULT_PRECOMPUTE), - "ecp_nistz256_mult_precompute"}, - {ERR_FUNC(EC_F_ECP_NISTZ256_PRE_COMP_NEW), "ecp_nistz256_pre_comp_new"}, {ERR_FUNC(EC_F_O2I_ECPUBLICKEY), "o2i_ECPublicKey"}, {ERR_FUNC(EC_F_OLD_EC_PRIV_DECODE), "OLD_EC_PRIV_DECODE"}, {ERR_FUNC(EC_F_PKEY_EC_CTRL), "PKEY_EC_CTRL"}, diff --git a/crypto/ec/ecp_nistz256.c b/crypto/ec/ecp_nistz256.c index 7e521d856b..35c56c7777 100644 --- a/crypto/ec/ecp_nistz256.c +++ b/crypto/ec/ecp_nistz256.c @@ -222,6 +222,18 @@ static BN_ULONG is_one(const BN_ULONG a[P256_LIMBS]) return is_zero(res); } +static int ecp_nistz256_set_words(BIGNUM *a, BN_ULONG words[P256_LIMBS]) + { + if (bn_wexpand(a, P256_LIMBS) == NULL) { + ECerr(EC_F_ECP_NISTZ256_SET_WORDS, ERR_R_MALLOC_FAILURE); + return 0; + } + memcpy(a->d, words, sizeof(BN_ULONG) * P256_LIMBS); + a->top = P256_LIMBS; + bn_correct_top(a); + return 1; +} + #ifndef ECP_NISTZ256_REFERENCE_IMPLEMENTATION void ecp_nistz256_point_double(P256_POINT *r, const P256_POINT *a); void ecp_nistz256_point_add(P256_POINT *r, @@ -1110,6 +1122,7 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, const EC_PRE_COMP *pre_comp = NULL; const EC_POINT *generator = NULL; unsigned int index = 0; + BN_CTX *new_ctx = NULL; const unsigned int window_size = 7; const unsigned int mask = (1 << (window_size + 1)) - 1; unsigned int wvalue; @@ -1123,6 +1136,7 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, EC_R_INCOMPATIBLE_OBJECTS); return 0; } + if ((scalar == NULL) && (num == 0)) return EC_POINT_set_to_infinity(group, r); @@ -1133,13 +1147,13 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, } } - /* Need 256 bits for space for all coordinates. */ - bn_wexpand(&r->X, P256_LIMBS); - bn_wexpand(&r->Y, P256_LIMBS); - bn_wexpand(&r->Z, P256_LIMBS); - r->X.top = P256_LIMBS; - r->Y.top = P256_LIMBS; - r->Z.top = P256_LIMBS; + if (ctx == NULL) { + ctx = new_ctx = BN_CTX_new(); + if (ctx == NULL) + goto err; + } + + BN_CTX_start(ctx); if (scalar) { generator = EC_GROUP_get0_generator(group); @@ -1164,8 +1178,10 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, goto err; if (!ecp_nistz256_set_from_affine - (pre_comp_generator, group, pre_comp->precomp[0], ctx)) + (pre_comp_generator, group, pre_comp->precomp[0], ctx)) { + EC_POINT_free(pre_comp_generator); goto err; + } if (0 == EC_POINT_cmp(group, generator, pre_comp_generator, ctx)) preComputedTable = (const PRECOMP256_ROW *)pre_comp->precomp; @@ -1269,14 +1285,14 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, new_scalars = OPENSSL_malloc((num + 1) * sizeof(BIGNUM *)); if (!new_scalars) { ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, ERR_R_MALLOC_FAILURE); - return 0; + goto err; } new_points = OPENSSL_malloc((num + 1) * sizeof(EC_POINT *)); if (!new_points) { OPENSSL_free(new_scalars); ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, ERR_R_MALLOC_FAILURE); - return 0; + goto err; } memcpy(new_scalars, scalars, num * sizeof(BIGNUM *)); @@ -1305,18 +1321,20 @@ static int ecp_nistz256_points_mul(const EC_GROUP *group, OPENSSL_free(scalars); } - memcpy(r->X.d, p.p.X, sizeof(p.p.X)); - memcpy(r->Y.d, p.p.Y, sizeof(p.p.Y)); - memcpy(r->Z.d, p.p.Z, sizeof(p.p.Z)); /* Not constant-time, but we're only operating on the public output. */ - bn_correct_top(&r->X); - bn_correct_top(&r->Y); - bn_correct_top(&r->Z); + if (!ecp_nistz256_set_words(&r->X, p.p.X) || + !ecp_nistz256_set_words(&r->Y, p.p.Y) || + !ecp_nistz256_set_words(&r->Z, p.p.Z)) { + goto err; + } r->Z_is_one = is_one(p.p.Z); ret = 1; - err: +err: + if (ctx) + BN_CTX_end(ctx); + BN_CTX_free(new_ctx); return ret; } @@ -1329,6 +1347,7 @@ static int ecp_nistz256_get_affine(const EC_GROUP *group, BN_ULONG x_aff[P256_LIMBS]; BN_ULONG y_aff[P256_LIMBS]; BN_ULONG point_x[P256_LIMBS], point_y[P256_LIMBS], point_z[P256_LIMBS]; + BN_ULONG x_ret[P256_LIMBS], y_ret[P256_LIMBS]; if (EC_POINT_is_at_infinity(group, point)) { ECerr(EC_F_ECP_NISTZ256_GET_AFFINE, EC_R_POINT_AT_INFINITY); @@ -1347,19 +1366,17 @@ static int ecp_nistz256_get_affine(const EC_GROUP *group, ecp_nistz256_mul_mont(x_aff, z_inv2, point_x); if (x != NULL) { - bn_wexpand(x, P256_LIMBS); - x->top = P256_LIMBS; - ecp_nistz256_from_mont(x->d, x_aff); - bn_correct_top(x); + ecp_nistz256_from_mont(x_ret, x_aff); + if (!ecp_nistz256_set_words(x, x_ret)) + return 0; } if (y != NULL) { ecp_nistz256_mul_mont(z_inv3, z_inv3, z_inv2); ecp_nistz256_mul_mont(y_aff, z_inv3, point_y); - bn_wexpand(y, P256_LIMBS); - y->top = P256_LIMBS; - ecp_nistz256_from_mont(y->d, y_aff); - bn_correct_top(y); + ecp_nistz256_from_mont(y_ret, y_aff); + if (!ecp_nistz256_set_words(y, y_ret)) + return 0; } return 1; -- 2.40.0