From 4acbbc6c139561a4a516766db5987f815df1c368 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 21 Aug 2020 10:11:50 -0700 Subject: [PATCH 1/5] quic: resolve http3 header todo Signed-off-by: James M Snell --- src/quic/node_quic_http3_application.cc | 15 ++++++--------- src/quic/node_quic_http3_application.h | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc index 82cd677d1ce558..30a767a6e1b9cf 100644 --- a/src/quic/node_quic_http3_application.cc +++ b/src/quic/node_quic_http3_application.cc @@ -646,7 +646,7 @@ void Http3Application::BeginHeaders( // by the QuicStream until stream->EndHeaders() is called, during which // the collected headers are converted to an array and passed off to // the javascript side. -bool Http3Application::ReceiveHeader( +void Http3Application::ReceiveHeader( int64_t stream_id, int32_t token, nghttp3_rcbuf* name, @@ -671,9 +671,9 @@ bool Http3Application::ReceiveHeader( name, value, flags); - return stream->AddHeader(std::move(header)); + // At this level, we don't care if the header is added or not. + USE(stream->AddHeader(std::move(header))); } - return true; } // Marks the completion of a headers block. @@ -830,10 +830,8 @@ int Http3Application::OnReceiveHeader( void* conn_user_data, void* stream_user_data) { Http3Application* app = static_cast(conn_user_data); - // TODO(@jasnell): Need to determine the appropriate response code here - // for when the header is not going to be accepted. - return app->ReceiveHeader(stream_id, token, name, value, flags) ? - 0 : NGHTTP3_ERR_CALLBACK_FAILURE; + app->ReceiveHeader(stream_id, token, name, value, flags); + return 0; } int Http3Application::OnEndHeaders( @@ -868,8 +866,7 @@ int Http3Application::OnReceivePushPromise( void* conn_user_data, void* stream_user_data) { Http3Application* app = static_cast(conn_user_data); - if (!app->ReceiveHeader(stream_id, token, name, value, flags)) - return NGHTTP3_ERR_CALLBACK_FAILURE; + app->ReceiveHeader(stream_id, token, name, value, flags); return 0; } diff --git a/src/quic/node_quic_http3_application.h b/src/quic/node_quic_http3_application.h index 353874146257fd..70a1727b0175ac 100644 --- a/src/quic/node_quic_http3_application.h +++ b/src/quic/node_quic_http3_application.h @@ -174,7 +174,7 @@ class Http3Application final : void BeginHeaders( int64_t stream_id, QuicStreamHeadersKind kind = QUICSTREAM_HEADERS_KIND_NONE); - bool ReceiveHeader( + void ReceiveHeader( int64_t stream_id, int32_t token, nghttp3_rcbuf* name, From 86111d662982d673b58ec7aae84d7daaae521e04 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 21 Aug 2020 10:14:19 -0700 Subject: [PATCH 2/5] quic: resolve debug output todo Signed-off-by: James M Snell --- src/quic/node_quic_session.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index 9b74f03b0cc635..c783b4f6ff54de 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -178,12 +178,8 @@ std::string QuicSession::RemoteTransportParamsDebug::ToString() const { out += " Original Connection ID: N/A \n"; } - if (params.preferred_address_present) { - out += " Preferred Address Present: Yes\n"; - // TODO(@jasnell): Serialize the IPv4 and IPv6 address options - } else { - out += " Preferred Address Present: No\n"; - } + out += " Preferred Address Present: " + + params.preferred_address_present ? "Yes" : "No"; if (params.stateless_reset_token_present) { StatelessResetToken token(params.stateless_reset_token); From 1e78da15e3e0f4de2ce587f99d85e65634849b16 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 21 Aug 2020 10:22:24 -0700 Subject: [PATCH 3/5] quic: clarify and resolve todo comment Signed-off-by: James M Snell --- src/quic/node_quic_socket.cc | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/quic/node_quic_socket.cc b/src/quic/node_quic_socket.cc index abbdd50e47040b..2172161e3c0a2e 100644 --- a/src/quic/node_quic_socket.cc +++ b/src/quic/node_quic_socket.cc @@ -542,19 +542,17 @@ void QuicSocket::OnReceive( // a stateless reset. The stateless reset contains a token derived // from the received destination connection ID. // - // TODO(@jasnell): Stateless resets are generated programmatically - // using HKDF with the sender provided dcid and a locally provided - // secret as input. It is entirely possible that a malicious - // peer could send multiple stateless reset eliciting packets - // with the specific intent of using the returned stateless - // reset to guess the stateless reset token secret used by - // the server. Once guessed, the malicious peer could use + // Stateless resets are generated programmatically using HKDF with + // the sender provided dcid and a locally provided secret as input. + // It is entirely possible that a malicious peer could send multiple + // stateless reset eliciting packets with the specific intent of using + // the returned stateless reset to guess the stateless reset token + // secret used by the server. Once guessed, the malicious peer could use // that secret as a DOS vector against other peers. We currently // implement some mitigations for this by limiting the number // of stateless resets that can be sent to a specific remote - // address but there are other possible mitigations, such as - // including the remote address as input in the generation of - // the stateless token. + // address and we generate a random nonce used in creation of the + // token. if (is_short_header && SendStatelessReset(dcid, local_addr, remote_addr, nread)) { Debug(this, "Sent stateless reset"); @@ -620,7 +618,7 @@ void QuicSocket::SendVersionNegotiation( SendPacket(local_addr, remote_address, std::move(packet)); } -// Possible generates and sends a stateless reset packet. +// Possibly generates and sends a stateless reset packet. // This is terminal for the connection. It is possible // that a malicious packet triggered this so we need to // be careful not to commit too many resources. From aaaa51fcc6968215a2531be7cd4df76bf69ef9f1 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 21 Aug 2020 10:23:37 -0700 Subject: [PATCH 4/5] quic: remove unneeded TODO comment Signed-off-by: James M Snell --- src/quic/node_quic_session.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index c783b4f6ff54de..253b333d80e430 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -3613,8 +3613,6 @@ void QuicSession::OnQlogWrite( BaseObjectPtr QLogStream::Create(Environment* env) { HandleScope scope(env->isolate()); - // TODO(@jasnell): There is identical code in heap_utils for the - // HeapSnapshotStream. We can consolidate the two. if (env->qlogoutputstream_constructor_template().IsEmpty()) { // Create FunctionTemplate for QLogStream Local os = FunctionTemplate::New(env->isolate()); From 097e9602602dfb421dee0f00c4cd2c65ed699b17 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 24 Aug 2020 09:48:45 -0700 Subject: [PATCH 5/5] deps: fixup win arm64 build for ngtcp2/nghttp3 --- deps/nghttp3/lib/nghttp3_ringbuf.c | 12 ++++++++---- deps/ngtcp2/lib/ngtcp2_ringbuf.c | 10 +++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/deps/nghttp3/lib/nghttp3_ringbuf.c b/deps/nghttp3/lib/nghttp3_ringbuf.c index 9ea91c81c8a1b9..a9d68680bbde19 100644 --- a/deps/nghttp3/lib/nghttp3_ringbuf.c +++ b/deps/nghttp3/lib/nghttp3_ringbuf.c @@ -33,21 +33,25 @@ #include "nghttp3_macro.h" -#if defined(_MSC_VER) && defined(_M_ARM64) -unsigned int __popcnt(unsigned int x) { +#if defined(_WIN32) +# if defined(_M_ARM64) +unsigned int __nghttp3_popcnt(unsigned int x) { unsigned int c = 0; for (; x; ++c) { x &= x - 1; } return c; } +# else +# define __nghttp3_popcnt __popcnt +# endif #endif int nghttp3_ringbuf_init(nghttp3_ringbuf *rb, size_t nmemb, size_t size, const nghttp3_mem *mem) { if (nmemb) { #ifdef WIN32 - assert(1 == __popcnt((unsigned int)nmemb)); + assert(1 == __nghttp3_popcnt((unsigned int)nmemb)); #else assert(1 == __builtin_popcount((unsigned int)nmemb)); #endif @@ -127,7 +131,7 @@ int nghttp3_ringbuf_reserve(nghttp3_ringbuf *rb, size_t nmemb) { } #ifdef WIN32 - assert(1 == __popcnt((unsigned int)nmemb)); + assert(1 == __nghttp3_popcnt((unsigned int)nmemb)); #else assert(1 == __builtin_popcount((unsigned int)nmemb)); #endif diff --git a/deps/ngtcp2/lib/ngtcp2_ringbuf.c b/deps/ngtcp2/lib/ngtcp2_ringbuf.c index e4deab1ff76b83..c11ed317b083fd 100644 --- a/deps/ngtcp2/lib/ngtcp2_ringbuf.c +++ b/deps/ngtcp2/lib/ngtcp2_ringbuf.c @@ -31,20 +31,24 @@ #include "ngtcp2_macro.h" -#if defined(_MSC_VER) && defined(_M_ARM64) -unsigned int __popcnt(unsigned int x) { +#if defined(_WIN32) +# if defined(_M_ARM64) +unsigned int __ngtcp2_popcnt(unsigned int x) { unsigned int c = 0; for (; x; ++c) { x &= x - 1; } return c; } +# else +# define __ngtcp2_popcnt __popcnt +# endif #endif int ngtcp2_ringbuf_init(ngtcp2_ringbuf *rb, size_t nmemb, size_t size, const ngtcp2_mem *mem) { #ifdef WIN32 - assert(1 == __popcnt((unsigned int)nmemb)); + assert(1 == __ngtcp2_popcnt((unsigned int)nmemb)); #else assert(1 == __builtin_popcount((unsigned int)nmemb)); #endif