Use scopers in crypto/fipsmodule/ec

I just picked a random directory here.

Bug: 394340436
Change-Id: I5f8409dd480e2e1ca00153fe57045f4cc3773a37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75947
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/fipsmodule/ec/ec.cc.inc

@@ -175,47 +175,44 @@ EC_GROUP *EC_GROUP_new_curve_GFp(const BIGNUM *p, const BIGNUM *a,
                                  const BIGNUM *b, BN_CTX *ctx) {
   if (BN_num_bytes(p) > EC_MAX_BYTES) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_FIELD);
-    return NULL;
+    return nullptr;
   }
 
-  BN_CTX *new_ctx = NULL;
-  if (ctx == NULL) {
-    ctx = new_ctx = BN_CTX_new();
-    if (ctx == NULL) {
-      return NULL;
+  bssl::UniquePtr<BN_CTX> new_ctx;
+  if (ctx == nullptr) {
+    new_ctx.reset(BN_CTX_new());
+    if (new_ctx == nullptr) {
+      return nullptr;
     }
+    ctx = new_ctx.get();
   }
 
   // Historically, |a| and |b| were not required to be fully reduced.
   // TODO(davidben): Can this be removed?
-  EC_GROUP *ret = NULL;
-  BN_CTX_start(ctx);
+  bssl::BN_CTXScope scope(ctx);
   BIGNUM *a_reduced = BN_CTX_get(ctx);
   BIGNUM *b_reduced = BN_CTX_get(ctx);
-  if (a_reduced == NULL || b_reduced == NULL ||
-      !BN_nnmod(a_reduced, a, p, ctx) || !BN_nnmod(b_reduced, b, p, ctx)) {
-    goto err;
+  if (a_reduced == nullptr || b_reduced == nullptr ||
+      !BN_nnmod(a_reduced, a, p, ctx) ||  //
+      !BN_nnmod(b_reduced, b, p, ctx)) {
+    return nullptr;
   }
 
-  ret = reinterpret_cast<EC_GROUP *>(OPENSSL_zalloc(sizeof(EC_GROUP)));
-  if (ret == NULL) {
-    return NULL;
+  bssl::UniquePtr<EC_GROUP> ret(
+      reinterpret_cast<EC_GROUP *>(OPENSSL_zalloc(sizeof(EC_GROUP))));
+  if (ret == nullptr) {
+    return nullptr;
   }
   ret->references = 1;
   ret->meth = EC_GFp_mont_method();
   bn_mont_ctx_init(&ret->field);
   bn_mont_ctx_init(&ret->order);
-  ret->generator.group = ret;
-  if (!ec_GFp_simple_group_set_curve(ret, p, a_reduced, b_reduced, ctx)) {
-    EC_GROUP_free(ret);
-    ret = NULL;
-    goto err;
+  ret->generator.group = ret.get();
+  if (!ec_GFp_simple_group_set_curve(ret.get(), p, a_reduced, b_reduced, ctx)) {
+    return nullptr;
   }
 
-err:
-  BN_CTX_end(ctx);
-  BN_CTX_free(new_ctx);
-  return ret;
+  return ret.release();
 }
 
 int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
@@ -246,20 +243,19 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
   // Note any curve which did not satisfy this must have been invalid or use a
   // tiny prime (less than 17). See the proof in |field_element_to_scalar| in
   // the ECDSA implementation.
-  int ret = 0;
-  BIGNUM *tmp = BN_new();
-  if (tmp == NULL || !BN_lshift1(tmp, order)) {
-    goto err;
+  bssl::UniquePtr<BIGNUM> tmp(BN_new());
+  if (tmp == nullptr || !BN_lshift1(tmp.get(), order)) {
+    return 0;
   }
-  if (BN_cmp(tmp, &group->field.N) <= 0) {
+  if (BN_cmp(tmp.get(), &group->field.N) <= 0) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
-    goto err;
+    return 0;
   }
 
   EC_AFFINE affine;
   if (!ec_jacobian_to_affine(group, &affine, &generator->raw) ||
       !BN_MONT_CTX_set(&group->order, order, NULL)) {
-    goto err;
+    return 0;
   }
 
   group->field_greater_than_order = BN_cmp(&group->field.N, order) > 0;
@@ -267,11 +263,7 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
   group->generator.raw.Y = affine.Y;
   // |raw.Z| was set to 1 by |EC_GROUP_new_curve_GFp|.
   group->has_order = 1;
-  ret = 1;
-
-err:
-  BN_free(tmp);
-  return ret;
+  return 1;
 }
 
 EC_GROUP *EC_GROUP_new_by_curve_name(int nid) {
@@ -689,14 +681,13 @@ int ec_point_mul_no_self_test(const EC_GROUP *group, EC_POINT *r,
     return 0;
   }
 
-  int ret = 0;
-  BN_CTX *new_ctx = NULL;
+  bssl::UniquePtr<BN_CTX> new_ctx;
   if (ctx == NULL) {
-    new_ctx = BN_CTX_new();
+    new_ctx.reset(BN_CTX_new());
     if (new_ctx == NULL) {
-      goto err;
+      return 0;
     }
-    ctx = new_ctx;
+    ctx = new_ctx.get();
   }
 
   // If both |g_scalar| and |p_scalar| are non-NULL,
@@ -714,7 +705,7 @@ int ec_point_mul_no_self_test(const EC_GROUP *group, EC_POINT *r,
     EC_SCALAR scalar;
     if (!arbitrary_bignum_to_scalar(group, &scalar, g_scalar, ctx) ||
         !ec_point_mul_scalar_base(group, &r->raw, &scalar)) {
-      goto err;
+      return 0;
     }
   }
 
@@ -723,7 +714,7 @@ int ec_point_mul_no_self_test(const EC_GROUP *group, EC_POINT *r,
     EC_JACOBIAN tmp;
     if (!arbitrary_bignum_to_scalar(group, &scalar, p_scalar, ctx) ||
         !ec_point_mul_scalar(group, &tmp, &p->raw, &scalar)) {
-      goto err;
+      return 0;
     }
     if (g_scalar == NULL) {
       OPENSSL_memcpy(&r->raw, &tmp, sizeof(EC_JACOBIAN));
@@ -732,11 +723,7 @@ int ec_point_mul_no_self_test(const EC_GROUP *group, EC_POINT *r,
     }
   }
 
-  ret = 1;
-
-err:
-  BN_CTX_free(new_ctx);
-  return ret;
+  return 1;
 }
 
 int EC_POINT_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar,

crypto/fipsmodule/ec/ec_key.cc.inc

@@ -316,40 +316,33 @@ end:
 
 int EC_KEY_set_public_key_affine_coordinates(EC_KEY *key, const BIGNUM *x,
                                              const BIGNUM *y) {
-  EC_POINT *point = NULL;
-  int ok = 0;
-
   if (!key || !key->group || !x || !y) {
     OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
     return 0;
   }
 
-  point = EC_POINT_new(key->group);
-  if (point == NULL ||
-      !EC_POINT_set_affine_coordinates_GFp(key->group, point, x, y, NULL) ||
-      !EC_KEY_set_public_key(key, point) || !EC_KEY_check_key(key)) {
-    goto err;
+  bssl::UniquePtr<EC_POINT> point(EC_POINT_new(key->group));
+  if (point == nullptr ||
+      !EC_POINT_set_affine_coordinates_GFp(key->group, point.get(), x, y,
+                                           nullptr) ||
+      !EC_KEY_set_public_key(key, point.get()) ||  //
+      !EC_KEY_check_key(key)) {
+    return 0;
   }
 
-  ok = 1;
-
-err:
-  EC_POINT_free(point);
-  return ok;
+  return 1;
 }
 
 int EC_KEY_oct2key(EC_KEY *key, const uint8_t *in, size_t len, BN_CTX *ctx) {
-  if (key->group == NULL) {
+  if (key->group == nullptr) {
     OPENSSL_PUT_ERROR(EC, EC_R_MISSING_PARAMETERS);
     return 0;
   }
 
-  EC_POINT *point = EC_POINT_new(key->group);
-  int ok = point != NULL &&
-           EC_POINT_oct2point(key->group, point, in, len, ctx) &&
-           EC_KEY_set_public_key(key, point);
-  EC_POINT_free(point);
-  return ok;
+  bssl::UniquePtr<EC_POINT> point(EC_POINT_new(key->group));
+  return point != nullptr &&
+         EC_POINT_oct2point(key->group, point.get(), in, len, ctx) &&
+         EC_KEY_set_public_key(key, point.get());
 }
 
 size_t EC_KEY_key2buf(const EC_KEY *key, point_conversion_form_t form,

crypto/fipsmodule/ec/oct.cc.inc

@@ -116,35 +116,30 @@ static int ec_GFp_simple_oct2point(const EC_GROUP *group, EC_POINT *point,
   // for primes which are not 3 (mod 4), namely P-224 and custom curves. P-224's
   // prime is particularly inconvenient for compressed coordinates. See
   // https://cr.yp.to/papers/sqroot.pdf
-  BN_CTX *new_ctx = NULL;
-  if (ctx == NULL) {
-    ctx = new_ctx = BN_CTX_new();
-    if (ctx == NULL) {
+  bssl::UniquePtr<BN_CTX> new_ctx;
+  if (ctx == nullptr) {
+    new_ctx.reset(BN_CTX_new());
+    if (new_ctx == nullptr) {
       return 0;
     }
+    ctx = new_ctx.get();
   }
 
-  int ret = 0;
-  BN_CTX_start(ctx);
+  bssl::BN_CTXScope scope(ctx);
   BIGNUM *x = BN_CTX_get(ctx);
-  if (x == NULL || !BN_bin2bn(buf + 1, field_len, x)) {
-    goto err;
+  if (x == nullptr || !BN_bin2bn(buf + 1, field_len, x)) {
+    return 0;
   }
   if (BN_ucmp(x, &group->field.N) >= 0) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_ENCODING);
-    goto err;
+    return 0;
   }
 
   if (!EC_POINT_set_compressed_coordinates_GFp(group, point, x, y_bit, ctx)) {
-    goto err;
+    return 0;
   }
 
-  ret = 1;
-
-err:
-  BN_CTX_end(ctx);
-  BN_CTX_free(new_ctx);
-  return ret;
+  return 1;
 }
 
 int EC_POINT_oct2point(const EC_GROUP *group, EC_POINT *point,
@@ -214,28 +209,27 @@ int EC_POINT_set_compressed_coordinates_GFp(const EC_GROUP *group,
     return 0;
   }
 
-  BN_CTX *new_ctx = NULL;
-  int ret = 0;
-
   ERR_clear_error();
 
-  if (ctx == NULL) {
-    ctx = new_ctx = BN_CTX_new();
-    if (ctx == NULL) {
+  bssl::UniquePtr<BN_CTX> new_ctx;
+  if (ctx == nullptr) {
+    new_ctx.reset(BN_CTX_new());
+    if (new_ctx == nullptr) {
       return 0;
     }
+    ctx = new_ctx.get();
   }
 
   y_bit = (y_bit != 0);
 
-  BN_CTX_start(ctx);
+  bssl::BN_CTXScope scope(ctx);
   BIGNUM *tmp1 = BN_CTX_get(ctx);
   BIGNUM *tmp2 = BN_CTX_get(ctx);
   BIGNUM *a = BN_CTX_get(ctx);
   BIGNUM *b = BN_CTX_get(ctx);
   BIGNUM *y = BN_CTX_get(ctx);
   if (y == NULL || !EC_GROUP_get_curve_GFp(group, NULL, a, b, ctx)) {
-    goto err;
+    return 0;
   }
 
   // Recover y.  We have a Weierstrass equation
@@ -245,7 +239,7 @@ int EC_POINT_set_compressed_coordinates_GFp(const EC_GROUP *group,
   // tmp1 := x^3
   if (!BN_mod_sqr(tmp2, x, field, ctx) ||
       !BN_mod_mul(tmp1, tmp2, x, field, ctx)) {
-    goto err;
+    return 0;
   }
 
   // tmp1 := tmp1 + a*x
@@ -253,18 +247,18 @@ int EC_POINT_set_compressed_coordinates_GFp(const EC_GROUP *group,
     if (!bn_mod_lshift1_consttime(tmp2, x, field, ctx) ||
         !bn_mod_add_consttime(tmp2, tmp2, x, field, ctx) ||
         !bn_mod_sub_consttime(tmp1, tmp1, tmp2, field, ctx)) {
-      goto err;
+      return 0;
     }
   } else {
     if (!BN_mod_mul(tmp2, a, x, field, ctx) ||
         !bn_mod_add_consttime(tmp1, tmp1, tmp2, field, ctx)) {
-      goto err;
+      return 0;
     }
   }
 
   // tmp1 := tmp1 + b
   if (!bn_mod_add_consttime(tmp1, tmp1, b, field, ctx)) {
-    goto err;
+    return 0;
   }
 
   if (!BN_mod_sqrt(y, tmp1, field, ctx)) {
@@ -276,31 +270,26 @@ int EC_POINT_set_compressed_coordinates_GFp(const EC_GROUP *group,
     } else {
       OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB);
     }
-    goto err;
+    return 0;
   }
 
   if (y_bit != BN_is_odd(y)) {
     if (BN_is_zero(y)) {
       OPENSSL_PUT_ERROR(EC, EC_R_INVALID_COMPRESSION_BIT);
-      goto err;
+      return 0;
     }
     if (!BN_usub(y, field, y)) {
-      goto err;
+      return 0;
     }
   }
   if (y_bit != BN_is_odd(y)) {
     OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR);
-    goto err;
+    return 0;
   }
 
   if (!EC_POINT_set_affine_coordinates_GFp(group, point, x, y, ctx)) {
-    goto err;
+    return 0;
   }
 
-  ret = 1;
-
-err:
-  BN_CTX_end(ctx);
-  BN_CTX_free(new_ctx);
-  return ret;
+  return 1;
 }

crypto/fipsmodule/ec/simple.cc.inc

@@ -45,11 +45,10 @@ int ec_GFp_simple_group_set_curve(EC_GROUP *group, const BIGNUM *p,
     return 0;
   }
 
-  int ret = 0;
-  BN_CTX_start(ctx);
+  bssl::BN_CTXScope scope(ctx);
   BIGNUM *tmp = BN_CTX_get(ctx);
-  if (tmp == NULL) {
-    goto err;
+  if (tmp == nullptr) {
+    return 0;
   }
 
   if (!BN_MONT_CTX_set(&group->field, p, ctx) ||
@@ -57,21 +56,17 @@ int ec_GFp_simple_group_set_curve(EC_GROUP *group, const BIGNUM *p,
       !ec_bignum_to_felem(group, &group->b, b) ||
       // Reuse Z from the generator to cache the value one.
       !ec_bignum_to_felem(group, &group->generator.raw.Z, BN_value_one())) {
-    goto err;
+    return 0;
   }
 
   // group->a_is_minus3
   if (!BN_copy(tmp, a) ||
       !BN_add_word(tmp, 3)) {
-    goto err;
+    return 0;
   }
   group->a_is_minus3 = (0 == BN_cmp(tmp, &group->field.N));
 
-  ret = 1;
-
-err:
-  BN_CTX_end(ctx);
-  return ret;
+  return 1;
 }
 
 int ec_GFp_simple_group_get_curve(const EC_GROUP *group, BIGNUM *p, BIGNUM *a,