From dd1abd4462e4e4fa84b8f8de2ec70375f9b0e191 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 28 Sep 2016 23:39:18 +0200 Subject: [PATCH] If an engine comes up explicitely, it must also come down explicitely In apps/apps.c, one can set up an engine with setup_engine(). However, we freed the structural reference immediately, which means that for engines that don't already have a structural reference somewhere else (because it's a built in engine), we end up returning an invalid reference. Instead, the function release_engine() is added, and called at the end of the routines that call setup_engine(). Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/1643) --- apps/apps.c | 12 +++++++++--- apps/apps.h | 1 + apps/ca.c | 1 + apps/cms.c | 1 + apps/dgst.c | 1 + apps/dhparam.c | 4 +++- apps/dsa.c | 1 + apps/dsaparam.c | 4 +++- apps/ec.c | 1 + apps/ecparam.c | 6 ++++-- apps/enc.c | 4 +++- apps/gendsa.c | 4 +++- apps/genpkey.c | 2 +- apps/genrsa.c | 1 + apps/pkcs12.c | 1 + apps/pkcs7.c | 4 +++- apps/pkcs8.c | 1 + apps/pkey.c | 1 + apps/pkeyparam.c | 4 +++- apps/pkeyutl.c | 1 + apps/rand.c | 4 +++- apps/req.c | 1 + apps/rsa.c | 1 + apps/rsautl.c | 1 + apps/s_client.c | 1 + apps/s_server.c | 1 + apps/smime.c | 1 + apps/speed.c | 4 +++- apps/spkac.c | 1 + apps/srp.c | 4 +++- apps/verify.c | 4 +++- apps/x509.c | 1 + 32 files changed, 63 insertions(+), 16 deletions(-) diff --git a/apps/apps.c b/apps/apps.c index 68f2f2710a..05e1c2d009 100644 --- a/apps/apps.c +++ b/apps/apps.c @@ -1280,14 +1280,20 @@ ENGINE *setup_engine(const char *engine, int debug) } BIO_printf(bio_err, "engine \"%s\" set.\n", ENGINE_get_id(e)); - - /* Free our "structural" reference. */ - ENGINE_free(e); } return e; } #endif +void release_engine(ENGINE *e) +{ +#ifndef OPENSSL_NO_ENGINE + if (e != NULL) + /* Free our "structural" reference. */ + ENGINE_free(e); +#endif +} + static unsigned long index_serial_hash(const OPENSSL_CSTRING *a) { const char *n; diff --git a/apps/apps.h b/apps/apps.h index 6083780a78..da4cc36384 100644 --- a/apps/apps.h +++ b/apps/apps.h @@ -435,6 +435,7 @@ __owur int ctx_set_ctlog_list_file(SSL_CTX *ctx, const char *path); # else ENGINE *setup_engine(const char *engine, int debug); # endif +void release_engine(ENGINE *e); # ifndef OPENSSL_NO_OCSP OCSP_RESPONSE *process_responder(OCSP_REQUEST *req, const char *host, const char *path, diff --git a/apps/ca.c b/apps/ca.c index 39de2db738..b95f2efa0b 100644 --- a/apps/ca.c +++ b/apps/ca.c @@ -1231,6 +1231,7 @@ end_of_options: X509_CRL_free(crl); NCONF_free(conf); NCONF_free(extconf); + release_engine(e); return (ret); } diff --git a/apps/cms.c b/apps/cms.c index 306e159f7e..3db36fee02 100644 --- a/apps/cms.c +++ b/apps/cms.c @@ -1109,6 +1109,7 @@ int cms_main(int argc, char **argv) EVP_PKEY_free(key); CMS_ContentInfo_free(cms); CMS_ContentInfo_free(rcms); + release_engine(e); BIO_free(rctin); BIO_free(in); BIO_free(indata); diff --git a/apps/dgst.c b/apps/dgst.c index 2fb5a758d0..48ab5493c7 100644 --- a/apps/dgst.c +++ b/apps/dgst.c @@ -398,6 +398,7 @@ int dgst_main(int argc, char **argv) sk_OPENSSL_STRING_free(macopts); OPENSSL_free(sigbuf); BIO_free(bmd); + release_engine(e); return (ret); } diff --git a/apps/dhparam.c b/apps/dhparam.c index 302be2dd57..5fca25e17a 100644 --- a/apps/dhparam.c +++ b/apps/dhparam.c @@ -70,6 +70,7 @@ int dhparam_main(int argc, char **argv) BIO *in = NULL, *out = NULL; DH *dh = NULL; char *infile = NULL, *outfile = NULL, *prog, *inrand = NULL; + ENGINE *e = NULL; #ifndef OPENSSL_NO_DSA int dsaparam = 0; #endif @@ -104,7 +105,7 @@ int dhparam_main(int argc, char **argv) outfile = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; case OPT_CHECK: check = 1; @@ -356,6 +357,7 @@ int dhparam_main(int argc, char **argv) BIO_free(in); BIO_free_all(out); DH_free(dh); + release_engine(e); return (ret); } diff --git a/apps/dsa.c b/apps/dsa.c index f608d7d276..7f985128a0 100644 --- a/apps/dsa.c +++ b/apps/dsa.c @@ -249,6 +249,7 @@ int dsa_main(int argc, char **argv) end: BIO_free_all(out); DSA_free(dsa); + release_engine(e); OPENSSL_free(passin); OPENSSL_free(passout); return (ret); diff --git a/apps/dsaparam.c b/apps/dsaparam.c index 027597b666..2522f1fb4b 100644 --- a/apps/dsaparam.c +++ b/apps/dsaparam.c @@ -66,6 +66,7 @@ const OPTIONS dsaparam_options[] = { int dsaparam_main(int argc, char **argv) { + ENGINE *e = NULL; DSA *dsa = NULL; BIO *in = NULL, *out = NULL; BN_GENCB *cb = NULL; @@ -105,7 +106,7 @@ int dsaparam_main(int argc, char **argv) outfile = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; case OPT_TIMEBOMB: # ifdef GENCB_TEST @@ -285,6 +286,7 @@ int dsaparam_main(int argc, char **argv) BIO_free(in); BIO_free_all(out); DSA_free(dsa); + release_engine(e); return (ret); } diff --git a/apps/ec.c b/apps/ec.c index 368d226f4e..eb343d16a0 100644 --- a/apps/ec.c +++ b/apps/ec.c @@ -273,6 +273,7 @@ int ec_main(int argc, char **argv) BIO_free(in); BIO_free_all(out); EC_KEY_free(eckey); + release_engine(e); OPENSSL_free(passin); OPENSSL_free(passout); return (ret); diff --git a/apps/ecparam.c b/apps/ecparam.c index d1dcf1d75e..7d4cef1006 100644 --- a/apps/ecparam.c +++ b/apps/ecparam.c @@ -87,6 +87,7 @@ static OPT_PAIR encodings[] = { int ecparam_main(int argc, char **argv) { + ENGINE *e = NULL; BIGNUM *ec_gen = NULL, *ec_order = NULL, *ec_cofactor = NULL; BIGNUM *ec_p = NULL, *ec_a = NULL, *ec_b = NULL; BIO *in = NULL, *out = NULL; @@ -168,7 +169,7 @@ int ecparam_main(int argc, char **argv) need_rand = 1; break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; } } @@ -454,9 +455,10 @@ int ecparam_main(int argc, char **argv) BN_free(ec_order); BN_free(ec_cofactor); OPENSSL_free(buffer); + EC_GROUP_free(group); + release_engine(e); BIO_free(in); BIO_free_all(out); - EC_GROUP_free(group); return (ret); } diff --git a/apps/enc.c b/apps/enc.c index 33edd6c2d1..f31e2c8ba7 100644 --- a/apps/enc.c +++ b/apps/enc.c @@ -82,6 +82,7 @@ int enc_main(int argc, char **argv) { static char buf[128]; static const char magic[] = "Salted__"; + ENGINE *e = NULL; BIO *in = NULL, *out = NULL, *b64 = NULL, *benc = NULL, *rbio = NULL, *wbio = NULL; EVP_CIPHER_CTX *ctx = NULL; @@ -151,7 +152,7 @@ int enc_main(int argc, char **argv) passarg = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; case OPT_D: enc = 0; @@ -552,6 +553,7 @@ int enc_main(int argc, char **argv) #ifdef ZLIB BIO_free(bzl); #endif + release_engine(e); OPENSSL_free(pass); return (ret); } diff --git a/apps/gendsa.c b/apps/gendsa.c index 272cbb17a5..c9563a74f7 100644 --- a/apps/gendsa.c +++ b/apps/gendsa.c @@ -46,6 +46,7 @@ const OPTIONS gendsa_options[] = { int gendsa_main(int argc, char **argv) { + ENGINE *e = NULL; BIO *out = NULL, *in = NULL; DSA *dsa = NULL; const EVP_CIPHER *enc = NULL; @@ -74,7 +75,7 @@ int gendsa_main(int argc, char **argv) passoutarg = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; case OPT_RAND: inrand = opt_arg(); @@ -139,6 +140,7 @@ int gendsa_main(int argc, char **argv) BIO_free(in); BIO_free_all(out); DSA_free(dsa); + release_engine(e); OPENSSL_free(passout); return (ret); } diff --git a/apps/genpkey.c b/apps/genpkey.c index 93e15d80f1..8f556f371b 100644 --- a/apps/genpkey.c +++ b/apps/genpkey.c @@ -193,8 +193,8 @@ int genpkey_main(int argc, char **argv) EVP_PKEY_CTX_free(ctx); BIO_free_all(out); BIO_free(in); + release_engine(e); OPENSSL_free(pass); - return ret; } diff --git a/apps/genrsa.c b/apps/genrsa.c index bdcb20c7ac..033e692b21 100644 --- a/apps/genrsa.c +++ b/apps/genrsa.c @@ -166,6 +166,7 @@ int genrsa_main(int argc, char **argv) BN_GENCB_free(cb); RSA_free(rsa); BIO_free_all(out); + release_engine(eng); OPENSSL_free(passout); if (ret != 0) ERR_print_errors(bio_err); diff --git a/apps/pkcs12.c b/apps/pkcs12.c index 24150089fa..d8e56fbf90 100644 --- a/apps/pkcs12.c +++ b/apps/pkcs12.c @@ -574,6 +574,7 @@ int pkcs12_main(int argc, char **argv) PKCS12_free(p12); if (export_cert || inrand) app_RAND_write_file(NULL); + release_engine(e); BIO_free(in); BIO_free_all(out); sk_OPENSSL_STRING_free(canames); diff --git a/apps/pkcs7.c b/apps/pkcs7.c index bd4d51e977..22681085fc 100644 --- a/apps/pkcs7.c +++ b/apps/pkcs7.c @@ -44,6 +44,7 @@ const OPTIONS pkcs7_options[] = { int pkcs7_main(int argc, char **argv) { + ENGINE *e = NULL; PKCS7 *p7 = NULL; BIO *in = NULL, *out = NULL; int informat = FORMAT_PEM, outformat = FORMAT_PEM; @@ -90,7 +91,7 @@ int pkcs7_main(int argc, char **argv) print_certs = 1; break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; } } @@ -189,6 +190,7 @@ int pkcs7_main(int argc, char **argv) ret = 0; end: PKCS7_free(p7); + release_engine(e); BIO_free(in); BIO_free_all(out); return (ret); diff --git a/apps/pkcs8.c b/apps/pkcs8.c index f31008a259..e12c5d36cd 100644 --- a/apps/pkcs8.c +++ b/apps/pkcs8.c @@ -343,6 +343,7 @@ int pkcs8_main(int argc, char **argv) X509_SIG_free(p8); PKCS8_PRIV_KEY_INFO_free(p8inf); EVP_PKEY_free(pkey); + release_engine(e); BIO_free_all(out); BIO_free(in); OPENSSL_free(passin); diff --git a/apps/pkey.c b/apps/pkey.c index 426ef92808..48bfda8c34 100644 --- a/apps/pkey.c +++ b/apps/pkey.c @@ -180,6 +180,7 @@ int pkey_main(int argc, char **argv) end: EVP_PKEY_free(pkey); + release_engine(e); BIO_free_all(out); BIO_free(in); OPENSSL_free(passin); diff --git a/apps/pkeyparam.c b/apps/pkeyparam.c index a387db9e84..fe07ea7645 100644 --- a/apps/pkeyparam.c +++ b/apps/pkeyparam.c @@ -33,6 +33,7 @@ const OPTIONS pkeyparam_options[] = { int pkeyparam_main(int argc, char **argv) { + ENGINE *e = NULL; BIO *in = NULL, *out = NULL; EVP_PKEY *pkey = NULL; int text = 0, noout = 0, ret = 1; @@ -58,7 +59,7 @@ int pkeyparam_main(int argc, char **argv) outfile = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; case OPT_TEXT: text = 1; @@ -95,6 +96,7 @@ int pkeyparam_main(int argc, char **argv) end: EVP_PKEY_free(pkey); + release_engine(e); BIO_free_all(out); BIO_free(in); diff --git a/apps/pkeyutl.c b/apps/pkeyutl.c index b847eb90fd..3ba75adf05 100644 --- a/apps/pkeyutl.c +++ b/apps/pkeyutl.c @@ -323,6 +323,7 @@ int pkeyutl_main(int argc, char **argv) end: EVP_PKEY_CTX_free(ctx); + release_engine(e); BIO_free(in); BIO_free_all(out); OPENSSL_free(buf_in); diff --git a/apps/rand.c b/apps/rand.c index e6e9e3ead4..33dbf57d9c 100644 --- a/apps/rand.c +++ b/apps/rand.c @@ -39,6 +39,7 @@ const OPTIONS rand_options[] = { int rand_main(int argc, char **argv) { + ENGINE *e = NULL; BIO *out = NULL; char *inrand = NULL, *outfile = NULL, *prog; OPTION_CHOICE o; @@ -60,7 +61,7 @@ int rand_main(int argc, char **argv) outfile = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; case OPT_RAND: inrand = opt_arg(); @@ -125,6 +126,7 @@ int rand_main(int argc, char **argv) end: if (ret != 0) ERR_print_errors(bio_err); + release_engine(e); BIO_free_all(out); return (ret); } diff --git a/apps/req.c b/apps/req.c index 55ca9354de..c4fc931a8e 100644 --- a/apps/req.c +++ b/apps/req.c @@ -820,6 +820,7 @@ int req_main(int argc, char **argv) X509_REQ_free(req); X509_free(x509ss); ASN1_INTEGER_free(serial); + release_engine(e); if (passin != nofree_passin) OPENSSL_free(passin); if (passout != nofree_passout) diff --git a/apps/rsa.c b/apps/rsa.c index 807bc702b2..73579a61a6 100644 --- a/apps/rsa.c +++ b/apps/rsa.c @@ -294,6 +294,7 @@ int rsa_main(int argc, char **argv) } else ret = 0; end: + release_engine(e); BIO_free_all(out); RSA_free(rsa); OPENSSL_free(passin); diff --git a/apps/rsautl.c b/apps/rsautl.c index 268c86cfd9..e5c1524813 100644 --- a/apps/rsautl.c +++ b/apps/rsautl.c @@ -267,6 +267,7 @@ int rsautl_main(int argc, char **argv) BIO_write(out, rsa_out, rsa_outlen); end: RSA_free(rsa); + release_engine(e); BIO_free(in); BIO_free_all(out); OPENSSL_free(rsa_in); diff --git a/apps/s_client.c b/apps/s_client.c index c2a00f539d..55803e98dd 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -2506,6 +2506,7 @@ int s_client_main(int argc, char **argv) OPENSSL_clear_free(cbuf, BUFSIZZ); OPENSSL_clear_free(sbuf, BUFSIZZ); OPENSSL_clear_free(mbuf, BUFSIZZ); + release_engine(e); BIO_free(bio_c_out); bio_c_out = NULL; BIO_free(bio_c_msg); diff --git a/apps/s_server.c b/apps/s_server.c index eaecb7ed8d..d32b9dffd5 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -1963,6 +1963,7 @@ int s_server_main(int argc, char *argv[]) ssl_excert_free(exc); sk_OPENSSL_STRING_free(ssl_args); SSL_CONF_CTX_free(cctx); + release_engine(engine); BIO_free(bio_s_out); bio_s_out = NULL; BIO_free(bio_s_msg); diff --git a/apps/smime.c b/apps/smime.c index 7696d3b18f..dbafd0f62d 100644 --- a/apps/smime.c +++ b/apps/smime.c @@ -614,6 +614,7 @@ int smime_main(int argc, char **argv) X509_free(signer); EVP_PKEY_free(key); PKCS7_free(p7); + release_engine(e); BIO_free(in); BIO_free(indata); BIO_free_all(out); diff --git a/apps/speed.c b/apps/speed.c index bc46d45908..046882fbd6 100644 --- a/apps/speed.c +++ b/apps/speed.c @@ -1219,6 +1219,7 @@ static int run_benchmark(int async_jobs, int speed_main(int argc, char **argv) { + ENGINE *e = NULL; loopargs_t *loopargs = NULL; int async_init = 0; int loopargs_len = 0; @@ -1566,7 +1567,7 @@ int speed_main(int argc, char **argv) #endif /* Initialize the engine after the fork */ - (void)setup_engine(engine_id, 0); + e = setup_engine(engine_id, 0); /* No parameters; turn on everything. */ if ((argc == 0) && !doit[D_EVP]) { @@ -2819,6 +2820,7 @@ int speed_main(int argc, char **argv) ASYNC_cleanup_thread(); } OPENSSL_free(loopargs); + release_engine(e); return (ret); } diff --git a/apps/spkac.c b/apps/spkac.c index 70eeafc8ca..871b4f06f8 100644 --- a/apps/spkac.c +++ b/apps/spkac.c @@ -187,6 +187,7 @@ int spkac_main(int argc, char **argv) NETSCAPE_SPKI_free(spki); BIO_free_all(out); EVP_PKEY_free(pkey); + release_engine(e); OPENSSL_free(passin); return (ret); } diff --git a/apps/srp.c b/apps/srp.c index 643352e75d..b213c6010d 100644 --- a/apps/srp.c +++ b/apps/srp.c @@ -209,6 +209,7 @@ const OPTIONS srp_options[] = { int srp_main(int argc, char **argv) { + ENGINE *e = NULL; CA_DB *db = NULL; CONF *conf = NULL; int gNindex = -1, maxgN = -1, ret = 1, errors = 0, verbose = 0, i; @@ -269,7 +270,7 @@ int srp_main(int argc, char **argv) passoutarg = opt_arg(); break; case OPT_ENGINE: - (void)setup_engine(opt_arg(), 0); + e = setup_engine(opt_arg(), 0); break; } } @@ -602,6 +603,7 @@ int srp_main(int argc, char **argv) app_RAND_write_file(randfile); NCONF_free(conf); free_index(db); + release_engine(e); return (ret); } #endif diff --git a/apps/verify.c b/apps/verify.c index a8a6209a1b..bd8349a508 100644 --- a/apps/verify.c +++ b/apps/verify.c @@ -60,6 +60,7 @@ const OPTIONS verify_options[] = { int verify_main(int argc, char **argv) { + ENGINE *e = NULL; STACK_OF(X509) *untrusted = NULL, *trusted = NULL; STACK_OF(X509_CRL) *crls = NULL; X509_STORE *store = NULL; @@ -140,7 +141,7 @@ int verify_main(int argc, char **argv) crl_download = 1; break; case OPT_ENGINE: - if (setup_engine(opt_arg(), 0) == NULL) { + if ((e = setup_engine(opt_arg(), 0)) == NULL) { /* Failure message already displayed */ goto end; } @@ -191,6 +192,7 @@ int verify_main(int argc, char **argv) sk_X509_pop_free(untrusted, X509_free); sk_X509_pop_free(trusted, X509_free); sk_X509_CRL_pop_free(crls, X509_CRL_free); + release_engine(e); return (ret < 0 ? 2 : ret); } diff --git a/apps/x509.c b/apps/x509.c index 78f2c027a3..182cfb055d 100644 --- a/apps/x509.c +++ b/apps/x509.c @@ -893,6 +893,7 @@ int x509_main(int argc, char **argv) sk_ASN1_OBJECT_pop_free(trust, ASN1_OBJECT_free); sk_ASN1_OBJECT_pop_free(reject, ASN1_OBJECT_free); ASN1_OBJECT_free(objtmp); + release_engine(e); OPENSSL_free(passin); return (ret); } -- 2.25.1