Creating JwksFetcher interface and impl v2

bazel/repositories.bzl

@@ -464,6 +464,10 @@ def _com_google_absl():
         name = "abseil_symbolize",
         actual = "@com_google_absl//absl/debugging:symbolize",
     )
+    native.bind(
+            name = "abseil_time",
+            actual = "@com_google_absl//absl/time:time",
+        )
 
 def _com_google_protobuf():
     _repository_impl("com_google_protobuf")

bazel/repository_locations.bzl

@@ -70,7 +70,7 @@ REPOSITORY_LOCATIONS = dict(
         remote = "https://github.com/google/googleapis",
     ),
     com_github_google_jwt_verify = dict(
-        commit = "4eb9e96485b71e00d43acc7207501caafb085b4a",
+        commit = "66792a057ec54e4b75c6a2eeda4e98220bd12a9a",
         remote = "https://github.com/google/jwt_verify_lib",
     ),
     com_github_nodejs_http_parser = dict(

source/extensions/filters/http/common/BUILD

@@ -23,3 +23,17 @@ envoy_cc_library(
         "//include/envoy/server:filter_config_interface",
     ],
 )
+
+envoy_cc_library(
+    name = "jwks_fetcher_lib",
+    srcs = ["jwks_fetcher.cc"],
+    hdrs = ["jwks_fetcher.h"],
+    external_deps = [
+        "jwt_verify_lib",
+    ],
+    deps = [
+        "//include/envoy/upstream:cluster_manager_interface",
+        "//source/common/http:utility_lib",
+        "@envoy_api//envoy/api/v2/core:http_uri_cc",
+    ],
+)

source/extensions/filters/http/common/jwks_fetcher.cc

@@ -0,0 +1,91 @@
+#include "extensions/filters/http/common/jwks_fetcher.h"
+
+#include "common/common/enum_to_int.h"
+#include "common/http/headers.h"
+#include "common/http/utility.h"
+
+#include "jwt_verify_lib/status.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace HttpFilters {
+namespace Common {
+namespace {
+class JwksFetcherImpl : public JwksFetcher,
+                        public Logger::Loggable<Logger::Id::filter>,
+                        public Http::AsyncClient::Callbacks {
+private:
+  Upstream::ClusterManager& cm_;
+  JwksFetcher::JwksReceiver* receiver_ = nullptr;
+  const ::envoy::api::v2::core::HttpUri* uri_ = nullptr;
+  Http::AsyncClient::Request* request_ = nullptr;
+
+public:
+  JwksFetcherImpl(Upstream::ClusterManager& cm) : cm_(cm) { ENVOY_LOG(trace, "{}", __func__); }
+
+  void close() {
+    if (request_) {
+      request_->cancel();
+      request_ = nullptr;
+      ENVOY_LOG(debug, "fetch pubkey [uri = {}]: canceled", uri_->uri());
+    }
+  }
+
+  void fetch(const ::envoy::api::v2::core::HttpUri& uri, JwksFetcher::JwksReceiver* receiver) {
+    ENVOY_LOG(trace, "{}", __func__);
+    receiver_ = receiver;
+    uri_ = &uri;
+    Http::MessagePtr message = Http::Utility::prepareHeaders(uri);
+    message->headers().insertMethod().value().setReference(Http::Headers::get().MethodValues.Get);
+    ENVOY_LOG(debug, "fetch pubkey from [uri = {}]: start", uri_->uri());
+    request_ =
+        cm_.httpAsyncClientForCluster(uri.cluster())
+            .send(std::move(message), *this,
+                  std::chrono::milliseconds(DurationUtil::durationToMilliseconds(uri.timeout())));
+  }
+
+  // HTTP async receive methods
+  void onSuccess(Http::MessagePtr&& response) {
+    ENVOY_LOG(trace, "{}", __func__);
+    request_ = nullptr;
+    const uint64_t status_code = Http::Utility::getResponseStatus(response->headers());
+    if (status_code == enumToInt(Http::Code::OK)) {
+      ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: success", __func__, uri_->uri());
+      if (response->body()) {
+        const auto len = response->body()->length();
+        const auto body = std::string(static_cast<char*>(response->body()->linearize(len)), len);
+        auto jwks =
+            google::jwt_verify::Jwks::createFrom(body, google::jwt_verify::Jwks::Type::JWKS);
+        if (jwks->getStatus() == google::jwt_verify::Status::Ok) {
+          ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: succeeded", __func__, uri_->uri());
+          receiver_->onJwksSuccess(std::move(jwks));
+        } else {
+          ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: invalid jwks", __func__, uri_->uri());
+          receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::invalid_jwks);
+        }
+      } else {
+        ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: body is empty", __func__, uri_->uri());
+        receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::network);
+      }
+    } else {
+      ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: response status code {}", __func__,
+                uri_->uri(), status_code);
+      receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::network);
+    }
+  }
+
+  void onFailure(Http::AsyncClient::FailureReason reason) {
+    ENVOY_LOG(debug, "{}: fetch pubkey [uri = {}]: network error {}", __func__, uri_->uri(),
+              enumToInt(reason));
+    receiver_->onJwksError(JwksFetcher::JwksReceiver::Failure::network);
+  }
+};
+} // namespace
+
+JwksFetcher::JwksFetcherPtr JwksFetcher::create(Upstream::ClusterManager& cm) {
+  return JwksFetcherPtr(new JwksFetcherImpl(cm));
+}
+} // namespace Common
+} // namespace HttpFilters
+} // namespace Extensions
+} // namespace Envoy

