From 493c41e37b5d0ef6195e491f9bd0737707c837cc Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 30 Dec 2023 16:45:40 -0800 Subject: [PATCH 01/12] quic: rework TLSContext Reworks TLSContext to allow server endpoints to share a common SSL_CTX while having different SSL objects for each connection. Removes the dependency on the SecureContext base object and ensures proper configuration of the TLS context for QUIC. Simplifies some of the options passed in for the TLSContext. --- src/crypto/crypto_context.cc | 6 +- src/crypto/crypto_context.h | 7 + src/quic/bindingdata.h | 7 +- src/quic/endpoint.cc | 68 ++- src/quic/endpoint.h | 6 +- src/quic/session.cc | 91 ++-- src/quic/session.h | 19 +- src/quic/tlscontext.cc | 805 ++++++++++++++++++----------------- src/quic/tlscontext.h | 242 +++++++---- 9 files changed, 678 insertions(+), 573 deletions(-) diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index 6e5bbe07d0c337..17b99177d69767 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -53,7 +53,7 @@ static const char system_cert_path[] = NODE_OPENSSL_SYSTEM_CERT_PATH; static bool extra_root_certs_loaded = false; -inline X509_STORE* GetOrCreateRootCertStore() { +X509_STORE* GetOrCreateRootCertStore() { // Guaranteed thread-safe by standard, just don't use -fno-threadsafe-statics. static X509_STORE* store = NewRootCertStore(); return store; @@ -140,6 +140,8 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, return ret; } +} // namespace + // Read a file that contains our certificate in "PEM" format, // possibly followed by a sequence of CA certificates that should be // sent to the peer in the Certificate message. @@ -194,8 +196,6 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, issuer); } -} // namespace - X509_STORE* NewRootCertStore() { static std::vector root_certs_vector; static Mutex root_certs_vector_mutex; diff --git a/src/crypto/crypto_context.h b/src/crypto/crypto_context.h index 607b0984ba647a..108ee0e2b2c8de 100644 --- a/src/crypto/crypto_context.h +++ b/src/crypto/crypto_context.h @@ -24,6 +24,8 @@ void IsExtraRootCertsFileLoaded( X509_STORE* NewRootCertStore(); +X509_STORE* GetOrCreateRootCertStore(); + BIOPointer LoadBIO(Environment* env, v8::Local v); class SecureContext final : public BaseObject { @@ -153,6 +155,11 @@ class SecureContext final : public BaseObject { unsigned char ticket_key_hmac_[16]; }; +int SSL_CTX_use_certificate_chain(SSL_CTX* ctx, + BIOPointer&& in, + X509Pointer* cert, + X509Pointer* issuer); + } // namespace crypto } // namespace node diff --git a/src/quic/bindingdata.h b/src/quic/bindingdata.h index 83264e48d3d965..18577d2472868b 100644 --- a/src/quic/bindingdata.h +++ b/src/quic/bindingdata.h @@ -135,7 +135,6 @@ constexpr size_t kMaxVectorCount = 16; V(failure, "failure") \ V(groups, "groups") \ V(handshake_timeout, "handshakeTimeout") \ - V(hostname, "hostname") \ V(http3_alpn, &NGHTTP3_ALPN_H3[1]) \ V(initial_max_data, "initialMaxData") \ V(initial_max_stream_data_bidi_local, "initialMaxStreamDataBidiLocal") \ @@ -172,11 +171,10 @@ constexpr size_t kMaxVectorCount = 16; V(reject_unauthorized, "rejectUnauthorized") \ V(reno, "reno") \ V(retry_token_expiration, "retryTokenExpiration") \ - V(request_peer_certificate, "requestPeerCertificate") \ V(reset_token_secret, "resetTokenSecret") \ V(rx_loss, "rxDiagnosticLoss") \ V(session, "Session") \ - V(session_id_ctx, "sessionIDContext") \ + V(sni, "sni") \ V(stream, "Stream") \ V(success, "success") \ V(tls_options, "tls") \ @@ -189,7 +187,8 @@ constexpr size_t kMaxVectorCount = 16; V(udp_ttl, "udpTTL") \ V(unacknowledged_packet_threshold, "unacknowledgedPacketThreshold") \ V(validate_address, "validateAddress") \ - V(verify_hostname_identity, "verifyHostnameIdentity") \ + V(verify_client, "verifyClient") \ + V(verify_private_key, "verifyPrivateKey") \ V(version, "version") // ============================================================================= diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc index a9b401d18fdb6c..fb0a5873dea406 100644 --- a/src/quic/endpoint.cc +++ b/src/quic/endpoint.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -958,9 +959,32 @@ bool Endpoint::Start() { void Endpoint::Listen(const Session::Options& options) { if (is_closed() || is_closing() || state_->listening == 1) return; - server_options_ = options; + DCHECK(!server_state_.has_value()); + + // We need at least one key and one cert to complete the tls handshake on the + // server. Why not make this an error? We could but it's not strictly + // necessary. + if (options.tls_options.keys.empty() || options.tls_options.certs.empty()) { + env()->EmitProcessEnvWarning(); + ProcessEmitWarning(env(), + "The QUIC TLS options did not include a key or cert. " + "This means the TLS handshake will fail. This is likely " + "not what you want."); + } + + auto context = TLSContext::CreateServer(env(), options.tls_options); + if (!*context) { + THROW_ERR_INVALID_STATE( + env(), "Failed to create TLS context: %s", context->validation_error()); + return; + } + + server_state_ = { + options, + std::move(context), + }; if (Start()) { - Debug(this, "Listening with options %s", server_options_.value()); + Debug(this, "Listening with options %s", server_state_->options); state_->listening = 1; } } @@ -972,8 +996,7 @@ BaseObjectPtr Endpoint::Connect( // If starting fails, the endpoint will be destroyed. if (!Start()) return BaseObjectPtr(); - Session::Config config( - *this, options, local_address(), remote_address, session_ticket); + Session::Config config(*this, options, local_address(), remote_address); IF_QUIC_DEBUG(env()) { Debug( @@ -985,7 +1008,21 @@ BaseObjectPtr Endpoint::Connect( session_ticket.has_value() ? "yes" : "no"); } - auto session = Session::Create(this, config); + auto tls_context = TLSContext::CreateClient(env(), options.tls_options); + if (!*tls_context) { + THROW_ERR_INVALID_STATE(env(), + "Failed to create TLS context: %s", + tls_context->validation_error()); + return BaseObjectPtr(); + } + auto session = + Session::Create(this, config, tls_context.get(), session_ticket); + if (!session->tls_session()) { + THROW_ERR_INVALID_STATE(env(), + "Failed to create TLS session: %s", + session->tls_session().validation_error()); + return BaseObjectPtr(); + } if (!session) return BaseObjectPtr(); session->set_wrapped(); @@ -1093,9 +1130,19 @@ void Endpoint::Receive(const uv_buf_t& buf, // as a server, then we cannot accept the initial packet. if (is_closed() || is_closing() || !is_listening()) return; - Debug(this, "Trying to create new session for %s", config.dcid); - auto session = Session::Create(this, config); + Debug(this, "Creating new session for %s", config.dcid); + + std::optional no_ticket = std::nullopt; + auto session = Session::Create( + this, config, server_state_->tls_context.get(), no_ticket); if (session) { + if (!session->tls_session()) { + Debug(this, + "Failed to create TLS session for %s: %s", + config.dcid, + session->tls_session().validation_error()); + return; + } receive(session.get(), std::move(store), config.local_address, @@ -1183,7 +1230,7 @@ void Endpoint::Receive(const uv_buf_t& buf, // because that is the value *this* session will use as the outbound dcid. Session::Config config(Side::SERVER, *this, - server_options_.value(), + server_state_->options, version, local_address, remote_address, @@ -1582,8 +1629,9 @@ bool Endpoint::is_listening() const { void Endpoint::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("options", options_); tracker->TrackField("udp", udp_); - if (server_options_.has_value()) { - tracker->TrackField("server_options", server_options_.value()); + if (server_state_.has_value()) { + tracker->TrackField("server_options", server_state_->options); + tracker->TrackField("server_tls_context", server_state_->tls_context); } tracker->TrackField("token_map", token_map_); tracker->TrackField("sessions", sessions_); diff --git a/src/quic/endpoint.h b/src/quic/endpoint.h index 470771eb1a1914..abed572e2e52a9 100644 --- a/src/quic/endpoint.h +++ b/src/quic/endpoint.h @@ -409,8 +409,12 @@ class Endpoint final : public AsyncWrap, public Packet::Listener { const Options options_; UDP udp_; + struct ServerState { + Session::Options options; + std::shared_ptr tls_context; + }; // Set if/when the endpoint is configured to listen. - std::optional server_options_{}; + std::optional server_state_ = std::nullopt; // A Session is generally identified by one or more CIDs. We use two // maps for this rather than one to avoid creating a whole bunch of diff --git a/src/quic/session.cc b/src/quic/session.cc index 4c9aad191ad650..c7d89c7731a855 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -90,7 +90,6 @@ namespace quic { V(BIDI_OUT_STREAM_COUNT, bidi_out_stream_count) \ V(UNI_IN_STREAM_COUNT, uni_in_stream_count) \ V(UNI_OUT_STREAM_COUNT, uni_out_stream_count) \ - V(KEYUPDATE_COUNT, keyupdate_count) \ V(LOSS_RETRANSMIT_COUNT, loss_retransmit_count) \ V(MAX_BYTES_IN_FLIGHT, max_bytes_in_flight) \ V(BYTES_IN_FLIGHT, bytes_in_flight) \ @@ -185,7 +184,7 @@ Session::SendPendingDataScope::~SendPendingDataScope() { namespace { -inline const char* getEncryptionLevelName(ngtcp2_encryption_level level) { +inline std::string to_string(ngtcp2_encryption_level level) { switch (level) { case NGTCP2_ENCRYPTION_LEVEL_1RTT: return "1rtt"; @@ -297,7 +296,6 @@ Session::Config::Config(Side side, const SocketAddress& remote_address, const CID& dcid, const CID& scid, - std::optional session_ticket, const CID& ocid) : side(side), options(options), @@ -306,8 +304,7 @@ Session::Config::Config(Side side, remote_address(remote_address), dcid(dcid), scid(scid), - ocid(ocid), - session_ticket(session_ticket) { + ocid(ocid) { ngtcp2_settings_default(&settings); settings.initial_ts = uv_hrtime(); @@ -343,7 +340,6 @@ Session::Config::Config(const Endpoint& endpoint, const Options& options, const SocketAddress& local_address, const SocketAddress& remote_address, - std::optional session_ticket, const CID& ocid) : Config(Side::CLIENT, endpoint, @@ -353,7 +349,6 @@ Session::Config::Config(const Endpoint& endpoint, remote_address, CID::Factory::random().Generate(NGTCP2_MIN_INITIAL_DCIDLEN), options.cid_factory->Generate(), - session_ticket, ocid) {} void Session::Config::MemoryInfo(MemoryTracker* tracker) const { @@ -364,8 +359,6 @@ void Session::Config::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("scid", scid); tracker->TrackField("ocid", ocid); tracker->TrackField("retry_scid", retry_scid); - if (session_ticket.has_value()) - tracker->TrackField("session_ticket", session_ticket.value()); } void Session::Config::set_token(const uint8_t* token, @@ -410,13 +403,6 @@ std::string Session::Config::ToString() const { res += prefix + "ocid: " + ocid.ToString(); res += prefix + "retry scid: " + retry_scid.ToString(); res += prefix + "preferred address cid: " + preferred_address_cid.ToString(); - - if (session_ticket.has_value()) { - res += prefix + "session ticket: yes"; - } else { - res += prefix + "session ticket: "; - } - res += indent.Close(); return res; } @@ -491,7 +477,9 @@ bool Session::HasInstance(Environment* env, Local value) { } BaseObjectPtr Session::Create(Endpoint* endpoint, - const Config& config) { + const Config& config, + TLSContext* tls_context, + std::optional& ticket) { Local obj; if (!GetConstructorTemplate(endpoint->env()) ->InstanceTemplate() @@ -500,12 +488,15 @@ BaseObjectPtr Session::Create(Endpoint* endpoint, return BaseObjectPtr(); } - return MakeDetachedBaseObject(endpoint, obj, config); + return MakeDetachedBaseObject( + endpoint, obj, config, tls_context, ticket); } Session::Session(Endpoint* endpoint, v8::Local object, - const Config& config) + const Config& config, + TLSContext* tls_context, + std::optional& session_ticket) : AsyncWrap(endpoint->env(), object, AsyncWrap::PROVIDER_QUIC_SESSION), stats_(env()->isolate()), state_(env()->isolate()), @@ -515,7 +506,7 @@ Session::Session(Endpoint* endpoint, local_address_(config.local_address), remote_address_(config.remote_address), connection_(InitConnection()), - tls_context_(env(), config_.side, this, config_.options.tls_options), + tls_session_(tls_context->NewSession(this, session_ticket)), application_(select_application()), timer_(env(), [this, self = BaseObjectPtr(this)] { OnTimeout(); }) { @@ -560,8 +551,6 @@ Session::Session(Endpoint* endpoint, endpoint_->AddSession(config_.scid, BaseObjectPtr(this)); endpoint_->AssociateCID(config_.dcid, config_.scid); - tls_context_.Start(); - UpdateDataStats(); } @@ -595,8 +584,8 @@ Endpoint& Session::endpoint() const { return *endpoint_; } -TLSContext& Session::tls_context() { - return tls_context_; +TLSSession& Session::tls_session() { + return *tls_session_; } Session::Application& Session::application() { @@ -1169,7 +1158,7 @@ void Session::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("local_address", local_address_); tracker->TrackField("remote_address", remote_address_); tracker->TrackField("application", application_); - tracker->TrackField("tls_context", tls_context_); + tracker->TrackField("tls_session", tls_session_); tracker->TrackField("timer", timer_); tracker->TrackField("conn_closebuf", conn_closebuf_); tracker->TrackField("qlog_stream", qlog_stream_); @@ -1189,7 +1178,6 @@ bool Session::wants_session_ticket() const { } void Session::SetStreamOpenAllowed() { - // TODO(@jasnell): Might remove this. May not be needed state_->stream_open_allowed = 1; } @@ -1434,7 +1422,7 @@ bool Session::HandshakeCompleted() { Debug(this, "Session handshake completed"); STAT_RECORD_TIMESTAMP(Stats, handshake_completed_at); - if (!tls_context_.early_data_was_accepted()) + if (!tls_session().early_data_was_accepted()) ngtcp2_conn_tls_early_data_rejected(*this); // When in a server session, handshake completed == handshake confirmed. @@ -1592,23 +1580,21 @@ void Session::EmitHandshakeComplete() { Undefined(isolate), // Cipher version Undefined(isolate), // Validation error reason Undefined(isolate), // Validation error code - v8::Boolean::New(isolate, tls_context_.early_data_was_accepted())}; - - int err = tls_context_.VerifyPeerIdentity(); + v8::Boolean::New(isolate, tls_session().early_data_was_accepted())}; - if (err != X509_V_OK && (!crypto::GetValidationErrorReason(env(), err) - .ToLocal(&argv[kValidationErrorReason]) || - !crypto::GetValidationErrorCode(env(), err) - .ToLocal(&argv[kValidationErrorCode]))) { + auto& tls = tls_session(); + auto peerVerifyError = tls.VerifyPeerIdentity(env()); + if (peerVerifyError.has_value() && + (!peerVerifyError->reason.ToLocal(&argv[kValidationErrorReason]) || + !peerVerifyError->code.ToLocal(&argv[kValidationErrorCode]))) { return; } - if (!ToV8Value(env()->context(), tls_context_.servername()) + if (!ToV8Value(env()->context(), tls.servername()) .ToLocal(&argv[kServerName]) || - !ToV8Value(env()->context(), tls_context_.alpn()) - .ToLocal(&argv[kSelectedAlpn]) || - tls_context_.cipher_name(env()).ToLocal(&argv[kCipherName]) || - !tls_context_.cipher_version(env()).ToLocal(&argv[kCipherVersion])) { + !ToV8Value(env()->context(), tls.alpn()).ToLocal(&argv[kSelectedAlpn]) || + !tls.cipher_name(env()).ToLocal(&argv[kCipherName]) || + !tls.cipher_version(env()).ToLocal(&argv[kCipherVersion])) { return; } @@ -1665,7 +1651,10 @@ void Session::EmitSessionTicket(Store&& ticket) { // If there is nothing listening for the session ticket, don't bother // emitting. - if (LIKELY(state_->session_ticket == 0)) return; + if (LIKELY(!wants_session_ticket())) { + Debug(this, "Session ticket was discarded"); + return; + } CallbackScope cb_scope(this); @@ -1777,7 +1766,7 @@ struct Session::Impl { Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); Local ret; - if (session->tls_context().cert(env).ToLocal(&ret)) + if (session->tls_session().cert(env).ToLocal(&ret)) args.GetReturnValue().Set(ret); } @@ -1787,7 +1776,7 @@ struct Session::Impl { ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); Local ret; if (!session->is_server() && - session->tls_context().ephemeral_key(env).ToLocal(&ret)) + session->tls_session().ephemeral_key(env).ToLocal(&ret)) args.GetReturnValue().Set(ret); } @@ -1796,7 +1785,7 @@ struct Session::Impl { Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); Local ret; - if (session->tls_context().peer_cert(env).ToLocal(&ret)) + if (session->tls_session().peer_cert(env).ToLocal(&ret)) args.GetReturnValue().Set(ret); } @@ -1820,7 +1809,8 @@ struct Session::Impl { // before the TLS handshake has been confirmed or while a previous // key update is being processed). When it fails, InitiateKeyUpdate() // will return false. - args.GetReturnValue().Set(session->tls_context().InitiateKeyUpdate()); + Debug(session, "Initiating key update"); + args.GetReturnValue().Set(session->tls_session().InitiateKeyUpdate()); } static void DoOpenStream(const FunctionCallbackInfo& args) { @@ -2025,7 +2015,7 @@ struct Session::Impl { Debug(session, "Receiving RX key for level %d for dcid %s", - getEncryptionLevelName(level), + to_string(level), session->config().dcid); if (!session->is_server() && (level == NGTCP2_ENCRYPTION_LEVEL_0RTT || @@ -2084,7 +2074,7 @@ struct Session::Impl { Debug(session, "Receiving TX key for level %d for dcid %s", - getEncryptionLevelName(level), + to_string(level), session->config().dcid); if (session->is_server() && (level == NGTCP2_ENCRYPTION_LEVEL_0RTT || @@ -2280,7 +2270,6 @@ Local Session::GetConstructorTemplate(Environment* env) { } else { \ SetProtoMethod(isolate, tmpl, #key, Impl::name); \ } - SESSION_JS_METHODS(V) #undef V @@ -2317,7 +2306,7 @@ Session::QuicConnectionPointer Session::InitConnection() { &allocator_, this), 0); - return QuicConnectionPointer(conn); + break; } case Side::CLIENT: { CHECK_EQ(ngtcp2_conn_client_new(&conn, @@ -2331,12 +2320,10 @@ Session::QuicConnectionPointer Session::InitConnection() { &allocator_, this), 0); - if (config_.session_ticket.has_value()) - tls_context_.MaybeSetEarlySession(config_.session_ticket.value()); - return QuicConnectionPointer(conn); + break; } } - UNREACHABLE(); + return QuicConnectionPointer(conn); } void Session::InitPerIsolate(IsolateData* data, diff --git a/src/quic/session.h b/src/quic/session.h index eded22481e8a36..ec7c525206b9d2 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -166,10 +166,6 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { CID retry_scid = CID::kInvalid; CID preferred_address_cid = CID::kInvalid; - // If this is a client session, the session_ticket is used to resume - // a TLS session using a previously established session ticket. - std::optional session_ticket = std::nullopt; - ngtcp2_settings settings = {}; operator ngtcp2_settings*() { return &settings; } operator const ngtcp2_settings*() const { return &settings; } @@ -182,14 +178,12 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { const SocketAddress& remote_address, const CID& dcid, const CID& scid, - std::optional session_ticket = std::nullopt, const CID& ocid = CID::kInvalid); Config(const Endpoint& endpoint, const Options& options, const SocketAddress& local_address, const SocketAddress& remote_address, - std::optional session_ticket = std::nullopt, const CID& ocid = CID::kInvalid); void set_token(const uint8_t* token, @@ -214,17 +208,21 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { static void RegisterExternalReferences(ExternalReferenceRegistry* registry); static BaseObjectPtr Create(Endpoint* endpoint, - const Config& config); + const Config& config, + TLSContext* tls_context, + std::optional& ticket); // Really should be private but MakeDetachedBaseObject needs visibility. Session(Endpoint* endpoint, v8::Local object, - const Config& config); + const Config& config, + TLSContext* tls_context, + std::optional& ticket); ~Session() override; uint32_t version() const; Endpoint& endpoint() const; - TLSContext& tls_context(); + TLSSession& tls_session(); Application& application(); const Config& config() const; const Options& options() const; @@ -418,7 +416,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { SocketAddress local_address_; SocketAddress remote_address_; QuicConnectionPointer connection_; - TLSContext tls_context_; + std::unique_ptr tls_session_; std::unique_ptr application_; StreamsMap streams_; TimerWrapHandle timer_; @@ -437,6 +435,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { friend struct SendPendingDataScope; friend class Stream; friend class TLSContext; + friend class TLSSession; friend class TransportParams; }; diff --git a/src/quic/tlscontext.cc b/src/quic/tlscontext.cc index 04d46f83380774..770f9ff4f051cd 100644 --- a/src/quic/tlscontext.cc +++ b/src/quic/tlscontext.cc @@ -3,13 +3,13 @@ #include "tlscontext.h" #include #include +#include #include #include #include #include #include #include -#include #include #include #include @@ -32,10 +32,9 @@ using v8::Value; namespace quic { -const TLSContext::Options TLSContext::Options::kDefault = {}; +// ============================================================================ namespace { - // TODO(@jasnell): One time initialization. ngtcp2 says this is optional but // highly recommended to deal with some perf regression. Unfortunately doing // this breaks some existing tests and we need to understand the potential @@ -45,202 +44,6 @@ namespace { // return 0; // }(); -constexpr size_t kMaxAlpnLen = 255; - -int AllowEarlyDataCallback(SSL* ssl, void* arg) { - // Currently, we always allow early data. Later we might make - // it configurable. - return 1; -} - -int NewSessionCallback(SSL* ssl, SSL_SESSION* session) { - // We use this event to trigger generation of the SessionTicket - // if the user has requested to receive it. - return TLSContext::From(ssl).OnNewSession(session); -} - -void KeylogCallback(const SSL* ssl, const char* line) { - TLSContext::From(ssl).Keylog(line); -} - -int AlpnSelectionCallback(SSL* ssl, - const unsigned char** out, - unsigned char* outlen, - const unsigned char* in, - unsigned int inlen, - void* arg) { - auto& context = TLSContext::From(ssl); - - auto requested = context.options().alpn; - if (requested.length() > kMaxAlpnLen) return SSL_TLSEXT_ERR_NOACK; - - // The Session supports exactly one ALPN identifier. If that does not match - // any of the ALPN identifiers provided in the client request, then we fail - // here. Note that this will not fail the TLS handshake, so we have to check - // later if the ALPN matches the expected identifier or not. - // - // We might eventually want to support the ability to negotiate multiple - // possible ALPN's on a single endpoint/session but for now, we only support - // one. - if (SSL_select_next_proto( - const_cast(out), - outlen, - reinterpret_cast(requested.c_str()), - requested.length(), - in, - inlen) == OPENSSL_NPN_NO_OVERLAP) { - return SSL_TLSEXT_ERR_NOACK; - } - - return SSL_TLSEXT_ERR_OK; -} - -BaseObjectPtr InitializeSecureContext( - Session* session, - Side side, - Environment* env, - const TLSContext::Options& options) { - auto context = crypto::SecureContext::Create(env); - - auto& ctx = context->ctx(); - - switch (side) { - case Side::SERVER: { - Debug(session, "Initializing secure context for server"); - ctx.reset(SSL_CTX_new(TLS_server_method())); - SSL_CTX_set_app_data(ctx.get(), context); - - if (ngtcp2_crypto_quictls_configure_server_context(ctx.get()) != 0) { - return BaseObjectPtr(); - } - - SSL_CTX_set_max_early_data(ctx.get(), UINT32_MAX); - SSL_CTX_set_allow_early_data_cb( - ctx.get(), AllowEarlyDataCallback, nullptr); - SSL_CTX_set_options(ctx.get(), - (SSL_OP_ALL & ~SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS) | - SSL_OP_SINGLE_ECDH_USE | - SSL_OP_CIPHER_SERVER_PREFERENCE | - SSL_OP_NO_ANTI_REPLAY); - SSL_CTX_set_mode(ctx.get(), SSL_MODE_RELEASE_BUFFERS); - SSL_CTX_set_alpn_select_cb(ctx.get(), AlpnSelectionCallback, nullptr); - SSL_CTX_set_session_ticket_cb(ctx.get(), - SessionTicket::GenerateCallback, - SessionTicket::DecryptedCallback, - nullptr); - - const unsigned char* sid_ctx = reinterpret_cast( - options.session_id_ctx.c_str()); - SSL_CTX_set_session_id_context( - ctx.get(), sid_ctx, options.session_id_ctx.length()); - - break; - } - case Side::CLIENT: { - Debug(session, "Initializing secure context for client"); - ctx.reset(SSL_CTX_new(TLS_client_method())); - SSL_CTX_set_app_data(ctx.get(), context); - - if (ngtcp2_crypto_quictls_configure_client_context(ctx.get()) != 0) { - return BaseObjectPtr(); - } - - SSL_CTX_set_session_cache_mode( - ctx.get(), SSL_SESS_CACHE_CLIENT | SSL_SESS_CACHE_NO_INTERNAL_STORE); - SSL_CTX_sess_set_new_cb(ctx.get(), NewSessionCallback); - break; - } - default: - UNREACHABLE(); - } - - SSL_CTX_set_default_verify_paths(ctx.get()); - - if (options.keylog) SSL_CTX_set_keylog_callback(ctx.get(), KeylogCallback); - - if (SSL_CTX_set_ciphersuites(ctx.get(), options.ciphers.c_str()) != 1) { - return BaseObjectPtr(); - } - - if (SSL_CTX_set1_groups_list(ctx.get(), options.groups.c_str()) != 1) { - return BaseObjectPtr(); - } - - // Handle CA certificates... - - const auto addCACert = [&](uv_buf_t ca) { - crypto::ClearErrorOnReturn clear_error_on_return; - crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(ca.base, ca.len); - if (!bio) return false; - context->SetCACert(bio); - return true; - }; - - const auto addRootCerts = [&] { - crypto::ClearErrorOnReturn clear_error_on_return; - context->SetRootCerts(); - }; - - if (!options.ca.empty()) { - for (auto& ca : options.ca) { - if (!addCACert(ca)) { - return BaseObjectPtr(); - } - } - } else { - addRootCerts(); - } - - // Handle Certs - - const auto addCert = [&](uv_buf_t cert) { - crypto::ClearErrorOnReturn clear_error_on_return; - crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(cert.base, cert.len); - if (!bio) return Just(false); - auto ret = context->AddCert(env, std::move(bio)); - return ret; - }; - - for (auto& cert : options.certs) { - if (!addCert(cert).IsJust()) { - return BaseObjectPtr(); - } - } - - // Handle keys - - const auto addKey = [&](auto& key) { - crypto::ClearErrorOnReturn clear_error_on_return; - return context->UseKey(env, key); - // TODO(@jasnell): Maybe SSL_CTX_check_private_key also? - }; - - for (auto& key : options.keys) { - if (!addKey(key).IsJust()) { - return BaseObjectPtr(); - } - } - - // Handle CRL - - const auto addCRL = [&](uv_buf_t crl) { - crypto::ClearErrorOnReturn clear_error_on_return; - crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(crl.base, crl.len); - if (!bio) return Just(false); - return context->SetCRL(env, bio); - }; - - for (auto& crl : options.crl) { - if (!addCRL(crl).IsJust()) { - return BaseObjectPtr(); - } - } - - // TODO(@jasnell): Possibly handle other bits. Such a pfx, client cert engine, - // and session timeout. - return BaseObjectPtr(context); -} - void EnableTrace(Environment* env, crypto::BIOPointer* bio, SSL* ssl) { #if HAVE_SSL_TRACE static bool warn_trace_tls = true; @@ -345,217 +148,270 @@ bool SetOption(Environment* env, } } // namespace -Side TLSContext::side() const { - return side_; +std::shared_ptr TLSContext::CreateClient(Environment* env, + const Options& options) { + return std::make_shared(env, Side::CLIENT, options); } -const TLSContext::Options& TLSContext::options() const { - return options_; +std::shared_ptr TLSContext::CreateServer(Environment* env, + const Options& options) { + return std::make_shared(env, Side::SERVER, options); } -inline const TLSContext& TLSContext::From(const SSL* ssl) { - auto ref = static_cast(SSL_get_app_data(ssl)); - TLSContext* context = ContainerOf(&TLSContext::conn_ref_, ref); - return *context; -} +TLSContext::TLSContext(Environment* env, Side side, const Options& options) + : env_(env), side_(side), options_(options), ctx_(Initialize()) {} -inline TLSContext& TLSContext::From(SSL* ssl) { - auto ref = static_cast(SSL_get_app_data(ssl)); - TLSContext* context = ContainerOf(&TLSContext::conn_ref_, ref); - return *context; +TLSContext::operator SSL_CTX*() const { + DCHECK(ctx_); + return ctx_.get(); } -TLSContext::TLSContext(Environment* env, - Side side, - Session* session, - const Options& options) - : conn_ref_({getConnection, this}), - side_(side), - env_(env), - session_(session), - options_(options), - secure_context_(InitializeSecureContext(session, side, env, options)) { - CHECK(secure_context_); - ssl_.reset(SSL_new(secure_context_->ctx().get())); - CHECK(ssl_ && SSL_is_quic(ssl_.get())); - - SSL_set_app_data(ssl_.get(), &conn_ref_); - SSL_set_verify(ssl_.get(), SSL_VERIFY_NONE, crypto::VerifyCallback); +int TLSContext::OnSelectAlpn(SSL* ssl, + const unsigned char** out, + unsigned char* outlen, + const unsigned char* in, + unsigned int inlen, + void* arg) { + static constexpr size_t kMaxAlpnLen = 255; + auto& session = TLSSession::From(ssl); - // Enable tracing if the `--trace-tls` command line flag is used. - if (UNLIKELY(env->options()->trace_tls || options.enable_tls_trace)) - EnableTrace(env, &bio_trace_, ssl_.get()); - - switch (side) { - case Side::CLIENT: { - SSL_set_connect_state(ssl_.get()); - CHECK_EQ(0, - SSL_set_alpn_protos(ssl_.get(), - reinterpret_cast( - options_.alpn.c_str()), - options_.alpn.length())); - CHECK_EQ(0, - SSL_set_tlsext_host_name(ssl_.get(), options_.hostname.c_str())); - break; - } - case Side::SERVER: { - SSL_set_accept_state(ssl_.get()); - if (options.request_peer_certificate) { - int verify_mode = SSL_VERIFY_PEER; - if (options.reject_unauthorized) - verify_mode |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT; - SSL_set_verify(ssl_.get(), verify_mode, crypto::VerifyCallback); - } - break; - } - default: - UNREACHABLE(); - } -} - -void TLSContext::Start() { - Debug(session_, "Crypto context is starting"); - ngtcp2_conn_set_tls_native_handle(*session_, ssl_.get()); + const auto& requested = session.context().options().alpn; + if (requested.length() > kMaxAlpnLen) return SSL_TLSEXT_ERR_NOACK; - TransportParams tp(ngtcp2_conn_get_local_transport_params(*session_)); - Store store = tp.Encode(env_); - if (store && store.length() > 0) { - ngtcp2_vec vec = store; - SSL_set_quic_transport_params(ssl_.get(), vec.base, vec.len); + // The Session supports exactly one ALPN identifier. If that does not match + // any of the ALPN identifiers provided in the client request, then we fail + // here. Note that this will not fail the TLS handshake, so we have to check + // later if the ALPN matches the expected identifier or not. + // + // We might eventually want to support the ability to negotiate multiple + // possible ALPN's on a single endpoint/session but for now, we only support + // one. + if (SSL_select_next_proto( + const_cast(out), + outlen, + reinterpret_cast(requested.data()), + requested.length(), + in, + inlen) == OPENSSL_NPN_NO_OVERLAP) { + Debug(&session.session(), "ALPN negotiation failed"); + return SSL_TLSEXT_ERR_NOACK; } -} -void TLSContext::Keylog(const char* line) const { - session_->EmitKeylog(line); + Debug(&session.session(), "ALPN negotiation succeeded"); + return SSL_TLSEXT_ERR_OK; } -int TLSContext::OnNewSession(SSL_SESSION* session) { - Debug(session_, "Crypto context received new crypto session"); - // Used to generate and emit a SessionTicket for TLS session resumption. - - // If there is nothing listening for the session ticket, don't both emitting. - if (!session_->wants_session_ticket()) return 0; - - // Pre-fight to see how much space we need to allocate for the session ticket. - size_t size = i2d_SSL_SESSION(session, nullptr); - - if (size > 0 && size < crypto::SecureContext::kMaxSessionSize) { - // Generate the actual ticket. If this fails, we'll simply carry on without - // emitting the ticket. - std::shared_ptr ticket = - ArrayBuffer::NewBackingStore(env_->isolate(), size); - unsigned char* data = reinterpret_cast(ticket->Data()); - if (i2d_SSL_SESSION(session, &data) <= 0) return 0; - session_->EmitSessionTicket(Store(std::move(ticket), size)); +int TLSContext::OnNewSession(SSL* ssl, SSL_SESSION* sess) { + auto& session = TLSSession::From(ssl).session(); + + // If there is nothing listening for the session ticket, do not bother. + if (session.wants_session_ticket()) { + Debug(&session, "Preparing TLS session resumption ticket");; + // Pre-fight to see how much space we need to allocate for the session + // ticket. + size_t size = i2d_SSL_SESSION(sess, nullptr); + + // If size is 0 or the size is greater than our max, let's ignore it + // and continue without emitting the sessionticket event. + if (size > 0 && size <= crypto::SecureContext::kMaxSessionSize) { + auto ticket = + ArrayBuffer::NewBackingStore(session.env()->isolate(), size); + auto data = reinterpret_cast(ticket->Data()); + if (i2d_SSL_SESSION(sess, &data) > 0) { + session.EmitSessionTicket(Store(std::move(ticket), size)); + } + } } - // If size == 0, there's no session ticket data to emit. Let's ignore it - // and continue without emitting the sessionticket event. return 0; } -bool TLSContext::InitiateKeyUpdate() { - Debug(session_, "Crypto context initiating key update"); - if (session_->is_destroyed() || in_key_update_) return false; - auto leave = OnScopeLeave([this] { in_key_update_ = false; }); - in_key_update_ = true; - - return ngtcp2_conn_initiate_key_update(*session_, uv_hrtime()) == 0; +void TLSContext::OnKeylog(const SSL* ssl, const char* line) { + TLSSession::From(ssl).session().EmitKeylog(line); } -int TLSContext::VerifyPeerIdentity() { - Debug(session_, "Crypto context verifying peer identity"); - return crypto::VerifyPeerCertificate(ssl_); +int TLSContext::OnVerifyClientCertificate(int preverify_ok, + X509_STORE_CTX* ctx) { + // TODO(@jasnell): Implement the logic to verify the client certificate + return 1; } -void TLSContext::MaybeSetEarlySession(const SessionTicket& sessionTicket) { - Debug(session_, "Crypto context setting early session"); - uv_buf_t buf = sessionTicket.ticket(); - crypto::SSLSessionPointer ticket = crypto::GetTLSSession( - reinterpret_cast(buf.base), buf.len); +std::unique_ptr TLSContext::NewSession( + Session* session, const std::optional& maybeSessionTicket) { + // Passing a session ticket only makes sense with a client session. + CHECK_IMPLIES(session->is_server(), !maybeSessionTicket.has_value()); + return std::make_unique( + session, shared_from_this(), maybeSessionTicket); +} - // Silently ignore invalid TLS session - if (!ticket || !SSL_SESSION_get_max_early_data(ticket.get())) return; +crypto::SSLCtxPointer TLSContext::Initialize() { + crypto::SSLCtxPointer ctx; + switch (side_) { + case Side::SERVER: { + static constexpr unsigned char kSidCtx[] = "Node.js QUIC Server"; + ctx.reset(SSL_CTX_new(TLS_server_method())); + CHECK_EQ(ngtcp2_crypto_quictls_configure_server_context(ctx.get()), 0); + CHECK_EQ(SSL_CTX_set_max_early_data(ctx.get(), UINT32_MAX), 1); + SSL_CTX_set_options(ctx.get(), + (SSL_OP_ALL & ~SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS) | + SSL_OP_SINGLE_ECDH_USE | + SSL_OP_CIPHER_SERVER_PREFERENCE | + SSL_OP_NO_ANTI_REPLAY); + SSL_CTX_set_mode(ctx.get(), SSL_MODE_RELEASE_BUFFERS); + SSL_CTX_set_alpn_select_cb(ctx.get(), OnSelectAlpn, this); + CHECK_EQ(SSL_CTX_set_session_id_context( + ctx.get(), kSidCtx, sizeof(kSidCtx) - 1), + 1); + + if (options_.verify_client) { + SSL_CTX_set_verify(ctx.get(), + SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE | + SSL_VERIFY_FAIL_IF_NO_PEER_CERT, + OnVerifyClientCertificate); + } - // The early data will just be ignored if it's invalid. - if (!crypto::SetTLSSession(ssl_, ticket)) return; + CHECK_EQ(SSL_CTX_set_session_ticket_cb(ctx.get(), + SessionTicket::GenerateCallback, + SessionTicket::DecryptedCallback, + nullptr), + 1); + break; + } + case Side::CLIENT: { + ctx_.reset(SSL_CTX_new(TLS_client_method())); + CHECK_EQ(ngtcp2_crypto_quictls_configure_client_context(ctx.get()), 0); - ngtcp2_vec rtp = sessionTicket.transport_params(); - // Decode and attempt to set the early transport parameters configured - // for the early session. If non-zero is returned, decoding or setting - // failed, in which case we just ignore it. - if (ngtcp2_conn_decode_and_set_0rtt_transport_params( - *session_, rtp.base, rtp.len) != 0) - return; + SSL_CTX_set_session_cache_mode( + ctx.get(), SSL_SESS_CACHE_CLIENT | SSL_SESS_CACHE_NO_INTERNAL); + SSL_CTX_sess_set_new_cb(ctx.get(), OnNewSession); + break; + } + } - session_->SetStreamOpenAllowed(); -} + SSL_CTX_set_default_verify_paths(ctx.get()); + SSL_CTX_set_keylog_callback(ctx.get(), OnKeylog); -void TLSContext::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("options", options_); - tracker->TrackField("secure_context", secure_context_); -} + if (SSL_CTX_set_ciphersuites(ctx.get(), options_.ciphers.c_str()) != 1) { + validation_error_ = "Invalid cipher suite"; + return crypto::SSLCtxPointer(); + } -MaybeLocal TLSContext::cert(Environment* env) const { - return crypto::X509Certificate::GetCert(env, ssl_); -} + if (SSL_CTX_set1_groups_list(ctx.get(), options_.groups.c_str()) != 1) { + validation_error_ = "Invalid cipher groups"; + return crypto::SSLCtxPointer(); + } -MaybeLocal TLSContext::peer_cert(Environment* env) const { - crypto::X509Certificate::GetPeerCertificateFlag flag = - side_ == Side::SERVER - ? crypto::X509Certificate::GetPeerCertificateFlag::SERVER - : crypto::X509Certificate::GetPeerCertificateFlag::NONE; - return crypto::X509Certificate::GetPeerCert(env, ssl_, flag); -} + { + crypto::ClearErrorOnReturn clear_error_on_return; + if (options_.ca.empty()) { + auto store = crypto::GetOrCreateRootCertStore(); + X509_STORE_up_ref(store); + SSL_CTX_set_cert_store(ctx.get(), store); + } else { + for (const auto& ca : options_.ca) { + uv_buf_t buf = ca; + if (buf.len == 0) { + auto store = crypto::GetOrCreateRootCertStore(); + X509_STORE_up_ref(store); + SSL_CTX_set_cert_store(ctx.get(), store); + } else { + crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(buf.base, buf.len); + CHECK(bio); + X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx.get()); + while ( + crypto::X509Pointer x509 = + crypto::X509Pointer(PEM_read_bio_X509_AUX( + bio.get(), nullptr, crypto::NoPasswordCallback, nullptr))) { + if (cert_store == crypto::GetOrCreateRootCertStore()) { + cert_store = crypto::NewRootCertStore(); + SSL_CTX_set_cert_store(ctx.get(), cert_store); + } + CHECK_EQ(1, X509_STORE_add_cert(cert_store, x509.get())); + CHECK_EQ(1, SSL_CTX_add_client_CA(ctx.get(), x509.get())); + } + } + } + } + } -MaybeLocal TLSContext::cipher_name(Environment* env) const { - return crypto::GetCurrentCipherName(env, ssl_); -} + { + crypto::ClearErrorOnReturn clear_error_on_return; + for (const auto& cert : options_.certs) { + uv_buf_t buf = cert; + if (buf.len > 0) { + crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(buf.base, buf.len); + CHECK(bio); + cert_.reset(); + issuer_.reset(); + if (crypto::SSL_CTX_use_certificate_chain( + ctx.get(), std::move(bio), &cert_, &issuer_) == 0) { + validation_error_ = "Invalid certificate"; + return crypto::SSLCtxPointer(); + } + } + } + } -MaybeLocal TLSContext::cipher_version(Environment* env) const { - return crypto::GetCurrentCipherVersion(env, ssl_); -} + { + crypto::ClearErrorOnReturn clear_error_on_return; + for (const auto& key : options_.keys) { + if (key->GetKeyType() != crypto::KeyType::kKeyTypePrivate) { + validation_error_ = "Invalid key"; + return crypto::SSLCtxPointer(); + } + if (!SSL_CTX_use_PrivateKey(ctx.get(), key->GetAsymmetricKey().get())) { + validation_error_ = "Invalid key"; + return crypto::SSLCtxPointer(); + } + } + } -MaybeLocal TLSContext::ephemeral_key(Environment* env) const { - return crypto::GetEphemeralKey(env, ssl_); -} + { + crypto::ClearErrorOnReturn clear_error_on_return; + for (const auto& crl : options_.crl) { + uv_buf_t buf = crl; + crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(buf.base, buf.len); + DeleteFnPtr crlptr(PEM_read_bio_X509_CRL( + bio.get(), nullptr, crypto::NoPasswordCallback, nullptr)); + + if (!crlptr) { + validation_error_ = "Invalid CRL"; + return crypto::SSLCtxPointer(); + } -const std::string_view TLSContext::servername() const { - const char* servername = crypto::GetServerName(ssl_.get()); - return servername != nullptr ? std::string_view(servername) - : std::string_view(); -} + X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx.get()); + if (cert_store == crypto::GetOrCreateRootCertStore()) { + cert_store = crypto::NewRootCertStore(); + SSL_CTX_set_cert_store(ctx.get(), cert_store); + } -const std::string_view TLSContext::alpn() const { - const unsigned char* alpn_buf = nullptr; - unsigned int alpnlen; - SSL_get0_alpn_selected(ssl_.get(), &alpn_buf, &alpnlen); - return alpnlen ? std::string_view(reinterpret_cast(alpn_buf), - alpnlen) - : std::string_view(); -} + CHECK_EQ(1, X509_STORE_add_crl(cert_store, crlptr.get())); + CHECK_EQ( + 1, + X509_STORE_set_flags( + cert_store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL)); + } + } -bool TLSContext::early_data_was_accepted() const { - return (early_data_ && - SSL_get_early_data_status(ssl_.get()) == SSL_EARLY_DATA_ACCEPTED); -} + { + crypto::ClearErrorOnReturn clear_error_on_return; + if (options_.verify_private_key && + SSL_CTX_check_private_key(ctx.get()) != 1) { + validation_error_ = "Invalid private key"; + return crypto::SSLCtxPointer(); + } + } -void TLSContext::Options::MemoryInfo(MemoryTracker* tracker) const { - tracker->TrackField("keys", keys); - tracker->TrackField("certs", certs); - tracker->TrackField("ca", ca); - tracker->TrackField("crl", crl); + return ctx; } -ngtcp2_conn* TLSContext::getConnection(ngtcp2_crypto_conn_ref* ref) { - TLSContext* context = ContainerOf(&TLSContext::conn_ref_, ref); - return *context->session_; +void TLSContext::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("options", options_); } Maybe TLSContext::Options::From(Environment* env, Local value) { if (value.IsEmpty()) { - THROW_ERR_INVALID_ARG_TYPE(env, "options must be an object"); return Nothing(); } @@ -563,19 +419,11 @@ Maybe TLSContext::Options::From(Environment* env, auto& state = BindingData::Get(env); if (value->IsUndefined()) { - // We need at least one key and one cert to complete the tls handshake. - // Why not make this an error? We could but it's not strictly necessary. - env->EmitProcessEnvWarning(); - ProcessEmitWarning( - env, - "The default QUIC TLS options are being used. " - "This means there is no key or certificate configured and the " - "TLS handshake will fail. This is likely not what you want."); - return Just(options); + return Just(TLSContext::Options::kDefault); } if (!value->IsObject()) { - THROW_ERR_INVALID_ARG_TYPE(env, "options must be an object"); + THROW_ERR_INVALID_ARG_TYPE(env, "tls options must be an object"); return Nothing(); } @@ -589,26 +437,15 @@ Maybe TLSContext::Options::From(Environment* env, SetOption( \ env, &options, params, state.name##_string()) - if (!SET(keylog) || !SET(reject_unauthorized) || !SET(enable_tls_trace) || - !SET(request_peer_certificate) || !SET(verify_hostname_identity) || - !SET(alpn) || !SET(hostname) || !SET(session_id_ctx) || !SET(ciphers) || - !SET(groups) || + if (!SET(verify_client) || !SET(enable_tls_trace) || !SET(alpn) || + !SET(sni) || !SET(ciphers) || !SET(groups) || !SET(verify_private_key) || + !SET(keylog) || !SET_VECTOR(std::shared_ptr, keys) || !SET_VECTOR(Store, certs) || !SET_VECTOR(Store, ca) || !SET_VECTOR(Store, crl)) { return Nothing(); } - // We need at least one key and one cert to complete the tls handshake. - // Why not make this an error? We could but it's not strictly necessary. - if (options.keys.empty() || options.certs.empty()) { - env->EmitProcessEnvWarning(); - ProcessEmitWarning(env, - "The QUIC TLS options did not include a key or cert. " - "This means the TLS handshake will fail. This is likely " - "not what you want."); - } - return Just(options); } @@ -617,18 +454,15 @@ std::string TLSContext::Options::ToString() const { auto prefix = indent.Prefix(); std::string res("{"); res += prefix + "alpn: " + alpn; - res += prefix + "hostname: " + hostname; + res += prefix + "sni: " + sni; res += prefix + "keylog: " + (keylog ? std::string("yes") : std::string("no")); - res += prefix + "reject_unauthorized: " + - (reject_unauthorized ? std::string("yes") : std::string("no")); + res += prefix + "verify client: " + + (verify_client ? std::string("yes") : std::string("no")); res += prefix + "enable_tls_trace: " + (enable_tls_trace ? std::string("yes") : std::string("no")); - res += prefix + "request_peer_certificate: " + - (request_peer_certificate ? std::string("yes") : std::string("no")); - res += prefix + "verify_hostname_identity: " + - (verify_hostname_identity ? std::string("yes") : std::string("no")); - res += prefix + "session_id_ctx: " + session_id_ctx; + res += prefix + "verify private key: " + + (verify_private_key ? std::string("yes") : std::string("no")); res += prefix + "ciphers: " + ciphers; res += prefix + "groups: " + groups; res += prefix + "keys: " + std::to_string(keys.size()); @@ -639,6 +473,179 @@ std::string TLSContext::Options::ToString() const { return res; } +void TLSContext::Options::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("keys", keys); + tracker->TrackField("certs", certs); + tracker->TrackField("ca", ca); + tracker->TrackField("crl", crl); +} + +const TLSContext::Options TLSContext::Options::kDefault = {}; + +// ============================================================================ + +const TLSSession& TLSSession::From(const SSL* ssl) { + auto ref = static_cast(SSL_get_app_data(ssl)); + CHECK_NOT_NULL(ref); + return *static_cast(ref->user_data); +} + +TLSSession::TLSSession(Session* session, + std::shared_ptr context, + const std::optional& maybeSessionTicket) + : ref_({connection, this}), + context_(std::move(context)), + session_(session), + ssl_(Initialize(maybeSessionTicket)) { + Debug(session_, "Created new TLS session for %s", session->config().dcid); +} + +TLSSession::operator SSL*() const { + CHECK(ssl_); + return ssl_.get(); +} + +bool TLSSession::early_data_was_accepted() const { + CHECK_NE(ngtcp2_conn_get_handshake_completed(*session_), 0); + return SSL_get_early_data_status(*this) == SSL_EARLY_DATA_ACCEPTED; +} + +crypto::SSLPointer TLSSession::Initialize( + const std::optional& maybeSessionTicket) { + auto& ctx = context(); + auto& options = ctx.options(); + crypto::SSLPointer ssl(SSL_new(ctx)); + SSL_set_app_data(ssl.get(), &ref_); + ngtcp2_conn_set_tls_native_handle(*session_, ssl.get()); + + // Enable tracing if the `--trace-tls` command line flag is used. + if (UNLIKELY(session_->env()->options()->trace_tls || + options.enable_tls_trace)) { + EnableTrace(session_->env(), &bio_trace_, *this); + } + + switch (ctx.side()) { + case Side::SERVER: { + SSL_set_accept_state(ssl.get()); + SSL_set_quic_early_data_enabled(ssl.get(), 1); + break; + } + case Side::CLIENT: { + SSL_set_connect_state(ssl.get()); + if (SSL_set_alpn_protos( + ssl.get(), + reinterpret_cast(options.alpn.data()), + options.alpn.size()) != 0) { + validation_error_ = "Invalid ALPN"; + return crypto::SSLPointer(); + } + + if (!options.sni.empty()) { + SSL_set_tlsext_host_name(ssl.get(), options.sni.data()); + } else { + SSL_set_tlsext_host_name(ssl.get(), "localhost"); + } + + if (maybeSessionTicket.has_value()) { + auto sessionTicket = maybeSessionTicket.value(); + uv_buf_t buf = sessionTicket.ticket(); + crypto::SSLSessionPointer ticket = crypto::GetTLSSession( + reinterpret_cast(buf.base), buf.len); + + // The early data will just be ignored if it's invalid. + if (crypto::SetTLSSession(ssl, ticket) && + SSL_SESSION_get_max_early_data(ticket.get()) != 0) { + ngtcp2_vec rtp = sessionTicket.transport_params(); + if (ngtcp2_conn_decode_and_set_0rtt_transport_params( + *session_, rtp.base, rtp.len) == 0) { + SSL_set_quic_early_data_enabled(ssl.get(), 1); + session_->SetStreamOpenAllowed(); + } + } + } + + break; + } + } + + TransportParams tp(ngtcp2_conn_get_local_transport_params(*session_)); + Store store = tp.Encode(session_->env()); + if (store && store.length() > 0) { + ngtcp2_vec vec = store; + SSL_set_quic_transport_params(ssl.get(), vec.base, vec.len); + } + + return ssl; +} + +std::optional +TLSSession::VerifyPeerIdentity(Environment* env) { + int err = crypto::VerifyPeerCertificate(ssl_); + if (err == X509_V_OK) return std::nullopt; + Local reason; + Local code; + if (!crypto::GetValidationErrorReason(env, err).ToLocal(&reason) || + !crypto::GetValidationErrorCode(env, err).ToLocal(&code)) { + // Getting the validation error details failed. We'll return a value but + // the fields will be empty. + return PeerIdentityValidationError{}; + } + return PeerIdentityValidationError{reason, code}; +} + +MaybeLocal TLSSession::cert(Environment* env) const { + return crypto::X509Certificate::GetCert(env, ssl_); +} + +MaybeLocal TLSSession::peer_cert(Environment* env) const { + crypto::X509Certificate::GetPeerCertificateFlag flag = + context_->side() == Side::SERVER + ? crypto::X509Certificate::GetPeerCertificateFlag::SERVER + : crypto::X509Certificate::GetPeerCertificateFlag::NONE; + return crypto::X509Certificate::GetPeerCert(env, ssl_, flag); +} + +MaybeLocal TLSSession::ephemeral_key(Environment* env) const { + return crypto::GetEphemeralKey(env, ssl_); +} + +MaybeLocal TLSSession::cipher_name(Environment* env) const { + return crypto::GetCurrentCipherName(env, ssl_); +} + +MaybeLocal TLSSession::cipher_version(Environment* env) const { + return crypto::GetCurrentCipherVersion(env, ssl_); +} + +const std::string_view TLSSession::servername() const { + const char* servername = crypto::GetServerName(ssl_.get()); + return servername != nullptr ? std::string_view(servername) + : std::string_view(); +} + +const std::string_view TLSSession::alpn() const { + const unsigned char* alpn_buf = nullptr; + unsigned int alpnlen; + SSL_get0_alpn_selected(ssl_.get(), &alpn_buf, &alpnlen); + return alpnlen ? std::string_view(reinterpret_cast(alpn_buf), + alpnlen) + : std::string_view(); +} + +bool TLSSession::InitiateKeyUpdate() { + if (session_->is_destroyed() || in_key_update_) return false; + auto leave = OnScopeLeave([this] { in_key_update_ = false; }); + in_key_update_ = true; + + Debug(session_, "Initiating key update"); + return ngtcp2_conn_initiate_key_update(*session_, uv_hrtime()) == 0; +} + +ngtcp2_conn* TLSSession::connection(ngtcp2_crypto_conn_ref* ref) { + CHECK_NOT_NULL(ref->user_data); + return static_cast(ref->user_data)->session(); +} + } // namespace quic } // namespace node diff --git a/src/quic/tlscontext.h b/src/quic/tlscontext.h index 65d8e2f4e74942..98e4110bff576e 100644 --- a/src/quic/tlscontext.h +++ b/src/quic/tlscontext.h @@ -16,86 +16,157 @@ namespace node { namespace quic { class Session; +class TLSContext; -// Every QUIC Session has exactly one TLSContext that maintains the state +// Every QUIC Session has exactly one TLSSession that maintains the state // of the TLS handshake and negotiated cipher keys after the handshake has // been completed. It is separated out from the main Session class only as a // convenience to help make the code more maintainable and understandable. -class TLSContext final : public MemoryRetainer { +// A TLSSession is created from a TLSContext and maintains a reference to +// the context. +class TLSSession final : public MemoryRetainer { public: - enum class EncryptionLevel { - INITIAL = NGTCP2_ENCRYPTION_LEVEL_INITIAL, - HANDSHAKE = NGTCP2_ENCRYPTION_LEVEL_HANDSHAKE, - ONERTT = NGTCP2_ENCRYPTION_LEVEL_1RTT, - ZERORTT = NGTCP2_ENCRYPTION_LEVEL_0RTT, + static const TLSSession& From(const SSL* ssl); + + // The constructor is public in order to satisify the call to std::make_unique + // in TLSContext::NewSession. It should not be called directly. + TLSSession(Session* session, + std::shared_ptr context, + const std::optional& maybeSessionTicket); + TLSSession(const TLSSession&) = delete; + TLSSession(TLSSession&&) = delete; + TLSSession& operator=(const TLSSession&) = delete; + TLSSession& operator=(TLSSession&&) = delete; + + inline operator bool() const { return ssl_ != nullptr; } + inline Session& session() const { return *session_; } + inline TLSContext& context() const { return *context_; } + + // Returns true if the handshake has been completed and early data was + // accepted by the TLS session. This will assert if the handshake has + // not been completed. + bool early_data_was_accepted() const; + + v8::MaybeLocal cert(Environment* env) const; + v8::MaybeLocal peer_cert(Environment* env) const; + v8::MaybeLocal ephemeral_key(Environment* env) const; + v8::MaybeLocal cipher_name(Environment* env) const; + v8::MaybeLocal cipher_version(Environment* env) const; + + // The SNI (server name) negotiated for the session + const std::string_view servername() const; + + // The ALPN (protocol name) negotiated for the session + const std::string_view alpn() const; + + // Triggers key update to begin. This will fail and return false if either a + // previous key update is in progress or if the initial handshake has not yet + // been confirmed. + bool InitiateKeyUpdate(); + + struct PeerIdentityValidationError { + v8::MaybeLocal reason; + v8::MaybeLocal code; }; + // Checks the peer identity against the configured CA and CRL. If the peer + // certificate is valid, std::nullopt is returned. Otherwise a + // PeerIdentityValidationError is returned with the reason and code for the + // failure. + std::optional VerifyPeerIdentity( + Environment* env); + + inline const std::string_view validation_error() const { + return validation_error_; + } + + SET_NO_MEMORY_INFO() + SET_MEMORY_INFO_NAME(TLSSession) + SET_SELF_SIZE(TLSSession) + + private: + operator SSL*() const; + crypto::SSLPointer Initialize( + const std::optional& maybeSessionTicket); + + static ngtcp2_conn* connection(ngtcp2_crypto_conn_ref* ref); + + ngtcp2_crypto_conn_ref ref_; + std::shared_ptr context_; + Session* session_; + crypto::SSLPointer ssl_; + crypto::BIOPointer bio_trace_; + std::string validation_error_ = ""; + bool in_key_update_ = false; +}; + +// The TLSContext is used to create a TLSSession. For the client, there is +// typically only a single TLSContext for each TLSSession. For the server, +// there is a single TLSContext for the server and a TLSSession for every +// QUIC session created by that server. +class TLSContext final : public MemoryRetainer, + public std::enable_shared_from_this { + public: static constexpr auto DEFAULT_CIPHERS = "TLS_AES_128_GCM_SHA256:" "TLS_AES_256_GCM_SHA384:" "TLS_CHACHA20_POLY1305_" "SHA256:TLS_AES_128_CCM_SHA256"; static constexpr auto DEFAULT_GROUPS = "X25519:P-256:P-384:P-521"; - static inline const TLSContext& From(const SSL* ssl); - static inline TLSContext& From(SSL* ssl); - struct Options final : public MemoryRetainer { - // The protocol identifier to be used by this Session. + // The SNI servername to use for this session. This option is only used by + // the client. + std::string sni = "localhost"; + + // The ALPN (protocol name) to use for this session. This option is only + // used by the client. std::string alpn = NGHTTP3_ALPN_H3; - // The SNI hostname to be used. This is used only by client Sessions to - // identify the SNI host in the TLS client hello message. - std::string hostname = ""; + // The list of TLS ciphers to use for this session. + std::string ciphers = DEFAULT_CIPHERS; + + // The list of TLS groups to use for this session. + std::string groups = DEFAULT_GROUPS; - // When true, TLS keylog data will be emitted to the JavaScript session. + // When true, enables keylog output for the session. bool keylog = false; - // When set, the peer certificate is verified against the list of supplied - // CAs. If verification fails, the connection will be refused. - bool reject_unauthorized = true; + // When true, the peer certificate is verified against the list of supplied + // CA. If verification fails, the connection will be refused. When set, + // instructs the server session to request a client auth certificate. This + // option is only used by the server side. + bool verify_client = false; - // When set, enables TLS tracing for the session. This should only be used + // When true, enables TLS tracing for the session. This should only be used // for debugging. + // JavaScript option name "tlsTrace". bool enable_tls_trace = false; - // Options only used by server sessions: + // When true, causes the private key passed in for the session to be + // verified. JavaScript option name "verifyPrivateKey" + bool verify_private_key = false; - // When set, instructs the server session to request a client authentication - // certificate. - bool request_peer_certificate = false; - - // Options only used by client sessions: - - // When set, instructs the client session to verify the hostname default. - // This is required by QUIC and enabled by default. We allow disabling it - // only for debugging. - bool verify_hostname_identity = true; - - // The TLS session ID context (only used on the server) - std::string session_id_ctx = "Node.js QUIC Server"; - - // TLS cipher suite - std::string ciphers = DEFAULT_CIPHERS; - - // TLS groups - std::string groups = DEFAULT_GROUPS; - - // The TLS private key to use for this session. + // The TLS private key(s) to use for this session. + // JavaScript option name "keys" std::vector> keys; // Collection of certificates to use for this session. + // JavaScript option name "certs" std::vector certs; // Optional certificate authority overrides to use. + // JavaScript option name "ca" std::vector ca; // Optional certificate revocation lists to use. + // JavaScript option name "crl" std::vector crl; void MemoryInfo(MemoryTracker* tracker) const override; - SET_MEMORY_INFO_NAME(CryptoContext::Options) + SET_MEMORY_INFO_NAME(TLSContext::Options) SET_SELF_SIZE(Options) + // The default TLS configuration. static const Options kDefault; static v8::Maybe From(Environment* env, @@ -104,74 +175,57 @@ class TLSContext final : public MemoryRetainer { std::string ToString() const; }; - static const Options kDefaultOptions; + static std::shared_ptr CreateClient(Environment* env, + const Options& options); + static std::shared_ptr CreateServer(Environment* env, + const Options& options); - TLSContext(Environment* env, - Side side, - Session* session, - const Options& options); + TLSContext(Environment* env, Side side, const Options& options); TLSContext(const TLSContext&) = delete; TLSContext(TLSContext&&) = delete; TLSContext& operator=(const TLSContext&) = delete; TLSContext& operator=(TLSContext&&) = delete; - // Start the TLS handshake. - void Start(); + // Each QUIC Session has exactly one TLSSession. Each TLSSession maintains + // a reference to the TLSContext used to create it. + std::unique_ptr NewSession( + Session* session, const std::optional& maybeSessionTicket); - // TLS Keylogging is enabled per-Session by attaching a handler to the - // "keylog" event. Each keylog line is emitted to JavaScript where it can be - // routed to whatever destination makes sense. Typically, this will be to a - // keylog file that can be consumed by tools like Wireshark to intercept and - // decrypt QUIC network traffic. - void Keylog(const char* line) const; + inline Side side() const { return side_; } + inline const Options& options() const { return options_; } + inline operator bool() const { return ctx_ != nullptr; } - v8::MaybeLocal cert(Environment* env) const; - v8::MaybeLocal peer_cert(Environment* env) const; - v8::MaybeLocal cipher_name(Environment* env) const; - v8::MaybeLocal cipher_version(Environment* env) const; - v8::MaybeLocal ephemeral_key(Environment* env) const; - - // The SNI servername negotiated for the session - const std::string_view servername() const; - - // The ALPN (protocol name) negotiated for the session - const std::string_view alpn() const; - - // Triggers key update to begin. This will fail and return false if either a - // previous key update is in progress and has not been confirmed or if the - // initial handshake has not yet been confirmed. - bool InitiateKeyUpdate(); - - int VerifyPeerIdentity(); - - Side side() const; - const Options& options() const; - - int OnNewSession(SSL_SESSION* session); - - void MaybeSetEarlySession(const SessionTicket& sessionTicket); - bool early_data_was_accepted() const; + inline const std::string_view validation_error() const { + return validation_error_; + } void MemoryInfo(MemoryTracker* tracker) const override; - SET_MEMORY_INFO_NAME(CryptoContext) + SET_MEMORY_INFO_NAME(TLSContext) SET_SELF_SIZE(TLSContext) private: - static ngtcp2_conn* getConnection(ngtcp2_crypto_conn_ref* ref); - ngtcp2_crypto_conn_ref conn_ref_; + crypto::SSLCtxPointer Initialize(); + operator SSL_CTX*() const; + + static void OnKeylog(const SSL* ssl, const char* line); + static int OnNewSession(SSL* ssl, SSL_SESSION* session); + static int OnSelectAlpn(SSL* ssl, + const unsigned char** out, + unsigned char* outlen, + const unsigned char* in, + unsigned int inlen, + void* arg); + static int OnVerifyClientCertificate(int preverify_ok, X509_STORE_CTX* ctx); - Side side_; Environment* env_; - Session* session_; - const Options options_; - BaseObjectPtr secure_context_; - crypto::SSLPointer ssl_; - crypto::BIOPointer bio_trace_; - - bool in_key_update_ = false; - bool early_data_ = false; + Side side_; + Options options_; + crypto::X509Pointer cert_; + crypto::X509Pointer issuer_; + crypto::SSLCtxPointer ctx_; + std::string validation_error_ = ""; - friend class Session; + friend class TLSSession; }; } // namespace quic From 498abf2c9ffb57deec6d1b6e831d5a9c421e429b Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 31 Dec 2023 08:33:39 -0800 Subject: [PATCH 02/12] quic: simplify cc algorithm stuff a bit --- src/quic/endpoint.cc | 18 +++++------------- src/quic/endpoint.h | 14 ++++++++++---- .../test-quic-internal-endpoint-options.js | 16 ++++++++-------- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc index fb0a5873dea406..a596aa60cb33db 100644 --- a/src/quic/endpoint.cc +++ b/src/quic/endpoint.cc @@ -72,11 +72,6 @@ namespace quic { V(STATELESS_RESET_COUNT, stateless_reset_count) \ V(IMMEDIATE_CLOSE_COUNT, immediate_close_count) -#define ENDPOINT_CC(V) \ - V(RENO, reno) \ - V(CUBIC, cubic) \ - V(BBR, bbr) - struct Endpoint::State { #define V(_, name, type) type name; ENDPOINT_STATE(V) @@ -341,12 +336,9 @@ std::string Endpoint::Options::ToString() const { auto ccalg = ([&] { switch (cc_algorithm) { - case NGTCP2_CC_ALGO_RENO: - return "reno"; - case NGTCP2_CC_ALGO_CUBIC: - return "cubic"; - case NGTCP2_CC_ALGO_BBR: - return "bbr"; +#define V(name, label) case NGTCP2_CC_ALGO_##name: return #label; + ENDPOINT_CC(V) +#undef V } return ""; })(); @@ -623,8 +615,8 @@ void Endpoint::InitPerIsolate(IsolateData* data, Local target) { void Endpoint::InitPerContext(Realm* realm, Local target) { #define V(name, str) \ - NODE_DEFINE_CONSTANT(target, QUIC_CC_ALGO_##name); \ - NODE_DEFINE_STRING_CONSTANT(target, "QUIC_CC_ALGO_" #name "_STR", #str); + NODE_DEFINE_CONSTANT(target, CC_ALGO_##name); \ + NODE_DEFINE_STRING_CONSTANT(target, "CC_ALGO_" #name "_STR", #str); ENDPOINT_CC(V) #undef V diff --git a/src/quic/endpoint.h b/src/quic/endpoint.h index abed572e2e52a9..7ea0b07f79f469 100644 --- a/src/quic/endpoint.h +++ b/src/quic/endpoint.h @@ -20,6 +20,11 @@ namespace node { namespace quic { +#define ENDPOINT_CC(V) \ + V(RENO, reno) \ + V(CUBIC, cubic) \ + V(BBR, bbr) + // An Endpoint encapsulates the UDP local port binding and is responsible for // sending and receiving QUIC packets. A single endpoint can act as both a QUIC // client and server simultaneously. @@ -33,9 +38,10 @@ class Endpoint final : public AsyncWrap, public Packet::Listener { static constexpr uint64_t DEFAULT_MAX_STATELESS_RESETS = 10; static constexpr uint64_t DEFAULT_MAX_RETRY_LIMIT = 10; - static constexpr auto QUIC_CC_ALGO_RENO = NGTCP2_CC_ALGO_RENO; - static constexpr auto QUIC_CC_ALGO_CUBIC = NGTCP2_CC_ALGO_CUBIC; - static constexpr auto QUIC_CC_ALGO_BBR = NGTCP2_CC_ALGO_BBR; +#define V(name, _) \ + static constexpr auto CC_ALGO_##name = NGTCP2_CC_ALGO_##name; + ENDPOINT_CC(V) +#undef V // Endpoint configuration options struct Options final : public MemoryRetainer { @@ -144,7 +150,7 @@ class Endpoint final : public AsyncWrap, public Packet::Listener { // which to use by default is arbitrary and we can choose whichever we'd // like. Additional performance profiling will be needed to determine which // is the better of the two for our needs. - ngtcp2_cc_algo cc_algorithm = NGTCP2_CC_ALGO_CUBIC; + ngtcp2_cc_algo cc_algorithm = CC_ALGO_CUBIC; // By default, when the endpoint is created, it will generate a // reset_token_secret at random. This is a secret used in generating diff --git a/test/parallel/test-quic-internal-endpoint-options.js b/test/parallel/test-quic-internal-endpoint-options.js index 0d19841e5e8598..672fac18b43779 100644 --- a/test/parallel/test-quic-internal-endpoint-options.js +++ b/test/parallel/test-quic-internal-endpoint-options.js @@ -136,14 +136,14 @@ const cases = [ { key: 'cc', valid: [ - quic.QUIC_CC_ALGO_RENO, - quic.QUIC_CC_ALGO_CUBIC, - quic.QUIC_CC_ALGO_BBR, - quic.QUIC_CC_ALGO_BBR2, - quic.QUIC_CC_ALGO_RENO_STR, - quic.QUIC_CC_ALGO_CUBIC_STR, - quic.QUIC_CC_ALGO_BBR_STR, - quic.QUIC_CC_ALGO_BBR2_STR, + quic.CC_ALGO_RENO, + quic.CC_ALGO_CUBIC, + quic.CC_ALGO_BBR, + quic.CC_ALGO_BBR2, + quic.CC_ALGO_RENO_STR, + quic.CC_ALGO_CUBIC_STR, + quic.CC_ALGO_BBR_STR, + quic.CC_ALGO_BBR2_STR, ], invalid: [-1, 4, 1n, 'a', null, false, true, {}, [], () => {}], }, From 9033e16e8af48f7c65391e7cc66109d04baf1e70 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 31 Dec 2023 14:45:06 -0800 Subject: [PATCH 03/12] quic: getting application working, fixing bugs --- src/quic/application.cc | 41 ++++++++++++++++++++++++++++++++++++----- src/quic/application.h | 2 ++ src/quic/cid.cc | 5 +++-- src/quic/cid.h | 7 +++---- src/quic/endpoint.cc | 2 +- src/quic/session.cc | 32 ++++++++++++++++---------------- 6 files changed, 61 insertions(+), 28 deletions(-) diff --git a/src/quic/application.cc b/src/quic/application.cc index 8308f261daf7dd..24edb45a35637f 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -1,3 +1,4 @@ +#include "ngtcp2/ngtcp2.h" #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "application.h" @@ -95,6 +96,20 @@ Maybe Session::Application_Options::From( return Just(options); } +// ============================================================================ + +std::string Session::Application::StreamData::ToString() const { + DebugIndentScope indent; + auto prefix = indent.Prefix(); + std::string res("{"); + res += prefix + "count: " + std::to_string(count); + res += prefix + "remaining: " + std::to_string(remaining); + res += prefix + "id: " + std::to_string(id); + res += prefix + "fin: " + std::to_string(fin); + res += indent.Close(); + return res; +} + Session::Application::Application(Session* session, const Options& options) : session_(session) {} @@ -251,13 +266,15 @@ void Session::Application::SendPendingData() { }; for (;;) { - ssize_t ndatalen; + ssize_t ndatalen = 0; StreamData stream_data; err = GetStreamData(&stream_data); if (err < 0) { session_->last_error_ = QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL); + Debug(session_, "Application failed to get stream data with error %s", + session_->last_error_); return session_->Close(Session::CloseMethod::SILENT); } @@ -270,13 +287,19 @@ void Session::Application::SendPendingData() { pos = ngtcp2_vec(*packet).base; } + Debug(session_, "Application using stream data: %s", stream_data); + DCHECK_NOT_NULL(pos); + DCHECK_NOT_NULL(stream_data.buf); ssize_t nwrite = WriteVStream(&path, pos, &ndatalen, stream_data); + Debug(session_, "Application accepted %zu bytes", ndatalen); if (nwrite <= 0) { switch (nwrite) { - case 0: + case 0: { + Debug(session_, "Congestion limited. Sending nothing."); if (stream_data.id >= 0) ResumeStream(stream_data.id); return congestionLimited(std::move(packet)); + } case NGTCP2_ERR_STREAM_DATA_BLOCKED: { session().StreamDataBlocked(stream_data.id); if (session().max_data_left() == 0) { @@ -287,6 +310,8 @@ void Session::Application::SendPendingData() { continue; } case NGTCP2_ERR_STREAM_SHUT_WR: { + Debug(session_, "Stream %" PRIi64 " is closed for writing", + stream_data.id); // Indicates that the writable side of the stream has been closed // locally or the stream is being reset. In either case, we can't send // any stream data! @@ -299,6 +324,7 @@ void Session::Application::SendPendingData() { continue; } case NGTCP2_ERR_WRITE_MORE: { + Debug(session_, "Application should write more to packet"); CHECK_GT(ndatalen, 0); if (!StreamCommit(&stream_data, ndatalen)) return session_->Close(); pos += ndatalen; @@ -308,6 +334,8 @@ void Session::Application::SendPendingData() { packet->Done(UV_ECANCELED); session_->last_error_ = QuicError::ForNgtcp2Error(nwrite); + Debug(session_, "Application failed to write stream data with error %s", + session_->last_error_); return session_->Close(Session::CloseMethod::SILENT); } @@ -322,9 +350,10 @@ void Session::Application::SendPendingData() { if (stream_data.id >= 0 && ndatalen < 0) ResumeStream(stream_data.id); + Debug(session_, "Packet ready to send with %zu bytes", nwrite); packet->Truncate(nwrite); - session_->Send(std::move(packet), path); - + session_->Send(packet, path); + packet = nullptr; pos = nullptr; if (++packetSendCount == maxPacketCount) { @@ -343,10 +372,12 @@ ssize_t Session::Application::WriteVStream(PathStorage* path, uint32_t flags = NGTCP2_WRITE_STREAM_FLAG_NONE; if (stream_data.remaining > 0) flags |= NGTCP2_WRITE_STREAM_FLAG_MORE; if (stream_data.fin) flags |= NGTCP2_WRITE_STREAM_FLAG_FIN; + ngtcp2_pkt_info pi; + ssize_t ret = ngtcp2_conn_writev_stream( *session_, &path->path, - nullptr, + &pi, buf, ngtcp2_conn_get_max_tx_udp_payload_size(*session_), ndatalen, diff --git a/src/quic/application.h b/src/quic/application.h index 5ecaede68e1c01..176704d802229b 100644 --- a/src/quic/application.h +++ b/src/quic/application.h @@ -151,6 +151,8 @@ struct Session::Application::StreamData final { BaseObjectPtr stream; inline operator nghttp3_vec() const { return {data[0].base, data[0].len}; } + + std::string ToString() const; }; } // namespace quic diff --git a/src/quic/cid.cc b/src/quic/cid.cc index e23c9357340ec6..17a0d70e981d4f 100644 --- a/src/quic/cid.cc +++ b/src/quic/cid.cc @@ -114,8 +114,8 @@ class RandomCIDFactory : public CID::Factory { return CID(start, length_hint); } - void GenerateInto(ngtcp2_cid* cid, - size_t length_hint = CID::kMaxLength) const override { + CID GenerateInto(ngtcp2_cid* cid, + size_t length_hint = CID::kMaxLength) const override { DCHECK_GE(length_hint, CID::kMinLength); DCHECK_LE(length_hint, CID::kMaxLength); Mutex::ScopedLock lock(mutex_); @@ -123,6 +123,7 @@ class RandomCIDFactory : public CID::Factory { auto start = pool_ + pos_; pos_ += length_hint; ngtcp2_cid_init(cid, start, length_hint); + return CID(cid); } private: diff --git a/src/quic/cid.h b/src/quic/cid.h index a3aa0e750f303a..00a5d254666b3f 100644 --- a/src/quic/cid.h +++ b/src/quic/cid.h @@ -111,10 +111,9 @@ class CID::Factory { virtual CID Generate(size_t length_hint = CID::kMaxLength) const = 0; // Generate a new CID into the given ngtcp2_cid. This variation of - // Generate should be used far less commonly. It is provided largely - // for a couple of internal cases. - virtual void GenerateInto(ngtcp2_cid* cid, - size_t length_hint = CID::kMaxLength) const = 0; + // Generate should be used far less commonly. + virtual CID GenerateInto(ngtcp2_cid* cid, + size_t length_hint = CID::kMaxLength) const = 0; // The default random CID generator instance. static const Factory& random(); diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc index a596aa60cb33db..cd211cc5220e80 100644 --- a/src/quic/endpoint.cc +++ b/src/quic/endpoint.cc @@ -1236,7 +1236,7 @@ void Endpoint::Receive(const uv_buf_t& buf, // identifies us. config.dcid should equal scid. config.scid should *not* // equal dcid. DCHECK(config.dcid == scid); - DCHECK(config.scid != dcid); + DCHECK(config.scid == dcid); const auto is_remote_address_validated = ([&] { auto info = addrLRU_.Peek(remote_address); diff --git a/src/quic/session.cc b/src/quic/session.cc index c7d89c7731a855..bb8ec48c4a4a87 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -1406,7 +1406,7 @@ void Session::DatagramReceived(const uint8_t* data, bool Session::GenerateNewConnectionId(ngtcp2_cid* cid, size_t len, uint8_t* token) { - CID cid_ = config_.options.cid_factory->Generate(len); + CID cid_ = config_.options.cid_factory->GenerateInto(cid, len); Debug(this, "Generated new connection id %s", cid_); StatelessResetToken new_token( token, endpoint_->options().reset_token_secret, cid_); @@ -1416,10 +1416,11 @@ bool Session::GenerateNewConnectionId(ngtcp2_cid* cid, } bool Session::HandshakeCompleted() { + Debug(this, "Session handshake completed"); + if (state_->handshake_completed) return false; - state_->handshake_completed = true; + state_->handshake_completed = 1; - Debug(this, "Session handshake completed"); STAT_RECORD_TIMESTAMP(Stats, handshake_completed_at); if (!tls_session().early_data_was_accepted()) @@ -2012,17 +2013,17 @@ struct Session::Impl { void* user_data) { auto session = Impl::From(conn, user_data); if (UNLIKELY(session->is_destroyed())) return NGTCP2_ERR_CALLBACK_FAILURE; + CHECK(!session->is_server()); + + if (level != NGTCP2_ENCRYPTION_LEVEL_1RTT) return NGTCP2_SUCCESS; Debug(session, "Receiving RX key for level %d for dcid %s", to_string(level), session->config().dcid); - if (!session->is_server() && (level == NGTCP2_ENCRYPTION_LEVEL_0RTT || - level == NGTCP2_ENCRYPTION_LEVEL_1RTT)) { - if (!session->application().Start()) return NGTCP2_ERR_CALLBACK_FAILURE; - } - return NGTCP2_SUCCESS; + return session->application().Start() ? + NGTCP2_SUCCESS : NGTCP2_ERR_CALLBACK_FAILURE; } static int on_receive_stateless_reset(ngtcp2_conn* conn, @@ -2071,17 +2072,16 @@ struct Session::Impl { void* user_data) { auto session = Impl::From(conn, user_data); if (UNLIKELY(session->is_destroyed())) return NGTCP2_ERR_CALLBACK_FAILURE; + CHECK(session->is_server()); + + if (level != NGTCP2_ENCRYPTION_LEVEL_1RTT) return NGTCP2_SUCCESS; Debug(session, "Receiving TX key for level %d for dcid %s", to_string(level), session->config().dcid); - - if (session->is_server() && (level == NGTCP2_ENCRYPTION_LEVEL_0RTT || - level == NGTCP2_ENCRYPTION_LEVEL_1RTT)) { - if (!session->application().Start()) return NGTCP2_ERR_CALLBACK_FAILURE; - } - return NGTCP2_SUCCESS; + return session->application().Start() ? + NGTCP2_SUCCESS : NGTCP2_ERR_CALLBACK_FAILURE; } static int on_receive_version_negotiation(ngtcp2_conn* conn, @@ -2206,7 +2206,7 @@ struct Session::Impl { on_stream_stop_sending, ngtcp2_crypto_version_negotiation_cb, on_receive_rx_key, - on_receive_tx_key, + nullptr, on_early_data_rejected}; static constexpr ngtcp2_callbacks SERVER = { @@ -2247,7 +2247,7 @@ struct Session::Impl { ngtcp2_crypto_get_path_challenge_data_cb, on_stream_stop_sending, ngtcp2_crypto_version_negotiation_cb, - on_receive_rx_key, + nullptr, on_receive_tx_key, on_early_data_rejected}; }; From 8e4797e7e39d5137f19403724be7b0992dfc97b2 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 1 Jan 2024 14:17:57 -0800 Subject: [PATCH 04/12] quic: update the packet generation logic --- src/quic/application.cc | 185 +++++++++++++++++++++++----------------- src/quic/application.h | 1 + src/quic/data.cc | 20 ++++- src/quic/data.h | 6 ++ src/quic/http3.cc | 3 + src/quic/packet.cc | 2 +- src/quic/session.cc | 13 ++- src/quic/session.h | 3 + 8 files changed, 149 insertions(+), 84 deletions(-) diff --git a/src/quic/application.cc b/src/quic/application.cc index 24edb45a35637f..0bac5a78ec40f4 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -204,7 +204,7 @@ Packet* Session::Application::CreateStreamDataPacket() { return Packet::Create(env(), session_->endpoint_.get(), session_->remote_address_, - ngtcp2_conn_get_max_tx_udp_payload_size(*session_), + session_->max_packet_size(), "stream data"); } @@ -238,37 +238,33 @@ void Session::Application::StreamReset(Stream* stream, void Session::Application::SendPendingData() { Debug(session_, "Application sending pending data"); PathStorage path; + PathStorage prev_path; Packet* packet = nullptr; uint8_t* pos = nullptr; + uint8_t* begin = nullptr; int err = 0; + size_t last_nwrite = 0; - size_t maxPacketCount = std::min(static_cast(64000), - ngtcp2_conn_get_send_quantum(*session_)); - size_t packetSendCount = 0; + // The maximum size of packet to create. + const size_t max_packet_size = session_->max_packet_size(); - const auto updateTimer = [&] { - Debug(session_, "Application updating the session timer"); - ngtcp2_conn_update_pkt_tx_time(*session_, uv_hrtime()); - session_->UpdateTimer(); - }; - - const auto congestionLimited = [&](auto packet) { - auto len = pos - ngtcp2_vec(*packet).base; - // We are either congestion limited or done. - if (len) { - // Some data was serialized into the packet. We need to send it. - packet->Truncate(len); - session_->Send(std::move(packet), path); - } + // The maximum number of packets to send in this call to SendPendingData. + const size_t max_packet_count = std::min( + static_cast(64000), + ngtcp2_conn_get_send_quantum(*session_) / max_packet_size); - updateTimer(); - }; + // The number of packets that have been sent in this call to SendPendingData. + size_t packet_send_count = 0; for (;;) { + // ndatalen is the amount of stream data that was accepted into the packet, + // if any. ssize_t ndatalen = 0; - StreamData stream_data; + // The stream_data is the next block of data from the application stream to + // send. + StreamData stream_data; err = GetStreamData(&stream_data); if (err < 0) { @@ -278,115 +274,148 @@ void Session::Application::SendPendingData() { return session_->Close(Session::CloseMethod::SILENT); } + // If we got here, we were at least successful in checking for stream data. + // There might not be any stream data to send. + Debug(session_, "Application using stream data: %s", stream_data); + + // Now let's make sure we have a packet to write data into. if (packet == nullptr) { packet = CreateStreamDataPacket(); if (packet == nullptr) { session_->last_error_ = QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL); return session_->Close(Session::CloseMethod::SILENT); } - pos = ngtcp2_vec(*packet).base; + pos = begin = ngtcp2_vec(*packet).base; } - Debug(session_, "Application using stream data: %s", stream_data); DCHECK_NOT_NULL(pos); + DCHECK_NOT_NULL(begin); DCHECK_NOT_NULL(stream_data.buf); - ssize_t nwrite = WriteVStream(&path, pos, &ndatalen, stream_data); - Debug(session_, "Application accepted %zu bytes", ndatalen); - if (nwrite <= 0) { + // Awesome, let's write our packet! + ssize_t nwrite = WriteVStream(&path, pos, &ndatalen, max_packet_size, stream_data); + Debug(session_, "Application accepted %zu bytes into packet", ndatalen); + + // A negative nwrite value indicates either an error or that there is more data + // to write into the packet. + if (nwrite < 0) { switch (nwrite) { - case 0: { - Debug(session_, "Congestion limited. Sending nothing."); - if (stream_data.id >= 0) ResumeStream(stream_data.id); - return congestionLimited(std::move(packet)); - } case NGTCP2_ERR_STREAM_DATA_BLOCKED: { - session().StreamDataBlocked(stream_data.id); - if (session().max_data_left() == 0) { - if (stream_data.id >= 0) ResumeStream(stream_data.id); - return congestionLimited(std::move(packet)); - } - CHECK_LE(ndatalen, 0); + // We could not write any data for this stream into the packet because + // the flow control for the stream itself indicates that the stream + // is blocked. We'll skip and move on to the next stream. + DCHECK_EQ(ndatalen, -1); + session_->StreamDataBlocked(stream_data.id); continue; } case NGTCP2_ERR_STREAM_SHUT_WR: { - Debug(session_, "Stream %" PRIi64 " is closed for writing", - stream_data.id); - // Indicates that the writable side of the stream has been closed + // Indicates that the writable side of the stream should be closed // locally or the stream is being reset. In either case, we can't send // any stream data! - CHECK_GE(stream_data.id, 0); - // We need to notify the stream that the writable side has been closed - // and no more outbound data can be sent. - CHECK_LE(ndatalen, 0); - auto stream = session_->FindStream(stream_data.id); - if (stream) stream->EndWritable(); + Debug(session_, "Stream %" PRIi64 " should be closed for writing", + stream_data.id); + DCHECK_EQ(ndatalen, -1); + // If we got this response, then there should have been a stream associated + // with the stream_data, otherwise the response wouldn't make any sense. + DCHECK(stream_data.stream); + stream_data.stream->EndWritable(); continue; } case NGTCP2_ERR_WRITE_MORE: { Debug(session_, "Application should write more to packet"); - CHECK_GT(ndatalen, 0); - if (!StreamCommit(&stream_data, ndatalen)) return session_->Close(); - pos += ndatalen; + DCHECK_GE(ndatalen, 0); + if (!StreamCommit(&stream_data, ndatalen)) { + packet->Done(UV_ECANCELED); + return session_->Close(CloseMethod::SILENT); + } continue; } } + // Some other type of error happened. + DCHECK_EQ(ndatalen, -1); + Debug(session_, "Application encountered error while writing packet: %s", + ngtcp2_strerror(nwrite)); packet->Done(UV_ECANCELED); - session_->last_error_ = QuicError::ForNgtcp2Error(nwrite); - Debug(session_, "Application failed to write stream data with error %s", - session_->last_error_); + session_->SetLastError(QuicError::ForNgtcp2Error(nwrite)); return session_->Close(Session::CloseMethod::SILENT); + } else if (ndatalen >= 0) { + // We wrote some data into the packet. We need to update the flow control + // by committing the data. + if (!StreamCommit(&stream_data, ndatalen)) { + packet->Done(UV_ECANCELED); + return session_->Close(CloseMethod::SILENT); + } } - pos += nwrite; - if (ndatalen > 0 && !StreamCommit(&stream_data, ndatalen)) { - // Since we are closing the session here, we don't worry about updating - // the pkt tx time. The failed StreamCommit should have updated the - // last_error_ appropriately. - packet->Done(UV_ECANCELED); - return session_->Close(Session::CloseMethod::SILENT); + // When nwrite is zero, it means we are congestion limited. + if (nwrite == 0) { + Debug(session_, "Congestion limited."); + // There might be a partial packet already prepared. If so, send it. + if (pos - begin) { + Debug(session_, "Packet has at least some data to send"); + // At least some data had been written into the packet. We should send it. + packet->Truncate(pos - begin); + session_->Send(packet, path); + } else { + packet->Done(UV_ECANCELED); + } + + session_->UpdatePacketTxTime(); + return; } - if (stream_data.id >= 0 && ndatalen < 0) ResumeStream(stream_data.id); + pos += nwrite; - Debug(session_, "Packet ready to send with %zu bytes", nwrite); - packet->Truncate(nwrite); - session_->Send(packet, path); - packet = nullptr; - pos = nullptr; + if (packet_send_count == 0) { + path.CopyTo(&prev_path); + last_nwrite = nwrite; + } else if (prev_path != path || + static_cast(nwrite) > last_nwrite || + (last_nwrite > max_packet_size && static_cast(nwrite) != last_nwrite)) { + auto datalen = pos - begin - nwrite; + packet->Truncate(datalen); + session_->Send(packet->Clone(), prev_path); + session_->Send(packet, path); + session_->UpdatePacketTxTime(); + packet = nullptr; + pos = nullptr; + begin = nullptr; + continue; + } - if (++packetSendCount == maxPacketCount) { - break; + if (++packet_send_count == max_packet_count || static_cast(nwrite) < last_nwrite) { + auto datalen = pos - begin; + packet->Truncate(datalen); + session_->Send(packet, path); + session_->UpdatePacketTxTime(); + return; } } - - updateTimer(); } ssize_t Session::Application::WriteVStream(PathStorage* path, - uint8_t* buf, + uint8_t* dest, ssize_t* ndatalen, + size_t max_packet_size, const StreamData& stream_data) { - CHECK_LE(stream_data.count, kMaxVectorCount); - uint32_t flags = NGTCP2_WRITE_STREAM_FLAG_NONE; - if (stream_data.remaining > 0) flags |= NGTCP2_WRITE_STREAM_FLAG_MORE; + DCHECK_LE(stream_data.count, kMaxVectorCount); + uint32_t flags = NGTCP2_WRITE_STREAM_FLAG_MORE; if (stream_data.fin) flags |= NGTCP2_WRITE_STREAM_FLAG_FIN; ngtcp2_pkt_info pi; - - ssize_t ret = ngtcp2_conn_writev_stream( + Debug(session_, "Writing max %zu bytes to packet", max_packet_size); + return ngtcp2_conn_writev_stream( *session_, &path->path, &pi, - buf, - ngtcp2_conn_get_max_tx_udp_payload_size(*session_), + dest, + max_packet_size, ndatalen, flags, stream_data.id, stream_data.buf, stream_data.count, uv_hrtime()); - return ret; } // The DefaultApplication is the default implementation of Session::Application diff --git a/src/quic/application.h b/src/quic/application.h index 176704d802229b..cbd9b37cf684f9 100644 --- a/src/quic/application.h +++ b/src/quic/application.h @@ -132,6 +132,7 @@ class Session::Application : public MemoryRetainer { ssize_t WriteVStream(PathStorage* path, uint8_t* buf, ssize_t* ndatalen, + size_t max_packet_size, const StreamData& stream_data); private: diff --git a/src/quic/data.cc b/src/quic/data.cc index c1216189219890..e1e69808338fc8 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -47,13 +47,27 @@ std::string Path::ToString() const { return res; } -PathStorage::PathStorage() { - ngtcp2_path_storage_zero(this); -} +PathStorage::PathStorage() { Reset(); } PathStorage::operator ngtcp2_path() { return path; } +void PathStorage::Reset() { + ngtcp2_path_storage_zero(this); +} + +void PathStorage::CopyTo(PathStorage* path) const { + ngtcp2_path_copy(&path->path, &this->path); +} + +bool PathStorage::operator==(const PathStorage& other) const { + return ngtcp2_path_eq(&path, &other.path) != 0; +} + +bool PathStorage::operator!=(const PathStorage& other) const { + return ngtcp2_path_eq(&path, &other.path) == 0; +} + // ============================================================================ Store::Store(std::shared_ptr store, diff --git a/src/quic/data.h b/src/quic/data.h index db715235bd768c..62c6bb2b153ebf 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -24,6 +24,12 @@ struct Path final : public ngtcp2_path { struct PathStorage final : public ngtcp2_path_storage { PathStorage(); operator ngtcp2_path(); + + void Reset(); + void CopyTo(PathStorage* path) const; + + bool operator==(const PathStorage& other) const; + bool operator!=(const PathStorage& other) const; }; class Store final : public MemoryRetainer { diff --git a/src/quic/http3.cc b/src/quic/http3.cc index eed0b2619327fd..ea5e003d93285f 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -362,6 +362,9 @@ class Http3Application final : public Session::Application { return static_cast(ret); } else { data->remaining = data->count = static_cast(ret); + if (data->id > 0) { + data->stream = session().FindStream(data->id); + } } } return 0; diff --git a/src/quic/packet.cc b/src/quic/packet.cc index dad5b59ad3cbe5..c43ac99b2442cd 100644 --- a/src/quic/packet.cc +++ b/src/quic/packet.cc @@ -161,7 +161,7 @@ Packet* Packet::FromFreeList(Environment* env, CHECK_NOT_NULL(packet); CHECK_EQ(env, packet->env()); Debug(packet, "Reusing packet from freelist"); - packet->data_ = data; + packet->data_ = std::move(data); packet->destination_ = destination; packet->listener_ = listener; return packet; diff --git a/src/quic/session.cc b/src/quic/session.cc index bb8ec48c4a4a87..71add7472d49f7 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -572,6 +572,10 @@ Session::~Session() { DCHECK(streams_.empty()); } +size_t Session::max_packet_size() const { + return ngtcp2_conn_get_max_tx_udp_payload_size(*this); +} + Session::operator ngtcp2_conn*() const { return connection_.get(); } @@ -862,6 +866,10 @@ void Session::Send(Packet* packet, const PathStorage& path) { Send(packet); } +void Session::UpdatePacketTxTime() { + ngtcp2_conn_update_pkt_tx_time(*this, uv_hrtime()); +} + uint64_t Session::SendDatagram(Store&& data) { auto tp = ngtcp2_conn_get_remote_transport_params(*this); uint64_t max_datagram_size = tp->max_datagram_frame_size; @@ -1322,8 +1330,8 @@ void Session::OnTimeout() { if (is_destroyed()) return; int ret = ngtcp2_conn_handle_expiry(*this, uv_hrtime()); - if (NGTCP2_OK(ret) && !is_in_closing_period() && !is_in_draining_period() && - env()->can_call_into_js()) { + if (NGTCP2_OK(ret) && !is_in_closing_period() && !is_in_draining_period()) { + Debug(this, "Sending pending data after timr expiry"); SendPendingDataScope send_scope(this); return; } @@ -1346,6 +1354,7 @@ void Session::UpdateTimer() { } auto timeout = (expiry - now) / NGTCP2_MILLISECONDS; + Debug(this, "Updating timeout to %zu milliseconds", timeout); // If timeout is zero here, it means our timer is less than a millisecond // off from expiry. Let's bump the timer to 1. diff --git a/src/quic/session.h b/src/quic/session.h index ec7c525206b9d2..8dc0f8cc444b18 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -235,6 +235,8 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { bool is_destroyed() const; bool is_server() const; + size_t max_packet_size() const; + void set_priority_supported(bool on = true); std::string diagnostic_name() const override; @@ -246,6 +248,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { TransportParams GetLocalTransportParams() const; TransportParams GetRemoteTransportParams() const; + void UpdatePacketTxTime(); void MemoryInfo(MemoryTracker* tracker) const override; SET_MEMORY_INFO_NAME(Session) From 973c50565739fa7d999c35ac41156a84a6de13ef Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 1 Jan 2024 15:09:54 -0800 Subject: [PATCH 05/12] quic: fixup application packet preparation --- src/quic/application.cc | 125 ++++++++++++++++++++-------------------- src/quic/application.h | 15 +++-- src/quic/http3.cc | 1 + 3 files changed, 70 insertions(+), 71 deletions(-) diff --git a/src/quic/application.cc b/src/quic/application.cc index 0bac5a78ec40f4..4baf81e88fe5b3 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -236,41 +236,57 @@ void Session::Application::StreamReset(Stream* stream, } void Session::Application::SendPendingData() { + static constexpr size_t kMaxPackets = 32; Debug(session_, "Application sending pending data"); PathStorage path; - PathStorage prev_path; - - Packet* packet = nullptr; - uint8_t* pos = nullptr; - uint8_t* begin = nullptr; - int err = 0; - size_t last_nwrite = 0; + StreamData stream_data; // The maximum size of packet to create. const size_t max_packet_size = session_->max_packet_size(); // The maximum number of packets to send in this call to SendPendingData. const size_t max_packet_count = std::min( - static_cast(64000), + kMaxPackets, ngtcp2_conn_get_send_quantum(*session_) / max_packet_size); // The number of packets that have been sent in this call to SendPendingData. size_t packet_send_count = 0; + Packet* packet = nullptr; + uint8_t* pos = nullptr; + uint8_t* begin = nullptr; + + auto ensure_packet = [&] { + if (packet == nullptr) { + packet = CreateStreamDataPacket(); + if (packet == nullptr) return false; + pos = begin = ngtcp2_vec(*packet).base; + } + DCHECK_NOT_NULL(packet); + DCHECK_NOT_NULL(pos); + DCHECK_NOT_NULL(begin); + return true; + }; + + // We're going to enter a loop here to prepare and send no more than + // max_packet_count packets. for (;;) { - // ndatalen is the amount of stream data that was accepted into the packet, - // if any. + // ndatalen is the amount of stream data that was accepted into the packet. ssize_t ndatalen = 0; - // The stream_data is the next block of data from the application stream to - // send. - StreamData stream_data; - err = GetStreamData(&stream_data); + // Make sure we have a packet to write data into. + if (!ensure_packet()) { + Debug(session_, "Failed to create packet for stream data"); + // Doh! Could not create a packet. Time to bail. + session_->last_error_ = QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL); + return session_->Close(Session::CloseMethod::SILENT); + } - if (err < 0) { + // The stream_data is the next block of data from the application stream. + if (GetStreamData(&stream_data) < 0) { + Debug(session_, "Application failed to get stream data"); session_->last_error_ = QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL); - Debug(session_, "Application failed to get stream data with error %s", - session_->last_error_); + packet->Done(UV_ECANCELED); return session_->Close(Session::CloseMethod::SILENT); } @@ -278,20 +294,6 @@ void Session::Application::SendPendingData() { // There might not be any stream data to send. Debug(session_, "Application using stream data: %s", stream_data); - // Now let's make sure we have a packet to write data into. - if (packet == nullptr) { - packet = CreateStreamDataPacket(); - if (packet == nullptr) { - session_->last_error_ = QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL); - return session_->Close(Session::CloseMethod::SILENT); - } - pos = begin = ngtcp2_vec(*packet).base; - } - - DCHECK_NOT_NULL(pos); - DCHECK_NOT_NULL(begin); - DCHECK_NOT_NULL(stream_data.buf); - // Awesome, let's write our packet! ssize_t nwrite = WriteVStream(&path, pos, &ndatalen, max_packet_size, stream_data); Debug(session_, "Application accepted %zu bytes into packet", ndatalen); @@ -304,7 +306,10 @@ void Session::Application::SendPendingData() { // We could not write any data for this stream into the packet because // the flow control for the stream itself indicates that the stream // is blocked. We'll skip and move on to the next stream. + // ndatalen = -1 means that no stream data was accepted into the + // packet, which is what we want here. DCHECK_EQ(ndatalen, -1); + DCHECK(stream_data.stream); session_->StreamDataBlocked(stream_data.id); continue; } @@ -314,14 +319,16 @@ void Session::Application::SendPendingData() { // any stream data! Debug(session_, "Stream %" PRIi64 " should be closed for writing", stream_data.id); + // ndatalen = -1 means that no stream data was accepted into the + // packet, which is what we want here. DCHECK_EQ(ndatalen, -1); - // If we got this response, then there should have been a stream associated - // with the stream_data, otherwise the response wouldn't make any sense. DCHECK(stream_data.stream); stream_data.stream->EndWritable(); continue; } case NGTCP2_ERR_WRITE_MORE: { + // This return value indicates that we should call into WriteVStream + // again to write more data into the same packet. Debug(session_, "Application should write more to packet"); DCHECK_GE(ndatalen, 0); if (!StreamCommit(&stream_data, ndatalen)) { @@ -336,8 +343,8 @@ void Session::Application::SendPendingData() { DCHECK_EQ(ndatalen, -1); Debug(session_, "Application encountered error while writing packet: %s", ngtcp2_strerror(nwrite)); - packet->Done(UV_ECANCELED); session_->SetLastError(QuicError::ForNgtcp2Error(nwrite)); + packet->Done(UV_ECANCELED); return session_->Close(Session::CloseMethod::SILENT); } else if (ndatalen >= 0) { // We wrote some data into the packet. We need to update the flow control @@ -349,48 +356,41 @@ void Session::Application::SendPendingData() { } // When nwrite is zero, it means we are congestion limited. + // We should stop trying to send additional packets. if (nwrite == 0) { Debug(session_, "Congestion limited."); // There might be a partial packet already prepared. If so, send it. - if (pos - begin) { - Debug(session_, "Packet has at least some data to send"); + size_t datalen = pos - begin; + if (datalen) { + Debug(session_, "Packet has %zu bytes to send", datalen); // At least some data had been written into the packet. We should send it. - packet->Truncate(pos - begin); + packet->Truncate(datalen); session_->Send(packet, path); } else { packet->Done(UV_ECANCELED); } - session_->UpdatePacketTxTime(); - return; + // If there was stream data selected, we should reschedule it to try sending again. + if (stream_data.id >= 0) ResumeStream(stream_data.id); + + return session_->UpdatePacketTxTime(); } + // At this point we have a packet prepared to send. pos += nwrite; - - if (packet_send_count == 0) { - path.CopyTo(&prev_path); - last_nwrite = nwrite; - } else if (prev_path != path || - static_cast(nwrite) > last_nwrite || - (last_nwrite > max_packet_size && static_cast(nwrite) != last_nwrite)) { - auto datalen = pos - begin - nwrite; - packet->Truncate(datalen); - session_->Send(packet->Clone(), prev_path); - session_->Send(packet, path); - session_->UpdatePacketTxTime(); - packet = nullptr; - pos = nullptr; - begin = nullptr; - continue; + size_t datalen = pos - begin; + Debug(session_, "Sending packet with %zu bytes", datalen); + packet->Truncate(datalen); + session_->Send(packet, path); + + // If we have sent the maximum number of packets, we're done. + if (++packet_send_count == max_packet_count) { + return session_->UpdatePacketTxTime(); } - if (++packet_send_count == max_packet_count || static_cast(nwrite) < last_nwrite) { - auto datalen = pos - begin; - packet->Truncate(datalen); - session_->Send(packet, path); - session_->UpdatePacketTxTime(); - return; - } + // Prepare to loop back around to prepare a new packet. + packet = nullptr; + pos = begin = nullptr; } } @@ -403,7 +403,6 @@ ssize_t Session::Application::WriteVStream(PathStorage* path, uint32_t flags = NGTCP2_WRITE_STREAM_FLAG_MORE; if (stream_data.fin) flags |= NGTCP2_WRITE_STREAM_FLAG_FIN; ngtcp2_pkt_info pi; - Debug(session_, "Writing max %zu bytes to packet", max_packet_size); return ngtcp2_conn_writev_stream( *session_, &path->path, diff --git a/src/quic/application.h b/src/quic/application.h index cbd9b37cf684f9..7f18339f2abcbc 100644 --- a/src/quic/application.h +++ b/src/quic/application.h @@ -115,19 +115,19 @@ class Session::Application : public MemoryRetainer { // the default stream priority. virtual StreamPriority GetStreamPriority(const Stream& stream); - protected: - inline Environment* env() const { return session_->env(); } - inline Session& session() { return *session_; } - inline const Session& session() const { return *session_; } - - Packet* CreateStreamDataPacket(); - struct StreamData; virtual int GetStreamData(StreamData* data) = 0; virtual bool StreamCommit(StreamData* data, size_t datalen) = 0; virtual bool ShouldSetFin(const StreamData& data) = 0; + inline Environment* env() const { return session_->env(); } + inline Session& session() { return *session_; } + inline const Session& session() const { return *session_; } + + private: + Packet* CreateStreamDataPacket(); + // Write the given stream_data into the buffer. ssize_t WriteVStream(PathStorage* path, uint8_t* buf, @@ -135,7 +135,6 @@ class Session::Application : public MemoryRetainer { size_t max_packet_size, const StreamData& stream_data); - private: Session* session_; }; diff --git a/src/quic/http3.cc b/src/quic/http3.cc index ea5e003d93285f..210de9a69b852e 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -367,6 +367,7 @@ class Http3Application final : public Session::Application { } } } + DCHECK_NOT_NULL(stream_data.buf); return 0; } From d8852b84391c3786711f40b6c927a8da1a1947f7 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 6 Jan 2024 10:22:43 -0800 Subject: [PATCH 06/12] quic: various minor cleanups --- src/quic/application.cc | 2 +- src/quic/application.h | 7 ++-- src/quic/bindingdata.h | 66 +++------------------------------ src/quic/data.cc | 19 +++++++--- src/quic/data.h | 17 ++++----- src/quic/defs.h | 81 ++++++++++++++++++++++++++++++++++++++--- src/quic/session.h | 2 - src/quic/tlscontext.h | 7 ++-- 8 files changed, 110 insertions(+), 91 deletions(-) diff --git a/src/quic/application.cc b/src/quic/application.cc index 4baf81e88fe5b3..834737c4cd1dd1 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -1,4 +1,3 @@ -#include "ngtcp2/ngtcp2.h" #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "application.h" @@ -6,6 +5,7 @@ #include #include #include +#include #include #include #include "defs.h" diff --git a/src/quic/application.h b/src/quic/application.h index 7f18339f2abcbc..fc228fafe357e5 100644 --- a/src/quic/application.h +++ b/src/quic/application.h @@ -1,9 +1,11 @@ #pragma once +#include "quic/defs.h" #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "bindingdata.h" +#include "defs.h" #include "session.h" #include "sessionticket.h" #include "streams.h" @@ -18,10 +20,7 @@ class Session::Application : public MemoryRetainer { using Options = Session::Application_Options; Application(Session* session, const Options& options); - Application(const Application&) = delete; - Application(Application&&) = delete; - Application& operator=(const Application&) = delete; - Application& operator=(Application&&) = delete; + DISALLOW_COPY_AND_MOVE(Application) virtual bool Start(); diff --git a/src/quic/bindingdata.h b/src/quic/bindingdata.h index 18577d2472868b..f5c19d1fbb8498 100644 --- a/src/quic/bindingdata.h +++ b/src/quic/bindingdata.h @@ -12,9 +12,9 @@ #include #include #include -#include #include #include +#include "defs.h" namespace node { namespace quic { @@ -22,61 +22,6 @@ namespace quic { class Endpoint; class Packet; -enum class Side { - CLIENT, - SERVER, -}; - -enum class EndpointLabel { - LOCAL, - REMOTE, -}; - -enum class Direction { - BIDIRECTIONAL, - UNIDIRECTIONAL, -}; - -enum class HeadersKind { - HINTS, - INITIAL, - TRAILING, -}; - -enum class HeadersFlags { - NONE, - TERMINAL, -}; - -enum class StreamPriority { - DEFAULT = NGHTTP3_DEFAULT_URGENCY, - LOW = NGHTTP3_URGENCY_LOW, - HIGH = NGHTTP3_URGENCY_HIGH, -}; - -enum class StreamPriorityFlags { - NONE, - NON_INCREMENTAL, -}; - -enum class PathValidationResult : uint8_t { - SUCCESS = NGTCP2_PATH_VALIDATION_RESULT_SUCCESS, - FAILURE = NGTCP2_PATH_VALIDATION_RESULT_FAILURE, - ABORTED = NGTCP2_PATH_VALIDATION_RESULT_ABORTED, -}; - -enum class DatagramStatus { - ACKNOWLEDGED, - LOST, -}; - -constexpr uint64_t NGTCP2_APP_NOERROR = 65280; -constexpr size_t kDefaultMaxPacketLength = NGTCP2_MAX_UDP_PAYLOAD_SIZE; -constexpr size_t kMaxSizeT = std::numeric_limits::max(); -constexpr uint64_t kMaxSafeJsInteger = 9007199254740991; -constexpr auto kSocketAddressInfoTimeout = 60 * NGTCP2_SECONDS; -constexpr size_t kMaxVectorCount = 16; - // ============================================================================ // The FunctionTemplates the BindingData will store for us. @@ -208,6 +153,7 @@ class BindingData final static BindingData& Get(Environment* env); BindingData(Realm* realm, v8::Local object); + DISALLOW_COPY_AND_MOVE(BindingData) void MemoryInfo(MemoryTracker* tracker) const override; SET_MEMORY_INFO_NAME(BindingData) @@ -287,6 +233,7 @@ void IllegalConstructor(const v8::FunctionCallbackInfo& args); struct NgTcp2CallbackScope { Environment* env; explicit NgTcp2CallbackScope(Environment* env); + DISALLOW_COPY_AND_MOVE(NgTcp2CallbackScope) ~NgTcp2CallbackScope(); static bool in_ngtcp2_callback(Environment* env); }; @@ -294,6 +241,7 @@ struct NgTcp2CallbackScope { struct NgHttp3CallbackScope { Environment* env; explicit NgHttp3CallbackScope(Environment* env); + DISALLOW_COPY_AND_MOVE(NgHttp3CallbackScope) ~NgHttp3CallbackScope(); static bool in_nghttp3_callback(Environment* env); }; @@ -304,10 +252,7 @@ struct CallbackScopeBase { v8::TryCatch try_catch; explicit CallbackScopeBase(Environment* env); - CallbackScopeBase(const CallbackScopeBase&) = delete; - CallbackScopeBase(CallbackScopeBase&&) = delete; - CallbackScopeBase& operator=(const CallbackScopeBase&) = delete; - CallbackScopeBase& operator=(CallbackScopeBase&&) = delete; + DISALLOW_COPY_AND_MOVE(CallbackScopeBase) ~CallbackScopeBase(); }; @@ -318,6 +263,7 @@ struct CallbackScope final : public CallbackScopeBase { BaseObjectPtr ref; explicit CallbackScope(const T* ptr) : CallbackScopeBase(ptr->env()), ref(ptr) {} + DISALLOW_COPY_AND_MOVE(CallbackScope) explicit CallbackScope(T* ptr) : CallbackScopeBase(ptr->env()), ref(ptr) {} }; diff --git a/src/quic/data.cc b/src/quic/data.cc index e1e69808338fc8..85a22994355b4d 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -175,8 +175,8 @@ QuicError::QuicError(const ngtcp2_ccerr& error) ptr_(&error_) {} QuicError::operator bool() const { - if ((code() == QUIC_NO_ERROR && type() == Type::TRANSPORT) || - ((code() == QUIC_APP_NO_ERROR && type() == Type::APPLICATION))) { + if ((code() == NO_ERROR && type() == Type::TRANSPORT) || + ((code() == APP_NO_ERROR && type() == Type::APPLICATION))) { return false; } return true; @@ -200,7 +200,7 @@ QuicError::Type QuicError::type() const { return static_cast(ptr_->type); } -QuicError::error_code QuicError::code() const { +error_code QuicError::code() const { return ptr_->error_code; } @@ -220,6 +220,14 @@ QuicError::operator const ngtcp2_ccerr*() const { return ptr_; } +std::string QuicError::reason_for_liberr(int liberr) { + return ngtcp2_strerror(liberr); +} + +std::string QuicError::reason_for_h3_liberr(int liberr) { + return nghttp3_strerror(liberr); +} + MaybeLocal QuicError::ToV8Value(Environment* env) const { Local argv[] = { Integer::New(env->isolate(), static_cast(type())), @@ -231,6 +239,7 @@ MaybeLocal QuicError::ToV8Value(Environment* env) const { !node::ToV8Value(env->context(), reason()).ToLocal(&argv[2])) { return MaybeLocal(); } + return Array::New(env->isolate(), argv, arraysize(argv)).As(); } @@ -289,9 +298,9 @@ QuicError QuicError::FromConnectionClose(ngtcp2_conn* session) { } QuicError QuicError::TRANSPORT_NO_ERROR = - QuicError::ForTransport(QuicError::QUIC_NO_ERROR); + QuicError::ForTransport(QuicError::NO_ERROR); QuicError QuicError::APPLICATION_NO_ERROR = - QuicError::ForApplication(QuicError::QUIC_APP_NO_ERROR); + QuicError::ForApplication(QuicError::APP_NO_ERROR); QuicError QuicError::VERSION_NEGOTIATION = QuicError::ForVersionNegotiation(); QuicError QuicError::IDLE_CLOSE = QuicError::ForIdleClose(); QuicError QuicError::INTERNAL_ERROR = diff --git a/src/quic/data.h b/src/quic/data.h index 62c6bb2b153ebf..23c3fac7466f4b 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -1,6 +1,5 @@ #pragma once -#include #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC @@ -11,6 +10,8 @@ #include #include #include +#include +#include "defs.h" namespace node { namespace quic { @@ -73,10 +74,8 @@ class Store final : public MemoryRetainer { class QuicError final : public MemoryRetainer { public: - using error_code = uint64_t; - - static constexpr error_code QUIC_NO_ERROR = NGTCP2_NO_ERROR; - static constexpr error_code QUIC_APP_NO_ERROR = 65280; + static constexpr error_code NO_ERROR = NGTCP2_NO_ERROR; + static constexpr error_code APP_NO_ERROR = 65280; enum class Type { TRANSPORT = NGTCP2_CCERR_TYPE_TRANSPORT, @@ -85,11 +84,6 @@ class QuicError final : public MemoryRetainer { IDLE_CLOSE = NGTCP2_CCERR_TYPE_IDLE_CLOSE, }; - static constexpr error_code QUIC_ERROR_TYPE_TRANSPORT = - NGTCP2_CCERR_TYPE_TRANSPORT; - static constexpr error_code QUIC_ERROR_TYPE_APPLICATION = - NGTCP2_CCERR_TYPE_APPLICATION; - explicit QuicError(const std::string_view reason = ""); explicit QuicError(const ngtcp2_ccerr* ptr); explicit QuicError(const ngtcp2_ccerr& error); @@ -116,6 +110,9 @@ class QuicError final : public MemoryRetainer { std::string ToString() const; v8::MaybeLocal ToV8Value(Environment* env) const; + static std::string reason_for_liberr(int liberr); + static std::string reason_for_h3_liberr(int liberr); + static QuicError ForTransport(error_code code, const std::string_view reason = ""); static QuicError ForApplication(error_code code, diff --git a/src/quic/defs.h b/src/quic/defs.h index 7802a1fa40a22e..49717262817cbb 100644 --- a/src/quic/defs.h +++ b/src/quic/defs.h @@ -3,8 +3,11 @@ #include #include #include +#include +#include #include #include +#include namespace node { namespace quic { @@ -16,6 +19,18 @@ namespace quic { #define IF_QUIC_DEBUG(env) \ if (UNLIKELY(env->enabled_debug_list()->enabled(DebugCategory::QUIC))) +#define DISALLOW_COPY(Name) \ + Name(const Name&) = delete; \ + Name& operator=(const Name&) = delete; + +#define DISALLOW_MOVE(Name) \ + Name(Name&&) = delete; \ + Name& operator=(Name&&) = delete; + +#define DISALLOW_COPY_AND_MOVE(Name) \ + DISALLOW_COPY(Name) \ + DISALLOW_MOVE(Name) + template bool SetOption(Environment* env, Opt* options, @@ -150,20 +165,74 @@ uint64_t GetStat(Stats* stats) { #define JS_METHOD(name) \ static void name(const v8::FunctionCallbackInfo& args) +enum class Side : uint8_t { + CLIENT, + SERVER, +}; + +enum class EndpointLabel : uint8_t { + LOCAL, + REMOTE, +}; + +enum class Direction : uint8_t { + BIDIRECTIONAL, + UNIDIRECTIONAL, +}; + +enum class HeadersKind : uint8_t { + HINTS, + INITIAL, + TRAILING, +}; + +enum class HeadersFlags : uint8_t { + NONE, + TERMINAL, +}; + +enum class StreamPriority : uint8_t { + DEFAULT = NGHTTP3_DEFAULT_URGENCY, + LOW = NGHTTP3_URGENCY_LOW, + HIGH = NGHTTP3_URGENCY_HIGH, +}; + +enum class StreamPriorityFlags : uint8_t { + NONE, + NON_INCREMENTAL, +}; + +enum class PathValidationResult : uint8_t { + SUCCESS = NGTCP2_PATH_VALIDATION_RESULT_SUCCESS, + FAILURE = NGTCP2_PATH_VALIDATION_RESULT_FAILURE, + ABORTED = NGTCP2_PATH_VALIDATION_RESULT_ABORTED, +}; + +enum class DatagramStatus : uint8_t { + ACKNOWLEDGED, + LOST, +}; + +constexpr uint64_t NGTCP2_APP_NOERROR = 65280; +constexpr size_t kDefaultMaxPacketLength = NGTCP2_MAX_UDP_PAYLOAD_SIZE; +constexpr size_t kMaxSizeT = std::numeric_limits::max(); +constexpr uint64_t kMaxSafeJsInteger = 9007199254740991; +constexpr auto kSocketAddressInfoTimeout = 60 * NGTCP2_SECONDS; +constexpr size_t kMaxVectorCount = 16; + +using error_code = uint64_t; + class DebugIndentScope { public: inline DebugIndentScope() { ++indent_; } - DebugIndentScope(const DebugIndentScope&) = delete; - DebugIndentScope(DebugIndentScope&&) = delete; - DebugIndentScope& operator=(const DebugIndentScope&) = delete; - DebugIndentScope& operator=(DebugIndentScope&&) = delete; + DISALLOW_COPY_AND_MOVE(DebugIndentScope) inline ~DebugIndentScope() { --indent_; } - std::string Prefix() const { + inline std::string Prefix() const { std::string res("\n"); res.append(indent_, '\t'); return res; } - std::string Close() const { + inline std::string Close() const { std::string res("\n"); res.append(indent_ - 1, '\t'); res += "}"; diff --git a/src/quic/session.h b/src/quic/session.h index 8dc0f8cc444b18..2b11b7a6fd30dd 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -1,7 +1,5 @@ #pragma once -#include -#include "quic/tokens.h" #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC diff --git a/src/quic/tlscontext.h b/src/quic/tlscontext.h index 98e4110bff576e..0e9251485d098b 100644 --- a/src/quic/tlscontext.h +++ b/src/quic/tlscontext.h @@ -19,8 +19,8 @@ class Session; class TLSContext; // Every QUIC Session has exactly one TLSSession that maintains the state -// of the TLS handshake and negotiated cipher keys after the handshake has -// been completed. It is separated out from the main Session class only as a +// of the TLS handshake and negotiated keys after the handshake has been +// completed. It is separated out from the main Session class only as a // convenience to help make the code more maintainable and understandable. // A TLSSession is created from a TLSContext and maintains a reference to // the context. @@ -143,7 +143,8 @@ class TLSContext final : public MemoryRetainer, bool enable_tls_trace = false; // When true, causes the private key passed in for the session to be - // verified. JavaScript option name "verifyPrivateKey" + // verified. + // JavaScript option name "verifyPrivateKey" bool verify_private_key = false; // The TLS private key(s) to use for this session. From 0d55c0bd238e2bbeb445bccfe9a6abf9e2636927 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 6 Jan 2024 12:15:16 -0800 Subject: [PATCH 07/12] quic: add quicerror tests --- node.gyp | 1 + src/quic/data.cc | 52 +++++++++---- src/quic/data.h | 21 ++++-- test/cctest/test_quic_error.cc | 128 ++++++++++++++++++++++++++++++++ test/cctest/test_quic_tokens.cc | 2 +- 5 files changed, 183 insertions(+), 21 deletions(-) create mode 100644 test/cctest/test_quic_error.cc diff --git a/node.gyp b/node.gyp index bddbdc5a24c9ff..1755c2d6d778eb 100644 --- a/node.gyp +++ b/node.gyp @@ -415,6 +415,7 @@ 'test/cctest/test_node_crypto.cc', 'test/cctest/test_node_crypto_env.cc', 'test/cctest/test_quic_cid.cc', + 'test/cctest/test_quic_error.cc', 'test/cctest/test_quic_tokens.cc', ], 'node_cctest_inspector_sources': [ diff --git a/src/quic/data.cc b/src/quic/data.cc index 85a22994355b4d..592c11ac9232d6 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -1,3 +1,4 @@ +#include "nghttp3/nghttp3.h" #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "data.h" @@ -160,7 +161,7 @@ std::string TypeName(QuicError::Type type) { } } // namespace -QuicError::QuicError(const std::string_view reason) +QuicError::QuicError(const std::string& reason) : reason_(reason), error_(), ptr_(&error_) { ngtcp2_ccerr_default(&error_); } @@ -228,6 +229,31 @@ std::string QuicError::reason_for_h3_liberr(int liberr) { return nghttp3_strerror(liberr); } +bool QuicError::is_fatal_liberror(int liberr) { + return ngtcp2_err_is_fatal(liberr) != 0; +} + +bool QuicError::is_fatal_h3_liberror(int liberr) { + return nghttp3_err_is_fatal(liberr) != 0; +} + +error_code QuicError::liberr_to_code(int liberr) { + return ngtcp2_err_infer_quic_transport_error_code(liberr); +} + +error_code QuicError::h3_liberr_to_code(int liberr) { + return nghttp3_err_infer_quic_app_error_code(liberr); +} + +bool QuicError::is_crypto() const { + return code() & NGTCP2_CRYPTO_ERROR; +} + +std::optional QuicError::crypto_error() const { + if (!is_crypto()) return std::nullopt; + return code() & ~NGTCP2_CRYPTO_ERROR; +} + MaybeLocal QuicError::ToV8Value(Environment* env) const { Local argv[] = { Integer::New(env->isolate(), static_cast(type())), @@ -256,38 +282,38 @@ void QuicError::MemoryInfo(MemoryTracker* tracker) const { } QuicError QuicError::ForTransport(error_code code, - const std::string_view reason) { - QuicError error(reason); + std::string reason) { + QuicError error(std::move(reason)); ngtcp2_ccerr_set_transport_error( &error.error_, code, error.reason_c_str(), reason.length()); return error; } QuicError QuicError::ForApplication(error_code code, - const std::string_view reason) { - QuicError error(reason); + std::string reason) { + QuicError error(std::move(reason)); ngtcp2_ccerr_set_application_error( &error.error_, code, error.reason_c_str(), reason.length()); return error; } -QuicError QuicError::ForVersionNegotiation(const std::string_view reason) { - return ForNgtcp2Error(NGTCP2_ERR_RECV_VERSION_NEGOTIATION, reason); +QuicError QuicError::ForVersionNegotiation(std::string reason) { + return ForNgtcp2Error(NGTCP2_ERR_RECV_VERSION_NEGOTIATION, std::move(reason)); } -QuicError QuicError::ForIdleClose(const std::string_view reason) { - return ForNgtcp2Error(NGTCP2_ERR_IDLE_CLOSE, reason); +QuicError QuicError::ForIdleClose(std::string reason) { + return ForNgtcp2Error(NGTCP2_ERR_IDLE_CLOSE, std::move(reason)); } -QuicError QuicError::ForNgtcp2Error(int code, const std::string_view reason) { - QuicError error(reason); +QuicError QuicError::ForNgtcp2Error(int code, std::string reason) { + QuicError error(std::move(reason)); ngtcp2_ccerr_set_liberr( &error.error_, code, error.reason_c_str(), reason.length()); return error; } -QuicError QuicError::ForTlsAlert(int code, const std::string_view reason) { - QuicError error(reason); +QuicError QuicError::ForTlsAlert(int code, std::string reason) { + QuicError error(std::move(reason)); ngtcp2_ccerr_set_tls_alert( &error.error_, code, error.reason_c_str(), reason.length()); return error; diff --git a/src/quic/data.h b/src/quic/data.h index 23c3fac7466f4b..ee6fee46ef2ee3 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -84,7 +84,7 @@ class QuicError final : public MemoryRetainer { IDLE_CLOSE = NGTCP2_CCERR_TYPE_IDLE_CLOSE, }; - explicit QuicError(const std::string_view reason = ""); + explicit QuicError(const std::string& reason = ""); explicit QuicError(const ngtcp2_ccerr* ptr); explicit QuicError(const ngtcp2_ccerr& error); @@ -100,6 +100,9 @@ class QuicError final : public MemoryRetainer { // transport or application. operator bool() const; + bool is_crypto() const; + std::optional crypto_error() const; + bool operator==(const QuicError& other) const; bool operator!=(const QuicError& other) const; @@ -112,15 +115,19 @@ class QuicError final : public MemoryRetainer { static std::string reason_for_liberr(int liberr); static std::string reason_for_h3_liberr(int liberr); + static bool is_fatal_liberror(int liberr); + static bool is_fatal_h3_liberror(int liberr); + static error_code liberr_to_code(int liberr); + static error_code h3_liberr_to_code(int liberr); static QuicError ForTransport(error_code code, - const std::string_view reason = ""); + std::string reason = ""); static QuicError ForApplication(error_code code, - const std::string_view reason = ""); - static QuicError ForVersionNegotiation(const std::string_view reason = ""); - static QuicError ForIdleClose(const std::string_view reason = ""); - static QuicError ForNgtcp2Error(int code, const std::string_view reason = ""); - static QuicError ForTlsAlert(int code, const std::string_view reason = ""); + std::string reason = ""); + static QuicError ForVersionNegotiation(std::string reason = ""); + static QuicError ForIdleClose(std::string reason = ""); + static QuicError ForNgtcp2Error(int code, std::string reason = ""); + static QuicError ForTlsAlert(int code, std::string reason = ""); static QuicError FromConnectionClose(ngtcp2_conn* session); diff --git a/test/cctest/test_quic_error.cc b/test/cctest/test_quic_error.cc new file mode 100644 index 00000000000000..e47f1020046cb2 --- /dev/null +++ b/test/cctest/test_quic_error.cc @@ -0,0 +1,128 @@ +#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC +#include +#include +#include +#include +#include + +using node::quic::QuicError; + +TEST(QuicError, NoError) { + QuicError err; + CHECK_EQ(err.code(), QuicError::NO_ERROR); + CHECK_EQ(err.type(), QuicError::Type::TRANSPORT); + CHECK_EQ(err.reason(), ""); + CHECK_EQ(err, QuicError::TRANSPORT_NO_ERROR); + CHECK(!err); + + QuicError err2("a reason"); + CHECK_EQ(err2.code(), QuicError::NO_ERROR); + CHECK_EQ(err2.type(), QuicError::Type::TRANSPORT); + CHECK_EQ(err2.reason(), "a reason"); + + // Equality check ignores the reason + CHECK_EQ(err2, QuicError::TRANSPORT_NO_ERROR); + + auto err3 = QuicError::ForTransport(QuicError::NO_ERROR); + CHECK_EQ(err3.code(), QuicError::NO_ERROR); + CHECK_EQ(err3.type(), QuicError::Type::TRANSPORT); + CHECK_EQ(err3.reason(), ""); + CHECK_EQ(err3, QuicError::TRANSPORT_NO_ERROR); + + // QuicError's are copy assignable + auto err4 = err3; + CHECK_EQ(err4, err3); + + // QuicError's are movable + auto err5 = std::move(err4); + CHECK_EQ(err5, err3); + + // Equality check ignores the reason + CHECK(err5 == err2); + CHECK(err5 != QuicError::APPLICATION_NO_ERROR); + + const ngtcp2_ccerr& ccerr = err5; + CHECK_EQ(ccerr.error_code, NGTCP2_NO_ERROR); + CHECK_EQ(ccerr.frame_type, 0); + CHECK_EQ(ccerr.type, NGTCP2_CCERR_TYPE_TRANSPORT); + CHECK_EQ(ccerr.reasonlen, 0); + + const ngtcp2_ccerr* ccerr2 = err5; + CHECK_EQ(ccerr2->error_code, NGTCP2_NO_ERROR); + CHECK_EQ(ccerr2->frame_type, 0); + CHECK_EQ(ccerr2->type, NGTCP2_CCERR_TYPE_TRANSPORT); + CHECK_EQ(ccerr2->reasonlen, 0); + + QuicError err6(ccerr); + QuicError err7(ccerr2); + CHECK_EQ(err6, err7); + + CHECK_EQ(err.ToString(), "QuicError(TRANSPORT) 0"); + CHECK_EQ(err2.ToString(), "QuicError(TRANSPORT) 0: a reason"); + + ngtcp2_ccerr ccerr3; + ngtcp2_ccerr_default(&ccerr3); + ccerr3.frame_type = 1; + QuicError err8(ccerr3); + CHECK_EQ(err8.frame_type(), 1); +} + +TEST(QuicError, ApplicationNoError) { + CHECK_EQ(QuicError::APPLICATION_NO_ERROR.code(), QuicError::APP_NO_ERROR); + CHECK_EQ(QuicError::APPLICATION_NO_ERROR.type(), QuicError::Type::APPLICATION); + CHECK_EQ(QuicError::APPLICATION_NO_ERROR.reason(), ""); + + auto err = QuicError::ForApplication(QuicError::APP_NO_ERROR, "a reason"); + CHECK_EQ(err.code(), QuicError::APP_NO_ERROR); + CHECK_EQ(err.type(), QuicError::Type::APPLICATION); + CHECK_EQ(err.reason(), "a reason"); + CHECK_EQ(err.ToString(), "QuicError(APPLICATION) 65280: a reason"); +} + +TEST(QuicError, VersionNegotiation) { + CHECK_EQ(QuicError::VERSION_NEGOTIATION.code(), 0); + CHECK_EQ(QuicError::VERSION_NEGOTIATION.type(), QuicError::Type::VERSION_NEGOTIATION); + CHECK_EQ(QuicError::VERSION_NEGOTIATION.reason(), ""); + + auto err = QuicError::ForVersionNegotiation("a reason"); + CHECK_EQ(err.code(), 0); + CHECK_EQ(err.type(), QuicError::Type::VERSION_NEGOTIATION); + CHECK_EQ(err.reason(), "a reason"); + CHECK_EQ(err.ToString(), "QuicError(VERSION_NEGOTIATION) 0: a reason"); +} + +TEST(QuicError, IdleClose) { + CHECK_EQ(QuicError::IDLE_CLOSE.code(), 0); + CHECK_EQ(QuicError::IDLE_CLOSE.type(), QuicError::Type::IDLE_CLOSE); + CHECK_EQ(QuicError::IDLE_CLOSE.reason(), ""); + + auto err = QuicError::ForIdleClose("a reason"); + CHECK_EQ(err.code(), 0); + CHECK_EQ(err.type(), QuicError::Type::IDLE_CLOSE); + CHECK_EQ(err.reason(), "a reason"); + CHECK_EQ(err.ToString(), "QuicError(IDLE_CLOSE) 0: a reason"); + + CHECK_EQ(QuicError::IDLE_CLOSE, err); +} + +TEST(QuicError, InternalError) { + auto err = QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL, "a reason"); + CHECK_EQ(err.code(), NGTCP2_INTERNAL_ERROR); + CHECK_EQ(err.type(), QuicError::Type::TRANSPORT); + CHECK_EQ(err.reason(), "a reason"); + CHECK_EQ(err.ToString(), "QuicError(TRANSPORT) 1: a reason"); + + printf("%s\n", QuicError::INTERNAL_ERROR.ToString().c_str()); + CHECK_EQ(err, QuicError::INTERNAL_ERROR); +} + +TEST(QuicError, TlsAlert) { + auto err = QuicError::ForTlsAlert(1, "a reason"); + CHECK_EQ(err.code(), 257); + CHECK_EQ(err.type(), QuicError::Type::TRANSPORT); + CHECK_EQ(err.reason(), "a reason"); + CHECK(err.is_crypto()); + CHECK_EQ(err.crypto_error(), 1); +} + +#endif // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC diff --git a/test/cctest/test_quic_tokens.cc b/test/cctest/test_quic_tokens.cc index cf5b12da4d558a..a480c003cc6f3a 100644 --- a/test/cctest/test_quic_tokens.cc +++ b/test/cctest/test_quic_tokens.cc @@ -14,7 +14,7 @@ using node::quic::RetryToken; using node::quic::StatelessResetToken; using node::quic::TokenSecret; -TEST(TokenScret, Basics) { +TEST(TokenSecret, Basics) { uint8_t secret[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6}; TokenSecret fixed_secret(secret); From bc6aa73b56c8bcebf3ef1bc1a2aa5cf3f220bcb2 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 6 Jan 2024 12:31:15 -0800 Subject: [PATCH 08/12] quic: cleanup preferred address code --- src/quic/preferredaddress.cc | 32 +++++++------------------------- src/quic/preferredaddress.h | 27 +++++++++++---------------- src/quic/session.cc | 8 ++++---- src/quic/session.h | 2 +- 4 files changed, 23 insertions(+), 46 deletions(-) diff --git a/src/quic/preferredaddress.cc b/src/quic/preferredaddress.cc index 7675aebd765f9c..3704486c63e17e 100644 --- a/src/quic/preferredaddress.cc +++ b/src/quic/preferredaddress.cc @@ -97,23 +97,6 @@ bool resolve(const PreferredAddress::AddressInfo& address, } } // namespace -Maybe PreferredAddress::GetPolicy( - Environment* env, Local value) { - CHECK(value->IsUint32()); - uint32_t val = 0; - if (value->Uint32Value(env->context()).To(&val)) { - switch (val) { - case QUIC_PREFERRED_ADDRESS_USE: - return Just(Policy::USE_PREFERRED_ADDRESS); - case QUIC_PREFERRED_ADDRESS_IGNORE: - return Just(Policy::IGNORE_PREFERRED_ADDRESS); - } - } - THROW_ERR_INVALID_ARG_VALUE( - env, "%d is not a valid preferred address policy", val); - return Nothing(); -} - PreferredAddress::PreferredAddress(ngtcp2_path* dest, const ngtcp2_preferred_addr* paddr) : dest_(dest), paddr_(paddr) { @@ -160,14 +143,13 @@ void PreferredAddress::Set(ngtcp2_transport_params* params, Maybe PreferredAddress::tryGetPolicy( Environment* env, Local value) { if (value->IsUndefined()) { - return Just(PreferredAddress::Policy::USE_PREFERRED_ADDRESS); + return Just(PreferredAddress::Policy::USE); } if (value->IsUint32()) { - auto val = value.As()->Value(); - if (val == static_cast(Policy::IGNORE_PREFERRED_ADDRESS)) - return Just(Policy::IGNORE_PREFERRED_ADDRESS); - if (val == static_cast(Policy::USE_PREFERRED_ADDRESS)) - return Just(Policy::USE_PREFERRED_ADDRESS); + switch(value.As()->Value()) { + case PREFERRED_ADDRESS_IGNORE: return Just(Policy::IGNORE); + case PREFERRED_ADDRESS_USE: return Just(Policy::USE); + } } THROW_ERR_INVALID_ARG_VALUE(env, "invalid preferred address policy"); return Nothing(); @@ -175,8 +157,8 @@ Maybe PreferredAddress::tryGetPolicy( void PreferredAddress::Initialize(Environment* env, v8::Local target) { - NODE_DEFINE_CONSTANT(target, QUIC_PREFERRED_ADDRESS_IGNORE); - NODE_DEFINE_CONSTANT(target, QUIC_PREFERRED_ADDRESS_USE); + NODE_DEFINE_CONSTANT(target, PREFERRED_ADDRESS_IGNORE); + NODE_DEFINE_CONSTANT(target, PREFERRED_ADDRESS_USE); NODE_DEFINE_CONSTANT(target, DEFAULT_PREFERRED_ADDRESS_POLICY); } diff --git a/src/quic/preferredaddress.h b/src/quic/preferredaddress.h index e29f4021883bd8..9f5b3f885df4f8 100644 --- a/src/quic/preferredaddress.h +++ b/src/quic/preferredaddress.h @@ -8,6 +8,7 @@ #include #include #include +#include "defs.h" namespace node { namespace quic { @@ -18,11 +19,11 @@ namespace quic { // the preferred address to be selected. class PreferredAddress final { public: - enum class Policy { + enum class Policy : uint32_t { // Ignore the server-advertised preferred address. - IGNORE_PREFERRED_ADDRESS, + IGNORE, // Use the server-advertised preferred address. - USE_PREFERRED_ADDRESS, + USE, }; static v8::Maybe tryGetPolicy(Environment* env, @@ -30,18 +31,15 @@ class PreferredAddress final { // The QUIC_* constants are expected to be exported out to be used on // the JavaScript side of the API. - static constexpr uint32_t QUIC_PREFERRED_ADDRESS_USE = - static_cast(Policy::USE_PREFERRED_ADDRESS); - static constexpr uint32_t QUIC_PREFERRED_ADDRESS_IGNORE = - static_cast(Policy::IGNORE_PREFERRED_ADDRESS); - static constexpr uint32_t DEFAULT_PREFERRED_ADDRESS_POLICY = - static_cast(Policy::USE_PREFERRED_ADDRESS); + static constexpr auto PREFERRED_ADDRESS_USE = + static_cast(Policy::USE); + static constexpr auto PREFERRED_ADDRESS_IGNORE = + static_cast(Policy::IGNORE); + static constexpr auto DEFAULT_PREFERRED_ADDRESS_POLICY = + static_cast(Policy::USE); static void Initialize(Environment* env, v8::Local target); - static v8::Maybe GetPolicy(Environment* env, - v8::Local value); - struct AddressInfo final { char host[NI_MAXHOST]; int family; @@ -51,10 +49,7 @@ class PreferredAddress final { explicit PreferredAddress(ngtcp2_path* dest, const ngtcp2_preferred_addr* paddr); - PreferredAddress(const PreferredAddress&) = delete; - PreferredAddress(PreferredAddress&&) = delete; - PreferredAddress& operator=(const PreferredAddress&) = delete; - PreferredAddress& operator=(PreferredAddress&&) = delete; + DISALLOW_COPY_AND_MOVE(PreferredAddress) void Use(const AddressInfo& address); diff --git a/src/quic/session.cc b/src/quic/session.cc index 71add7472d49f7..628a83ff568c17 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -231,7 +231,7 @@ bool SetOption(Environment* env, const v8::Local& name) { Local value; PreferredAddress::Policy policy = - PreferredAddress::Policy::USE_PREFERRED_ADDRESS; + PreferredAddress::Policy::USE; if (!object->Get(env->context(), name).ToLocal(&value) || !PreferredAddress::tryGetPolicy(env, value).To(&policy)) { return false; @@ -454,9 +454,9 @@ std::string Session::Options::ToString() const { auto policy = ([&] { switch (preferred_address_strategy) { - case PreferredAddress::Policy::USE_PREFERRED_ADDRESS: + case PreferredAddress::Policy::USE: return "use"; - case PreferredAddress::Policy::IGNORE_PREFERRED_ADDRESS: + case PreferredAddress::Policy::IGNORE: return "ignore"; } return ""; @@ -1467,7 +1467,7 @@ void Session::HandshakeConfirmed() { void Session::SelectPreferredAddress(PreferredAddress* preferredAddress) { if (config_.options.preferred_address_strategy == - PreferredAddress::Policy::IGNORE_PREFERRED_ADDRESS) { + PreferredAddress::Policy::IGNORE) { Debug(this, "Ignoring preferred address"); return; } diff --git a/src/quic/session.h b/src/quic/session.h index 2b11b7a6fd30dd..561478d73c864a 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -106,7 +106,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { // By default a client session will use the preferred address advertised by // the the server. This option is only relevant for client sessions. PreferredAddress::Policy preferred_address_strategy = - PreferredAddress::Policy::USE_PREFERRED_ADDRESS; + PreferredAddress::Policy::USE; TransportParams::Options transport_params = TransportParams::Options::kDefault; From 6ca6f8844e572acc2c0a9df93da721ae95140852 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 6 Jan 2024 12:43:06 -0800 Subject: [PATCH 09/12] quic: eliminate some boilerplate --- src/quic/cid.cc | 6 ++---- src/quic/cid.h | 4 ++-- src/quic/packet.h | 6 ++---- src/quic/session.cc | 6 +----- src/quic/session.h | 5 +---- src/quic/sessionticket.h | 6 ++---- src/quic/tlscontext.h | 11 +++-------- src/quic/tokens.h | 6 +++--- test/cctest/test_quic_cid.cc | 1 + 9 files changed, 17 insertions(+), 34 deletions(-) diff --git a/src/quic/cid.cc b/src/quic/cid.cc index 17a0d70e981d4f..3982499733f998 100644 --- a/src/quic/cid.cc +++ b/src/quic/cid.cc @@ -1,3 +1,4 @@ +#include "quic/defs.h" #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "cid.h" #include @@ -99,10 +100,7 @@ namespace { class RandomCIDFactory : public CID::Factory { public: RandomCIDFactory() = default; - RandomCIDFactory(const RandomCIDFactory&) = delete; - RandomCIDFactory(RandomCIDFactory&&) = delete; - RandomCIDFactory& operator=(const RandomCIDFactory&) = delete; - RandomCIDFactory& operator=(RandomCIDFactory&&) = delete; + DISALLOW_COPY_AND_MOVE(RandomCIDFactory) CID Generate(size_t length_hint) const override { DCHECK_GE(length_hint, CID::kMinLength); diff --git a/src/quic/cid.h b/src/quic/cid.h index 00a5d254666b3f..89d8fd2f2a0fed 100644 --- a/src/quic/cid.h +++ b/src/quic/cid.h @@ -5,6 +5,7 @@ #include #include #include +#include "defs.h" namespace node { namespace quic { @@ -50,9 +51,8 @@ class CID final : public MemoryRetainer { explicit CID(const ngtcp2_cid* cid); CID(const CID& other); - CID(CID&& other) = delete; - CID& operator=(const CID& other); + CID(CID&&) = delete; struct Hash final { size_t operator()(const CID& cid) const; diff --git a/src/quic/packet.h b/src/quic/packet.h index c92f2fd4a60f82..c6416169d9b777 100644 --- a/src/quic/packet.h +++ b/src/quic/packet.h @@ -15,6 +15,7 @@ #include "bindingdata.h" #include "cid.h" #include "data.h" +#include "defs.h" #include "tokens.h" namespace node { @@ -76,10 +77,7 @@ class Packet final : public ReqWrap { const SocketAddress& destination, std::shared_ptr data); - Packet(const Packet&) = delete; - Packet(Packet&&) = delete; - Packet& operator=(const Packet&) = delete; - Packet& operator=(Packet&&) = delete; + DISALLOW_COPY_AND_MOVE(Packet); const SocketAddress& destination() const; size_t length() const; diff --git a/src/quic/session.cc b/src/quic/session.cc index 628a83ff568c17..c6c8139edca7f8 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -141,11 +141,7 @@ struct Session::MaybeCloseConnectionScope final { silent ? "yes" : "no"); session->connection_close_depth_++; } - MaybeCloseConnectionScope(const MaybeCloseConnectionScope&) = delete; - MaybeCloseConnectionScope(MaybeCloseConnectionScope&&) = delete; - MaybeCloseConnectionScope& operator=(const MaybeCloseConnectionScope&) = - delete; - MaybeCloseConnectionScope& operator=(MaybeCloseConnectionScope&&) = delete; + DISALLOW_COPY_AND_MOVE(MaybeCloseConnectionScope) ~MaybeCloseConnectionScope() { // We only want to trigger the sending the connection close if ... // a) Silent is not explicitly true at this scope. diff --git a/src/quic/session.h b/src/quic/session.h index 561478d73c864a..e24fd9ae22348c 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -289,10 +289,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { Session* session; explicit SendPendingDataScope(Session* session); explicit SendPendingDataScope(const BaseObjectPtr& session); - SendPendingDataScope(const SendPendingDataScope&) = delete; - SendPendingDataScope(SendPendingDataScope&&) = delete; - SendPendingDataScope& operator=(const SendPendingDataScope&) = delete; - SendPendingDataScope& operator=(SendPendingDataScope&&) = delete; + DISALLOW_COPY_AND_MOVE(SendPendingDataScope); ~SendPendingDataScope(); }; diff --git a/src/quic/sessionticket.h b/src/quic/sessionticket.h index 4700af5743954e..5776459d550d13 100644 --- a/src/quic/sessionticket.h +++ b/src/quic/sessionticket.h @@ -9,6 +9,7 @@ #include #include #include "data.h" +#include "defs.h" namespace node { namespace quic { @@ -74,10 +75,7 @@ class SessionTicket::AppData final { }; explicit AppData(SSL* session); - AppData(const AppData&) = delete; - AppData(AppData&&) = delete; - AppData& operator=(const AppData&) = delete; - AppData& operator=(AppData&&) = delete; + DISALLOW_COPY_AND_MOVE(AppData) bool Set(const uv_buf_t& data); std::optional Get() const; diff --git a/src/quic/tlscontext.h b/src/quic/tlscontext.h index 0e9251485d098b..25adb66240a581 100644 --- a/src/quic/tlscontext.h +++ b/src/quic/tlscontext.h @@ -10,6 +10,7 @@ #include #include "bindingdata.h" #include "data.h" +#include "defs.h" #include "sessionticket.h" namespace node { @@ -33,10 +34,7 @@ class TLSSession final : public MemoryRetainer { TLSSession(Session* session, std::shared_ptr context, const std::optional& maybeSessionTicket); - TLSSession(const TLSSession&) = delete; - TLSSession(TLSSession&&) = delete; - TLSSession& operator=(const TLSSession&) = delete; - TLSSession& operator=(TLSSession&&) = delete; + DISALLOW_COPY_AND_MOVE(TLSSession) inline operator bool() const { return ssl_ != nullptr; } inline Session& session() const { return *session_; } @@ -182,10 +180,7 @@ class TLSContext final : public MemoryRetainer, const Options& options); TLSContext(Environment* env, Side side, const Options& options); - TLSContext(const TLSContext&) = delete; - TLSContext(TLSContext&&) = delete; - TLSContext& operator=(const TLSContext&) = delete; - TLSContext& operator=(TLSContext&&) = delete; + DISALLOW_COPY_AND_MOVE(TLSContext) // Each QUIC Session has exactly one TLSSession. Each TLSSession maintains // a reference to the TLSContext used to create it. diff --git a/src/quic/tokens.h b/src/quic/tokens.h index c66f898429d651..bd62faee9eabd6 100644 --- a/src/quic/tokens.h +++ b/src/quic/tokens.h @@ -8,6 +8,7 @@ #include #include #include "cid.h" +#include "defs.h" namespace node { namespace quic { @@ -35,8 +36,7 @@ class TokenSecret final : public MemoryRetainer { TokenSecret(const TokenSecret&) = default; TokenSecret& operator=(const TokenSecret&) = default; - TokenSecret(TokenSecret&&) = delete; - TokenSecret& operator=(TokenSecret&&) = delete; + DISALLOW_MOVE(TokenSecret) operator const uint8_t*() const; uint8_t operator[](int pos) const; @@ -99,7 +99,7 @@ class StatelessResetToken final : public MemoryRetainer { explicit StatelessResetToken(const uint8_t* token); StatelessResetToken(const StatelessResetToken& other); - StatelessResetToken(StatelessResetToken&&) = delete; + DISALLOW_MOVE(StatelessResetToken) std::string ToString() const; diff --git a/test/cctest/test_quic_cid.cc b/test/cctest/test_quic_cid.cc index 44e4e5d7b998e7..0d9e0d5ab3d3c3 100644 --- a/test/cctest/test_quic_cid.cc +++ b/test/cctest/test_quic_cid.cc @@ -3,6 +3,7 @@ #include #include #include +#include #include #include From ddb0351f284614da693d8c90a706fddd699dc014 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 6 Jan 2024 13:15:16 -0800 Subject: [PATCH 10/12] quic: fixup linting issues --- src/quic/application.cc | 47 ++++++++++++++++++---------------- src/quic/cid.cc | 2 +- src/quic/data.cc | 11 ++++---- src/quic/data.h | 6 ++--- src/quic/defs.h | 4 +-- src/quic/endpoint.cc | 6 +++-- src/quic/endpoint.h | 3 +-- src/quic/http3.cc | 2 +- src/quic/packet.h | 2 +- src/quic/preferredaddress.cc | 8 +++--- src/quic/session.cc | 22 ++++++++-------- src/quic/session.h | 13 +++++----- src/quic/tlscontext.cc | 13 +++++----- test/cctest/test_quic_cid.cc | 2 +- test/cctest/test_quic_error.cc | 8 +++--- 15 files changed, 78 insertions(+), 71 deletions(-) diff --git a/src/quic/application.cc b/src/quic/application.cc index 834737c4cd1dd1..c4f397fc837c3a 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -3,9 +3,9 @@ #include "application.h" #include #include +#include #include #include -#include #include #include #include "defs.h" @@ -246,8 +246,7 @@ void Session::Application::SendPendingData() { // The maximum number of packets to send in this call to SendPendingData. const size_t max_packet_count = std::min( - kMaxPackets, - ngtcp2_conn_get_send_quantum(*session_) / max_packet_size); + kMaxPackets, ngtcp2_conn_get_send_quantum(*session_) / max_packet_size); // The number of packets that have been sent in this call to SendPendingData. size_t packet_send_count = 0; @@ -295,11 +294,12 @@ void Session::Application::SendPendingData() { Debug(session_, "Application using stream data: %s", stream_data); // Awesome, let's write our packet! - ssize_t nwrite = WriteVStream(&path, pos, &ndatalen, max_packet_size, stream_data); + ssize_t nwrite = + WriteVStream(&path, pos, &ndatalen, max_packet_size, stream_data); Debug(session_, "Application accepted %zu bytes into packet", ndatalen); - // A negative nwrite value indicates either an error or that there is more data - // to write into the packet. + // A negative nwrite value indicates either an error or that there is more + // data to write into the packet. if (nwrite < 0) { switch (nwrite) { case NGTCP2_ERR_STREAM_DATA_BLOCKED: { @@ -317,7 +317,8 @@ void Session::Application::SendPendingData() { // Indicates that the writable side of the stream should be closed // locally or the stream is being reset. In either case, we can't send // any stream data! - Debug(session_, "Stream %" PRIi64 " should be closed for writing", + Debug(session_, + "Stream %" PRIi64 " should be closed for writing", stream_data.id); // ndatalen = -1 means that no stream data was accepted into the // packet, which is what we want here. @@ -341,7 +342,8 @@ void Session::Application::SendPendingData() { // Some other type of error happened. DCHECK_EQ(ndatalen, -1); - Debug(session_, "Application encountered error while writing packet: %s", + Debug(session_, + "Application encountered error while writing packet: %s", ngtcp2_strerror(nwrite)); session_->SetLastError(QuicError::ForNgtcp2Error(nwrite)); packet->Done(UV_ECANCELED); @@ -363,14 +365,16 @@ void Session::Application::SendPendingData() { size_t datalen = pos - begin; if (datalen) { Debug(session_, "Packet has %zu bytes to send", datalen); - // At least some data had been written into the packet. We should send it. + // At least some data had been written into the packet. We should send + // it. packet->Truncate(datalen); session_->Send(packet, path); } else { packet->Done(UV_ECANCELED); } - // If there was stream data selected, we should reschedule it to try sending again. + // If there was stream data selected, we should reschedule it to try + // sending again. if (stream_data.id >= 0) ResumeStream(stream_data.id); return session_->UpdatePacketTxTime(); @@ -403,18 +407,17 @@ ssize_t Session::Application::WriteVStream(PathStorage* path, uint32_t flags = NGTCP2_WRITE_STREAM_FLAG_MORE; if (stream_data.fin) flags |= NGTCP2_WRITE_STREAM_FLAG_FIN; ngtcp2_pkt_info pi; - return ngtcp2_conn_writev_stream( - *session_, - &path->path, - &pi, - dest, - max_packet_size, - ndatalen, - flags, - stream_data.id, - stream_data.buf, - stream_data.count, - uv_hrtime()); + return ngtcp2_conn_writev_stream(*session_, + &path->path, + &pi, + dest, + max_packet_size, + ndatalen, + flags, + stream_data.id, + stream_data.buf, + stream_data.count, + uv_hrtime()); } // The DefaultApplication is the default implementation of Session::Application diff --git a/src/quic/cid.cc b/src/quic/cid.cc index 3982499733f998..7c30d0d542aeaf 100644 --- a/src/quic/cid.cc +++ b/src/quic/cid.cc @@ -1,10 +1,10 @@ -#include "quic/defs.h" #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "cid.h" #include #include #include #include +#include "quic/defs.h" namespace node { namespace quic { diff --git a/src/quic/data.cc b/src/quic/data.cc index 592c11ac9232d6..37656dae81aa24 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -1,4 +1,3 @@ -#include "nghttp3/nghttp3.h" #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC #include "data.h" @@ -48,7 +47,9 @@ std::string Path::ToString() const { return res; } -PathStorage::PathStorage() { Reset(); } +PathStorage::PathStorage() { + Reset(); +} PathStorage::operator ngtcp2_path() { return path; } @@ -281,16 +282,14 @@ void QuicError::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("reason", reason_.length()); } -QuicError QuicError::ForTransport(error_code code, - std::string reason) { +QuicError QuicError::ForTransport(error_code code, std::string reason) { QuicError error(std::move(reason)); ngtcp2_ccerr_set_transport_error( &error.error_, code, error.reason_c_str(), reason.length()); return error; } -QuicError QuicError::ForApplication(error_code code, - std::string reason) { +QuicError QuicError::ForApplication(error_code code, std::string reason) { QuicError error(std::move(reason)); ngtcp2_ccerr_set_application_error( &error.error_, code, error.reason_c_str(), reason.length()); diff --git a/src/quic/data.h b/src/quic/data.h index ee6fee46ef2ee3..6fd8d3a79ff700 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -120,10 +120,8 @@ class QuicError final : public MemoryRetainer { static error_code liberr_to_code(int liberr); static error_code h3_liberr_to_code(int liberr); - static QuicError ForTransport(error_code code, - std::string reason = ""); - static QuicError ForApplication(error_code code, - std::string reason = ""); + static QuicError ForTransport(error_code code, std::string reason = ""); + static QuicError ForApplication(error_code code, std::string reason = ""); static QuicError ForVersionNegotiation(std::string reason = ""); static QuicError ForIdleClose(std::string reason = ""); static QuicError ForNgtcp2Error(int code, std::string reason = ""); diff --git a/src/quic/defs.h b/src/quic/defs.h index 49717262817cbb..0e2e3095ad8676 100644 --- a/src/quic/defs.h +++ b/src/quic/defs.h @@ -2,9 +2,9 @@ #include #include -#include -#include #include +#include +#include #include #include #include diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc index cd211cc5220e80..03050f8b2cdcad 100644 --- a/src/quic/endpoint.cc +++ b/src/quic/endpoint.cc @@ -336,7 +336,9 @@ std::string Endpoint::Options::ToString() const { auto ccalg = ([&] { switch (cc_algorithm) { -#define V(name, label) case NGTCP2_CC_ALGO_##name: return #label; +#define V(name, label) \ + case NGTCP2_CC_ALGO_##name: \ + return #label; ENDPOINT_CC(V) #undef V } @@ -615,7 +617,7 @@ void Endpoint::InitPerIsolate(IsolateData* data, Local target) { void Endpoint::InitPerContext(Realm* realm, Local target) { #define V(name, str) \ - NODE_DEFINE_CONSTANT(target, CC_ALGO_##name); \ + NODE_DEFINE_CONSTANT(target, CC_ALGO_##name); \ NODE_DEFINE_STRING_CONSTANT(target, "CC_ALGO_" #name "_STR", #str); ENDPOINT_CC(V) #undef V diff --git a/src/quic/endpoint.h b/src/quic/endpoint.h index 7ea0b07f79f469..f04131185e29a7 100644 --- a/src/quic/endpoint.h +++ b/src/quic/endpoint.h @@ -38,8 +38,7 @@ class Endpoint final : public AsyncWrap, public Packet::Listener { static constexpr uint64_t DEFAULT_MAX_STATELESS_RESETS = 10; static constexpr uint64_t DEFAULT_MAX_RETRY_LIMIT = 10; -#define V(name, _) \ - static constexpr auto CC_ALGO_##name = NGTCP2_CC_ALGO_##name; +#define V(name, _) static constexpr auto CC_ALGO_##name = NGTCP2_CC_ALGO_##name; ENDPOINT_CC(V) #undef V diff --git a/src/quic/http3.cc b/src/quic/http3.cc index 210de9a69b852e..11196a5f6ca060 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -367,7 +367,7 @@ class Http3Application final : public Session::Application { } } } - DCHECK_NOT_NULL(stream_data.buf); + DCHECK_NOT_NULL(data->buf); return 0; } diff --git a/src/quic/packet.h b/src/quic/packet.h index c6416169d9b777..761fb3af4e609d 100644 --- a/src/quic/packet.h +++ b/src/quic/packet.h @@ -77,7 +77,7 @@ class Packet final : public ReqWrap { const SocketAddress& destination, std::shared_ptr data); - DISALLOW_COPY_AND_MOVE(Packet); + DISALLOW_COPY_AND_MOVE(Packet) const SocketAddress& destination() const; size_t length() const; diff --git a/src/quic/preferredaddress.cc b/src/quic/preferredaddress.cc index 3704486c63e17e..54292b793ffc2b 100644 --- a/src/quic/preferredaddress.cc +++ b/src/quic/preferredaddress.cc @@ -146,9 +146,11 @@ Maybe PreferredAddress::tryGetPolicy( return Just(PreferredAddress::Policy::USE); } if (value->IsUint32()) { - switch(value.As()->Value()) { - case PREFERRED_ADDRESS_IGNORE: return Just(Policy::IGNORE); - case PREFERRED_ADDRESS_USE: return Just(Policy::USE); + switch (value.As()->Value()) { + case PREFERRED_ADDRESS_IGNORE: + return Just(Policy::IGNORE); + case PREFERRED_ADDRESS_USE: + return Just(Policy::USE); } } THROW_ERR_INVALID_ARG_VALUE(env, "invalid preferred address policy"); diff --git a/src/quic/session.cc b/src/quic/session.cc index c6c8139edca7f8..91672e83e26749 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -226,8 +226,7 @@ bool SetOption(Environment* env, const v8::Local& object, const v8::Local& name) { Local value; - PreferredAddress::Policy policy = - PreferredAddress::Policy::USE; + PreferredAddress::Policy policy = PreferredAddress::Policy::USE; if (!object->Get(env->context(), name).ToLocal(&value) || !PreferredAddress::tryGetPolicy(env, value).To(&policy)) { return false; @@ -472,10 +471,11 @@ bool Session::HasInstance(Environment* env, Local value) { return GetConstructorTemplate(env)->HasInstance(value); } -BaseObjectPtr Session::Create(Endpoint* endpoint, - const Config& config, - TLSContext* tls_context, - std::optional& ticket) { +BaseObjectPtr Session::Create( + Endpoint* endpoint, + const Config& config, + TLSContext* tls_context, + const std::optional& ticket) { Local obj; if (!GetConstructorTemplate(endpoint->env()) ->InstanceTemplate() @@ -492,7 +492,7 @@ Session::Session(Endpoint* endpoint, v8::Local object, const Config& config, TLSContext* tls_context, - std::optional& session_ticket) + const std::optional& session_ticket) : AsyncWrap(endpoint->env(), object, AsyncWrap::PROVIDER_QUIC_SESSION), stats_(env()->isolate()), state_(env()->isolate()), @@ -2027,8 +2027,8 @@ struct Session::Impl { to_string(level), session->config().dcid); - return session->application().Start() ? - NGTCP2_SUCCESS : NGTCP2_ERR_CALLBACK_FAILURE; + return session->application().Start() ? NGTCP2_SUCCESS + : NGTCP2_ERR_CALLBACK_FAILURE; } static int on_receive_stateless_reset(ngtcp2_conn* conn, @@ -2085,8 +2085,8 @@ struct Session::Impl { "Receiving TX key for level %d for dcid %s", to_string(level), session->config().dcid); - return session->application().Start() ? - NGTCP2_SUCCESS : NGTCP2_ERR_CALLBACK_FAILURE; + return session->application().Start() ? NGTCP2_SUCCESS + : NGTCP2_ERR_CALLBACK_FAILURE; } static int on_receive_version_negotiation(ngtcp2_conn* conn, diff --git a/src/quic/session.h b/src/quic/session.h index e24fd9ae22348c..9dde18ec01c9cc 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -205,17 +205,18 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { static void InitPerContext(Realm* env, v8::Local target); static void RegisterExternalReferences(ExternalReferenceRegistry* registry); - static BaseObjectPtr Create(Endpoint* endpoint, - const Config& config, - TLSContext* tls_context, - std::optional& ticket); + static BaseObjectPtr Create( + Endpoint* endpoint, + const Config& config, + TLSContext* tls_context, + const std::optional& ticket); // Really should be private but MakeDetachedBaseObject needs visibility. Session(Endpoint* endpoint, v8::Local object, const Config& config, TLSContext* tls_context, - std::optional& ticket); + const std::optional& ticket); ~Session() override; uint32_t version() const; @@ -289,7 +290,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { Session* session; explicit SendPendingDataScope(Session* session); explicit SendPendingDataScope(const BaseObjectPtr& session); - DISALLOW_COPY_AND_MOVE(SendPendingDataScope); + DISALLOW_COPY_AND_MOVE(SendPendingDataScope) ~SendPendingDataScope(); }; diff --git a/src/quic/tlscontext.cc b/src/quic/tlscontext.cc index 770f9ff4f051cd..c468d51de8e2f8 100644 --- a/src/quic/tlscontext.cc +++ b/src/quic/tlscontext.cc @@ -21,7 +21,6 @@ namespace node { using v8::ArrayBuffer; -using v8::BackingStore; using v8::Just; using v8::Local; using v8::Maybe; @@ -206,7 +205,8 @@ int TLSContext::OnNewSession(SSL* ssl, SSL_SESSION* sess) { // If there is nothing listening for the session ticket, do not bother. if (session.wants_session_ticket()) { - Debug(&session, "Preparing TLS session resumption ticket");; + Debug(&session, "Preparing TLS session resumption ticket"); + // Pre-fight to see how much space we need to allocate for the session // ticket. size_t size = i2d_SSL_SESSION(sess, nullptr); @@ -318,10 +318,11 @@ crypto::SSLCtxPointer TLSContext::Initialize() { crypto::BIOPointer bio = crypto::NodeBIO::NewFixed(buf.base, buf.len); CHECK(bio); X509_STORE* cert_store = SSL_CTX_get_cert_store(ctx.get()); - while ( - crypto::X509Pointer x509 = - crypto::X509Pointer(PEM_read_bio_X509_AUX( - bio.get(), nullptr, crypto::NoPasswordCallback, nullptr))) { + while (crypto::X509Pointer x509 = crypto::X509Pointer( + PEM_read_bio_X509_AUX(bio.get(), + nullptr, + crypto::NoPasswordCallback, + nullptr))) { if (cert_store == crypto::GetOrCreateRootCertStore()) { cert_store = crypto::NewRootCertStore(); SSL_CTX_set_cert_store(ctx.get(), cert_store); diff --git a/test/cctest/test_quic_cid.cc b/test/cctest/test_quic_cid.cc index 0d9e0d5ab3d3c3..6d8a19431f714f 100644 --- a/test/cctest/test_quic_cid.cc +++ b/test/cctest/test_quic_cid.cc @@ -1,9 +1,9 @@ #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC +#include #include #include #include #include -#include #include #include diff --git a/test/cctest/test_quic_error.cc b/test/cctest/test_quic_error.cc index e47f1020046cb2..74c978615ce647 100644 --- a/test/cctest/test_quic_error.cc +++ b/test/cctest/test_quic_error.cc @@ -1,7 +1,7 @@ #if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC +#include #include #include -#include #include #include @@ -69,7 +69,8 @@ TEST(QuicError, NoError) { TEST(QuicError, ApplicationNoError) { CHECK_EQ(QuicError::APPLICATION_NO_ERROR.code(), QuicError::APP_NO_ERROR); - CHECK_EQ(QuicError::APPLICATION_NO_ERROR.type(), QuicError::Type::APPLICATION); + CHECK_EQ(QuicError::APPLICATION_NO_ERROR.type(), + QuicError::Type::APPLICATION); CHECK_EQ(QuicError::APPLICATION_NO_ERROR.reason(), ""); auto err = QuicError::ForApplication(QuicError::APP_NO_ERROR, "a reason"); @@ -81,7 +82,8 @@ TEST(QuicError, ApplicationNoError) { TEST(QuicError, VersionNegotiation) { CHECK_EQ(QuicError::VERSION_NEGOTIATION.code(), 0); - CHECK_EQ(QuicError::VERSION_NEGOTIATION.type(), QuicError::Type::VERSION_NEGOTIATION); + CHECK_EQ(QuicError::VERSION_NEGOTIATION.type(), + QuicError::Type::VERSION_NEGOTIATION); CHECK_EQ(QuicError::VERSION_NEGOTIATION.reason(), ""); auto err = QuicError::ForVersionNegotiation("a reason"); From 76659e8a8bdee4eb6264e6c2b83f8b455c43599e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 6 Jan 2024 20:13:34 -0800 Subject: [PATCH 11/12] quic: fixup changes that were problematic on windows --- src/quic/data.cc | 8 ++++---- src/quic/data.h | 4 ++-- src/quic/preferredaddress.cc | 6 +++--- src/quic/preferredaddress.h | 10 +++++----- src/quic/session.cc | 8 ++++---- src/quic/session.h | 2 +- test/cctest/test_quic_error.cc | 16 +++++++++------- 7 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/quic/data.cc b/src/quic/data.cc index 37656dae81aa24..b01e10ad05707b 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -177,8 +177,8 @@ QuicError::QuicError(const ngtcp2_ccerr& error) ptr_(&error_) {} QuicError::operator bool() const { - if ((code() == NO_ERROR && type() == Type::TRANSPORT) || - ((code() == APP_NO_ERROR && type() == Type::APPLICATION))) { + if ((code() == QUIC_NO_ERROR && type() == Type::TRANSPORT) || + ((code() == QUIC_APP_NO_ERROR && type() == Type::APPLICATION))) { return false; } return true; @@ -323,9 +323,9 @@ QuicError QuicError::FromConnectionClose(ngtcp2_conn* session) { } QuicError QuicError::TRANSPORT_NO_ERROR = - QuicError::ForTransport(QuicError::NO_ERROR); + QuicError::ForTransport(QuicError::QUIC_NO_ERROR); QuicError QuicError::APPLICATION_NO_ERROR = - QuicError::ForApplication(QuicError::APP_NO_ERROR); + QuicError::ForApplication(QuicError::QUIC_APP_NO_ERROR); QuicError QuicError::VERSION_NEGOTIATION = QuicError::ForVersionNegotiation(); QuicError QuicError::IDLE_CLOSE = QuicError::ForIdleClose(); QuicError QuicError::INTERNAL_ERROR = diff --git a/src/quic/data.h b/src/quic/data.h index 6fd8d3a79ff700..ef36e248bbf562 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -74,8 +74,8 @@ class Store final : public MemoryRetainer { class QuicError final : public MemoryRetainer { public: - static constexpr error_code NO_ERROR = NGTCP2_NO_ERROR; - static constexpr error_code APP_NO_ERROR = 65280; + static constexpr error_code QUIC_NO_ERROR = NGTCP2_NO_ERROR; + static constexpr error_code QUIC_APP_NO_ERROR = 65280; enum class Type { TRANSPORT = NGTCP2_CCERR_TYPE_TRANSPORT, diff --git a/src/quic/preferredaddress.cc b/src/quic/preferredaddress.cc index 54292b793ffc2b..42788d4afc136b 100644 --- a/src/quic/preferredaddress.cc +++ b/src/quic/preferredaddress.cc @@ -143,14 +143,14 @@ void PreferredAddress::Set(ngtcp2_transport_params* params, Maybe PreferredAddress::tryGetPolicy( Environment* env, Local value) { if (value->IsUndefined()) { - return Just(PreferredAddress::Policy::USE); + return Just(PreferredAddress::Policy::USE_PREFERRED); } if (value->IsUint32()) { switch (value.As()->Value()) { case PREFERRED_ADDRESS_IGNORE: - return Just(Policy::IGNORE); + return Just(Policy::IGNORE_PREFERRED); case PREFERRED_ADDRESS_USE: - return Just(Policy::USE); + return Just(Policy::USE_PREFERRED); } } THROW_ERR_INVALID_ARG_VALUE(env, "invalid preferred address policy"); diff --git a/src/quic/preferredaddress.h b/src/quic/preferredaddress.h index 9f5b3f885df4f8..37003e2eb704c7 100644 --- a/src/quic/preferredaddress.h +++ b/src/quic/preferredaddress.h @@ -21,9 +21,9 @@ class PreferredAddress final { public: enum class Policy : uint32_t { // Ignore the server-advertised preferred address. - IGNORE, + IGNORE_PREFERRED, // Use the server-advertised preferred address. - USE, + USE_PREFERRED, }; static v8::Maybe tryGetPolicy(Environment* env, @@ -32,11 +32,11 @@ class PreferredAddress final { // The QUIC_* constants are expected to be exported out to be used on // the JavaScript side of the API. static constexpr auto PREFERRED_ADDRESS_USE = - static_cast(Policy::USE); + static_cast(Policy::USE_PREFERRED); static constexpr auto PREFERRED_ADDRESS_IGNORE = - static_cast(Policy::IGNORE); + static_cast(Policy::IGNORE_PREFERRED); static constexpr auto DEFAULT_PREFERRED_ADDRESS_POLICY = - static_cast(Policy::USE); + static_cast(Policy::USE_PREFERRED); static void Initialize(Environment* env, v8::Local target); diff --git a/src/quic/session.cc b/src/quic/session.cc index 91672e83e26749..c53e7f584470f3 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -226,7 +226,7 @@ bool SetOption(Environment* env, const v8::Local& object, const v8::Local& name) { Local value; - PreferredAddress::Policy policy = PreferredAddress::Policy::USE; + PreferredAddress::Policy policy = PreferredAddress::Policy::USE_PREFERRED; if (!object->Get(env->context(), name).ToLocal(&value) || !PreferredAddress::tryGetPolicy(env, value).To(&policy)) { return false; @@ -449,9 +449,9 @@ std::string Session::Options::ToString() const { auto policy = ([&] { switch (preferred_address_strategy) { - case PreferredAddress::Policy::USE: + case PreferredAddress::Policy::USE_PREFERRED: return "use"; - case PreferredAddress::Policy::IGNORE: + case PreferredAddress::Policy::IGNORE_PREFERRED: return "ignore"; } return ""; @@ -1463,7 +1463,7 @@ void Session::HandshakeConfirmed() { void Session::SelectPreferredAddress(PreferredAddress* preferredAddress) { if (config_.options.preferred_address_strategy == - PreferredAddress::Policy::IGNORE) { + PreferredAddress::Policy::IGNORE_PREFERRED) { Debug(this, "Ignoring preferred address"); return; } diff --git a/src/quic/session.h b/src/quic/session.h index 9dde18ec01c9cc..41a797dad6844d 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -106,7 +106,7 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { // By default a client session will use the preferred address advertised by // the the server. This option is only relevant for client sessions. PreferredAddress::Policy preferred_address_strategy = - PreferredAddress::Policy::USE; + PreferredAddress::Policy::USE_PREFERRED; TransportParams::Options transport_params = TransportParams::Options::kDefault; diff --git a/test/cctest/test_quic_error.cc b/test/cctest/test_quic_error.cc index 74c978615ce647..f342f8b9ebf060 100644 --- a/test/cctest/test_quic_error.cc +++ b/test/cctest/test_quic_error.cc @@ -9,22 +9,22 @@ using node::quic::QuicError; TEST(QuicError, NoError) { QuicError err; - CHECK_EQ(err.code(), QuicError::NO_ERROR); + CHECK_EQ(err.code(), QuicError::QUIC_NO_ERROR); CHECK_EQ(err.type(), QuicError::Type::TRANSPORT); CHECK_EQ(err.reason(), ""); CHECK_EQ(err, QuicError::TRANSPORT_NO_ERROR); CHECK(!err); QuicError err2("a reason"); - CHECK_EQ(err2.code(), QuicError::NO_ERROR); + CHECK_EQ(err2.code(), QuicError::QUIC_NO_ERROR); CHECK_EQ(err2.type(), QuicError::Type::TRANSPORT); CHECK_EQ(err2.reason(), "a reason"); // Equality check ignores the reason CHECK_EQ(err2, QuicError::TRANSPORT_NO_ERROR); - auto err3 = QuicError::ForTransport(QuicError::NO_ERROR); - CHECK_EQ(err3.code(), QuicError::NO_ERROR); + auto err3 = QuicError::ForTransport(QuicError::QUIC_NO_ERROR); + CHECK_EQ(err3.code(), QuicError::QUIC_NO_ERROR); CHECK_EQ(err3.type(), QuicError::Type::TRANSPORT); CHECK_EQ(err3.reason(), ""); CHECK_EQ(err3, QuicError::TRANSPORT_NO_ERROR); @@ -68,13 +68,15 @@ TEST(QuicError, NoError) { } TEST(QuicError, ApplicationNoError) { - CHECK_EQ(QuicError::APPLICATION_NO_ERROR.code(), QuicError::APP_NO_ERROR); + CHECK_EQ(QuicError::APPLICATION_NO_ERROR.code(), + QuicError::QUIC_APP_NO_ERROR); CHECK_EQ(QuicError::APPLICATION_NO_ERROR.type(), QuicError::Type::APPLICATION); CHECK_EQ(QuicError::APPLICATION_NO_ERROR.reason(), ""); - auto err = QuicError::ForApplication(QuicError::APP_NO_ERROR, "a reason"); - CHECK_EQ(err.code(), QuicError::APP_NO_ERROR); + auto err = + QuicError::ForApplication(QuicError::QUIC_APP_NO_ERROR, "a reason"); + CHECK_EQ(err.code(), QuicError::QUIC_APP_NO_ERROR); CHECK_EQ(err.type(), QuicError::Type::APPLICATION); CHECK_EQ(err.reason(), "a reason"); CHECK_EQ(err.ToString(), "QuicError(APPLICATION) 65280: a reason"); From e03af7bf5b39ec8fbd2c062e7429a52a75eea3a1 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 20 Apr 2024 12:42:04 -0700 Subject: [PATCH 12/12] src: additional fixups for tlscontext --- src/quic/endpoint.cc | 4 ++-- src/quic/tlscontext.cc | 14 ++++++-------- src/quic/tlscontext.h | 9 +++------ 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/quic/endpoint.cc b/src/quic/endpoint.cc index 03050f8b2cdcad..9e54e8e08ae0bc 100644 --- a/src/quic/endpoint.cc +++ b/src/quic/endpoint.cc @@ -966,7 +966,7 @@ void Endpoint::Listen(const Session::Options& options) { "not what you want."); } - auto context = TLSContext::CreateServer(env(), options.tls_options); + auto context = TLSContext::CreateServer(options.tls_options); if (!*context) { THROW_ERR_INVALID_STATE( env(), "Failed to create TLS context: %s", context->validation_error()); @@ -1002,7 +1002,7 @@ BaseObjectPtr Endpoint::Connect( session_ticket.has_value() ? "yes" : "no"); } - auto tls_context = TLSContext::CreateClient(env(), options.tls_options); + auto tls_context = TLSContext::CreateClient(options.tls_options); if (!*tls_context) { THROW_ERR_INVALID_STATE(env(), "Failed to create TLS context: %s", diff --git a/src/quic/tlscontext.cc b/src/quic/tlscontext.cc index c468d51de8e2f8..8441e491d1ca6a 100644 --- a/src/quic/tlscontext.cc +++ b/src/quic/tlscontext.cc @@ -147,18 +147,16 @@ bool SetOption(Environment* env, } } // namespace -std::shared_ptr TLSContext::CreateClient(Environment* env, - const Options& options) { - return std::make_shared(env, Side::CLIENT, options); +std::shared_ptr TLSContext::CreateClient(const Options& options) { + return std::make_shared(Side::CLIENT, options); } -std::shared_ptr TLSContext::CreateServer(Environment* env, - const Options& options) { - return std::make_shared(env, Side::SERVER, options); +std::shared_ptr TLSContext::CreateServer(const Options& options) { + return std::make_shared(Side::SERVER, options); } -TLSContext::TLSContext(Environment* env, Side side, const Options& options) - : env_(env), side_(side), options_(options), ctx_(Initialize()) {} +TLSContext::TLSContext(Side side, const Options& options) + : side_(side), options_(options), ctx_(Initialize()) {} TLSContext::operator SSL_CTX*() const { DCHECK(ctx_); diff --git a/src/quic/tlscontext.h b/src/quic/tlscontext.h index 25adb66240a581..d0dfea6106c00e 100644 --- a/src/quic/tlscontext.h +++ b/src/quic/tlscontext.h @@ -174,12 +174,10 @@ class TLSContext final : public MemoryRetainer, std::string ToString() const; }; - static std::shared_ptr CreateClient(Environment* env, - const Options& options); - static std::shared_ptr CreateServer(Environment* env, - const Options& options); + static std::shared_ptr CreateClient(const Options& options); + static std::shared_ptr CreateServer(const Options& options); - TLSContext(Environment* env, Side side, const Options& options); + TLSContext(Side side, const Options& options); DISALLOW_COPY_AND_MOVE(TLSContext) // Each QUIC Session has exactly one TLSSession. Each TLSSession maintains @@ -213,7 +211,6 @@ class TLSContext final : public MemoryRetainer, void* arg); static int OnVerifyClientCertificate(int preverify_ok, X509_STORE_CTX* ctx); - Environment* env_; Side side_; Options options_; crypto::X509Pointer cert_;