From b9c4ff2da2a91feb263eda5f39bc7e8307a669ee Mon Sep 17 00:00:00 2001 From: code Date: Thu, 19 Dec 2024 20:33:51 +0800 Subject: [PATCH] Revert "jwt_authn: Add logic to refetch JWT on KID mismatch (#36458)" (#37763) --- .../filters/http/jwt_authn/v3/config.proto | 19 -- changelogs/current.yaml | 5 - .../extensions/filters/http/jwt_authn/BUILD | 1 - .../filters/http/jwt_authn/authenticator.cc | 69 ++--- .../filters/http/jwt_authn/authenticator.h | 2 +- .../filters/http/jwt_authn/filter_config.cc | 3 +- .../filters/http/jwt_authn/filter_config.h | 6 +- .../http/jwt_authn/jwks_async_fetcher.cc | 28 +-- .../http/jwt_authn/jwks_async_fetcher.h | 15 +- .../filters/http/jwt_authn/jwks_cache.cc | 84 +------ .../filters/http/jwt_authn/jwks_cache.h | 12 +- .../http/jwt_authn/authenticator_test.cc | 123 +-------- .../http/jwt_authn/filter_integration_test.cc | 236 ++---------------- .../http/jwt_authn/jwks_async_fetcher_test.cc | 48 +--- .../filters/http/jwt_authn/jwks_cache_test.cc | 39 +-- test/extensions/filters/http/jwt_authn/mock.h | 4 +- .../filters/http/jwt_authn/test_common.h | 71 ------ tools/spelling/spelling_dictionary.txt | 4 - 18 files changed, 65 insertions(+), 704 deletions(-) diff --git a/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto b/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto index be6a4372db0b..99212f3edfd9 100644 --- a/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto +++ b/api/envoy/extensions/filters/http/jwt_authn/v3/config.proto @@ -383,7 +383,6 @@ message JwtCacheConfig { } // This message specifies how to fetch JWKS from remote and how to cache it. -// [#next-free-field: 6] message RemoteJwks { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.http.jwt_authn.v2alpha.RemoteJwks"; @@ -453,24 +452,6 @@ message RemoteJwks { // // config.core.v3.RetryPolicy retry_policy = 4; - - // Refetch JWKS if extracted JWT has no KID or a KID that does not match any cached JWKS's KID. - // - // - // In envoy, if :ref:`async JWKS fetching ` - // is enabled along with this field, then KID mismatch will trigger a new async fetch after appropriate backoff delay. - // - // - // If async fetching is disabled, new JWKS is fetched on demand and the cache is isolated to the fetched worker thread. - // - // There is exponential backoff built into this retrieval system for two cases to avoid DoS on JWKS Server: - // - // * If there is a request containing a JWT with no KID, a new fetch will be made for this request. Upon retrieval, - // a backoff will be triggered. - // * If there is a fetch due to KID mismatch, which results in a failed fetch or verification, a backoff will be triggered. - // - // During a backoff, no further fetches will be made due to KID mismatch. - bool refetch_jwks_on_kid_mismatch = 5; } // Fetch Jwks asynchronously in the main thread when the filter config is parsed. diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 24a72f4122fb..d5b342017b8e 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -342,11 +342,6 @@ new_features: change: | Added :ref:`attribute ` ``upstream.cx_pool_ready_duration`` to get the duration from when the upstream request was created to when the upstream connection pool is ready. -- area: jwt_authn - change: | - Added :ref:`refetch_jwks_on_kid_mismatch - ` - to allow filter to refetch JWKS when extracted JWT's KID does not match cached JWKS's KID. - area: health_check change: | Added new health check filter stats including total requests, successful/failed checks, cached responses, and diff --git a/source/extensions/filters/http/jwt_authn/BUILD b/source/extensions/filters/http/jwt_authn/BUILD index ad0fb61e4547..c01f977750ba 100644 --- a/source/extensions/filters/http/jwt_authn/BUILD +++ b/source/extensions/filters/http/jwt_authn/BUILD @@ -55,7 +55,6 @@ envoy_cc_library( "jwks_async_fetcher_lib", ":jwt_cache_lib", "//source/common/config:datasource_lib", - "//source/common/config:utility_lib", "@com_github_google_jwt_verify//:jwt_verify_lib", "@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto", ], diff --git a/source/extensions/filters/http/jwt_authn/authenticator.cc b/source/extensions/filters/http/jwt_authn/authenticator.cc index cb079fd52b73..cecbc2c0b0da 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.cc +++ b/source/extensions/filters/http/jwt_authn/authenticator.cc @@ -51,12 +51,11 @@ class AuthenticatorImpl : public Logger::Loggable, const absl::optional& provider, bool allow_failed, bool allow_missing, JwksCache& jwks_cache, Upstream::ClusterManager& cluster_manager, - CreateJwksFetcherCb create_jwks_fetcher_cb, TimeSource& time_source, - Event::Dispatcher& dispatcher) + CreateJwksFetcherCb create_jwks_fetcher_cb, TimeSource& time_source) : jwks_cache_(jwks_cache), cm_(cluster_manager), create_jwks_fetcher_cb_(create_jwks_fetcher_cb), check_audience_(check_audience), provider_(provider), is_allow_failed_(allow_failed), is_allow_missing_(allow_missing), - time_source_(time_source), dispatcher_(dispatcher) {} + time_source_(time_source) {} // Following functions are for JwksFetcher::JwksReceiver interface void onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) override; void onJwksError(Failure reason) override; @@ -131,9 +130,6 @@ class AuthenticatorImpl : public Logger::Loggable, const bool is_allow_missing_; TimeSource& time_source_; ::google::jwt_verify::Jwt* jwt_{}; - Event::Dispatcher& dispatcher_; - // Set to true if the JWT in this request caused a KID mismatch. - bool kid_mismatch_request_{false}; }; std::string AuthenticatorImpl::name() const { @@ -272,24 +268,16 @@ void AuthenticatorImpl::startVerify() { auto jwks_obj = jwks_data_->getJwksObj(); if (jwks_obj != nullptr && !jwks_data_->isExpired()) { - // If KID mismatch fetching is set and fetch is disallowed, trigger the - // verification process. - if (jwks_data_->getJwtProvider().remote_jwks().refetch_jwks_on_kid_mismatch() && - jwks_data_->isRemoteJwksFetchAllowed()) { - if (!jwt_->kid_.empty()) { - for (const auto& jwk : jwks_obj->keys()) { - if (jwk->kid_ == jwt_->kid_) { - verifyKey(); - return; - } - } - } - ENVOY_LOG(info, "Triggering refetch of JWKS due to KID mismatch"); - kid_mismatch_request_ = true; - } else { - verifyKey(); - return; - } + // TODO(qiwzhang): It would seem there's a window of error whereby if the JWT issuer + // has started signing with a new key that's not in our cache, then the + // verification will fail even though the JWT is valid. A simple fix + // would be to check the JWS kid header field; if present check we have + // the key cached, if we do proceed to verify else try a new JWKS retrieval. + // JWTs without a kid header field in the JWS we might be best to get each + // time? This all only matters for remote JWKS. + + verifyKey(); + return; } // TODO(potatop): potential optimization. @@ -302,10 +290,6 @@ void AuthenticatorImpl::startVerify() { fetcher_ = create_jwks_fetcher_cb_(cm_, jwks_data_->getJwtProvider().remote_jwks()); } fetcher_->fetch(*parent_span_, *this); - // Disallow fetches across other worker threads due to in-flight fetch. - if (kid_mismatch_request_) { - dispatcher_.post([this]() { jwks_data_->allowRemoteJwksFetch(absl::nullopt, true); }); - } return; } // No valid keys for this issuer. This may happen as a result of incorrect local @@ -315,14 +299,7 @@ void AuthenticatorImpl::startVerify() { void AuthenticatorImpl::onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) { jwks_cache_.stats().jwks_fetch_success_.inc(); - - const Status status = - jwks_data_ - ->setRemoteJwks(std::move(jwks), - jwks_data_->getJwtProvider().remote_jwks().has_async_fetch() && - kid_mismatch_request_) - ->getStatus(); - + const Status status = jwks_data_->setRemoteJwks(std::move(jwks))->getStatus(); if (status != Status::Ok) { doneWithStatus(status); } else { @@ -442,15 +419,6 @@ void AuthenticatorImpl::handleGoodJwt(bool cache_hit) { // move the ownership of "owned_jwt_" into the function. jwks_data_->getJwtCache().insert(curr_token_->token(), std::move(owned_jwt_)); } - - // On successful retrieval & verification of JWKS due to KID mismatch, - // - If retrieved due to KID being empty, trigger backoff. - // - Else, shut down backoff. - if (kid_mismatch_request_ && !jwks_data_->isRemoteJwksFetchAllowed()) { - dispatcher_.post([this]() { jwks_data_->allowRemoteJwksFetch(!jwt_->kid_.empty(), false); }); - kid_mismatch_request_ = false; - } - doneWithStatus(Status::Ok); } @@ -483,13 +451,6 @@ void AuthenticatorImpl::doneWithStatus(const Status& status) { // Forward the failed status to dynamic metadata ENVOY_LOG(debug, "status is: {}", ::google::jwt_verify::getStatusString(status)); - // Trigger backoff to disallow further fetches when retrieval & verification is due to KID - // mismatch and fails. - if (kid_mismatch_request_ && !jwks_data_->isRemoteJwksFetchAllowed()) { - dispatcher_.post([this]() { jwks_data_->allowRemoteJwksFetch(false, false); }); - kid_mismatch_request_ = false; - } - std::string failed_status_in_metadata; if (jwks_data_) { @@ -546,10 +507,10 @@ AuthenticatorPtr Authenticator::create(const CheckAudience* check_audience, bool allow_failed, bool allow_missing, JwksCache& jwks_cache, Upstream::ClusterManager& cluster_manager, CreateJwksFetcherCb create_jwks_fetcher_cb, - TimeSource& time_source, Event::Dispatcher& dispatcher) { + TimeSource& time_source) { return std::make_unique(check_audience, provider, allow_failed, allow_missing, jwks_cache, cluster_manager, create_jwks_fetcher_cb, - time_source, dispatcher); + time_source); } } // namespace JwtAuthn diff --git a/source/extensions/filters/http/jwt_authn/authenticator.h b/source/extensions/filters/http/jwt_authn/authenticator.h index 811cb07e272e..d54157a472f4 100644 --- a/source/extensions/filters/http/jwt_authn/authenticator.h +++ b/source/extensions/filters/http/jwt_authn/authenticator.h @@ -47,7 +47,7 @@ class Authenticator { bool allow_missing, JwksCache& jwks_cache, Upstream::ClusterManager& cluster_manager, CreateJwksFetcherCb create_jwks_fetcher_cb, - TimeSource& time_source, Event::Dispatcher& dispatcher); + TimeSource& time_source); }; /** diff --git a/source/extensions/filters/http/jwt_authn/filter_config.cc b/source/extensions/filters/http/jwt_authn/filter_config.cc index a51b8428f110..71565c4486a9 100644 --- a/source/extensions/filters/http/jwt_authn/filter_config.cc +++ b/source/extensions/filters/http/jwt_authn/filter_config.cc @@ -18,8 +18,7 @@ FilterConfigImpl::FilterConfigImpl( const std::string& stats_prefix, Server::Configuration::FactoryContext& context) : proto_config_(std::move(proto_config)), stats_(generateStats(stats_prefix, context.scope())), cm_(context.serverFactoryContext().clusterManager()), - time_source_(context.serverFactoryContext().mainThreadDispatcher().timeSource()), - dispatcher_(context.serverFactoryContext().mainThreadDispatcher()) { + time_source_(context.serverFactoryContext().mainThreadDispatcher().timeSource()) { ENVOY_LOG(debug, "Loaded JwtAuthConfig: {}", proto_config_.DebugString()); diff --git a/source/extensions/filters/http/jwt_authn/filter_config.h b/source/extensions/filters/http/jwt_authn/filter_config.h index f75b9416cdbc..a69aaed5caf0 100644 --- a/source/extensions/filters/http/jwt_authn/filter_config.h +++ b/source/extensions/filters/http/jwt_authn/filter_config.h @@ -76,8 +76,6 @@ class FilterConfigImpl : public Logger::Loggable, Upstream::ClusterManager& cm() const { return cm_; } TimeSource& timeSource() const { return time_source_; } - Event::Dispatcher& dispatcher() const { return dispatcher_; } - // FilterConfig JwtAuthnFilterStats& stats() override { return stats_; } @@ -114,8 +112,7 @@ class FilterConfigImpl : public Logger::Loggable, const absl::optional& provider, bool allow_failed, bool allow_missing) const override { return Authenticator::create(check_audience, provider, allow_failed, allow_missing, - getJwksCache(), cm(), Common::JwksFetcher::create, timeSource(), - dispatcher()); + getJwksCache(), cm(), Common::JwksFetcher::create, timeSource()); } private: @@ -150,7 +147,6 @@ class FilterConfigImpl : public Logger::Loggable, // all requirement_names for debug std::string all_requirement_names_; TimeSource& time_source_; - Event::Dispatcher& dispatcher_; }; } // namespace JwtAuthn diff --git a/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.cc b/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.cc index 01c030002d22..af2e567b6252 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.cc +++ b/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.cc @@ -36,12 +36,9 @@ std::chrono::milliseconds getFailedRefetchDuration(const JwksAsyncFetch& async_f JwksAsyncFetcher::JwksAsyncFetcher(const RemoteJwks& remote_jwks, Server::Configuration::FactoryContext& context, CreateJwksFetcherCb create_fetcher_fn, - JwtAuthnFilterStats& stats, JwksDoneFetched done_fn, - isRemoteJwksFetchAllowedCb is_fetch_allowed_fn, - allowRemoteJwksFetchCb allow_fetch_fn) + JwtAuthnFilterStats& stats, JwksDoneFetched done_fn) : remote_jwks_(remote_jwks), context_(context), create_fetcher_fn_(create_fetcher_fn), - stats_(stats), done_fn_(done_fn), is_fetch_allowed_fn_(is_fetch_allowed_fn), - allow_fetch_fn_(allow_fetch_fn), + stats_(stats), done_fn_(done_fn), debug_name_(absl::StrCat("Jwks async fetching url=", remote_jwks_.http_uri().uri())) { // if async_fetch is not enabled, do nothing. if (!remote_jwks_.has_async_fetch()) { @@ -81,14 +78,7 @@ std::chrono::seconds JwksAsyncFetcher::getCacheDuration(const RemoteJwks& remote return DefaultCacheExpirationSec; } -void JwksAsyncFetcher::resetFetchTimer() { refetch_timer_->enableTimer(good_refetch_duration_); } - void JwksAsyncFetcher::fetch() { - if (remote_jwks_.refetch_jwks_on_kid_mismatch() && !is_fetch_allowed_fn_()) { - resetFetchTimer(); - return; - } - if (fetcher_) { fetcher_->cancel(); } @@ -96,10 +86,6 @@ void JwksAsyncFetcher::fetch() { ENVOY_LOG(debug, "{}: started", debug_name_); fetcher_ = create_fetcher_fn_(context_.serverFactoryContext().clusterManager(), remote_jwks_); fetcher_->fetch(Tracing::NullSpan::instance(), *this); - - if (remote_jwks_.refetch_jwks_on_kid_mismatch()) { - allow_fetch_fn_(absl::nullopt, true); - } } void JwksAsyncFetcher::handleFetchDone() { @@ -111,12 +97,8 @@ void JwksAsyncFetcher::handleFetchDone() { void JwksAsyncFetcher::onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) { done_fn_(std::move(jwks)); - if (remote_jwks_.refetch_jwks_on_kid_mismatch()) { - // Don't modify backoff for async fetch. - allow_fetch_fn_(absl::nullopt, false); - } handleFetchDone(); - resetFetchTimer(); + refetch_timer_->enableTimer(good_refetch_duration_); stats_.jwks_fetch_success_.inc(); // Note: not to free fetcher_ within onJwksSuccess or onJwksError function. @@ -131,10 +113,6 @@ void JwksAsyncFetcher::onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) { void JwksAsyncFetcher::onJwksError(Failure) { ENVOY_LOG(warn, "{}: failed", debug_name_); - if (remote_jwks_.refetch_jwks_on_kid_mismatch()) { - // Don't modify backoff for async fetch. - allow_fetch_fn_(absl::nullopt, false); - } handleFetchDone(); refetch_timer_->enableTimer(failed_refetch_duration_); stats_.jwks_fetch_failed_.inc(); diff --git a/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.h b/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.h index 541e2d1037d6..975ddd6ec468 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.h +++ b/source/extensions/filters/http/jwt_authn/jwks_async_fetcher.h @@ -25,10 +25,6 @@ using CreateJwksFetcherCb = std::function; -using isRemoteJwksFetchAllowedCb = std::function; - -using allowRemoteJwksFetchCb = std::function, bool)>; - // This class handles fetching Jwks asynchronously. // It will be no-op if async_fetch is not enabled. // At its constructor, it will start to fetch Jwks, register with init_manager if not fast_listener. @@ -39,17 +35,12 @@ class JwksAsyncFetcher : public Logger::Loggable, public: JwksAsyncFetcher(const envoy::extensions::filters::http::jwt_authn::v3::RemoteJwks& remote_jwks, Server::Configuration::FactoryContext& context, CreateJwksFetcherCb fetcher_fn, - JwtAuthnFilterStats& stats, JwksDoneFetched done_fn, - isRemoteJwksFetchAllowedCb is_fetch_allowed_fn, - allowRemoteJwksFetchCb allow_fetch_fn); + JwtAuthnFilterStats& stats, JwksDoneFetched done_fn); // Get the remote Jwks cache duration. static std::chrono::seconds getCacheDuration(const envoy::extensions::filters::http::jwt_authn::v3::RemoteJwks& remote_jwks); - // Reset async fetch timer to default value. - void resetFetchTimer(); - private: // Fetch the Jwks void fetch(); @@ -84,10 +75,6 @@ class JwksAsyncFetcher : public Logger::Loggable, // The init target. std::unique_ptr init_target_; - const isRemoteJwksFetchAllowedCb is_fetch_allowed_fn_; - - const allowRemoteJwksFetchCb allow_fetch_fn_; - // Used in logs. const std::string debug_name_; }; diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.cc b/source/extensions/filters/http/jwt_authn/jwks_cache.cc index 6732dfa66c0b..146fc70a08b2 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.cc +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.cc @@ -29,27 +29,12 @@ namespace HttpFilters { namespace JwtAuthn { namespace { -static constexpr uint32_t RetryInitialDelayMilliseconds = 1000; -static constexpr uint32_t RetryMaxDelayMilliseconds = 30000; - class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable { public: JwksDataImpl(const JwtProvider& jwt_provider, Server::Configuration::FactoryContext& context, CreateJwksFetcherCb fetcher_cb, JwtAuthnFilterStats& stats) : jwt_provider_(jwt_provider), time_source_(context.serverFactoryContext().timeSource()), - tls_(context.serverFactoryContext().threadLocal()), - random_(context.serverFactoryContext().api().randomGenerator()), - dispatcher_(context.serverFactoryContext().mainThreadDispatcher()) { - - remote_fetch_backoff_strategy_ = std::make_unique( - RetryInitialDelayMilliseconds, RetryMaxDelayMilliseconds, random_); - - remote_fetch_disallow_timer_ = - context.serverFactoryContext().mainThreadDispatcher().createTimer([this]() -> void { - tls_.runOnAllThreads( - [](OptRef obj) { obj->is_backoff_enabled_ = false; }); - ENVOY_LOG(debug, "JWKS fetch backoff completed. Fetching can resume."); - }); + tls_(context.serverFactoryContext().threadLocal()) { if (jwt_provider_.has_remote_jwks()) { // remote_jwks.retry_policy has an invalid case that could not be validated by the @@ -94,8 +79,7 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable(enable_jwt_cache, config, dispatcher.timeSource(), - false, false); + return std::make_shared(enable_jwt_cache, config, dispatcher.timeSource()); }); const auto inline_jwks = @@ -116,11 +100,7 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable( jwt_provider_.remote_jwks(), context, fetcher_cb, stats, - [this](google::jwt_verify::JwksPtr&& jwks) { setJwksToAllThreads(std::move(jwks)); }, - [this]() -> bool { return isRemoteJwksFetchAllowed(); }, - [this](absl::optional stop_backoff, bool fetch_in_flight) { - allowRemoteJwksFetch(stop_backoff, fetch_in_flight); - }); + [this](google::jwt_verify::JwksPtr&& jwks) { setJwksToAllThreads(std::move(jwks)); }); } } } @@ -164,60 +144,23 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable= tls_->expire_; } - const ::google::jwt_verify::Jwks* setRemoteJwks(JwksConstPtr&& jwks, - bool update_all_threads) override { + const ::google::jwt_verify::Jwks* setRemoteJwks(JwksConstPtr&& jwks) override { + // convert unique_ptr to shared_ptr JwksConstSharedPtr shared_jwks = std::move(jwks); tls_->jwks_ = shared_jwks; tls_->expire_ = time_source_.monotonicTime() + JwksAsyncFetcher::getCacheDuration(jwt_provider_.remote_jwks()); - - if (update_all_threads) { - dispatcher_.post([this, jwks = shared_jwks]() mutable { setJwksToAllThreads(jwks); }); - } - return shared_jwks.get(); } JwtCache& getJwtCache() override { return *tls_->jwt_cache_; } - bool isRemoteJwksFetchAllowed() override { - if (!jwt_provider_.has_remote_jwks() && - !jwt_provider_.remote_jwks().refetch_jwks_on_kid_mismatch()) { - return true; - } - return !(tls_->fetch_in_flight_ || tls_->is_backoff_enabled_); - } - - void allowRemoteJwksFetch(absl::optional stop_backoff, bool fetch_in_flight) override { - ASSERT(dispatcher_.isThreadSafe()); - - if (stop_backoff.has_value()) { - tls_.runOnAllThreads([stop_backoff, fetch_in_flight](OptRef obj) { - obj->is_backoff_enabled_ = stop_backoff.value(); - obj->fetch_in_flight_ = fetch_in_flight; - }); - if (stop_backoff.value()) { - remote_fetch_disallow_timer_->disableTimer(); - remote_fetch_backoff_strategy_->reset(); - } else { - remote_fetch_disallow_timer_->enableTimer( - std::chrono::milliseconds(remote_fetch_backoff_strategy_->nextBackOffMs())); - ENVOY_LOG(debug, "JWKS fetch backoff triggered due to fetch/verification failure."); - } - } else { - tls_.runOnAllThreads([fetch_in_flight](OptRef obj) { - obj->fetch_in_flight_ = fetch_in_flight; - }); - } - } - private: struct ThreadLocalCache : public ThreadLocal::ThreadLocalObject { ThreadLocalCache(bool enable_jwt_cache, const envoy::extensions::filters::http::jwt_authn::v3::JwtCacheConfig& config, - TimeSource& time_source, bool fetch_in_flight, bool backoff_enabled) - : jwt_cache_(JwtCache::create(enable_jwt_cache, config, time_source)), - fetch_in_flight_(fetch_in_flight), is_backoff_enabled_(backoff_enabled) {} + TimeSource& time_source) + : jwt_cache_(JwtCache::create(enable_jwt_cache, config, time_source)) {} // The jwks object. JwksConstSharedPtr jwks_; @@ -225,13 +168,11 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable obj) { obj->jwks_ = shared_jwks; obj->expire_ = std::chrono::steady_clock::time_point::max(); @@ -250,13 +191,6 @@ class JwksDataImpl : public JwksCache::JwksData, public Logger::Loggable> sub_matcher_; absl::optional max_exp_; - - // For exponential backoff, when fetch/verify fails on KID mismatch. - Random::RandomGenerator& random_; - JitteredExponentialBackOffStrategyPtr remote_fetch_backoff_strategy_; - Event::TimerPtr remote_fetch_disallow_timer_; - - Event::Dispatcher& dispatcher_; }; using JwksDataImplPtr = std::unique_ptr; diff --git a/source/extensions/filters/http/jwt_authn/jwks_cache.h b/source/extensions/filters/http/jwt_authn/jwks_cache.h index 17ef92b4d008..9cd0aac6926a 100644 --- a/source/extensions/filters/http/jwt_authn/jwks_cache.h +++ b/source/extensions/filters/http/jwt_authn/jwks_cache.h @@ -4,11 +4,9 @@ #include "envoy/api/api.h" #include "envoy/common/pure.h" -#include "envoy/common/random_generator.h" #include "envoy/common/time.h" #include "envoy/extensions/filters/http/jwt_authn/v3/config.pb.h" -#include "source/common/config/utility.h" #include "source/extensions/filters/http/common/jwks_fetcher.h" #include "source/extensions/filters/http/jwt_authn/jwks_async_fetcher.h" #include "source/extensions/filters/http/jwt_authn/jwt_cache.h" @@ -75,18 +73,10 @@ class JwksCache { virtual bool isExpired() const PURE; // Set a remote Jwks. - virtual const ::google::jwt_verify::Jwks* setRemoteJwks(JwksConstPtr&& jwks, - bool update_all_threads) PURE; + virtual const ::google::jwt_verify::Jwks* setRemoteJwks(JwksConstPtr&& jwks) PURE; // Get Token Cache. virtual JwtCache& getJwtCache() PURE; - - // Return true if fetch is allowed. - virtual bool isRemoteJwksFetchAllowed() PURE; - - // Update to allow fetches. - virtual void allowRemoteJwksFetch(absl::optional fetch_succeeded, - bool fetch_in_flight) PURE; }; // If there is only one provider in the config, return the data for that provider. diff --git a/test/extensions/filters/http/jwt_authn/authenticator_test.cc b/test/extensions/filters/http/jwt_authn/authenticator_test.cc index 87311fdecffb..0202d588358b 100644 --- a/test/extensions/filters/http/jwt_authn/authenticator_test.cc +++ b/test/extensions/filters/http/jwt_authn/authenticator_test.cc @@ -53,7 +53,7 @@ class AuthenticatorTest : public testing::Test { check_audience, provider, allow_failed, allow_missing, filter_config_->getJwksCache(), filter_config_->cm(), [this](Upstream::ClusterManager&, const RemoteJwks&) { return std::move(fetcher_); }, - filter_config_->timeSource(), filter_config_->dispatcher()); + filter_config_->timeSource()); jwks_ = Jwks::createFrom(PublicKey, Jwks::JWKS); EXPECT_TRUE(jwks_->getStatus() == Status::Ok); } @@ -178,124 +178,6 @@ TEST_F(AuthenticatorTest, TestOkJWTandCache) { EXPECT_EQ(0U, filter_config_->stats().jwks_fetch_failed_.value()); } -// Test to validate refetching JWKS when KID of JWT to be verified -// does not match cached JWKS's KID. `PublicKey` contains KID 1 and 2, -// which will validate both `GoodTokenWithKid1` and `GoodTokenWithKid2`, -// but will fail `GoodTokenWithKid3` which will trigger a JWKS refetch. -TEST_F(AuthenticatorTest, TestRefetchingJwksWithMultipleKidJwks) { - (*proto_config_.mutable_providers())[std::string(ProviderName)] - .mutable_remote_jwks() - ->set_refetch_jwks_on_kid_mismatch(true); - createAuthenticator(); - - // JWT signed by KID1. Authenticator fetches the first JWKS, PublicKey that contains both KID1 - // and KID2. - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - Http::TestRequestHeaderMapImpl headers{ - {"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}}; - expectVerifyStatus(Status::Ok, headers); - jwks_ = Jwks::createFrom(PublicKey, Jwks::JWKS); - - // JWT signed by KID2. Since cached JWKS already contains KID2, no refetching would be done. - EXPECT_CALL(*raw_fetcher_, fetch(_, _)).Times(0); - headers = - Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}}; - expectVerifyStatus(Status::Ok, headers); - - // JWT signed by KID3. Since cached JWKS doesn't contain KID3, refetching would be done. - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - headers = - Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodTokenWithKid3)}}; - expectVerifyStatus(Status::JwtVerificationFail, headers); -} - -// Test to validate refetching JWKS and backoff when `PublicKey` is split based on KIDs -TEST_F(AuthenticatorTest, TestRefetchingJwksAndBackoffWithSingleKidJwks) { - (*proto_config_.mutable_providers())[std::string(ProviderName)] - .mutable_remote_jwks() - ->set_refetch_jwks_on_kid_mismatch(true); - createAuthenticator(); - - // JWT signed by KID1. Authenticator fetches the first JWKS, PublicKey1 that contains KID1. - jwks_ = Jwks::createFrom(PublicKey1, Jwks::JWKS); - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - Http::TestRequestHeaderMapImpl headers{ - {"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}}; - expectVerifyStatus(Status::Ok, headers); - - // JWT signed by KID2. Authenticator refetches the current JWKS, PublicKey1 that fails target JWT. - jwks_ = Jwks::createFrom(PublicKey1, Jwks::JWKS); - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - headers = - Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}}; - expectVerifyStatus(Status::JwksKidAlgMismatch, headers); - - // Backoff triggered due to previous failed JWKS validation causing no fetch. - jwks_ = Jwks::createFrom(PublicKey2, Jwks::JWKS); - EXPECT_CALL(*raw_fetcher_, fetch(_, _)).Times(0); - headers = - Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}}; - expectVerifyStatus(Status::JwksKidAlgMismatch, headers); - - // Successful JWKS validation to end backoff. - headers = - Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}}; - expectVerifyStatus(Status::Ok, headers); - - // Attempt refetching KID2 due to backoff completion. - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - headers = - Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}}; - expectVerifyStatus(Status::Ok, headers); -} - -// Test to validate refetching JWKS and backoff when JWT contains no KID. -TEST_F(AuthenticatorTest, TestRefetchingJwksAndBackoffWithNoKidJwt) { - (*proto_config_.mutable_providers())[std::string(ProviderName)] - .mutable_remote_jwks() - ->set_refetch_jwks_on_kid_mismatch(true); - createAuthenticator(); - - jwks_ = Jwks::createFrom(PublicKey1, Jwks::JWKS); - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - Http::TestRequestHeaderMapImpl headers{ - {"Authorization", "Bearer " + std::string(GoodToken)}}; // Has No KID - expectVerifyStatus(Status::Ok, headers); - jwks_ = Jwks::createFrom(PublicKey2, Jwks::JWKS); - - // Test with JWT that has no KID, which would trigger a refetch. - EXPECT_CALL(*raw_fetcher_, fetch(_, _)) - .WillOnce(Invoke([this](Tracing::Span&, JwksFetcher::JwksReceiver& receiver) { - receiver.onJwksSuccess(std::move(jwks_)); - })); - headers = Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; - expectVerifyStatus(Status::Ok, headers); - - // Backoff triggered, hence no fetch. - EXPECT_CALL(*raw_fetcher_, fetch(_, _)).Times(0); - headers = Http::TestRequestHeaderMapImpl{{"Authorization", "Bearer " + std::string(GoodToken)}}; - // Verification done against existing JWKS cache due to active backoff. - expectVerifyStatus(Status::Ok, headers); -} - TEST_F(AuthenticatorTest, TestCompletePaddingInJwtPayload) { (*proto_config_.mutable_providers())[std::string(ProviderName)].set_pad_forward_payload_header( true); @@ -1212,7 +1094,7 @@ class AuthenticatorJwtCacheTest : public testing::Test { void createAuthenticator(const absl::optional& provider) { auth_ = Authenticator::create(nullptr, provider, false, false, jwks_cache_, cm_, - mock_fetcher_.AsStdFunction(), time_system_, dispatcher_); + mock_fetcher_.AsStdFunction(), time_system_); } void expectVerifyStatus(Status expected_status, Http::RequestHeaderMap& headers) { @@ -1239,7 +1121,6 @@ class AuthenticatorJwtCacheTest : public testing::Test { NiceMock parent_span_; std::string out_name_; ProtobufWkt::Struct out_extracted_data_; - Event::MockDispatcher dispatcher_; }; TEST_F(AuthenticatorJwtCacheTest, TestNonProvider) { diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc index 8dd7abbaa4da..1203f4c00868 100644 --- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc +++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc @@ -20,8 +20,7 @@ namespace JwtAuthn { namespace { std::string getAuthFilterConfig(const std::string& config_str, bool use_local_jwks, - bool strip_failure_response, bool refetch_jwks_on_kid_mismatch, - bool clear_route_cache = false) { + bool strip_failure_response, bool clear_route_cache = false) { JwtAuthentication proto_config; TestUtility::loadFromYaml(config_str, proto_config); proto_config.set_strip_failure_response(strip_failure_response); @@ -31,9 +30,6 @@ std::string getAuthFilterConfig(const std::string& config_str, bool use_local_jw provider0.clear_remote_jwks(); auto local_jwks = provider0.mutable_local_jwks(); local_jwks->set_inline_string(PublicKey); - } else if (refetch_jwks_on_kid_mismatch) { - auto& provider0 = (*proto_config.mutable_providers())[std::string(ProviderName)]; - provider0.mutable_remote_jwks()->set_refetch_jwks_on_kid_mismatch(refetch_jwks_on_kid_mismatch); } if (clear_route_cache) { @@ -48,15 +44,13 @@ std::string getAuthFilterConfig(const std::string& config_str, bool use_local_jw } std::string getAsyncFetchFilterConfig(const std::string& config_str, bool fast_listener, - bool strip_failure_response, - bool refetch_jwks_on_kid_mismatch) { + bool strip_failure_response) { JwtAuthentication proto_config; TestUtility::loadFromYaml(config_str, proto_config); proto_config.set_strip_failure_response(strip_failure_response); auto& provider0 = (*proto_config.mutable_providers())[std::string(ProviderName)]; auto* async_fetch = provider0.mutable_remote_jwks()->mutable_async_fetch(); - provider0.mutable_remote_jwks()->set_refetch_jwks_on_kid_mismatch(refetch_jwks_on_kid_mismatch); async_fetch->set_fast_listener(fast_listener); // Set failed_refetch_duration to a big value to disable failed refetch. // as it interferes the two FailedAsyncFetch integration tests. @@ -69,9 +63,9 @@ std::string getAsyncFetchFilterConfig(const std::string& config_str, bool fast_l } std::string getFilterConfig(bool use_local_jwks, bool strip_failure_response, - bool refetch_jwks_on_kid_mismatch, bool clear_route_cache = false) { + bool clear_route_cache = false) { return getAuthFilterConfig(ExampleConfig, use_local_jwks, strip_failure_response, - refetch_jwks_on_kid_mismatch, clear_route_cache); + clear_route_cache); } class LocalJwksIntegrationTest : public HttpProtocolIntegrationTest {}; @@ -84,7 +78,7 @@ INSTANTIATE_TEST_SUITE_P( // With local Jwks, this test verifies a request is passed with a good Jwt token. TEST_P(LocalJwksIntegrationTest, WithGoodToken) { - config_helper_.prependFilter(getFilterConfig(true, false, false)); + config_helper_.prependFilter(getFilterConfig(true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -112,7 +106,7 @@ TEST_P(LocalJwksIntegrationTest, WithGoodToken) { // With local Jwks, this test verifies a request is rejected with an expired Jwt token. TEST_P(LocalJwksIntegrationTest, ExpiredToken) { - config_helper_.prependFilter(getFilterConfig(true, false, false)); + config_helper_.prependFilter(getFilterConfig(true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -135,7 +129,7 @@ TEST_P(LocalJwksIntegrationTest, ExpiredToken) { } TEST_P(LocalJwksIntegrationTest, MissingToken) { - config_helper_.prependFilter(getFilterConfig(true, false, false)); + config_helper_.prependFilter(getFilterConfig(true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -156,7 +150,7 @@ TEST_P(LocalJwksIntegrationTest, MissingToken) { } TEST_P(LocalJwksIntegrationTest, ExpiredTokenHeadReply) { - config_helper_.prependFilter(getFilterConfig(true, false, false)); + config_helper_.prependFilter(getFilterConfig(true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -183,7 +177,7 @@ TEST_P(LocalJwksIntegrationTest, ExpiredTokenHeadReply) { // With local Jwks, this test verifies a request is rejected with an expired Jwt token // with only a 401 status without WWWAuthenticate/Body set with error details TEST_P(LocalJwksIntegrationTest, ExpiredTokenWithStripFailureResponse) { - config_helper_.prependFilter(getFilterConfig(true, true, false)); + config_helper_.prependFilter(getFilterConfig(true, true)); initialize(); @@ -218,7 +212,7 @@ TEST_P(LocalJwksIntegrationTest, ExpiredTokenWithStripFailureResponse) { // This test verifies a request is passed with a path that don't match any requirements. TEST_P(LocalJwksIntegrationTest, NoRequiresPath) { - config_helper_.prependFilter(getFilterConfig(true, false, false)); + config_helper_.prependFilter(getFilterConfig(true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -240,7 +234,7 @@ TEST_P(LocalJwksIntegrationTest, NoRequiresPath) { // This test verifies a CORS preflight request without JWT token is allowed. TEST_P(LocalJwksIntegrationTest, CorsPreflight) { - config_helper_.prependFilter(getFilterConfig(true, false, false)); + config_helper_.prependFilter(getFilterConfig(true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -288,7 +282,7 @@ TEST_P(LocalJwksIntegrationTest, FilterStateRequirement) { provider_name: example_provider )"; - config_helper_.prependFilter(getAuthFilterConfig(auth_filter_conf, true, false, false, false)); + config_helper_.prependFilter(getAuthFilterConfig(auth_filter_conf, true, false)); config_helper_.prependFilter(R"( name: header-to-filter-state typed_config: @@ -362,7 +356,7 @@ TEST_P(LocalJwksIntegrationTest, FilterStateRequirement) { // Verify that JWT config with RegEx matcher can handle CONNECT requests. TEST_P(LocalJwksIntegrationTest, ConnectRequestWithRegExMatch) { - config_helper_.prependFilter(getAuthFilterConfig(ExampleConfigWithRegEx, true, false, false)); + config_helper_.prependFilter(getAuthFilterConfig(ExampleConfigWithRegEx, true, false)); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -391,10 +385,8 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { addFakeUpstream(GetParam().upstream_protocol); } - void initializeFilter(bool add_cluster, bool refetch_jwks_on_kid_mismatch, - bool clear_route_cache = false) { - config_helper_.prependFilter( - getFilterConfig(false, false, refetch_jwks_on_kid_mismatch, clear_route_cache)); + void initializeFilter(bool add_cluster, bool clear_route_cache = false) { + config_helper_.prependFilter(getFilterConfig(false, false, clear_route_cache)); if (add_cluster) { config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { @@ -420,9 +412,8 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { initialize(); } - void initializeAsyncFetchFilter(bool fast_listener, bool refetch_jwks_on_kid_mismatch) { - config_helper_.prependFilter(getAsyncFetchFilterConfig(ExampleConfig, fast_listener, false, - refetch_jwks_on_kid_mismatch)); + void initializeAsyncFetchFilter(bool fast_listener) { + config_helper_.prependFilter(getAsyncFetchFilterConfig(ExampleConfig, fast_listener, false)); config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* jwks_cluster = bootstrap.mutable_static_resources()->add_clusters(); @@ -446,12 +437,6 @@ class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest { jwks_request_->encodeHeaders(response_headers, false); Buffer::OwnedImpl response_data1(jwks_body); jwks_request_->encodeData(response_data1, true); - - // Reusing connections causes assertions above to fail during multiple fetches in same test. - result = fake_jwks_connection_->close(); - RELEASE_ASSERT(result, result.message()); - result = fake_jwks_connection_->waitForDisconnect(); - RELEASE_ASSERT(result, result.message()); } void cleanup() { @@ -482,7 +467,7 @@ INSTANTIATE_TEST_SUITE_P( // With remote Jwks, this test verifies a request is passed with a good Jwt token // and a good public key fetched from a remote server. TEST_P(RemoteJwksIntegrationTest, WithGoodToken) { - initializeFilter(/*add_cluster=*/true, /*refetch_jwks_on_kid_mismatch=*/false); + initializeFilter(/*add_cluster=*/true); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -515,7 +500,7 @@ TEST_P(RemoteJwksIntegrationTest, WithGoodToken) { } TEST_P(RemoteJwksIntegrationTest, WithGoodTokenClearRouteCache) { - initializeFilter(/*add_cluster=*/true, /*refetch_jwks_on_kid_mismatch=*/false, true); + initializeFilter(/*add_cluster=*/true, true); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -540,7 +525,7 @@ TEST_P(RemoteJwksIntegrationTest, WithGoodTokenClearRouteCache) { // With remote Jwks, this test verifies a request is rejected even with a good Jwt token // when the remote jwks server replied with 500. TEST_P(RemoteJwksIntegrationTest, FetchFailedJwks) { - initializeFilter(/*add_cluster=*/true, /*refetch_jwks_on_kid_mismatch=*/false); + initializeFilter(/*add_cluster=*/true); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -566,7 +551,7 @@ TEST_P(RemoteJwksIntegrationTest, FetchFailedJwks) { } TEST_P(RemoteJwksIntegrationTest, FetchFailedMissingCluster) { - initializeFilter(/*add_cluster=*/false, /*refetch_jwks_on_kid_mismatch=*/false); + initializeFilter(/*add_cluster=*/false); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -589,7 +574,7 @@ TEST_P(RemoteJwksIntegrationTest, FetchFailedMissingCluster) { TEST_P(RemoteJwksIntegrationTest, WithGoodTokenAsyncFetch) { on_server_init_function_ = [this]() { waitForJwksResponse("200", PublicKey); }; - initializeAsyncFetchFilter(false, false); + initializeAsyncFetchFilter(false); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -619,178 +604,9 @@ TEST_P(RemoteJwksIntegrationTest, WithGoodTokenAsyncFetch) { cleanup(); } -// Test to validate refetching of JWKS during KID mismatch with async fetch enabled. -TEST_P(RemoteJwksIntegrationTest, RefetchJwksWithDifferentKidJwtsAsyncFetch) { - on_server_init_function_ = [this]() { waitForJwksResponse("200", PublicKey1); }; - initializeAsyncFetchFilter(true, true); - codec_client_ = makeHttpConnection(lookupPort("http")); - // JWKS is already fetched with fast listener. - test_server_->waitForCounterGe("http.config_test.jwt_authn.jwks_fetch_success", 1); - - auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}, - }); - - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); - - // Request with JWT containing new KID that should trigger a refetch. - auto response2 = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}, - }); - - waitForJwksResponse("200", PublicKey2); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 2); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response2->waitForEndStream()); - ASSERT_TRUE(response2->complete()); - EXPECT_EQ("200", response2->headers().getStatusValue()); - cleanup(); -} - -// Test to validate refetching of JWKS during KID mismatch with async fetch disabled. -TEST_P(RemoteJwksIntegrationTest, RefetchJwksWithDifferentKidJwts) { - initializeFilter(/*add_cluster=*/true, /*refetch_jwks_on_kid_mismatch=*/true); - codec_client_ = makeHttpConnection(lookupPort("http")); - - auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}, - }); - - waitForJwksResponse("200", PublicKey1); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 1); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); - - // Request with JWT containing new KID that should trigger a refetch. - auto response2 = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}, - }); - - waitForJwksResponse("200", PublicKey2); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 2); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response2->waitForEndStream()); - ASSERT_TRUE(response2->complete()); - EXPECT_EQ("200", response2->headers().getStatusValue()); - - cleanup(); -} - -// Test to validate failed verification despite a refetch. -TEST_P(RemoteJwksIntegrationTest, RefetchJwksWithDifferentKidJwtsFailedVerify) { - initializeFilter(/*add_cluster=*/true, /*refetch_jwks_on_kid_mismatch=*/true); - codec_client_ = makeHttpConnection(lookupPort("http")); - - auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}, - }); - - waitForJwksResponse("200", PublicKey1); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 1); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); - - // Request with JWT containing no KID that should trigger a refetch. - auto response2 = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid2)}, - }); - - // Since JWKS retrieved still contains only KID1, this will cause a verification failure. - waitForJwksResponse("200", PublicKey1); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 2); - - ASSERT_TRUE(response2->waitForEndStream()); - ASSERT_TRUE(response2->complete()); - EXPECT_EQ("401", response2->headers().getStatusValue()); - - cleanup(); -} - -// Test to validate refetching of JWKS due to no KID being present. -TEST_P(RemoteJwksIntegrationTest, RefetchJwksWithNoKidJwt) { - initializeFilter(/*add_cluster=*/true, /*refetch_jwks_on_kid_mismatch=*/true); - codec_client_ = makeHttpConnection(lookupPort("http")); - - auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodTokenWithKid1)}, - }); - - waitForJwksResponse("200", PublicKey1); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 1); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); - - // Request containing JWT with no KID - response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/"}, - {":scheme", "http"}, - {":authority", "host"}, - {"Authorization", "Bearer " + std::string(GoodToken)}, - }); - - waitForJwksResponse("200", PublicKey1); - test_server_->waitForCounterEq("http.config_test.jwt_authn.jwks_fetch_success", 2); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - ASSERT_TRUE(response->waitForEndStream()); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); -} - TEST_P(RemoteJwksIntegrationTest, WithGoodTokenAsyncFetchFast) { on_server_init_function_ = [this]() { waitForJwksResponse("200", PublicKey); }; - initializeAsyncFetchFilter(true, false); + initializeAsyncFetchFilter(true); // This test is only expecting one jwks fetch, but there is a race condition in the test: // In fast fetch mode, the listener is activated without waiting for jwks fetch to be @@ -831,7 +647,7 @@ TEST_P(RemoteJwksIntegrationTest, WithGoodTokenAsyncFetchFast) { TEST_P(RemoteJwksIntegrationTest, WithFailedJwksAsyncFetch) { on_server_init_function_ = [this]() { waitForJwksResponse("500", ""); }; - initializeAsyncFetchFilter(false, false); + initializeAsyncFetchFilter(false); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -852,7 +668,7 @@ TEST_P(RemoteJwksIntegrationTest, WithFailedJwksAsyncFetch) { TEST_P(RemoteJwksIntegrationTest, WithFailedJwksAsyncFetchFast) { on_server_init_function_ = [this]() { waitForJwksResponse("500", ""); }; - initializeAsyncFetchFilter(true, false); + initializeAsyncFetchFilter(true); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -874,7 +690,7 @@ TEST_P(RemoteJwksIntegrationTest, WithFailedJwksAsyncFetchFast) { class PerRouteIntegrationTest : public HttpProtocolIntegrationTest { public: void setup(const std::string& filter_config, const PerRouteConfig& per_route) { - config_helper_.prependFilter(getAuthFilterConfig(filter_config, true, false, false)); + config_helper_.prependFilter(getAuthFilterConfig(filter_config, true, false)); config_helper_.addConfigModifier( [per_route]( diff --git a/test/extensions/filters/http/jwt_authn/jwks_async_fetcher_test.cc b/test/extensions/filters/http/jwt_authn/jwks_async_fetcher_test.cc index 48593ba0a482..1b82d0996160 100644 --- a/test/extensions/filters/http/jwt_authn/jwks_async_fetcher_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwks_async_fetcher_test.cc @@ -64,16 +64,11 @@ class JwksAsyncFetcherTest : public testing::TestWithParam { [this](Upstream::ClusterManager&, const RemoteJwks&) { return std::make_unique( [this](Common::JwksFetcher::JwksReceiver& receiver) { - if (fetch_allowed) { - fetch_receiver_array_.push_back(&receiver); - } + fetch_receiver_array_.push_back(&receiver); }); }, stats_, - [this](google::jwt_verify::JwksPtr&& jwks) { out_jwks_array_.push_back(std::move(jwks)); }, - // Simulating the isRemoteJwksFetchAllowed and allowRemoteJwksFetch callbacks. - [this]() -> bool { return fetch_allowed; }, - [this](absl::optional, bool fetch_allowed_) { fetch_allowed = fetch_allowed_; }); + [this](google::jwt_verify::JwksPtr&& jwks) { out_jwks_array_.push_back(std::move(jwks)); }); if (initManagerUsed()) { init_target_handle_->initialize(init_watcher_); @@ -90,7 +85,6 @@ class JwksAsyncFetcherTest : public testing::TestWithParam { Init::TargetHandlePtr init_target_handle_; NiceMock init_watcher_; Event::MockTimer* timer_{}; - bool fetch_allowed{true}; }; INSTANTIATE_TEST_SUITE_P(JwksAsyncFetcherTest, JwksAsyncFetcherTest, @@ -213,44 +207,6 @@ TEST_P(JwksAsyncFetcherTest, TestGoodFetchAndRefresh) { EXPECT_EQ(0U, stats_.jwks_fetch_failed_.value()); } -TEST_P(JwksAsyncFetcherTest, TestBackoffDuringAsyncFetch) { - const char config[] = R"( - http_uri: - uri: https://pubkey_server/pubkey_path - cluster: pubkey_cluster - async_fetch: {} - refetch_jwks_on_kid_mismatch: true -)"; - - setupAsyncFetcher(config); - - // Initial fetch is successful. - EXPECT_EQ(fetch_receiver_array_.size(), 1); - auto jwks = google::jwt_verify::Jwks::createFrom(PublicKey, google::jwt_verify::Jwks::JWKS); - fetch_receiver_array_[0]->onJwksSuccess(std::move(jwks)); - - // Output 1 jwks. - EXPECT_EQ(out_jwks_array_.size(), 1); - - // Disallow fetch. - fetch_allowed = false; - timer_->invokeCallback(); - - // No fetch is done since fetch is disallowed. - EXPECT_EQ(fetch_receiver_array_.size(), 1); - EXPECT_EQ(out_jwks_array_.size(), 1); - - // Re-allow fetch. - fetch_allowed = true; - timer_->invokeCallback(); - - // New fetch is successful since fetch is now allowed. - EXPECT_EQ(fetch_receiver_array_.size(), 2); - jwks = google::jwt_verify::Jwks::createFrom(PublicKey, google::jwt_verify::Jwks::JWKS); - fetch_receiver_array_[1]->onJwksSuccess(std::move(jwks)); - EXPECT_EQ(out_jwks_array_.size(), 2); -} - TEST_P(JwksAsyncFetcherTest, TestNetworkFailureFetchWithDefaultRefetch) { const char config[] = R"( http_uri: diff --git a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc index db2b78472317..0b7786736374 100644 --- a/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc +++ b/test/extensions/filters/http/jwt_authn/jwks_cache_test.cc @@ -98,7 +98,7 @@ TEST_F(JwksCacheTest, TestSetRemoteJwks) { auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); - EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_), false)->getStatus(), Status::Ok); + EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_))->getStatus(), Status::Ok); EXPECT_FALSE(jwks->getJwksObj() == nullptr); EXPECT_FALSE(jwks->isExpired()); @@ -107,41 +107,6 @@ TEST_F(JwksCacheTest, TestSetRemoteJwks) { EXPECT_TRUE(jwks->isExpired()); } -// Test setRemoteJwks to all threads -TEST_F(JwksCacheTest, TestSetRemoteJwksToAllThreads) { - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); - - auto jwks = cache_->findByIssuer("https://example.com"); - EXPECT_TRUE(jwks->getJwksObj() == nullptr); - - EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_), true)->getStatus(), Status::Ok); - EXPECT_FALSE(jwks->getJwksObj() == nullptr); -} - -// Test allowing/disallowing remote fetch. -TEST_F(JwksCacheTest, TestRemoteFetchAllowed) { - cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_); - auto jwks = cache_->findByIssuer("https://example.com"); - - // Fetch should be allowed by default. - EXPECT_TRUE(jwks->isRemoteJwksFetchAllowed()); - - // Start backoff which should disallow fetch. - jwks->allowRemoteJwksFetch(true, false); - EXPECT_FALSE(jwks->isRemoteJwksFetchAllowed()); - // Stop backoff which should allow fetch. - jwks->allowRemoteJwksFetch(false, false); - EXPECT_TRUE(jwks->isRemoteJwksFetchAllowed()); - - // Fetch in flight. - jwks->allowRemoteJwksFetch(false, true); - EXPECT_FALSE(jwks->isRemoteJwksFetchAllowed()); - - // Fetch in flight completed, which should allow further fetches. - jwks->allowRemoteJwksFetch(false, false); - EXPECT_TRUE(jwks->isRemoteJwksFetchAllowed()); -} - // Test setRemoteJwks and use default cache duration. TEST_F(JwksCacheTest, TestSetRemoteJwksWithDefaultCacheDuration) { auto& provider0 = (*config_.mutable_providers())[std::string(ProviderName)]; @@ -152,7 +117,7 @@ TEST_F(JwksCacheTest, TestSetRemoteJwksWithDefaultCacheDuration) { auto jwks = cache_->findByIssuer("https://example.com"); EXPECT_TRUE(jwks->getJwksObj() == nullptr); - EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_), false)->getStatus(), Status::Ok); + EXPECT_EQ(jwks->setRemoteJwks(std::move(jwks_))->getStatus(), Status::Ok); EXPECT_FALSE(jwks->getJwksObj() == nullptr); EXPECT_FALSE(jwks->isExpired()); } diff --git a/test/extensions/filters/http/jwt_authn/mock.h b/test/extensions/filters/http/jwt_authn/mock.h index 6df52eb7eec5..aa963462b8e5 100644 --- a/test/extensions/filters/http/jwt_authn/mock.h +++ b/test/extensions/filters/http/jwt_authn/mock.h @@ -88,9 +88,7 @@ class MockJwksData : public JwksCache::JwksData { (), (const)); MOCK_METHOD(const ::google::jwt_verify::Jwks*, getJwksObj, (), (const)); MOCK_METHOD(bool, isExpired, (), (const)); - MOCK_METHOD(const ::google::jwt_verify::Jwks*, setRemoteJwks, (JwksConstPtr&&, bool), ()); - MOCK_METHOD(bool, isRemoteJwksFetchAllowed, (), ()); - MOCK_METHOD(void, allowRemoteJwksFetch, (absl::optional, bool), ()); + MOCK_METHOD(const ::google::jwt_verify::Jwks*, setRemoteJwks, (JwksConstPtr &&), ()); MOCK_METHOD(JwtCache&, getJwtCache, (), ()); envoy::extensions::filters::http::jwt_authn::v3::JwtProvider jwt_provider_; diff --git a/test/extensions/filters/http/jwt_authn/test_common.h b/test/extensions/filters/http/jwt_authn/test_common.h index 3b6795fc0aaf..aa9eb099a537 100644 --- a/test/extensions/filters/http/jwt_authn/test_common.h +++ b/test/extensions/filters/http/jwt_authn/test_common.h @@ -66,37 +66,6 @@ const char PublicKey[] = R"( } )"; -// Split the above public key into two keys -const char PublicKey1[] = R"( -{ - "keys": [ - { - "kty": "RSA", - "alg": "RS256", - "use": "sig", - "kid": "62a93512c9ee4c7f8067b5a216dade2763d32a47", - "n": "up97uqrF9MWOPaPkwSaBeuAPLOr9FKcaWGdVEGzQ4f3Zq5WKVZowx9TCBxmImNJ1qmUi13pB8otwM_l5lfY1AFBMxVbQCUXntLovhDaiSvYp4wGDjFzQiYA-pUq8h6MUZBnhleYrkU7XlCBwNVyN8qNMkpLA7KFZYz-486GnV2NIJJx_4BGa3HdKwQGxi2tjuQsQvao5W4xmSVaaEWopBwMy2QmlhSFQuPUpTaywTqUcUq_6SfAHhZ4IDa_FxEd2c2z8gFGtfst9cY3lRYf-c_ZdboY3mqN9Su3-j3z5r2SHWlhB_LNAjyWlBGsvbGPlTqDziYQwZN4aGsqVKQb9Vw", - "e": "AQAB" - }, - ] -} -)"; - -const char PublicKey2[] = R"( -{ - "keys": [ - { - "kty": "RSA", - "alg": "RS256", - "use": "sig", - "kid": "b3319a147514df7ee5e4bcdee51350cc890cc89e", - "n": "up97uqrF9MWOPaPkwSaBeuAPLOr9FKcaWGdVEGzQ4f3Zq5WKVZowx9TCBxmImNJ1qmUi13pB8otwM_l5lfY1AFBMxVbQCUXntLovhDaiSvYp4wGDjFzQiYA-pUq8h6MUZBnhleYrkU7XlCBwNVyN8qNMkpLA7KFZYz-486GnV2NIJJx_4BGa3HdKwQGxi2tjuQsQvao5W4xmSVaaEWopBwMy2QmlhSFQuPUpTaywTqUcUq_6SfAHhZ4IDa_FxEd2c2z8gFGtfst9cY3lRYf-c_ZdboY3mqN9Su3-j3z5r2SHWlhB_LNAjyWlBGsvbGPlTqDziYQwZN4aGsqVKQb9Vw", - "e": "AQAB" - }, - ] -} -)"; - // Provider config with various subject constraints const char SubjectConfig[] = R"( providers: @@ -185,9 +154,6 @@ const char ExampleConfig[] = R"( - example_service - http://example_service1 - https://example_service2/ - - example_service_with_kid1 - - example_service_with_kid2 - - example_service_with_kid3 remote_jwks: http_uri: uri: https://www.pubkey-server.com/pubkey-path @@ -414,43 +380,6 @@ const char OtherGoodToken[] = "drRvLcvlT5gB4adOIOlmhm8xtXgYpvqrXfmMJCHbP9no7JATFaTEAkmA3OOxDsaOju4BFgMtRZtDM8p12QQG0rFl_FE-" "2FqYX9qA4q41HJ4vxTSxgObeLGA"; -// { "iss": "https://example.com", "sub": "kid1@example.com", -// "kid": "62a93512c9ee4c7f8067b5a216dade2763d32a47", "exp": 2001001001, -// "aud": "example_service_with_kid1" } -const char GoodTokenWithKid1[] = - "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjYyYTkzNTEyYzllZTRjN2Y4MDY3YjVhMjE2ZGFkZTI3NjNkMz" - "JhNDcifQ.eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoia2lkMUBleGFtcGxlLmNvbSIsImV4cCI6MjAwM" - "TAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2aWNlX3dpdGhfa2lkMSJ9.m_ZdaiIep0LoWkWKDluwCsJK_MwqGWR24tkaSG" - "PbCVWLzO_tfDXDSPQrtpMbNKmopgqKXQcqEHfwy44dKzIaMfRnK4ICmxPmXXcqWFHfxJM1YO8UmPwwzKXssijGnQvrJ43y" - "Z0050NTWhmhGhkx7RYLfgk1qMsSWNVls5CYllke8MWF55Ns-4vFr2WdTmz06iMBMuPZI3E0v932tYqSMrBjbhmbT0WS-" - "a0dgqfpMDgQThBHK0MXFvDWjMqQ1YBiLT8Utdkswc0Mu2FPtjVUAXU5DfyselEe3isgPTnbNTPvGkGYJ-JvJTJRfUF5-" - "IMGxEBJX9yQ0EuA16HCH13eQKA"; - -// { "iss": "https://example.com", "sub": "kid2@example.com", -// "kid": "b3319a147514df7ee5e4bcdee51350cc890cc89e", "exp": 2001001001, -// "aud": "example_service_with_kid2" } -const char GoodTokenWithKid2[] = - "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImIzMzE5YTE0NzUxNGRmN2VlNWU0YmNkZWU1MTM1MGNjODkwY2" - "M4OWUifQ.eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoia2lkMkBleGFtcGxlLmNvbSIsImV4cCI6MjAwM" - "TAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2aWNlX3dpdGhfa2lkMiJ9.PpYWGuuxdTWdLAznYvA4DHBxa7BkgGMKvwp_" - "neb7mW9cKIRPz6hSVkSyI7hdgafeOIayGCEGWYYcs_UAS068Bd8oDI8p5iITGv0XyAer-zh-" - "jqNcNmNrdgUr3WkBSxWyeEPC9RYxaHamm115U0HA6DbvMJu3nm1ir3oKHVgq" - "-uSfhZQjtpXXKJU0v6ZEjJSaqyVt84ZDIwtpkDIHS7fjAT1U6pteVNdKyJy8MH7IMHzohY0b563kxI3PHubgPn8nJPRjzX" - "YnUBp3rc6N" - "iNhvYYQjXep_JFarh0aivlXrjtkhzoNb10raaIkboICp88KyH-seXc59we1naCMgRp_fag"; - -// { "iss": "https://example.com", "sub": "kid3@example.com", -// "kid": "6465fc44cf92ae01027d89ebb0e18e02aeb6dfff", "exp": 2001001001, "aud": -// "example_service_with_kid3" } -const char GoodTokenWithKid3[] = - "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjY0NjVmYzQ0Y2Y5MmFlMDEwMjdkODllYmIwZTE4ZTAyYWViNm" - "RmZmYifQ.eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoia2lkM0BleGFtcGxlLmNvbSIsImV4cCI6MjAwM" - "TAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2aWNlX3dpdGhfa2lkMyJ9.p3Pwx6kMYn46jXExHZyt5vnD-" - "NlIeA9XW3W8rJykUuPg5yQLMamm24PItH64PZUZgun7rhKxzsgZgAmUpmxB2b4pb5lhPJLHRQoJouzdU9Usr5H9GOt4a1p" - "77-6D7ocSX4iGMT4dqis4sncbMGgb5kVR_48iwD17kGDwCimH5mAdcoMyeuvR7c2xRaremyvqzS24CqyxrcxdRppyS-I5W" - "vNG4WtFaalNWUFVZNFtOjVuNlouo9IudLMtv7WFoJNOPRN-R-34Kj50Yk2" - "hXOQResUZwxsV6W83VleiWiOE2uNTWHTfJLVK1BtqVrSrxj7zv1NxVCW5M_X97Uq3ht6_JQ"; - //{ // "iss": "https://example.com", // "sub": "test@example.com", diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 3dbef85c4e3e..e2648218fdfe 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -90,8 +90,6 @@ gperf HEXDIG HEXDIGIT HPE -KID -KIDs Kwasi LRU Mensah @@ -258,7 +256,6 @@ JSON JSONs JWKS JWKs -JWKS's JWS JWT JWTs @@ -1199,7 +1196,6 @@ refcount referencee referer refetch -refetches reframed refvec regex