From 925939afe0e7a409d98df57455b50b02955e99ea Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 23 Apr 2018 18:29:11 +0200 Subject: [PATCH] src: more automatic memory management in node_crypto.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prefer custom smart pointers fitted to the OpenSSL data structures over more manual memory management and lots of `goto`s. Backport-PR-URL: https://github.com/nodejs/node/pull/20609 PR-URL: https://github.com/nodejs/node/pull/20238 Reviewed-By: Daniel Bevenius Reviewed-By: Tobias Nießen Reviewed-By: Joyee Cheung Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- src/node_crypto.cc | 1422 +++++++++++++++++++------------------------- src/node_crypto.h | 122 ++-- src/tls_wrap.cc | 47 +- src/util.h | 32 +- 4 files changed, 725 insertions(+), 898 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index c2f51ad4c1ce73..c1d508c0ea6949 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -90,9 +90,21 @@ using v8::Value; struct StackOfX509Deleter { void operator()(STACK_OF(X509)* p) const { sk_X509_pop_free(p, X509_free); } }; - using StackOfX509 = std::unique_ptr; +struct StackOfXASN1Deleter { + void operator()(STACK_OF(ASN1_OBJECT)* p) const { + sk_ASN1_OBJECT_pop_free(p, ASN1_OBJECT_free); + } +}; +using StackOfASN1 = std::unique_ptr; + +// OPENSSL_free is a macro, so we need a wrapper function. +struct OpenSSLBufferDeleter { + void operator()(char* pointer) const { OPENSSL_free(pointer); } +}; +using OpenSSLBuffer = std::unique_ptr; + static const char* const root_certs[] = { #include "node_root_certs.h" // NOLINT(build/include_order) }; @@ -424,24 +436,24 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { } } - sc->ctx_ = SSL_CTX_new(method); - SSL_CTX_set_app_data(sc->ctx_, sc); + sc->ctx_.reset(SSL_CTX_new(method)); + SSL_CTX_set_app_data(sc->ctx_.get(), sc); // Disable SSLv2 in the case when method == TLS_method() and the // cipher list contains SSLv2 ciphers (not the default, should be rare.) // The bundled OpenSSL doesn't have SSLv2 support but the system OpenSSL may. // SSLv3 is disabled because it's susceptible to downgrade attacks (POODLE.) - SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv2); - SSL_CTX_set_options(sc->ctx_, SSL_OP_NO_SSLv3); + SSL_CTX_set_options(sc->ctx_.get(), SSL_OP_NO_SSLv2); + SSL_CTX_set_options(sc->ctx_.get(), SSL_OP_NO_SSLv3); // SSL session cache configuration - SSL_CTX_set_session_cache_mode(sc->ctx_, + SSL_CTX_set_session_cache_mode(sc->ctx_.get(), SSL_SESS_CACHE_SERVER | SSL_SESS_CACHE_NO_INTERNAL | SSL_SESS_CACHE_NO_AUTO_CLEAR); - SSL_CTX_set_min_proto_version(sc->ctx_, min_version); - SSL_CTX_set_max_proto_version(sc->ctx_, max_version); + SSL_CTX_set_min_proto_version(sc->ctx_.get(), min_version); + SSL_CTX_set_max_proto_version(sc->ctx_.get(), max_version); // OpenSSL 1.1.0 changed the ticket key size, but the OpenSSL 1.0.x size was // exposed in the public API. To retain compatibility, install a callback // which restores the old algorithm. @@ -450,7 +462,7 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { RAND_bytes(sc->ticket_key_aes_, sizeof(sc->ticket_key_aes_)) <= 0) { return env->ThrowError("Error generating ticket keys"); } - SSL_CTX_set_tlsext_ticket_key_cb(sc->ctx_, + SSL_CTX_set_tlsext_ticket_key_cb(sc->ctx_.get(), SecureContext::TicketCompatibilityCallback); } @@ -495,19 +507,19 @@ void SecureContext::SetKey(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_STRING(env, args[1], "Pass phrase"); } - BIO *bio = LoadBIO(env, args[0]); + BIOPointer bio(LoadBIO(env, args[0])); if (!bio) return; node::Utf8Value passphrase(env->isolate(), args[1]); - EVP_PKEY* key = PEM_read_bio_PrivateKey(bio, - nullptr, - PasswordCallback, - len == 1 ? nullptr : *passphrase); + EVPKeyPointer key( + PEM_read_bio_PrivateKey(bio.get(), + nullptr, + PasswordCallback, + len == 1 ? nullptr : *passphrase)); if (!key) { - BIO_free_all(bio); unsigned long err = ERR_get_error(); // NOLINT(runtime/int) if (!err) { return env->ThrowError("PEM_read_bio_PrivateKey"); @@ -515,9 +527,7 @@ void SecureContext::SetKey(const FunctionCallbackInfo& args) { return ThrowCryptoError(env, err); } - int rv = SSL_CTX_use_PrivateKey(sc->ctx_, key); - EVP_PKEY_free(key); - BIO_free_all(bio); + int rv = SSL_CTX_use_PrivateKey(sc->ctx_.get(), key.get()); if (!rv) { unsigned long err = ERR_get_error(); // NOLINT(runtime/int) @@ -530,24 +540,24 @@ void SecureContext::SetKey(const FunctionCallbackInfo& args) { int SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert, X509** issuer) { X509_STORE* store = SSL_CTX_get_cert_store(ctx); - X509_STORE_CTX* store_ctx = X509_STORE_CTX_new(); - int ret = store_ctx != nullptr && - X509_STORE_CTX_init(store_ctx, store, nullptr, nullptr) == 1 && - X509_STORE_CTX_get1_issuer(issuer, store_ctx, cert) == 1; - X509_STORE_CTX_free(store_ctx); - return ret; + DeleteFnPtr store_ctx( + X509_STORE_CTX_new()); + return store_ctx.get() != nullptr && + X509_STORE_CTX_init(store_ctx.get(), store, nullptr, nullptr) == 1 && + X509_STORE_CTX_get1_issuer(issuer, store_ctx.get(), cert) == 1; } int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, - X509* x, + X509Pointer&& x, STACK_OF(X509)* extra_certs, - X509** cert, - X509** issuer) { - CHECK_EQ(*issuer, nullptr); - CHECK_EQ(*cert, nullptr); + X509Pointer* cert, + X509Pointer* issuer_) { + CHECK(!*issuer_); + CHECK(!*cert); + X509* issuer = nullptr; - int ret = SSL_CTX_use_certificate(ctx, x); + int ret = SSL_CTX_use_certificate(ctx, x.get()); if (ret) { // If we could set up our certificate, now proceed to @@ -564,7 +574,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, if (!r) { ret = 0; - *issuer = nullptr; + issuer = nullptr; goto end; } // Note that we must not free r if it was successfully @@ -573,24 +583,24 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, // by SSL_CTX_use_certificate). // Find issuer - if (*issuer != nullptr || X509_check_issued(ca, x) != X509_V_OK) + if (issuer != nullptr || X509_check_issued(ca, x.get()) != X509_V_OK) continue; - *issuer = ca; + issuer = ca; } } // Try getting issuer from a cert store if (ret) { - if (*issuer == nullptr) { - ret = SSL_CTX_get_issuer(ctx, x, issuer); + if (issuer == nullptr) { + ret = SSL_CTX_get_issuer(ctx, x.get(), &issuer); ret = ret < 0 ? 0 : 1; // NOTE: get_cert_store doesn't increment reference count, // no need to free `store` } else { // Increment issuer reference count - *issuer = X509_dup(*issuer); - if (*issuer == nullptr) { + issuer = X509_dup(issuer); + if (issuer == nullptr) { ret = 0; goto end; } @@ -598,9 +608,11 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, } end: + issuer_->reset(issuer); + if (ret && x != nullptr) { - *cert = X509_dup(x); - if (*cert == nullptr) + cert->reset(X509_dup(x.get())); + if (!*cert) ret = 0; } return ret; @@ -613,21 +625,20 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, // // Taken from OpenSSL - edited for style. int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, - BIO* in, - X509** cert, - X509** issuer) { - X509* x = nullptr; - + BIOPointer&& in, + X509Pointer* cert, + X509Pointer* issuer) { // Just to ensure that `ERR_peek_last_error` below will return only errors // that we are interested in ERR_clear_error(); - x = PEM_read_bio_X509_AUX(in, nullptr, NoPasswordCallback, nullptr); + X509Pointer x( + PEM_read_bio_X509_AUX(in.get(), nullptr, NoPasswordCallback, nullptr)); - if (x == nullptr) { + if (!x) return 0; - } + // TODO(addaleax): Turn this into smart pointer as well. X509* extra = nullptr; int ret = 0; unsigned long err = 0; // NOLINT(runtime/int) @@ -636,7 +647,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, if (!extra_certs) goto done; - while ((extra = PEM_read_bio_X509(in, + while ((extra = PEM_read_bio_X509(in.get(), nullptr, NoPasswordCallback, nullptr))) { @@ -658,15 +669,17 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, goto done; } - ret = SSL_CTX_use_certificate_chain(ctx, x, extra_certs.get(), cert, issuer); + ret = SSL_CTX_use_certificate_chain(ctx, + std::move(x), + extra_certs.get(), + cert, + issuer); if (!ret) goto done; done: if (extra != nullptr) X509_free(extra); - if (x != nullptr) - X509_free(x); return ret; } @@ -682,27 +695,18 @@ void SecureContext::SetCert(const FunctionCallbackInfo& args) { return THROW_ERR_MISSING_ARGS(env, "Certificate argument is mandatory"); } - BIO* bio = LoadBIO(env, args[0]); + BIOPointer bio(LoadBIO(env, args[0])); if (!bio) return; - // Free previous certs - if (sc->issuer_ != nullptr) { - X509_free(sc->issuer_); - sc->issuer_ = nullptr; - } - if (sc->cert_ != nullptr) { - X509_free(sc->cert_); - sc->cert_ = nullptr; - } + sc->cert_.reset(); + sc->issuer_.reset(); - int rv = SSL_CTX_use_certificate_chain(sc->ctx_, - bio, + int rv = SSL_CTX_use_certificate_chain(sc->ctx_.get(), + std::move(bio), &sc->cert_, &sc->issuer_); - BIO_free_all(bio); - if (!rv) { unsigned long err = ERR_get_error(); // NOLINT(runtime/int) if (!err) { @@ -756,24 +760,21 @@ void SecureContext::AddCACert(const FunctionCallbackInfo& args) { return THROW_ERR_MISSING_ARGS(env, "CA certificate argument is mandatory"); } - BIO* bio = LoadBIO(env, args[0]); - if (!bio) { + BIOPointer bio(LoadBIO(env, args[0])); + if (!bio) return; - } - X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_); - while (X509* x509 = - PEM_read_bio_X509(bio, nullptr, NoPasswordCallback, nullptr)) { + X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_.get()); + while (X509* x509 = PEM_read_bio_X509( + bio.get(), nullptr, NoPasswordCallback, nullptr)) { if (cert_store == root_cert_store) { cert_store = NewRootCertStore(); - SSL_CTX_set_cert_store(sc->ctx_, cert_store); + SSL_CTX_set_cert_store(sc->ctx_.get(), cert_store); } X509_STORE_add_cert(cert_store, x509); - SSL_CTX_add_client_CA(sc->ctx_, x509); + SSL_CTX_add_client_CA(sc->ctx_.get(), x509); X509_free(x509); } - - BIO_free_all(bio); } @@ -789,30 +790,25 @@ void SecureContext::AddCRL(const FunctionCallbackInfo& args) { ClearErrorOnReturn clear_error_on_return; - BIO *bio = LoadBIO(env, args[0]); + BIOPointer bio(LoadBIO(env, args[0])); if (!bio) return; - X509_CRL* crl = - PEM_read_bio_X509_CRL(bio, nullptr, NoPasswordCallback, nullptr); + DeleteFnPtr crl( + PEM_read_bio_X509_CRL(bio.get(), nullptr, NoPasswordCallback, nullptr)); - if (crl == nullptr) { - BIO_free_all(bio); + if (!crl) return env->ThrowError("Failed to parse CRL"); - } - X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_); + X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_.get()); if (cert_store == root_cert_store) { cert_store = NewRootCertStore(); - SSL_CTX_set_cert_store(sc->ctx_, cert_store); + SSL_CTX_set_cert_store(sc->ctx_.get(), cert_store); } - X509_STORE_add_crl(cert_store, crl); + X509_STORE_add_crl(cert_store, crl.get()); X509_STORE_set_flags(cert_store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); - - BIO_free_all(bio); - X509_CRL_free(crl); } @@ -827,17 +823,15 @@ static unsigned long AddCertsFromFile( // NOLINT(runtime/int) ERR_clear_error(); MarkPopErrorOnReturn mark_pop_error_on_return; - BIO* bio = BIO_new_file(file, "r"); - if (!bio) { + BIOPointer bio(BIO_new_file(file, "r")); + if (!bio) return ERR_get_error(); - } while (X509* x509 = - PEM_read_bio_X509(bio, nullptr, NoPasswordCallback, nullptr)) { + PEM_read_bio_X509(bio.get(), nullptr, NoPasswordCallback, nullptr)) { X509_STORE_add_cert(store, x509); X509_free(x509); } - BIO_free_all(bio); unsigned long err = ERR_peek_error(); // NOLINT(runtime/int) // Ignore error if its EOF/no start line found. @@ -876,7 +870,7 @@ void SecureContext::AddRootCerts(const FunctionCallbackInfo& args) { // Increment reference count so global store is not deleted along with CTX. X509_STORE_up_ref(root_cert_store); - SSL_CTX_set_cert_store(sc->ctx_, root_cert_store); + SSL_CTX_set_cert_store(sc->ctx_.get(), root_cert_store); } @@ -893,7 +887,7 @@ void SecureContext::SetCiphers(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_STRING(env, args[0], "Ciphers"); const node::Utf8Value ciphers(args.GetIsolate(), args[0]); - SSL_CTX_set_cipher_list(sc->ctx_, *ciphers); + SSL_CTX_set_cipher_list(sc->ctx_.get(), *ciphers); } @@ -912,7 +906,7 @@ void SecureContext::SetECDHCurve(const FunctionCallbackInfo& args) { if (strcmp(*curve, "auto") == 0) return; - if (!SSL_CTX_set1_curves_list(sc->ctx_, *curve)) + if (!SSL_CTX_set1_curves_list(sc->ctx_.get(), *curve)) return env->ThrowError("Failed to set ECDH curve"); } @@ -928,19 +922,21 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo& args) { if (args.Length() != 1) return THROW_ERR_MISSING_ARGS(env, "DH argument is mandatory"); - // Invalid dhparam is silently discarded and DHE is no longer used. - BIO* bio = LoadBIO(env, args[0]); - if (!bio) - return; + DHPointer dh; + { + BIOPointer bio(LoadBIO(env, args[0])); + if (!bio) + return; - DH* dh = PEM_read_bio_DHparams(bio, nullptr, nullptr, nullptr); - BIO_free_all(bio); + dh.reset(PEM_read_bio_DHparams(bio.get(), nullptr, nullptr, nullptr)); + } - if (dh == nullptr) + // Invalid dhparam is silently discarded and DHE is no longer used. + if (!dh) return; const BIGNUM* p; - DH_get0_pqg(dh, &p, nullptr, nullptr); + DH_get0_pqg(dh.get(), &p, nullptr, nullptr); const int size = BN_num_bits(p); if (size < 1024) { return THROW_ERR_INVALID_ARG_VALUE( @@ -950,9 +946,8 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo& args) { env->isolate(), "DH parameter is less than 2048 bits")); } - SSL_CTX_set_options(sc->ctx_, SSL_OP_SINGLE_DH_USE); - int r = SSL_CTX_set_tmp_dh(sc->ctx_, dh); - DH_free(dh); + SSL_CTX_set_options(sc->ctx_.get(), SSL_OP_SINGLE_DH_USE); + int r = SSL_CTX_set_tmp_dh(sc->ctx_.get(), dh.get()); if (!r) return env->ThrowTypeError("Error setting temp DH parameter"); @@ -969,7 +964,7 @@ void SecureContext::SetOptions(const FunctionCallbackInfo& args) { } SSL_CTX_set_options( - sc->ctx_, + sc->ctx_.get(), static_cast(args[0]->IntegerValue())); // NOLINT(runtime/int) } @@ -992,23 +987,21 @@ void SecureContext::SetSessionIdContext( reinterpret_cast(*sessionIdContext); unsigned int sid_ctx_len = sessionIdContext.length(); - int r = SSL_CTX_set_session_id_context(sc->ctx_, sid_ctx, sid_ctx_len); + int r = SSL_CTX_set_session_id_context(sc->ctx_.get(), sid_ctx, sid_ctx_len); if (r == 1) return; - BIO* bio; BUF_MEM* mem; Local message; - bio = BIO_new(BIO_s_mem()); - if (bio == nullptr) { + BIOPointer bio(BIO_new(BIO_s_mem())); + if (!bio) { message = FIXED_ONE_BYTE_STRING(args.GetIsolate(), "SSL_CTX_set_session_id_context error"); } else { - ERR_print_errors(bio); - BIO_get_mem_ptr(bio, &mem); + ERR_print_errors(bio.get()); + BIO_get_mem_ptr(bio.get(), &mem); message = OneByteString(args.GetIsolate(), mem->data, mem->length); - BIO_free_all(bio); } args.GetIsolate()->ThrowException(Exception::TypeError(message)); @@ -1025,14 +1018,14 @@ void SecureContext::SetSessionTimeout(const FunctionCallbackInfo& args) { } int32_t sessionTimeout = args[0]->Int32Value(); - SSL_CTX_set_timeout(sc->ctx_, sessionTimeout); + SSL_CTX_set_timeout(sc->ctx_.get(), sessionTimeout); } void SecureContext::Close(const FunctionCallbackInfo& args) { SecureContext* sc; ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder()); - sc->FreeCTXMem(); + sc->Reset(); } @@ -1040,12 +1033,7 @@ void SecureContext::Close(const FunctionCallbackInfo& args) { void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - BIO* in = nullptr; - PKCS12* p12 = nullptr; - EVP_PKEY* pkey = nullptr; - X509* cert = nullptr; - STACK_OF(X509)* extra_certs = nullptr; - char* pass = nullptr; + std::vector pass; bool ret = false; SecureContext* sc; @@ -1056,64 +1044,61 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { return THROW_ERR_MISSING_ARGS(env, "PFX certificate argument is mandatory"); } - in = LoadBIO(env, args[0]); - if (in == nullptr) { + BIOPointer in(LoadBIO(env, args[0])); + if (!in) return env->ThrowError("Unable to load BIO"); - } if (args.Length() >= 2) { THROW_AND_RETURN_IF_NOT_BUFFER(env, args[1], "Pass phrase"); size_t passlen = Buffer::Length(args[1]); - pass = new char[passlen + 1]; - memcpy(pass, Buffer::Data(args[1]), passlen); + pass.resize(passlen + 1); + memcpy(pass.data(), Buffer::Data(args[1]), passlen); pass[passlen] = '\0'; } // Free previous certs - if (sc->issuer_ != nullptr) { - X509_free(sc->issuer_); - sc->issuer_ = nullptr; - } - if (sc->cert_ != nullptr) { - X509_free(sc->cert_); - sc->cert_ = nullptr; - } - - X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_); - - if (d2i_PKCS12_bio(in, &p12) && - PKCS12_parse(p12, pass, &pkey, &cert, &extra_certs) && - SSL_CTX_use_certificate_chain(sc->ctx_, - cert, - extra_certs, + sc->issuer_.reset(); + sc->cert_.reset(); + + X509_STORE* cert_store = SSL_CTX_get_cert_store(sc->ctx_.get()); + + DeleteFnPtr p12; + EVPKeyPointer pkey; + X509Pointer cert; + StackOfX509 extra_certs; + + PKCS12* p12_ptr = nullptr; + EVP_PKEY* pkey_ptr = nullptr; + X509* cert_ptr = nullptr; + STACK_OF(X509)* extra_certs_ptr = nullptr; + if (d2i_PKCS12_bio(in.get(), &p12_ptr) && + (p12.reset(p12_ptr), true) && // Move ownership to the smart pointer. + PKCS12_parse(p12.get(), pass.data(), + &pkey_ptr, + &cert_ptr, + &extra_certs_ptr) && + (pkey.reset(pkey_ptr), cert.reset(cert_ptr), + extra_certs.reset(extra_certs_ptr), true) && // Move ownership. + SSL_CTX_use_certificate_chain(sc->ctx_.get(), + std::move(cert), + extra_certs.get(), &sc->cert_, &sc->issuer_) && - SSL_CTX_use_PrivateKey(sc->ctx_, pkey)) { + SSL_CTX_use_PrivateKey(sc->ctx_.get(), pkey.get())) { // Add CA certs too - for (int i = 0; i < sk_X509_num(extra_certs); i++) { - X509* ca = sk_X509_value(extra_certs, i); + for (int i = 0; i < sk_X509_num(extra_certs.get()); i++) { + X509* ca = sk_X509_value(extra_certs.get(), i); if (cert_store == root_cert_store) { cert_store = NewRootCertStore(); - SSL_CTX_set_cert_store(sc->ctx_, cert_store); + SSL_CTX_set_cert_store(sc->ctx_.get(), cert_store); } X509_STORE_add_cert(cert_store, ca); - SSL_CTX_add_client_CA(sc->ctx_, ca); + SSL_CTX_add_client_CA(sc->ctx_.get(), ca); } ret = true; } - if (pkey != nullptr) - EVP_PKEY_free(pkey); - if (cert != nullptr) - X509_free(cert); - if (extra_certs != nullptr) - sk_X509_pop_free(extra_certs, X509_free); - - PKCS12_free(p12); - BIO_free_all(in); - delete[] pass; - if (!ret) { unsigned long err = ERR_get_error(); // NOLINT(runtime/int) const char* str = ERR_reason_error_string(err); @@ -1123,6 +1108,9 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { #ifndef OPENSSL_NO_ENGINE +// Helper for the smart pointer. +void ENGINE_free_fn(ENGINE* engine) { ENGINE_free(engine); } + void SecureContext::SetClientCertEngine( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -1146,18 +1134,16 @@ void SecureContext::SetClientCertEngine( const node::Utf8Value engine_id(env->isolate(), args[0]); char errmsg[1024]; - ENGINE* engine = LoadEngineById(*engine_id, &errmsg); + DeleteFnPtr engine( + LoadEngineById(*engine_id, &errmsg)); - if (engine == nullptr) { + if (!engine) return env->ThrowError(errmsg); - } - int r = SSL_CTX_set_client_cert_engine(sc->ctx_, engine); - // Free reference (SSL_CTX_set_client_cert_engine took it via ENGINE_init). - ENGINE_free(engine); - if (r == 0) { + // Note that this takes another reference to `engine`. + int r = SSL_CTX_set_client_cert_engine(sc->ctx_.get(), engine.get()); + if (r == 0) return ThrowCryptoError(env, ERR_get_error()); - } sc->client_cert_engine_provided_ = true; } #endif // !OPENSSL_NO_ENGINE @@ -1216,7 +1202,7 @@ void SecureContext::EnableTicketKeyCallback( SecureContext* wrap; ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); - SSL_CTX_set_tlsext_ticket_key_cb(wrap->ctx_, TicketKeyCallback); + SSL_CTX_set_tlsext_ticket_key_cb(wrap->ctx_.get(), TicketKeyCallback); } @@ -1345,9 +1331,9 @@ void SecureContext::GetCertificate(const FunctionCallbackInfo& args) { X509* cert; if (primary) - cert = wrap->cert_; + cert = wrap->cert_.get(); else - cert = wrap->issuer_; + cert = wrap->issuer_.get(); if (cert == nullptr) return args.GetReturnValue().SetNull(); @@ -1399,8 +1385,8 @@ template void SSLWrap::ConfigureSecureContext(SecureContext* sc) { #ifdef NODE__HAVE_TLSEXT_STATUS_CB // OCSP stapling - SSL_CTX_set_tlsext_status_cb(sc->ctx_, TLSExtStatusCallback); - SSL_CTX_set_tlsext_status_arg(sc->ctx_, nullptr); + SSL_CTX_set_tlsext_status_cb(sc->ctx_.get(), TLSExtStatusCallback); + SSL_CTX_set_tlsext_status_arg(sc->ctx_.get(), nullptr); #endif // NODE__HAVE_TLSEXT_STATUS_CB } @@ -1413,10 +1399,7 @@ SSL_SESSION* SSLWrap::GetSessionCallback(SSL* s, Base* w = static_cast(SSL_get_app_data(s)); *copy = 0; - SSL_SESSION* sess = w->next_sess_; - w->next_sess_ = nullptr; - - return sess; + return w->next_sess_.release(); } @@ -1554,29 +1537,29 @@ static Local X509ToObject(Environment* env, X509* cert) { Local context = env->context(); Local info = Object::New(env->isolate()); - BIO* bio = BIO_new(BIO_s_mem()); + BIOPointer bio(BIO_new(BIO_s_mem())); BUF_MEM* mem; - if (X509_NAME_print_ex(bio, + if (X509_NAME_print_ex(bio.get(), X509_get_subject_name(cert), 0, X509_NAME_FLAGS) > 0) { - BIO_get_mem_ptr(bio, &mem); + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, env->subject_string(), String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); } - USE(BIO_reset(bio)); + USE(BIO_reset(bio.get())); X509_NAME* issuer_name = X509_get_issuer_name(cert); - if (X509_NAME_print_ex(bio, issuer_name, 0, X509_NAME_FLAGS) > 0) { - BIO_get_mem_ptr(bio, &mem); + if (X509_NAME_print_ex(bio.get(), issuer_name, 0, X509_NAME_FLAGS) > 0) { + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, env->issuer_string(), String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); } - USE(BIO_reset(bio)); + USE(BIO_reset(bio.get())); int nids[] = { NID_subject_alt_name, NID_info_access }; Local keys[] = { env->subjectaltname_string(), @@ -1593,85 +1576,79 @@ static Local X509ToObject(Environment* env, X509* cert) { ext = X509_get_ext(cert, index); CHECK_NE(ext, nullptr); - if (!SafeX509ExtPrint(bio, ext)) { - rv = X509V3_EXT_print(bio, ext, 0, 0); + if (!SafeX509ExtPrint(bio.get(), ext)) { + rv = X509V3_EXT_print(bio.get(), ext, 0, 0); CHECK_EQ(rv, 1); } - BIO_get_mem_ptr(bio, &mem); + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, keys[i], String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); - USE(BIO_reset(bio)); + USE(BIO_reset(bio.get())); } - EVP_PKEY* pkey = X509_get_pubkey(cert); - RSA* rsa = nullptr; - if (pkey != nullptr) - rsa = EVP_PKEY_get1_RSA(pkey); + EVPKeyPointer pkey(X509_get_pubkey(cert)); + RSAPointer rsa; + if (pkey) + rsa.reset(EVP_PKEY_get1_RSA(pkey.get())); - if (rsa != nullptr) { + if (rsa) { const BIGNUM* n; const BIGNUM* e; - RSA_get0_key(rsa, &n, &e, nullptr); - BN_print(bio, n); - BIO_get_mem_ptr(bio, &mem); + RSA_get0_key(rsa.get(), &n, &e, nullptr); + BN_print(bio.get(), n); + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, env->modulus_string(), String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); - USE(BIO_reset(bio)); + USE(BIO_reset(bio.get())); uint64_t exponent_word = static_cast(BN_get_word(e)); uint32_t lo = static_cast(exponent_word); uint32_t hi = static_cast(exponent_word >> 32); if (hi == 0) { - BIO_printf(bio, "0x%x", lo); + BIO_printf(bio.get(), "0x%x", lo); } else { - BIO_printf(bio, "0x%x%08x", hi, lo); + BIO_printf(bio.get(), "0x%x%08x", hi, lo); } - BIO_get_mem_ptr(bio, &mem); + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, env->exponent_string(), String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); - USE(BIO_reset(bio)); + USE(BIO_reset(bio.get())); - int size = i2d_RSA_PUBKEY(rsa, nullptr); + int size = i2d_RSA_PUBKEY(rsa.get(), nullptr); CHECK_GE(size, 0); Local pubbuff = Buffer::New(env, size).ToLocalChecked(); unsigned char* pubserialized = reinterpret_cast(Buffer::Data(pubbuff)); - i2d_RSA_PUBKEY(rsa, &pubserialized); + i2d_RSA_PUBKEY(rsa.get(), &pubserialized); info->Set(env->pubkey_string(), pubbuff); } - if (pkey != nullptr) { - EVP_PKEY_free(pkey); - pkey = nullptr; - } - if (rsa != nullptr) { - RSA_free(rsa); - rsa = nullptr; - } + pkey.reset(); + rsa.reset(); - ASN1_TIME_print(bio, X509_get_notBefore(cert)); - BIO_get_mem_ptr(bio, &mem); + ASN1_TIME_print(bio.get(), X509_get_notBefore(cert)); + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, env->valid_from_string(), String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); - USE(BIO_reset(bio)); + USE(BIO_reset(bio.get())); - ASN1_TIME_print(bio, X509_get_notAfter(cert)); - BIO_get_mem_ptr(bio, &mem); + ASN1_TIME_print(bio.get(), X509_get_notAfter(cert)); + BIO_get_mem_ptr(bio.get(), &mem); info->Set(context, env->valid_to_string(), String::NewFromUtf8(env->isolate(), mem->data, String::kNormalString, mem->length)).FromJust(); - BIO_free_all(bio); + bio.reset(); unsigned char md[EVP_MAX_MD_SIZE]; unsigned int md_size; @@ -1687,32 +1664,35 @@ static Local X509ToObject(Environment* env, X509* cert) { OneByteString(env->isolate(), fingerprint)).FromJust(); } - STACK_OF(ASN1_OBJECT)* eku = static_cast( - X509_get_ext_d2i(cert, NID_ext_key_usage, nullptr, nullptr)); - if (eku != nullptr) { + StackOfASN1 eku(static_cast( + X509_get_ext_d2i(cert, NID_ext_key_usage, nullptr, nullptr))); + if (eku) { Local ext_key_usage = Array::New(env->isolate()); char buf[256]; int j = 0; - for (int i = 0; i < sk_ASN1_OBJECT_num(eku); i++) { - if (OBJ_obj2txt(buf, sizeof(buf), sk_ASN1_OBJECT_value(eku, i), 1) >= 0) + for (int i = 0; i < sk_ASN1_OBJECT_num(eku.get()); i++) { + if (OBJ_obj2txt(buf, + sizeof(buf), + sk_ASN1_OBJECT_value(eku.get(), i), 1) >= 0) { ext_key_usage->Set(context, j++, OneByteString(env->isolate(), buf)).FromJust(); + } } - sk_ASN1_OBJECT_pop_free(eku, ASN1_OBJECT_free); + eku.reset(); info->Set(context, env->ext_key_usage_string(), ext_key_usage).FromJust(); } if (ASN1_INTEGER* serial_number = X509_get_serialNumber(cert)) { - if (BIGNUM* bn = ASN1_INTEGER_to_BN(serial_number, nullptr)) { - if (char* buf = BN_bn2hex(bn)) { + BignumPointer bn(ASN1_INTEGER_to_BN(serial_number, nullptr)); + if (bn) { + OpenSSLBuffer buf(BN_bn2hex(bn.get())); + if (buf) { info->Set(context, env->serial_number_string(), - OneByteString(env->isolate(), buf)).FromJust(); - OPENSSL_free(buf); + OneByteString(env->isolate(), buf.get())).FromJust(); } - BN_free(bn); } } @@ -1728,17 +1708,17 @@ static Local X509ToObject(Environment* env, X509* cert) { } -static Local AddIssuerChainToObject(X509** cert, +static Local AddIssuerChainToObject(X509Pointer* cert, Local object, - StackOfX509 peer_certs, + StackOfX509&& peer_certs, Environment* const env) { Local context = env->isolate()->GetCurrentContext(); - *cert = sk_X509_delete(peer_certs.get(), 0); + cert->reset(sk_X509_delete(peer_certs.get(), 0)); for (;;) { int i; for (i = 0; i < sk_X509_num(peer_certs.get()); i++) { X509* ca = sk_X509_value(peer_certs.get(), i); - if (X509_check_issued(ca, *cert) != X509_V_OK) + if (X509_check_issued(ca, cert->get()) != X509_V_OK) continue; Local ca_info = X509ToObject(env, ca); @@ -1746,10 +1726,8 @@ static Local AddIssuerChainToObject(X509** cert, object = ca_info; // NOTE: Intentionally freeing cert that is not used anymore. - X509_free(*cert); - // Delete cert and continue aggregating issuers. - *cert = sk_X509_delete(peer_certs.get(), i); + cert->reset(sk_X509_delete(peer_certs.get(), i)); break; } @@ -1761,41 +1739,38 @@ static Local AddIssuerChainToObject(X509** cert, } -static StackOfX509 CloneSSLCerts(X509** cert, +static StackOfX509 CloneSSLCerts(X509Pointer&& cert, const STACK_OF(X509)* const ssl_certs) { StackOfX509 peer_certs(sk_X509_new(nullptr)); - if (*cert != nullptr) - sk_X509_push(peer_certs.get(), *cert); + if (cert) + sk_X509_push(peer_certs.get(), cert.release()); for (int i = 0; i < sk_X509_num(ssl_certs); i++) { - *cert = X509_dup(sk_X509_value(ssl_certs, i)); - if (*cert == nullptr) - return StackOfX509(); - if (!sk_X509_push(peer_certs.get(), *cert)) + X509Pointer cert(X509_dup(sk_X509_value(ssl_certs, i))); + if (!cert || !sk_X509_push(peer_certs.get(), cert.get())) return StackOfX509(); + // `cert` is now managed by the stack. + cert.release(); } return peer_certs; } -static Local GetLastIssuedCert(X509** cert, - const SSL* const ssl, +static Local GetLastIssuedCert(X509Pointer* cert, + const SSLPointer& ssl, Local issuer_chain, Environment* const env) { Local context = env->isolate()->GetCurrentContext(); - while (X509_check_issued(*cert, *cert) != X509_V_OK) { + while (X509_check_issued(cert->get(), cert->get()) != X509_V_OK) { X509* ca; - if (SSL_CTX_get_issuer(SSL_get_SSL_CTX(ssl), *cert, &ca) <= 0) + if (SSL_CTX_get_issuer(SSL_get_SSL_CTX(ssl.get()), cert->get(), &ca) <= 0) break; Local ca_info = X509ToObject(env, ca); issuer_chain->Set(context, env->issuercert_string(), ca_info).FromJust(); issuer_chain = ca_info; - // NOTE: Intentionally freeing cert that is not used anymore. - X509_free(*cert); - - // Delete cert and continue aggregating issuers. - *cert = ca; + // Delete previous cert and continue aggregating issuers. + cert->reset(ca); } return issuer_chain; } @@ -1816,40 +1791,35 @@ void SSLWrap::GetPeerCertificate( // NOTE: This is because of the odd OpenSSL behavior. On client `cert_chain` // contains the `peer_certificate`, but on server it doesn't. - X509* cert = w->is_server() ? SSL_get_peer_certificate(w->ssl_) : nullptr; - STACK_OF(X509)* ssl_certs = SSL_get_peer_cert_chain(w->ssl_); - if (cert == nullptr && (ssl_certs == nullptr || sk_X509_num(ssl_certs) == 0)) + X509Pointer cert( + w->is_server() ? SSL_get_peer_certificate(w->ssl_.get()) : nullptr); + STACK_OF(X509)* ssl_certs = SSL_get_peer_cert_chain(w->ssl_.get()); + if (!cert && (ssl_certs == nullptr || sk_X509_num(ssl_certs) == 0)) goto done; // Short result requested. if (args.Length() < 1 || !args[0]->IsTrue()) { - X509* target_cert = cert; - if (target_cert == nullptr) - target_cert = sk_X509_value(ssl_certs, 0); - result = X509ToObject(env, target_cert); + result = X509ToObject(env, cert ? cert.get() : sk_X509_value(ssl_certs, 0)); goto done; } - if (auto peer_certs = CloneSSLCerts(&cert, ssl_certs)) { + if (auto peer_certs = CloneSSLCerts(std::move(cert), ssl_certs)) { // First and main certificate. - cert = sk_X509_value(peer_certs.get(), 0); - result = X509ToObject(env, cert); + X509Pointer cert(sk_X509_value(peer_certs.get(), 0)); + CHECK(cert); + result = X509ToObject(env, cert.release()); issuer_chain = AddIssuerChainToObject(&cert, result, std::move(peer_certs), env); issuer_chain = GetLastIssuedCert(&cert, w->ssl_, issuer_chain, env); // Last certificate should be self-signed. - if (X509_check_issued(cert, cert) == X509_V_OK) + if (X509_check_issued(cert.get(), cert.get()) == X509_V_OK) issuer_chain->Set(env->context(), env->issuercert_string(), issuer_chain).FromJust(); - - CHECK_NE(cert, nullptr); } done: - if (cert != nullptr) - X509_free(cert); if (result.IsEmpty()) result = Object::New(env->isolate()); args.GetReturnValue().Set(result); @@ -1869,12 +1839,12 @@ void SSLWrap::GetFinished(const FunctionCallbackInfo& args) { // sections 7.21.2.1, 7.21.1.2, and 7.1.4, would be violated. // Thus, we use a dummy byte. char dummy[1]; - size_t len = SSL_get_finished(w->ssl_, dummy, sizeof dummy); + size_t len = SSL_get_finished(w->ssl_.get(), dummy, sizeof dummy); if (len == 0) return; char* buf = Malloc(len); - CHECK_EQ(len, SSL_get_finished(w->ssl_, buf, len)); + CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf, len)); args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); } @@ -1892,12 +1862,12 @@ void SSLWrap::GetPeerFinished(const FunctionCallbackInfo& args) { // sections 7.21.2.1, 7.21.1.2, and 7.1.4, would be violated. // Thus, we use a dummy byte. char dummy[1]; - size_t len = SSL_get_peer_finished(w->ssl_, dummy, sizeof dummy); + size_t len = SSL_get_peer_finished(w->ssl_.get(), dummy, sizeof dummy); if (len == 0) return; char* buf = Malloc(len); - CHECK_EQ(len, SSL_get_peer_finished(w->ssl_, buf, len)); + CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf, len)); args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); } @@ -1909,7 +1879,7 @@ void SSLWrap::GetSession(const FunctionCallbackInfo& args) { Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - SSL_SESSION* sess = SSL_get_session(w->ssl_); + SSL_SESSION* sess = SSL_get_session(w->ssl_.get()); if (sess == nullptr) return; @@ -1936,19 +1906,18 @@ void SSLWrap::SetSession(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Session"); size_t slen = Buffer::Length(args[0]); - char* sbuf = new char[slen]; - memcpy(sbuf, Buffer::Data(args[0]), slen); - - const unsigned char* p = reinterpret_cast(sbuf); - SSL_SESSION* sess = d2i_SSL_SESSION(nullptr, &p, slen); + std::vector sbuf(slen); + if (char* p = Buffer::Data(args[0])) + sbuf.assign(p, p + slen); - delete[] sbuf; + const unsigned char* p = reinterpret_cast(sbuf.data()); + SSLSessionPointer sess( + d2i_SSL_SESSION(nullptr, &p, slen)); if (sess == nullptr) return; - int r = SSL_set_session(w->ssl_, sess); - SSL_SESSION_free(sess); + int r = SSL_set_session(w->ssl_.get(), sess.get()); if (!r) return env->ThrowError("SSL_set_session error"); @@ -1968,9 +1937,7 @@ void SSLWrap::LoadSession(const FunctionCallbackInfo& args) { SSL_SESSION* sess = d2i_SSL_SESSION(nullptr, &p, slen); // Setup next session and move hello to the BIO buffer - if (w->next_sess_ != nullptr) - SSL_SESSION_free(w->next_sess_); - w->next_sess_ = sess; + w->next_sess_.reset(sess); } } @@ -1979,7 +1946,7 @@ template void SSLWrap::IsSessionReused(const FunctionCallbackInfo& args) { Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - bool yes = SSL_session_reused(w->ssl_); + bool yes = SSL_session_reused(w->ssl_.get()); args.GetReturnValue().Set(yes); } @@ -1999,7 +1966,7 @@ void SSLWrap::Renegotiate(const FunctionCallbackInfo& args) { ClearErrorOnReturn clear_error_on_return; - bool yes = SSL_renegotiate(w->ssl_) == 1; + bool yes = SSL_renegotiate(w->ssl_.get()) == 1; args.GetReturnValue().Set(yes); } @@ -2009,7 +1976,7 @@ void SSLWrap::Shutdown(const FunctionCallbackInfo& args) { Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - int rv = SSL_shutdown(w->ssl_); + int rv = SSL_shutdown(w->ssl_.get()); args.GetReturnValue().Set(rv); } @@ -2020,7 +1987,7 @@ void SSLWrap::GetTLSTicket(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); Environment* env = w->ssl_env(); - SSL_SESSION* sess = SSL_get_session(w->ssl_); + SSL_SESSION* sess = SSL_get_session(w->ssl_.get()); if (sess == nullptr) return; @@ -2072,7 +2039,7 @@ void SSLWrap::RequestOCSP( Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - SSL_set_tlsext_status_type(w->ssl_, TLSEXT_STATUSTYPE_ocsp); + SSL_set_tlsext_status_type(w->ssl_.get(), TLSEXT_STATUSTYPE_ocsp); #endif // NODE__HAVE_TLSEXT_STATUS_CB } @@ -2085,7 +2052,7 @@ void SSLWrap::GetEphemeralKeyInfo( Environment* env = Environment::GetCurrent(args); Local context = env->context(); - CHECK_NE(w->ssl_, nullptr); + CHECK(w->ssl_); // tmp key is available on only client if (w->is_server()) @@ -2095,7 +2062,7 @@ void SSLWrap::GetEphemeralKeyInfo( EVP_PKEY* key; - if (SSL_get_server_tmp_key(w->ssl_, &key)) { + if (SSL_get_server_tmp_key(w->ssl_.get(), &key)) { int kid = EVP_PKEY_id(key); switch (kid) { case EVP_PKEY_DH: @@ -2145,7 +2112,9 @@ void SSLWrap::SetMaxSendFragment( Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - int rv = SSL_set_max_send_fragment(w->ssl_, args[0]->Int32Value()); + int rv = SSL_set_max_send_fragment( + w->ssl_.get(), + args[0]->Int32Value(w->ssl_env()->context()).FromJust()); args.GetReturnValue().Set(rv); } #endif // SSL_set_max_send_fragment @@ -2155,7 +2124,7 @@ template void SSLWrap::IsInitFinished(const FunctionCallbackInfo& args) { Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - bool yes = SSL_is_init_finished(w->ssl_); + bool yes = SSL_is_init_finished(w->ssl_.get()); args.GetReturnValue().Set(yes); } @@ -2170,9 +2139,9 @@ void SSLWrap::VerifyError(const FunctionCallbackInfo& args) { // here before. long x509_verify_error = // NOLINT(runtime/int) X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT; - if (X509* peer_cert = SSL_get_peer_certificate(w->ssl_)) { + if (X509* peer_cert = SSL_get_peer_certificate(w->ssl_.get())) { X509_free(peer_cert); - x509_verify_error = SSL_get_verify_result(w->ssl_); + x509_verify_error = SSL_get_verify_result(w->ssl_.get()); } if (x509_verify_error == X509_V_OK) @@ -2232,7 +2201,7 @@ void SSLWrap::GetCurrentCipher(const FunctionCallbackInfo& args) { Environment* env = w->ssl_env(); Local context = env->context(); - const SSL_CIPHER* c = SSL_get_current_cipher(w->ssl_); + const SSL_CIPHER* c = SSL_get_current_cipher(w->ssl_.get()); if (c == nullptr) return; @@ -2251,7 +2220,7 @@ void SSLWrap::GetProtocol(const FunctionCallbackInfo& args) { Base* w; ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); - const char* tls_version = SSL_get_version(w->ssl_); + const char* tls_version = SSL_get_version(w->ssl_.get()); args.GetReturnValue().Set(OneByteString(args.GetIsolate(), tls_version)); } @@ -2299,7 +2268,7 @@ void SSLWrap::GetALPNNegotiatedProto( const unsigned char* alpn_proto; unsigned int alpn_proto_len; - SSL_get0_alpn_selected(w->ssl_, &alpn_proto, &alpn_proto_len); + SSL_get0_alpn_selected(w->ssl_.get(), &alpn_proto, &alpn_proto_len); if (!alpn_proto) return args.GetReturnValue().Set(false); @@ -2324,16 +2293,17 @@ void SSLWrap::SetALPNProtocols( const unsigned char* alpn_protos = reinterpret_cast(Buffer::Data(args[0])); unsigned alpn_protos_len = Buffer::Length(args[0]); - int r = SSL_set_alpn_protos(w->ssl_, alpn_protos, alpn_protos_len); + int r = SSL_set_alpn_protos(w->ssl_.get(), alpn_protos, alpn_protos_len); CHECK_EQ(r, 0); } else { CHECK( w->object()->SetPrivate( - env->context(), - env->alpn_buffer_private_symbol(), - args[0]).FromJust()); + env->context(), + env->alpn_buffer_private_symbol(), + args[0]).FromJust()); // Server should select ALPN protocol from list of advertised by client - SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(w->ssl_), SelectALPNCallback, + SSL_CTX_set_alpn_select_cb(SSL_get_SSL_CTX(w->ssl_.get()), + SelectALPNCallback, nullptr); } #endif // TLSEXT_TYPE_application_layer_protocol_negotiation @@ -2469,17 +2439,17 @@ void SSLWrap::CertCbDone(const FunctionCallbackInfo& args) { int rv; // NOTE: reference count is not increased by this API methods - X509* x509 = SSL_CTX_get0_certificate(sc->ctx_); - EVP_PKEY* pkey = SSL_CTX_get0_privatekey(sc->ctx_); + X509* x509 = SSL_CTX_get0_certificate(sc->ctx_.get()); + EVP_PKEY* pkey = SSL_CTX_get0_privatekey(sc->ctx_.get()); STACK_OF(X509)* chain; - rv = SSL_CTX_get0_chain_certs(sc->ctx_, &chain); + rv = SSL_CTX_get0_chain_certs(sc->ctx_.get(), &chain); if (rv) - rv = SSL_use_certificate(w->ssl_, x509); + rv = SSL_use_certificate(w->ssl_.get(), x509); if (rv) - rv = SSL_use_PrivateKey(w->ssl_, pkey); + rv = SSL_use_PrivateKey(w->ssl_.get(), pkey); if (rv && chain != nullptr) - rv = SSL_set1_chain(w->ssl_, chain); + rv = SSL_set1_chain(w->ssl_.get(), chain); if (rv) rv = w->SetCACerts(sc); if (!rv) { @@ -2512,19 +2482,18 @@ void SSLWrap::CertCbDone(const FunctionCallbackInfo& args) { template void SSLWrap::DestroySSL() { - if (ssl_ == nullptr) + if (!ssl_) return; - SSL_free(ssl_); env_->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize); - ssl_ = nullptr; + ssl_.reset(); } template void SSLWrap::SetSNIContext(SecureContext* sc) { ConfigureSecureContext(sc); - CHECK_EQ(SSL_set_SSL_CTX(ssl_, sc->ctx_), sc->ctx_); + CHECK_EQ(SSL_set_SSL_CTX(ssl_.get(), sc->ctx_.get()), sc->ctx_.get()); SetCACerts(sc); } @@ -2532,15 +2501,16 @@ void SSLWrap::SetSNIContext(SecureContext* sc) { template int SSLWrap::SetCACerts(SecureContext* sc) { - int err = SSL_set1_verify_cert_store(ssl_, SSL_CTX_get_cert_store(sc->ctx_)); + int err = SSL_set1_verify_cert_store(ssl_.get(), + SSL_CTX_get_cert_store(sc->ctx_.get())); if (err != 1) return err; STACK_OF(X509_NAME)* list = SSL_dup_CA_list( - SSL_CTX_get_client_CA_list(sc->ctx_)); + SSL_CTX_get_client_CA_list(sc->ctx_.get())); // NOTE: `SSL_set_client_CA_list` takes the ownership of `list` - SSL_set_client_CA_list(ssl_, list); + SSL_set_client_CA_list(ssl_.get(), list); return 1; } @@ -2629,11 +2599,10 @@ void CipherBase::Init(const char* cipher_type, } #endif // NODE_FIPS_MODE - CHECK_EQ(ctx_, nullptr); + CHECK(!ctx_); const EVP_CIPHER* const cipher = EVP_get_cipherbyname(cipher_type); - if (cipher == nullptr) { + if (cipher == nullptr) return env()->ThrowError("Unknown cipher"); - } unsigned char key[EVP_MAX_KEY_LENGTH]; unsigned char iv[EVP_MAX_IV_LENGTH]; @@ -2647,11 +2616,11 @@ void CipherBase::Init(const char* cipher_type, key, iv); - ctx_ = EVP_CIPHER_CTX_new(); + ctx_.reset(EVP_CIPHER_CTX_new()); const bool encrypt = (kind_ == kCipher); - EVP_CipherInit_ex(ctx_, cipher, nullptr, nullptr, nullptr, encrypt); + EVP_CipherInit_ex(ctx_.get(), cipher, nullptr, nullptr, nullptr, encrypt); - int mode = EVP_CIPHER_CTX_mode(ctx_); + int mode = EVP_CIPHER_CTX_mode(ctx_.get()); if (encrypt && (mode == EVP_CIPH_CTR_MODE || mode == EVP_CIPH_GCM_MODE || mode == EVP_CIPH_CCM_MODE)) { // Ignore the return value (i.e. possible exception) because we are @@ -2662,7 +2631,7 @@ void CipherBase::Init(const char* cipher_type, } if (mode == EVP_CIPH_WRAP_MODE) - EVP_CIPHER_CTX_set_flags(ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); + EVP_CIPHER_CTX_set_flags(ctx_.get(), EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); if (IsAuthenticatedMode()) { if (!InitAuthenticated(cipher_type, EVP_CIPHER_iv_length(cipher), @@ -2670,9 +2639,9 @@ void CipherBase::Init(const char* cipher_type, return; } - CHECK_EQ(1, EVP_CIPHER_CTX_set_key_length(ctx_, key_len)); + CHECK_EQ(1, EVP_CIPHER_CTX_set_key_length(ctx_.get(), key_len)); - EVP_CipherInit_ex(ctx_, + EVP_CipherInit_ex(ctx_.get(), nullptr, nullptr, reinterpret_cast(key), @@ -2730,13 +2699,13 @@ void CipherBase::InitIv(const char* cipher_type, return env()->ThrowError("Invalid IV length"); } - ctx_ = EVP_CIPHER_CTX_new(); + ctx_.reset(EVP_CIPHER_CTX_new()); if (mode == EVP_CIPH_WRAP_MODE) - EVP_CIPHER_CTX_set_flags(ctx_, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); + EVP_CIPHER_CTX_set_flags(ctx_.get(), EVP_CIPHER_CTX_FLAG_WRAP_ALLOW); const bool encrypt = (kind_ == kCipher); - EVP_CipherInit_ex(ctx_, cipher, nullptr, nullptr, nullptr, encrypt); + EVP_CipherInit_ex(ctx_.get(), cipher, nullptr, nullptr, nullptr, encrypt); if (IsAuthenticatedMode()) { CHECK(has_iv); @@ -2744,13 +2713,12 @@ void CipherBase::InitIv(const char* cipher_type, return; } - if (!EVP_CIPHER_CTX_set_key_length(ctx_, key_len)) { - EVP_CIPHER_CTX_free(ctx_); - ctx_ = nullptr; + if (!EVP_CIPHER_CTX_set_key_length(ctx_.get(), key_len)) { + ctx_.reset(); return env()->ThrowError("Invalid key length"); } - EVP_CipherInit_ex(ctx_, + EVP_CipherInit_ex(ctx_.get(), nullptr, nullptr, reinterpret_cast(key), @@ -2791,12 +2759,15 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len, int auth_tag_len) { CHECK(IsAuthenticatedMode()); - if (!EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_AEAD_SET_IVLEN, iv_len, nullptr)) { + if (!EVP_CIPHER_CTX_ctrl(ctx_.get(), + EVP_CTRL_AEAD_SET_IVLEN, + iv_len, + nullptr)) { env()->ThrowError("Invalid IV length"); return false; } - if (EVP_CIPHER_CTX_mode(ctx_) == EVP_CIPH_CCM_MODE) { + if (EVP_CIPHER_CTX_mode(ctx_.get()) == EVP_CIPH_CCM_MODE) { if (auth_tag_len < 0) { char msg[128]; snprintf(msg, sizeof(msg), "authTagLength required for %s", cipher_type); @@ -2812,8 +2783,8 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len, } #endif - if (!EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_CCM_SET_TAG, auth_tag_len, - nullptr)) { + if (!EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_CCM_SET_TAG, auth_tag_len, + nullptr)) { env()->ThrowError("Invalid authentication tag length"); return false; } @@ -2836,8 +2807,8 @@ bool CipherBase::InitAuthenticated(const char *cipher_type, int iv_len, bool CipherBase::CheckCCMMessageLength(int message_len) { - CHECK_NE(ctx_, nullptr); - CHECK(EVP_CIPHER_CTX_mode(ctx_) == EVP_CIPH_CCM_MODE); + CHECK(ctx_); + CHECK(EVP_CIPHER_CTX_mode(ctx_.get()) == EVP_CIPH_CCM_MODE); if (message_len > max_message_size_) { env()->ThrowError("Message exceeds maximum size"); @@ -2850,8 +2821,8 @@ bool CipherBase::CheckCCMMessageLength(int message_len) { bool CipherBase::IsAuthenticatedMode() const { // Check if this cipher operates in an AEAD mode that we support. - CHECK_NE(ctx_, nullptr); - const int mode = EVP_CIPHER_CTX_mode(ctx_); + CHECK(ctx_); + const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); return mode == EVP_CIPH_GCM_MODE || mode == EVP_CIPH_CCM_MODE; } @@ -2862,7 +2833,7 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); // Only callable after Final and if encrypting. - if (cipher->ctx_ != nullptr || + if (cipher->ctx_ || cipher->kind_ != kCipher || cipher->auth_tag_len_ == 0) { return args.GetReturnValue().SetUndefined(); @@ -2879,7 +2850,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - if (cipher->ctx_ == nullptr || + if (!cipher->ctx_ || !cipher->IsAuthenticatedMode() || cipher->kind_ != kDecipher) { return args.GetReturnValue().Set(false); @@ -2887,7 +2858,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { // Restrict GCM tag lengths according to NIST 800-38d, page 9. unsigned int tag_len = Buffer::Length(args[0]); - const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_); + const int mode = EVP_CIPHER_CTX_mode(cipher->ctx_.get()); if (mode == EVP_CIPH_GCM_MODE) { if (tag_len > 16 || (tag_len < 12 && tag_len != 8 && tag_len != 4)) { char msg[125]; @@ -2909,11 +2880,11 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { bool CipherBase::SetAAD(const char* data, unsigned int len, int plaintext_len) { - if (ctx_ == nullptr || !IsAuthenticatedMode()) + if (!ctx_ || !IsAuthenticatedMode()) return false; int outlen; - const int mode = EVP_CIPHER_CTX_mode(ctx_); + const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); // When in CCM mode, we need to set the authentication tag and the plaintext // length in advance. @@ -2927,7 +2898,7 @@ bool CipherBase::SetAAD(const char* data, unsigned int len, int plaintext_len) { return false; if (kind_ == kDecipher && !auth_tag_set_ && auth_tag_len_ > 0) { - if (!EVP_CIPHER_CTX_ctrl(ctx_, + if (!EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_CCM_SET_TAG, auth_tag_len_, reinterpret_cast(auth_tag_))) { @@ -2937,11 +2908,11 @@ bool CipherBase::SetAAD(const char* data, unsigned int len, int plaintext_len) { } // Specify the plaintext length. - if (!EVP_CipherUpdate(ctx_, nullptr, &outlen, nullptr, plaintext_len)) + if (!EVP_CipherUpdate(ctx_.get(), nullptr, &outlen, nullptr, plaintext_len)) return false; } - return 1 == EVP_CipherUpdate(ctx_, + return 1 == EVP_CipherUpdate(ctx_.get(), nullptr, &outlen, reinterpret_cast(data), @@ -2967,10 +2938,10 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, int len, unsigned char** out, int* out_len) { - if (ctx_ == nullptr) + if (!ctx_) return kErrorState; - const int mode = EVP_CIPHER_CTX_mode(ctx_); + const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); if (mode == EVP_CIPH_CCM_MODE) { if (!CheckCCMMessageLength(len)) @@ -2980,7 +2951,7 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, // on first update: if (kind_ == kDecipher && IsAuthenticatedMode() && auth_tag_len_ > 0 && !auth_tag_set_) { - EVP_CIPHER_CTX_ctrl(ctx_, + EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_GCM_SET_TAG, auth_tag_len_, reinterpret_cast(auth_tag_)); @@ -2988,11 +2959,11 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, } *out_len = 0; - int buff_len = len + EVP_CIPHER_CTX_block_size(ctx_); + int buff_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); // For key wrapping algorithms, get output size by calling // EVP_CipherUpdate() with null output. if (mode == EVP_CIPH_WRAP_MODE && - EVP_CipherUpdate(ctx_, + EVP_CipherUpdate(ctx_.get(), nullptr, &buff_len, reinterpret_cast(data), @@ -3001,7 +2972,7 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, } *out = Malloc(buff_len); - int r = EVP_CipherUpdate(ctx_, + int r = EVP_CipherUpdate(ctx_.get(), *out, out_len, reinterpret_cast(data), @@ -3059,9 +3030,9 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { bool CipherBase::SetAutoPadding(bool auto_padding) { - if (ctx_ == nullptr) + if (!ctx_) return false; - return EVP_CIPHER_CTX_set_padding(ctx_, auto_padding); + return EVP_CIPHER_CTX_set_padding(ctx_.get(), auto_padding); } @@ -3075,13 +3046,13 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { bool CipherBase::Final(unsigned char** out, int *out_len) { - if (ctx_ == nullptr) + if (!ctx_) return false; - const int mode = EVP_CIPHER_CTX_mode(ctx_); + const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); *out = Malloc( - static_cast(EVP_CIPHER_CTX_block_size(ctx_))); + static_cast(EVP_CIPHER_CTX_block_size(ctx_.get()))); // In CCM mode, final() only checks whether authentication failed in update(). // EVP_CipherFinal_ex must not be called and will fail. @@ -3089,21 +3060,20 @@ bool CipherBase::Final(unsigned char** out, int *out_len) { if (kind_ == kDecipher && mode == EVP_CIPH_CCM_MODE) { ok = !pending_auth_failed_; } else { - ok = EVP_CipherFinal_ex(ctx_, *out, out_len) == 1; + ok = EVP_CipherFinal_ex(ctx_.get(), *out, out_len) == 1; if (ok && kind_ == kCipher && IsAuthenticatedMode()) { // For GCM, the tag length is static (16 bytes), while the CCM tag length // must be specified in advance. if (mode == EVP_CIPH_GCM_MODE) auth_tag_len_ = sizeof(auth_tag_); - CHECK_EQ(1, EVP_CIPHER_CTX_ctrl(ctx_, EVP_CTRL_AEAD_GET_TAG, + CHECK_EQ(1, EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG, auth_tag_len_, reinterpret_cast(auth_tag_))); } } - EVP_CIPHER_CTX_free(ctx_); - ctx_ = nullptr; + ctx_.reset(); return ok; } @@ -3146,11 +3116,6 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { } -Hmac::~Hmac() { - HMAC_CTX_free(ctx_); -} - - void Hmac::Initialize(Environment* env, v8::Local target) { Local t = env->NewFunctionTemplate(New); @@ -3180,11 +3145,9 @@ void Hmac::HmacInit(const char* hash_type, const char* key, int key_len) { if (key_len == 0) { key = ""; } - ctx_ = HMAC_CTX_new(); - if (ctx_ == nullptr || - !HMAC_Init_ex(ctx_, key, key_len, md, nullptr)) { - HMAC_CTX_free(ctx_); - ctx_ = nullptr; + ctx_.reset(HMAC_CTX_new()); + if (!ctx_ || !HMAC_Init_ex(ctx_.get(), key, key_len, md, nullptr)) { + ctx_.reset(); return ThrowCryptoError(env(), ERR_get_error()); } } @@ -3203,9 +3166,11 @@ void Hmac::HmacInit(const FunctionCallbackInfo& args) { bool Hmac::HmacUpdate(const char* data, int len) { - if (ctx_ == nullptr) + if (!ctx_) return false; - int r = HMAC_Update(ctx_, reinterpret_cast(data), len); + int r = HMAC_Update(ctx_.get(), + reinterpret_cast(data), + len); return r == 1; } @@ -3250,10 +3215,9 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { unsigned char md_value[EVP_MAX_MD_SIZE]; unsigned int md_len = 0; - if (hmac->ctx_ != nullptr) { - HMAC_Final(hmac->ctx_, md_value, &md_len); - HMAC_CTX_free(hmac->ctx_); - hmac->ctx_ = nullptr; + if (hmac->ctx_) { + HMAC_Final(hmac->ctx_.get(), md_value, &md_len); + hmac->ctx_.reset(); } Local error; @@ -3272,11 +3236,6 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { } -Hash::~Hash() { - EVP_MD_CTX_free(mdctx_); -} - - void Hash::Initialize(Environment* env, v8::Local target) { Local t = env->NewFunctionTemplate(New); @@ -3306,11 +3265,9 @@ bool Hash::HashInit(const char* hash_type) { const EVP_MD* md = EVP_get_digestbyname(hash_type); if (md == nullptr) return false; - mdctx_ = EVP_MD_CTX_new(); - if (mdctx_ == nullptr || - EVP_DigestInit_ex(mdctx_, md, nullptr) <= 0) { - EVP_MD_CTX_free(mdctx_); - mdctx_ = nullptr; + mdctx_.reset(EVP_MD_CTX_new()); + if (!mdctx_ || EVP_DigestInit_ex(mdctx_.get(), md, nullptr) <= 0) { + mdctx_.reset(); return false; } finalized_ = false; @@ -3319,9 +3276,9 @@ bool Hash::HashInit(const char* hash_type) { bool Hash::HashUpdate(const char* data, int len) { - if (mdctx_ == nullptr) + if (!mdctx_) return false; - EVP_DigestUpdate(mdctx_, data, len); + EVP_DigestUpdate(mdctx_.get(), data, len); return true; } @@ -3365,7 +3322,7 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { unsigned char md_value[EVP_MAX_MD_SIZE]; unsigned int md_len; - EVP_DigestFinal_ex(hash->mdctx_, md_value, &md_len); + EVP_DigestFinal_ex(hash->mdctx_.get(), md_value, &md_len); hash->finalized_ = true; Local error; @@ -3384,11 +3341,6 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { } -SignBase::~SignBase() { - EVP_MD_CTX_free(mdctx_); -} - - SignBase::Error SignBase::Init(const char* sign_type) { CHECK_EQ(mdctx_, nullptr); // Historically, "dss1" and "DSS1" were DSA aliases for SHA-1 @@ -3401,11 +3353,9 @@ SignBase::Error SignBase::Init(const char* sign_type) { if (md == nullptr) return kSignUnknownDigest; - mdctx_ = EVP_MD_CTX_new(); - if (mdctx_ == nullptr || - !EVP_DigestInit_ex(mdctx_, md, nullptr)) { - EVP_MD_CTX_free(mdctx_); - mdctx_ = nullptr; + mdctx_.reset(EVP_MD_CTX_new()); + if (!mdctx_ || !EVP_DigestInit_ex(mdctx_.get(), md, nullptr)) { + mdctx_.reset(); return kSignInit; } @@ -3416,7 +3366,7 @@ SignBase::Error SignBase::Init(const char* sign_type) { SignBase::Error SignBase::Update(const char* data, int len) { if (mdctx_ == nullptr) return kSignNotInitialised; - if (!EVP_DigestUpdate(mdctx_, data, len)) + if (!EVP_DigestUpdate(mdctx_.get(), data, len)) return kSignUpdate; return kSignOk; } @@ -3459,10 +3409,12 @@ void SignBase::CheckThrow(SignBase::Error error) { } } -static bool ApplyRSAOptions(EVP_PKEY* pkey, EVP_PKEY_CTX* pkctx, int padding, +static bool ApplyRSAOptions(const EVPKeyPointer& pkey, + EVP_PKEY_CTX* pkctx, + int padding, int salt_len) { - if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA || - EVP_PKEY_id(pkey) == EVP_PKEY_RSA2) { + if (EVP_PKEY_id(pkey.get()) == EVP_PKEY_RSA || + EVP_PKEY_id(pkey.get()) == EVP_PKEY_RSA2) { if (EVP_PKEY_CTX_set_rsa_padding(pkctx, padding) <= 0) return false; if (padding == RSA_PKCS1_PSS_PADDING) { @@ -3516,35 +3468,29 @@ void Sign::SignUpdate(const FunctionCallbackInfo& args) { sign->CheckThrow(err); } -static int Node_SignFinal(EVP_MD_CTX* mdctx, unsigned char* md, - unsigned int* sig_len, EVP_PKEY* pkey, int padding, +static int Node_SignFinal(EVPMDPointer&& mdctx, unsigned char* md, + unsigned int* sig_len, + const EVPKeyPointer& pkey, int padding, int pss_salt_len) { unsigned char m[EVP_MAX_MD_SIZE]; unsigned int m_len; - int rv = 0; - EVP_PKEY_CTX* pkctx = nullptr; *sig_len = 0; - if (!EVP_DigestFinal_ex(mdctx, m, &m_len)) - return rv; - - size_t sltmp = static_cast(EVP_PKEY_size(pkey)); - pkctx = EVP_PKEY_CTX_new(pkey, nullptr); - if (pkctx == nullptr) - goto err; - if (EVP_PKEY_sign_init(pkctx) <= 0) - goto err; - if (!ApplyRSAOptions(pkey, pkctx, padding, pss_salt_len)) - goto err; - if (EVP_PKEY_CTX_set_signature_md(pkctx, EVP_MD_CTX_md(mdctx)) <= 0) - goto err; - if (EVP_PKEY_sign(pkctx, md, &sltmp, m, m_len) <= 0) - goto err; - *sig_len = sltmp; - rv = 1; - err: - EVP_PKEY_CTX_free(pkctx); - return rv; + if (!EVP_DigestFinal_ex(mdctx.get(), m, &m_len)) + return 0; + + size_t sltmp = static_cast(EVP_PKEY_size(pkey.get())); + EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); + if (pkctx && + EVP_PKEY_sign_init(pkctx.get()) > 0 && + ApplyRSAOptions(pkey, pkctx.get(), padding, pss_salt_len) && + EVP_PKEY_CTX_set_signature_md(pkctx.get(), + EVP_MD_CTX_md(mdctx.get())) > 0 && + EVP_PKEY_sign(pkctx.get(), md, &sltmp, m, m_len) > 0) { + *sig_len = sltmp; + return 1; + } + return 0; } SignBase::Error Sign::SignFinal(const char* key_pem, @@ -3557,24 +3503,22 @@ SignBase::Error Sign::SignFinal(const char* key_pem, if (!mdctx_) return kSignNotInitialised; - BIO* bp = nullptr; - EVP_PKEY* pkey = nullptr; - bool fatal = true; + EVPMDPointer mdctx = std::move(mdctx_); - bp = BIO_new_mem_buf(const_cast(key_pem), key_pem_len); - if (bp == nullptr) - goto exit; + BIOPointer bp(BIO_new_mem_buf(const_cast(key_pem), key_pem_len)); + if (!bp) + return kSignPrivateKey; - pkey = PEM_read_bio_PrivateKey(bp, - nullptr, - PasswordCallback, - const_cast(passphrase)); + EVPKeyPointer pkey(PEM_read_bio_PrivateKey(bp.get(), + nullptr, + PasswordCallback, + const_cast(passphrase))); // Errors might be injected into OpenSSL's error stack // without `pkey` being set to nullptr; // cf. the test of `test_bad_rsa_privkey.pem` for an example. - if (pkey == nullptr || 0 != ERR_peek_error()) - goto exit; + if (!pkey || 0 != ERR_peek_error()) + return kSignPrivateKey; #ifdef NODE_FIPS_MODE /* Validate DSA2 parameters from FIPS 186-4 */ @@ -3593,28 +3537,15 @@ SignBase::Error Sign::SignFinal(const char* key_pem, result = true; if (!result) { - fatal = true; - goto exit; + return kSignPrivateKey; } } #endif // NODE_FIPS_MODE - if (Node_SignFinal(mdctx_, sig, sig_len, pkey, padding, salt_len)) - fatal = false; - - exit: - if (pkey != nullptr) - EVP_PKEY_free(pkey); - if (bp != nullptr) - BIO_free_all(bp); - - EVP_MD_CTX_free(mdctx_); - mdctx_ = nullptr; - - if (fatal) + if (Node_SignFinal(std::move(mdctx), sig, sig_len, pkey, padding, salt_len)) + return kSignOk; + else return kSignPrivateKey; - - return kSignOk; } @@ -3715,87 +3646,60 @@ SignBase::Error Verify::VerifyFinal(const char* key_pem, if (!mdctx_) return kSignNotInitialised; - EVP_PKEY* pkey = nullptr; - BIO* bp = nullptr; - X509* x509 = nullptr; - bool fatal = true; + EVPKeyPointer pkey; unsigned char m[EVP_MAX_MD_SIZE]; unsigned int m_len; int r = 0; - EVP_PKEY_CTX* pkctx = nullptr; + *verify_result = false; + EVPMDPointer mdctx = std::move(mdctx_); - bp = BIO_new_mem_buf(const_cast(key_pem), key_pem_len); - if (bp == nullptr) - goto exit; + BIOPointer bp(BIO_new_mem_buf(const_cast(key_pem), key_pem_len)); + if (!bp) + return kSignPublicKey; // Check if this is a PKCS#8 or RSA public key before trying as X.509. // Split this out into a separate function once we have more than one // consumer of public keys. if (strncmp(key_pem, PUBLIC_KEY_PFX, PUBLIC_KEY_PFX_LEN) == 0) { - pkey = PEM_read_bio_PUBKEY(bp, nullptr, NoPasswordCallback, nullptr); - if (pkey == nullptr) - goto exit; + pkey.reset( + PEM_read_bio_PUBKEY(bp.get(), nullptr, NoPasswordCallback, nullptr)); } else if (strncmp(key_pem, PUBRSA_KEY_PFX, PUBRSA_KEY_PFX_LEN) == 0) { - RSA* rsa = - PEM_read_bio_RSAPublicKey(bp, nullptr, PasswordCallback, nullptr); + RSAPointer rsa(PEM_read_bio_RSAPublicKey( + bp.get(), nullptr, PasswordCallback, nullptr)); if (rsa) { - pkey = EVP_PKEY_new(); + pkey.reset(EVP_PKEY_new()); if (pkey) - EVP_PKEY_set1_RSA(pkey, rsa); - RSA_free(rsa); + EVP_PKEY_set1_RSA(pkey.get(), rsa.get()); } - if (pkey == nullptr) - goto exit; } else { // X.509 fallback - x509 = PEM_read_bio_X509(bp, nullptr, NoPasswordCallback, nullptr); - if (x509 == nullptr) - goto exit; - - pkey = X509_get_pubkey(x509); - if (pkey == nullptr) - goto exit; - } - - if (!EVP_DigestFinal_ex(mdctx_, m, &m_len)) { - goto exit; - } - - fatal = false; - - pkctx = EVP_PKEY_CTX_new(pkey, nullptr); - if (pkctx == nullptr) - goto err; - if (EVP_PKEY_verify_init(pkctx) <= 0) - goto err; - if (!ApplyRSAOptions(pkey, pkctx, padding, saltlen)) - goto err; - if (EVP_PKEY_CTX_set_signature_md(pkctx, EVP_MD_CTX_md(mdctx_)) <= 0) - goto err; - r = EVP_PKEY_verify(pkctx, - reinterpret_cast(sig), - siglen, - m, - m_len); - - err: - EVP_PKEY_CTX_free(pkctx); - - exit: - if (pkey != nullptr) - EVP_PKEY_free(pkey); - if (bp != nullptr) - BIO_free_all(bp); - if (x509 != nullptr) - X509_free(x509); + X509Pointer x509(PEM_read_bio_X509( + bp.get(), nullptr, NoPasswordCallback, nullptr)); + if (!x509) + return kSignPublicKey; - EVP_MD_CTX_free(mdctx_); - mdctx_ = nullptr; + pkey.reset(X509_get_pubkey(x509.get())); + } + if (!pkey) + return kSignPublicKey; - if (fatal) + if (!EVP_DigestFinal_ex(mdctx.get(), m, &m_len)) return kSignPublicKey; - *verify_result = r == 1; + EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); + if (pkctx && + EVP_PKEY_verify_init(pkctx.get()) > 0 && + ApplyRSAOptions(pkey, pkctx.get(), padding, saltlen) && + EVP_PKEY_CTX_set_signature_md(pkctx.get(), + EVP_MD_CTX_md(mdctx.get())) > 0) { + r = EVP_PKEY_verify(pkctx.get(), + reinterpret_cast(sig), + siglen, + m, + m_len); + *verify_result = r == 1; + } + return kSignOk; } @@ -3844,81 +3748,60 @@ bool PublicKeyCipher::Cipher(const char* key_pem, int len, unsigned char** out, size_t* out_len) { - EVP_PKEY* pkey = nullptr; - EVP_PKEY_CTX* ctx = nullptr; - BIO* bp = nullptr; - X509* x509 = nullptr; - bool fatal = true; + EVPKeyPointer pkey; - bp = BIO_new_mem_buf(const_cast(key_pem), key_pem_len); - if (bp == nullptr) - goto exit; + BIOPointer bp(BIO_new_mem_buf(const_cast(key_pem), key_pem_len)); + if (!bp) + return false; // Check if this is a PKCS#8 or RSA public key before trying as X.509 and // private key. if (operation == kPublic && strncmp(key_pem, PUBLIC_KEY_PFX, PUBLIC_KEY_PFX_LEN) == 0) { - pkey = PEM_read_bio_PUBKEY(bp, nullptr, nullptr, nullptr); - if (pkey == nullptr) - goto exit; + pkey.reset(PEM_read_bio_PUBKEY(bp.get(), nullptr, nullptr, nullptr)); } else if (operation == kPublic && strncmp(key_pem, PUBRSA_KEY_PFX, PUBRSA_KEY_PFX_LEN) == 0) { - RSA* rsa = PEM_read_bio_RSAPublicKey(bp, nullptr, nullptr, nullptr); + RSAPointer rsa( + PEM_read_bio_RSAPublicKey(bp.get(), nullptr, nullptr, nullptr)); if (rsa) { - pkey = EVP_PKEY_new(); + pkey.reset(EVP_PKEY_new()); if (pkey) - EVP_PKEY_set1_RSA(pkey, rsa); - RSA_free(rsa); + EVP_PKEY_set1_RSA(pkey.get(), rsa.get()); } - if (pkey == nullptr) - goto exit; } else if (operation == kPublic && strncmp(key_pem, CERTIFICATE_PFX, CERTIFICATE_PFX_LEN) == 0) { - x509 = PEM_read_bio_X509(bp, nullptr, NoPasswordCallback, nullptr); - if (x509 == nullptr) - goto exit; + X509Pointer x509( + PEM_read_bio_X509(bp.get(), nullptr, NoPasswordCallback, nullptr)); + if (!x509) + return false; - pkey = X509_get_pubkey(x509); - if (pkey == nullptr) - goto exit; + pkey.reset(X509_get_pubkey(x509.get())); } else { - pkey = PEM_read_bio_PrivateKey(bp, - nullptr, - PasswordCallback, - const_cast(passphrase)); - if (pkey == nullptr) - goto exit; + pkey.reset(PEM_read_bio_PrivateKey(bp.get(), + nullptr, + PasswordCallback, + const_cast(passphrase))); } + if (!pkey) + return false; - ctx = EVP_PKEY_CTX_new(pkey, nullptr); + EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); if (!ctx) - goto exit; - if (EVP_PKEY_cipher_init(ctx) <= 0) - goto exit; - if (EVP_PKEY_CTX_set_rsa_padding(ctx, padding) <= 0) - goto exit; + return false; + if (EVP_PKEY_cipher_init(ctx.get()) <= 0) + return false; + if (EVP_PKEY_CTX_set_rsa_padding(ctx.get(), padding) <= 0) + return false; - if (EVP_PKEY_cipher(ctx, nullptr, out_len, data, len) <= 0) - goto exit; + if (EVP_PKEY_cipher(ctx.get(), nullptr, out_len, data, len) <= 0) + return false; *out = Malloc(*out_len); - if (EVP_PKEY_cipher(ctx, *out, out_len, data, len) <= 0) - goto exit; - - fatal = false; - - exit: - if (x509 != nullptr) - X509_free(x509); - if (pkey != nullptr) - EVP_PKEY_free(pkey); - if (bp != nullptr) - BIO_free_all(bp); - if (ctx != nullptr) - EVP_PKEY_CTX_free(ctx); + if (EVP_PKEY_cipher(ctx.get(), *out, out_len, data, len) <= 0) + return false; - return !fatal; + return true; } @@ -4032,8 +3915,8 @@ void DiffieHellman::Initialize(Environment* env, Local target) { bool DiffieHellman::Init(int primeLength, int g) { - dh = DH_new(); - if (!DH_generate_parameters_ex(dh, primeLength, g, 0)) + dh_.reset(DH_new()); + if (!DH_generate_parameters_ex(dh_.get(), primeLength, g, 0)) return false; bool result = VerifyContext(); if (!result) @@ -4044,12 +3927,12 @@ bool DiffieHellman::Init(int primeLength, int g) { bool DiffieHellman::Init(const char* p, int p_len, int g) { - dh = DH_new(); + dh_.reset(DH_new()); BIGNUM* bn_p = BN_bin2bn(reinterpret_cast(p), p_len, nullptr); BIGNUM* bn_g = BN_new(); if (!BN_set_word(bn_g, g) || - !DH_set0_pqg(dh, bn_p, nullptr, bn_g)) { + !DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)) { BN_free(bn_p); BN_free(bn_g); return false; @@ -4063,10 +3946,10 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) { bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) { - dh = DH_new(); + dh_.reset(DH_new()); BIGNUM *bn_p = BN_bin2bn(reinterpret_cast(p), p_len, 0); BIGNUM *bn_g = BN_bin2bn(reinterpret_cast(g), g_len, 0); - if (!DH_set0_pqg(dh, bn_p, nullptr, bn_g)) { + if (!DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)) { BN_free(bn_p); BN_free(bn_g); return false; @@ -4154,12 +4037,12 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { return ThrowCryptoError(env, ERR_get_error(), "Not initialized"); } - if (!DH_generate_key(diffieHellman->dh)) { + if (!DH_generate_key(diffieHellman->dh_.get())) { return ThrowCryptoError(env, ERR_get_error(), "Key generation failed"); } const BIGNUM* pub_key; - DH_get0_key(diffieHellman->dh, &pub_key, nullptr); + DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr); size_t size = BN_num_bytes(pub_key); char* data = Malloc(size); BN_bn2bin(pub_key, reinterpret_cast(data)); @@ -4176,7 +4059,7 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, ASSIGN_OR_RETURN_UNWRAP(&dh, args.Holder()); if (!dh->initialised_) return env->ThrowError("Not initialized"); - const BIGNUM* num = get_field(dh->dh); + const BIGNUM* num = get_field(dh->dh_.get()); if (num == nullptr) return env->ThrowError(err_if_null); size_t size = BN_num_bytes(num); @@ -4232,33 +4115,31 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { } ClearErrorOnReturn clear_error_on_return; - BIGNUM* key = nullptr; if (args.Length() == 0) { return THROW_ERR_MISSING_ARGS( env, "Other party's public key argument is mandatory"); - } else { - THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Other party's public key"); - key = BN_bin2bn( - reinterpret_cast(Buffer::Data(args[0])), - Buffer::Length(args[0]), - 0); } - int dataSize = DH_size(diffieHellman->dh); - char* data = Malloc(dataSize); + THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Other party's public key"); + BignumPointer key(BN_bin2bn( + reinterpret_cast(Buffer::Data(args[0])), + Buffer::Length(args[0]), + 0)); + + MallocedBuffer data(DH_size(diffieHellman->dh_.get())); - int size = DH_compute_key(reinterpret_cast(data), - key, - diffieHellman->dh); + int size = DH_compute_key(reinterpret_cast(data.data), + key.get(), + diffieHellman->dh_.get()); if (size == -1) { int checkResult; int checked; - checked = DH_check_pub_key(diffieHellman->dh, key, &checkResult); - BN_free(key); - free(data); + checked = DH_check_pub_key(diffieHellman->dh_.get(), + key.get(), + &checkResult); if (!checked) { return ThrowCryptoError(env, ERR_get_error(), "Invalid Key"); @@ -4277,21 +4158,20 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { UNREACHABLE(); } - BN_free(key); CHECK_GE(size, 0); // DH_size returns number of bytes in a prime number // DH_compute_key returns number of bytes in a remainder of exponent, which // may have less bytes than a prime number. Therefore add 0-padding to the // allocated buffer. - if (size != dataSize) { - CHECK(dataSize > size); - memmove(data + dataSize - size, data, size); - memset(data, 0, dataSize - size); + if (static_cast(size) != data.size) { + CHECK_GT(data.size, static_cast(size)); + memmove(data.data + data.size - size, data.data, size); + memset(data.data, 0, data.size - size); } - auto rc = Buffer::New(env->isolate(), data, dataSize).ToLocalChecked(); - args.GetReturnValue().Set(rc); + args.GetReturnValue().Set( + Buffer::New(env->isolate(), data.release(), data.size).ToLocalChecked()); } void DiffieHellman::SetKey(const v8::FunctionCallbackInfo& args, @@ -4318,7 +4198,7 @@ void DiffieHellman::SetKey(const v8::FunctionCallbackInfo& args, BN_bin2bn(reinterpret_cast(Buffer::Data(args[0])), Buffer::Length(args[0]), nullptr); CHECK_NE(num, nullptr); - CHECK_EQ(1, set_field(dh->dh, num)); + CHECK_EQ(1, set_field(dh->dh_.get(), num)); } @@ -4357,7 +4237,7 @@ void DiffieHellman::VerifyErrorGetter(const FunctionCallbackInfo& args) { bool DiffieHellman::VerifyContext() { int codes; - if (!DH_check(dh, &codes)) + if (!DH_check(dh_.get(), &codes)) return false; verifyError_ = codes; return true; @@ -4397,11 +4277,11 @@ void ECDH::New(const FunctionCallbackInfo& args) { return THROW_ERR_INVALID_ARG_VALUE(env, "First argument should be a valid curve name"); - EC_KEY* key = EC_KEY_new_by_curve_name(nid); - if (key == nullptr) + ECKeyPointer key(EC_KEY_new_by_curve_name(nid)); + if (!key) return env->ThrowError("Failed to create EC_KEY using curve name"); - new ECDH(env, args.This(), key); + new ECDH(env, args.This(), std::move(key)); } @@ -4411,34 +4291,31 @@ void ECDH::GenerateKeys(const FunctionCallbackInfo& args) { ECDH* ecdh; ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder()); - if (!EC_KEY_generate_key(ecdh->key_)) + if (!EC_KEY_generate_key(ecdh->key_.get())) return env->ThrowError("Failed to generate EC_KEY"); } -EC_POINT* ECDH::BufferToPoint(Environment* env, - const EC_GROUP* group, - char* data, - size_t len) { - EC_POINT* pub; +ECPointPointer ECDH::BufferToPoint(Environment* env, + const EC_GROUP* group, + char* data, + size_t len) { int r; - pub = EC_POINT_new(group); - if (pub == nullptr) { + ECPointPointer pub(EC_POINT_new(group)); + if (!pub) { env->ThrowError("Failed to allocate EC_POINT for a public key"); - return nullptr; + return pub; } r = EC_POINT_oct2point( group, - pub, + pub.get(), reinterpret_cast(data), len, nullptr); - if (!r) { - EC_POINT_free(pub); - return nullptr; - } + if (!r) + return ECPointPointer(); return pub; } @@ -4457,11 +4334,12 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { if (!ecdh->IsKeyPairValid()) return env->ThrowError("Invalid key pair"); - EC_POINT* pub = ECDH::BufferToPoint(env, - ecdh->group_, - Buffer::Data(args[0]), - Buffer::Length(args[0])); - if (pub == nullptr) { + ECPointPointer pub( + ECDH::BufferToPoint(env, + ecdh->group_, + Buffer::Data(args[0]), + Buffer::Length(args[0]))); + if (!pub) { args.GetReturnValue().Set( FIXED_ONE_BYTE_STRING(env->isolate(), "ERR_CRYPTO_ECDH_INVALID_PUBLIC_KEY")); @@ -4473,8 +4351,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { size_t out_len = (field_size + 7) / 8; char* out = node::Malloc(out_len); - int r = ECDH_compute_key(out, out_len, pub, ecdh->key_, nullptr); - EC_POINT_free(pub); + int r = ECDH_compute_key(out, out_len, pub.get(), ecdh->key_.get(), nullptr); if (!r) { free(out); return env->ThrowError("Failed to compute ECDH key"); @@ -4494,7 +4371,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo& args) { ECDH* ecdh; ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder()); - const EC_POINT* pub = EC_KEY_get0_public_key(ecdh->key_); + const EC_POINT* pub = EC_KEY_get0_public_key(ecdh->key_.get()); if (pub == nullptr) return env->ThrowError("Failed to get ECDH public key"); @@ -4526,7 +4403,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { ECDH* ecdh; ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder()); - const BIGNUM* b = EC_KEY_get0_private_key(ecdh->key_); + const BIGNUM* b = EC_KEY_get0_private_key(ecdh->key_.get()); if (b == nullptr) return env->ThrowError("Failed to get ECDH private key"); @@ -4552,20 +4429,19 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Private key"); - BIGNUM* priv = BN_bin2bn( + BignumPointer priv(BN_bin2bn( reinterpret_cast(Buffer::Data(args[0].As())), Buffer::Length(args[0].As()), - nullptr); - if (priv == nullptr) + nullptr)); + if (!priv) return env->ThrowError("Failed to convert Buffer to BN"); if (!ecdh->IsKeyValidForCurve(priv)) { - BN_free(priv); return env->ThrowError("Private key is not valid for specified curve."); } - int result = EC_KEY_set_private_key(ecdh->key_, priv); - BN_free(priv); + int result = EC_KEY_set_private_key(ecdh->key_.get(), priv.get()); + priv.reset(); if (!result) { return env->ThrowError("Failed to convert BN to a private key"); @@ -4573,28 +4449,24 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo& args) { // To avoid inconsistency, clear the current public key in-case computing // the new one fails for some reason. - EC_KEY_set_public_key(ecdh->key_, nullptr); + EC_KEY_set_public_key(ecdh->key_.get(), nullptr); MarkPopErrorOnReturn mark_pop_error_on_return; USE(&mark_pop_error_on_return); - const BIGNUM* priv_key = EC_KEY_get0_private_key(ecdh->key_); + const BIGNUM* priv_key = EC_KEY_get0_private_key(ecdh->key_.get()); CHECK_NE(priv_key, nullptr); - EC_POINT* pub = EC_POINT_new(ecdh->group_); - CHECK_NE(pub, nullptr); + ECPointPointer pub(EC_POINT_new(ecdh->group_)); + CHECK(pub); - if (!EC_POINT_mul(ecdh->group_, pub, priv_key, nullptr, nullptr, nullptr)) { - EC_POINT_free(pub); + if (!EC_POINT_mul(ecdh->group_, pub.get(), priv_key, + nullptr, nullptr, nullptr)) { return env->ThrowError("Failed to generate ECDH public key"); } - if (!EC_KEY_set_public_key(ecdh->key_, pub)) { - EC_POINT_free(pub); + if (!EC_KEY_set_public_key(ecdh->key_.get(), pub.get())) return env->ThrowError("Failed to set generated public key"); - } - - EC_POINT_free(pub); } @@ -4608,41 +4480,39 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo& args) { MarkPopErrorOnReturn mark_pop_error_on_return; - EC_POINT* pub = ECDH::BufferToPoint(env, - ecdh->group_, - Buffer::Data(args[0].As()), - Buffer::Length(args[0].As())); - if (pub == nullptr) + ECPointPointer pub( + ECDH::BufferToPoint(env, + ecdh->group_, + Buffer::Data(args[0].As()), + Buffer::Length(args[0].As()))); + if (!pub) return env->ThrowError("Failed to convert Buffer to EC_POINT"); - int r = EC_KEY_set_public_key(ecdh->key_, pub); - EC_POINT_free(pub); + int r = EC_KEY_set_public_key(ecdh->key_.get(), pub.get()); if (!r) return env->ThrowError("Failed to set EC_POINT as the public key"); } -bool ECDH::IsKeyValidForCurve(const BIGNUM* private_key) { - CHECK_NE(group_, nullptr); - CHECK_NE(private_key, nullptr); +bool ECDH::IsKeyValidForCurve(const BignumPointer& private_key) { + CHECK(group_); + CHECK(private_key); // Private keys must be in the range [1, n-1]. // Ref: Section 3.2.1 - http://www.secg.org/sec1-v2.pdf - if (BN_cmp(private_key, BN_value_one()) < 0) { + if (BN_cmp(private_key.get(), BN_value_one()) < 0) { return false; } - BIGNUM* order = BN_new(); - CHECK_NE(order, nullptr); - bool result = EC_GROUP_get_order(group_, order, nullptr) && - BN_cmp(private_key, order) < 0; - BN_free(order); - return result; + BignumPointer order(BN_new()); + CHECK(order); + return EC_GROUP_get_order(group_, order.get(), nullptr) && + BN_cmp(private_key.get(), order.get()) < 0; } bool ECDH::IsKeyPairValid() { MarkPopErrorOnReturn mark_pop_error_on_return; USE(&mark_pop_error_on_return); - return 1 == EC_KEY_check_key(key_); + return 1 == EC_KEY_check_key(key_.get()); } @@ -4651,36 +4521,17 @@ class PBKDF2Request : public AsyncWrap { PBKDF2Request(Environment* env, Local object, const EVP_MD* digest, - int passlen, - char* pass, - int saltlen, - char* salt, - int iter, - int keylen) + MallocedBuffer&& pass, + MallocedBuffer&& salt, + int keylen, + int iteration_count) : AsyncWrap(env, object, AsyncWrap::PROVIDER_PBKDF2REQUEST), digest_(digest), success_(false), - passlen_(passlen), - pass_(pass), - saltlen_(saltlen), - salt_(salt), - keylen_(keylen), - key_(node::Malloc(keylen)), - iter_(iter) { - } - - ~PBKDF2Request() override { - free(pass_); - pass_ = nullptr; - passlen_ = 0; - - free(salt_); - salt_ = nullptr; - saltlen_ = 0; - - free(key_); - key_ = nullptr; - keylen_ = 0; + pass_(std::move(pass)), + salt_(std::move(salt)), + key_(keylen), + iteration_count_(iteration_count) { } uv_work_t* work_req() { @@ -4700,23 +4551,23 @@ class PBKDF2Request : public AsyncWrap { uv_work_t work_req_; const EVP_MD* digest_; bool success_; - int passlen_; - char* pass_; - int saltlen_; - char* salt_; - int keylen_; - char* key_; - int iter_; + MallocedBuffer pass_; + MallocedBuffer salt_; + MallocedBuffer key_; + int iteration_count_; }; void PBKDF2Request::Work() { success_ = PKCS5_PBKDF2_HMAC( - pass_, passlen_, reinterpret_cast(salt_), saltlen_, - iter_, digest_, keylen_, reinterpret_cast(key_)); - OPENSSL_cleanse(pass_, passlen_); - OPENSSL_cleanse(salt_, saltlen_); + pass_.data, pass_.size, + reinterpret_cast(salt_.data), salt_.size, + iteration_count_, digest_, + key_.size, + reinterpret_cast(key_.data)); + OPENSSL_cleanse(pass_.data, pass_.size); + OPENSSL_cleanse(salt_.data, salt_.size); } @@ -4729,9 +4580,8 @@ void PBKDF2Request::Work(uv_work_t* work_req) { void PBKDF2Request::After(Local (*argv)[2]) { if (success_) { (*argv)[0] = Null(env()->isolate()); - (*argv)[1] = Buffer::New(env(), key_, keylen_).ToLocalChecked(); - key_ = nullptr; - keylen_ = 0; + (*argv)[1] = Buffer::New(env(), key_.release(), key_.size) + .ToLocalChecked(); } else { (*argv)[0] = Exception::Error(env()->pbkdf2_error_string()); (*argv)[1] = Undefined(env()->isolate()); @@ -4760,37 +4610,27 @@ void PBKDF2(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); const EVP_MD* digest = nullptr; - char* pass = nullptr; - char* salt = nullptr; - int passlen = -1; - int saltlen = -1; - double raw_keylen = -1; int keylen = -1; - int iter = -1; + int iteration_count = -1; Local obj; - passlen = Buffer::Length(args[0]); + int passlen = Buffer::Length(args[0]); - pass = node::Malloc(passlen); - memcpy(pass, Buffer::Data(args[0]), passlen); + MallocedBuffer pass(passlen); + memcpy(pass.data, Buffer::Data(args[0]), passlen); - saltlen = Buffer::Length(args[1]); + int saltlen = Buffer::Length(args[1]); - salt = node::Malloc(saltlen); - memcpy(salt, Buffer::Data(args[1]), saltlen); + MallocedBuffer salt(saltlen); + memcpy(salt.data, Buffer::Data(args[1]), saltlen); - iter = args[2]->Int32Value(); - - raw_keylen = args[3]->NumberValue(); - - keylen = static_cast(raw_keylen); + iteration_count = args[2]->Int32Value(env->context()).FromJust(); + keylen = args[3]->IntegerValue(env->context()).FromJust(); if (args[4]->IsString()) { node::Utf8Value digest_name(env->isolate(), args[4]); digest = EVP_get_digestbyname(*digest_name); if (digest == nullptr) { - free(salt); - free(pass); args.GetReturnValue().Set(-1); return; } @@ -4803,8 +4643,11 @@ void PBKDF2(const FunctionCallbackInfo& args) { obj = env->pbkdf2_constructor_template()-> NewInstance(env->context()).ToLocalChecked(); std::unique_ptr req( - new PBKDF2Request(env, obj, digest, passlen, pass, saltlen, salt, iter, - keylen)); + new PBKDF2Request(env, obj, digest, + std::move(pass), + std::move(salt), + keylen, + iteration_count)); if (args[5]->IsFunction()) { obj->Set(env->context(), env->ondone_string(), args[5]).FromJust(); @@ -5047,14 +4890,14 @@ void RandomBytesBuffer(const FunctionCallbackInfo& args) { void GetSSLCiphers(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - SSL_CTX* ctx = SSL_CTX_new(TLS_method()); - CHECK_NE(ctx, nullptr); + SSLCtxPointer ctx(SSL_CTX_new(TLS_method())); + CHECK(ctx); - SSL* ssl = SSL_new(ctx); - CHECK_NE(ssl, nullptr); + SSLPointer ssl(SSL_new(ctx.get())); + CHECK(ssl); Local arr = Array::New(env->isolate()); - STACK_OF(SSL_CIPHER)* ciphers = SSL_get_ciphers(ssl); + STACK_OF(SSL_CIPHER)* ciphers = SSL_get_ciphers(ssl.get()); for (int i = 0; i < sk_SSL_CIPHER_num(ciphers); ++i) { const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i); @@ -5064,9 +4907,6 @@ void GetSSLCiphers(const FunctionCallbackInfo& args) { SSL_CIPHER_get_name(cipher))).FromJust(); } - SSL_free(ssl); - SSL_CTX_free(ctx); - args.GetReturnValue().Set(arr); } @@ -5117,12 +4957,11 @@ void GetCurves(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); const size_t num_curves = EC_get_builtin_curves(nullptr, 0); Local arr = Array::New(env->isolate(), num_curves); - EC_builtin_curve* curves; if (num_curves) { - curves = node::Malloc(num_curves); + std::vector curves(num_curves); - if (EC_get_builtin_curves(curves, num_curves)) { + if (EC_get_builtin_curves(curves.data(), num_curves)) { for (size_t i = 0; i < num_curves; i++) { arr->Set(env->context(), i, @@ -5130,8 +4969,6 @@ void GetCurves(const FunctionCallbackInfo& args) { OBJ_nid2sn(curves[i].nid))).FromJust(); } } - - free(curves); } args.GetReturnValue().Set(arr); @@ -5139,28 +4976,15 @@ void GetCurves(const FunctionCallbackInfo& args) { bool VerifySpkac(const char* data, unsigned int len) { - bool verify_result = false; - EVP_PKEY* pkey = nullptr; - NETSCAPE_SPKI* spki = nullptr; - - spki = NETSCAPE_SPKI_b64_decode(data, len); - if (spki == nullptr) - goto exit; - - pkey = X509_PUBKEY_get(spki->spkac->pubkey); - if (pkey == nullptr) - goto exit; - - verify_result = NETSCAPE_SPKI_verify(spki, pkey) > 0; - - exit: - if (pkey != nullptr) - EVP_PKEY_free(pkey); + NetscapeSPKIPointer spki(NETSCAPE_SPKI_b64_decode(data, len)); + if (!spki) + return false; - if (spki != nullptr) - NETSCAPE_SPKI_free(spki); + EVPKeyPointer pkey(X509_PUBKEY_get(spki->spkac->pubkey)); + if (!pkey) + return false; - return verify_result; + return NETSCAPE_SPKI_verify(spki.get(), pkey.get()) > 0; } @@ -5182,41 +5006,29 @@ void VerifySpkac(const FunctionCallbackInfo& args) { char* ExportPublicKey(const char* data, int len, size_t* size) { char* buf = nullptr; - EVP_PKEY* pkey = nullptr; - NETSCAPE_SPKI* spki = nullptr; - BIO* bio = BIO_new(BIO_s_mem()); - if (bio == nullptr) - goto exit; + BIOPointer bio(BIO_new(BIO_s_mem())); + if (!bio) + return nullptr; - spki = NETSCAPE_SPKI_b64_decode(data, len); - if (spki == nullptr) - goto exit; + NetscapeSPKIPointer spki(NETSCAPE_SPKI_b64_decode(data, len)); + if (!spki) + return nullptr; - pkey = NETSCAPE_SPKI_get_pubkey(spki); - if (pkey == nullptr) - goto exit; + EVPKeyPointer pkey(NETSCAPE_SPKI_get_pubkey(spki.get())); + if (!pkey) + return nullptr; - if (PEM_write_bio_PUBKEY(bio, pkey) <= 0) - goto exit; + if (PEM_write_bio_PUBKEY(bio.get(), pkey.get()) <= 0) + return nullptr; BUF_MEM* ptr; - BIO_get_mem_ptr(bio, &ptr); + BIO_get_mem_ptr(bio.get(), &ptr); *size = ptr->length; buf = Malloc(*size); memcpy(buf, ptr->data, *size); - exit: - if (pkey != nullptr) - EVP_PKEY_free(pkey); - - if (spki != nullptr) - NETSCAPE_SPKI_free(spki); - - if (bio != nullptr) - BIO_free_all(bio); - return buf; } @@ -5241,19 +5053,15 @@ void ExportPublicKey(const FunctionCallbackInfo& args) { } -const char* ExportChallenge(const char* data, int len) { - NETSCAPE_SPKI* sp = nullptr; - - sp = NETSCAPE_SPKI_b64_decode(data, len); - if (sp == nullptr) +OpenSSLBuffer ExportChallenge(const char* data, int len) { + NetscapeSPKIPointer sp(NETSCAPE_SPKI_b64_decode(data, len)); + if (!sp) return nullptr; unsigned char* buf = nullptr; ASN1_STRING_to_UTF8(&buf, sp->spkac->challenge); - NETSCAPE_SPKI_free(sp); - - return reinterpret_cast(buf); + return OpenSSLBuffer(reinterpret_cast(buf)); } @@ -5267,13 +5075,12 @@ void ExportChallenge(const FunctionCallbackInfo& args) { char* data = Buffer::Data(args[0]); CHECK_NE(data, nullptr); - const char* cert = ExportChallenge(data, len); - if (cert == nullptr) + OpenSSLBuffer cert = ExportChallenge(data, len); + if (!cert) return args.GetReturnValue().SetEmptyString(); - Local outString = Encode(env->isolate(), cert, strlen(cert), BUFFER); - - OPENSSL_free(const_cast(cert)); + Local outString = + Encode(env->isolate(), cert.get(), strlen(cert.get()), BUFFER); args.GetReturnValue().Set(outString); } @@ -5295,19 +5102,16 @@ void ConvertKey(const FunctionCallbackInfo& args) { if (nid == NID_undef) return env->ThrowTypeError("Invalid ECDH curve name"); - EC_GROUP* group = EC_GROUP_new_by_curve_name(nid); + ECGroupPointer group( + EC_GROUP_new_by_curve_name(nid)); if (group == nullptr) return env->ThrowError("Failed to get EC_GROUP"); - EC_POINT* pub = ECDH::BufferToPoint(env, - group, - Buffer::Data(args[0]), - len); - - std::shared_ptr cleanup(nullptr, [group, pub] (...) { - EC_GROUP_free(group); - EC_POINT_free(pub); - }); + ECPointPointer pub( + ECDH::BufferToPoint(env, + group.get(), + Buffer::Data(args[0]), + len)); if (pub == nullptr) return env->ThrowError("Failed to convert Buffer to EC_POINT"); @@ -5315,13 +5119,15 @@ void ConvertKey(const FunctionCallbackInfo& args) { point_conversion_form_t form = static_cast(args[2]->Uint32Value()); - int size = EC_POINT_point2oct(group, pub, form, nullptr, 0, nullptr); + int size = EC_POINT_point2oct( + group.get(), pub.get(), form, nullptr, 0, nullptr); + if (size == 0) return env->ThrowError("Failed to get public key length"); unsigned char* out = node::Malloc(size); - int r = EC_POINT_point2oct(group, pub, form, out, size, nullptr); + int r = EC_POINT_point2oct(group.get(), pub.get(), form, out, size, nullptr); if (r != size) { free(out); return env->ThrowError("Failed to get public key"); diff --git a/src/node_crypto.h b/src/node_crypto.h index b3b91b30396c4a..8b78d272df6149 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -75,6 +75,32 @@ struct MarkPopErrorOnReturn { ~MarkPopErrorOnReturn() { ERR_pop_to_mark(); } }; +template +struct FunctionDeleter { + void operator()(T* pointer) const { function(pointer); } + typedef std::unique_ptr Pointer; +}; + +template +using DeleteFnPtr = typename FunctionDeleter::Pointer; + +// Define smart pointers for the most commonly used OpenSSL types: +using X509Pointer = DeleteFnPtr; +using BIOPointer = DeleteFnPtr; +using SSLCtxPointer = DeleteFnPtr; +using SSLSessionPointer = DeleteFnPtr; +using SSLPointer = DeleteFnPtr; +using EVPKeyPointer = DeleteFnPtr; +using EVPKeyCtxPointer = DeleteFnPtr; +using EVPMDPointer = DeleteFnPtr; +using RSAPointer = DeleteFnPtr; +using BignumPointer = DeleteFnPtr; +using NetscapeSPKIPointer = DeleteFnPtr; +using ECGroupPointer = DeleteFnPtr; +using ECPointPointer = DeleteFnPtr; +using ECKeyPointer = DeleteFnPtr; +using DHPointer = DeleteFnPtr; + enum CheckResult { CHECK_CERT_REVOKED = 0, CHECK_OK = 1 @@ -87,14 +113,14 @@ extern void UseExtraCaCerts(const std::string& file); class SecureContext : public BaseObject { public: ~SecureContext() override { - FreeCTXMem(); + Reset(); } static void Initialize(Environment* env, v8::Local target); - SSL_CTX* ctx_; - X509* cert_; - X509* issuer_; + SSLCtxPointer ctx_; + X509Pointer cert_; + X509Pointer issuer_; #ifndef OPENSSL_NO_ENGINE bool client_cert_engine_provided_ = false; #endif // !OPENSSL_NO_ENGINE @@ -171,28 +197,16 @@ class SecureContext : public BaseObject { #endif SecureContext(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - ctx_(nullptr), - cert_(nullptr), - issuer_(nullptr) { + : BaseObject(env, wrap) { MakeWeak(); env->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize); } - void FreeCTXMem() { - if (!ctx_) { - return; - } - + inline void Reset() { env()->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize); - SSL_CTX_free(ctx_); - if (cert_ != nullptr) - X509_free(cert_); - if (issuer_ != nullptr) - X509_free(issuer_); - ctx_ = nullptr; - cert_ = nullptr; - issuer_ = nullptr; + ctx_.reset(); + cert_.reset(); + issuer_.reset(); } }; @@ -215,20 +229,15 @@ class SSLWrap { cert_cb_(nullptr), cert_cb_arg_(nullptr), cert_cb_running_(false) { - ssl_ = SSL_new(sc->ctx_); + ssl_.reset(SSL_new(sc->ctx_.get())); + CHECK(ssl_); env_->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize); - CHECK_NE(ssl_, nullptr); } virtual ~SSLWrap() { DestroySSL(); - if (next_sess_ != nullptr) { - SSL_SESSION_free(next_sess_); - next_sess_ = nullptr; - } } - inline SSL* ssl() const { return ssl_; } inline void enable_session_callbacks() { session_callbacks_ = true; } inline bool is_server() const { return kind_ == kServer; } inline bool is_client() const { return kind_ == kClient; } @@ -319,8 +328,8 @@ class SSLWrap { Environment* const env_; Kind kind_; - SSL_SESSION* next_sess_; - SSL* ssl_; + SSLSessionPointer next_sess_; + SSLPointer ssl_; bool session_callbacks_; bool new_session_wait_; @@ -344,10 +353,6 @@ class SSLWrap { class CipherBase : public BaseObject { public: - ~CipherBase() override { - EVP_CIPHER_CTX_free(ctx_); - } - static void Initialize(Environment* env, v8::Local target); protected: @@ -405,7 +410,7 @@ class CipherBase : public BaseObject { } private: - EVP_CIPHER_CTX* ctx_; + DeleteFnPtr ctx_; const CipherKind kind_; bool auth_tag_set_; unsigned int auth_tag_len_; @@ -416,8 +421,6 @@ class CipherBase : public BaseObject { class Hmac : public BaseObject { public: - ~Hmac() override; - static void Initialize(Environment* env, v8::Local target); protected: @@ -436,13 +439,11 @@ class Hmac : public BaseObject { } private: - HMAC_CTX* ctx_; + DeleteFnPtr ctx_; }; class Hash : public BaseObject { public: - ~Hash() override; - static void Initialize(Environment* env, v8::Local target); bool HashInit(const char* hash_type); @@ -461,7 +462,7 @@ class Hash : public BaseObject { } private: - EVP_MD_CTX* mdctx_; + EVPMDPointer mdctx_; bool finalized_; }; @@ -478,19 +479,16 @@ class SignBase : public BaseObject { } Error; SignBase(Environment* env, v8::Local wrap) - : BaseObject(env, wrap), - mdctx_(nullptr) { + : BaseObject(env, wrap) { } - ~SignBase() override; - Error Init(const char* sign_type); Error Update(const char* data, int len); protected: void CheckThrow(Error error); - EVP_MD_CTX* mdctx_; + EVPMDPointer mdctx_; }; class Sign : public SignBase { @@ -571,12 +569,6 @@ class PublicKeyCipher { class DiffieHellman : public BaseObject { public: - ~DiffieHellman() override { - if (dh != nullptr) { - DH_free(dh); - } - } - static void Initialize(Environment* env, v8::Local target); bool Init(int primeLength, int g); @@ -601,8 +593,7 @@ class DiffieHellman : public BaseObject { DiffieHellman(Environment* env, v8::Local wrap) : BaseObject(env, wrap), initialised_(false), - verifyError_(0), - dh(nullptr) { + verifyError_(0) { MakeWeak(); } @@ -616,29 +607,26 @@ class DiffieHellman : public BaseObject { bool initialised_; int verifyError_; - DH* dh; + DHPointer dh_; }; class ECDH : public BaseObject { public: ~ECDH() override { - if (key_ != nullptr) - EC_KEY_free(key_); - key_ = nullptr; group_ = nullptr; } static void Initialize(Environment* env, v8::Local target); - static EC_POINT* BufferToPoint(Environment* env, - const EC_GROUP* group, - char* data, - size_t len); + static ECPointPointer BufferToPoint(Environment* env, + const EC_GROUP* group, + char* data, + size_t len); protected: - ECDH(Environment* env, v8::Local wrap, EC_KEY* key) + ECDH(Environment* env, v8::Local wrap, ECKeyPointer&& key) : BaseObject(env, wrap), - key_(key), - group_(EC_KEY_get0_group(key_)) { + key_(std::move(key)), + group_(EC_KEY_get0_group(key_.get())) { MakeWeak(); CHECK_NE(group_, nullptr); } @@ -652,9 +640,9 @@ class ECDH : public BaseObject { static void SetPublicKey(const v8::FunctionCallbackInfo& args); bool IsKeyPairValid(); - bool IsKeyValidForCurve(const BIGNUM* private_key); + bool IsKeyValidForCurve(const BignumPointer& private_key); - EC_KEY* key_; + ECKeyPointer key_; const EC_GROUP* group_; }; diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 3764e2f3c93d4c..5d84a10da2e0b1 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -74,8 +74,10 @@ TLSWrap::TLSWrap(Environment* env, CHECK_NE(sc, nullptr); // We've our own session callbacks - SSL_CTX_sess_set_get_cb(sc_->ctx_, SSLWrap::GetSessionCallback); - SSL_CTX_sess_set_new_cb(sc_->ctx_, SSLWrap::NewSessionCallback); + SSL_CTX_sess_set_get_cb(sc_->ctx_.get(), + SSLWrap::GetSessionCallback); + SSL_CTX_sess_set_new_cb(sc_->ctx_.get(), + SSLWrap::NewSessionCallback); stream->PushStreamListener(this); @@ -116,35 +118,36 @@ void TLSWrap::InitSSL() { crypto::NodeBIO::FromBIO(enc_in_)->AssignEnvironment(env()); crypto::NodeBIO::FromBIO(enc_out_)->AssignEnvironment(env()); - SSL_set_bio(ssl_, enc_in_, enc_out_); + SSL_set_bio(ssl_.get(), enc_in_, enc_out_); // NOTE: This could be overridden in SetVerifyMode - SSL_set_verify(ssl_, SSL_VERIFY_NONE, crypto::VerifyCallback); + SSL_set_verify(ssl_.get(), SSL_VERIFY_NONE, crypto::VerifyCallback); #ifdef SSL_MODE_RELEASE_BUFFERS - long mode = SSL_get_mode(ssl_); // NOLINT(runtime/int) - SSL_set_mode(ssl_, mode | SSL_MODE_RELEASE_BUFFERS); + long mode = SSL_get_mode(ssl_.get()); // NOLINT(runtime/int) + SSL_set_mode(ssl_.get(), mode | SSL_MODE_RELEASE_BUFFERS); #endif // SSL_MODE_RELEASE_BUFFERS - SSL_set_app_data(ssl_, this); - SSL_set_info_callback(ssl_, SSLInfoCallback); + SSL_set_app_data(ssl_.get(), this); + SSL_set_info_callback(ssl_.get(), SSLInfoCallback); #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB if (is_server()) { - SSL_CTX_set_tlsext_servername_callback(sc_->ctx_, SelectSNIContextCallback); + SSL_CTX_set_tlsext_servername_callback(sc_->ctx_.get(), + SelectSNIContextCallback); } #endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB ConfigureSecureContext(sc_); - SSL_set_cert_cb(ssl_, SSLWrap::SSLCertCallback, this); + SSL_set_cert_cb(ssl_.get(), SSLWrap::SSLCertCallback, this); if (is_server()) { - SSL_set_accept_state(ssl_); + SSL_set_accept_state(ssl_.get()); } else if (is_client()) { // Enough space for server response (hello, cert) crypto::NodeBIO::FromBIO(enc_in_)->set_initial(kInitialClientBufferLength); - SSL_set_connect_state(ssl_); + SSL_set_connect_state(ssl_.get()); } else { // Unexpected ABORT(); @@ -342,7 +345,7 @@ Local TLSWrap::GetSSLError(int status, int* err, std::string* msg) { if (ssl_ == nullptr) return Local(); - *err = SSL_get_error(ssl_, status); + *err = SSL_get_error(ssl_.get(), status); switch (*err) { case SSL_ERROR_NONE: case SSL_ERROR_WANT_READ: @@ -395,7 +398,7 @@ void TLSWrap::ClearOut() { char out[kClearOutChunkSize]; int read; for (;;) { - read = SSL_read(ssl_, out, sizeof(out)); + read = SSL_read(ssl_.get(), out, sizeof(out)); if (read <= 0) break; @@ -421,7 +424,7 @@ void TLSWrap::ClearOut() { } } - int flags = SSL_get_shutdown(ssl_); + int flags = SSL_get_shutdown(ssl_.get()); if (!eof_ && flags & SSL_RECEIVED_SHUTDOWN) { eof_ = true; EmitRead(UV_EOF); @@ -469,7 +472,7 @@ bool TLSWrap::ClearIn() { for (i = 0; i < buffers.size(); ++i) { size_t avail = buffers[i].len; char* data = buffers[i].base; - written = SSL_write(ssl_, data, avail); + written = SSL_write(ssl_.get(), data, avail); CHECK(written == -1 || written == static_cast(avail)); if (written == -1) break; @@ -610,7 +613,7 @@ int TLSWrap::DoWrite(WriteWrap* w, int written = 0; for (i = 0; i < count; i++) { - written = SSL_write(ssl_, bufs[i].base, bufs[i].len); + written = SSL_write(ssl_.get(), bufs[i].base, bufs[i].len); CHECK(written == -1 || written == static_cast(bufs[i].len)); if (written == -1) break; @@ -690,8 +693,8 @@ ShutdownWrap* TLSWrap::CreateShutdownWrap(Local req_wrap_object) { int TLSWrap::DoShutdown(ShutdownWrap* req_wrap) { crypto::MarkPopErrorOnReturn mark_pop_error_on_return; - if (ssl_ != nullptr && SSL_shutdown(ssl_) == 0) - SSL_shutdown(ssl_); + if (ssl_ && SSL_shutdown(ssl_.get()) == 0) + SSL_shutdown(ssl_.get()); shutdown_ = true; EncOut(); @@ -726,7 +729,7 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo& args) { } // Always allow a connection. We'll reject in javascript. - SSL_set_verify(wrap->ssl_, verify_mode, crypto::VerifyCallback); + SSL_set_verify(wrap->ssl_.get(), verify_mode, crypto::VerifyCallback); } @@ -783,7 +786,7 @@ void TLSWrap::GetServername(const FunctionCallbackInfo& args) { CHECK_NE(wrap->ssl_, nullptr); - const char* servername = SSL_get_servername(wrap->ssl_, + const char* servername = SSL_get_servername(wrap->ssl_.get(), TLSEXT_NAMETYPE_host_name); if (servername != nullptr) { args.GetReturnValue().Set(OneByteString(env->isolate(), servername)); @@ -808,7 +811,7 @@ void TLSWrap::SetServername(const FunctionCallbackInfo& args) { #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB node::Utf8Value servername(env->isolate(), args[0].As()); - SSL_set_tlsext_host_name(wrap->ssl_, *servername); + SSL_set_tlsext_host_name(wrap->ssl_.get(), *servername); #endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB } diff --git a/src/util.h b/src/util.h index 133899008fb7ce..7a1c6c109fdbff 100644 --- a/src/util.h +++ b/src/util.h @@ -410,7 +410,6 @@ class BufferValue : public MaybeStackBuffer { // Use this when a variable or parameter is unused in order to explicitly // silence a compiler warning about that. template inline void USE(T&&) {} -} // namespace node // Run a function when exiting the current scope. struct OnScopeLeave { @@ -420,6 +419,37 @@ struct OnScopeLeave { ~OnScopeLeave() { fn_(); } }; +// Simple RAII wrapper for contiguous data that uses malloc()/free(). +template +struct MallocedBuffer { + T* data; + size_t size; + + T* release() { + T* ret = data; + data = nullptr; + return ret; + } + + MallocedBuffer() : data(nullptr) {} + explicit MallocedBuffer(size_t size) : data(Malloc(size)), size(size) {} + MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) { + other.data = nullptr; + } + MallocedBuffer& operator=(MallocedBuffer&& other) { + this->~MallocedBuffer(); + return *new(this) MallocedBuffer(other); + } + ~MallocedBuffer() { + free(data); + } + MallocedBuffer(const MallocedBuffer&) = delete; + MallocedBuffer& operator=(const MallocedBuffer&) = delete; +}; + +} // namespace node + + #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_UTIL_H_