commit 45210dd4e21ace9d28cb76b3f83303fcdd2efcce
author Adam Langley <alangley@gmail.com> Sun Feb 04 11:23:22 2018 -0800
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Mon Feb 05 04:40:59 2018 +0000
tree 489659f17a2e041d33d9a6d9473f75908428771c
parent 2044181e01261b2790dc83a5cc6bbc40e61f4385

Tidy up |ec_GFp_simple_point2oct| and friend.

(Just happened to see these as I went by.)

Change-Id: I348b163e6986bfca8b58e56885c35a813efe28f6
Reviewed-on: https://boringssl-review.googlesource.com/25725
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/fipsmodule/ec/ec_test.cc

diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc
index 25c7783..8a215e9 100644
--- a/crypto/fipsmodule/ec/ec_test.cc
+++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -617,6 +617,8 @@
   size_t serialized_len =
       EC_POINT_point2oct(group.get(), point.get(),
                          POINT_CONVERSION_UNCOMPRESSED, nullptr, 0, nullptr);
+  ASSERT_NE(0u, serialized_len);
+
   std::vector<uint8_t> serialized(serialized_len);
   ASSERT_EQ(serialized_len,
             EC_POINT_point2oct(group.get(), point.get(),

crypto/fipsmodule/ec/oct.c

diff --git a/crypto/fipsmodule/ec/oct.c b/crypto/fipsmodule/ec/oct.c
index 96c138a..3a6b4dd 100644
--- a/crypto/fipsmodule/ec/oct.c
+++ b/crypto/fipsmodule/ec/oct.c
@@ -77,11 +77,9 @@
                                       const EC_POINT *point,
                                       point_conversion_form_t form,
                                       uint8_t *buf, size_t len, BN_CTX *ctx) {
-  size_t ret;
+  size_t ret = 0;
   BN_CTX *new_ctx = NULL;
   int used_ctx = 0;
-  BIGNUM *x, *y;
-  size_t field_len, i;
 
   if ((form != POINT_CONVERSION_COMPRESSED) &&
       (form != POINT_CONVERSION_UNCOMPRESSED)) {
@@ -94,14 +92,16 @@
     goto err;
   }
 
-  // ret := required output buffer length
-  field_len = BN_num_bytes(&group->field);
-  ret =
-      (form == POINT_CONVERSION_COMPRESSED) ? 1 + field_len : 1 + 2 * field_len;
+  const size_t field_len = BN_num_bytes(&group->field);
+  size_t output_len = 1 /* type byte */ + field_len;
+  if (form == POINT_CONVERSION_UNCOMPRESSED) {
+    // Uncompressed points have a second coordinate.
+    output_len += field_len;
+  }
 
   // if 'buf' is NULL, just return required length
   if (buf != NULL) {
-    if (len < ret) {
+    if (len < output_len) {
       OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL);
       goto err;
     }
@@ -115,8 +115,8 @@
 
     BN_CTX_start(ctx);
     used_ctx = 1;
-    x = BN_CTX_get(ctx);
-    y = BN_CTX_get(ctx);
+    BIGNUM *x = BN_CTX_get(ctx);
+    BIGNUM *y = BN_CTX_get(ctx);
     if (y == NULL) {
       goto err;
     }
@@ -131,7 +131,7 @@
     } else {
       buf[0] = form;
     }
-    i = 1;
+    size_t i = 1;
 
     if (!BN_bn2bin_padded(buf + i, field_len, x)) {
       OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
@@ -147,70 +147,66 @@
       i += field_len;
     }
 
-    if (i != ret) {
+    if (i != output_len) {
       OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
       goto err;
     }
   }
 
-  if (used_ctx) {
-    BN_CTX_end(ctx);
-  }
-  BN_CTX_free(new_ctx);
-  return ret;
+  ret = output_len;
 
 err:
   if (used_ctx) {
     BN_CTX_end(ctx);
   }
   BN_CTX_free(new_ctx);
-  return 0;
+  return ret;
 }
 
-
 static int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
                                    const uint8_t *buf, size_t len,
                                    BN_CTX *ctx) {
-  point_conversion_form_t form;
-  int y_bit;
   BN_CTX *new_ctx = NULL;
-  BIGNUM *x, *y;
-  size_t field_len, enc_len;
-  int ret = 0;
+  int ret = 0, used_ctx = 0;
 
   if (len == 0) {
     OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL);
-    return 0;
+    goto err;
   }
-  form = buf[0];
-  y_bit = form & 1;
+
+  point_conversion_form_t form = buf[0];
+  const int y_bit = form & 1;
   form = form & ~1U;
   if ((form != POINT_CONVERSION_COMPRESSED &&
        form != POINT_CONVERSION_UNCOMPRESSED) ||
       (form == POINT_CONVERSION_UNCOMPRESSED && y_bit)) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
-    return 0;
+    goto err;
   }
 
-  field_len = BN_num_bytes(&group->field);
-  enc_len =
-      (form == POINT_CONVERSION_COMPRESSED) ? 1 + field_len : 1 + 2 * field_len;
+  const size_t field_len = BN_num_bytes(&group->field);
+  size_t enc_len = 1 /* type byte */ + field_len;
+  if (form == POINT_CONVERSION_UNCOMPRESSED) {
+    // Uncompressed points have a second coordinate.
+    enc_len += field_len;
+  }
 
   if (len != enc_len) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
-    return 0;
+    goto err;
   }
 
   if (ctx == NULL) {
     ctx = new_ctx = BN_CTX_new();
     if (ctx == NULL) {
-      return 0;
+      goto err;
     }
   }
 
   BN_CTX_start(ctx);
-  x = BN_CTX_get(ctx);
-  y = BN_CTX_get(ctx);
+  used_ctx = 1;
+  BIGNUM *x = BN_CTX_get(ctx);
+  BIGNUM *y = BN_CTX_get(ctx);
   if (x == NULL || y == NULL) {
     goto err;
   }
@@ -244,7 +240,9 @@
   ret = 1;
 
 err:
-  BN_CTX_end(ctx);
+  if (used_ctx) {
+    BN_CTX_end(ctx);
+  }
   BN_CTX_free(new_ctx);
   return ret;
 }