upstream: remove most uses of BN_CTX

We weren't following the rules re BN_CTX_start/BN_CTX_end and the places
we were using it didn't benefit from its use anyway. ok dtucker@

OpenBSD-Commit-ID: ea9ba6c0d2e6f6adfe00b309a8f41842fe12fc7a
This commit is contained in:
djm@openbsd.org 2019-11-15 06:00:20 +00:00 committed by Damien Miller
parent 39b87104cd
commit fd1a96490c
4 changed files with 47 additions and 84 deletions

View File

@ -1,4 +1,4 @@
/* $OpenBSD: moduli.c,v 1.36 2019/10/04 03:26:58 dtucker Exp $ */ /* $OpenBSD: moduli.c,v 1.37 2019/11/15 06:00:20 djm Exp $ */
/* /*
* Copyright 1994 Phil Karn <karn@qualcomm.com> * Copyright 1994 Phil Karn <karn@qualcomm.com>
* Copyright 1996-1998, 2003 William Allen Simpson <wsimpson@greendragon.com> * Copyright 1996-1998, 2003 William Allen Simpson <wsimpson@greendragon.com>
@ -578,7 +578,6 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted,
char *checkpoint_file, unsigned long start_lineno, unsigned long num_lines) char *checkpoint_file, unsigned long start_lineno, unsigned long num_lines)
{ {
BIGNUM *q, *p, *a; BIGNUM *q, *p, *a;
BN_CTX *ctx;
char *cp, *lp; char *cp, *lp;
u_int32_t count_in = 0, count_out = 0, count_possible = 0; u_int32_t count_in = 0, count_out = 0, count_possible = 0;
u_int32_t generator_known, in_tests, in_tries, in_type, in_size; u_int32_t generator_known, in_tests, in_tries, in_type, in_size;
@ -602,8 +601,6 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted,
fatal("BN_new failed"); fatal("BN_new failed");
if ((q = BN_new()) == NULL) if ((q = BN_new()) == NULL)
fatal("BN_new failed"); fatal("BN_new failed");
if ((ctx = BN_CTX_new()) == NULL)
fatal("BN_CTX_new failed");
debug2("%.24s Final %u Miller-Rabin trials (%x generator)", debug2("%.24s Final %u Miller-Rabin trials (%x generator)",
ctime(&time_start), trials, generator_wanted); ctime(&time_start), trials, generator_wanted);
@ -753,7 +750,7 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted,
* that p is also prime. A single pass will weed out the * that p is also prime. A single pass will weed out the
* vast majority of composite q's. * vast majority of composite q's.
*/ */
is_prime = BN_is_prime_ex(q, 1, ctx, NULL); is_prime = BN_is_prime_ex(q, 1, NULL, NULL);
if (is_prime < 0) if (is_prime < 0)
fatal("BN_is_prime_ex failed"); fatal("BN_is_prime_ex failed");
if (is_prime == 0) { if (is_prime == 0) {
@ -769,7 +766,7 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted,
* will show up on the first Rabin-Miller iteration so it * will show up on the first Rabin-Miller iteration so it
* doesn't hurt to specify a high iteration count. * doesn't hurt to specify a high iteration count.
*/ */
is_prime = BN_is_prime_ex(p, trials, ctx, NULL); is_prime = BN_is_prime_ex(p, trials, NULL, NULL);
if (is_prime < 0) if (is_prime < 0)
fatal("BN_is_prime_ex failed"); fatal("BN_is_prime_ex failed");
if (is_prime == 0) { if (is_prime == 0) {
@ -779,7 +776,7 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted,
debug("%10u: p is almost certainly prime", count_in); debug("%10u: p is almost certainly prime", count_in);
/* recheck q more rigorously */ /* recheck q more rigorously */
is_prime = BN_is_prime_ex(q, trials - 1, ctx, NULL); is_prime = BN_is_prime_ex(q, trials - 1, NULL, NULL);
if (is_prime < 0) if (is_prime < 0)
fatal("BN_is_prime_ex failed"); fatal("BN_is_prime_ex failed");
if (is_prime == 0) { if (is_prime == 0) {
@ -802,7 +799,6 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted,
free(lp); free(lp);
BN_free(p); BN_free(p);
BN_free(q); BN_free(q);
BN_CTX_free(ctx);
if (checkpoint_file != NULL) if (checkpoint_file != NULL)
unlink(checkpoint_file); unlink(checkpoint_file);

View File

@ -282,15 +282,13 @@ pack_public_key_ecdsa(fido_cred_t *cred, struct sk_enroll_response *response)
BIGNUM *x = NULL, *y = NULL; BIGNUM *x = NULL, *y = NULL;
EC_POINT *q = NULL; EC_POINT *q = NULL;
EC_GROUP *g = NULL; EC_GROUP *g = NULL;
BN_CTX *bn_ctx = NULL;
int ret = -1; int ret = -1;
response->public_key = NULL; response->public_key = NULL;
response->public_key_len = 0; response->public_key_len = 0;
if ((bn_ctx = BN_CTX_new()) == NULL || if ((x = BN_new()) == NULL ||
(x = BN_CTX_get(bn_ctx)) == NULL || (y = BN_new()) == NULL ||
(y = BN_CTX_get(bn_ctx)) == NULL ||
(g = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)) == NULL || (g = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)) == NULL ||
(q = EC_POINT_new(g)) == NULL) { (q = EC_POINT_new(g)) == NULL) {
skdebug(__func__, "libcrypto setup failed"); skdebug(__func__, "libcrypto setup failed");
@ -311,12 +309,12 @@ pack_public_key_ecdsa(fido_cred_t *cred, struct sk_enroll_response *response)
skdebug(__func__, "BN_bin2bn failed"); skdebug(__func__, "BN_bin2bn failed");
goto out; goto out;
} }
if (EC_POINT_set_affine_coordinates_GFp(g, q, x, y, bn_ctx) != 1) { if (EC_POINT_set_affine_coordinates_GFp(g, q, x, y, NULL) != 1) {
skdebug(__func__, "EC_POINT_set_affine_coordinates_GFp failed"); skdebug(__func__, "EC_POINT_set_affine_coordinates_GFp failed");
goto out; goto out;
} }
response->public_key_len = EC_POINT_point2oct(g, q, response->public_key_len = EC_POINT_point2oct(g, q,
POINT_CONVERSION_UNCOMPRESSED, NULL, 0, bn_ctx); POINT_CONVERSION_UNCOMPRESSED, NULL, 0, NULL);
if (response->public_key_len == 0 || response->public_key_len > 2048) { if (response->public_key_len == 0 || response->public_key_len > 2048) {
skdebug(__func__, "bad pubkey length %zu", skdebug(__func__, "bad pubkey length %zu",
response->public_key_len); response->public_key_len);
@ -327,7 +325,7 @@ pack_public_key_ecdsa(fido_cred_t *cred, struct sk_enroll_response *response)
goto out; goto out;
} }
if (EC_POINT_point2oct(g, q, POINT_CONVERSION_UNCOMPRESSED, if (EC_POINT_point2oct(g, q, POINT_CONVERSION_UNCOMPRESSED,
response->public_key, response->public_key_len, bn_ctx) == 0) { response->public_key, response->public_key_len, NULL) == 0) {
skdebug(__func__, "EC_POINT_point2oct failed"); skdebug(__func__, "EC_POINT_point2oct failed");
goto out; goto out;
} }
@ -341,7 +339,8 @@ pack_public_key_ecdsa(fido_cred_t *cred, struct sk_enroll_response *response)
} }
EC_POINT_free(q); EC_POINT_free(q);
EC_GROUP_free(g); EC_GROUP_free(g);
BN_CTX_free(bn_ctx); BN_clear_free(x);
BN_clear_free(y);
return ret; return ret;
} }

