Merge branch 'main' into fix-dangling-ref

.github/workflows/mobile-release.yml

@@ -24,8 +24,7 @@ jobs:
       packages: read
     if: >-
       ${{
-          (github.repository == 'envoyproxy/envoy'
-           || vars.ENVOY_CI)
+          github.repository == 'envoyproxy/envoy'
           && (github.event.schedule
               || !contains(github.actor, '[bot]'))
       }}

api/envoy/extensions/filters/network/ext_authz/v3/ext_authz.proto

@@ -25,7 +25,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // gRPC Authorization API defined by
 // :ref:`CheckRequest <envoy_v3_api_msg_service.auth.v3.CheckRequest>`.
 // A failed check will cause this filter to close the TCP connection.
-// [#next-free-field: 8]
+// [#next-free-field: 9]
 message ExtAuthz {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.network.ext_authz.v2.ExtAuthz";
@@ -62,4 +62,10 @@ message ExtAuthz {
   // :ref:`destination<envoy_v3_api_field_service.auth.v3.AttributeContext.destination>`.
   // The labels will be read from :ref:`metadata<envoy_v3_api_msg_config.core.v3.Node>` with the specified key.
   string bootstrap_metadata_labels_key = 7;
+
+  // Specifies if the TLS session level details like SNI are sent to the external service.
+  //
+  // When this field is true, Envoy will include the SNI name used for TLSClientHello, if available, in the
+  // :ref:`tls_session<envoy_v3_api_field_service.auth.v3.AttributeContext.tls_session>`.
+  bool include_tls_session = 8;
 }

api/envoy/service/auth/v3/attribute_context.proto

@@ -187,7 +187,11 @@ message AttributeContext {
   config.core.v3.Metadata route_metadata_context = 13;
 
   // TLS session details of the underlying connection.
-  // This is not populated by default and will be populated if ext_authz filter's
-  // :ref:`include_tls_session <config_http_filters_ext_authz>` is set to true.
+  // This is not populated by default and will be populated only if the ext_authz filter has
+  // been specifically configured to include this information.
+  // For HTTP ext_authz, that requires :ref:`include_tls_session <config_http_filters_ext_authz>`
+  // to be set to true.
+  // For network ext_authz, that requires :ref:`include_tls_session <config_network_filters_ext_authz>`
+  // to be set to true.
   TLSSession tls_session = 12;
 }

changelogs/current.yaml

@@ -384,6 +384,10 @@ new_features:
   change: |
     Added :ref:`configuration option <envoy_v3_api_field_extensions.filters.http.cors.v3.CorsPolicy.forward_not_matching_preflights>`
     to return local response when CORS preflight's origin does not match allowed origin.
+- area: ext_authz
+  change: |
+    Added support for populating the :ref:`tls_session<envoy_v3_api_field_service.auth.v3.AttributeContext.tls_session>`
+    check request attribute for network ext_authz by setting :ref:`include_tls_session <config_network_filters_ext_authz>` to true.
 
 deprecated:
 - area: listener

mobile/.bazelrc

@@ -265,21 +265,23 @@ test:mobile-remote-ci-android --config=mobile-test-android
 test:mobile-remote-ci-android --config=mobile-remote-ci
 
 build:mobile-remote-ci-cc --config=mobile-remote-ci
+# Temporary revert to C++17 for mobile NDK builds.
+build:mobile-remote-ci-cc --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
 test:mobile-remote-ci-cc --action_env=LD_LIBRARY_PATH
 
-build:mobile-remote-ci-cc-no-yaml --config=mobile-remote-ci
+build:mobile-remote-ci-cc-no-yaml --config=mobile-remote-ci-cc
 build:mobile-remote-ci-cc-no-yaml --define=envoy_yaml=disabled
 build:mobile-remote-ci-cc-no-yaml --define=envoy_full_protos=disabled
 build:mobile-remote-ci-cc-no-yaml --test_env=ENVOY_IP_TEST_VERSIONS=v4only
 
-build:mobile-remote-ci-cc-no-exceptions --config=mobile-remote-ci
+build:mobile-remote-ci-cc-no-exceptions --config=mobile-remote-ci-cc
 build:mobile-remote-ci-cc-no-exceptions --define=envoy_yaml=disabled
 build:mobile-remote-ci-cc-no-exceptions --define envoy_exceptions=disabled
 build:mobile-remote-ci-cc-no-exceptions --copt=-fno-exceptions
 
-build:mobile-remote-ci-cc-test --config=mobile-remote-ci
+build:mobile-remote-ci-cc-test --config=mobile-remote-ci-cc
 test:mobile-remote-ci-cc-test --test_output=all
-test:mobile-remote-ci-cc-test --config=mobile-remote-ci
+test:mobile-remote-ci-cc-test --config=mobile-remote-ci-cc
 test:mobile-remote-ci-cc-test --define=envoy_yaml=disabled
 
 build:mobile-remote-ci-macos-kotlin --config=mobile-remote-ci-macos

mobile/library/cc/engine_builder.cc

@@ -186,6 +186,11 @@ EngineBuilder& EngineBuilder::setOnEngineRunning(std::function<void()> closure)
   return *this;
 }
 
