commit f2f3cfedb798a45a0605cc8c0a4e157f0119bb04
author David Benjamin <davidben@chromium.org> Mon Nov 10 17:28:53 2014 -0500
committer Adam Langley <agl@google.com> Mon Nov 10 23:04:52 2014 +0000
tree 59e7e1201a291124b197de2114fee3aff110628e
parent b145c8140b4335d523f79b52929e05e047a05c06

Use BN_bn2bin_padded rather than doing math to figure out leading zeros.

Saves doing it ad-hoc all the time.

Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8
Reviewed-on: https://boringssl-review.googlesource.com/2241
Reviewed-by: Adam Langley <agl@google.com>

crypto/ec/oct.c

diff --git a/crypto/ec/oct.c b/crypto/ec/oct.c
index 1c5ea14..487cc4e 100644
--- a/crypto/ec/oct.c
+++ b/crypto/ec/oct.c
@@ -81,7 +81,7 @@
   BN_CTX *new_ctx = NULL;
   int used_ctx = 0;
   BIGNUM *x, *y;
-  size_t field_len, i, skip;
+  size_t field_len, i;
 
   if ((form != POINT_CONVERSION_COMPRESSED) &&
       (form != POINT_CONVERSION_UNCOMPRESSED) &&
@@ -117,58 +117,45 @@
 
     if (ctx == NULL) {
       ctx = new_ctx = BN_CTX_new();
-      if (ctx == NULL)
+      if (ctx == NULL) {
         return 0;
+      }
     }
 
     BN_CTX_start(ctx);
     used_ctx = 1;
     x = BN_CTX_get(ctx);
     y = BN_CTX_get(ctx);
-    if (y == NULL)
+    if (y == NULL) {
       goto err;
+    }
 
-    if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx))
+    if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx)) {
       goto err;
+    }
 
     if ((form == POINT_CONVERSION_COMPRESSED ||
          form == POINT_CONVERSION_HYBRID) &&
-        BN_is_odd(y))
+        BN_is_odd(y)) {
       buf[0] = form + 1;
-    else
+    } else {
       buf[0] = form;
-
+    }
     i = 1;
 
-    skip = field_len - BN_num_bytes(x);
-    if (skip > field_len) {
+    if (!BN_bn2bin_padded(buf + i, field_len, x)) {
       OPENSSL_PUT_ERROR(EC, ec_GFp_simple_point2oct, ERR_R_INTERNAL_ERROR);
       goto err;
     }
-    while (skip > 0) {
-      buf[i++] = 0;
-      skip--;
-    }
-    skip = BN_bn2bin(x, buf + i);
-    i += skip;
-    if (i != 1 + field_len) {
-      OPENSSL_PUT_ERROR(EC, ec_GFp_simple_point2oct, ERR_R_INTERNAL_ERROR);
-      goto err;
-    }
+    i += field_len;
 
     if (form == POINT_CONVERSION_UNCOMPRESSED ||
         form == POINT_CONVERSION_HYBRID) {
-      skip = field_len - BN_num_bytes(y);
-      if (skip > field_len) {
+      if (!BN_bn2bin_padded(buf + i, field_len, y)) {
         OPENSSL_PUT_ERROR(EC, ec_GFp_simple_point2oct, ERR_R_INTERNAL_ERROR);
         goto err;
       }
-      while (skip > 0) {
-        buf[i++] = 0;
-        skip--;
-      }
-      skip = BN_bn2bin(y, buf + i);
-      i += skip;
+      i += field_len;
     }
 
     if (i != ret) {
@@ -177,17 +164,21 @@
     }
   }
 
-  if (used_ctx)
+  if (used_ctx) {
     BN_CTX_end(ctx);
-  if (new_ctx != NULL)
+  }
+  if (new_ctx != NULL) {
     BN_CTX_free(new_ctx);
+  }
   return ret;
 
 err:
-  if (used_ctx)
+  if (used_ctx) {
     BN_CTX_end(ctx);
-  if (new_ctx != NULL)
+  }
+  if (new_ctx != NULL) {
     BN_CTX_free(new_ctx);
+  }
   return 0;
 }
 

crypto/ecdh/ecdh.c

diff --git a/crypto/ecdh/ecdh.c b/crypto/ecdh/ecdh.c
index c64a0ad..0d05653 100644
--- a/crypto/ecdh/ecdh.c
+++ b/crypto/ecdh/ecdh.c
@@ -81,7 +81,7 @@
   const BIGNUM *priv;
   const EC_GROUP *group;
   int ret = -1;
-  size_t buflen, len;
+  size_t buflen;
   uint8_t *buf = NULL;
 
   if ((ctx = BN_CTX_new()) == NULL) {
@@ -116,20 +116,14 @@
   }
 
   buflen = (EC_GROUP_get_degree(group) + 7) / 8;
-  len = BN_num_bytes(x);
-  if (len > buflen) {
-    OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_INTERNAL_ERROR);
-    goto err;
-  }
   buf = OPENSSL_malloc(buflen);
   if (buf == NULL) {
     OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_MALLOC_FAILURE);
     goto err;
   }
 
-  memset(buf, 0, buflen - len);
-  if (len != (size_t)BN_bn2bin(x, buf + buflen - len)) {
-    OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_BN_LIB);
+  if (!BN_bn2bin_padded(buf, buflen, x)) {
+    OPENSSL_PUT_ERROR(ECDH, ECDH_compute_key, ERR_R_INTERNAL_ERROR);
     goto err;
   }
 

crypto/ecdsa/ecdsa_test.c

diff --git a/crypto/ecdsa/ecdsa_test.c b/crypto/ecdsa/ecdsa_test.c
index 523cbdf..127d76f 100644
--- a/crypto/ecdsa/ecdsa_test.c
+++ b/crypto/ecdsa/ecdsa_test.c
@@ -217,14 +217,15 @@
       goto builtin_err;
     }
     buf_len = 2 * bn_len;
-    raw_buf = OPENSSL_malloc(buf_len);
+    raw_buf = OPENSSL_malloc(2 * bn_len);
     if (raw_buf == NULL) {
       goto builtin_err;
     }
     /* Pad the bignums with leading zeroes. */
-    memset(raw_buf, 0, buf_len);
-    BN_bn2bin(ecdsa_sig->r, raw_buf + bn_len - r_len);
-    BN_bn2bin(ecdsa_sig->s, raw_buf + buf_len - s_len);
+    if (!BN_bn2bin_padded(raw_buf, bn_len, ecdsa_sig->r) ||
+        !BN_bn2bin_padded(raw_buf + bn_len, bn_len, ecdsa_sig->s)) {
+      goto builtin_err;
+    }
 
     /* Modify a single byte in the buffer. */
     offset = raw_buf[10] % buf_len;