From 4b0275ab87f3cf60eb990300a5bc2cc05d5f38d5 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Tue, 28 Jul 2020 09:47:28 -0700
Subject: [PATCH] quic: refactor clientHello

Refactor the `'clientHello'` event into a `clientHelloHandler`
configuration option and async function. Remove the addContext
API as it's not needed.

PR-URL: https://github.com/nodejs/node/pull/34541
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
---
 doc/api/quic.md                          |  68 +++++++-------
 lib/internal/quic/core.js                | 108 +++++++++--------------
 lib/internal/quic/util.js                |  14 ++-
 src/quic/node_quic_session-inl.h         |  11 ---
 src/quic/node_quic_session.cc            |  97 +++++++++++---------
 src/quic/node_quic_session.h             |   6 +-
 test/parallel/test-quic-client-server.js |  32 +++----
 7 files changed, 159 insertions(+), 177 deletions(-)

diff --git a/doc/api/quic.md b/doc/api/quic.md
index 9a2d94080cde71..897f39245c2200 100644
--- a/doc/api/quic.md
+++ b/doc/api/quic.md
@@ -1274,38 +1274,6 @@ The `QuicServerSession` class implements the server side of a QUIC connection.
 Instances are created internally and are emitted using the `QuicSocket`
 `'session'` event.
 
-#### Event: `'clientHello'`
-<!-- YAML
-added: REPLACEME
--->
-
-Emitted at the start of the TLS handshake when the `QuicServerSession` receives
-the initial TLS Client Hello.
-
-The event handler is given a callback function that *must* be invoked for the
-handshake to continue.
-
-The callback is invoked with four arguments:
-
-* `alpn` {string} The ALPN protocol identifier requested by the client.
-* `servername` {string} The SNI servername requested by the client.
-* `ciphers` {string[]} The list of TLS cipher algorithms requested by the
-  client.
-* `callback` {Function} A callback function that must be called in order for
-  the TLS handshake to continue.
-
-The `'clientHello'` event will not be emitted more than once.
-
-#### `quicserversession.addContext(servername\[, context\])`
-<!-- YAML
-added: REPLACEME
--->
-
-* `servername` {string} A DNS name to associate with the given context.
-* `context` {tls.SecureContext} A TLS SecureContext to associate with the `servername`.
-
-TBD
-
 ### Class: `QuicSocket`
 <!-- YAML
 added: REPLACEME
@@ -1766,6 +1734,9 @@ added: REPLACEME
     uppercased in order for OpenSSL to accept them.
   * `clientCertEngine` {string} Name of an OpenSSL engine which can provide the
     client certificate.
+  * `clientHelloHandler` {Function} An async function that may be used to
+    set a {tls.SecureContext} for the given server name at the start of the
+    TLS handshake. See [Handling client hello][] for details.
   * `crl` {string|string[]|Buffer|Buffer[]} PEM formatted CRLs (Certificate
     Revocation Lists).
   * `defaultEncoding` {string} The default encoding that is used when no