+EngineBuilder& EngineBuilder::setEventTracker(std::unique_ptr<EnvoyEventTracker> event_tracker) {
+  event_tracker_ = std::move(event_tracker);
+  return *this;
+}
+
 EngineBuilder& EngineBuilder::addConnectTimeoutSeconds(int connect_timeout_seconds) {
   connect_timeout_seconds_ = connect_timeout_seconds;
   return *this;
@@ -916,10 +921,8 @@ std::unique_ptr<envoy::config::bootstrap::v3::Bootstrap> EngineBuilder::generate
 }
 
 EngineSharedPtr EngineBuilder::build() {
-  envoy_event_tracker null_tracker{};
-
   InternalEngine* envoy_engine =
-      new InternalEngine(std::move(callbacks_), std::move(logger_), null_tracker);
+      new InternalEngine(std::move(callbacks_), std::move(logger_), std::move(event_tracker_));
 
   for (const auto& [name, store] : key_value_stores_) {
     // TODO(goaway): This leaks, but it's tied to the life of the engine.

mobile/library/cc/engine_builder.h

@@ -131,6 +131,7 @@ class EngineBuilder {
   EngineBuilder& setEngineCallbacks(std::unique_ptr<EngineCallbacks> callbacks);
   [[deprecated("Use EngineBuilder::setEngineCallbacks instead")]] EngineBuilder&
   setOnEngineRunning(std::function<void()> closure);
+  EngineBuilder& setEventTracker(std::unique_ptr<EnvoyEventTracker> event_tracker);
   EngineBuilder& addConnectTimeoutSeconds(int connect_timeout_seconds);
   EngineBuilder& addDnsRefreshSeconds(int dns_refresh_seconds);
   EngineBuilder& addDnsFailureRefreshSeconds(int base, int max);
@@ -219,12 +220,13 @@ class EngineBuilder {
   Logger::Logger::Levels log_level_ = Logger::Logger::Levels::info;
   std::unique_ptr<EnvoyLogger> logger_{nullptr};
   std::unique_ptr<EngineCallbacks> callbacks_;
+  std::unique_ptr<EnvoyEventTracker> event_tracker_{nullptr};
 
   int connect_timeout_seconds_ = 30;
   int dns_refresh_seconds_ = 60;
   int dns_failure_refresh_seconds_base_ = 2;
   int dns_failure_refresh_seconds_max_ = 10;
-  int dns_query_timeout_seconds_ = 25;
+  int dns_query_timeout_seconds_ = 5;
   bool use_system_resolver_ = true;
   int h2_connection_keepalive_idle_interval_milliseconds_ = 100000000;
   int h2_connection_keepalive_timeout_seconds_ = 10;

mobile/library/common/BUILD

@@ -82,6 +82,8 @@ envoy_cc_library(
     ],
     repository = "@envoy",
     deps = [
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/strings",
         "@envoy//source/common/common:base_logger_lib",
     ],
 )

mobile/library/common/engine_types.h

@@ -15,6 +15,9 @@
 
 #include "source/common/common/base_logger.h"
 
+#include "absl/container/flat_hash_map.h"
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 
 /** The callbacks for the Envoy Engine. */
@@ -30,4 +33,13 @@ struct EnvoyLogger {
   std::function<void()> on_exit = [] {};
 };
 
+inline constexpr absl::string_view ENVOY_EVENT_TRACKER_API_NAME = "event_tracker_api";
+
+/** The callbacks for Envoy Event Tracker. */
+struct EnvoyEventTracker {
+  std::function<void(const absl::flat_hash_map<std::string, std::string>&)> on_track =
+      [](const absl::flat_hash_map<std::string, std::string>&) {};
+  std::function<void()> on_exit = [] {};
+};
+
 } // namespace Envoy

mobile/library/common/internal_engine.cc

@@ -5,8 +5,6 @@
 #include "source/common/runtime/runtime_features.h"
 
 #include "absl/synchronization/notification.h"
-#include "library/common/bridge/utility.h"
-#include "library/common/data/utility.h"
 #include "library/common/stats/utility.h"
 
 namespace Envoy {
@@ -17,17 +15,14 @@ envoy_stream_t InternalEngine::initStream() { return current_stream_handle_++; }
 
 InternalEngine::InternalEngine(std::unique_ptr<EngineCallbacks> callbacks,
                                std::unique_ptr<EnvoyLogger> logger,
-                               envoy_event_tracker event_tracker,
+                               std::unique_ptr<EnvoyEventTracker> event_tracker,
                                Thread::PosixThreadFactoryPtr thread_factory)
     : thread_factory_(std::move(thread_factory)), callbacks_(std::move(callbacks)),
-      logger_(std::move(logger)), event_tracker_(event_tracker),
+      logger_(std::move(logger)), event_tracker_(std::move(event_tracker)),
       dispatcher_(std::make_unique<Event::ProvisionalDispatcher>()) {
   ExtensionRegistry::registerFactories();
 
-  // TODO(Augustyniak): Capturing an address of event_tracker_ and registering it in the API
-  // registry may lead to crashes at Engine shutdown. To be figured out as part of
-  // https://github.com/envoyproxy/envoy-mobile/issues/332
-  Envoy::Api::External::registerApi(std::string(envoy_event_tracker_api_name), &event_tracker_);
+  Api::External::registerApi(std::string(ENVOY_EVENT_TRACKER_API_NAME), &event_tracker_);
   // Envoy Mobile always requires dfp_mixed_scheme for the TLS and cleartext DFP clusters.
   // While dfp_mixed_scheme defaults to true, some environments force it to false (e.g. within
   // Google), so we force it back to true in Envoy Mobile.
@@ -37,8 +32,8 @@ InternalEngine::InternalEngine(std::unique_ptr<EngineCallbacks> callbacks,
 
 InternalEngine::InternalEngine(std::unique_ptr<EngineCallbacks> callbacks,
                                std::unique_ptr<EnvoyLogger> logger,
-                               envoy_event_tracker event_tracker)
-    : InternalEngine(std::move(callbacks), std::move(logger), event_tracker,
+                               std::unique_ptr<EnvoyEventTracker> event_tracker)
+    : InternalEngine(std::move(callbacks), std::move(logger), std::move(event_tracker),
                      Thread::PosixThreadFactory::create()) {}
 
 envoy_status_t InternalEngine::run(const std::string& config, const std::string& log_level) {
@@ -68,18 +63,18 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
   {
     Thread::LockGuard lock(mutex_);
     TRY_NEEDS_AUDIT {
-      if (event_tracker_.track != nullptr) {
+      if (event_tracker_ != nullptr) {
         assert_handler_registration_ =
             Assert::addDebugAssertionFailureRecordAction([this](const char* location) {
-              const auto event = Bridge::Utility::makeEnvoyMap(
-                  {{"name", "assertion"}, {"location", std::string(location)}});
-              event_tracker_.track(event, event_tracker_.context);
+              absl::flat_hash_map<std::string, std::string> event{
+                  {{"name", "assertion"}, {"location", std::string(location)}}};
+              event_tracker_->on_track(event);
             });
         bug_handler_registration_ =
             Assert::addEnvoyBugFailureRecordAction([this](const char* location) {
-              const auto event = Bridge::Utility::makeEnvoyMap(
-                  {{"name", "bug"}, {"location", std::string(location)}});
-              event_tracker_.track(event, event_tracker_.context);
+              absl::flat_hash_map<std::string, std::string> event{
+                  {{"name", "bug"}, {"location", std::string(location)}}};
+              event_tracker_->on_track(event);
             });
       }
 
@@ -149,6 +144,9 @@ envoy_status_t InternalEngine::main(std::shared_ptr<Envoy::OptionsImplBase> opti
   bug_handler_registration_.reset(nullptr);
   assert_handler_registration_.reset(nullptr);
 
+  if (event_tracker_ != nullptr) {
+    event_tracker_->on_exit();
+  }
   callbacks_->on_exit();
 
   return run_success ? ENVOY_SUCCESS : ENVOY_FAILURE;

mobile/library/common/internal_engine.h

@@ -26,7 +26,7 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
    * @param event_tracker, the event tracker to use for the emission of events.
    */
   InternalEngine(std::unique_ptr<EngineCallbacks> callbacks, std::unique_ptr<EnvoyLogger> logger,
-                 envoy_event_tracker event_tracker);
+                 std::unique_ptr<EnvoyEventTracker> event_tracker);
 
   /**
    * InternalEngine destructor.
@@ -123,7 +123,8 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
   GTEST_FRIEND_CLASS(InternalEngineTest, ThreadCreationFailed);
 
   InternalEngine(std::unique_ptr<EngineCallbacks> callbacks, std::unique_ptr<EnvoyLogger> logger,
-                 envoy_event_tracker event_tracker, Thread::PosixThreadFactoryPtr thread_factory);
+                 std::unique_ptr<EnvoyEventTracker> event_tracker,
+                 Thread::PosixThreadFactoryPtr thread_factory);
 
   envoy_status_t main(std::shared_ptr<Envoy::OptionsImplBase> options);
   static void logInterfaces(absl::string_view event,
@@ -135,7 +136,7 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
   Stats::StatNameSetPtr stat_name_set_;
   std::unique_ptr<EngineCallbacks> callbacks_;
   std::unique_ptr<EnvoyLogger> logger_;
-  envoy_event_tracker event_tracker_;
+  std::unique_ptr<EnvoyEventTracker> event_tracker_;
   Assert::ActionRegistrationPtr assert_handler_registration_;
   Assert::ActionRegistrationPtr bug_handler_registration_;
   Thread::MutexBasicLockable mutex_;

mobile/library/common/logger/logger_delegate.cc

@@ -9,14 +9,14 @@ namespace Logger {
 
 void EventTrackingDelegate::logWithStableName(absl::string_view stable_name, absl::string_view,
                                               absl::string_view, absl::string_view msg) {
-  if (event_tracker_.track == nullptr) {
+  if (event_tracker_ == nullptr || (*event_tracker_) == nullptr) {
     return;
   }
 
-  event_tracker_.track(Bridge::Utility::makeEnvoyMap({{"name", "event_log"},
-                                                      {"log_name", std::string(stable_name)},
-                                                      {"message", std::string(msg)}}),
-                       event_tracker_.context);
+  (*event_tracker_)
+      ->on_track({{"name", "event_log"},
+                  {"log_name", std::string(stable_name)},
+                  {"message", std::string(msg)}});
 }
 
 LambdaDelegate::LambdaDelegate(std::unique_ptr<EnvoyLogger> logger,

mobile/library/common/logger/logger_delegate.h

@@ -15,14 +15,14 @@ namespace Logger {
 class EventTrackingDelegate : public SinkDelegate {
 public:
   explicit EventTrackingDelegate(DelegatingLogSinkSharedPtr log_sink)
-      : SinkDelegate(log_sink), event_tracker_(*static_cast<envoy_event_tracker*>(
-                                    Api::External::retrieveApi(envoy_event_tracker_api_name))) {}
+      : SinkDelegate(log_sink), event_tracker_(static_cast<std::unique_ptr<EnvoyEventTracker>*>(
+                                    Api::External::retrieveApi(ENVOY_EVENT_TRACKER_API_NAME))) {}
 
   void logWithStableName(absl::string_view stable_name, absl::string_view level,
                          absl::string_view component, absl::string_view msg) override;
 
 private:
-  envoy_event_tracker& event_tracker_;
+  std::unique_ptr<EnvoyEventTracker>* event_tracker_;
 };
 
 using EventTrackingDelegatePtr = std::unique_ptr<EventTrackingDelegate>;

mobile/library/common/types/c_types.cc

@@ -72,5 +72,3 @@ const envoy_data envoy_nodata = {0, NULL, envoy_noop_release, NULL};
 const envoy_headers envoy_noheaders = {0, NULL};
 
 const envoy_stats_tags envoy_stats_notags = {0, NULL};
-
-const char* envoy_event_tracker_api_name = "event_tracker_api";

mobile/library/common/types/c_types.h

@@ -73,9 +73,6 @@ typedef enum {
   ENVOY_NET_WWAN = 2,
 } envoy_network_t;
 
-// The name used to registered event tracker api.
-extern const char* envoy_event_tracker_api_name;
-
 #ifdef __cplusplus
 extern "C" { // release function
 #endif
@@ -411,15 +408,6 @@ typedef void (*envoy_on_cancel_f)(envoy_stream_intel stream_intel,
  */
 typedef void (*envoy_on_send_window_available_f)(envoy_stream_intel stream_intel, void* context);
 
-/**
- * Called when envoy's event tracker tracks an event.
- *
- * @param event, the dictionary with attributes that describe the event.
- * @param context, contains the necessary state to carry out platform-specific dispatch and
- * execution.
- */
-typedef void (*envoy_event_tracker_track_f)(envoy_map event, const void* context);
-
 #ifdef __cplusplus
 } // function pointers
 #endif
@@ -440,15 +428,6 @@ typedef struct {
   void* context;
 } envoy_http_callbacks;
 
-/**
- * Interface for event tracking.
- */
-typedef struct {
-  envoy_event_tracker_track_f track;
-  // Context passed through to callbacks to provide dispatch and execution state.
-  const void* context;
-} envoy_event_tracker;
-
 /**
  * The list of certificate verification results returned from Java side to the
  * C++ side.

mobile/library/java/org/chromium/net/impl/NativeCronvoyEngineBuilderImpl.java

@@ -37,7 +37,7 @@ public class NativeCronvoyEngineBuilderImpl extends CronvoyEngineBuilderImpl {
   private final int mDnsRefreshSeconds = 60;
   private final int mDnsFailureRefreshSecondsBase = 2;
   private final int mDnsFailureRefreshSecondsMax = 10;
-  private final int mDnsQueryTimeoutSeconds = 25;
+  private int mDnsQueryTimeoutSeconds = 5;
   private final int mDnsMinRefreshSeconds = 60;
   private final List<String> mDnsPreresolveHostnames = Collections.emptyList();
   private final boolean mEnableDNSCache = false;
@@ -82,6 +82,19 @@ public NativeCronvoyEngineBuilderImpl setEnableDrainPostDnsRefresh(boolean enabl
     return this;
   }
 
+  /**
+   * Set the DNS query timeout, in seconds, which ensures that DNS queries succeed or fail
+   * within that time range. See the DnsCacheConfig.dns_query_timeout proto field for details.
+   *
+   * The default is 5s.
+   *
+   * @param timeout The DNS query timeout value, in seconds.
+   */
+  public NativeCronvoyEngineBuilderImpl setDnsQueryTimeoutSeconds(int timeout) {
+    mDnsQueryTimeoutSeconds = timeout;
+    return this;
+  }
+
   /**
    * Indicates to skip the TLS certificate verification.
    *