source/extensions/filters/http/common/jwks_fetcher.h

@@ -0,0 +1,63 @@
+#pragma once
+
+#include "envoy/api/v2/core/http_uri.pb.h"
+#include "envoy/common/pure.h"
+#include "envoy/upstream/cluster_manager.h"
+
+#include "jwt_verify_lib/jwks.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace HttpFilters {
+namespace Common {
+
+class JwksFetcher {
+public:
+  typedef std::unique_ptr<JwksFetcher> JwksFetcherPtr;
+
+  class JwksReceiver {
+  public:
+    enum class Failure {
+      unknown,
+      network,
+      invalid_jwks,
+    };
+
+    virtual ~JwksReceiver(){};
+    /*
+     * Successful retrieval callback.
+     * of the returned JWKS object.
+     * @param jwks the JWKS object retrieved.
+     */
+    virtual void onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) PURE;
+    /*
+     * Retrieval error callback.
+     * * @param reason the failure reason.
+     */
+    virtual void onJwksError(Failure reason) PURE;
+  };
+
+  virtual ~JwksFetcher(){};
+
+  /*
+   * Close/stop any inflight request.
+   */
+  virtual void close() PURE;
+
+  /*
+   * Retrieve a JWKS resource from a remote HTTP host.
+   * @param uri the uri to retrieve the jwks from.
+   */
+  virtual void fetch(const ::envoy::api::v2::core::HttpUri& uri, JwksReceiver* receiver) PURE;
+
+  /*
+   * Factory method for creating a JwksFetcher.
+   * @param cm the cluster manager to use during Jwks retrieval
+   * @return a JwksFetcher instance
+   */
+  static JwksFetcherPtr create(Upstream::ClusterManager& cm);
+};
+} // namespace Common
+} // namespace HttpFilters
+} // namespace Extensions
+} // namespace Envoy

source/extensions/filters/http/jwt_authn/BUILD

@@ -46,6 +46,7 @@ envoy_cc_library(
         "//include/envoy/server:filter_config_interface",
         "//include/envoy/stats:stats_macros",
         "//source/common/http:message_lib",
+        "//source/extensions/filters/http/common:jwks_fetcher_lib",
     ],
 )
 

source/extensions/filters/http/jwt_authn/authenticator.cc