@@ -2479,6 +2450,38 @@ async function ocspClientHandler(type, { data }) {
 sock.connect({ ocspHandler: ocspClientHandler });
 ```
 
+### Handling client hello
+
+When `quicsocket.listen()` is called, a {tls.SecureContext} is created and used
+by default for all new `QuicServerSession` instances. There are times, however,
+when the {tls.SecureContext} to be used for a `QuicSession` can only be
+determined once the client initiates a connection. This is accomplished using
+the `clientHelloHandler` option when calling `quicsocket.listen()`.
+
+The value of `clientHelloHandler` is an async function that is called at the
+start of a new `QuicServerSession`. It is invoked with three arguments:
+
+* `alpn` {string} The ALPN protocol identifier specified by the client.
+* `servername` {string} The SNI server name specified by the client.
+* `ciphers` {string[]} The array of TLS 1.3 ciphers specified by the client.
+
+The `clientHelloHandler` *may* return a new {tls.SecureContext} object that will
+be used to continue the TLS handshake. If the function returns `undefined`, the
+default {tls.SecureContext} will be used. Returning any other value will cause
+an error to be thrown that will destroy the `QuicServerSession` instance.
+
+```js
+const server = createQuicSocket();
+
+server.listen({
+  async clientHelloHandler(alpn, servername, ciphers) {
+    console.log(alpn);
+    console.log(servername);
+    console.log(ciphers);
+  }
+});
+```
+
 [`crypto.getCurves()`]: crypto.html#crypto_crypto_getcurves
 [`stream.Readable`]: #stream_class_stream_readable
 [`tls.DEFAULT_ECDH_CURVE`]: #tls_tls_default_ecdh_curve
@@ -2487,6 +2490,7 @@ sock.connect({ ocspHandler: ocspClientHandler });
 [RFC 4007]: https://tools.ietf.org/html/rfc4007
 [Certificate Object]: https://nodejs.org/dist/latest-v12.x/docs/api/tls.html#tls_certificate_object
 [custom DNS lookup function]: #quic_custom_dns_lookup_functions
+[Handling client hello]: #quic_handling_client_hello
 [modifying the default cipher suite]: tls.html#tls_modifying_the_default_tls_cipher_suite
 [OCSP requests]: #quic_online_certificate_status_protocol_ocsp
 [OCSP responses]: #quic_online_certificate_status_protocol_ocsp
diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js
index 821b2839fe9375..3a2bddb8278997 100644
--- a/lib/internal/quic/core.js
+++ b/lib/internal/quic/core.js
@@ -20,7 +20,6 @@ const {
   PromiseAll,
   PromiseReject,
   PromiseResolve,
-  RegExp,
   Set,
   Symbol,
 } = primordials;
@@ -200,7 +199,6 @@ const {
   validateBoolean,
   validateInteger,
   validateObject,
-  validateString,
 } = require('internal/validators');
 
 const emit = EventEmitter.prototype.emit;
@@ -297,30 +295,18 @@ function onSessionClose(code, family, silent, statelessReset) {
   this[owner_symbol][kDestroy](code, family, silent, statelessReset);
 }
 
-// Used only within the onSessionClientHello function. Invoked
-// to complete the client hello process.
-function clientHelloCallback(err, ...args) {
-  if (err) {
-    this[owner_symbol].destroy(err);
-    return;
-  }
-  try {
-    this.onClientHelloDone(...args);
-  } catch (err) {
-    this[owner_symbol].destroy(err);
-  }
-}
-
 // This callback is invoked at the start of the TLS handshake to provide
 // some basic information about the ALPN, SNI, and Ciphers that are
-// being requested. It is only called if the 'clientHello' event is
-// listened for.
+// being requested. It is only called if the 'clientHelloHandler' option is
+// specified on listen().
 function onSessionClientHello(alpn, servername, ciphers) {
-  this[owner_symbol][kClientHello](
-    alpn,
-    servername,
-    ciphers,
-    clientHelloCallback.bind(this));
+  this[owner_symbol][kClientHello](alpn, servername, ciphers)
+    .then((context) => {
+      if (context !== undefined && !context?.context)
+        throw new ERR_INVALID_ARG_TYPE('context', 'SecureContext', context);
+      this.onClientHelloDone(context?.context);
+    })
+    .catch((error) => this[owner_symbol].destroy(error));
 }
 
 // This callback is only ever invoked for QuicServerSession instances,
@@ -329,9 +315,7 @@ function onSessionClientHello(alpn, servername, ciphers) {
 // TLS handshake to continue.
 function onSessionCert(servername) {
   this[owner_symbol][kHandleOcsp](servername)
-    .then(({ context, data }) => {
-      if (context !== undefined && !context?.context)
-        throw new ERR_INVALID_ARG_TYPE('context', 'SecureContext', context);
+    .then((data) => {
       if (data !== undefined) {
         if (typeof data === 'string')
           data = Buffer.from(data);
@@ -342,7 +326,7 @@ function onSessionCert(servername) {
             data);
         }
       }
-      this.onCertDone(context?.context, data);
+      this.onCertDone(data);
     })
     .catch((error) => this[owner_symbol].destroy(error));
 }
@@ -919,6 +903,7 @@ class QuicSocket extends EventEmitter {
     listenPromise: undefined,
     lookup: undefined,
     ocspHandler: undefined,
+    clientHelloHandler: undefined,
     server: undefined,
     serverSecureContext: undefined,
     sessions: new Set(),
@@ -1010,15 +995,14 @@ class QuicSocket extends EventEmitter {
     this.destroy(err);
   }
 
-  // Returns the default QuicStream options for peer-initiated
-  // streams. These are passed on to new client and server
-  // QuicSession instances when they are created.
   get [kStreamOptions]() {
     const state = this[kInternalState];
     return {
       highWaterMark: state.highWaterMark,
       defaultEncoding: state.defaultEncoding,
       ocspHandler: state.ocspHandler,
+      clientHelloHandler: state.clientHelloHandler,
+      context: state.serverSecureContext,
     };
   }
 
@@ -1212,11 +1196,10 @@ class QuicSocket extends EventEmitter {
       defaultEncoding,
       highWaterMark,
       ocspHandler,
+      clientHelloHandler,
       transportParams,
     } = validateQuicSocketListenOptions(options);
 
-    // Store the secure context so that it is not garbage collected
-    // while we still need to make use of it.
     state.serverSecureContext =
       createSecureContext({
         ...options,
@@ -1228,6 +1211,7 @@ class QuicSocket extends EventEmitter {
     state.alpn = alpn;
     state.listenPending = true;
     state.ocspHandler = ocspHandler;
+    state.clientHelloHandler = clientHelloHandler;
 
     await this[kMaybeBind]();
 
@@ -1484,9 +1468,6 @@ class QuicSocket extends EventEmitter {
     return Array.from(this[kInternalState].endpoints);
   }
 
-  // The sever secure context is the SecureContext specified when calling
-  // listen. It is the context that will be used with all new server
-  // QuicSession instances.
   get serverSecureContext() {
     return this[kInternalState].serverSecureContext;
   }
@@ -1639,6 +1620,7 @@ class QuicSession extends EventEmitter {
     alpn: undefined,
     cipher: undefined,
     cipherVersion: undefined,
+    clientHelloHandler: undefined,
     closeCode: NGTCP2_NO_ERROR,
     closeFamily: QUIC_ERROR_APPLICATION,
     closePromise: undefined,
@@ -1676,6 +1658,7 @@ class QuicSession extends EventEmitter {
       highWaterMark,
       defaultEncoding,
       ocspHandler,
+      clientHelloHandler,
     } = options;
     super({ captureRejections: true });
     this.on('newListener', onNewListener);
@@ -1687,6 +1670,7 @@ class QuicSession extends EventEmitter {
     state.highWaterMark = highWaterMark;
     state.defaultEncoding = defaultEncoding;
     state.ocspHandler = ocspHandler;
+    state.clientHelloHandler = clientHelloHandler;
     socket[kAddSession](this);
   }
 
@@ -1751,6 +1735,7 @@ class QuicSession extends EventEmitter {
       state.handshakeAckHistogram = new Histogram(handle.ack);
       state.handshakeContinuationHistogram = new Histogram(handle.rate);
       state.state.ocspEnabled = state.ocspHandler !== undefined;
+      state.state.clientHelloEnabled = state.clientHelloHandler !== undefined;
       if (handle.qlogStream !== undefined) {
         this[kSetQLogStream](handle.qlogStream);
         handle.qlogStream = undefined;
@@ -2270,7 +2255,7 @@ class QuicSession extends EventEmitter {
 }
 class QuicServerSession extends QuicSession {
   [kInternalServerState] = {
-    contexts: []
+    context: undefined
   };
 
   constructor(socket, handle, options) {
@@ -2278,51 +2263,40 @@ class QuicServerSession extends QuicSession {
       highWaterMark,
       defaultEncoding,
       ocspHandler,
+      clientHelloHandler,
+      context,
     } = options;
-    super(socket, { highWaterMark, defaultEncoding, ocspHandler });
+    super(socket, {
+      highWaterMark,
+      defaultEncoding,
+      ocspHandler,
+      clientHelloHandler
+    });
+    this[kInternalServerState].context = context;
     this[kSetHandle](handle);
   }
 
   // Called only when a clientHello event handler is registered.
   // Allows user code an opportunity to interject into the start
   // of the TLS handshake.
-  [kClientHello](alpn, servername, ciphers, callback) {
-    this.emit(
-      'clientHello',
-      alpn,
-      servername,
-      ciphers,
-      callback.bind(this[kHandle]));
+  async [kClientHello](alpn, servername, ciphers) {
+    const internalState = this[kInternalState];
+    return internalState.clientHelloHandler?.(alpn, servername, ciphers);
   }
 
   async [kHandleOcsp](servername) {
     const internalState = this[kInternalState];
-    const state = this[kInternalServerState];
-    const { context } = this.socket.serverSecureContext;
-
-    return internalState.ocspHandler?.(
-      'request',
-      {
-        servername,
-        context,
-        contexts: Array.from(state.contexts)
-      });
+    const { context } = this[kInternalServerState];
+    const certificate = context?.context.getCertificate?.();
+    const issuer = context?.context.getIssuer?.();
+    return internalState.ocspHandler?.('request', {
+      servername,
+      certificate,
+      issuer
+    });
   }
 
   get allowEarlyData() { return false; }
-
-  addContext(servername, context = {}) {
-    validateString(servername, 'servername');
-    validateObject(context, 'context');
-
-    // TODO(@jasnell): Consider unrolling regex
-    const re = new RegExp('^' +
-    servername.replace(/([.^$+?\-\\[\]{}])/g, '\\$1')
-              .replace(/\*/g, '[^.]*') +
-    '$');
-    this[kInternalServerState].contexts.push(
-      [re, _createSecureContext(context)]);
-  }
 }
 
 class QuicClientSession extends QuicSession {
diff --git a/lib/internal/quic/util.js b/lib/internal/quic/util.js
index 3d0ebb0884af7a..1d742b5da7a5b9 100644
--- a/lib/internal/quic/util.js
+++ b/lib/internal/quic/util.js
@@ -612,6 +612,7 @@ function validateQuicSocketListenOptions(options = {}) {
     defaultEncoding,
     highWaterMark,
     ocspHandler,
+    clientHelloHandler,
     requestCert,
     rejectUnauthorized,
     lookup,
@@ -626,9 +627,16 @@ function validateQuicSocketListenOptions(options = {}) {
   if (ocspHandler !== undefined && typeof ocspHandler !== 'function') {
     throw new ERR_INVALID_ARG_TYPE(
       'options.ocspHandler',
-      'functon',
+      'function',
       ocspHandler);
   }
+  if (clientHelloHandler !== undefined &&
+      typeof clientHelloHandler !== 'function') {
+    throw new ERR_INVALID_ARG_TYPE(
+      'options.clientHelloHandler',
+      'function',
+      clientHelloHandler);
+  }
   const transportParams =
     validateTransportParams(
       options,
@@ -639,6 +647,7 @@ function validateQuicSocketListenOptions(options = {}) {
     alpn,
     lookup,
     ocspHandler,
+    clientHelloHandler,
     rejectUnauthorized,
     requestCert,
     transportParams,
@@ -812,9 +821,6 @@ function toggleListeners(state, event, on) {
     case 'keylog':
       state.keylogEnabled = on;
       break;
-    case 'clientHello':
-      state.clientHelloEnabled = on;
-      break;
     case 'pathValidation':
       state.pathValidatedEnabled = on;
       break;
diff --git a/src/quic/node_quic_session-inl.h b/src/quic/node_quic_session-inl.h
index 4ab084463b37e5..8b1ac00806c4db 100644
--- a/src/quic/node_quic_session-inl.h
+++ b/src/quic/node_quic_session-inl.h
@@ -109,17 +109,6 @@ void QuicCryptoContext::Keylog(const char* line) {
     session_->listener()->OnKeylog(line, strlen(line));
 }
 
-void QuicCryptoContext::OnClientHelloDone() {
-  // Continue the TLS handshake when this function exits
-  // otherwise it will stall and fail.
-  TLSHandshakeScope handshake_scope(
-      this,
-      [&]() { set_in_client_hello(false); });
-
-  // Disable the callback at this point so we don't loop continuously
-  session_->state_->client_hello_enabled = 0;
-}
-
 // Following a pause in the handshake for OCSP or client hello, we kickstart
 // the handshake again here by triggering ngtcp2 to serialize data.
 void QuicCryptoContext::ResumeHandshake() {
diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc
index 4aa5f47a51e9ab..c2c4172393cefa 100644
--- a/src/quic/node_quic_session.cc
+++ b/src/quic/node_quic_session.cc
@@ -393,19 +393,28 @@ void JSQuicSessionListener::OnClientHello(
   HandleScope scope(env->isolate());
   Context::Scope context_scope(env->context());
 
+  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[] = {
-    Undefined(env->isolate()),
-    Undefined(env->isolate()),
-    session()->crypto_context()->hello_ciphers().ToLocalChecked()
+    alpn_string,
+    servername,
+    ciphers
   };
 
-  if (alpn != nullptr) {
-    argv[0] = String::NewFromUtf8(env->isolate(), alpn).ToLocalChecked();
-  }
-  if (server_name != nullptr) {
-    argv[1] = String::NewFromUtf8(env->isolate(), server_name).ToLocalChecked();
-  }
-
   // Grab a shared pointer to this to prevent the QuicSession
   // from being freed while the MakeCallback is running.
   BaseObjectPtr<QuicSession> ptr(session());
@@ -570,12 +579,16 @@ void JSQuicSessionListener::OnHandshakeCompleted() {
         String::NewFromUtf8(env->isolate(), hostname).ToLocalChecked();
   }
 
+  // TODO(@jasnell): Find a more graceful way of handling the
+  // possible ToLocal failures more gracefully.
   Local<Value> validationErrorReason = v8::Null(env->isolate());
   Local<Value> validationErrorCode = v8::Null(env->isolate());
   int err = ctx->VerifyPeerIdentity();
   if (err != X509_V_OK) {
-    crypto::GetValidationErrorReason(env, err).ToLocal(&validationErrorReason);
-    crypto::GetValidationErrorCode(env, err).ToLocal(&validationErrorCode);
+    CHECK(crypto::GetValidationErrorReason(env, err)
+        .ToLocal(&validationErrorReason));
+    CHECK(crypto::GetValidationErrorCode(env, err)
+        .ToLocal(&validationErrorCode));
   }
 
   Local<Value> argv[] = {
@@ -973,23 +986,20 @@ int QuicCryptoContext::OnOCSP() {
   return is_in_ocsp_request() ? -1 : 1;
 }
 
-// The OnCertDone function is called by the QuicSessionOnCertDone
-// function when usercode is done handling the OCSPRequest event.
-void QuicCryptoContext::OnOCSPDone(
-    BaseObjectPtr<SecureContext> context,
-    Local<Value> ocsp_response) {
+void QuicCryptoContext::OnClientHelloDone(
+    BaseObjectPtr<SecureContext> context) {
   Debug(session(),
-        "OCSPRequest completed. Context Provided? %s, OCSP Provided? %s",
-        context ? "Yes" : "No",
-        ocsp_response->IsArrayBufferView() ? "Yes" : "No");
+        "ClientHello completed. Context Provided? %s\n",
+        context ? "Yes" : "No");
+
   // Continue the TLS handshake when this function exits
   // otherwise it will stall and fail.
   TLSHandshakeScope handshake_scope(
       this,
-      [&]() { set_in_ocsp_request(false); });
+      [&]() { set_in_client_hello(false); });
 
   // Disable the callback at this point so we don't loop continuously
-  session_->state_->ocsp_enabled = 0;
+  session_->state_->client_hello_enabled = 0;
 
   if (context) {
     int err = crypto::UseSNIContext(ssl_, context);
@@ -999,7 +1009,24 @@ void QuicCryptoContext::OnOCSPDone(
           THROW_ERR_QUIC_FAILURE_SETTING_SNI_CONTEXT(session_->env()) :
           crypto::ThrowCryptoError(session_->env(), err);
     }
+    secure_context_ = context;
   }
+}
+
+// The OnCertDone function is called by the QuicSessionOnCertDone
+// function when usercode is done handling the OCSPRequest event.
+void QuicCryptoContext::OnOCSPDone(Local<Value> ocsp_response) {
+  Debug(session(),
+        "OCSPRequest completed. Response Provided? %s",
+        ocsp_response->IsArrayBufferView() ? "Yes" : "No");
+  // Continue the TLS handshake when this function exits
+  // otherwise it will stall and fail.
+  TLSHandshakeScope handshake_scope(
+      this,
+      [&]() { set_in_ocsp_request(false); });
+
+  // Disable the callback at this point so we don't loop continuously
+  session_->state_->ocsp_enabled = 0;
 
   if (ocsp_response->IsArrayBufferView()) {
     ocsp_response_.Reset(
@@ -2698,32 +2725,22 @@ void QuicSession::InitServer(
 }
 
 namespace {
-// A pointer to this function is passed to the JavaScript side during
-// the client hello and is called by user code when the TLS handshake
-// should resume.
 void QuicSessionOnClientHelloDone(const FunctionCallbackInfo<Value>& args) {
-  QuicSession* session;
-  ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
-  session->crypto_context()->OnClientHelloDone();
-}
-
-// This callback is invoked by user code after completing handling
-// of the 'OCSPRequest' event. The callback is invoked with two
-// possible arguments, both of which are optional
-//   1. A replacement SecureContext
-//   2. An OCSP response
-void QuicSessionOnCertDone(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   QuicSession* session;
   ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
-
   Local<FunctionTemplate> cons = env->secure_context_constructor_template();
   SecureContext* context = nullptr;
   if (args[0]->IsObject() && cons->HasInstance(args[0]))
     context = Unwrap<SecureContext>(args[0].As<Object>());
-  session->crypto_context()->OnOCSPDone(
-      BaseObjectPtr<SecureContext>(context),
-      args[1]);
+  session->crypto_context()->OnClientHelloDone(
+      BaseObjectPtr<SecureContext>(context));
+}
+
+void QuicSessionOnCertDone(const FunctionCallbackInfo<Value>& args) {
+  QuicSession* session;
+  ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());
+  session->crypto_context()->OnOCSPDone(args[0]);
 }
 }  // namespace
 
diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h
index 2bdb290478d351..165a61872052f5 100644
--- a/src/quic/node_quic_session.h
+++ b/src/quic/node_quic_session.h
@@ -407,13 +407,11 @@ class QuicCryptoContext final : public MemoryRetainer {
 
   int OnClientHello();
 
-  inline void OnClientHelloDone();
+  void OnClientHelloDone(BaseObjectPtr<crypto::SecureContext> context);
 
   int OnOCSP();
 
-  void OnOCSPDone(
-      BaseObjectPtr<crypto::SecureContext> secure_context,
-      v8::Local<v8::Value> ocsp_response);
+  void OnOCSPDone(v8::Local<v8::Value> ocsp_response);
 
   bool OnSecrets(
       ngtcp2_crypto_level level,
diff --git a/test/parallel/test-quic-client-server.js b/test/parallel/test-quic-client-server.js
index 2777fb7c113958..4f2bc37fff1953 100644
--- a/test/parallel/test-quic-client-server.js
+++ b/test/parallel/test-quic-client-server.js
@@ -54,31 +54,32 @@ const ocspHandler = common.mustCall(async function(type, options) {
     case 'request':
       const {
         servername,
-        context
+        certificate,
+        issuer,
       } = options;
 
       assert.strictEqual(servername, kServerName);
 
-      // This will be a SecureContext. By default it will
-      // be the SecureContext used to create the QuicSession.
-      // If the user wishes to do something with it, it can,
-      // but if it wishes to pass in a new SecureContext,
-      // it can pass it in as the second argument to the
-      // callback below.
-      assert(context);
-      debug('QuicServerSession Certificate: ', context.getCertificate());
-      debug('QuicServerSession Issuer: ', context.getIssuer());
+      debug('QuicServerSession Certificate: ', certificate);
+      debug('QuicServerSession Issuer: ', issuer);
 
       // Handshake will pause until the Promise resolves
       await setImmediatePromise();
 
-      return { data: Buffer.from('hello') };
+      return Buffer.from('hello');
     case 'response':
       const { data } = options;
       assert.strictEqual(data.toString(), 'hello');
   }
 }, 2);
 
+const clientHelloHandler = common.mustCall(
+  async (alpn, servername, ciphers) => {
+    assert.strictEqual(alpn, kALPN);
+    assert.strictEqual(servername, kServerName);
+    assert.strictEqual(ciphers.length, 4);
+  });
+
 const options = { key, cert, ca, alpn: kALPN, qlog, ocspHandler };
 
 const client = createQuicSocket({ qlog, client: options });
@@ -136,14 +137,6 @@ client.on('close', common.mustCall(onSocketClose.bind(client)));
       debug(`QuicServerSession Client ${family} address ${address}:${port}`);
     }
 
-    session.on('clientHello', common.mustCall(
-      (alpn, servername, ciphers, cb) => {
-        assert.strictEqual(alpn, kALPN);
-        assert.strictEqual(servername, kServerName);
-        assert.strictEqual(ciphers.length, 4);
-        cb();
-      }));
-
     session.on('secure', common.mustCall((servername, alpn, cipher) => {
       debug('QuicServerSession TLS Handshake Complete');
       debug('  Server name: %s', servername);
@@ -249,6 +242,7 @@ client.on('close', common.mustCall(onSocketClose.bind(client)));
   await server.listen({
     requestCert: true,
     rejectUnauthorized: false,
+    clientHelloHandler
   });
 
   const endpoints = server.endpoints;