View File

@ -1,4 +1,4 @@
/* $OpenBSD: sshbuf-getput-crypto.c,v 1.7 2019/01/21 09:54:11 djm Exp $ */ /* $OpenBSD: sshbuf-getput-crypto.c,v 1.8 2019/11/15 06:00:20 djm Exp $ */
/* /*
* Copyright (c) 2011 Damien Miller * Copyright (c) 2011 Damien Miller
* *
@ -154,23 +154,17 @@ int
sshbuf_put_ec(struct sshbuf *buf, const EC_POINT *v, const EC_GROUP *g) sshbuf_put_ec(struct sshbuf *buf, const EC_POINT *v, const EC_GROUP *g)
{ {
u_char d[SSHBUF_MAX_ECPOINT]; u_char d[SSHBUF_MAX_ECPOINT];
BN_CTX *bn_ctx;
size_t len; size_t len;
int ret; int ret;
if ((bn_ctx = BN_CTX_new()) == NULL)
return SSH_ERR_ALLOC_FAIL;
if ((len = EC_POINT_point2oct(g, v, POINT_CONVERSION_UNCOMPRESSED, if ((len = EC_POINT_point2oct(g, v, POINT_CONVERSION_UNCOMPRESSED,
NULL, 0, bn_ctx)) > SSHBUF_MAX_ECPOINT) { NULL, 0, NULL)) > SSHBUF_MAX_ECPOINT) {
BN_CTX_free(bn_ctx);
return SSH_ERR_INVALID_ARGUMENT; return SSH_ERR_INVALID_ARGUMENT;
} }
if (EC_POINT_point2oct(g, v, POINT_CONVERSION_UNCOMPRESSED, if (EC_POINT_point2oct(g, v, POINT_CONVERSION_UNCOMPRESSED,
d, len, bn_ctx) != len) { d, len, NULL) != len) {
BN_CTX_free(bn_ctx);
return SSH_ERR_INTERNAL_ERROR; /* Shouldn't happen */ return SSH_ERR_INTERNAL_ERROR; /* Shouldn't happen */
} }
BN_CTX_free(bn_ctx);
ret = sshbuf_put_string(buf, d, len); ret = sshbuf_put_string(buf, d, len);
explicit_bzero(d, len); explicit_bzero(d, len);
return ret; return ret;

