Skip to content

Commit

Permalink
quic: introduce QuicCallbackScope
Browse files Browse the repository at this point in the history
Alternative to `CallbackScope` that handles destroying the
`QuicSession` in the try_catch cleanup.

PR-URL: #34541
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
  • Loading branch information
jasnell authored and gengjiawen committed Aug 3, 2020
1 parent 4b0275a commit d96083b
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 35 deletions.
14 changes: 12 additions & 2 deletions src/quic/node_quic_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,14 @@ Local<Value> GetALPNProtocol(const QuicSession& session) {
namespace {
int CertCB(SSL* ssl, void* arg) {
QuicSession* session = static_cast<QuicSession*>(arg);
return SSL_get_tlsext_status_type(ssl) == TLSEXT_STATUSTYPE_ocsp ?
session->crypto_context()->OnOCSP() : 1;
int ret;
switch (SSL_get_tlsext_status_type(ssl)) {
case TLSEXT_STATUSTYPE_ocsp:
ret = session->crypto_context()->OnOCSP();
return UNLIKELY(session->is_destroyed()) ? 0 : ret;
default:
return 1;
}
}

void Keylog_CB(const SSL* ssl, const char* line) {
Expand All @@ -366,6 +372,10 @@ int Client_Hello_CB(
void* arg) {
QuicSession* session = static_cast<QuicSession*>(SSL_get_app_data(ssl));
int ret = session->crypto_context()->OnClientHello();
if (UNLIKELY(session->is_destroyed())) {
*tls_alert = SSL_R_SSL_HANDSHAKE_FAILURE;
return 0;
}
switch (ret) {
case 0:
return 1;
Expand Down
105 changes: 72 additions & 33 deletions src/quic/node_quic_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,40 @@ using v8::String;
using v8::Undefined;
using v8::Value;

using TryCatchScope = node::errors::TryCatchScope;

namespace quic {

QuicCallbackScope::QuicCallbackScope(QuicSession* session)
: session_(session),
private_(new InternalCallbackScope(
session->env(),
session->object(),
{
session->get_async_id(),
session->get_trigger_async_id()
})),
try_catch_(session->env()->isolate()) {
try_catch_.SetVerbose(true);
}

QuicCallbackScope::~QuicCallbackScope() {
Environment* env = session_->env();
if (UNLIKELY(try_catch_.HasCaught())) {
session_->crypto_context()->set_in_client_hello(false);
session_->crypto_context()->set_in_ocsp_request(false);
if (!try_catch_.HasTerminated() && env->can_call_into_js()) {
session_->set_last_error({
QUIC_ERROR_SESSION,
uint64_t{NGTCP2_INTERNAL_ERROR}
});
session_->Close();
CHECK(session_->is_destroyed());
}
private_->MarkAsFailed();
}
}

typedef ssize_t(*ngtcp2_close_fn)(
ngtcp2_conn* conn,
ngtcp2_path* path,
Expand Down Expand Up @@ -393,53 +425,59 @@ void JSQuicSessionListener::OnClientHello(
HandleScope scope(env->isolate());
Context::Scope context_scope(env->context());

// Why this instead of using MakeCallback? We need to catch any
// errors that happen both when preparing the arguments and
// invoking the callback so that we can properly signal a failure
// to the peer.
QuicCallbackScope cb_scope(session());

Local<Array> ciphers;
Local<Value> alpn_string = Undefined(env->isolate());
Local<Value> servername = Undefined(env->isolate());

// TODO(@jasnell): Need to decide how to handle the possible
// ToLocal failures more gracefully than crashing.

CHECK(session()->crypto_context()->hello_ciphers().ToLocal(&ciphers));

if (alpn != nullptr)
CHECK(String::NewFromUtf8(env->isolate(), alpn).ToLocal(&alpn_string));

if (server_name != nullptr)
CHECK(String::NewFromUtf8(env->isolate(), server_name)
.ToLocal(&servername));

Local<Value> argv[] = {
alpn_string,
servername,
ciphers
};
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<Value> argv[] = {
alpn_string,
servername,
ciphers
};

// Grab a shared pointer to this to prevent the QuicSession
// from being freed while the MakeCallback is running.
BaseObjectPtr<QuicSession> ptr(session());
session()->MakeCallback(
env->quic_on_session_client_hello_function(),
arraysize(argv), argv);
// Grab a shared pointer to this to prevent the QuicSession
// from being freed while the MakeCallback is running.
BaseObjectPtr<QuicSession> ptr(session());
env->quic_on_session_client_hello_function()->Call(
env->context(),
session()->object(),
arraysize(argv),
argv);
}
}

void JSQuicSessionListener::OnCert(const char* server_name) {
Environment* env = session()->env();
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());

QuicCallbackScope cb_scope(session());

Local<Value> servername = Undefined(env->isolate());
if (server_name != nullptr) {
servername = OneByteString(
env->isolate(),
server_name,
strlen(server_name));
}

// Grab a shared pointer to this to prevent the QuicSession
// from being freed while the MakeCallback is running.
BaseObjectPtr<QuicSession> ptr(session());
session()->MakeCallback(env->quic_on_session_cert_function(), 1, &servername);
if (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);
}
}

void JSQuicSessionListener::OnStreamHeaders(
Expand Down Expand Up @@ -2913,11 +2951,12 @@ int QuicSession::OnReceiveCryptoData(
return NGTCP2_ERR_CALLBACK_FAILURE;

QuicSession::NgCallbackScope callback_scope(session);
return session->crypto_context()->Receive(
int ret = session->crypto_context()->Receive(
crypto_level,
offset,
data,
datalen);
return ret == 0 ? 0 : NGTCP2_ERR_CALLBACK_FAILURE;
}

// Called by ngtcp2 for both client and server connections
Expand Down
16 changes: 16 additions & 0 deletions src/quic/node_quic_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -1507,6 +1507,22 @@ class QuicSession final : public AsyncWrap,
friend class JSQuicSessionListener;
};

class QuicCallbackScope {
public:
explicit QuicCallbackScope(QuicSession* session);
~QuicCallbackScope();

void operator=(const QuicCallbackScope&) = delete;
void operator=(QuicCallbackScope&&) = delete;
QuicCallbackScope(const QuicCallbackScope&) = delete;
QuicCallbackScope(QuicCallbackScope&&) = delete;

private:
BaseObjectPtr<QuicSession> session_;
std::unique_ptr<InternalCallbackScope> private_;
v8::TryCatch try_catch_;
};

} // namespace quic
} // namespace node

Expand Down

0 comments on commit d96083b

Please sign in to comment.