diff --git a/api/envoy/api/v2/core/protocol.proto b/api/envoy/api/v2/core/protocol.proto index 200b8517abd1..88a82077a428 100644 --- a/api/envoy/api/v2/core/protocol.proto +++ b/api/envoy/api/v2/core/protocol.proto @@ -49,6 +49,7 @@ message Http1ProtocolOptions { string default_host_for_http_10 = 3; } +// [#comment:next free field: 13] message Http2ProtocolOptions { // `Maximum table size `_ // (in octets) that the encoder is permitted to use for the dynamic HPACK table. Valid values @@ -91,6 +92,63 @@ message Http2ProtocolOptions { // docs](https://github.com/envoyproxy/envoy/blob/master/source/docs/h2_metadata.md) for more // information. bool allow_metadata = 6; + + // Limit the number of pending outbound downstream frames of all types (frames that are waiting to + // be written into the socket). Exceeding this limit triggers flood mitigation and connection is + // terminated. The ``http2.outbound_flood`` stat tracks the number of terminated connections due + // to flood mitigation. The default limit is 10000. + // [#comment:TODO: implement same limits for upstream outbound frames as well.] + google.protobuf.UInt32Value max_outbound_frames = 7 [(validate.rules).uint32 = {gte: 1}]; + + // Limit the number of pending outbound downstream frames of types PING, SETTINGS and RST_STREAM, + // preventing high memory utilization when receiving continuous stream of these frames. Exceeding + // this limit triggers flood mitigation and connection is terminated. The + // ``http2.outbound_control_flood`` stat tracks the number of terminated connections due to flood + // mitigation. The default limit is 1000. + // [#comment:TODO: implement same limits for upstream outbound frames as well.] + google.protobuf.UInt32Value max_outbound_control_frames = 8 [(validate.rules).uint32 = {gte: 1}]; + + // Limit the number of consecutive inbound frames of types HEADERS, CONTINUATION and DATA with an + // empty payload and no end stream flag. Those frames have no legitimate use and are abusive, but + // might be a result of a broken HTTP/2 implementation. The `http2.inbound_empty_frames_flood`` + // stat tracks the number of connections terminated due to flood mitigation. + // Setting this to 0 will terminate connection upon receiving first frame with an empty payload + // and no end stream flag. The default limit is 1. + // [#comment:TODO: implement same limits for upstream inbound frames as well.] + google.protobuf.UInt32Value max_consecutive_inbound_frames_with_empty_payload = 9; + + // Limit the number of inbound PRIORITY frames allowed per each opened stream. If the number + // of PRIORITY frames received over the lifetime of connection exceeds the value calculated + // using this formula:: + // + // max_inbound_priority_frames_per_stream * (1 + inbound_streams) + // + // the connection is terminated. The ``http2.inbound_priority_frames_flood`` stat tracks + // the number of connections terminated due to flood mitigation. The default limit is 100. + // [#comment:TODO: implement same limits for upstream inbound frames as well.] + google.protobuf.UInt32Value max_inbound_priority_frames_per_stream = 10; + + // Limit the number of inbound WINDOW_UPDATE frames allowed per DATA frame sent. If the number + // of WINDOW_UPDATE frames received over the lifetime of connection exceeds the value calculated + // using this formula:: + // + // 1 + 2 * (inbound_streams + + // max_inbound_window_update_frames_per_data_frame_sent * outbound_data_frames) + // + // the connection is terminated. The ``http2.inbound_priority_frames_flood`` stat tracks + // the number of connections terminated due to flood mitigation. The default limit is 10. + // Setting this to 1 should be enough to support HTTP/2 implementations with basic flow control, + // but more complex implementations that try to estimate available bandwidth require at least 2. + // [#comment:TODO: implement same limits for upstream inbound frames as well.] + google.protobuf.UInt32Value max_inbound_window_update_frames_per_data_frame_sent = 11 + [(validate.rules).uint32 = {gte: 1}]; + + // Allows invalid HTTP messaging and headers. When this option is disabled (default), then + // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, + // when this option is enabled, only the offending stream is terminated. + // + // See [RFC7540, sec. 8.1](https://tools.ietf.org/html/rfc7540#section-8.1) for details. + bool stream_error_on_invalid_http_messaging = 12; } // [#not-implemented-hide:] diff --git a/docs/root/configuration/http_conn_man/stats.rst b/docs/root/configuration/http_conn_man/stats.rst index 0269fefc1d4b..dc4f79879042 100644 --- a/docs/root/configuration/http_conn_man/stats.rst +++ b/docs/root/configuration/http_conn_man/stats.rst @@ -111,6 +111,11 @@ All http2 statistics are rooted at *http2.* header_overflow, Counter, Total number of connections reset due to the headers being larger than the :ref:`configured value `. headers_cb_no_stream, Counter, Total number of errors where a header callback is called without an associated stream. This tracks an unexpected occurrence due to an as yet undiagnosed bug + inbound_empty_frames_flood, Counter, Total number of connections terminated for exceeding the limit on consecutive inbound frames with an empty payload and no end stream flag. The limit is configured by setting the :ref:`max_consecutive_inbound_frames_with_empty_payload config setting `. + inbound_priority_frames_flood, Counter, Total number of connections terminated for exceeding the limit on inbound frames of type PRIORITY. The limit is configured by setting the :ref:`max_inbound_priority_frames_per_stream config setting `. + inbound_window_update_frames_flood, Counter, Total number of connections terminated for exceeding the limit on inbound frames of type WINDOW_UPDATE. The limit is configured by setting the :ref:`max_inbound_window_updateframes_per_data_frame_sent config setting `. + outbound_flood, Counter, Total number of connections terminated for exceeding the limit on outbound frames of all types. The limit is configured by setting the :ref:`max_outbound_frames config setting `. + outbound_control_flood, Counter, "Total number of connections terminated for exceeding the limit on outbound frames of types PING, SETTINGS and RST_STREAM. The limit is configured by setting the :ref:`max_outbound_control_frames config setting `." rx_messaging_error, Counter, Total number of invalid received frames that violated `section 8 `_ of the HTTP/2 spec. This will result in a *tx_reset* rx_reset, Counter, Total number of reset stream frames received by Envoy too_many_header_frames, Counter, Total number of times an HTTP2 connection is reset due to receiving too many headers frames. Envoy currently supports proxying at most one header frame for 100-Continue one non-100 response code header frame and one frame with trailers diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 220514b18cf1..e1de54b58df9 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,6 +3,18 @@ Version history 1.11.0 (Pending) ================ +1.11.1 (Pending) +================ +* http: added mitigation of client initiated atacks that result in flooding of the downstream HTTP/2 connections. +* http: added :ref:`inbound_empty_frames_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the limit on consecutive inbound frames with an empty payload and no end stream flag. The limit is configured by setting the :ref:`max_consecutive_inbound_frames_with_empty_payload config setting `. +* http: added :ref:`inbound_priority_frames_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the limit on inbound PRIORITY frames. The limit is configured by setting the :ref:`max_inbound_priority_frames_per_stream config setting `. +* http: added :ref:`inbound_window_update_frames_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the limit on inbound WINDOW_UPDATE frames. The limit is configured by setting the :ref:`max_inbound_window_update_frames_per_data_frame_sent config setting `. +* http: added :ref:`outbound_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the outbound queue limit. The limit is configured by setting the :ref:`max_outbound_frames config setting ` +* http: added :ref:`outbound_control_flood ` counter stat to the HTTP/2 codec stats, for tracking number of connections terminated for exceeding the outbound queue limit for PING, SETTINGS and RST_STREAM frames. The limit is configured by setting the :ref:`max_outbound_control_frames config setting `. +* http: enabled strict validation of HTTP/2 messaging. Previous behavior can be restored using :ref:`stream_error_on_invalid_http_messaging config setting `. + +1.11.0 (July 11, 2019) +====================== * access log: added a new field for downstream TLS session ID to file and gRPC access logger. * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * admin: the administration interface now includes a :ref:`/ready endpoint ` for easier readiness checks. diff --git a/include/envoy/buffer/buffer.h b/include/envoy/buffer/buffer.h index 48f5c1842a0f..7407548597c5 100644 --- a/include/envoy/buffer/buffer.h +++ b/include/envoy/buffer/buffer.h @@ -33,6 +33,7 @@ struct RawSlice { */ class BufferFragment { public: + virtual ~BufferFragment() = default; /** * @return const void* a pointer to the referenced data. */ @@ -47,9 +48,6 @@ class BufferFragment { * Called by a buffer when the referenced data is no longer needed. */ virtual void done() PURE; - -protected: - virtual ~BufferFragment() {} }; /** diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index 4bcceeaca978..55f4fb7154f4 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -235,6 +235,14 @@ struct Http2Settings { uint32_t initial_connection_window_size_{DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE}; bool allow_connect_{DEFAULT_ALLOW_CONNECT}; bool allow_metadata_{DEFAULT_ALLOW_METADATA}; + bool stream_error_on_invalid_http_messaging_{DEFAULT_STREAM_ERROR_ON_INVALID_HTTP_MESSAGING}; + uint32_t max_outbound_frames_{DEFAULT_MAX_OUTBOUND_FRAMES}; + uint32_t max_outbound_control_frames_{DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES}; + uint32_t max_consecutive_inbound_frames_with_empty_payload_{ + DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD}; + uint32_t max_inbound_priority_frames_per_stream_{DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM}; + uint32_t max_inbound_window_update_frames_per_data_frame_sent_{ + DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT}; // disable HPACK compression static const uint32_t MIN_HPACK_TABLE_SIZE = 0; @@ -272,6 +280,20 @@ struct Http2Settings { static const bool DEFAULT_ALLOW_CONNECT = false; // By default Envoy does not allow METADATA support. static const bool DEFAULT_ALLOW_METADATA = false; + // By default Envoy does not allow invalid headers. + static const bool DEFAULT_STREAM_ERROR_ON_INVALID_HTTP_MESSAGING = false; + + // Default limit on the number of outbound frames of all types. + static const uint32_t DEFAULT_MAX_OUTBOUND_FRAMES = 10000; + // Default limit on the number of outbound frames of types PING, SETTINGS and RST_STREAM. + static const uint32_t DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES = 1000; + // Default limit on the number of consecutive inbound frames with an empty payload + // and no end stream flag. + static const uint32_t DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD = 1; + // Default limit on the number of inbound frames of type PRIORITY (per stream). + static const uint32_t DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM = 100; + // Default limit on the number of inbound frames of type WINDOW_UPDATE (per DATA frame sent). + static const uint32_t DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT = 10; }; /** diff --git a/source/common/buffer/BUILD b/source/common/buffer/BUILD index f4e6d9602c19..ea7d6654f68b 100644 --- a/source/common/buffer/BUILD +++ b/source/common/buffer/BUILD @@ -26,6 +26,7 @@ envoy_cc_library( "//include/envoy/buffer:buffer_interface", "//source/common/common:non_copyable", "//source/common/common:stack_array", + "//source/common/common:utility_lib", "//source/common/event:libevent_lib", ], ) diff --git a/source/common/buffer/buffer_impl.h b/source/common/buffer/buffer_impl.h index b79069d461b6..41be35d2db95 100644 --- a/source/common/buffer/buffer_impl.h +++ b/source/common/buffer/buffer_impl.h @@ -10,6 +10,7 @@ #include "common/common/assert.h" #include "common/common/non_copyable.h" +#include "common/common/utility.h" #include "common/event/libevent.h" namespace Envoy { @@ -203,7 +204,8 @@ class Slice { using SlicePtr = std::unique_ptr; -class OwnedSlice : public Slice { +// OwnedSlice can not be derived from as it has variable sized array as member. +class OwnedSlice final : public Slice, public InlineStorage { public: /** * Create an empty OwnedSlice. @@ -230,16 +232,7 @@ class OwnedSlice : public Slice { return slice; } - // Custom delete operator to keep C++14 from using the global operator delete(void*, size_t), - // which would result in the compiler error: - // "exception cleanup for this placement new selects non-placement operator delete" - static void operator delete(void* address) { ::operator delete(address); } - private: - static void* operator new(size_t object_size, size_t data_size) { - return ::operator new(object_size + data_size); - } - OwnedSlice(uint64_t size) : Slice(0, 0, size) { base_ = storage_; } /** @@ -582,5 +575,46 @@ class OwnedImpl : public LibEventInstance { Event::Libevent::BufferPtr buffer_; }; +using BufferFragmentPtr = std::unique_ptr; + +/** + * An implementation of BufferFragment where a releasor callback is called when the data is + * no longer needed. Copies data into internal buffer. + */ +class OwnedBufferFragmentImpl final : public BufferFragment, public InlineStorage { +public: + using Releasor = std::function; + + /** + * Copies the data into internal buffer. The releasor is called when the data has been + * fully drained or the buffer that contains this fragment is destroyed. + * @param data external data to reference + * @param releasor a callback function to be called when data is no longer needed. + */ + + static BufferFragmentPtr create(absl::string_view data, const Releasor& releasor) { + return BufferFragmentPtr(new (sizeof(OwnedBufferFragmentImpl) + data.size()) + OwnedBufferFragmentImpl(data, releasor)); + } + + // Buffer::BufferFragment + const void* data() const override { return data_; } + size_t size() const override { return size_; } + void done() override { releasor_(this); } + +private: + OwnedBufferFragmentImpl(absl::string_view data, const Releasor& releasor) + : releasor_(releasor), size_(data.size()) { + ASSERT(releasor != nullptr); + memcpy(data_, data.data(), data.size()); + } + + const Releasor releasor_; + const size_t size_; + uint8_t data_[]; +}; + +using OwnedBufferFragmentImplPtr = std::unique_ptr; + } // namespace Buffer } // namespace Envoy diff --git a/source/common/common/BUILD b/source/common/common/BUILD index a758943e1c1a..2c2cb2dfaf5f 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -238,6 +238,7 @@ envoy_cc_library( deps = [ ":assert_lib", ":hash_lib", + ":non_copyable", "//include/envoy/common:interval_set_interface", "//include/envoy/common:time_interface", "//source/common/singleton:const_singleton", diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc index 83ed53c3b952..190cfc40b598 100644 --- a/source/common/common/utility.cc +++ b/source/common/common/utility.cc @@ -532,4 +532,9 @@ double WelfordStandardDeviation::computeStandardDeviation() const { return (std::isnan(variance) || variance < 0) ? std::nan("") : sqrt(variance); } +InlineString::InlineString(const char* str, size_t size) : size_(size) { + RELEASE_ASSERT(size <= 0xffffffff, "size must fit in 32 bits"); + memcpy(data_, str, size); +} + } // namespace Envoy diff --git a/source/common/common/utility.h b/source/common/common/utility.h index 0de2bb32709d..5f9a99a5e109 100644 --- a/source/common/common/utility.h +++ b/source/common/common/utility.h @@ -14,6 +14,7 @@ #include "common/common/assert.h" #include "common/common/hash.h" +#include "common/common/non_copyable.h" #include "absl/strings/string_view.h" @@ -632,4 +633,111 @@ template struct TrieLookupTable { TrieEntry root_; }; +// Mix-in class for allocating classes with variable-sized inlined storage. +// +// Use this class by inheriting from it, ensuring that: +// - The variable sized array is declared as VarType[] as the last +// member variable of the class. +// - YourType accurately describes the type that will be stored there, +// to enable the compiler to perform correct alignment. No casting +// should be needed. +// - The class constructor is private, because you need to allocate the +// class the placed new operator exposed in the protected section below. +// Constructing the class directly will not provide space for the +// variable-size data. +// - You expose a public factory method that return a placement-new, e.g. +// static YourClass* alloc(size_t num_elements, constructor_args...) { +// new (num_elements * sizeof(VarType)) YourClass(constructor_args...); +// } +// +// See InlineString below for an example usage. +// +// +// Perf note: The alignment will be correct and safe without further +// consideration as long as there are no casts. But for micro-optimization, +// consider this case: +// struct MyStruct : public InlineStorage { uint64_t a_; uint16_t b_; uint8_t data_[]; }; +// When compiled with a typical compiler on a 64-bit machine: +// sizeof(MyStruct) == 16, because the compiler will round up from 10 for uint64_t alignment. +// So: +// calling new (6) MyStruct() causes an allocation of 16+6=22, rounded up to 24 bytes. +// But data_ doesn't need 8-byte alignment, so it will wind up adjacent to the uint16_t. +// ((char*) my_struct.data) - ((char*) &my_struct) == 10 +// If we had instead declared data_[6], then the whole allocation would have fit in 16 bytes. +// Instead: +// - the starting address of data will not be 8-byte aligned. This is not required +// by the C++ standard for a uint8_t, but may be suboptimal on some processors. +// - the 6 bytes of data will be at byte offsets 10 to 15, and bytes 16 to 23 will be +// unused. This may be surprising to some users, and suboptimal in resource usage. +// One possible tweak is to declare data_ as a uint64_t[], or to use an `alignas` +// declaration. As always, micro-optimizations should be informed by +// microbenchmarks, showing the benefit. +class InlineStorage : public NonCopyable { +public: + // Custom delete operator to keep C++14 from using the global operator delete(void*, size_t), + // which would result in the compiler error: + // "exception cleanup for this placement new selects non-placement operator delete" + static void operator delete(void* address) { ::operator delete(address); } + +protected: + /** + * @param object_size the size of the base object; supplied automatically by the compiler. + * @param data_size the amount of variable-size storage to be added, in bytes. + * @return a variable-size object based on data_size_bytes. + */ + static void* operator new(size_t object_size, size_t data_size_bytes) { + return ::operator new(object_size + data_size_bytes); + } +}; + +class InlineString; +using InlineStringPtr = std::unique_ptr; + +// Represents immutable string data, keeping the storage inline with the +// object. These cannot be copied or held by value; they must be created +// as unique pointers. +// +// Note: this is not yet proven better (smaller or faster) than std::string for +// all applications, but memory-size improvements have been measured for one +// application (Stats::SymbolTableImpl). This is presented here to serve as an +// example of how to use InlineStorage. +class InlineString : public InlineStorage { +public: + /** + * @param str the string_view for which to create an InlineString + * @return a unique_ptr to the InlineString containing the bytes of str. + */ + static InlineStringPtr create(absl::string_view str) { + return InlineStringPtr(new (str.size()) InlineString(str.data(), str.size())); + } + + /** + * @return a std::string copy of the InlineString. + */ + std::string toString() const { return std::string(data_, size_); } + + /** + * @return a string_view into the InlineString. + */ + absl::string_view toStringView() const { return absl::string_view(data_, size_); } + + /** + * @return the number of bytes in the string + */ + size_t size() const { return size_; } + + /** + * @return a pointer to the first byte of the string. + */ + const char* data() const { return data_; } + +private: + // Constructor is declared private so that no one constructs one without the + // proper size allocation. to accommodate the variable-size buffer. + InlineString(const char* str, size_t size); + + uint32_t size_; + char data_[]; +}; + } // namespace Envoy diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 12dce494f369..80436c011634 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -180,10 +180,12 @@ void ConnectionManagerImpl::doEndStream(ActiveStream& stream) { // explicitly nulls out response_encoder to avoid the downstream being notified of the // Envoy-internal stream instance being ended. if (stream.response_encoder_ != nullptr && - (!stream.state_.remote_complete_ || !stream.state_.local_complete_)) { + (!stream.state_.remote_complete_ || !stream.state_.codec_saw_local_complete_)) { // Indicate local is complete at this point so that if we reset during a continuation, we don't // raise further data or trailers. + ENVOY_STREAM_LOG(debug, "doEndStream() resetting stream", stream); stream.state_.local_complete_ = true; + stream.state_.codec_saw_local_complete_ = true; stream.response_encoder_->getStream().resetStream(StreamResetReason::LocalReset); reset_stream = true; } @@ -254,6 +256,18 @@ StreamDecoder& ConnectionManagerImpl::newStream(StreamEncoder& response_encoder, return **streams_.begin(); } +void ConnectionManagerImpl::handleCodecException(const char* error) { + ENVOY_CONN_LOG(debug, "dispatch error: {}", read_callbacks_->connection(), error); + + // In the protocol error case, we need to reset all streams now. The connection might stick around + // long enough for a pending stream to come back and try to encode. + resetAllStreams(); + + // HTTP/1.1 codec has already sent a 400 response if possible. HTTP/2 codec has already sent + // GOAWAY. + read_callbacks_->connection().close(Network::ConnectionCloseType::FlushWriteAndDelay); +} + Network::FilterStatus ConnectionManagerImpl::onData(Buffer::Instance& data, bool) { if (!codec_) { codec_ = config_.createCodec(read_callbacks_->connection(), data, *this); @@ -272,18 +286,12 @@ Network::FilterStatus ConnectionManagerImpl::onData(Buffer::Instance& data, bool try { codec_->dispatch(data); + } catch (const FrameFloodException& e) { + handleCodecException(e.what()); + return Network::FilterStatus::StopIteration; } catch (const CodecProtocolException& e) { - // HTTP/1.1 codec has already sent a 400 response if possible. HTTP/2 codec has already sent - // GOAWAY. - ENVOY_CONN_LOG(debug, "dispatch error: {}", read_callbacks_->connection(), e.what()); stats_.named_.downstream_cx_protocol_error_.inc(); - - // In the protocol error case, we need to reset all streams now. Since we do a flush write and - // delayed close, the connection might stick around long enough for a pending stream to come - // back and try to encode. - resetAllStreams(); - - read_callbacks_->connection().close(Network::ConnectionCloseType::FlushWriteAndDelay); + handleCodecException(e.what()); return Network::FilterStatus::StopIteration; } @@ -311,8 +319,17 @@ Network::FilterStatus ConnectionManagerImpl::onData(Buffer::Instance& data, bool void ConnectionManagerImpl::resetAllStreams() { while (!streams_.empty()) { - // Mimic a downstream reset in this case. - streams_.front()->onResetStream(StreamResetReason::ConnectionTermination, absl::string_view()); + // Mimic a downstream reset in this case. We must also remove callbacks here. Though we are + // about to close the connection and will disable further reads, it is possible that flushing + // data out can cause stream callbacks to fire (e.g., low watermark callbacks). + // + // TODO(mattklein123): I tried to actually reset through the codec here, but ran into issues + // with nghttp2 state and being unhappy about sending reset frames after the connection had + // been terminated via GOAWAY. It might be possible to do something better here inside the h2 + // codec but there are no easy answers and this seems simpler. + auto& stream = *streams_.front(); + stream.response_encoder_->getStream().removeCallbacks(stream); + stream.onResetStream(StreamResetReason::ConnectionTermination, absl::string_view()); } } @@ -1517,6 +1534,8 @@ void ConnectionManagerImpl::ActiveStream::encodeTrailers(ActiveStreamEncoderFilt void ConnectionManagerImpl::ActiveStream::maybeEndEncode(bool end_stream) { if (end_stream) { + ASSERT(!state_.codec_saw_local_complete_); + state_.codec_saw_local_complete_ = true; stream_info_.onLastDownstreamTxByteSent(); request_response_timespan_->complete(); connection_manager_.doEndStream(*this); @@ -1542,6 +1561,7 @@ void ConnectionManagerImpl::ActiveStream::onResetStream(StreamResetReason, absl: // 2) The codec TX a codec level reset // 3) The codec RX a reset // If we need to differentiate we need to do it inside the codec. Can start with this. + ENVOY_STREAM_LOG(debug, "stream reset", *this); connection_manager_.stats_.named_.downstream_rq_rx_reset_.inc(); connection_manager_.doDeferredStreamDestroy(*this); } diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index fbf53d7e7ac8..36ab18ac1964 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -450,8 +450,8 @@ class ConnectionManagerImpl : Logger::Loggable, // All state for the stream. Put here for readability. struct State { State() - : remote_complete_(false), local_complete_(false), saw_connection_close_(false), - successful_upgrade_(false), created_filter_chain_(false), + : remote_complete_(false), local_complete_(false), codec_saw_local_complete_(false), + saw_connection_close_(false), successful_upgrade_(false), created_filter_chain_(false), is_internally_created_(false) {} uint32_t filter_call_state_{0}; @@ -462,7 +462,11 @@ class ConnectionManagerImpl : Logger::Loggable, bool decoder_filters_streaming_{true}; bool destroyed_{false}; bool remote_complete_ : 1; - bool local_complete_ : 1; + bool local_complete_ : 1; // This indicates that local is complete prior to filter processing. + // A filter can still stop the stream from being complete as seen + // by the codec. + bool codec_saw_local_complete_ : 1; // This indicates that local is complete as written all + // the way through to the codec. bool saw_connection_close_ : 1; bool successful_upgrade_ : 1; bool created_filter_chain_ : 1; @@ -554,6 +558,7 @@ class ConnectionManagerImpl : Logger::Loggable, void onDrainTimeout(); void startDrainSequence(); Tracing::HttpTracer& tracer() { return http_context_.tracer(); } + void handleCodecException(const char* error); enum class DrainState { NotDraining, Draining, Closing }; diff --git a/source/common/http/exception.h b/source/common/http/exception.h index 445ef5fb1407..1a7ef668b24b 100644 --- a/source/common/http/exception.h +++ b/source/common/http/exception.h @@ -16,6 +16,14 @@ class CodecProtocolException : public EnvoyException { CodecProtocolException(const std::string& message) : EnvoyException(message) {} }; +/** + * Raised when outbound frame queue flood is detected. + */ +class FrameFloodException : public CodecProtocolException { +public: + FrameFloodException(const std::string& message) : CodecProtocolException(message) {} +}; + /** * Raised when a response is received on a connection that did not send a request. In practice * this can only happen on HTTP/1.1 connections. diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 5fe699fc1a98..ea9ff93cc33f 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -11,6 +11,7 @@ #include "envoy/stats/scope.h" #include "common/common/assert.h" +#include "common/common/cleanup.h" #include "common/common/enum_to_int.h" #include "common/common/fmt.h" #include "common/common/stack_array.h" @@ -251,7 +252,15 @@ int ConnectionImpl::StreamImpl::onDataSourceSend(const uint8_t* framehd, size_t // https://nghttp2.org/documentation/types.html#c.nghttp2_send_data_callback static const uint64_t FRAME_HEADER_SIZE = 9; - Buffer::OwnedImpl output(framehd, FRAME_HEADER_SIZE); + parent_.outbound_data_frames_++; + + Buffer::OwnedImpl output; + if (!parent_.addOutboundFrameFragment(output, framehd, FRAME_HEADER_SIZE)) { + ENVOY_CONN_LOG(debug, "error sending data frame: Too many frames in the outbound queue", + parent_.connection_); + return NGHTTP2_ERR_FLOODED; + } + output.move(pending_send_data_, length); parent_.connection_.write(output, false); return 0; @@ -348,6 +357,10 @@ void ConnectionImpl::dispatch(Buffer::Instance& data) { dispatching_ = true; ssize_t rc = nghttp2_session_mem_recv(session_, static_cast(slice.mem_), slice.len_); + if (rc == NGHTTP2_ERR_FLOODED || flood_detected_) { + throw FrameFloodException( + "Flooding was detected in this HTTP/2 session, and it must be closed"); + } if (rc != static_cast(slice.len_)) { throw CodecProtocolException(fmt::format("{}", nghttp2_strerror(rc))); } @@ -397,9 +410,36 @@ void ConnectionImpl::shutdownNotice() { sendPendingFrames(); } +int ConnectionImpl::onBeforeFrameReceived(const nghttp2_frame_hd* hd) { + ENVOY_CONN_LOG(trace, "about to recv frame type={}, flags={}", connection_, + static_cast(hd->type), static_cast(hd->flags)); + + // Track all the frames without padding here, since this is the only callback we receive + // for some of them (e.g. CONTINUATION frame, frames sent on closed streams, etc.). + // HEADERS frame is tracked in onBeginHeaders(), DATA frame is tracked in onFrameReceived(). + if (hd->type != NGHTTP2_HEADERS && hd->type != NGHTTP2_DATA) { + if (!trackInboundFrames(hd, 0)) { + return NGHTTP2_ERR_FLOODED; + } + } + + return 0; +} + int ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) { ENVOY_CONN_LOG(trace, "recv frame type={}", connection_, static_cast(frame->hd.type)); + // onFrameReceived() is called with a complete HEADERS frame assembled from all the HEADERS + // and CONTINUATION frames, but we track them separately: HEADERS frames in onBeginHeaders() + // and CONTINUATION frames in onBeforeFrameReceived(). + ASSERT(frame->hd.type != NGHTTP2_CONTINUATION); + + if (frame->hd.type == NGHTTP2_DATA) { + if (!trackInboundFrames(&frame->hd, frame->data.padlen)) { + return NGHTTP2_ERR_FLOODED; + } + } + // Only raise GOAWAY once, since we don't currently expose stream information. Shutdown // notifications are the same as a normal GOAWAY. if (frame->hd.type == NGHTTP2_GOAWAY && !raised_goaway_) { @@ -511,6 +551,7 @@ int ConnectionImpl::onFrameSend(const nghttp2_frame* frame) { ENVOY_CONN_LOG(trace, "sent frame type={}", connection_, static_cast(frame->hd.type)); switch (frame->hd.type) { case NGHTTP2_GOAWAY: { + ENVOY_CONN_LOG(debug, "sent goaway code={}", connection_, frame->goaway.error_code); if (frame->goaway.error_code != NGHTTP2_NO_ERROR) { return NGHTTP2_ERR_CALLBACK_FAILURE; } @@ -538,25 +579,96 @@ int ConnectionImpl::onInvalidFrame(int32_t stream_id, int error_code) { ENVOY_CONN_LOG(debug, "invalid frame: {} on stream {}", connection_, nghttp2_strerror(error_code), stream_id); - // The stream is about to be closed due to an invalid header or messaging. Don't kill the - // entire connection if one stream has bad headers or messaging. if (error_code == NGHTTP2_ERR_HTTP_HEADER || error_code == NGHTTP2_ERR_HTTP_MESSAGING) { stats_.rx_messaging_error_.inc(); - StreamImpl* stream = getStream(stream_id); - if (stream != nullptr) { - // See comment below in onStreamClose() for why we do this. - stream->reset_due_to_messaging_error_ = true; + + if (stream_error_on_invalid_http_messaging_) { + // The stream is about to be closed due to an invalid header or messaging. Don't kill the + // entire connection if one stream has bad headers or messaging. + StreamImpl* stream = getStream(stream_id); + if (stream != nullptr) { + // See comment below in onStreamClose() for why we do this. + stream->reset_due_to_messaging_error_ = true; + } + return 0; } - return 0; } // Cause dispatch to return with an error code. return NGHTTP2_ERR_CALLBACK_FAILURE; } +int ConnectionImpl::onBeforeFrameSend(const nghttp2_frame* frame) { + ENVOY_CONN_LOG(trace, "about to send frame type={}, flags={}", connection_, + static_cast(frame->hd.type), static_cast(frame->hd.flags)); + ASSERT(!is_outbound_flood_monitored_control_frame_); + // Flag flood monitored outbound control frames. + is_outbound_flood_monitored_control_frame_ = + ((frame->hd.type == NGHTTP2_PING || frame->hd.type == NGHTTP2_SETTINGS) && + frame->hd.flags & NGHTTP2_FLAG_ACK) || + frame->hd.type == NGHTTP2_RST_STREAM; + return 0; +} + +void ConnectionImpl::incrementOutboundFrameCount(bool is_outbound_flood_monitored_control_frame) { + ++outbound_frames_; + if (is_outbound_flood_monitored_control_frame) { + ++outbound_control_frames_; + } + checkOutboundQueueLimits(); +} + +bool ConnectionImpl::addOutboundFrameFragment(Buffer::OwnedImpl& output, const uint8_t* data, + size_t length) { + // Reset the outbound frame type (set in the onBeforeFrameSend callback) since the + // onBeforeFrameSend callback is not called for DATA frames. + bool is_outbound_flood_monitored_control_frame = false; + std::swap(is_outbound_flood_monitored_control_frame, is_outbound_flood_monitored_control_frame_); + try { + incrementOutboundFrameCount(is_outbound_flood_monitored_control_frame); + } catch (const FrameFloodException&) { + return false; + } + + auto fragment = Buffer::OwnedBufferFragmentImpl::create( + absl::string_view(reinterpret_cast(data), length), + is_outbound_flood_monitored_control_frame ? control_frame_buffer_releasor_ + : frame_buffer_releasor_); + + // The Buffer::OwnedBufferFragmentImpl object will be deleted in the *frame_buffer_releasor_ + // callback. + output.addBufferFragment(*fragment.release()); + return true; +} + +void ConnectionImpl::releaseOutboundFrame(const Buffer::OwnedBufferFragmentImpl* fragment) { + ASSERT(outbound_frames_ >= 1); + --outbound_frames_; + delete fragment; +} + +void ConnectionImpl::releaseOutboundControlFrame(const Buffer::OwnedBufferFragmentImpl* fragment) { + ASSERT(outbound_control_frames_ >= 1); + --outbound_control_frames_; + releaseOutboundFrame(fragment); +} + ssize_t ConnectionImpl::onSend(const uint8_t* data, size_t length) { ENVOY_CONN_LOG(trace, "send data: bytes={}", connection_, length); - Buffer::OwnedImpl buffer(data, length); + Buffer::OwnedImpl buffer; + if (!addOutboundFrameFragment(buffer, data, length)) { + ENVOY_CONN_LOG(debug, "error sending frame: Too many frames in the outbound queue.", + connection_); + return NGHTTP2_ERR_FLOODED; + } + + // While the buffer is transient the fragment it contains will be moved into the + // write_buffer_ of the underlying connection_ by the write method below. + // This creates lifetime dependency between the write_buffer_ of the underlying connection + // and the codec object. Specifically the write_buffer_ MUST be either fully drained or + // deleted before the codec object is deleted. This is presently guaranteed by the + // destruction order of the Network::ConnectionImpl object where write_buffer_ is + // destroyed before the filter_manager_ which owns the codec through Http::ConnectionManagerImpl. connection_.write(buffer, false); return length; } @@ -662,6 +774,15 @@ void ConnectionImpl::sendPendingFrames() { int rc = nghttp2_session_send(session_); if (rc != 0) { ASSERT(rc == NGHTTP2_ERR_CALLBACK_FAILURE); + // For errors caused by the pending outbound frame flood the FrameFloodException has + // to be thrown. However the nghttp2 library returns only the generic error code for + // all failure types. Check queue limits and throw FrameFloodException if they were + // exceeded. + if (outbound_frames_ > max_outbound_frames_ || + outbound_control_frames_ > max_outbound_control_frames_) { + throw FrameFloodException("Too many frames in the outbound queue."); + } + throw CodecProtocolException(fmt::format("{}", nghttp2_strerror(rc))); } @@ -793,6 +914,11 @@ ConnectionImpl::Http2Callbacks::Http2Callbacks() { return static_cast(user_data)->onData(stream_id, data, len); }); + nghttp2_session_callbacks_set_on_begin_frame_callback( + callbacks_, [](nghttp2_session*, const nghttp2_frame_hd* hd, void* user_data) -> int { + return static_cast(user_data)->onBeforeFrameReceived(hd); + }); + nghttp2_session_callbacks_set_on_frame_recv_callback( callbacks_, [](nghttp2_session*, const nghttp2_frame* frame, void* user_data) -> int { return static_cast(user_data)->onFrameReceived(frame); @@ -809,6 +935,11 @@ ConnectionImpl::Http2Callbacks::Http2Callbacks() { return static_cast(user_data)->onFrameSend(frame); }); + nghttp2_session_callbacks_set_before_frame_send_callback( + callbacks_, [](nghttp2_session*, const nghttp2_frame* frame, void* user_data) -> int { + return static_cast(user_data)->onBeforeFrameSend(frame); + }); + nghttp2_session_callbacks_set_on_frame_not_send_callback( callbacks_, [](nghttp2_session*, const nghttp2_frame*, int, void*) -> int { // We used to always return failure here but it looks now this can get called if the other @@ -948,6 +1079,11 @@ ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, int ServerConnectionImpl::onBeginHeaders(const nghttp2_frame* frame) { // For a server connection, we should never get push promise frames. ASSERT(frame->hd.type == NGHTTP2_HEADERS); + + if (!trackInboundFrames(&frame->hd, frame->headers.padlen)) { + return NGHTTP2_ERR_FLOODED; + } + if (frame->headers.cat != NGHTTP2_HCAT_REQUEST) { stats_.trailers_.inc(); ASSERT(frame->headers.cat == NGHTTP2_HCAT_HEADERS); @@ -978,6 +1114,107 @@ int ServerConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na return saveHeader(frame, std::move(name), std::move(value)); } +bool ServerConnectionImpl::trackInboundFrames(const nghttp2_frame_hd* hd, uint32_t padding_length) { + ENVOY_CONN_LOG(trace, "track inbound frame type={} flags={} length={} padding_length={}", + connection_, static_cast(hd->type), static_cast(hd->flags), + static_cast(hd->length), padding_length); + switch (hd->type) { + case NGHTTP2_HEADERS: + case NGHTTP2_CONTINUATION: + // Track new streams. + if (hd->flags & NGHTTP2_FLAG_END_HEADERS) { + inbound_streams_++; + } + FALLTHRU; + case NGHTTP2_DATA: + // Track frames with an empty payload and no end stream flag. + if (hd->length - padding_length == 0 && !(hd->flags & NGHTTP2_FLAG_END_STREAM)) { + ENVOY_CONN_LOG(trace, "frame with an empty payload and no end stream flag.", connection_); + consecutive_inbound_frames_with_empty_payload_++; + } else { + consecutive_inbound_frames_with_empty_payload_ = 0; + } + break; + case NGHTTP2_PRIORITY: + inbound_priority_frames_++; + break; + case NGHTTP2_WINDOW_UPDATE: + inbound_window_update_frames_++; + break; + default: + break; + } + + if (!checkInboundFrameLimits()) { + // NGHTTP2_ERR_FLOODED is overridden within nghttp2 library and it doesn't propagate + // all the way to nghttp2_session_mem_recv() where we need it. + flood_detected_ = true; + return false; + } + + return true; +} + +bool ServerConnectionImpl::checkInboundFrameLimits() { + ASSERT(dispatching_downstream_data_); + + if (consecutive_inbound_frames_with_empty_payload_ > + max_consecutive_inbound_frames_with_empty_payload_) { + ENVOY_CONN_LOG(trace, + "error reading frame: Too many consecutive frames with an empty payload " + "received in this HTTP/2 session.", + connection_); + stats_.inbound_empty_frames_flood_.inc(); + return false; + } + + if (inbound_priority_frames_ > max_inbound_priority_frames_per_stream_ * (1 + inbound_streams_)) { + ENVOY_CONN_LOG(trace, + "error reading frame: Too many PRIORITY frames received in this HTTP/2 session.", + connection_); + stats_.inbound_priority_frames_flood_.inc(); + return false; + } + + if (inbound_window_update_frames_ > + 1 + 2 * (inbound_streams_ + + max_inbound_window_update_frames_per_data_frame_sent_ * outbound_data_frames_)) { + ENVOY_CONN_LOG( + trace, + "error reading frame: Too many WINDOW_UPDATE frames received in this HTTP/2 session.", + connection_); + stats_.inbound_window_update_frames_flood_.inc(); + return false; + } + + return true; +} + +void ServerConnectionImpl::checkOutboundQueueLimits() { + if (outbound_frames_ > max_outbound_frames_ && dispatching_downstream_data_) { + stats_.outbound_flood_.inc(); + throw FrameFloodException("Too many frames in the outbound queue."); + } + if (outbound_control_frames_ > max_outbound_control_frames_ && dispatching_downstream_data_) { + stats_.outbound_control_flood_.inc(); + throw FrameFloodException("Too many control frames in the outbound queue."); + } +} + +void ServerConnectionImpl::dispatch(Buffer::Instance& data) { + ASSERT(!dispatching_downstream_data_); + dispatching_downstream_data_ = true; + + // Make sure the dispatching_downstream_data_ is set to false even + // when ConnectionImpl::dispatch throws an exception. + Cleanup cleanup([this]() { dispatching_downstream_data_ = false; }); + + // Make sure downstream outbound queue was not flooded by the upstream frames. + checkOutboundQueueLimits(); + + ConnectionImpl::dispatch(data); +} + } // namespace Http2 } // namespace Http } // namespace Envoy diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index 1277e217e891..351fe128376b 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -37,16 +38,19 @@ const std::string CLIENT_MAGIC_PREFIX = "PRI * HTTP/2"; /** * All stats for the HTTP/2 codec. @see stats_macros.h */ -// clang-format off #define ALL_HTTP2_CODEC_STATS(COUNTER) \ COUNTER(header_overflow) \ COUNTER(headers_cb_no_stream) \ + COUNTER(inbound_empty_frames_flood) \ + COUNTER(inbound_priority_frames_flood) \ + COUNTER(inbound_window_update_frames_flood) \ + COUNTER(outbound_control_flood) \ + COUNTER(outbound_flood) \ COUNTER(rx_messaging_error) \ COUNTER(rx_reset) \ COUNTER(too_many_header_frames) \ COUNTER(trailers) \ COUNTER(tx_reset) -// clang-format on /** * Wrapper struct for the HTTP/2 codec stats. @see stats_macros.h @@ -77,12 +81,29 @@ class ConnectionImpl : public virtual Connection, protected Logger::LoggablesetEnabled(enable_half_close_ ? 0 : Event::FileReadyType::Closed); } } else { closeSocket(ConnectionEvent::LocalClose); diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h index b0ba9671de93..536632741a72 100644 --- a/source/common/network/connection_impl.h +++ b/source/common/network/connection_impl.h @@ -146,6 +146,8 @@ class ConnectionImpl : public FilterManagerConnection, Buffer::OwnedImpl read_buffer_; // This must be a WatermarkBuffer, but as it is created by a factory the ConnectionImpl only has // a generic pointer. + // It MUST be defined after the filter_manager_ as some filters may have callbacks that + // write_buffer_ invokes during its clean up. Buffer::InstancePtr write_buffer_; uint32_t read_buffer_limit_ = 0; std::chrono::milliseconds delayed_close_timeout_{0}; diff --git a/test/common/buffer/buffer_test.cc b/test/common/buffer/buffer_test.cc index 5df98389460a..95707f7f8183 100644 --- a/test/common/buffer/buffer_test.cc +++ b/test/common/buffer/buffer_test.cc @@ -209,6 +209,22 @@ TEST(UnownedSliceTest, CreateDelete) { EXPECT_TRUE(release_callback_called); } +TEST(UnownedSliceTest, CreateDeleteOwnedBufferFragment) { + constexpr char input[] = "hello world"; + bool release_callback_called = false; + auto fragment = OwnedBufferFragmentImpl::create( + {input, sizeof(input) - 1}, [&release_callback_called](const OwnedBufferFragmentImpl*) { + release_callback_called = true; + }); + auto slice = std::make_unique(*fragment); + EXPECT_EQ(11, slice->dataSize()); + EXPECT_EQ(0, slice->reservableSize()); + EXPECT_EQ(0, memcmp(slice->data(), input, slice->dataSize())); + EXPECT_FALSE(release_callback_called); + slice.reset(nullptr); + EXPECT_TRUE(release_callback_called); +} + TEST(SliceDequeTest, CreateDelete) { bool slice1_deleted = false; bool slice2_deleted = false; diff --git a/test/common/buffer/owned_impl_test.cc b/test/common/buffer/owned_impl_test.cc index 07a658bf80bd..277a3a1617e7 100644 --- a/test/common/buffer/owned_impl_test.cc +++ b/test/common/buffer/owned_impl_test.cc @@ -96,6 +96,57 @@ TEST_P(OwnedImplTest, AddBufferFragmentDynamicAllocation) { EXPECT_TRUE(release_callback_called_); } +TEST_P(OwnedImplTest, AddOwnedBufferFragmentWithCleanup) { + char input[] = "hello world"; + const size_t expected_length = sizeof(input) - 1; + auto frag = OwnedBufferFragmentImpl::create( + {input, expected_length}, + [this](const OwnedBufferFragmentImpl*) { release_callback_called_ = true; }); + Buffer::OwnedImpl buffer; + verifyImplementation(buffer); + buffer.addBufferFragment(*frag); + EXPECT_EQ(expected_length, buffer.length()); + + const uint64_t partial_drain_size = 5; + buffer.drain(partial_drain_size); + EXPECT_EQ(expected_length - partial_drain_size, buffer.length()); + EXPECT_FALSE(release_callback_called_); + + buffer.drain(expected_length - partial_drain_size); + EXPECT_EQ(0, buffer.length()); + EXPECT_TRUE(release_callback_called_); +} + +// Verify that OwnedBufferFragment work correctly when input buffer is allocated on the heap. +TEST_P(OwnedImplTest, AddOwnedBufferFragmentDynamicAllocation) { + char input_stack[] = "hello world"; + const size_t expected_length = sizeof(input_stack) - 1; + char* input = new char[expected_length]; + std::copy(input_stack, input_stack + expected_length, input); + + auto* frag = OwnedBufferFragmentImpl::create({input, expected_length}, + [this, input](const OwnedBufferFragmentImpl* frag) { + release_callback_called_ = true; + delete[] input; + delete frag; + }) + .release(); + + Buffer::OwnedImpl buffer; + verifyImplementation(buffer); + buffer.addBufferFragment(*frag); + EXPECT_EQ(expected_length, buffer.length()); + + const uint64_t partial_drain_size = 5; + buffer.drain(partial_drain_size); + EXPECT_EQ(expected_length - partial_drain_size, buffer.length()); + EXPECT_FALSE(release_callback_called_); + + buffer.drain(expected_length - partial_drain_size); + EXPECT_EQ(0, buffer.length()); + EXPECT_TRUE(release_callback_called_); +} + TEST_P(OwnedImplTest, Add) { const std::string string1 = "Hello, ", string2 = "World!"; Buffer::OwnedImpl buffer; diff --git a/test/common/common/utility_test.cc b/test/common/common/utility_test.cc index 1ea15ef2e191..a90272214509 100644 --- a/test/common/common/utility_test.cc +++ b/test/common/common/utility_test.cc @@ -838,4 +838,10 @@ TEST(TrieLookupTable, LongestPrefix) { EXPECT_EQ(nullptr, trie.findLongestPrefix(" ")); } +TEST(InlineStorageTest, InlineString) { + InlineStringPtr hello = InlineString::create("Hello, world!"); + EXPECT_EQ("Hello, world!", hello->toStringView()); + EXPECT_EQ("Hello, world!", hello->toString()); +} + } // namespace Envoy diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index d8c8d3121aaa..95f94a938062 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -2260,6 +2260,7 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamProtocolError) { throw CodecProtocolException("protocol error"); })); + EXPECT_CALL(response_encoder_.stream_, removeCallbacks(_)); EXPECT_CALL(filter_factory_, createFilterChain(_)).Times(0); // A protocol exception should result in reset of the streams followed by a remote or local close @@ -2273,6 +2274,28 @@ TEST_F(HttpConnectionManagerImplTest, DownstreamProtocolError) { conn_manager_->onData(fake_input, false); } +// Verify that FrameFloodException causes connection to be closed abortively. +TEST_F(HttpConnectionManagerImplTest, FrameFloodError) { + InSequence s; + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + conn_manager_->newStream(response_encoder_); + throw FrameFloodException("too many outbound frames."); + })); + + EXPECT_CALL(response_encoder_.stream_, removeCallbacks(_)); + EXPECT_CALL(filter_factory_, createFilterChain(_)).Times(0); + + // FrameFloodException should result in reset of the streams followed by abortive close. + EXPECT_CALL(filter_callbacks_.connection_, + close(Network::ConnectionCloseType::FlushWriteAndDelay)); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); +} + TEST_F(HttpConnectionManagerImplTest, IdleTimeoutNoCodec) { // Not used in the test. delete codec_; @@ -3312,6 +3335,53 @@ TEST_F(HttpConnectionManagerImplTest, FilterHeadReply) { conn_manager_->onData(fake_input, false); } +// Verify that if an encoded stream has been ended, but gets stopped by a filter chain, we end +// up resetting the stream in the doEndStream() path (e.g., via filter reset due to timeout, etc.), +// we emit a reset to the codec. +TEST_F(HttpConnectionManagerImplTest, ResetWithStoppedFilter) { + InSequence s; + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance& data) -> void { + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + HeaderMapPtr headers{ + new TestHeaderMapImpl{{":authority", "host"}, {":path", "/"}, {":method", "GET"}}}; + decoder->decodeHeaders(std::move(headers), true); + data.drain(4); + })); + + setupFilterChain(1, 1); + + EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, true)) + .WillOnce(InvokeWithoutArgs([&]() -> FilterHeadersStatus { + decoder_filters_[0]->callbacks_->sendLocalReply(Code::BadRequest, "Bad request", nullptr, + absl::nullopt, ""); + return FilterHeadersStatus::Continue; + })); + + EXPECT_CALL(*encoder_filters_[0], encodeHeaders(_, false)) + .WillOnce(Invoke([&](HeaderMap& headers, bool) -> FilterHeadersStatus { + EXPECT_EQ("11", headers.ContentLength()->value().getStringView()); + return FilterHeadersStatus::Continue; + })); + EXPECT_CALL(response_encoder_, encodeHeaders(_, false)); + EXPECT_CALL(*encoder_filters_[0], encodeData(_, true)) + .WillOnce(Invoke([&](Buffer::Instance&, bool) -> FilterDataStatus { + return FilterDataStatus::StopIterationAndBuffer; + })); + + EXPECT_CALL(*encoder_filters_[0], encodeComplete()); + EXPECT_CALL(*decoder_filters_[0], decodeComplete()); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); + + EXPECT_CALL(response_encoder_.stream_, resetStream(_)); + expectOnDestroy(); + encoder_filters_[0]->callbacks_->resetStream(); +} + TEST_F(HttpConnectionManagerImplTest, FilterContinueAndEndStreamHeaders) { InSequence s; setup(false, ""); diff --git a/test/common/http/http2/BUILD b/test/common/http/http2/BUILD index 0c245388afb9..354055602126 100644 --- a/test/common/http/http2/BUILD +++ b/test/common/http/http2/BUILD @@ -32,6 +32,7 @@ envoy_cc_test( envoy_cc_test_library( name = "codec_impl_test_util", hdrs = ["codec_impl_test_util.h"], + external_deps = ["abseil_optional"], deps = [ "//source/common/http/http2:codec_lib", ], @@ -56,6 +57,15 @@ envoy_cc_test( ], ) +envoy_cc_test_library( + name = "http2_frame", + srcs = ["http2_frame.cc"], + hdrs = ["http2_frame.h"], + deps = [ + "//source/common/common:macros", + ], +) + envoy_cc_test_library( name = "frame_replay_lib", srcs = ["frame_replay.cc"], diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 441bf5fdd09a..a1153ce49ebd 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -97,6 +97,14 @@ class Http2CodecImplTestFixture { setting.initial_stream_window_size_ = ::testing::get<2>(tp); setting.initial_connection_window_size_ = ::testing::get<3>(tp); setting.allow_metadata_ = allow_metadata_; + setting.stream_error_on_invalid_http_messaging_ = stream_error_on_invalid_http_messaging_; + setting.max_outbound_frames_ = max_outbound_frames_; + setting.max_outbound_control_frames_ = max_outbound_control_frames_; + setting.max_consecutive_inbound_frames_with_empty_payload_ = + max_consecutive_inbound_frames_with_empty_payload_; + setting.max_inbound_priority_frames_per_stream_ = max_inbound_priority_frames_per_stream_; + setting.max_inbound_window_update_frames_per_data_frame_sent_ = + max_inbound_window_update_frames_per_data_frame_sent_; } // corruptMetadataFramePayload assumes data contains at least 10 bytes of the beginning of a @@ -121,6 +129,7 @@ class Http2CodecImplTestFixture { const Http2SettingsTuple client_settings_; const Http2SettingsTuple server_settings_; bool allow_metadata_ = false; + bool stream_error_on_invalid_http_messaging_ = false; Stats::IsolatedStoreImpl stats_store_; Http2Settings client_http2settings_; NiceMock client_connection_; @@ -141,6 +150,14 @@ class Http2CodecImplTestFixture { bool corrupt_metadata_frame_ = false; uint32_t max_request_headers_kb_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; + uint32_t max_outbound_frames_ = Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES; + uint32_t max_outbound_control_frames_ = Http2Settings::DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES; + uint32_t max_consecutive_inbound_frames_with_empty_payload_ = + Http2Settings::DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD; + uint32_t max_inbound_priority_frames_per_stream_ = + Http2Settings::DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM; + uint32_t max_inbound_window_update_frames_per_data_frame_sent_ = + Http2Settings::DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT; }; class Http2CodecImplTest : public ::testing::TestWithParam, @@ -187,6 +204,20 @@ TEST_P(Http2CodecImplTest, ContinueHeaders) { TEST_P(Http2CodecImplTest, InvalidContinueWithFin) { initialize(); + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, true)); + request_encoder_->encodeHeaders(request_headers, true); + + TestHeaderMapImpl continue_headers{{":status", "100"}}; + EXPECT_THROW(response_encoder_->encodeHeaders(continue_headers, true), CodecProtocolException); + EXPECT_EQ(1, stats_store_.counter("http2.rx_messaging_error").value()); +} + +TEST_P(Http2CodecImplTest, InvalidContinueWithFinAllowed) { + stream_error_on_invalid_http_messaging_ = true; + initialize(); + MockStreamCallbacks request_callbacks; request_encoder_->getStream().addCallbacks(request_callbacks); @@ -214,6 +245,23 @@ TEST_P(Http2CodecImplTest, InvalidContinueWithFin) { TEST_P(Http2CodecImplTest, InvalidRepeatContinue) { initialize(); + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, true)); + request_encoder_->encodeHeaders(request_headers, true); + + TestHeaderMapImpl continue_headers{{":status", "100"}}; + EXPECT_CALL(response_decoder_, decode100ContinueHeaders_(_)); + response_encoder_->encode100ContinueHeaders(continue_headers); + + EXPECT_THROW(response_encoder_->encodeHeaders(continue_headers, true), CodecProtocolException); + EXPECT_EQ(1, stats_store_.counter("http2.rx_messaging_error").value()); +}; + +TEST_P(Http2CodecImplTest, InvalidRepeatContinueAllowed) { + stream_error_on_invalid_http_messaging_ = true; + initialize(); + MockStreamCallbacks request_callbacks; request_encoder_->getStream().addCallbacks(request_callbacks); @@ -265,6 +313,28 @@ TEST_P(Http2CodecImplTest, Invalid103) { TEST_P(Http2CodecImplTest, Invalid204WithContentLength) { initialize(); + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, true)); + request_encoder_->encodeHeaders(request_headers, true); + + TestHeaderMapImpl response_headers{{":status", "204"}, {"content-length", "3"}}; + // What follows is a hack to get headers that should span into continuation frames. The default + // maximum frame size is 16K. We will add 3,000 headers that will take us above this size and + // not easily compress with HPACK. (I confirmed this generates 26,468 bytes of header data + // which should contain a continuation.) + for (uint i = 1; i < 3000; i++) { + response_headers.addCopy(std::to_string(i), std::to_string(i)); + } + + EXPECT_THROW(response_encoder_->encodeHeaders(response_headers, false), CodecProtocolException); + EXPECT_EQ(1, stats_store_.counter("http2.rx_messaging_error").value()); +}; + +TEST_P(Http2CodecImplTest, Invalid204WithContentLengthAllowed) { + stream_error_on_invalid_http_messaging_ = true; + initialize(); + MockStreamCallbacks request_callbacks; request_encoder_->getStream().addCallbacks(request_callbacks); @@ -314,7 +384,15 @@ TEST_P(Http2CodecImplTest, RefusedStreamReset) { response_encoder_->getStream().resetStream(StreamResetReason::LocalRefusedStreamReset); } -TEST_P(Http2CodecImplTest, InvalidFrame) { +TEST_P(Http2CodecImplTest, InvalidHeadersFrame) { + initialize(); + + EXPECT_THROW(request_encoder_->encodeHeaders(TestHeaderMapImpl{}, true), CodecProtocolException); + EXPECT_EQ(1, stats_store_.counter("http2.rx_messaging_error").value()); +} + +TEST_P(Http2CodecImplTest, InvalidHeadersFrameAllowed) { + stream_error_on_invalid_http_messaging_ = true; initialize(); MockStreamCallbacks request_callbacks; @@ -1039,6 +1117,212 @@ TEST_P(Http2CodecImplTestAll, TestCodecHeaderCompression) { } } +// Verify that codec detects PING flood +TEST_P(Http2CodecImplTest, PingFlood) { + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + // Send one frame above the outbound control queue size limit + for (uint32_t i = 0; i < Http2Settings::DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES + 1; ++i) { + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + } + + int ack_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &ack_count](Buffer::Instance& frame, bool) { + ++ack_count; + buffer.move(frame); + })); + + EXPECT_THROW(client_->sendPendingFrames(), FrameFloodException); + EXPECT_EQ(ack_count, Http2Settings::DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES); + EXPECT_EQ(1, stats_store_.counter("http2.outbound_control_flood").value()); +} + +// Verify that outbound control frame counter decreases when send buffer is drained +TEST_P(Http2CodecImplTest, PingFloodCounterReset) { + static const int kMaxOutboundControlFrames = 100; + max_outbound_control_frames_ = kMaxOutboundControlFrames; + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + for (int i = 0; i < kMaxOutboundControlFrames; ++i) { + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + } + + int ack_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &ack_count](Buffer::Instance& frame, bool) { + ++ack_count; + buffer.move(frame); + })); + + // We should be 1 frame under the control frame flood mitigation threshold. + EXPECT_NO_THROW(client_->sendPendingFrames()); + EXPECT_EQ(ack_count, kMaxOutboundControlFrames); + + // Drain kMaxOutboundFrames / 2 slices from the send buffer + buffer.drain(buffer.length() / 2); + + // Send kMaxOutboundFrames / 2 more pings. + for (int i = 0; i < kMaxOutboundControlFrames / 2; ++i) { + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + } + // The number of outbound frames should be half of max so the connection should not be terminated. + EXPECT_NO_THROW(client_->sendPendingFrames()); + + // 1 more ping frame should overflow the outbound frame limit. + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + EXPECT_THROW(client_->sendPendingFrames(), FrameFloodException); +} + +// Verify that codec detects flood of outbound HEADER frames +TEST_P(Http2CodecImplTest, ResponseHeadersFlood) { + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + int frame_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) { + ++frame_count; + buffer.move(frame); + })); + + TestHeaderMapImpl response_headers{{":status", "200"}}; + for (uint32_t i = 0; i < Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES + 1; ++i) { + EXPECT_NO_THROW(response_encoder_->encodeHeaders(response_headers, false)); + } + // Presently flood mitigation is done only when processing downstream data + // So we need to send stream from downstream client to trigger mitigation + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + EXPECT_THROW(client_->sendPendingFrames(), FrameFloodException); + + EXPECT_EQ(frame_count, Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES + 1); + EXPECT_EQ(1, stats_store_.counter("http2.outbound_flood").value()); +} + +// Verify that codec detects flood of outbound DATA frames +TEST_P(Http2CodecImplTest, ResponseDataFlood) { + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + int frame_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) { + ++frame_count; + buffer.move(frame); + })); + + TestHeaderMapImpl response_headers{{":status", "200"}}; + response_encoder_->encodeHeaders(response_headers, false); + // Account for the single HEADERS frame above + for (uint32_t i = 0; i < Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES; ++i) { + Buffer::OwnedImpl data("0"); + EXPECT_NO_THROW(response_encoder_->encodeData(data, false)); + } + // Presently flood mitigation is done only when processing downstream data + // So we need to send stream from downstream client to trigger mitigation + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + EXPECT_THROW(client_->sendPendingFrames(), FrameFloodException); + + EXPECT_EQ(frame_count, Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES + 1); + EXPECT_EQ(1, stats_store_.counter("http2.outbound_flood").value()); +} + +// Verify that outbound frame counter decreases when send buffer is drained +TEST_P(Http2CodecImplTest, ResponseDataFloodCounterReset) { + static const int kMaxOutboundFrames = 100; + max_outbound_frames_ = kMaxOutboundFrames; + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + int frame_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) { + ++frame_count; + buffer.move(frame); + })); + + TestHeaderMapImpl response_headers{{":status", "200"}}; + response_encoder_->encodeHeaders(response_headers, false); + // Account for the single HEADERS frame above + for (uint32_t i = 0; i < kMaxOutboundFrames - 1; ++i) { + Buffer::OwnedImpl data("0"); + EXPECT_NO_THROW(response_encoder_->encodeData(data, false)); + } + + EXPECT_EQ(frame_count, kMaxOutboundFrames); + // Drain kMaxOutboundFrames / 2 slices from the send buffer + buffer.drain(buffer.length() / 2); + + for (uint32_t i = 0; i < kMaxOutboundFrames / 2 + 1; ++i) { + Buffer::OwnedImpl data("0"); + EXPECT_NO_THROW(response_encoder_->encodeData(data, false)); + } + + // Presently flood mitigation is done only when processing downstream data + // So we need to send a frame from downstream client to trigger mitigation + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + EXPECT_THROW(client_->sendPendingFrames(), FrameFloodException); +} + +// Verify that control frames are added to the counter of outbound frames of all types. +TEST_P(Http2CodecImplTest, PingStacksWithDataFlood) { + initialize(); + + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + EXPECT_CALL(request_decoder_, decodeHeaders_(_, false)); + request_encoder_->encodeHeaders(request_headers, false); + + int frame_count = 0; + Buffer::OwnedImpl buffer; + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&buffer, &frame_count](Buffer::Instance& frame, bool) { + ++frame_count; + buffer.move(frame); + })); + + TestHeaderMapImpl response_headers{{":status", "200"}}; + response_encoder_->encodeHeaders(response_headers, false); + // Account for the single HEADERS frame above + for (uint32_t i = 0; i < Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES - 1; ++i) { + Buffer::OwnedImpl data("0"); + EXPECT_NO_THROW(response_encoder_->encodeData(data, false)); + } + // Send one PING frame above the outbound queue size limit + EXPECT_EQ(0, nghttp2_submit_ping(client_->session(), NGHTTP2_FLAG_NONE, nullptr)); + EXPECT_THROW(client_->sendPendingFrames(), FrameFloodException); + + EXPECT_EQ(frame_count, Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES); + EXPECT_EQ(1, stats_store_.counter("http2.outbound_flood").value()); +} + } // namespace Http2 } // namespace Http } // namespace Envoy diff --git a/test/common/http/http2/codec_impl_test_util.h b/test/common/http/http2/codec_impl_test_util.h index b642701265c4..2c4652ee0f2f 100644 --- a/test/common/http/http2/codec_impl_test_util.h +++ b/test/common/http/http2/codec_impl_test_util.h @@ -26,6 +26,7 @@ class TestClientConnectionImpl : public ClientConnectionImpl { } nghttp2_session* session() { return session_; } using ClientConnectionImpl::getStream; + using ConnectionImpl::sendPendingFrames; }; } // namespace Http2 diff --git a/test/common/http/http2/http2_frame.cc b/test/common/http/http2/http2_frame.cc new file mode 100644 index 000000000000..368630e1f6ec --- /dev/null +++ b/test/common/http/http2/http2_frame.cc @@ -0,0 +1,221 @@ +#include "test/common/http/http2/http2_frame.h" + +#include + +#include + +namespace { + +// Make request stream ID in the network byte order +uint32_t makeRequestStreamId(uint32_t stream_id) { return htonl((stream_id << 1) | 1); } + +// All this templatized stuff is for the typesafe constexpr bitwise ORing of the "enum class" values +template struct FirstArgType { using type = First; }; + +template constexpr uint8_t orFlags(Flag flag) { return static_cast(flag); } + +template constexpr uint8_t orFlags(Flag first, Flags... rest) { + static_assert(std::is_same::type>::value, + "All flag types must be the same!"); + return static_cast(first) | orFlags(rest...); +} + +} // namespace + +namespace Envoy { +namespace Http { +namespace Http2 { + +const char Http2Frame::Preamble[25] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; + +void Http2Frame::setHeader(absl::string_view header) { + ASSERT(header.size() >= HeaderSize); + data_.assign(HeaderSize, 0); + memcpy(&data_[0], header.data(), HeaderSize); + data_.resize(HeaderSize + payloadSize()); +} + +void Http2Frame::setPayload(absl::string_view payload) { + ASSERT(payload.size() >= payloadSize()); + memcpy(&data_[HeaderSize], payload.data(), payloadSize()); +} + +uint32_t Http2Frame::payloadSize() const { + return (uint32_t(data_[0]) << 16) + (uint32_t(data_[1]) << 8) + uint32_t(data_[2]); +} + +Http2Frame::ResponseStatus Http2Frame::responseStatus() const { + if (empty() || Type::HEADERS != type() || size() <= HeaderSize || + ((data_[HeaderSize] & 0x80) == 0)) { + return ResponseStatus::UNKNOWN; + } + // See https://tools.ietf.org/html/rfc7541#appendix-A for header values + switch (static_cast(data_[HeaderSize] & 0x7f)) { + case StaticHeaderIndex::STATUS_200: + return ResponseStatus::_200; + case StaticHeaderIndex::STATUS_404: + return ResponseStatus::_404; + default: + break; + } + return ResponseStatus::UNKNOWN; +} + +void Http2Frame::buildHeader(Type type, uint32_t payload_size, uint8_t flags, uint32_t stream_id) { + data_.assign(payload_size + HeaderSize, 0); + setPayloadSize(payload_size); + data_[3] = static_cast(type); + data_[4] = flags; + if (stream_id) { + memcpy(&data_[5], &stream_id, sizeof(stream_id)); + } +} + +void Http2Frame::setPayloadSize(uint32_t size) { + data_[0] = (size >> 16) & 0xff; + data_[1] = (size >> 8) & 0xff; + data_[2] = size & 0xff; +} + +void Http2Frame::appendHpackInt(uint64_t value, unsigned char prefix_mask) { + if (value < prefix_mask) { + data_.push_back(value); + } else { + data_.push_back(prefix_mask); + value -= prefix_mask; + + while (value >= 128) { + data_.push_back((value & 0x7f) | 0x80); + value >>= 7; + } + data_.push_back(value); + } +} + +// See https://tools.ietf.org/html/rfc7541#section-6.1 for header representations + +void Http2Frame::appendStaticHeader(StaticHeaderIndex index) { + data_.push_back(0x80 | static_cast(index)); +} + +void Http2Frame::appendHeaderWithoutIndexing(StaticHeaderIndex index, absl::string_view value) { + appendHpackInt(static_cast(index), 0xf); + appendHpackInt(value.size(), 0x7f); + appendData(value); +} + +void Http2Frame::appendEmptyHeader() { + data_.push_back(0x40); + data_.push_back(0x00); + data_.push_back(0x00); +} + +Http2Frame Http2Frame::makePingFrame(absl::string_view data) { + static constexpr size_t kPingPayloadSize = 8; + Http2Frame frame; + frame.buildHeader(Type::PING, kPingPayloadSize); + if (!data.empty()) { + memcpy(&frame.data_[HeaderSize], data.data(), std::min(kPingPayloadSize, data.size())); + } + return frame; +} + +Http2Frame Http2Frame::makeEmptySettingsFrame(SettingsFlags flags) { + Http2Frame frame; + frame.buildHeader(Type::SETTINGS, 0, static_cast(flags)); + return frame; +} + +Http2Frame Http2Frame::makeEmptyHeadersFrame(uint32_t stream_index, HeadersFlags flags) { + Http2Frame frame; + frame.buildHeader(Type::HEADERS, 0, static_cast(flags), + makeRequestStreamId(stream_index)); + return frame; +} + +Http2Frame Http2Frame::makeEmptyContinuationFrame(uint32_t stream_index, HeadersFlags flags) { + Http2Frame frame; + frame.buildHeader(Type::CONTINUATION, 0, static_cast(flags), + makeRequestStreamId(stream_index)); + return frame; +} + +Http2Frame Http2Frame::makeEmptyDataFrame(uint32_t stream_index, DataFlags flags) { + Http2Frame frame; + frame.buildHeader(Type::DATA, 0, static_cast(flags), makeRequestStreamId(stream_index)); + return frame; +} + +Http2Frame Http2Frame::makePriorityFrame(uint32_t stream_index, uint32_t dependent_index) { + static constexpr size_t kPriorityPayloadSize = 5; + Http2Frame frame; + frame.buildHeader(Type::PRIORITY, kPriorityPayloadSize, 0, makeRequestStreamId(stream_index)); + uint32_t dependent_net = makeRequestStreamId(dependent_index); + memcpy(&frame.data_[HeaderSize], reinterpret_cast(&dependent_net), sizeof(uint32_t)); + return frame; +} + +Http2Frame Http2Frame::makeWindowUpdateFrame(uint32_t stream_index, uint32_t increment) { + static constexpr size_t kWindowUpdatePayloadSize = 4; + Http2Frame frame; + frame.buildHeader(Type::WINDOW_UPDATE, kWindowUpdatePayloadSize, 0, + makeRequestStreamId(stream_index)); + uint32_t increment_net = htonl(increment); + memcpy(&frame.data_[HeaderSize], reinterpret_cast(&increment_net), sizeof(uint32_t)); + return frame; +} + +Http2Frame Http2Frame::makeMalformedRequest(uint32_t stream_index) { + Http2Frame frame; + frame.buildHeader(Type::HEADERS, 0, orFlags(HeadersFlags::END_STREAM, HeadersFlags::END_HEADERS), + makeRequestStreamId(stream_index)); + frame.appendStaticHeader( + StaticHeaderIndex::STATUS_200); // send :status as request header, which is invalid + frame.adjustPayloadSize(); + return frame; +} + +Http2Frame Http2Frame::makeMalformedRequestWithZerolenHeader(uint32_t stream_index, + absl::string_view host, + absl::string_view path) { + Http2Frame frame; + frame.buildHeader(Type::HEADERS, 0, orFlags(HeadersFlags::END_STREAM, HeadersFlags::END_HEADERS), + makeRequestStreamId(stream_index)); + frame.appendStaticHeader(StaticHeaderIndex::METHOD_GET); + frame.appendStaticHeader(StaticHeaderIndex::SCHEME_HTTPS); + frame.appendHeaderWithoutIndexing(StaticHeaderIndex::PATH, path); + frame.appendHeaderWithoutIndexing(StaticHeaderIndex::HOST, host); + frame.appendEmptyHeader(); + frame.adjustPayloadSize(); + return frame; +} + +Http2Frame Http2Frame::makeRequest(uint32_t stream_index, absl::string_view host, + absl::string_view path) { + Http2Frame frame; + frame.buildHeader(Type::HEADERS, 0, orFlags(HeadersFlags::END_STREAM, HeadersFlags::END_HEADERS), + makeRequestStreamId(stream_index)); + frame.appendStaticHeader(StaticHeaderIndex::METHOD_GET); + frame.appendStaticHeader(StaticHeaderIndex::SCHEME_HTTPS); + frame.appendHeaderWithoutIndexing(StaticHeaderIndex::PATH, path); + frame.appendHeaderWithoutIndexing(StaticHeaderIndex::HOST, host); + frame.adjustPayloadSize(); + return frame; +} + +Http2Frame Http2Frame::makePostRequest(uint32_t stream_index, absl::string_view host, + absl::string_view path) { + Http2Frame frame; + frame.buildHeader(Type::HEADERS, 0, orFlags(HeadersFlags::END_HEADERS), + makeRequestStreamId(stream_index)); + frame.appendStaticHeader(StaticHeaderIndex::METHOD_POST); + frame.appendStaticHeader(StaticHeaderIndex::SCHEME_HTTPS); + frame.appendHeaderWithoutIndexing(StaticHeaderIndex::PATH, path); + frame.appendHeaderWithoutIndexing(StaticHeaderIndex::HOST, host); + frame.adjustPayloadSize(); + return frame; +} + +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/http2/http2_frame.h b/test/common/http/http2/http2_frame.h new file mode 100644 index 000000000000..52b838dbb987 --- /dev/null +++ b/test/common/http/http2/http2_frame.h @@ -0,0 +1,145 @@ +#pragma once + +#include +#include +#include + +#include "common/common/assert.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Http { +namespace Http2 { + +// Rudimentary facility for building and parsing of HTTP2 frames for unit tests +class Http2Frame { + using DataContainer = std::vector; + +public: + Http2Frame() = default; + + using iterator = DataContainer::iterator; + using const_iterator = DataContainer::const_iterator; + + static constexpr size_t HeaderSize = 9; + static const char Preamble[25]; + + enum class Type : uint8_t { + DATA = 0, + HEADERS, + PRIORITY, + RST_STREAM, + SETTINGS, + PUSH_PROMISE, + PING, + GOAWAY, + WINDOW_UPDATE, + CONTINUATION + }; + + enum class SettingsFlags : uint8_t { + NONE = 0, + ACK = 1, + }; + + enum class HeadersFlags : uint8_t { + NONE = 0, + END_STREAM = 1, + END_HEADERS = 4, + }; + + enum class DataFlags : uint8_t { + NONE = 0, + END_STREAM = 1, + }; + + // See https://tools.ietf.org/html/rfc7541#appendix-A for static header indexes + enum class StaticHeaderIndex : uint8_t { + UNKNOWN, + METHOD_GET = 2, + METHOD_POST = 3, + PATH = 4, + STATUS_200 = 8, + STATUS_404 = 13, + SCHEME_HTTPS = 7, + HOST = 38, + }; + + enum class ResponseStatus { UNKNOWN, _200, _404 }; + + // Methods for creating HTTP2 frames + static Http2Frame makePingFrame(absl::string_view data = nullptr); + static Http2Frame makeEmptySettingsFrame(SettingsFlags flags = SettingsFlags::NONE); + static Http2Frame makeEmptyHeadersFrame(uint32_t stream_index, + HeadersFlags flags = HeadersFlags::NONE); + static Http2Frame makeEmptyContinuationFrame(uint32_t stream_index, + HeadersFlags flags = HeadersFlags::NONE); + static Http2Frame makeEmptyDataFrame(uint32_t stream_index, DataFlags flags = DataFlags::NONE); + static Http2Frame makePriorityFrame(uint32_t stream_index, uint32_t dependent_index); + static Http2Frame makeWindowUpdateFrame(uint32_t stream_index, uint32_t increment); + static Http2Frame makeMalformedRequest(uint32_t stream_index); + static Http2Frame makeMalformedRequestWithZerolenHeader(uint32_t stream_index, + absl::string_view host, + absl::string_view path); + static Http2Frame makeRequest(uint32_t stream_index, absl::string_view host, + absl::string_view path); + static Http2Frame makePostRequest(uint32_t stream_index, absl::string_view host, + absl::string_view path); + + Type type() const { return static_cast(data_[3]); } + ResponseStatus responseStatus() const; + + // Copy HTTP2 header. The `header` parameter must at least be HeaderSize long. + // Allocates payload size based on the value in the header. + void setHeader(absl::string_view header); + + // Copy payloadSize() bytes from the `payload`. The `payload` must be at least payloadSize() long. + void setPayload(absl::string_view payload); + + // Convert to `std::string` for convenience. + explicit operator std::string() const { + if (data_.empty()) { + return {}; + } + return std::string(reinterpret_cast(data()), size()); + } + + uint32_t payloadSize() const; + // Total size of the frame + size_t size() const { return data_.size(); } + // Access to the raw frame bytes + const uint8_t* data() const { return data_.data(); } + iterator begin() { return data_.begin(); } + iterator end() { return data_.end(); } + const_iterator begin() const { return data_.begin(); } + const_iterator end() const { return data_.end(); } + bool empty() const { return data_.empty(); } + +private: + void buildHeader(Type type, uint32_t payload_size = 0, uint8_t flags = 0, uint32_t stream_id = 0); + void setPayloadSize(uint32_t size); + + // This method appends HPACK encoded uint64_t to the payload. adjustPayloadSize() must be called + // after calling this method (possibly multiple times) to write new payload length to the HTTP2 + // header. + void appendHpackInt(uint64_t value, unsigned char prefix_mask); + void appendData(absl::string_view data) { data_.insert(data_.end(), data.begin(), data.end()); } + + // Headers are directly encoded + void appendStaticHeader(StaticHeaderIndex index); + void appendHeaderWithoutIndexing(StaticHeaderIndex index, absl::string_view value); + void appendEmptyHeader(); + + // This method updates payload length in the HTTP2 header based on the size of the data_ + void adjustPayloadSize() { + ASSERT(size() >= HeaderSize); + setPayloadSize(size() - HeaderSize); + } + + DataContainer data_; +}; + +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index e2e354c1da9a..f339f3de185e 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -264,6 +264,15 @@ TEST(HttpUtility, parseHttp2Settings) { http2_settings.initial_stream_window_size_); EXPECT_EQ(Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE, http2_settings.initial_connection_window_size_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES, http2_settings.max_outbound_frames_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES, + http2_settings.max_outbound_control_frames_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD, + http2_settings.max_consecutive_inbound_frames_with_empty_payload_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM, + http2_settings.max_inbound_priority_frames_per_stream_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT, + http2_settings.max_inbound_window_update_frames_per_data_frame_sent_); } { diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 8ddfb9f31535..d93604d12c9e 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -1734,6 +1734,7 @@ TEST_F(PostCloseConnectionImplTest, ReadAfterCloseFlushWriteDelayIgnored) { // Delayed connection close. EXPECT_CALL(dispatcher_, createTimer_(_)); + EXPECT_CALL(*file_event_, setEnabled(Event::FileReadyType::Closed)); connection_->close(ConnectionCloseType::FlushWriteAndDelay); // Read event, doRead() happens on connection but no filter onData(). @@ -1758,6 +1759,10 @@ TEST_F(PostCloseConnectionImplTest, ReadAfterCloseFlushWriteDelayIgnoredWithWrit // Delayed connection close. EXPECT_CALL(dispatcher_, createTimer_(_)); + // With half-close semantics enabled we will not wait for early close notification. + // See the `Envoy::Network::ConnectionImpl::readDisable()' method for more details. + EXPECT_CALL(*file_event_, setEnabled(0)); + connection_->enableHalfClose(true); connection_->close(ConnectionCloseType::FlushWriteAndDelay); // Read event, doRead() happens on connection but no filter onData(). diff --git a/test/config/utility.cc b/test/config/utility.cc index a27970824b5b..7ba995c00c63 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -598,6 +598,21 @@ void ConfigHelper::addConfigModifier(HttpModifierFunction function) { }); } +void ConfigHelper::setOutboundFramesLimits(uint32_t max_all_frames, uint32_t max_control_frames) { + auto filter = getFilterFromListener("envoy.http_connection_manager"); + if (filter) { + envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager hcm_config; + loadHttpConnectionManager(hcm_config); + if (hcm_config.codec_type() == + envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::HTTP2) { + auto* options = hcm_config.mutable_http2_protocol_options(); + options->mutable_max_outbound_frames()->set_value(max_all_frames); + options->mutable_max_outbound_control_frames()->set_value(max_control_frames); + storeHttpConnectionManager(hcm_config); + } + } +} + EdsHelper::EdsHelper() : eds_path_(TestEnvironment::writeStringToFileForTest("eds.pb_text", "")) { // cluster.cluster_0.update_success will be incremented on the initial // load when Envoy comes up. diff --git a/test/config/utility.h b/test/config/utility.h index e93d5d579f9c..24f12841c6e4 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -143,6 +143,9 @@ class ConfigHelper { // Modifiers will be applied just before ports are modified in finalize void addConfigModifier(HttpModifierFunction function); + // Set limits on pending outbound frames. + void setOutboundFramesLimits(uint32_t max_all_frames, uint32_t max_control_frames); + // Return the bootstrap configuration for hand-off to Envoy. const envoy::config::bootstrap::v2::Bootstrap& bootstrap() { return bootstrap_; } diff --git a/test/integration/BUILD b/test/integration/BUILD index 5cb9cdf1c721..7c78b1292d5d 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -211,6 +211,7 @@ envoy_cc_test( "//source/extensions/filters/http/buffer:config", "//source/extensions/filters/http/dynamo:config", "//source/extensions/filters/http/health_check:config", + "//test/common/http/http2:http2_frame", "//test/mocks/http:http_mocks", "//test/mocks/upstream:upstream_mocks", "//test/test_common:utility_lib", diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 44da6c4eab02..148338ce9d06 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -1,5 +1,6 @@ #include "test/integration/http2_integration_test.h" +#include #include #include "common/buffer/buffer_impl.h" @@ -430,6 +431,23 @@ TEST_P(Http2IntegrationTest, GrpcRouterNotFound) { TEST_P(Http2IntegrationTest, GrpcRetry) { testGrpcRetry(); } +// Verify the case where there is an HTTP/2 codec/protocol error with an active stream. +TEST_P(Http2IntegrationTest, CodecErrorAfterStreamStart) { + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + // Sends a request. + auto response = codec_client_->makeRequestWithBody(default_request_headers_, 10); + waitForNextUpstreamRequest(); + + // Send bogus raw data on the connection. + Buffer::OwnedImpl bogus_data("some really bogus data"); + codec_client_->rawConnection().write(bogus_data, false); + + // Verifies reset is received. + response->waitForReset(); +} + TEST_P(Http2IntegrationTest, BadMagic) { initialize(); Buffer::OwnedImpl buffer("hello"); @@ -1028,4 +1046,362 @@ TEST_P(Http2RingHashIntegrationTest, CookieRoutingWithCookieWithTtlSet) { EXPECT_EQ(served_by.size(), 1); } +namespace { +const int64_t TransmitThreshold = 100 * 1024 * 1024; +} // namespace + +void Http2FloodMitigationTest::setNetworkConnectionBufferSize() { + // nghttp2 library has its own internal mitigation for outbound control frames. The mitigation is + // trigerred when there are more than 10000 PING or SETTINGS frames with ACK flag in the nghttp2 + // internal outbound queue. It is possible to trigger this mitigation in nghttp2 before triggering + // Envoy's own flood mitigation. This can happen when a buffer larger enough to contain over 10K + // PING or SETTINGS frames is dispatched to the nghttp2 library. To prevent this from happening + // the network connection receive buffer needs to be smaller than 90Kb (which is 10K SETTINGS + // frames). Set it to the arbitrarily chosen value of 32K. + config_helper_.addConfigModifier([](envoy::config::bootstrap::v2::Bootstrap& bootstrap) -> void { + RELEASE_ASSERT(bootstrap.mutable_static_resources()->listeners_size() >= 1, ""); + auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); + listener->mutable_per_connection_buffer_limit_bytes()->set_value(32 * 1024); + }); +} + +void Http2FloodMitigationTest::beginSession() { + setDownstreamProtocol(Http::CodecClient::Type::HTTP2); + setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + // set lower outbound frame limits to make tests run faster + config_helper_.setOutboundFramesLimits(1000, 100); + initialize(); + tcp_client_ = makeTcpConnection(lookupPort("http")); + startHttp2Session(); +} + +Http2Frame Http2FloodMitigationTest::readFrame() { + Http2Frame frame; + tcp_client_->waitForData(frame.HeaderSize); + frame.setHeader(tcp_client_->data()); + tcp_client_->clearData(frame.HeaderSize); + auto len = frame.payloadSize(); + if (len) { + tcp_client_->waitForData(len); + frame.setPayload(tcp_client_->data()); + tcp_client_->clearData(len); + } + return frame; +} + +void Http2FloodMitigationTest::sendFame(const Http2Frame& frame) { + ASSERT_TRUE(tcp_client_->connected()); + tcp_client_->write(std::string(frame), false, false); +} + +void Http2FloodMitigationTest::startHttp2Session() { + tcp_client_->write(Http2Frame::Preamble, false, false); + + // Send empty initial SETTINGS frame. + auto settings = Http2Frame::makeEmptySettingsFrame(); + tcp_client_->write(std::string(settings), false, false); + + // Read initial SETTINGS frame from the server. + readFrame(); + + // Send an SETTINGS ACK. + settings = Http2Frame::makeEmptySettingsFrame(Http2Frame::SettingsFlags::ACK); + tcp_client_->write(std::string(settings), false, false); + + // read pending SETTINGS and WINDOW_UPDATE frames + readFrame(); + readFrame(); +} + +// Verify that the server detects the flood of the given frame. +void Http2FloodMitigationTest::floodServer(const Http2Frame& frame, const std::string& flood_stat) { + // pack the as many frames as we can into 16k buffer + const int FrameCount = (16 * 1024) / frame.size(); + std::vector buf(FrameCount * frame.size()); + for (auto pos = buf.begin(); pos != buf.end();) { + pos = std::copy(frame.begin(), frame.end(), pos); + } + + tcp_client_->readDisable(true); + int64_t total_bytes_sent = 0; + // If the flood protection is not working this loop will keep going + // forever until it is killed by blaze timer or run out of memory. + // Add early stop if we have sent more than 100M of frames, as it this + // point it is obvious something is wrong. + while (total_bytes_sent < TransmitThreshold && tcp_client_->connected()) { + tcp_client_->write({buf.begin(), buf.end()}, false, false); + total_bytes_sent += buf.size(); + } + + EXPECT_LE(total_bytes_sent, TransmitThreshold) << "Flood mitigation is broken."; + EXPECT_EQ(1, test_server_->counter(flood_stat)->value()); + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +// Verify that the server detects the flood using specified request parameters. +void Http2FloodMitigationTest::floodServer(absl::string_view host, absl::string_view path, + Http2Frame::ResponseStatus expected_http_status, + const std::string& flood_stat) { + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(request_idx, host, path); + sendFame(request); + auto frame = readFrame(); + EXPECT_EQ(Http2Frame::Type::HEADERS, frame.type()); + EXPECT_EQ(expected_http_status, frame.responseStatus()); + tcp_client_->readDisable(true); + uint64_t total_bytes_sent = 0; + while (total_bytes_sent < TransmitThreshold && tcp_client_->connected()) { + request = Http2Frame::makeRequest(++request_idx, host, path); + sendFame(request); + total_bytes_sent += request.size(); + } + EXPECT_LE(total_bytes_sent, TransmitThreshold) << "Flood mitigation is broken."; + if (!flood_stat.empty()) { + EXPECT_EQ(1, test_server_->counter(flood_stat)->value()); + } + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +INSTANTIATE_TEST_SUITE_P(IpVersions, Http2FloodMitigationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +TEST_P(Http2FloodMitigationTest, Ping) { + setNetworkConnectionBufferSize(); + beginSession(); + floodServer(Http2Frame::makePingFrame(), "http2.outbound_control_flood"); +} + +TEST_P(Http2FloodMitigationTest, Settings) { + setNetworkConnectionBufferSize(); + beginSession(); + floodServer(Http2Frame::makeEmptySettingsFrame(), "http2.outbound_control_flood"); +} + +// Verify that the server can detect flood of internally generated 404 responses. +TEST_P(Http2FloodMitigationTest, 404) { + // Change the default route to be restrictive, and send a request to a non existent route. + config_helper_.setDefaultHostAndRoute("foo.com", "/found"); + beginSession(); + + // Send requests to a non existent path to generate 404s + floodServer("host", "/notfound", Http2Frame::ResponseStatus::_404, "http2.outbound_flood"); +} + +// Verify that the server can detect flood of DATA frames +TEST_P(Http2FloodMitigationTest, Data) { + // Set large buffer limits so the test is not affected by the flow control. + config_helper_.setBufferLimits(1024 * 1024 * 1024, 1024 * 1024 * 1024); + autonomous_upstream_ = true; + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + floodServer("host", "/test/long/url", Http2Frame::ResponseStatus::_200, "http2.outbound_flood"); +} + +// Verify that the server can detect flood of RST_STREAM frames. +TEST_P(Http2FloodMitigationTest, RST_STREAM) { + // Use invalid HTTP headers to trigger sending RST_STREAM frames. + config_helper_.addConfigModifier( + [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + }); + beginSession(); + + int i = 0; + auto request = Http::Http2::Http2Frame::makeMalformedRequest(i); + sendFame(request); + auto response = readFrame(); + // Make sure we've got RST_STREAM from the server + EXPECT_EQ(Http2Frame::Type::RST_STREAM, response.type()); + uint64_t total_bytes_sent = 0; + while (total_bytes_sent < TransmitThreshold && tcp_client_->connected()) { + request = Http::Http2::Http2Frame::makeMalformedRequest(++i); + sendFame(request); + total_bytes_sent += request.size(); + } + EXPECT_LE(total_bytes_sent, TransmitThreshold) << "Flood mitigation is broken."; + EXPECT_EQ(1, test_server_->counter("http2.outbound_control_flood")->value()); + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +// Verify that the server stop reading downstream connection on protocol error. +TEST_P(Http2FloodMitigationTest, TooManyStreams) { + config_helper_.addConfigModifier( + [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_http2_protocol_options()->mutable_max_concurrent_streams()->set_value(2); + }); + autonomous_upstream_ = true; + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + // Exceed the number of streams allowed by the server. The server should stop reading from the + // client. Verify that the client was unable to stuff a lot of data into the server. + floodServer("host", "/test/long/url", Http2Frame::ResponseStatus::_200, ""); +} + +TEST_P(Http2FloodMitigationTest, EmptyHeaders) { + config_helper_.addConfigModifier( + [&](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_http2_protocol_options() + ->mutable_max_consecutive_inbound_frames_with_empty_payload() + ->set_value(0); + }); + beginSession(); + + uint32_t request_idx = 0; + auto request = Http2Frame::makeEmptyHeadersFrame(request_idx); + sendFame(request); + + tcp_client_->waitForDisconnect(); + + EXPECT_EQ(1, test_server_->counter("http2.inbound_empty_frames_flood")->value()); + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +TEST_P(Http2FloodMitigationTest, EmptyHeadersContinuation) { + beginSession(); + + uint32_t request_idx = 0; + auto request = Http2Frame::makeEmptyHeadersFrame(request_idx); + sendFame(request); + + for (int i = 0; i < 2; i++) { + request = Http2Frame::makeEmptyContinuationFrame(request_idx); + sendFame(request); + } + + tcp_client_->waitForDisconnect(); + + EXPECT_EQ(1, test_server_->counter("http2.inbound_empty_frames_flood")->value()); + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +TEST_P(Http2FloodMitigationTest, EmptyData) { + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + uint32_t request_idx = 0; + auto request = Http2Frame::makePostRequest(request_idx, "host", "/"); + sendFame(request); + + for (int i = 0; i < 2; i++) { + request = Http2Frame::makeEmptyDataFrame(request_idx); + sendFame(request); + } + + tcp_client_->waitForDisconnect(); + + EXPECT_EQ(1, test_server_->counter("http2.inbound_empty_frames_flood")->value()); + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +TEST_P(Http2FloodMitigationTest, PriorityIdleStream) { + beginSession(); + + floodServer(Http2Frame::makePriorityFrame(0, 1), "http2.inbound_priority_frames_flood"); +} + +TEST_P(Http2FloodMitigationTest, PriorityOpenStream) { + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + // Open stream. + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(request_idx, "host", "/"); + sendFame(request); + + floodServer(Http2Frame::makePriorityFrame(request_idx, request_idx + 1), + "http2.inbound_priority_frames_flood"); +} + +TEST_P(Http2FloodMitigationTest, PriorityClosedStream) { + autonomous_upstream_ = true; + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + // Open stream. + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(request_idx, "host", "/"); + sendFame(request); + // Reading response marks this stream as closed in nghttp2. + auto frame = readFrame(); + EXPECT_EQ(Http2Frame::Type::HEADERS, frame.type()); + + floodServer(Http2Frame::makePriorityFrame(request_idx, request_idx + 1), + "http2.inbound_priority_frames_flood"); +} + +TEST_P(Http2FloodMitigationTest, WindowUpdate) { + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + // Open stream. + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(request_idx, "host", "/"); + sendFame(request); + + floodServer(Http2Frame::makeWindowUpdateFrame(request_idx, 1), + "http2.inbound_window_update_frames_flood"); +} + +// Verify that the HTTP/2 connection is terminated upon receiving invalid HEADERS frame. +TEST_P(Http2FloodMitigationTest, ZerolenHeader) { + beginSession(); + + // Send invalid request. + uint32_t request_idx = 0; + auto request = Http2Frame::makeMalformedRequestWithZerolenHeader(request_idx, "host", "/"); + sendFame(request); + + tcp_client_->waitForDisconnect(); + + EXPECT_EQ(1, test_server_->counter("http2.rx_messaging_error")->value()); + EXPECT_EQ(1, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + +// Verify that only the offending stream is terminated upon receiving invalid HEADERS frame. +TEST_P(Http2FloodMitigationTest, ZerolenHeaderAllowed) { + config_helper_.addConfigModifier( + [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + }); + autonomous_upstream_ = true; + beginSession(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + // Send invalid request. + uint32_t request_idx = 0; + auto request = Http2Frame::makeMalformedRequestWithZerolenHeader(request_idx, "host", "/"); + sendFame(request); + // Make sure we've got RST_STREAM from the server. + auto response = readFrame(); + EXPECT_EQ(Http2Frame::Type::RST_STREAM, response.type()); + + // Send valid request using the same connection. + request_idx++; + request = Http2Frame::makeRequest(request_idx, "host", "/"); + sendFame(request); + response = readFrame(); + EXPECT_EQ(Http2Frame::Type::HEADERS, response.type()); + EXPECT_EQ(Http2Frame::ResponseStatus::_200, response.responseStatus()); + + tcp_client_->close(); + + EXPECT_EQ(1, test_server_->counter("http2.rx_messaging_error")->value()); + EXPECT_EQ(0, + test_server_->counter("http.config_test.downstream_cx_delayed_close_timeout")->value()); +} + } // namespace Envoy diff --git a/test/integration/http2_integration_test.h b/test/integration/http2_integration_test.h index c910d6e78cbd..6282ee13c4c8 100644 --- a/test/integration/http2_integration_test.h +++ b/test/integration/http2_integration_test.h @@ -1,9 +1,12 @@ #pragma once +#include "test/common/http/http2/http2_frame.h" #include "test/integration/http_integration.h" #include "gtest/gtest.h" +using Envoy::Http::Http2::Http2Frame; + namespace Envoy { class Http2IntegrationTest : public testing::TestWithParam, public HttpIntegrationTest { @@ -46,4 +49,22 @@ class Http2MetadataIntegrationTest : public Http2IntegrationTest { setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); } }; + +class Http2FloodMitigationTest : public testing::TestWithParam, + public HttpIntegrationTest { +public: + Http2FloodMitigationTest() : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, GetParam()) {} + +protected: + void startHttp2Session(); + void floodServer(const Http2Frame& frame, const std::string& flood_stat); + void floodServer(absl::string_view host, absl::string_view path, + Http2Frame::ResponseStatus expected_http_status, const std::string& flood_stat); + Http2Frame readFrame(); + void sendFame(const Http2Frame& frame); + void setNetworkConnectionBufferSize(); + void beginSession(); + + IntegrationTcpClientPtr tcp_client_; +}; } // namespace Envoy diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index c914831cb6b6..00beabdd0114 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -39,6 +39,7 @@ class IntegrationCodecClient : public Http::CodecClientProd { bool waitForDisconnect(std::chrono::milliseconds time_to_wait = std::chrono::milliseconds(0)); Network::ClientConnection* connection() const { return connection_.get(); } Network::ConnectionEvent last_connection_event() const { return last_connection_event_; } + Network::Connection& rawConnection() { return *connection_; } private: struct ConnectionCallbacks : public Network::ConnectionCallbacks { diff --git a/test/integration/integration.cc b/test/integration/integration.cc index ed2e1b956ac9..d228f183a541 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -180,6 +180,15 @@ void IntegrationTcpClient::waitForData(const std::string& data, bool exact_match connection_->dispatcher().run(Event::Dispatcher::RunType::Block); } +void IntegrationTcpClient::waitForData(size_t length) { + if (payload_reader_->data().size() >= length) { + return; + } + + payload_reader_->setLengthToWaitFor(length); + connection_->dispatcher().run(Event::Dispatcher::RunType::Block); +} + void IntegrationTcpClient::waitForDisconnect(bool ignore_spurious_events) { if (ignore_spurious_events) { while (!disconnected_) { diff --git a/test/integration/integration.h b/test/integration/integration.h index 3f300c4ab758..2004cc530a45 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -91,12 +91,16 @@ class IntegrationTcpClient { void close(); void waitForData(const std::string& data, bool exact_match = true); + // wait for at least `length` bytes to be received + void waitForData(size_t length); void waitForDisconnect(bool ignore_spurious_events = false); void waitForHalfClose(); void readDisable(bool disabled); void write(const std::string& data, bool end_stream = false, bool verify = true); const std::string& data() { return payload_reader_->data(); } bool connected() const { return !disconnected_; } + // clear up to the `count` number of bytes of received data + void clearData(size_t count = std::string::npos) { payload_reader_->clearData(count); } private: struct ConnectionCallbacks : public Network::ConnectionCallbacks { diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 41b7451acea1..2f7b1b9ebaab 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -592,6 +592,36 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLength) { {"content-length", "-1"}}); auto response = std::move(encoder_decoder.second); + codec_client_->waitForDisconnect(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + ASSERT_TRUE(response->complete()); + EXPECT_EQ("400", response->headers().Status()->value().getStringView()); + } else { + ASSERT_TRUE(response->reset()); + EXPECT_EQ(Http::StreamResetReason::ConnectionTermination, response->reset_reason()); + } +} + +// TODO(PiotrSikora): move this HTTP/2 only variant to http2_integration_test.cc. +TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLengthAllowed) { + config_helper_.addConfigModifier( + [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + }); + + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":authority", "host"}, + {"content-length", "-1"}}); + auto response = std::move(encoder_decoder.second); + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { codec_client_->waitForDisconnect(); } else { @@ -618,6 +648,34 @@ TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengths) { {"content-length", "3,2"}}); auto response = std::move(encoder_decoder.second); + codec_client_->waitForDisconnect(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + ASSERT_TRUE(response->complete()); + EXPECT_EQ("400", response->headers().Status()->value().getStringView()); + } else { + ASSERT_TRUE(response->reset()); + EXPECT_EQ(Http::StreamResetReason::ConnectionTermination, response->reset_reason()); + } +} + +// TODO(PiotrSikora): move this HTTP/2 only variant to http2_integration_test.cc. +TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengthsAllowed) { + config_helper_.addConfigModifier( + [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) + -> void { + hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + }); + + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":authority", "host"}, + {"content-length", "3,2"}}); + auto response = std::move(encoder_decoder.second); + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { codec_client_->waitForDisconnect(); } else { diff --git a/test/integration/tcp_proxy_integration_test.cc b/test/integration/tcp_proxy_integration_test.cc index f695a5033036..b481a569349f 100644 --- a/test/integration/tcp_proxy_integration_test.cc +++ b/test/integration/tcp_proxy_integration_test.cc @@ -43,6 +43,15 @@ TEST_P(TcpProxyIntegrationTest, TcpProxyUpstreamWritesFirst) { // Make sure inexact matches work also on data already received. tcp_client->waitForData("ello", false); + // Make sure length based wait works for the data already received + tcp_client->waitForData(5); + tcp_client->waitForData(4); + + // Drain part of the received message + tcp_client->clearData(2); + tcp_client->waitForData("llo"); + tcp_client->waitForData(3); + tcp_client->write("hello"); ASSERT_TRUE(fake_upstream_connection->waitForData(5)); diff --git a/test/integration/utility.cc b/test/integration/utility.cc index a5cbc2c6c83e..872638b115a8 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -146,6 +146,11 @@ Network::FilterStatus WaitForPayloadReader::onData(Buffer::Instance& data, bool dispatcher_.exit(); } + if (wait_for_length_ && data_.size() >= length_to_wait_for_) { + wait_for_length_ = false; + dispatcher_.exit(); + } + return Network::FilterStatus::StopIteration; } diff --git a/test/integration/utility.h b/test/integration/utility.h index e39a295f682f..099e6fc04986 100644 --- a/test/integration/utility.h +++ b/test/integration/utility.h @@ -181,8 +181,14 @@ class WaitForPayloadReader : public Network::ReadFilterBaseImpl { data_to_wait_for_ = data; exact_match_ = exact_match; } + void setLengthToWaitFor(size_t length) { + ASSERT(!wait_for_length_); + length_to_wait_for_ = length; + wait_for_length_ = true; + } const std::string& data() { return data_; } bool readLastByte() { return read_end_stream_; } + void clearData(size_t count = std::string::npos) { data_.erase(0, count); } private: Event::Dispatcher& dispatcher_; @@ -190,6 +196,8 @@ class WaitForPayloadReader : public Network::ReadFilterBaseImpl { std::string data_; bool exact_match_{true}; bool read_end_stream_{}; + size_t length_to_wait_for_{0}; + bool wait_for_length_{false}; }; } // namespace Envoy diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index e1fa7b104451..2e12367559a4 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -336,6 +336,11 @@ const uint32_t Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS; const uint32_t Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE; const uint32_t Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE; const uint32_t Http2Settings::MIN_INITIAL_STREAM_WINDOW_SIZE; +const uint32_t Http2Settings::DEFAULT_MAX_OUTBOUND_FRAMES; +const uint32_t Http2Settings::DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES; +const uint32_t Http2Settings::DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD; +const uint32_t Http2Settings::DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM; +const uint32_t Http2Settings::DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT; TestHeaderMapImpl::TestHeaderMapImpl() : HeaderMapImpl() {} diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index bf29fb00b196..a7e2f5cefb52 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -297,6 +297,7 @@ accessors acls addr agg +alignas alignof alloc alloca