commit 5c0e0cec83a2c30eee4213b4f90112a46c1843df
author David Benjamin <davidben@google.com> Mon Apr 23 18:33:20 2018 -0400
committer Adam Langley <alangley@gmail.com> Tue Apr 24 16:16:53 2018 +0000
tree ada7dec61e6ae1b46d6742a8c34218e4b4561813
parent f5858ca0088a8dc2933c3804c3a7d4cdb015a14b

Remove Z = 1 special-case in generic point_get_affine.

As the point may be the output of some private key operation, whether Z
accidentally hit one is secret.

Bug: 239
Change-Id: I7db34cd3b5dd5ca4b96980e8993a9b4eda49eb88
Reviewed-on: https://boringssl-review.googlesource.com/27664
Reviewed-by: Adam Langley <alangley@gmail.com>

crypto/fipsmodule/ec/ec_montgomery.c

diff --git a/crypto/fipsmodule/ec/ec_montgomery.c b/crypto/fipsmodule/ec/ec_montgomery.c
index 9ede5a1..d80fa23 100644
--- a/crypto/fipsmodule/ec/ec_montgomery.c
+++ b/crypto/fipsmodule/ec/ec_montgomery.c
@@ -184,68 +184,58 @@
 
   BN_CTX_start(ctx);
 
-  if (BN_cmp(&point->Z, &group->one) == 0) {
-    // |point| is already affine.
-    if (x != NULL && !BN_from_montgomery(x, &point->X, group->mont, ctx)) {
+  // transform  (X, Y, Z)  into  (x, y) := (X/Z^2, Y/Z^3)
+
+  BIGNUM *Z_1 = BN_CTX_get(ctx);
+  BIGNUM *Z_2 = BN_CTX_get(ctx);
+  BIGNUM *Z_3 = BN_CTX_get(ctx);
+  if (Z_1 == NULL ||
+      Z_2 == NULL ||
+      Z_3 == NULL) {
+    goto err;
+  }
+
+  // The straightforward way to calculate the inverse of a Montgomery-encoded
+  // value where the result is Montgomery-encoded is:
+  //
+  //    |BN_from_montgomery| + invert + |BN_to_montgomery|.
+  //
+  // This is equivalent, but more efficient, because |BN_from_montgomery|
+  // is more efficient (at least in theory) than |BN_to_montgomery|, since it
+  // doesn't have to do the multiplication before the reduction.
+  //
+  // Use Fermat's Little Theorem instead of |BN_mod_inverse_odd| since this
+  // inversion may be done as the final step of private key operations.
+  // Unfortunately, this is suboptimal for ECDSA verification.
+  if (!BN_from_montgomery(Z_1, &point->Z, group->mont, ctx) ||
+      !BN_from_montgomery(Z_1, Z_1, group->mont, ctx) ||
+      !bn_mod_inverse_prime(Z_1, Z_1, &group->field, ctx, group->mont)) {
+    goto err;
+  }
+
+  if (!BN_mod_mul_montgomery(Z_2, Z_1, Z_1, group->mont, ctx)) {
+    goto err;
+  }
+
+  // Instead of using |BN_from_montgomery| to convert the |x| coordinate
+  // and then calling |BN_from_montgomery| again to convert the |y|
+  // coordinate below, convert the common factor |Z_2| once now, saving one
+  // reduction.
+  if (!BN_from_montgomery(Z_2, Z_2, group->mont, ctx)) {
+    goto err;
+  }
+
+  if (x != NULL) {
+    if (!BN_mod_mul_montgomery(x, &point->X, Z_2, group->mont, ctx)) {
       goto err;
     }
-    if (y != NULL && !BN_from_montgomery(y, &point->Y, group->mont, ctx)) {
+  }
+
+  if (y != NULL) {
+    if (!BN_mod_mul_montgomery(Z_3, Z_2, Z_1, group->mont, ctx) ||
+        !BN_mod_mul_montgomery(y, &point->Y, Z_3, group->mont, ctx)) {
       goto err;
     }
-  } else {
-    // transform  (X, Y, Z)  into  (x, y) := (X/Z^2, Y/Z^3)
-
-    BIGNUM *Z_1 = BN_CTX_get(ctx);
-    BIGNUM *Z_2 = BN_CTX_get(ctx);
-    BIGNUM *Z_3 = BN_CTX_get(ctx);
-    if (Z_1 == NULL ||
-        Z_2 == NULL ||
-        Z_3 == NULL) {
-      goto err;
-    }
-
-    // The straightforward way to calculate the inverse of a Montgomery-encoded
-    // value where the result is Montgomery-encoded is:
-    //
-    //    |BN_from_montgomery| + invert + |BN_to_montgomery|.
-    //
-    // This is equivalent, but more efficient, because |BN_from_montgomery|
-    // is more efficient (at least in theory) than |BN_to_montgomery|, since it
-    // doesn't have to do the multiplication before the reduction.
-    //
-    // Use Fermat's Little Theorem instead of |BN_mod_inverse_odd| since this
-    // inversion may be done as the final step of private key operations.
-    // Unfortunately, this is suboptimal for ECDSA verification.
-    if (!BN_from_montgomery(Z_1, &point->Z, group->mont, ctx) ||
-        !BN_from_montgomery(Z_1, Z_1, group->mont, ctx) ||
-        !bn_mod_inverse_prime(Z_1, Z_1, &group->field, ctx, group->mont)) {
-      goto err;
-    }
-
-    if (!BN_mod_mul_montgomery(Z_2, Z_1, Z_1, group->mont, ctx)) {
-      goto err;
-    }
-
-    // Instead of using |BN_from_montgomery| to convert the |x| coordinate
-    // and then calling |BN_from_montgomery| again to convert the |y|
-    // coordinate below, convert the common factor |Z_2| once now, saving one
-    // reduction.
-    if (!BN_from_montgomery(Z_2, Z_2, group->mont, ctx)) {
-      goto err;
-    }
-
-    if (x != NULL) {
-      if (!BN_mod_mul_montgomery(x, &point->X, Z_2, group->mont, ctx)) {
-        goto err;
-      }
-    }
-
-    if (y != NULL) {
-      if (!BN_mod_mul_montgomery(Z_3, Z_2, Z_1, group->mont, ctx) ||
-          !BN_mod_mul_montgomery(y, &point->Y, Z_3, group->mont, ctx)) {
-        goto err;
-      }
-    }
   }
 
   ret = 1;