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>
diff --git a/crypto/fipsmodule/ec/ec.cc.inc b/crypto/fipsmodule/ec/ec.cc.inc index f741840..f1409d9 100644 --- a/crypto/fipsmodule/ec/ec.cc.inc +++ b/crypto/fipsmodule/ec/ec.cc.inc
@@ -175,47 +175,44 @@ 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 @@ // 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ } } - 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,
diff --git a/crypto/fipsmodule/ec/ec_key.cc.inc b/crypto/fipsmodule/ec/ec_key.cc.inc index 8c402f4..01fbe93 100644 --- a/crypto/fipsmodule/ec/ec_key.cc.inc +++ b/crypto/fipsmodule/ec/ec_key.cc.inc
@@ -316,40 +316,33 @@ 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,
diff --git a/crypto/fipsmodule/ec/oct.cc.inc b/crypto/fipsmodule/ec/oct.cc.inc index 15c3a2f..69323dc 100644 --- a/crypto/fipsmodule/ec/oct.cc.inc +++ b/crypto/fipsmodule/ec/oct.cc.inc
@@ -116,35 +116,30 @@ // 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 @@ 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 @@ // 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 @@ 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 @@ } 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; }
diff --git a/crypto/fipsmodule/ec/simple.cc.inc b/crypto/fipsmodule/ec/simple.cc.inc index f053ea1..59a130b 100644 --- a/crypto/fipsmodule/ec/simple.cc.inc +++ b/crypto/fipsmodule/ec/simple.cc.inc
@@ -45,11 +45,10 @@ 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 @@ !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,