Merge pull request #353 from istio-private/envoy-cve-patches-2022-06-…

…09-rel14

docs/root/version_history/current.rst

@@ -15,7 +15,6 @@ Bug Fixes
 ---------
 *Changes expected to improve the state of the world and are unlikely to have negative effects*
 
-
 Removed Config or Runtime
 -------------------------
 *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/common/http/conn_manager_impl.cc

@@ -1729,6 +1729,12 @@ void ConnectionManagerImpl::ActiveStream::recreateStream(
             filter_state->parent(), StreamInfo::FilterState::LifeSpan::FilterChain);
   }
 
+  // Make sure that relevant information makes it from the original stream info
+  // to the new one. Generally this should consist of all downstream related
+  // data, and not include upstream related data.
+  (*connection_manager_.streams_.begin())
+      ->filter_manager_.streamInfo()
+      .setFromForRecreateStream(filter_manager_.streamInfo());
   new_stream.decodeHeaders(std::move(request_headers_), !proxy_body);
   if (proxy_body) {
     // This functionality is currently only used for internal redirects, which the router only

source/common/http/filter_manager.cc

@@ -327,7 +327,7 @@ Buffer::InstancePtr& ActiveStreamDecoderFilter::bufferedData() {
   return parent_.buffered_request_data_;
 }
 
-bool ActiveStreamDecoderFilter::complete() { return parent_.state_.remote_decode_complete_; }
+bool ActiveStreamDecoderFilter::complete() { return parent_.remoteDecodeComplete(); }
 
 void ActiveStreamDecoderFilter::doHeaders(bool end_stream) {
   parent_.decodeHeaders(this, *parent_.filter_manager_callbacks_.requestHeaders(), end_stream);
@@ -843,9 +843,8 @@ void FilterManager::decodeMetadata(ActiveStreamDecoderFilter* filter, MetadataMa
 }
 
 void FilterManager::maybeEndDecode(bool end_stream) {
-  ASSERT(!state_.remote_decode_complete_);
-  state_.remote_decode_complete_ = end_stream;
-  if (end_stream) {
+  // If recreateStream is called, the HCM rewinds state and may send more encodeData calls.
+  if (end_stream && !remoteDecodeComplete()) {
     stream_info_.downstreamTiming().onLastDownstreamRxByteReceived(dispatcher().timeSource());
     ENVOY_STREAM_LOG(debug, "request end stream", *this);
   }

source/common/http/filter_manager.h

@@ -907,7 +907,10 @@ class FilterManager : public ScopeTrackedObject,
   /**
    * Whether remote processing has been marked as complete.
    */
-  bool remoteDecodeComplete() const { return state_.remote_decode_complete_; }
+  bool remoteDecodeComplete() const {
+    return stream_info_.downstreamTiming() &&
+           stream_info_.downstreamTiming()->lastDownstreamRxByteReceived().has_value();
+  }
 
   /**
    * Instructs the FilterManager to not create a filter chain. This makes it possible to issue
@@ -1058,15 +1061,14 @@ class FilterManager : public ScopeTrackedObject,
 
   struct State {
     State()
-        : remote_encode_complete_(false), remote_decode_complete_(false), local_complete_(false),
-          has_1xx_headers_(false), created_filter_chain_(false), is_head_request_(false),
-          is_grpc_request_(false), non_100_response_headers_encoded_(false),
-          under_on_local_reply_(false), decoder_filter_chain_aborted_(false),
-          encoder_filter_chain_aborted_(false), saw_downstream_reset_(false) {}
+        : remote_encode_complete_(false), local_complete_(false), has_1xx_headers_(false),
+          created_filter_chain_(false), is_head_request_(false), is_grpc_request_(false),
+          non_100_response_headers_encoded_(false), under_on_local_reply_(false),
+          decoder_filter_chain_aborted_(false), encoder_filter_chain_aborted_(false),
+          saw_downstream_reset_(false) {}
     uint32_t filter_call_state_{0};
 
     bool remote_encode_complete_ : 1;
-    bool remote_decode_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.

source/common/http/http1/codec_impl.cc

@@ -1203,8 +1203,10 @@ ParserStatus ServerConnectionImpl::onMessageCompleteBase() {
 }
 
 void ServerConnectionImpl::onResetStream(StreamResetReason reason) {
-  active_request_->response_encoder_.runResetCallbacks(reason);
-  connection_.dispatcher().deferredDelete(std::move(active_request_));
+  if (active_request_) {
+    active_request_->response_encoder_.runResetCallbacks(reason);
+    connection_.dispatcher().deferredDelete(std::move(active_request_));
+  }
 }
 
 Status ServerConnectionImpl::sendProtocolError(absl::string_view details) {

source/common/runtime/runtime_features.cc

@@ -40,6 +40,7 @@ RUNTIME_GUARD(envoy_reloadable_features_correctly_validate_alpn);
 RUNTIME_GUARD(envoy_reloadable_features_deprecate_global_ints);
 RUNTIME_GUARD(envoy_reloadable_features_disable_tls_inspector_injection);
 RUNTIME_GUARD(envoy_reloadable_features_do_not_await_headers_on_upstream_timeout_to_emit_stats);
+RUNTIME_GUARD(envoy_reloadable_features_enable_compression_bomb_protection);
 RUNTIME_GUARD(envoy_reloadable_features_enable_grpc_async_client_cache);
 RUNTIME_GUARD(envoy_reloadable_features_fix_added_trailers);
 RUNTIME_GUARD(envoy_reloadable_features_handle_stream_reset_during_hcm_encoding);

source/common/stream_info/stream_info_impl.h

@@ -308,9 +308,23 @@ struct StreamInfoImpl : public StreamInfo {
     ASSERT(downstream_bytes_meter_.get() == downstream_bytes_meter.get());
   }
 
+  // This function is used to persist relevant information from the original
+  // stream into to the new one, when recreating the stream. Generally this
+  // includes information about the downstream stream, but not the upstream
+  // stream.
+  void setFromForRecreateStream(StreamInfo& info) {
+    downstream_timing_ = info.downstreamTiming();
+    protocol_ = info.protocol();
+    bytes_received_ = info.bytesReceived();
+    downstream_bytes_meter_ = info.getDownstreamBytesMeter();
+    // These two are set in the constructor, but to T(recreate), and should be T(create)
+    start_time_ = info.startTime();
+    start_time_monotonic_ = info.startTimeMonotonic();
+  }
+
   TimeSource& time_source_;
-  const SystemTime start_time_;
-  const MonotonicTime start_time_monotonic_;
+  SystemTime start_time_;
+  MonotonicTime start_time_monotonic_;
   absl::optional<MonotonicTime> final_time_;
 
   absl::optional<Http::Protocol> protocol_;

source/common/upstream/health_checker_impl.cc

@@ -815,10 +815,17 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onGoAway(
   // Even if we have active health check probe, fail it on GOAWAY and schedule new one.
   if (request_encoder_) {
     handleFailure(envoy::data::core::v3::NETWORK);
-    expect_reset_ = true;
-    request_encoder_->getStream().resetStream(Http::StreamResetReason::LocalReset);
+    // request_encoder_ can already be destroyed if the host was removed during the failure callback
+    // above.
+    if (request_encoder_ != nullptr) {
+      expect_reset_ = true;
+      request_encoder_->getStream().resetStream(Http::StreamResetReason::LocalReset);
+    }
+  }
+  // client_ can already be destroyed if the host was removed during the failure callback above.
+  if (client_ != nullptr) {
+    client_->close();
   }
-  client_->close();
 }
 
 bool GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::isHealthCheckSucceeded(
@@ -852,12 +859,17 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onRpcComplete(
   if (end_stream) {
     resetState();
   } else {
-    // resetState() will be called by onResetStream().
-    expect_reset_ = true;
-    request_encoder_->getStream().resetStream(Http::StreamResetReason::LocalReset);
+    // request_encoder_ can already be destroyed if the host was removed during the failure callback
+    // above.
+    if (request_encoder_ != nullptr) {
+      // resetState() will be called by onResetStream().
+      expect_reset_ = true;
+      request_encoder_->getStream().resetStream(Http::StreamResetReason::LocalReset);
+    }
   }
 
-  if (!parent_.reuse_connection_ || goaway) {
+  // client_ can already be destroyed if the host was removed during the failure callback above.
+  if (client_ != nullptr && (!parent_.reuse_connection_ || goaway)) {
     client_->close();
   }
 }

source/extensions/compression/brotli/common/base.cc

@@ -6,9 +6,10 @@ namespace Compression {
 namespace Brotli {
 namespace Common {
 
-BrotliContext::BrotliContext(const uint32_t chunk_size)
-    : chunk_size_{chunk_size}, chunk_ptr_{std::make_unique<uint8_t[]>(chunk_size)}, next_in_{},
-      next_out_{chunk_ptr_.get()}, avail_in_{0}, avail_out_{chunk_size} {}
+BrotliContext::BrotliContext(uint32_t chunk_size, uint32_t max_output_size)
+    : max_output_size_{max_output_size}, chunk_size_{chunk_size},
+      chunk_ptr_{std::make_unique<uint8_t[]>(chunk_size)}, next_in_{}, next_out_{chunk_ptr_.get()},
+      avail_in_{0}, avail_out_{chunk_size} {}
 
 void BrotliContext::updateOutput(Buffer::Instance& output_buffer) {
   if (avail_out_ == 0) {

source/extensions/compression/brotli/common/base.h

@@ -12,11 +12,12 @@ namespace Common {
 
 // Keeps a `Brotli` compression stream's state.
 struct BrotliContext {
-  BrotliContext(const uint32_t chunk_size);
+  BrotliContext(uint32_t chunk_size, uint32_t max_output_size = 0);
 
   void updateOutput(Buffer::Instance& output_buffer);
   void finalizeOutput(Buffer::Instance& output_buffer);
 
+  const uint32_t max_output_size_;
   const uint32_t chunk_size_;
   std::unique_ptr<uint8_t[]> chunk_ptr_;
   const uint8_t* next_in_;

source/extensions/compression/brotli/decompressor/BUILD

@@ -19,6 +19,7 @@ envoy_cc_library(
         "//envoy/stats:stats_interface",
         "//envoy/stats:stats_macros",
         "//source/common/buffer:buffer_lib",
+        "//source/common/runtime:runtime_features_lib",
         "//source/extensions/compression/brotli/common:brotli_base_lib",
     ],
 )

source/extensions/compression/brotli/decompressor/brotli_decompressor_impl.cc

@@ -2,12 +2,24 @@
 
 #include <memory>
 
+#include "source/common/runtime/runtime_features.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace Compression {
 namespace Brotli {
 namespace Decompressor {
 
+namespace {
+
+// How many times the output buffer is allowed to be bigger than the input
+// buffer. This value is used to detect compression bombs.
+// TODO(rojkov): Re-design the Decompressor interface to handle compression
+// bombs gracefully instead of this quick solution.
+constexpr uint32_t MaxInflateRatio = 100;
+
+} // namespace
+
 BrotliDecompressorImpl::BrotliDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix,
                                                const uint32_t chunk_size,
                                                const bool disable_ring_buffer_reallocation)
@@ -22,7 +34,7 @@ BrotliDecompressorImpl::BrotliDecompressorImpl(Stats::Scope& scope, const std::s
 
 void BrotliDecompressorImpl::decompress(const Buffer::Instance& input_buffer,
                                         Buffer::Instance& output_buffer) {
-  Common::BrotliContext ctx(chunk_size_);
+  Common::BrotliContext ctx(chunk_size_, MaxInflateRatio * input_buffer.length());
 
   for (const Buffer::RawSlice& input_slice : input_buffer.getRawSlices()) {
     ctx.avail_in_ = input_slice.len_;
@@ -58,6 +70,13 @@ bool BrotliDecompressorImpl::process(Common::BrotliContext& ctx, Buffer::Instanc
     return false;
   }
 
+  if (Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.enable_compression_bomb_protection") &&
+      (output_buffer.length() > ctx.max_output_size_)) {
+    stats_.brotli_error_.inc();
+    return false;
+  }
+
   ctx.updateOutput(output_buffer);
 
   return true;

source/extensions/compression/gzip/common/base.h

@@ -12,7 +12,6 @@ namespace Zlib {
 /**
  * Shared code between the compressor and the decompressor.
  */
-// TODO(junr03): move to extensions tree once the compressor side is moved to extensions.
 class Base {
 public:
   Base(uint64_t chunk_size, std::function<void(z_stream*)> zstream_deleter);

source/extensions/compression/gzip/decompressor/BUILD

@@ -21,6 +21,7 @@ envoy_cc_library(
         "//source/common/buffer:buffer_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:minimal_logger_lib",
+        "//source/common/runtime:runtime_features_lib",
         "//source/extensions/compression/gzip/common:zlib_base_lib",
     ],
 )

source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.cc

@@ -7,6 +7,7 @@
 #include "envoy/common/exception.h"
 
 #include "source/common/common/assert.h"
+#include "source/common/runtime/runtime_features.h"
 
 #include "absl/container/fixed_array.h"
 
@@ -16,6 +17,16 @@ namespace Compression {
 namespace Gzip {
 namespace Decompressor {
 
+namespace {
+
+// How many times the output buffer is allowed to be bigger than the size of
+// accumulated input. This value is used to detect compression bombs.
+// TODO(rojkov): Re-design the Decompressor interface to handle compression
+// bombs gracefully instead of this quick solution.
+constexpr uint64_t MaxInflateRatio = 100;
+
+} // namespace
+
 ZlibDecompressorImpl::ZlibDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix)
     : ZlibDecompressorImpl(scope, stats_prefix, 4096) {}
 
@@ -43,13 +54,26 @@ void ZlibDecompressorImpl::init(int64_t window_bits) {
 
 void ZlibDecompressorImpl::decompress(const Buffer::Instance& input_buffer,
                                       Buffer::Instance& output_buffer) {
+  uint64_t limit = MaxInflateRatio * input_buffer.length();
+
   for (const Buffer::RawSlice& input_slice : input_buffer.getRawSlices()) {
     zstream_ptr_->avail_in = input_slice.len_;
     zstream_ptr_->next_in = static_cast<Bytef*>(input_slice.mem_);
     while (inflateNext()) {
       if (zstream_ptr_->avail_out == 0) {
         updateOutput(output_buffer);
       }
+
+      if (Runtime::runtimeFeatureEnabled(
+              "envoy.reloadable_features.enable_compression_bomb_protection") &&
+          (output_buffer.length() > limit)) {
+        stats_.zlib_data_error_.inc();
+        ENVOY_LOG(trace,
+                  "excessive decompression ratio detected: output "
+                  "size {} for input size {}",
+                  output_buffer.length(), input_buffer.length());
+        return;
+      }
     }
   }
 

source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.cc

@@ -1,11 +1,23 @@
 #include "source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h"
 
+#include "source/common/runtime/runtime_features.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace Compression {
 namespace Zstd {
 namespace Decompressor {
 
+namespace {
+
+// How many times the output buffer is allowed to be bigger than the size of
+// accumulated input. This value is used to detect compression bombs.
+// TODO(rojkov): Re-design the Decompressor interface to handle compression
+// bombs gracefully instead of this quick solution.
+constexpr uint64_t MaxInflateRatio = 100;
+
+} // namespace
+
 ZstdDecompressorImpl::ZstdDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix,
                                            const ZstdDDictManagerPtr& ddict_manager,
                                            uint32_t chunk_size)
@@ -14,6 +26,8 @@ ZstdDecompressorImpl::ZstdDecompressorImpl(Stats::Scope& scope, const std::strin
 
 void ZstdDecompressorImpl::decompress(const Buffer::Instance& input_buffer,
                                       Buffer::Instance& output_buffer) {
+  uint64_t limit = MaxInflateRatio * input_buffer.length();
+
   for (const Buffer::RawSlice& input_slice : input_buffer.getRawSlices()) {
     if (input_slice.len_ > 0) {
       if (ddict_manager_ && !is_dictionary_set_) {
@@ -38,6 +52,16 @@ void ZstdDecompressorImpl::decompress(const Buffer::Instance& input_buffer,
       if (!process(output_buffer)) {
         return;
       }
+      if (Runtime::runtimeFeatureEnabled(
+              "envoy.reloadable_features.enable_compression_bomb_protection") &&
+          (output_buffer.length() > limit)) {
+        stats_.zstd_generic_error_.inc();
+        ENVOY_LOG(trace,
+                  "excessive decompression ratio detected: output "
+                  "size {} for input size {}",
+                  output_buffer.length(), input_buffer.length());
+        return;
+      }
     }
   }
 }

source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h

@@ -4,6 +4,7 @@
 #include "envoy/stats/scope.h"
 #include "envoy/stats/stats_macros.h"
 
+#include "source/common/common/logger.h"
 #include "source/extensions/compression/zstd/common/base.h"
 #include "source/extensions/compression/zstd/common/dictionary_manager.h"
 
@@ -40,6 +41,7 @@ struct ZstdDecompressorStats {
  */
 class ZstdDecompressorImpl : public Common::Base,
                              public Envoy::Compression::Decompressor::Decompressor,
+                             public Logger::Loggable<Logger::Id::decompression>,
                              NonCopyable {
 public:
   ZstdDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix,