Use UniquePtr in DSAImpl We still have CRYPTO_EX_DATA to destruct, but otherwise the rest is RAII. Change-Id: I45ebaab67ace887682be985e0839c5d5002b46a4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/90249 Commit-Queue: Rudolf Polzer <rpolzer@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Rudolf Polzer <rpolzer@google.com>
diff --git a/crypto/dsa/dsa.cc b/crypto/dsa/dsa.cc index cf48dfb..fa720fc 100644 --- a/crypto/dsa/dsa.cc +++ b/crypto/dsa/dsa.cc
@@ -16,6 +16,8 @@ #include <string.h> +#include <utility> + #include <openssl/bn.h> #include <openssl/dh.h> #include <openssl/digest.h> @@ -54,15 +56,7 @@ CRYPTO_new_ex_data(&ex_data); } -DSAImpl::~DSAImpl() { - CRYPTO_free_ex_data(&g_ex_data_class, &ex_data); - - BN_clear_free(p); - BN_clear_free(q); - BN_clear_free(g); - BN_clear_free(pub_key); - BN_clear_free(priv_key); -} +DSAImpl::~DSAImpl() { CRYPTO_free_ex_data(&g_ex_data_class, &ex_data); } void DSA_free(DSA *dsa) { if (dsa == nullptr) { @@ -79,43 +73,31 @@ } unsigned DSA_bits(const DSA *dsa) { - auto *impl = FromOpaque(dsa); - return BN_num_bits(impl->p); + return BN_num_bits(FromOpaque(dsa)->p.get()); } const BIGNUM *DSA_get0_pub_key(const DSA *dsa) { - auto *impl = FromOpaque(dsa); - return impl->pub_key; + return FromOpaque(dsa)->pub_key.get(); } const BIGNUM *DSA_get0_priv_key(const DSA *dsa) { - auto *impl = FromOpaque(dsa); - return impl->priv_key; + return FromOpaque(dsa)->priv_key.get(); } -const BIGNUM *DSA_get0_p(const DSA *dsa) { - auto *impl = FromOpaque(dsa); - return impl->p; -} +const BIGNUM *DSA_get0_p(const DSA *dsa) { return FromOpaque(dsa)->p.get(); } -const BIGNUM *DSA_get0_q(const DSA *dsa) { - auto *impl = FromOpaque(dsa); - return impl->q; -} +const BIGNUM *DSA_get0_q(const DSA *dsa) { return FromOpaque(dsa)->q.get(); } -const BIGNUM *DSA_get0_g(const DSA *dsa) { - auto *impl = FromOpaque(dsa); - return impl->g; -} +const BIGNUM *DSA_get0_g(const DSA *dsa) { return FromOpaque(dsa)->g.get(); } void DSA_get0_key(const DSA *dsa, const BIGNUM **out_pub_key, const BIGNUM **out_priv_key) { auto *impl = FromOpaque(dsa); if (out_pub_key != nullptr) { - *out_pub_key = impl->pub_key; + *out_pub_key = impl->pub_key.get(); } if (out_priv_key != nullptr) { - *out_priv_key = impl->priv_key; + *out_priv_key = impl->priv_key.get(); } } @@ -123,13 +105,13 @@ const BIGNUM **out_g) { auto *impl = FromOpaque(dsa); if (out_p != nullptr) { - *out_p = impl->p; + *out_p = impl->p.get(); } if (out_q != nullptr) { - *out_q = impl->q; + *out_q = impl->q.get(); } if (out_g != nullptr) { - *out_g = impl->g; + *out_g = impl->g.get(); } } @@ -141,12 +123,10 @@ } if (pub_key != nullptr) { - BN_free(impl->pub_key); - impl->pub_key = pub_key; + impl->pub_key.reset(pub_key); } if (priv_key != nullptr) { - BN_free(impl->priv_key); - impl->priv_key = priv_key; + impl->priv_key.reset(priv_key); } return 1; @@ -162,16 +142,13 @@ } if (p != nullptr) { - BN_free(impl->p); - impl->p = p; + impl->p.reset(p); } if (q != nullptr) { - BN_free(impl->q); - impl->q = q; + impl->q.reset(q); } if (g != nullptr) { - BN_free(impl->g); - impl->g = g; + impl->g.reset(g); } impl->method_mont_p = nullptr; @@ -404,12 +381,9 @@ return 0; } - BN_free(impl->p); - BN_free(impl->q); - BN_free(impl->g); - impl->p = BN_dup(p); - impl->q = BN_dup(q); - impl->g = BN_dup(g); + impl->p.reset(BN_dup(p)); + impl->q.reset(BN_dup(q)); + impl->g.reset(BN_dup(g)); if (impl->p == nullptr || impl->q == nullptr || impl->g == nullptr) { return 0; } @@ -429,9 +403,9 @@ if (ret == nullptr) { return nullptr; } - ret->p = BN_dup(impl->p); - ret->q = BN_dup(impl->q); - ret->g = BN_dup(impl->g); + ret->p.reset(BN_dup(impl->p.get())); + ret->q.reset(BN_dup(impl->q.get())); + ret->g.reset(BN_dup(impl->g.get())); if (ret->p == nullptr || ret->q == nullptr || ret->g == nullptr) { DSA_free(ret); return nullptr; @@ -447,55 +421,26 @@ } UniquePtr<BN_CTX> ctx(BN_CTX_new()); - if (ctx == nullptr) { + UniquePtr<BIGNUM> pub_key(BN_new()), priv_key(BN_new()); + if (ctx == nullptr || pub_key == nullptr || priv_key == nullptr) { return 0; } - int ok = 0; - BIGNUM *pub_key = nullptr; - BIGNUM *priv_key = impl->priv_key; - if (priv_key == nullptr) { - priv_key = BN_new(); - if (priv_key == nullptr) { - goto err; - } - } - - if (!BN_rand_range_ex(priv_key, 1, impl->q)) { - goto err; - } - - pub_key = impl->pub_key; - if (pub_key == nullptr) { - pub_key = BN_new(); - if (pub_key == nullptr) { - goto err; - } - } - - if (!BN_MONT_CTX_set_locked(&impl->method_mont_p, &impl->method_mont_lock, - impl->p, ctx.get()) || - !BN_mod_exp_mont_consttime(pub_key, impl->g, priv_key, impl->p, ctx.get(), + if (!BN_rand_range_ex(priv_key.get(), 1, impl->q.get()) || + !BN_MONT_CTX_set_locked(&impl->method_mont_p, &impl->method_mont_lock, + impl->p.get(), ctx.get()) || + !BN_mod_exp_mont_consttime(pub_key.get(), impl->g.get(), priv_key.get(), + impl->p.get(), ctx.get(), impl->method_mont_p.get())) { - goto err; + return 0; } // The public key is computed from the private key, but is public. - bn_declassify(pub_key); + bn_declassify(pub_key.get()); - impl->priv_key = priv_key; - impl->pub_key = pub_key; - ok = 1; - -err: - if (impl->pub_key == nullptr) { - BN_free(pub_key); - } - if (impl->priv_key == nullptr) { - BN_free(priv_key); - } - - return ok; + impl->priv_key = std::move(priv_key); + impl->pub_key = std::move(pub_key); + return 1; } DSA_SIG *DSA_SIG_new() { return NewZeroed<DSA_SIG>(); } @@ -588,11 +533,11 @@ goto err; } - if (digest_len > BN_num_bytes(impl->q)) { + if (digest_len > BN_num_bytes(impl->q.get())) { // If the digest length is greater than the size of |impl->q| use the // BN_num_bits(impl->q) leftmost bits of the digest, see FIPS 186-3, 4.2. // Note the above check that |impl->q| is a multiple of 8 bits. - digest_len = BN_num_bytes(impl->q); + digest_len = BN_num_bytes(impl->q.get()); } if (BN_bin2bn(digest, digest_len, &m) == nullptr) { @@ -603,7 +548,7 @@ // violates |bn_mod_add_consttime| and |mod_mul_consttime|'s preconditions. // (The underlying algorithms could accept looser bounds, but we reduce for // simplicity.) - size_t q_width = bn_minimal_width(impl->q); + size_t q_width = bn_minimal_width(impl->q.get()); if (!bn_resize_words(&m, q_width) || !bn_resize_words(&xr, q_width)) { goto err; } @@ -612,9 +557,9 @@ // Compute s = inv(k) (m + xr) mod q. Note |impl->method_mont_q| is // initialized by |dsa_sign_setup|. - if (!mod_mul_consttime(&xr, impl->priv_key, r, impl->method_mont_q.get(), - ctx) || - !bn_mod_add_consttime(s, &xr, &m, impl->q, ctx) || + if (!mod_mul_consttime(&xr, impl->priv_key.get(), r, + impl->method_mont_q.get(), ctx) || + !bn_mod_add_consttime(s, &xr, &m, impl->q.get(), ctx) || !mod_mul_consttime(s, s, kinv, impl->method_mont_q.get(), ctx)) { goto err; } @@ -692,20 +637,20 @@ } if (BN_is_zero(sig->r) || BN_is_negative(sig->r) || - BN_ucmp(sig->r, impl->q) >= 0) { + BN_ucmp(sig->r, impl->q.get()) >= 0) { ret = 1; goto err; } if (BN_is_zero(sig->s) || BN_is_negative(sig->s) || - BN_ucmp(sig->s, impl->q) >= 0) { + BN_ucmp(sig->s, impl->q.get()) >= 0) { ret = 1; goto err; } if (!BN_MONT_CTX_set_locked(&impl->method_mont_p, &impl->method_mont_lock, - impl->p, ctx) || + impl->p.get(), ctx) || !BN_MONT_CTX_set_locked(&impl->method_mont_q, &impl->method_mont_lock, - impl->q, ctx)) { + impl->q.get(), ctx)) { goto err; } @@ -713,12 +658,12 @@ // more efficiently computed as FromMont(s)^-1 = (s * R^-1)^-1 = s^-1 * R, // instead of ToMont(s^-1) = s^-1 * R. if (!BN_from_montgomery(&u2, sig->s, impl->method_mont_q.get(), ctx) || - !BN_mod_inverse(&u2, &u2, impl->q, ctx)) { + !BN_mod_inverse(&u2, &u2, impl->q.get(), ctx)) { goto err; } // save M in u1 - unsigned q_bits = BN_num_bits(impl->q); + unsigned q_bits = BN_num_bits(impl->q.get()); if (digest_len > (q_bits >> 3)) { // if the digest length is greater than the size of q use the // BN_num_bits(impl->q) leftmost bits of the digest, see @@ -743,13 +688,13 @@ goto err; } - if (!BN_mod_exp2_mont(&t1, impl->g, &u1, impl->pub_key, &u2, impl->p, ctx, - impl->method_mont_p.get())) { + if (!BN_mod_exp2_mont(&t1, impl->g.get(), &u1, impl->pub_key.get(), &u2, + impl->p.get(), ctx, impl->method_mont_p.get())) { goto err; } // let u1 = u1 mod q - if (!BN_mod(&u1, &t1, impl->q, ctx)) { + if (!BN_mod(&u1, &t1, impl->q.get(), ctx)) { goto err; } @@ -848,7 +793,7 @@ return 0; } - size_t order_len = BN_num_bytes(impl->q); + size_t order_len = BN_num_bytes(impl->q.get()); // Compute the maximum length of an |order_len| byte integer. Defensively // assume that the leading 0x00 is included. size_t integer_len = 1 /* tag */ + der_len_len(order_len + 1) + 1 + order_len; @@ -877,13 +822,13 @@ BIGNUM *kinv = BN_new(); if (r == nullptr || kinv == nullptr || // Get random k - !BN_rand_range_ex(&k, 1, dsa->q) || + !BN_rand_range_ex(&k, 1, dsa->q.get()) || !BN_MONT_CTX_set_locked(&dsa->method_mont_p, &dsa->method_mont_lock, - dsa->p, ctx) || + dsa->p.get(), ctx) || !BN_MONT_CTX_set_locked(&dsa->method_mont_q, &dsa->method_mont_lock, - dsa->q, ctx) || + dsa->q.get(), ctx) || // Compute r = (g^k mod p) mod q - !BN_mod_exp_mont_consttime(r, dsa->g, &k, dsa->p, ctx, + !BN_mod_exp_mont_consttime(r, dsa->g.get(), &k, dsa->p.get(), ctx, dsa->method_mont_p.get())) { OPENSSL_PUT_ERROR(DSA, ERR_R_BN_LIB); goto err; @@ -896,10 +841,10 @@ // revealed in the signature anyway (g^k (mod p) (mod q)), going from it to // |k| would require computing a discrete log. bn_declassify(r); - if (!BN_mod(r, r, dsa->q, ctx) || + if (!BN_mod(r, r, dsa->q.get(), ctx) || // Compute part of 's = inv(k) (m + xr) mod q' using Fermat's Little // Theorem. - !bn_mod_inverse_prime(kinv, &k, dsa->q, ctx, dsa->method_mont_q.get())) { + !bn_mod_inverse_prime(kinv, &k, dsa->q.get(), ctx, dsa->method_mont_q.get())) { OPENSSL_PUT_ERROR(DSA, ERR_R_BN_LIB); goto err; } @@ -950,7 +895,6 @@ DH *DSA_dup_DH(const DSA *dsa) { auto *impl = FromOpaque(dsa); - if (dsa == nullptr) { return nullptr; } @@ -961,15 +905,15 @@ return nullptr; } if (impl->q != nullptr) { - dh->priv_length = BN_num_bits(impl->q); - if (!copy_bn(&dh->q, impl->q)) { + dh->priv_length = BN_num_bits(impl->q.get()); + if (!copy_bn(&dh->q, impl->q.get())) { return nullptr; } } - if (!copy_bn(&dh->p, impl->p) || // - !copy_bn(&dh->g, impl->g) || // - !copy_bn(&dh->pub_key, impl->pub_key) || // - !copy_bn(&dh->priv_key, impl->priv_key)) { + if (!copy_bn(&dh->p, impl->p.get()) || // + !copy_bn(&dh->g, impl->g.get()) || // + !copy_bn(&dh->pub_key, impl->pub_key.get()) || // + !copy_bn(&dh->priv_key, impl->priv_key.get())) { return nullptr; }
diff --git a/crypto/dsa/dsa_asn1.cc b/crypto/dsa/dsa_asn1.cc index 916289f..a133868 100644 --- a/crypto/dsa/dsa_asn1.cc +++ b/crypto/dsa/dsa_asn1.cc
@@ -43,19 +43,20 @@ // caller. However, we check bounds on all values to avoid DoS vectors even // when domain parameters are invalid. In particular, signing will infinite // loop if |g| is zero. - if (BN_is_negative(dsa->p) || BN_is_negative(dsa->q) || BN_is_zero(dsa->p) || - BN_is_zero(dsa->q) || !BN_is_odd(dsa->p) || !BN_is_odd(dsa->q) || + if (BN_is_negative(dsa->p.get()) || BN_is_negative(dsa->q.get()) || + BN_is_zero(dsa->p.get()) || BN_is_zero(dsa->q.get()) || + !BN_is_odd(dsa->p.get()) || !BN_is_odd(dsa->q.get()) || // |q| must be a prime divisor of |p - 1|, which implies |q < p|. - BN_cmp(dsa->q, dsa->p) >= 0 || + BN_cmp(dsa->q.get(), dsa->p.get()) >= 0 || // |g| is in the multiplicative group of |p|. - BN_is_negative(dsa->g) || BN_is_zero(dsa->g) || - BN_cmp(dsa->g, dsa->p) >= 0) { + BN_is_negative(dsa->g.get()) || BN_is_zero(dsa->g.get()) || + BN_cmp(dsa->g.get(), dsa->p.get()) >= 0) { OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS); return 0; } // FIPS 186-4 allows only three different sizes for q. - unsigned q_bits = BN_num_bits(dsa->q); + unsigned q_bits = BN_num_bits(dsa->q.get()); if (q_bits != 160 && q_bits != 224 && q_bits != 256) { OPENSSL_PUT_ERROR(DSA, DSA_R_BAD_Q_VALUE); return 0; @@ -63,15 +64,15 @@ // Bound |dsa->p| to avoid a DoS vector. Note this limit is much larger than // the one in FIPS 186-4, which only allows L = 1024, 2048, and 3072. - if (BN_num_bits(dsa->p) > OPENSSL_DSA_MAX_MODULUS_BITS) { + if (BN_num_bits(dsa->p.get()) > OPENSSL_DSA_MAX_MODULUS_BITS) { OPENSSL_PUT_ERROR(DSA, DSA_R_MODULUS_TOO_LARGE); return 0; } if (dsa->pub_key != nullptr) { // The public key is also in the multiplicative group of |p|. - if (BN_is_negative(dsa->pub_key) || BN_is_zero(dsa->pub_key) || - BN_cmp(dsa->pub_key, dsa->p) >= 0) { + if (BN_is_negative(dsa->pub_key.get()) || BN_is_zero(dsa->pub_key.get()) || + BN_cmp(dsa->pub_key.get(), dsa->p.get()) >= 0) { OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS); return 0; } @@ -80,9 +81,10 @@ if (dsa->priv_key != nullptr) { // The private key is a non-zero element of the scalar field, determined by // |q|. - if (BN_is_negative(dsa->priv_key) || - constant_time_declassify_int(BN_is_zero(dsa->priv_key)) || - constant_time_declassify_int(BN_cmp(dsa->priv_key, dsa->q) >= 0)) { + if (BN_is_negative(dsa->priv_key.get()) || + constant_time_declassify_int(BN_is_zero(dsa->priv_key.get())) || + constant_time_declassify_int( + BN_cmp(dsa->priv_key.get(), dsa->q.get()) >= 0)) { OPENSSL_PUT_ERROR(DSA, DSA_R_INVALID_PARAMETERS); return 0; } @@ -91,13 +93,13 @@ return 1; } -static int parse_integer(CBS *cbs, BIGNUM **out) { +static int parse_integer(CBS *cbs, UniquePtr<BIGNUM> *out) { assert(*out == nullptr); - *out = BN_new(); + out->reset(BN_new()); if (*out == nullptr) { return 0; } - return BN_parse_asn1_unsigned(cbs, *out); + return BN_parse_asn1_unsigned(cbs, out->get()); } static int marshal_integer(CBB *cbb, BIGNUM *bn) { @@ -115,14 +117,17 @@ return nullptr; } CBS child; + UniquePtr<BIGNUM> r, s; if (!CBS_get_asn1(cbs, &child, CBS_ASN1_SEQUENCE) || - !parse_integer(&child, &ret->r) || - !parse_integer(&child, &ret->s) || + !parse_integer(&child, &r) || + !parse_integer(&child, &s) || CBS_len(&child) != 0) { OPENSSL_PUT_ERROR(DSA, DSA_R_DECODE_ERROR); DSA_SIG_free(ret); return nullptr; } + ret->r = r.release(); + ret->s = s.release(); return ret; } @@ -164,9 +169,10 @@ CBB child; if (!CBB_add_asn1(cbb, &child, CBS_ASN1_SEQUENCE) || - !marshal_integer(&child, impl->pub_key) || - !marshal_integer(&child, impl->p) || !marshal_integer(&child, impl->q) || - !marshal_integer(&child, impl->g) || !CBB_flush(cbb)) { + !marshal_integer(&child, impl->pub_key.get()) || + !marshal_integer(&child, impl->p.get()) || + !marshal_integer(&child, impl->q.get()) || + !marshal_integer(&child, impl->g.get()) || !CBB_flush(cbb)) { OPENSSL_PUT_ERROR(DSA, DSA_R_ENCODE_ERROR); return 0; } @@ -198,8 +204,9 @@ CBB child; if (!CBB_add_asn1(cbb, &child, CBS_ASN1_SEQUENCE) || - !marshal_integer(&child, impl->p) || !marshal_integer(&child, impl->q) || - !marshal_integer(&child, impl->g) || !CBB_flush(cbb)) { + !marshal_integer(&child, impl->p.get()) || + !marshal_integer(&child, impl->q.get()) || + !marshal_integer(&child, impl->g.get()) || !CBB_flush(cbb)) { OPENSSL_PUT_ERROR(DSA, DSA_R_ENCODE_ERROR); return 0; } @@ -247,10 +254,11 @@ CBB child; if (!CBB_add_asn1(cbb, &child, CBS_ASN1_SEQUENCE) || !CBB_add_asn1_uint64(&child, 0 /* version */) || - !marshal_integer(&child, impl->p) || !marshal_integer(&child, impl->q) || - !marshal_integer(&child, impl->g) || - !marshal_integer(&child, impl->pub_key) || - !marshal_integer(&child, impl->priv_key) || !CBB_flush(cbb)) { + !marshal_integer(&child, impl->p.get()) || + !marshal_integer(&child, impl->q.get()) || + !marshal_integer(&child, impl->g.get()) || + !marshal_integer(&child, impl->pub_key.get()) || + !marshal_integer(&child, impl->priv_key.get()) || !CBB_flush(cbb)) { OPENSSL_PUT_ERROR(DSA, DSA_R_ENCODE_ERROR); return 0; }
diff --git a/crypto/dsa/internal.h b/crypto/dsa/internal.h index c00d05d..70f77e7 100644 --- a/crypto/dsa/internal.h +++ b/crypto/dsa/internal.h
@@ -29,12 +29,12 @@ public: DSAImpl(); - BIGNUM *p = nullptr; - BIGNUM *q = nullptr; - BIGNUM *g = nullptr; + UniquePtr<BIGNUM> p; + UniquePtr<BIGNUM> q; + UniquePtr<BIGNUM> g; - BIGNUM *pub_key = nullptr; - BIGNUM *priv_key = nullptr; + UniquePtr<BIGNUM> pub_key; + UniquePtr<BIGNUM> priv_key; // Normally used to cache montgomery values mutable Mutex method_mont_lock;
diff --git a/crypto/evp/p_dsa.cc b/crypto/evp/p_dsa.cc index 52d3159..d664710 100644 --- a/crypto/evp/p_dsa.cc +++ b/crypto/evp/p_dsa.cc
@@ -45,12 +45,12 @@ return evp_decode_error; } - dsa->pub_key = BN_new(); + dsa->pub_key.reset(BN_new()); if (dsa->pub_key == nullptr) { return evp_decode_error; } - if (!BN_parse_asn1_unsigned(key, dsa->pub_key) || CBS_len(key) != 0) { + if (!BN_parse_asn1_unsigned(key, dsa->pub_key.get()) || CBS_len(key) != 0) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return evp_decode_error; } @@ -73,7 +73,7 @@ (has_params && !DSA_marshal_parameters(&algorithm, dsa)) || !CBB_add_asn1(&spki, &key_bitstring, CBS_ASN1_BITSTRING) || !CBB_add_u8(&key_bitstring, 0 /* padding */) || - !BN_marshal_asn1(&key_bitstring, dsa->pub_key) || !CBB_flush(out)) { + !BN_marshal_asn1(&key_bitstring, dsa->pub_key.get()) || !CBB_flush(out)) { OPENSSL_PUT_ERROR(EVP, EVP_R_ENCODE_ERROR); return 0; } @@ -93,11 +93,11 @@ return evp_decode_error; } - dsa->priv_key = BN_new(); + dsa->priv_key.reset(BN_new()); if (dsa->priv_key == nullptr) { return evp_decode_error; } - if (!BN_parse_asn1_unsigned(key, dsa->priv_key) || CBS_len(key) != 0) { + if (!BN_parse_asn1_unsigned(key, dsa->priv_key.get()) || CBS_len(key) != 0) { OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR); return evp_decode_error; } @@ -112,10 +112,11 @@ // Calculate the public key. UniquePtr<BN_CTX> ctx(BN_CTX_new()); - dsa->pub_key = BN_new(); + dsa->pub_key.reset(BN_new()); if (ctx == nullptr || dsa->pub_key == nullptr || - !BN_mod_exp_mont_consttime(dsa->pub_key, dsa->g, dsa->priv_key, dsa->p, - ctx.get(), nullptr)) { + !BN_mod_exp_mont_consttime(dsa->pub_key.get(), dsa->g.get(), + dsa->priv_key.get(), dsa->p.get(), ctx.get(), + nullptr)) { return evp_decode_error; } @@ -139,7 +140,7 @@ dsa_asn1_meth.oid_len) || !DSA_marshal_parameters(&algorithm, dsa) || !CBB_add_asn1(&pkcs8, &private_key, CBS_ASN1_OCTETSTRING) || - !BN_marshal_asn1(&private_key, dsa->priv_key) || !CBB_flush(out)) { + !BN_marshal_asn1(&private_key, dsa->priv_key.get()) || !CBB_flush(out)) { OPENSSL_PUT_ERROR(EVP, EVP_R_ENCODE_ERROR); return 0; } @@ -166,22 +167,21 @@ return 0; } -static int dup_bn_into(BIGNUM **out, BIGNUM *src) { - UniquePtr<BIGNUM> a(BN_dup(src)); - if (a == nullptr) { +static int dup_bn_into(UniquePtr<BIGNUM> *out, const BIGNUM *src) { + UniquePtr<BIGNUM> copy(BN_dup(src)); + if (copy == nullptr) { return 0; } - BN_free(*out); - *out = a.release(); + *out = std::move(copy); return 1; } static int dsa_copy_parameters(EvpPkey *to, const EvpPkey *from) { DSAImpl *to_dsa = reinterpret_cast<DSAImpl *>(to->pkey); const DSAImpl *from_dsa = reinterpret_cast<const DSAImpl *>(from->pkey); - if (!dup_bn_into(&to_dsa->p, from_dsa->p) || - !dup_bn_into(&to_dsa->q, from_dsa->q) || - !dup_bn_into(&to_dsa->g, from_dsa->g)) { + if (!dup_bn_into(&to_dsa->p, from_dsa->p.get()) || + !dup_bn_into(&to_dsa->q, from_dsa->q.get()) || + !dup_bn_into(&to_dsa->g, from_dsa->g.get())) { return 0; }