From 0c9b7b5de2884cccd12527ff82253d3d0c29a26c Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 30 Dec 2017 00:33:54 -0500 Subject: [PATCH] Align various point_get_affine_coordinates implementations. The P-224 implementation was missing the optimization to avoid doing extra work when asking for only one coordinate (ECDH and ECDSA both involve an x-coordinate query). The P-256 implementation was missing the optimization to do one less Montgomery reduction. TODO - Benchmarks Change-Id: I268d9c24737c6da9efaf1c73395b73dd97355de7 Reviewed-on: https://boringssl-review.googlesource.com/24690 Reviewed-by: Adam Langley Commit-Queue: Adam Langley CQ-Verified: CQ bot account: commit-bot@chromium.org --- crypto/fipsmodule/ec/p224-64.c | 33 +++++++++++++++++++-------------- third_party/fiat/p256.c | 7 +++++-- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/crypto/fipsmodule/ec/p224-64.c b/crypto/fipsmodule/ec/p224-64.c index 56ece932..71a8af0a 100644 --- a/crypto/fipsmodule/ec/p224-64.c +++ b/crypto/fipsmodule/ec/p224-64.c @@ -1015,22 +1015,27 @@ static int ec_GFp_nistp224_point_get_affine_coordinates(const EC_GROUP *group, p224_felem_inv(z2, z1); p224_felem_square(tmp, z2); p224_felem_reduce(z1, tmp); - p224_felem_mul(tmp, x_in, z1); - p224_felem_reduce(x_in, tmp); - p224_felem_contract(x_out, x_in); - if (x != NULL && !p224_felem_to_BN(x, x_out)) { - OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); - return 0; + + if (x != NULL) { + p224_felem_mul(tmp, x_in, z1); + p224_felem_reduce(x_in, tmp); + p224_felem_contract(x_out, x_in); + if (!p224_felem_to_BN(x, x_out)) { + OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); + return 0; + } } - p224_felem_mul(tmp, z1, z2); - p224_felem_reduce(z1, tmp); - p224_felem_mul(tmp, y_in, z1); - p224_felem_reduce(y_in, tmp); - p224_felem_contract(y_out, y_in); - if (y != NULL && !p224_felem_to_BN(y, y_out)) { - OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); - return 0; + if (y != NULL) { + p224_felem_mul(tmp, z1, z2); + p224_felem_reduce(z1, tmp); + p224_felem_mul(tmp, y_in, z1); + p224_felem_reduce(y_in, tmp); + p224_felem_contract(y_out, y_in); + if (!p224_felem_to_BN(y, y_out)) { + OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); + return 0; + } } return 1; diff --git a/third_party/fiat/p256.c b/third_party/fiat/p256.c index fa17f990..f1d53165 100644 --- a/third_party/fiat/p256.c +++ b/third_party/fiat/p256.c @@ -1645,9 +1645,13 @@ static int ec_GFp_nistp256_point_get_affine_coordinates(const EC_GROUP *group, fe_inv(z2, z1); fe_sqr(z1, z2); + // Instead of using |fe_from_montgomery| to convert the |x| coordinate and + // then calling |fe_from_montgomery| again to convert the |y| coordinate + // below, convert the common factor |z1| once now, saving one reduction. + fe_from_montgomery(z1); + if (x_out != NULL) { fe_mul(x, x, z1); - fe_from_montgomery(x); if (!fe_to_BN(x_out, x)) { OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); return 0; @@ -1657,7 +1661,6 @@ static int ec_GFp_nistp256_point_get_affine_coordinates(const EC_GROUP *group, if (y_out != NULL) { fe_mul(z1, z1, z2); fe_mul(y, y, z1); - fe_from_montgomery(y); if (!fe_to_BN(y_out, y)) { OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); return 0;