@@ -24,29 +24,23 @@ namespace {
  */
 class AuthenticatorImpl : public Logger::Loggable<Logger::Id::filter>,
                           public Authenticator,
-                          public Http::AsyncClient::Callbacks {
+                          public Common::JwksFetcher::JwksReceiver {
 public:
-  AuthenticatorImpl(FilterConfigSharedPtr config) : config_(config) {}
+  AuthenticatorImpl(FilterConfigSharedPtr config, Common::JwksFetcher::JwksFetcherPtr& fetcher)
+      : config_(config), fetcher_(std::move(fetcher)) {}
 
+  // Following functions are for JwksFetcher::JwksReceiver interface
+  void onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) override;
+  void onJwksError(Failure reason) override;
   // Following functions are for Authenticator interface
   void verify(Http::HeaderMap& headers, Authenticator::Callbacks* callback) override;
   void onDestroy() override;
   void sanitizePayloadHeaders(Http::HeaderMap& headers) const override;
 
 private:
-  // Fetch a remote public key.
-  void fetchRemoteJwks();
-
-  // Following two functions are for AyncClient::Callbacks
-  void onSuccess(Http::MessagePtr&& response) override;
-  void onFailure(Http::AsyncClient::FailureReason) override;
-
   // Verify with a specific public key.
   void verifyKey();
 
-  // Handle the public key fetch done event.
-  void onFetchRemoteJwksDone(const std::string& jwks_str);
-
   // Calls the callback with status.
   void doneWithStatus(const Status& status);
 
@@ -56,6 +50,9 @@ class AuthenticatorImpl : public Logger::Loggable<Logger::Id::filter>,
   // The config object.
   FilterConfigSharedPtr config_;
 
+  // The Jwks fetcher object
+  Common::JwksFetcher::JwksFetcherPtr fetcher_;
+
   // The token data
   JwtLocationConstPtr token_;
   // The JWT object.
@@ -115,13 +112,19 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback
     return;
   }
 
+  // TODO: Cross-platform-wise the below unix_timestamp code is wrong as the
+  // epoch is not guaranteed to be defined as the unix epoch. We should use
+  // the abseil time functionality instead.
+  // TODO: We should use the jwt_verify_lib to check the validity of a JWT.
   // Check "exp" claim.
   const auto unix_timestamp = std::chrono::duration_cast<std::chrono::seconds>(
                                   std::chrono::system_clock::now().time_since_epoch())
                                   .count();
-  // NOTE: Service account tokens generally don't have an expiration time (due to being long lived)
-  // and defaulted to 0 by google::jwt_verify library but are still valid.
-  if (jwt_.exp_ > 0 && jwt_.exp_ < unix_timestamp) {
+  if (jwt_.nbf_ > unix_timestamp) {
+    doneWithStatus(Status::JwtNotYetValid);
+    return;
+  }
+  if (jwt_.exp && jwt_.exp_ < unix_timestamp) {
     doneWithStatus(Status::JwtExpired);
     return;
   }
@@ -137,7 +140,8 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback
     return;
   }
 
-  if (jwks_data_->getJwksObj() != nullptr && !jwks_data_->isExpired()) {
+  auto jwks_obj = jwks_data_->getJwksObj();
+  if (jwks_obj != nullptr && !jwks_data_->isExpired()) {
     verifyKey();
     return;
   }
@@ -146,72 +150,28 @@ void AuthenticatorImpl::verify(Http::HeaderMap& headers, Authenticator::Callback
   // If request 1 triggers a remote jwks fetching, but is not yet replied when the request 2
   // of using the same jwks comes. The request 2 will trigger another remote fetching for the
   // jwks. This can be optimized; the same remote jwks fetching can be shared by two requrests.
-  fetchRemoteJwks();
-}
-
-void AuthenticatorImpl::fetchRemoteJwks() {
-  const auto& http_uri = jwks_data_->getJwtProvider().remote_jwks().http_uri();
-
-  Http::MessagePtr message = Http::Utility::prepareHeaders(http_uri);
-  message->headers().insertMethod().value().setReference(Http::Headers::get().MethodValues.Get);
-
-  if (config_->cm().get(http_uri.cluster()) == nullptr) {
-    doneWithStatus(Status::JwksFetchFail);
-    return;
-  }
-
-  uri_ = http_uri.uri();
-  ENVOY_LOG(debug, "fetch pubkey from [uri = {}]: start", uri_);
-  request_ = config_->cm()
-                 .httpAsyncClientForCluster(http_uri.cluster())
-                 .send(std::move(message), *this,
-                       std::chrono::milliseconds(
-                           DurationUtil::durationToMilliseconds(http_uri.timeout())));
-}
-
-void AuthenticatorImpl::onSuccess(Http::MessagePtr&& response) {
-  request_ = nullptr;
-  const uint64_t status_code = Http::Utility::getResponseStatus(response->headers());
-  if (status_code == enumToInt(Http::Code::OK)) {
-    ENVOY_LOG(debug, "fetch pubkey [uri = {}]: success", uri_);
-    if (response->body()) {
-      const auto len = response->body()->length();
-      const auto body = std::string(static_cast<char*>(response->body()->linearize(len)), len);
-      onFetchRemoteJwksDone(body);
-      return;
-    } else {
-      ENVOY_LOG(debug, "fetch pubkey [uri = {}]: body is empty", uri_);
-    }
+  if (jwks_data_->getJwtProvider().has_remote_jwks()) {
+    fetcher_->fetch(jwks_data_->getJwtProvider().remote_jwks().http_uri(), this);
   } else {
-    ENVOY_LOG(debug, "fetch pubkey [uri = {}]: response status code {}", uri_, status_code);
-  }
-  doneWithStatus(Status::JwksFetchFail);
-}
-
-void AuthenticatorImpl::onFailure(Http::AsyncClient::FailureReason) {
-  request_ = nullptr;
-  ENVOY_LOG(debug, "fetch pubkey [uri = {}]: failed", uri_);
-  doneWithStatus(Status::JwksFetchFail);
-}
-
-void AuthenticatorImpl::onDestroy() {
-  if (request_ != nullptr) {
-    request_->cancel();
-    request_ = nullptr;
-    ENVOY_LOG(debug, "fetch pubkey [uri = {}]: canceled", uri_);
+    // No valid keys for this issuer. This may happen as a result of incorrect local
+    // JWKS configuration.
+    doneWithStatus(Status::JwksNoValidKeys);
   }
 }
 
-// Handle the public key fetch done event.
-void AuthenticatorImpl::onFetchRemoteJwksDone(const std::string& jwks_str) {
-  const Status status = jwks_data_->setRemoteJwks(jwks_str);
+void AuthenticatorImpl::onJwksSuccess(google::jwt_verify::JwksPtr&& jwks) {
+  const Status status = jwks_data_->setRemoteJwks(std::move(jwks))->getStatus();
   if (status != Status::Ok) {
     doneWithStatus(status);
   } else {
     verifyKey();
   }
 }
 
+void AuthenticatorImpl::onJwksError(Failure) { doneWithStatus(Status::JwksFetchFail); }
+
+void AuthenticatorImpl::onDestroy() { fetcher_->close(); }
+
 // Verify with a specific public key.
 void AuthenticatorImpl::verifyKey() {
   const Status status = ::google::jwt_verify::verifyJwt(jwt_, *jwks_data_->getJwksObj());
@@ -249,8 +209,9 @@ void AuthenticatorImpl::doneWithStatus(const Status& status) {
 
 } // namespace
 
-AuthenticatorPtr Authenticator::create(FilterConfigSharedPtr config) {
-  return std::make_unique<AuthenticatorImpl>(config);
+AuthenticatorPtr Authenticator::create(FilterConfigSharedPtr config,
+                                       Common::JwksFetcher::JwksFetcherPtr& fetcher) {
+  return std::make_unique<AuthenticatorImpl>(config, fetcher);
 }
 
 } // namespace JwtAuthn

source/extensions/filters/http/jwt_authn/authenticator.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "extensions/filters/http/common/jwks_fetcher.h"
 #include "extensions/filters/http/jwt_authn/filter_config.h"
 
 #include "jwt_verify_lib/status.h"
@@ -35,7 +36,8 @@ class Authenticator {
   virtual void sanitizePayloadHeaders(Http::HeaderMap& headers) const PURE;
 
   // Authenticator factory function.
-  static AuthenticatorPtr create(FilterConfigSharedPtr config);
+  static AuthenticatorPtr create(FilterConfigSharedPtr config,
+                                 Common::JwksFetcher::JwksFetcherPtr& fetcher);
 };
 
 } // namespace JwtAuthn