OAuth2 filter: improve the csrf token with "Signed Double-Submit Cookie"

bazel/envoy_examples.patch

@@ -0,0 +1,29 @@
+diff --git a/single-page-app/verify.sh b/single-page-app/verify.sh
+index 6e188e6..819661e 100755
+--- a/single-page-app/verify.sh
++++ b/single-page-app/verify.sh
+@@ -149,7 +149,10 @@ test_auth () {
+         "${proxy_scheme}://localhost:${proxy_port}/login" \
+         "${curl_args[@]}"
+
+-    encoded_state=$(echo -n "{\"url\":\"${proxy_scheme}://localhost:${proxy_port}/login\",\"nonce\":\"12345678\"}" | basenc --base64url --wrap=0 | sed 's/=//g')
++    random=$(head /dev/urandom | xxd -p | head -c 16)
++    hmac=$(echo -n "${random}" | openssl dgst -sha256 -hmac "${HMAC_SECRET}" -binary|base64)
++    csrf_token=${random}.${hmac}
++    encoded_state=$(echo -n "{\"url\":\"${proxy_scheme}://localhost:${proxy_port}/login\",\"csrf_token\":\"${csrf_token}\"}" | basenc --base64url --wrap=0 | sed 's/=//g')
+
+     run_log "Fetch the myhub authorization page"
+     if [[ "$STATE_BASE64URL_ENCODE" == "true" ]]; then
+@@ -175,10 +178,10 @@ test_auth () {
+     run_log "Return to the app and receive creds"
+     if [[ "$STATE_BASE64URL_ENCODE" == "true" ]]; then
+         CODE=$(_curl "${curl_args[@]}" --head "http://localhost:${PORT_MYHUB}/authorize?client_id=0123456789&redirect_uri=${proxy_scheme}%3A%2F%2Flocalhost%3A${proxy_port}%2Fauthorize&response_type=code&scope=user%3Aemail&state=${encoded_state}" | grep Location | cut -d= -f2 | cut -d\& -f1)
+-        RESPONSE=$(_curl "${curl_args[@]}" --cookie "OauthNonce=12345678" --head "${proxy_scheme}://localhost:${proxy_port}/authorize?code=$CODE&state=${encoded_state}")
++        RESPONSE=$(_curl "${curl_args[@]}" --cookie "OauthNonce=${csrf_token}" --head "${proxy_scheme}://localhost:${proxy_port}/authorize?code=$CODE&state=${encoded_state}")
+     else
+         CODE=$(_curl "${curl_args[@]}" --head "http://localhost:${PORT_MYHUB}/authorize?client_id=0123456789&redirect_uri=${proxy_scheme}%3A%2F%2Flocalhost%3A${proxy_port}%2Fauthorize&response_type=code&scope=user%3Aemail&state=url%3D${proxy_scheme}%253A%252F%252Flocalhost%253A${proxy_port}%252Flogin%26nonce%3D12345678" | grep Location | cut -d= -f2 | cut -d\& -f1)
+-        RESPONSE=$(_curl "${curl_args[@]}" --cookie "OauthNonce=12345678" --head "${proxy_scheme}://localhost:${proxy_port}/authorize?code=$CODE&state=url%3D${proxy_scheme}%253A%252F%252Flocalhost%253A${proxy_port}%252Flogin%26nonce%3D12345678")
++        RESPONSE=$(_curl "${curl_args[@]}" --cookie "OauthNonce=${csrf_token}" --head "${proxy_scheme}://localhost:${proxy_port}/authorize?code=$CODE&state=url%3D${proxy_scheme}%253A%252F%252Flocalhost%253A${proxy_port}%252Flogin%26nonce%3D12345678")
+     fi
+     echo "$RESPONSE" | grep "HTTP/1.1 302 Found"
+     echo "$RESPONSE" | grep "location: ${proxy_scheme}://localhost:${proxy_port}/login"

bazel/repositories.bzl

@@ -223,7 +223,11 @@ def envoy_dependencies(skip_targets = []):
     external_http_archive("bazel_features")
     external_http_archive("bazel_toolchains")
     external_http_archive("bazel_compdb")
-    external_http_archive(name = "envoy_examples")
+    external_http_archive(
+        name = "envoy_examples",
+        patch_args = ["-p1"],
+        patches = ["@envoy//bazel:envoy_examples.patch"],
+    )
 
     _com_github_maxmind_libmaxminddb()
 

changelogs/current.yaml

@@ -73,6 +73,10 @@ minor_behavior_changes:
   change: |
     The ``state`` parameter in the OAuth2 authorization request has been changed to a base64url-encoded JSON object.
     The JSON object contains the original request URL and a nonce for CSRF prevention.
+- area: oauth2
+  change: |
+    Implement the Signed Double-Submit Cookie pattern, as recommended by OWASP, by using the HMAC secret to sign and verify
+    the nonce.
 - area: quic
   change: |
     Enable UDP GRO in QUIC client connections by default. This behavior can be reverted by setting

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

@@ -52,7 +52,7 @@ constexpr absl::string_view queryParamsState = "state";
 constexpr absl::string_view queryParamsRedirectUri = "redirect_uri";
 
 constexpr absl::string_view stateParamsUrl = "url";
-constexpr absl::string_view stateParamsNonce = "nonce";
+constexpr absl::string_view stateParamsCsrfToken = "csrf_token";
 
 constexpr absl::string_view REDIRECT_RACE = "oauth.race_redirect";
 constexpr absl::string_view REDIRECT_LOGGED_IN = "oauth.logged_in";
@@ -179,20 +179,41 @@ std::string encodeHmac(const std::vector<uint8_t>& secret, absl::string_view dom
   return encodeHmacBase64(secret, domain, expires, token, id_token, refresh_token);
 }
 
-// Generates a non-guessable nonce that can be used to prevent CSRF attacks.
-std::string generateNonce(Random::RandomGenerator& random) {
-  return Hex::uint64ToHex(random.random());
+// Generates a CSRF token that can be used to prevent CSRF attacks.
+// The token is in the format of <nonce>.<hmac(nonce)> recommended by
+// https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#signed-double-submit-cookie-recommended
+std::string generateCsrfToken(const std::string& hmac_secret, Random::RandomGenerator& random) {
+  std::vector<uint8_t> hmac_secret_vec(hmac_secret.begin(), hmac_secret.end());
+  std::string random_string = Hex::uint64ToHex(random.random());
+  std::string hmac = generateHmacBase64(hmac_secret_vec, random_string);
+  std::string csrf_token = fmt::format("{}.{}", random_string, hmac);
+  return csrf_token;
+}
+
+// validate the csrf token hmac to prevent csrf token forgery
+bool validateCsrfTokenHmac(const std::string& hmac_secret, const std::string& csrf_token) {
+  size_t pos = csrf_token.find('.');
+  if (pos == std::string::npos) {
+    return false;
+  }
+
+  std::string token = std::string(csrf_token.substr(0, pos));
+  std::string hmac = std::string(csrf_token.substr(pos + 1));
+  std::vector<uint8_t> hmac_secret_vec(hmac_secret.begin(), hmac_secret.end());
+  return generateHmacBase64(hmac_secret_vec, token) == hmac;
 }
 
 /**
  * Encodes the state parameter for the OAuth2 flow.
  * The state parameter is a base64Url encoded JSON object containing the original request URL and a
- * nonce for CSRF protection.
+ * CSRF token for CSRF protection.
  */
-std::string encodeState(const std::string& original_request_url, const std::string& nonce) {
+std::string encodeState(const std::string& original_request_url, const std::string& csrf_token) {
   std::string buffer;
   absl::string_view sanitized_url = Json::sanitize(buffer, original_request_url);
-  std::string json = fmt::format(R"({{"url":"{}","nonce":"{}"}})", sanitized_url, nonce);
+  absl::string_view sanitized_csrf_token = Json::sanitize(buffer, csrf_token);
+  std::string json =
+      fmt::format(R"({{"url":"{}","csrf_token":"{}"}})", sanitized_url, sanitized_csrf_token);
   return Base64Url::encode(json.data(), json.size());
 }
 
@@ -487,7 +508,7 @@ bool OAuth2Filter::canRedirectToOAuthServer(Http::RequestHeaderMap& headers) con
   return true;
 }
 
-void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const {
+void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) {
   Http::ResponseHeaderMapPtr response_headers{Http::createHeaderMap<Http::ResponseHeaderMapImpl>(
       {{Http::Headers::get().Status, std::to_string(enumToInt(Http::Code::Found))}})};
   // Construct the correct scheme. We default to https since this is a requirement for OAuth to
@@ -501,23 +522,27 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const
   const std::string base_path = absl::StrCat(scheme, "://", host_);
   const std::string original_url = absl::StrCat(base_path, headers.Path()->value().getStringView());
 
-  // Encode the original request URL and the nonce to the state parameter.
-  // Generate a nonce to prevent CSRF attacks.
-  std::string nonce;
-  bool nonce_cookie_exists = false;
-  const auto nonce_cookie = Http::Utility::parseCookies(headers, [this](absl::string_view key) {
-    return key == config_->cookieNames().oauth_nonce_;
-  });
-  if (nonce_cookie.find(config_->cookieNames().oauth_nonce_) != nonce_cookie.end()) {
-    nonce = nonce_cookie.at(config_->cookieNames().oauth_nonce_);
-    nonce_cookie_exists = true;
+  // First, check if the CSRF token cookie exists.
+  // The CSRF token cookie contains the CSRF token that is used to prevent CSRF attacks for the
+  // OAuth flow. It was named "oauth_nonce" because the CSRF token contains a generated nonce.
+  // "oauth_csrf_token" would be a more accurate name for the cookie.
+  std::string csrf_token;
+  bool csrf_token_cookie_exists = false;
+  const auto csrf_token_cookie =
+      Http::Utility::parseCookies(headers, [this](absl::string_view key) {
+        return key == config_->cookieNames().oauth_nonce_;
+      });
+  if (csrf_token_cookie.find(config_->cookieNames().oauth_nonce_) != csrf_token_cookie.end()) {
+    csrf_token = csrf_token_cookie.at(config_->cookieNames().oauth_nonce_);
+    csrf_token_cookie_exists = true;
   } else {
-    nonce = generateNonce(random_);
+    // Generate a CSRF token to prevent CSRF attacks.
+    csrf_token = generateCsrfToken(config_->hmacSecret(), random_);
   }
 
-  // Set the nonce cookie if it does not exist.
-  if (!nonce_cookie_exists) {
-    // Expire the nonce cookie in 10 minutes.
+  // Set the CSRF token cookie if it does not exist.
+  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);
@@ -527,10 +552,17 @@ void OAuth2Filter::redirectToOAuthServer(Http::RequestHeaderMap& headers) const
     }
     response_headers->addReferenceKey(
         Http::Headers::get().SetCookie,
-        absl::StrCat(config_->cookieNames().oauth_nonce_, "=", nonce, cookie_tail_http_only));
+        absl::StrCat(config_->cookieNames().oauth_nonce_, "=", csrf_token, cookie_tail_http_only));
   }
-  const std::string state = encodeState(original_url, nonce);
 
+  // Validate the CSRF token HMAC if the CSRF token cookie exists.
+  if (csrf_token_cookie_exists && !validateCsrfTokenHmac(config_->hmacSecret(), csrf_token)) {
+    ENVOY_LOG(error, "csrf token validation failed");
+    sendUnauthorizedResponse();
+    return;
+  }
+
+  const std::string state = encodeState(original_url, csrf_token);
   auto query_params = config_->authorizationQueryParams();
   query_params.overwrite(queryParamsState, state);
 
@@ -834,8 +866,8 @@ void OAuth2Filter::sendUnauthorizedResponse() {
 // Validates the OAuth callback request.
 // * Does the query parameters contain an error response?
 // * Does the query parameters contain the code and state?
-// * Does the state contain the original request URL and nonce?
-// * Does the nonce in the state match the nonce in the cookie?
+// * Does the state contain the original request URL and the CSRF token?
+// * Does the CSRF token in the state match the one in the cookie?
 CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::RequestHeaderMap& headers,
                                                              const absl::string_view path_str) {
   // Return 401 unauthorized if the query parameters contain an error response.
@@ -854,8 +886,8 @@ CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::Request
   }
 
   // Return 401 unauthorized if the state query parameter does not contain the original request URL
-  // or nonce.
-  // Decode the state parameter to get the original request URL and nonce.
+  // or the CSRF token.
+  // Decode the state parameter to get the original request URL and the CSRF token.
   const std::string state = Base64Url::decode(stateVal.value());
   bool has_unknown_field;
   ProtobufWkt::Struct message;
@@ -867,20 +899,21 @@ CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::Request
   }
 
   const auto& filed_value_pair = message.fields();
-  if (!filed_value_pair.contains(stateParamsUrl) || !filed_value_pair.contains(stateParamsNonce)) {
-    ENVOY_LOG(error, "state query param does not contain url or nonce: \n{}", state);
+  if (!filed_value_pair.contains(stateParamsUrl) ||
+      !filed_value_pair.contains(stateParamsCsrfToken)) {
+    ENVOY_LOG(error, "state query param does not contain url or CSRF token: \n{}", state);
     return {false, "", ""};
   }
 
-  // Return 401 unauthorized if the nonce cookie does not match the nonce in the state.
+  // Return 401 unauthorized if the CSRF token cookie does not match the CSRF token in the state.
   //
   // This is to prevent attackers from injecting their own access token into a victim's
   // sessions via CSRF attack. The attack can result in victims saving their sensitive data
   // in the attacker's account.
   // More information can be found at https://datatracker.ietf.org/doc/html/rfc6819#section-5.3.5
-  std::string nonce = filed_value_pair.at(stateParamsNonce).string_value();
-  if (!validateNonce(headers, nonce)) {
-    ENVOY_LOG(error, "nonce cookie does not match nonce in the state: \n{}", nonce);
+  std::string csrf_token = filed_value_pair.at(stateParamsCsrfToken).string_value();
+  if (!validateCsrfToken(headers, csrf_token)) {
+    ENVOY_LOG(error, "csrf token validation failed");
     return {false, "", ""};
   }
   const std::string original_request_url = filed_value_pair.at(stateParamsUrl).string_value();
@@ -895,15 +928,17 @@ CallbackValidationResult OAuth2Filter::validateOAuthCallback(const Http::Request
   return {true, codeVal.value(), original_request_url};
 }
 
-// Validates the nonce in the state parameter against the nonce in the cookie.
-bool OAuth2Filter::validateNonce(const Http::RequestHeaderMap& headers,
-                                 const std::string& nonce) const {
-  const auto nonce_cookie = Http::Utility::parseCookies(headers, [this](absl::string_view key) {
-    return key == config_->cookieNames().oauth_nonce_;
-  });
+// Validates the csrf_token in the state parameter against the one in the cookie.
+bool OAuth2Filter::validateCsrfToken(const Http::RequestHeaderMap& headers,
+                                     const std::string& csrf_token) const {
+  const auto csrf_token_cookie =
+      Http::Utility::parseCookies(headers, [this](absl::string_view key) {
+        return key == config_->cookieNames().oauth_nonce_;
+      });
 
-  if (nonce_cookie.find(config_->cookieNames().oauth_nonce_) != nonce_cookie.end() &&
-      nonce_cookie.at(config_->cookieNames().oauth_nonce_) == nonce) {
+  if (csrf_token_cookie.find(config_->cookieNames().oauth_nonce_) != csrf_token_cookie.end() &&
+      csrf_token_cookie.at(config_->cookieNames().oauth_nonce_) == csrf_token &&
+      validateCsrfTokenHmac(config_->hmacSecret(), csrf_token)) {
     return true;
   }
   return false;

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

@@ -322,7 +322,7 @@ class OAuth2Filter : public Http::PassThroughFilter,
   // connection is mTLS, etc.)
   bool canSkipOAuth(Http::RequestHeaderMap& headers) const;
   bool canRedirectToOAuthServer(Http::RequestHeaderMap& headers) const;
-  void redirectToOAuthServer(Http::RequestHeaderMap& headers) const;
+  void redirectToOAuthServer(Http::RequestHeaderMap& headers);
 
   Http::FilterHeadersStatus signOutUser(const Http::RequestHeaderMap& headers);
 
@@ -335,7 +335,8 @@ class OAuth2Filter : public Http::PassThroughFilter,
   const std::string& bearerPrefix() const;
   CallbackValidationResult validateOAuthCallback(const Http::RequestHeaderMap& headers,
                                                  const absl::string_view path_str);
-  bool validateNonce(const Http::RequestHeaderMap& headers, const std::string& nonce) const;
+  bool validateCsrfToken(const Http::RequestHeaderMap& headers,
+                         const std::string& csrf_token) const;
 };
 
 } // namespace Oauth2