From 6e65f26b73c67869a3ac5ee2caf0dfd011177d66 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 29 Jul 2020 14:19:25 -0700 Subject: [PATCH] quic: use QuicCallbackScope consistently for QuicSession PR-URL: https://github.com/nodejs/node/pull/34541 Reviewed-By: Anna Henningsen Reviewed-By: Jiawen Geng --- lib/internal/quic/core.js | 9 +- src/quic/node_quic_session.cc | 277 +++++++++++++++++++++++----------- 2 files changed, 190 insertions(+), 96 deletions(-) diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index 3a2bddb8278997..47269ec00aa23e 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -2287,12 +2287,11 @@ class QuicServerSession extends QuicSession { async [kHandleOcsp](servername) { const internalState = this[kInternalState]; const { context } = this[kInternalServerState]; - const certificate = context?.context.getCertificate?.(); - const issuer = context?.context.getIssuer?.(); - return internalState.ocspHandler?.('request', { + if (!internalState.ocspHandler || !context) return undefined; + return internalState.ocspHandler('request', { servername, - certificate, - issuer + certificate: context.context.getCertificate(), + issuer: context.context.getIssuer() }); } diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index c306226f0c8913..f458fcabbaea46 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -398,14 +398,24 @@ void JSQuicSessionListener::OnKeylog(const char* line, size_t len) { HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); - Local line_bf = Buffer::Copy(env, line, 1 + len).ToLocalChecked(); - char* data = Buffer::Data(line_bf); + + QuicCallbackScope cb_scope(session()); + + Local line_buf; + if (!Buffer::Copy(env, line, 1 + len).ToLocal(&line_buf)) + return; + + char* data = Buffer::Data(line_buf); data[len] = '\n'; // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback(env->quic_on_session_keylog_function(), 1, &line_bf); + USE(env->quic_on_session_keylog_function()->Call( + env->context(), + session()->object(), + 1, + &line_buf)); } void JSQuicSessionListener::OnStreamBlocked(int64_t stream_id) { @@ -413,8 +423,14 @@ void JSQuicSessionListener::OnStreamBlocked(int64_t stream_id) { HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); + + QuicCallbackScope cb_scope(session()); + BaseObjectPtr stream = session()->FindStream(stream_id); - stream->MakeCallback(env->quic_on_stream_blocked_function(), 0, nullptr); + USE(env->quic_on_stream_blocked_function()->Call( + env->context(), + stream->object(), + 0, nullptr)); } void JSQuicSessionListener::OnClientHello( @@ -435,26 +451,32 @@ void JSQuicSessionListener::OnClientHello( Local alpn_string = Undefined(env->isolate()); Local servername = Undefined(env->isolate()); - if (session()->crypto_context()->hello_ciphers().ToLocal(&ciphers) && - (alpn == nullptr || - String::NewFromUtf8(env->isolate(), alpn).ToLocal(&alpn_string)) && - (server_name == nullptr || - String::NewFromUtf8(env->isolate(), server_name).ToLocal(&servername))) { - Local argv[] = { - alpn_string, - servername, - ciphers - }; - - // Grab a shared pointer to this to prevent the QuicSession - // from being freed while the MakeCallback is running. - BaseObjectPtr ptr(session()); - env->quic_on_session_client_hello_function()->Call( - env->context(), - session()->object(), - arraysize(argv), - argv); + if (!session()->crypto_context()->hello_ciphers().ToLocal(&ciphers) || + (alpn != nullptr && + !String::NewFromUtf8( + env->isolate(), + alpn).ToLocal(&alpn_string)) || + (server_name != nullptr && + !String::NewFromUtf8( + env->isolate(), + server_name).ToLocal(&servername))) { + return; } + + Local argv[] = { + alpn_string, + servername, + ciphers + }; + + // Grab a shared pointer to this to prevent the QuicSession + // from being freed while the MakeCallback is running. + BaseObjectPtr ptr(session()); + USE(env->quic_on_session_client_hello_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnCert(const char* server_name) { @@ -465,19 +487,21 @@ void JSQuicSessionListener::OnCert(const char* server_name) { QuicCallbackScope cb_scope(session()); Local servername = Undefined(env->isolate()); - - BaseObjectPtr ptr(session()); - if (server_name == nullptr || - String::NewFromUtf8( + if (UNLIKELY(server_name != nullptr && + !String::NewFromUtf8( env->isolate(), server_name, v8::NewStringType::kNormal, - strlen(server_name)).ToLocal(&servername)) { - env->quic_on_session_cert_function()->Call( - env->context(), - session()->object(), - 1, &servername); + strlen(server_name)).ToLocal(&servername))) { + return; } + + BaseObjectPtr ptr(session()); + + USE(env->quic_on_session_cert_function()->Call( + env->context(), + session()->object(), + 1, &servername)); } void JSQuicSessionListener::OnStreamHeaders( @@ -490,35 +514,50 @@ void JSQuicSessionListener::OnStreamHeaders( Context::Scope context_scope(env->context()); MaybeStackBuffer, 16> head(headers.size()); size_t n = 0; + + QuicCallbackScope cb_scope(session()); + for (const auto& header : headers) { - // name and value should never be empty here, and if - // they are, there's an actual bug so go ahead and crash - Local pair[] = { - header->GetName(session()->application()).ToLocalChecked(), - header->GetValue(session()->application()).ToLocalChecked() - }; + Local pair[2]; + + if (UNLIKELY(!header->GetName(session()->application()).ToLocal(&pair[0]))) + return; + + if (UNLIKELY(!header->GetValue(session()->application()).ToLocal(&pair[1]))) + return; + head[n++] = Array::New(env->isolate(), pair, arraysize(pair)); } + Local argv[] = { Number::New(env->isolate(), static_cast(stream_id)), Array::New(env->isolate(), head.out(), n), Integer::New(env->isolate(), kind), Undefined(env->isolate()) }; + if (kind == QUICSTREAM_HEADERS_KIND_PUSH) argv[3] = Number::New(env->isolate(), static_cast(push_id)); + BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_stream_headers_function(), - arraysize(argv), argv); + + USE(env->quic_on_stream_headers_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnOCSP(Local ocsp) { Environment* env = session()->env(); HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); + QuicCallbackScope cb_scope(session()); BaseObjectPtr ptr(session()); - session()->MakeCallback(env->quic_on_session_status_function(), 1, &ocsp); + USE(env->quic_on_session_status_function()->Call( + env->context(), + session()->object(), + 1, &ocsp)); } void JSQuicSessionListener::OnStreamClose( @@ -528,6 +567,8 @@ void JSQuicSessionListener::OnStreamClose( HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); + QuicCallbackScope cb_scope(session()); + Local argv[] = { Number::New(env->isolate(), static_cast(stream_id)), Number::New(env->isolate(), static_cast(app_error_code)) @@ -536,10 +577,12 @@ void JSQuicSessionListener::OnStreamClose( // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_stream_close_function(), + + USE(env->quic_on_stream_close_function()->Call( + env->context(), + session()->object(), arraysize(argv), - argv); + argv)); } void JSQuicSessionListener::OnStreamReset( @@ -549,6 +592,8 @@ void JSQuicSessionListener::OnStreamReset( HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); + QuicCallbackScope cb_scope(session()); + Local argv[] = { Number::New(env->isolate(), static_cast(stream_id)), Number::New(env->isolate(), static_cast(app_error_code)) @@ -556,10 +601,12 @@ void JSQuicSessionListener::OnStreamReset( // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_stream_reset_function(), + + USE(env->quic_on_stream_reset_function()->Call( + env->context(), + session()->object(), arraysize(argv), - argv); + argv)); } void JSQuicSessionListener::OnSessionClose(QuicError error, int flags) { @@ -567,6 +614,8 @@ void JSQuicSessionListener::OnSessionClose(QuicError error, int flags) { HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); + QuicCallbackScope cb_scope(session()); + Local argv[] = { Number::New(env->isolate(), static_cast(error.code)), Integer::New(env->isolate(), error.family), @@ -581,15 +630,20 @@ void JSQuicSessionListener::OnSessionClose(QuicError error, int flags) { // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_session_close_function(), - arraysize(argv), argv); + USE(env->quic_on_session_close_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnStreamReady(BaseObjectPtr stream) { Environment* env = session()->env(); HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); + + QuicCallbackScope cb_scope(session()); + Local argv[] = { stream->object(), Number::New(env->isolate(), static_cast(stream->id())), @@ -599,9 +653,12 @@ void JSQuicSessionListener::OnStreamReady(BaseObjectPtr stream) { // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_stream_ready_function(), - arraysize(argv), argv); + + USE(env->quic_on_stream_ready_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnHandshakeCompleted() { @@ -610,30 +667,40 @@ void JSQuicSessionListener::OnHandshakeCompleted() { Context::Scope context_scope(env->context()); QuicCryptoContext* ctx = session()->crypto_context(); + + QuicCallbackScope cb_scope(session()); + Local servername = Undefined(env->isolate()); + Local validationErrorReason = v8::Null(env->isolate()); + Local validationErrorCode = v8::Null(env->isolate()); + Local cipher_name; + Local cipher_version; + const char* hostname = ctx->servername(); - if (hostname != nullptr) { - servername = - String::NewFromUtf8(env->isolate(), hostname).ToLocalChecked(); + if (hostname != nullptr && + !String::NewFromUtf8(env->isolate(), hostname).ToLocal(&servername)) { + return; + } + + if (!ctx->cipher_name().ToLocal(&cipher_name) || + !ctx->cipher_version().ToLocal(&cipher_version)) { + return; } - // TODO(@jasnell): Find a more graceful way of handling the - // possible ToLocal failures more gracefully. - Local validationErrorReason = v8::Null(env->isolate()); - Local validationErrorCode = v8::Null(env->isolate()); int err = ctx->VerifyPeerIdentity(); - if (err != X509_V_OK) { - CHECK(crypto::GetValidationErrorReason(env, err) - .ToLocal(&validationErrorReason)); - CHECK(crypto::GetValidationErrorCode(env, err) - .ToLocal(&validationErrorCode)); + if (err != X509_V_OK && + (!crypto::GetValidationErrorReason(env, err) + .ToLocal(&validationErrorReason) || + !crypto::GetValidationErrorCode(env, err) + .ToLocal(&validationErrorCode))) { + return; } Local argv[] = { servername, GetALPNProtocol(*session()), - ctx->cipher_name().ToLocalChecked(), - ctx->cipher_version().ToLocalChecked(), + cipher_name, + cipher_version, Integer::New(env->isolate(), session()->max_pktlen_), validationErrorReason, validationErrorCode, @@ -645,10 +712,12 @@ void JSQuicSessionListener::OnHandshakeCompleted() { // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_session_handshake_function(), + + USE(env->quic_on_session_handshake_function()->Call( + env->context(), + session()->object(), arraysize(argv), - argv); + argv)); } void JSQuicSessionListener::OnPathValidation( @@ -662,6 +731,9 @@ void JSQuicSessionListener::OnPathValidation( HandleScope scope(env->isolate()); Local context = env->context(); Context::Scope context_scope(context); + + QuicCallbackScope cb_scope(session()); + Local argv[] = { Integer::New(env->isolate(), res), AddressToJS(env, local), @@ -670,10 +742,12 @@ void JSQuicSessionListener::OnPathValidation( // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_session_path_validation_function(), + + USE(env->quic_on_session_path_validation_function()->Call( + env->context(), + session()->object(), arraysize(argv), - argv); + argv)); } void JSQuicSessionListener::OnSessionTicket(int size, SSL_SESSION* sess) { @@ -681,6 +755,8 @@ void JSQuicSessionListener::OnSessionTicket(int size, SSL_SESSION* sess) { HandleScope scope(env->isolate()); Context::Scope context_scope(env->context()); + QuicCallbackScope cb_scope(session()); + Local argv[] = { v8::Undefined(env->isolate()), v8::Undefined(env->isolate()) @@ -692,22 +768,28 @@ void JSQuicSessionListener::OnSessionTicket(int size, SSL_SESSION* sess) { unsigned char* session_data = reinterpret_cast(session_ticket.data()); memset(session_data, 0, size); - if (i2d_SSL_SESSION(sess, &session_data) > 0) - argv[0] = session_ticket.ToBuffer().ToLocalChecked(); + if (i2d_SSL_SESSION(sess, &session_data) > 0 && + !session_ticket.ToBuffer().ToLocal(&argv[0])) { + return; + } } - if (session()->is_transport_params_set()) { - argv[1] = Buffer::Copy( - env, - reinterpret_cast(&session()->transport_params_), - sizeof(session()->transport_params_)).ToLocalChecked(); + if (session()->is_transport_params_set() && + !Buffer::Copy(env, + reinterpret_cast(&session()->transport_params_), + sizeof(session()->transport_params_)).ToLocal(&argv[1])) { + return; } + // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_session_ticket_function(), - arraysize(argv), argv); + + USE(env->quic_on_session_ticket_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnUsePreferredAddress( @@ -718,6 +800,8 @@ void JSQuicSessionListener::OnUsePreferredAddress( Local context = env->context(); Context::Scope context_scope(context); + QuicCallbackScope cb_scope(session()); + std::string hostname = family == AF_INET ? preferred_address.ipv4_address(): preferred_address.ipv6_address(); @@ -733,9 +817,12 @@ void JSQuicSessionListener::OnUsePreferredAddress( }; BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_session_use_preferred_address_function(), - arraysize(argv), argv); + + USE(env->quic_on_session_use_preferred_address_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnVersionNegotiation( @@ -747,6 +834,8 @@ void JSQuicSessionListener::OnVersionNegotiation( Local context = env->context(); Context::Scope context_scope(context); + QuicCallbackScope cb_scope(session()); + MaybeStackBuffer, 4> versions(vcnt); for (size_t n = 0; n < vcnt; n++) versions[n] = Integer::New(env->isolate(), vers[n]); @@ -766,9 +855,11 @@ void JSQuicSessionListener::OnVersionNegotiation( // Grab a shared pointer to this to prevent the QuicSession // from being freed while the MakeCallback is running. BaseObjectPtr ptr(session()); - session()->MakeCallback( - env->quic_on_session_version_negotiation_function(), - arraysize(argv), argv); + USE(env->quic_on_session_version_negotiation_function()->Call( + env->context(), + session()->object(), + arraysize(argv), + argv)); } void JSQuicSessionListener::OnQLog(QLogStream* qlog_stream) { @@ -776,8 +867,12 @@ void JSQuicSessionListener::OnQLog(QLogStream* qlog_stream) { Environment* env = session()->env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); + QuicCallbackScope cb_scope(session()); Local obj = qlog_stream->object(); - session()->MakeCallback(env->quic_on_session_qlog_function(), 1, &obj); + USE(env->quic_on_session_qlog_function()->Call( + env->context(), + session()->object(), + 1, &obj)); } // Generates a new random connection ID.