View File

@ -1,4 +1,4 @@
/* $OpenBSD: sshkey.c,v 1.92 2019/11/13 22:00:21 markus Exp $ */ /* $OpenBSD: sshkey.c,v 1.93 2019/11/15 06:00:20 djm Exp $ */
/* /*
* Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved.
* Copyright (c) 2008 Alexander von Gernler. All rights reserved. * Copyright (c) 2008 Alexander von Gernler. All rights reserved.
@ -706,9 +706,6 @@ sshkey_equal_public(const struct sshkey *a, const struct sshkey *b)
const BIGNUM *rsa_e_b, *rsa_n_b; const BIGNUM *rsa_e_b, *rsa_n_b;
const BIGNUM *dsa_p_a, *dsa_q_a, *dsa_g_a, *dsa_pub_key_a; const BIGNUM *dsa_p_a, *dsa_q_a, *dsa_g_a, *dsa_pub_key_a;
const BIGNUM *dsa_p_b, *dsa_q_b, *dsa_g_b, *dsa_pub_key_b; const BIGNUM *dsa_p_b, *dsa_q_b, *dsa_g_b, *dsa_pub_key_b;
# if defined(OPENSSL_HAS_ECC)
BN_CTX *bnctx;
# endif /* OPENSSL_HAS_ECC */
#endif /* WITH_OPENSSL */ #endif /* WITH_OPENSSL */
if (a == NULL || b == NULL || if (a == NULL || b == NULL ||
@ -751,17 +748,12 @@ sshkey_equal_public(const struct sshkey *a, const struct sshkey *b)
EC_KEY_get0_public_key(a->ecdsa) == NULL || EC_KEY_get0_public_key(a->ecdsa) == NULL ||
EC_KEY_get0_public_key(b->ecdsa) == NULL) EC_KEY_get0_public_key(b->ecdsa) == NULL)
return 0; return 0;
if ((bnctx = BN_CTX_new()) == NULL)
return 0;
if (EC_GROUP_cmp(EC_KEY_get0_group(a->ecdsa), if (EC_GROUP_cmp(EC_KEY_get0_group(a->ecdsa),
EC_KEY_get0_group(b->ecdsa), bnctx) != 0 || EC_KEY_get0_group(b->ecdsa), NULL) != 0 ||
EC_POINT_cmp(EC_KEY_get0_group(a->ecdsa), EC_POINT_cmp(EC_KEY_get0_group(a->ecdsa),
EC_KEY_get0_public_key(a->ecdsa), EC_KEY_get0_public_key(a->ecdsa),
EC_KEY_get0_public_key(b->ecdsa), bnctx) != 0) { EC_KEY_get0_public_key(b->ecdsa), NULL) != 0)
BN_CTX_free(bnctx);
return 0; return 0;
}
BN_CTX_free(bnctx);
return 1; return 1;
# endif /* OPENSSL_HAS_ECC */ # endif /* OPENSSL_HAS_ECC */
#endif /* WITH_OPENSSL */ #endif /* WITH_OPENSSL */
@ -1659,7 +1651,6 @@ sshkey_ecdsa_key_to_nid(EC_KEY *k)
}; };
int nid; int nid;
u_int i; u_int i;
BN_CTX *bnctx;
const EC_GROUP *g = EC_KEY_get0_group(k); const EC_GROUP *g = EC_KEY_get0_group(k);
/* /*
@ -1672,18 +1663,13 @@ sshkey_ecdsa_key_to_nid(EC_KEY *k)
*/ */
if ((nid = EC_GROUP_get_curve_name(g)) > 0) if ((nid = EC_GROUP_get_curve_name(g)) > 0)
return nid; return nid;
if ((bnctx = BN_CTX_new()) == NULL)
return -1;
for (i = 0; nids[i] != -1; i++) { for (i = 0; nids[i] != -1; i++) {
if ((eg = EC_GROUP_new_by_curve_name(nids[i])) == NULL) { if ((eg = EC_GROUP_new_by_curve_name(nids[i])) == NULL)
BN_CTX_free(bnctx);
return -1; return -1;
} if (EC_GROUP_cmp(g, eg, NULL) == 0)
if (EC_GROUP_cmp(g, eg, bnctx) == 0)
break; break;
EC_GROUP_free(eg); EC_GROUP_free(eg);
} }
BN_CTX_free(bnctx);
if (nids[i] != -1) { if (nids[i] != -1) {
/* Use the group with the NID attached */ /* Use the group with the NID attached */
EC_GROUP_set_asn1_flag(eg, OPENSSL_EC_NAMED_CURVE); EC_GROUP_set_asn1_flag(eg, OPENSSL_EC_NAMED_CURVE);
@ -3788,9 +3774,8 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
int int
sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public) sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
{ {
BN_CTX *bnctx;
EC_POINT *nq = NULL; EC_POINT *nq = NULL;
BIGNUM *order, *x, *y, *tmp; BIGNUM *order = NULL, *x = NULL, *y = NULL, *tmp = NULL;
int ret = SSH_ERR_KEY_INVALID_EC_VALUE; int ret = SSH_ERR_KEY_INVALID_EC_VALUE;
/* /*
@ -3801,10 +3786,6 @@ sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
* EC_POINT_oct2point then the caller will need to explicitly check. * EC_POINT_oct2point then the caller will need to explicitly check.
*/ */
if ((bnctx = BN_CTX_new()) == NULL)
return SSH_ERR_ALLOC_FAIL;
BN_CTX_start(bnctx);
/* /*
* We shouldn't ever hit this case because bignum_get_ecpoint() * We shouldn't ever hit this case because bignum_get_ecpoint()
* refuses to load GF2m points. * refuses to load GF2m points.
@ -3817,18 +3798,18 @@ sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
if (EC_POINT_is_at_infinity(group, public)) if (EC_POINT_is_at_infinity(group, public))
goto out; goto out;
if ((x = BN_CTX_get(bnctx)) == NULL || if ((x = BN_new()) == NULL ||
(y = BN_CTX_get(bnctx)) == NULL || (y = BN_new()) == NULL ||
(order = BN_CTX_get(bnctx)) == NULL || (order = BN_new()) == NULL ||
(tmp = BN_CTX_get(bnctx)) == NULL) { (tmp = BN_new()) == NULL) {
ret = SSH_ERR_ALLOC_FAIL; ret = SSH_ERR_ALLOC_FAIL;
goto out; goto out;
} }
/* log2(x) > log2(order)/2, log2(y) > log2(order)/2 */ /* log2(x) > log2(order)/2, log2(y) > log2(order)/2 */
if (EC_GROUP_get_order(group, order, bnctx) != 1 || if (EC_GROUP_get_order(group, order, NULL) != 1 ||
EC_POINT_get_affine_coordinates_GFp(group, public, EC_POINT_get_affine_coordinates_GFp(group, public,
x, y, bnctx) != 1) { x, y, NULL) != 1) {
ret = SSH_ERR_LIBCRYPTO_ERROR; ret = SSH_ERR_LIBCRYPTO_ERROR;
goto out; goto out;
} }
@ -3841,7 +3822,7 @@ sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
ret = SSH_ERR_ALLOC_FAIL; ret = SSH_ERR_ALLOC_FAIL;
goto out; goto out;
} }
if (EC_POINT_mul(group, nq, NULL, public, order, bnctx) != 1) { if (EC_POINT_mul(group, nq, NULL, public, order, NULL) != 1) {
ret = SSH_ERR_LIBCRYPTO_ERROR; ret = SSH_ERR_LIBCRYPTO_ERROR;
goto out; goto out;
} }
@ -3857,7 +3838,10 @@ sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
goto out; goto out;
ret = 0; ret = 0;
out: out:
BN_CTX_free(bnctx); BN_clear_free(x);
BN_clear_free(y);
BN_clear_free(order);
BN_clear_free(tmp);
EC_POINT_free(nq); EC_POINT_free(nq);
return ret; return ret;
} }
@ -3865,22 +3849,16 @@ sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
int int
sshkey_ec_validate_private(const EC_KEY *key) sshkey_ec_validate_private(const EC_KEY *key)
{ {
BN_CTX *bnctx; BIGNUM *order = NULL, *tmp = NULL;
BIGNUM *order, *tmp;
int ret = SSH_ERR_KEY_INVALID_EC_VALUE; int ret = SSH_ERR_KEY_INVALID_EC_VALUE;
if ((bnctx = BN_CTX_new()) == NULL) if ((order = BN_new()) == NULL || (tmp = BN_new()) == NULL) {
return SSH_ERR_ALLOC_FAIL;
BN_CTX_start(bnctx);
if ((order = BN_CTX_get(bnctx)) == NULL ||
(tmp = BN_CTX_get(bnctx)) == NULL) {
ret = SSH_ERR_ALLOC_FAIL; ret = SSH_ERR_ALLOC_FAIL;
goto out; goto out;
} }
/* log2(private) > log2(order)/2 */ /* log2(private) > log2(order)/2 */
if (EC_GROUP_get_order(EC_KEY_get0_group(key), order, bnctx) != 1) { if (EC_GROUP_get_order(EC_KEY_get0_group(key), order, NULL) != 1) {
ret = SSH_ERR_LIBCRYPTO_ERROR; ret = SSH_ERR_LIBCRYPTO_ERROR;
goto out; goto out;
} }
@ -3897,47 +3875,43 @@ sshkey_ec_validate_private(const EC_KEY *key)
goto out; goto out;
ret = 0; ret = 0;
out: out:
BN_CTX_free(bnctx); BN_clear_free(order);
BN_clear_free(tmp);
return ret; return ret;
} }
void void
sshkey_dump_ec_point(const EC_GROUP *group, const EC_POINT *point) sshkey_dump_ec_point(const EC_GROUP *group, const EC_POINT *point)
{ {
BIGNUM *x, *y; BIGNUM *x = NULL, *y = NULL;
BN_CTX *bnctx;
if (point == NULL) { if (point == NULL) {
fputs("point=(NULL)\n", stderr); fputs("point=(NULL)\n", stderr);
return; return;
} }
if ((bnctx = BN_CTX_new()) == NULL) { if ((x = BN_new()) == NULL || (y = BN_new()) == NULL) {
fprintf(stderr, "%s: BN_CTX_new failed\n", __func__); fprintf(stderr, "%s: BN_new failed\n", __func__);
return; goto out;
}
BN_CTX_start(bnctx);
if ((x = BN_CTX_get(bnctx)) == NULL ||
(y = BN_CTX_get(bnctx)) == NULL) {
fprintf(stderr, "%s: BN_CTX_get failed\n", __func__);
return;
} }
if (EC_METHOD_get_field_type(EC_GROUP_method_of(group)) != if (EC_METHOD_get_field_type(EC_GROUP_method_of(group)) !=
NID_X9_62_prime_field) { NID_X9_62_prime_field) {
fprintf(stderr, "%s: group is not a prime field\n", __func__); fprintf(stderr, "%s: group is not a prime field\n", __func__);
return; goto out;
} }
if (EC_POINT_get_affine_coordinates_GFp(group, point, x, y, if (EC_POINT_get_affine_coordinates_GFp(group, point,
bnctx) != 1) { x, y, NULL) != 1) {
fprintf(stderr, "%s: EC_POINT_get_affine_coordinates_GFp\n", fprintf(stderr, "%s: EC_POINT_get_affine_coordinates_GFp\n",
__func__); __func__);
return; goto out;
} }
fputs("x=", stderr); fputs("x=", stderr);
BN_print_fp(stderr, x); BN_print_fp(stderr, x);
fputs("\ny=", stderr); fputs("\ny=", stderr);
BN_print_fp(stderr, y); BN_print_fp(stderr, y);
fputs("\n", stderr); fputs("\n", stderr);
BN_CTX_free(bnctx); out:
BN_clear_free(x);
BN_clear_free(y);
} }
void void