commit 0c9b7b5de2884cccd12527ff82253d3d0c29a26c
author David Benjamin <davidben@google.com> Sat Dec 30 00:33:54 2017 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Mon Jan 08 20:03:42 2018 +0000
tree c903a42b1bf899b24abda593842134a5ab681cf6
parent 9112631c1f5a0eea3ad90984da5b3a913d8e5caa

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 <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/fipsmodule/ec/p224-64.c

diff --git a/crypto/fipsmodule/ec/p224-64.c b/crypto/fipsmodule/ec/p224-64.c
index 56ece93..71a8af0 100644
--- a/crypto/fipsmodule/ec/p224-64.c
+++ b/crypto/fipsmodule/ec/p224-64.c
@@ -1015,22 +1015,27 @@
   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;

third_party/fiat/p256.c

diff --git a/third_party/fiat/p256.c b/third_party/fiat/p256.c
index fa17f99..f1d5316 100644
--- a/third_party/fiat/p256.c
+++ b/third_party/fiat/p256.c
@@ -1645,9 +1645,13 @@
   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 @@
   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;