OAuth2: Add samesite attribute support for all OAuth2 supported cookie types (envoyproxy#37952)

Commit Message: OAuth2: Add samesite attribute support for all OAuth2
supported cookie types

Additional Description: The SameSite attribute offers three values to
control whether cookies are shared within the same site or across
different sites. It's an optional setting, with a "Disabled" option that
omits the SameSite attribute altogether. By default, this setting is
disabled to ensure no changes are made to existing deployments, but
operators now have the option to enable SameSite. The six cookies
supporting SameSite attribute are:

bearer_token_cookie
hmac_cookie
expires_cookie
id_token_cookie
refresh_token_cookie
nonce_cookie
The samesite attribute value allowed are:

Strict
Lax
None
Disabled (Default, if no value is set in config)
The operator can also optionally do not specify any SameSite attributes
for cookie. This will result DISABLED value to be set for all cookie's
SameSite attribute value. in this case no same site attribute will be
returned by filter.

The operator can also choose different same site attribute to be
configured by different cookies. This means the SameSite attributes for
different cookies listed above can be different. Also the operator can
optionally specify SameSite attribute for some cookie but miss it for
others. it is not mandatory to specify SameSite explicitly for all
cookies

Risk Level: Medium
Testing: unit
Docs Changes: proto is documented
Release Notes: changelog entry added

---------

Signed-off-by: Yueren Wang <yuerenwang@lyft.com>
Signed-off-by: Yueren Wang <yuerenwang@tm4wwrxcwd.tailbaa43.ts.net>
Co-authored-by: Yueren Wang <yuerenwang@tm4wwrxcwd.tailbaa43.ts.net>

Author: Yueren-Wang

api/envoy/extensions/filters/http/oauth2/v3/oauth.proto

@@ -25,6 +25,41 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#extension: envoy.filters.http.oauth2]
 //
 
+// OAuth cookie configuration attributes.
+//
+message CookieConfig {
+  enum SameSite {
+    DISABLED = 0;
+    STRICT = 1;
+    LAX = 2;
+    NONE = 3;
+  }
+
+  // The value used for the SameSite cookie attribute.
+  SameSite same_site = 1 [(validate.rules).enum = {defined_only: true}];
+}
+
+// [#next-free-field: 7]
+message CookieConfigs {
+  // Configuration for the bearer token cookie.
+  CookieConfig bearer_token_cookie_config = 1;
+
+  // Configuration for the OAuth HMAC cookie.
+  CookieConfig oauth_hmac_cookie_config = 2;
+
+  // Configuration for the OAuth expires cookie.
+  CookieConfig oauth_expires_cookie_config = 3;
+
+  // Configuration for the ID token cookie.
+  CookieConfig id_token_cookie_config = 4;
+
+  // Configuration for the refresh token cookie.
+  CookieConfig refresh_token_cookie_config = 5;
+
+  // Configuration for the OAuth nonce cookie.
+  CookieConfig oauth_nonce_cookie_config = 6;
+}
+
 // [#next-free-field: 6]
 message OAuth2Credentials {
   // [#next-free-field: 7]
@@ -84,7 +119,7 @@ message OAuth2Credentials {
 
 // OAuth config
 //
-// [#next-free-field: 21]
+// [#next-free-field: 22]
 message OAuth2Config {
   enum AuthType {
     // The ``client_id`` and ``client_secret`` will be sent in the URL encoded request body.
@@ -186,6 +221,9 @@ message OAuth2Config {
   // will still process incoming Refresh Tokens as part of the HMAC if they are there. This is to ensure compatibility while switching this setting on. Future
   // sessions would not set the Refresh Token cookie header.
   bool disable_refresh_token_set_cookie = 20;
+
+  // Controls for attributes that can be set on the cookies.
+  CookieConfigs cookie_configs = 21;
 }
 
 // Filter config.

changelogs/current.yaml

@@ -24,6 +24,13 @@ removed_config_or_runtime:
     Removed runtime flag ``envoy.reloadable_features.dns_details`` and legacy code paths.
 
 new_features:
+- area: oauth2
+  change: |
+    Add the option to specify SameSite cookie attribute values for oauth2 supported cookies.
+    To specify SameSite attribute, choose one of the values from ``strict``,``lax`` or ``none``. If not specified,
+    a default value of ``disabled`` will be assigned and there will be no SameSite value in the cookie attribute. See
+    :ref:`apply_on_stream_done <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Config.cookie_configs>`
+    for more details.
 - area: spiffe
   change: |
     Added :ref:`trust_bundles

source/extensions/filters/http/oauth2/filter.cc

@@ -41,7 +41,7 @@ Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::Request
 
 constexpr const char* CookieDeleteFormatString =
     "{}=deleted; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT";
-constexpr const char* CookieTailHttpOnlyFormatString = ";path=/;Max-Age={};secure;HttpOnly";
+constexpr const char* CookieTailHttpOnlyFormatString = ";path=/;Max-Age={};secure;HttpOnly{}";
 constexpr const char* CookieDomainFormatString = ";domain={}";
 
 constexpr absl::string_view UnauthorizedBodyMessage = "OAuth flow failed.";
@@ -60,6 +60,9 @@ constexpr absl::string_view REDIRECT_FOR_CREDENTIALS = "oauth.missing_credential
 constexpr absl::string_view SIGN_OUT = "oauth.sign_out";
 constexpr absl::string_view DEFAULT_AUTH_SCOPE = "user";
 
+constexpr absl::string_view SameSiteLax = ";SameSite=Lax";
+constexpr absl::string_view SameSiteStrict = ";SameSite=Strict";
+constexpr absl::string_view SameSiteNone = ";SameSite=None";
 constexpr absl::string_view HmacPayloadSeparator = "\n";
 
 template <class T>
@@ -129,6 +132,28 @@ getAuthType(envoy::extensions::filters::http::oauth2::v3::OAuth2Config_AuthType
   }
 }
 
+// Helper function to get SameSite attribute string from proto enum.
+std::string
+getSameSiteString(envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite same_site) {
+  switch (same_site) {
+    PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
+  case envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite::
+      CookieConfig_SameSite_STRICT:
+    return std::string(SameSiteStrict);
+  case envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite::
+      CookieConfig_SameSite_LAX:
+    return std::string(SameSiteLax);
+  case envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite::
+      CookieConfig_SameSite_NONE:
+    return std::string(SameSiteNone);
+  case envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite::
+      CookieConfig_SameSite_DISABLED:
+    return EMPTY_STRING;
+  }
+  IS_ENVOY_BUG("unexpected same_site enum value");
+  return EMPTY_STRING;
+}
+
 Http::Utility::QueryParamsMulti buildAutorizationQueryParams(
     const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config) {
   auto query_params =
@@ -246,7 +271,37 @@ FilterConfig::FilterConfig(
       use_refresh_token_(FilterConfig::shouldUseRefreshToken(proto_config)),
       disable_id_token_set_cookie_(proto_config.disable_id_token_set_cookie()),
       disable_access_token_set_cookie_(proto_config.disable_access_token_set_cookie()),
-      disable_refresh_token_set_cookie_(proto_config.disable_refresh_token_set_cookie()) {
+      disable_refresh_token_set_cookie_(proto_config.disable_refresh_token_set_cookie()),
+      bearer_token_cookie_settings_(
+          (proto_config.has_cookie_configs() &&
+           proto_config.cookie_configs().has_bearer_token_cookie_config())
+              ? CookieSettings(proto_config.cookie_configs().bearer_token_cookie_config())
+              : CookieSettings()),
+      hmac_cookie_settings_(
+          (proto_config.has_cookie_configs() &&
+           proto_config.cookie_configs().has_oauth_hmac_cookie_config())
+              ? CookieSettings(proto_config.cookie_configs().oauth_hmac_cookie_config())
+              : CookieSettings()),
+      expires_cookie_settings_(
+          (proto_config.has_cookie_configs() &&
+           proto_config.cookie_configs().has_oauth_expires_cookie_config())
+              ? CookieSettings(proto_config.cookie_configs().oauth_expires_cookie_config())
+              : CookieSettings()),
+      id_token_cookie_settings_(
+          (proto_config.has_cookie_configs() &&
+           proto_config.cookie_configs().has_id_token_cookie_config())
+              ? CookieSettings(proto_config.cookie_configs().id_token_cookie_config())
+              : CookieSettings()),
+      refresh_token_cookie_settings_(
+          (proto_config.has_cookie_configs() &&
+           proto_config.cookie_configs().has_refresh_token_cookie_config())
+              ? CookieSettings(proto_config.cookie_configs().refresh_token_cookie_config())
+              : CookieSettings()),
+      nonce_cookie_settings_(
+          (proto_config.has_cookie_configs() &&
+           proto_config.cookie_configs().has_oauth_nonce_cookie_config())
+              ? CookieSettings(proto_config.cookie_configs().oauth_nonce_cookie_config())
+              : CookieSettings()) {
   if (!context.clusterManager().clusters().hasCluster(oauth_token_endpoint_.cluster())) {
     throw EnvoyException(fmt::format("OAuth2 filter: unknown cluster '{}' in config. Please "
                                      "specify which cluster to direct OAuth requests to.",
@@ -544,8 +599,10 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) {
   if (!csrf_token_cookie_exists) {
     // Expire the CSRF token cookie in 10 minutes.
     // This should be enough time for the user to complete the OAuth flow.
-    std::string expire_in = std::to_string(10 * 60);
-    std::string cookie_tail_http_only = fmt::format(CookieTailHttpOnlyFormatString, expire_in);
+    std::string csrf_expires = std::to_string(10 * 60);
+    std::string same_site = getSameSiteString(config_->nonceCookieSettings().same_site_);
+    std::string cookie_tail_http_only =
+        fmt::format(CookieTailHttpOnlyFormatString, csrf_expires, same_site);
     if (!config_->cookieDomain().empty()) {
       cookie_tail_http_only = absl::StrCat(
           fmt::format(CookieDomainFormatString, config_->cookieDomain()), cookie_tail_http_only);
@@ -728,6 +785,43 @@ std::string OAuth2Filter::getExpiresTimeForIdToken(const std::string& id_token,
   return std::to_string(expires_in.count());
 }
 
+// Helper function to build the cookie tail string.
+std::string OAuth2Filter::BuildCookieTail(int cookie_type) const {
+  std::string same_site;
+  std::string expires_time = expires_in_;
+
+  switch (cookie_type) {
+    PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
+  case 1: // BEARER_TOKEN TYPE
+    same_site = getSameSiteString(config_->bearerTokenCookieSettings().same_site_);
+    break;
+  case 2: // OAUTH_HMAC TYPE
+    same_site = getSameSiteString(config_->hmacCookieSettings().same_site_);
+    break;
+  case 3: // OAUTH_EXPIRES TYPE
+    same_site = getSameSiteString(config_->expiresCookieSettings().same_site_);
+    break;
+  case 4: // ID_TOKEN TYPE
+    same_site = getSameSiteString(config_->idTokenCookieSettings().same_site_);
+    expires_time = expires_id_token_in_;
+    break;
+  case 5: // REFRESH_TOKEN TYPE
+    same_site = getSameSiteString(config_->refreshTokenCookieSettings().same_site_);
+    expires_time = expires_refresh_token_in_;
+    break;
+  case 6: // OAUTH_NONCE TYPE
+    same_site = getSameSiteString(config_->refreshTokenCookieSettings().same_site_);
+    break;
+  }
+
+  std::string cookie_tail = fmt::format(CookieTailHttpOnlyFormatString, expires_time, same_site);
+  if (!config_->cookieDomain().empty()) {
+    cookie_tail =
+        absl::StrCat(fmt::format(CookieDomainFormatString, config_->cookieDomain()), cookie_tail);
+  }
+  return cookie_tail;
+}
+
 void OAuth2Filter::onGetAccessTokenSuccess(const std::string& access_code,
                                            const std::string& id_token,
                                            const std::string& refresh_token,
@@ -809,51 +903,33 @@ void OAuth2Filter::onRefreshAccessTokenFailure() {
 void OAuth2Filter::addResponseCookies(Http::ResponseHeaderMap& headers,
                                       const std::string& encoded_token) const {
   // We use HTTP Only cookies.
-  std::string cookie_tail_http_only = fmt::format(CookieTailHttpOnlyFormatString, expires_in_);
-  if (!config_->cookieDomain().empty()) {
-    cookie_tail_http_only = absl::StrCat(
-        fmt::format(CookieDomainFormatString, config_->cookieDomain()), cookie_tail_http_only);
-  }
-
   const CookieNames& cookie_names = config_->cookieNames();
 
+  // Set the cookies in the response headers.
   headers.addReferenceKey(
       Http::Headers::get().SetCookie,
-      absl::StrCat(cookie_names.oauth_hmac_, "=", encoded_token, cookie_tail_http_only));
-  headers.addReferenceKey(
-      Http::Headers::get().SetCookie,
-      absl::StrCat(cookie_names.oauth_expires_, "=", new_expires_, cookie_tail_http_only));
+      absl::StrCat(cookie_names.oauth_hmac_, "=", encoded_token, BuildCookieTail(2))); // OAUTH_HMAC
+
+  headers.addReferenceKey(Http::Headers::get().SetCookie,
+                          absl::StrCat(cookie_names.oauth_expires_, "=", new_expires_,
+                                       BuildCookieTail(3))); // OAUTH_EXPIRES
 
   if (!access_token_.empty()) {
-    headers.addReferenceKey(
-        Http::Headers::get().SetCookie,
-        absl::StrCat(cookie_names.bearer_token_, "=", access_token_, cookie_tail_http_only));
+    headers.addReferenceKey(Http::Headers::get().SetCookie,
+                            absl::StrCat(cookie_names.bearer_token_, "=", access_token_,
+                                         BuildCookieTail(1))); // BEARER_TOKEN
   }
 
   if (!id_token_.empty()) {
-    std::string id_token_cookie_tail_http_only =
-        fmt::format(CookieTailHttpOnlyFormatString, expires_id_token_in_);
-    if (!config_->cookieDomain().empty()) {
-      id_token_cookie_tail_http_only =
-          absl::StrCat(fmt::format(CookieDomainFormatString, config_->cookieDomain()),
-                       id_token_cookie_tail_http_only);
-    }
     headers.addReferenceKey(
         Http::Headers::get().SetCookie,
-        absl::StrCat(cookie_names.id_token_, "=", id_token_, id_token_cookie_tail_http_only));
+        absl::StrCat(cookie_names.id_token_, "=", id_token_, BuildCookieTail(4))); // ID_TOKEN
   }
 
   if (!refresh_token_.empty()) {
-    std::string refresh_token_cookie_tail_http_only =
-        fmt::format(CookieTailHttpOnlyFormatString, expires_refresh_token_in_);
-    if (!config_->cookieDomain().empty()) {
-      refresh_token_cookie_tail_http_only =
-          absl::StrCat(fmt::format(CookieDomainFormatString, config_->cookieDomain()),
-                       refresh_token_cookie_tail_http_only);
-    }
     headers.addReferenceKey(Http::Headers::get().SetCookie,
                             absl::StrCat(cookie_names.refresh_token_, "=", refresh_token_,
-                                         refresh_token_cookie_tail_http_only));
+                                         BuildCookieTail(5))); // REFRESH_TOKEN
   }
 }
 

source/extensions/filters/http/oauth2/filter.h

@@ -168,6 +168,26 @@ class FilterConfig {
   }
   bool shouldUseRefreshToken(
       const envoy::extensions::filters::http::oauth2::v3::OAuth2Config& proto_config) const;
+  struct CookieSettings {
+    CookieSettings(const envoy::extensions::filters::http::oauth2::v3::CookieConfig& config)
+        : same_site_(config.same_site()) {}
+
+    // Default constructor
+    CookieSettings()
+        : same_site_(envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite::
+                         CookieConfig_SameSite_DISABLED) {}
+
+    const envoy::extensions::filters::http::oauth2::v3::CookieConfig_SameSite same_site_;
+  };
+
+  const CookieSettings& bearerTokenCookieSettings() const { return bearer_token_cookie_settings_; }
+  const CookieSettings& hmacCookieSettings() const { return hmac_cookie_settings_; }
+  const CookieSettings& expiresCookieSettings() const { return expires_cookie_settings_; }
+  const CookieSettings& idTokenCookieSettings() const { return id_token_cookie_settings_; }
+  const CookieSettings& refreshTokenCookieSettings() const {
+    return refresh_token_cookie_settings_;
+  }
+  const CookieSettings& nonceCookieSettings() const { return nonce_cookie_settings_; }
 
 private:
   static FilterStats generateStats(const std::string& prefix, Stats::Scope& scope);
@@ -199,6 +219,12 @@ class FilterConfig {
   const bool disable_access_token_set_cookie_ : 1;
   const bool disable_refresh_token_set_cookie_ : 1;
   absl::optional<RouteRetryPolicy> retry_policy_;
+  const CookieSettings bearer_token_cookie_settings_;
+  const CookieSettings hmac_cookie_settings_;
+  const CookieSettings expires_cookie_settings_;
+  const CookieSettings id_token_cookie_settings_;
+  const CookieSettings refresh_token_cookie_settings_;
+  const CookieSettings nonce_cookie_settings_;
 };
 
 using FilterConfigSharedPtr = std::shared_ptr<FilterConfig>;
@@ -268,7 +294,7 @@ class OAuth2Filter : public Http::PassThroughFilter,
                      Logger::Loggable<Logger::Id::oauth2> {
 public:
   OAuth2Filter(FilterConfigSharedPtr config, std::unique_ptr<OAuth2Client>&& oauth_client,
-               TimeSource& time_source, Random::RandomGenerator& random);
+               TimeSource& time_source, Random::RandomGenerator& randomhmacCookieSettings);
 
   // Http::PassThroughFilter
   Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override;
@@ -331,6 +357,7 @@ class OAuth2Filter : public Http::PassThroughFilter,
                                             const std::chrono::seconds& expires_in) const;
   std::string getExpiresTimeForIdToken(const std::string& id_token,
                                        const std::chrono::seconds& expires_in) const;
+  std::string BuildCookieTail(int cookie_type) const;
   void addResponseCookies(Http::ResponseHeaderMap& headers, const std::string& encoded_token) const;
   const std::string& bearerPrefix() const;
   CallbackValidationResult validateOAuthCallback(const Http::RequestHeaderMap& headers,