quic: a few general cleanups

src/node_quic_crypto.cc

@@ -65,30 +65,32 @@ constexpr char QUIC_SERVER_HANDSHAKE_TRAFFIC_SECRET[] =
 constexpr char QUIC_SERVER_TRAFFIC_SECRET_0[] =
     "QUIC_SERVER_TRAFFIC_SECRET_0";
 
+namespace {
+// Used solely to derive the keys used to generate retry tokens.
 bool DeriveTokenKey(
     uint8_t* token_key,
     uint8_t* token_iv,
     const uint8_t* rand_data,
     size_t rand_datalen,
-    const ngtcp2_crypto_ctx* ctx,
-    std::array<uint8_t, TOKEN_SECRETLEN>* token_secret) {
+    const ngtcp2_crypto_ctx& ctx,
+    const std::array<uint8_t, TOKEN_SECRETLEN>& token_secret) {
   TokenSecret secret;
 
   return
       NGTCP2_OK(ngtcp2_crypto_hkdf_extract(
           secret.data(),
           secret.size(),
-          &ctx->md,
-          token_secret->data(),
-          token_secret->size(),
+          &ctx.md,
+          token_secret.data(),
+          token_secret.size(),
           rand_data,
           rand_datalen)) &&
       NGTCP2_OK(ngtcp2_crypto_derive_packet_protection_key(
           token_key,
           token_iv,
           nullptr,
-          &ctx->aead,
-          &ctx->md,
+          &ctx.aead,
+          &ctx.md,
           secret.data(),
           secret.size()));
 }
@@ -101,29 +103,25 @@ bool MessageDigest(
   ctx.reset(EVP_MD_CTX_new());
   CHECK(ctx);
 
-  if (EVP_DigestInit_ex(ctx.get(), meth, nullptr) != 1)
-    return false;
-
-  if (EVP_DigestUpdate(ctx.get(), rand.data(), rand.size()) != 1)
+  if (EVP_DigestInit_ex(ctx.get(), meth, nullptr) != 1 ||
+      EVP_DigestUpdate(ctx.get(), rand.data(), rand.size()) != 1) {
     return false;
+  }
 
   unsigned int mdlen = EVP_MD_size(meth);
 
   return EVP_DigestFinal_ex(ctx.get(), dest->data(), &mdlen) == 1;
 }
 
-bool GenerateRandData(uint8_t* buf, size_t len) {
+void GenerateRandData(uint8_t* buf, size_t len) {
   std::array<uint8_t, 16> rand;
   std::array<uint8_t, 32> md;
   EntropySource(rand.data(), rand.size());
-
-  if (!MessageDigest(&md, rand))
-    return false;
-
+  CHECK(MessageDigest(&md, rand));
   CHECK_LE(len, md.size());
   std::copy_n(std::begin(md), len, buf);
-  return true;
 }
+}  // namespace
 
 // The Retry Token is an encrypted token that is sent to the client
 // by the server as part of the path validation flow. The plaintext
@@ -139,7 +137,7 @@ bool GenerateRetryToken(
     size_t* tokenlen,
     const sockaddr* addr,
     const ngtcp2_cid* ocid,
-    std::array<uint8_t, TOKEN_SECRETLEN>* token_secret) {
+    const std::array<uint8_t, TOKEN_SECRETLEN>& token_secret) {
   std::array<uint8_t, 4096> plaintext;
 
   ngtcp2_crypto_ctx ctx;
@@ -159,15 +157,14 @@ bool GenerateRetryToken(
   TokenKey token_key;
   TokenIV token_iv;
 
-  if (!GenerateRandData(rand_data.data(), TOKEN_RAND_DATALEN))
-    return false;
+  GenerateRandData(rand_data.data(), TOKEN_RAND_DATALEN);
 
   if (!DeriveTokenKey(
           token_key.data(),
           token_iv.data(),
           rand_data.data(),
           TOKEN_RAND_DATALEN,
-          &ctx,
+          ctx,
           token_secret)) {
     return false;
   }
@@ -192,12 +189,13 @@ bool GenerateRetryToken(
   return true;
 }
 
+// True if the received retry token is invalid.
 bool InvalidRetryToken(
     Environment* env,
     ngtcp2_cid* ocid,
     const ngtcp2_pkt_hd* hd,
     const sockaddr* addr,
-    std::array<uint8_t, TOKEN_SECRETLEN>* token_secret,
+    const std::array<uint8_t, TOKEN_SECRETLEN>& token_secret,
     uint64_t verification_expiration) {
 
   ngtcp2_crypto_ctx ctx;
@@ -207,7 +205,7 @@ bool InvalidRetryToken(
   const size_t addrlen = SocketAddress::GetLength(addr);
 
   if (hd->tokenlen < TOKEN_RAND_DATALEN)
-    return  true;
+    return true;
 
   uint8_t* rand_data = hd->token + hd->tokenlen - TOKEN_RAND_DATALEN;
   uint8_t* ciphertext = hd->token;
@@ -221,7 +219,7 @@ bool InvalidRetryToken(
           token_iv.data(),
           rand_data,
           TOKEN_RAND_DATALEN,
-          &ctx,
+          ctx,
           token_secret)) {
     return true;
   }
@@ -494,7 +492,7 @@ Local<Value> GetALPNProtocol(QuicSession* session) {
   unsigned int alpnlen;
   QuicCryptoContext* ctx = session->CryptoContext();
 
-  SSL_get0_alpn_selected(**ctx, &alpn_buf, &alpnlen);
+  SSL_get0_alpn_selected(ctx->ssl(), &alpn_buf, &alpnlen);
   if (alpnlen == sizeof(NGTCP2_ALPN_H3) - 2 &&
       memcmp(alpn_buf, NGTCP2_ALPN_H3 + 1, sizeof(NGTCP2_ALPN_H3) - 2) == 0) {
     alpn = session->env()->quic_alpn_string();
@@ -651,10 +649,11 @@ void SetHostname(SSL* ssl, const std::string& hostname) {
 void InitializeTLS(QuicSession* session) {
   QuicCryptoContext* ctx = session->CryptoContext();
 
-  SSL_set_app_data(**ctx, session);
-  SSL_set_cert_cb(**ctx, CertCB, session);
-  SSL_set_verify(**ctx, SSL_VERIFY_NONE, crypto::VerifyCallback);
-  SSL_set_quic_early_data_enabled(**ctx, 1);
+  SSL* ssl = ctx->ssl();
+  SSL_set_app_data(ssl, session);
+  SSL_set_cert_cb(ssl, CertCB, session);
+  SSL_set_verify(ssl, SSL_VERIFY_NONE, crypto::VerifyCallback);
+  SSL_set_quic_early_data_enabled(ssl, 1);
 
   // Enable tracing if the `--trace-tls` command line flag
   // is used. TODO(@jasnell): Add process warning for this
@@ -663,28 +662,28 @@ void InitializeTLS(QuicSession* session) {
 
   switch (ctx->Side()) {
     case NGTCP2_CRYPTO_SIDE_CLIENT: {
-      SSL_set_connect_state(**ctx);
-      crypto::SetALPN(**ctx, session->GetALPN());
-      SetHostname(**ctx, session->GetHostname());
+      SSL_set_connect_state(ssl);
+      crypto::SetALPN(ssl, session->GetALPN());
+      SetHostname(ssl, session->GetHostname());
       if (ctx->IsOptionSet(QUICCLIENTSESSION_OPTION_REQUEST_OCSP))
-        SSL_set_tlsext_status_type(**ctx, TLSEXT_STATUSTYPE_ocsp);
+        SSL_set_tlsext_status_type(ssl, TLSEXT_STATUSTYPE_ocsp);
       break;
     }
     case NGTCP2_CRYPTO_SIDE_SERVER: {
-      SSL_set_accept_state(**ctx);
+      SSL_set_accept_state(ssl);
       if (ctx->IsOptionSet(QUICSERVERSESSION_OPTION_REQUEST_CERT)) {
         int verify_mode = SSL_VERIFY_PEER;
         if (ctx->IsOptionSet(QUICSERVERSESSION_OPTION_REJECT_UNAUTHORIZED))
           verify_mode |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
-        SSL_set_verify(**ctx, verify_mode, crypto::VerifyCallback);
+        SSL_set_verify(ssl, verify_mode, crypto::VerifyCallback);
       }
       break;
     }
     default:
       UNREACHABLE();
   }
 
-  SetTransportParams(session->Connection(), **ctx);
+  SetTransportParams(session->Connection(), ssl);
 }
 
 void InitializeSecureContext(
@@ -736,10 +735,11 @@ bool SetCryptoSecrets(
   SessionKey tx_hp;
 
   QuicCryptoContext* ctx = session->CryptoContext();
+  SSL* ssl = ctx->ssl();
 
   if (NGTCP2_ERR(ngtcp2_crypto_derive_and_install_key(
           session->Connection(),
-          **ctx,
+          ssl,
           rx_key.data(),
           rx_iv.data(),
           rx_hp.data(),
@@ -757,31 +757,31 @@ bool SetCryptoSecrets(
   switch (level) {
   case NGTCP2_CRYPTO_LEVEL_EARLY:
     crypto::LogSecret(
-        **ctx,
+        ssl,
         QUIC_CLIENT_EARLY_TRAFFIC_SECRET,
         rx_secret,
         secretlen);
     break;
   case NGTCP2_CRYPTO_LEVEL_HANDSHAKE:
     crypto::LogSecret(
-        **ctx,
+        ssl,
         QUIC_CLIENT_HANDSHAKE_TRAFFIC_SECRET,
         rx_secret,
         secretlen);
     crypto::LogSecret(
-        **ctx,
+        ssl,
         QUIC_SERVER_HANDSHAKE_TRAFFIC_SECRET,
         tx_secret,
         secretlen);
     break;
   case NGTCP2_CRYPTO_LEVEL_APP:
     crypto::LogSecret(
-        **ctx,
+        ssl,
         QUIC_CLIENT_TRAFFIC_SECRET_0,
         rx_secret,
         secretlen);
     crypto::LogSecret(
-        **ctx,
+        ssl,
         QUIC_SERVER_TRAFFIC_SECRET_0,
         tx_secret,
         secretlen);

src/node_quic_crypto.h

@@ -68,14 +68,14 @@ bool GenerateRetryToken(
     size_t* tokenlen,
     const sockaddr* addr,
     const ngtcp2_cid* ocid,
-    std::array<uint8_t, TOKEN_SECRETLEN>* token_secret);
+    const std::array<uint8_t, TOKEN_SECRETLEN>& token_secret);
 
 bool InvalidRetryToken(
     Environment* env,
     ngtcp2_cid* ocid,
     const ngtcp2_pkt_hd* hd,
     const sockaddr* addr,
-    std::array<uint8_t, TOKEN_SECRETLEN>* token_secret,
+    const std::array<uint8_t, TOKEN_SECRETLEN>& token_secret,
     uint64_t verification_expiration);
 
 int VerifyHostnameIdentity(SSL* ssl, const char* hostname);

src/node_quic_session.cc

@@ -2741,7 +2741,7 @@ void QuicSession::InitServer(
           &path,
           version,
           &callbacks[crypto_context_->Side()],
-          **config,
+          config->data(),
           &alloc_info_,
           static_cast<QuicSession*>(this)), 0);
 
@@ -2876,7 +2876,7 @@ bool QuicSession::InitClient(
           &path,
           NGTCP2_PROTO_VER,
           &callbacks[crypto_context_->Side()],
-          *config,
+          config.data(),
           &alloc_info_,
           static_cast<QuicSession*>(this)), 0);
 

src/node_quic_session.h

@@ -82,7 +82,10 @@ class QuicSessionConfig {
 
   void SetQlog(const ngtcp2_qlog_settings& qlog);
 
-  const ngtcp2_settings* operator*() const { return &settings_; }
+  const ngtcp2_settings& operator*() const { return settings_; }
+  const ngtcp2_settings* operator->() const { return &settings_; }
+
+  const ngtcp2_settings* data() { return &settings_; }
 
  private:
   uint64_t max_crypto_buffer_ = DEFAULT_MAX_CRYPTO_BUFFER;
@@ -309,8 +312,6 @@ class RandomConnectionIDStrategy : public ConnectionIDStrategy {
 // handshake details on behalf of a QuicSession.
 class QuicCryptoContext : public MemoryRetainer {
  public:
-  SSL* operator*() { return ssl_.get(); }
-
   uint64_t Cancel();
 
   // Outgoing crypto data must be retained in memory until it is
@@ -380,6 +381,7 @@ class QuicCryptoContext : public MemoryRetainer {
   ngtcp2_crypto_side Side() const { return side_; }
 
   SSL* ssl() { return ssl_.get(); }
+  SSL* operator->() { return ssl_.get(); }
 
   void WriteHandshake(
       ngtcp2_crypto_level level,

src/node_quic_socket.cc

@@ -484,7 +484,7 @@ void QuicSocket::SendInitialConnectionClose(
   ngtcp2_conn* conn;
   ngtcp2_conn_server_new(
     &conn,
-    *dcid,
+    dcid.cid(),
     &scid,
     &path,
     version,
@@ -555,8 +555,8 @@ bool QuicSocket::SendRetry(
           token.data(),
           &tokenlen,
           addr,
-          *dcid,
-          &token_secret_)) {
+          dcid.cid(),
+          token_secret_)) {
     return false;
   }
 
@@ -568,7 +568,7 @@ bool QuicSocket::SendRetry(
   hd.token = nullptr;
   hd.tokenlen = 0;
   hd.len = 0;
-  hd.dcid = **scid;
+  hd.dcid = *scid.cid();
   hd.scid.datalen = NGTCP2_SV_SCIDLEN;
 
   EntropySource(hd.scid.data, NGTCP2_SV_SCIDLEN);
@@ -579,7 +579,7 @@ bool QuicSocket::SendRetry(
           reinterpret_cast<uint8_t*>(buf.data),
           NGTCP2_MAX_PKTLEN_IPV4,
           &hd,
-          *dcid,
+          dcid.cid(),
           token.data(),
           tokenlen);
   if (nwrite <= 0)
@@ -685,7 +685,7 @@ BaseObjectPtr<QuicSession> QuicSocket::AcceptInitialPacket(
               &ocid,
               &hd,
               addr,
-              &token_secret_,
+              token_secret_,
               retry_token_expiration_)) {
         Debug(this, "A valid retry token was not found. Sending retry.");
         SendRetry(version, dcid, scid, addr);
@@ -703,9 +703,9 @@ BaseObjectPtr<QuicSession> QuicSocket::AcceptInitialPacket(
       QuicSession::CreateServer(
           this,
           &server_session_config_,
-          *dcid,
+          dcid.cid(),
           addr,
-          *scid,
+          scid.cid(),
           ocid_ptr,
           version,
           server_alpn_,

src/node_quic_util.h

@@ -141,7 +141,9 @@ class QuicCID {
 
   inline std::string ToHex() const;
 
-  const ngtcp2_cid* operator*() const { return &cid_; }
+  const ngtcp2_cid& operator*() const { return cid_; }
+  const ngtcp2_cid* operator->() const { return &cid_; }
+  const ngtcp2_cid* cid() const { return &cid_; }
 
   const uint8_t* data() const { return cid_.data; }
   size_t length() const { return cid_.datalen; }