envoy icon indicating copy to clipboard operation
envoy copied to clipboard

oauth2 filter: encrypt tokens

Open zhaohuabing opened this issue 7 months ago • 2 comments

Commit Message: This PR encrypts the access, ID, and refresh tokens for the OAuth2 filter.

Risk Level: low Testing: Yes Docs Changes: No Release Notes: Yes Implements #23508

zhaohuabing avatar Apr 10 '25 05:04 zhaohuabing

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions[bot] avatar May 16 '25 12:05 github-actions[bot]

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions[bot] avatar May 23 '25 16:05 github-actions[bot]

I am fine to this change. But I think it deserve a runtime flag (prefer this one) or a bool flag in the API for this behavior change.

wbpcode avatar Jun 23 '25 15:06 wbpcode

I am fine to this change. But I think it deserve a runtime flag (prefer this one) or a bool flag in the API for this behavior change.

This behavior change is internal to the OAuth2 implementation only and not exposed to users/applications.

  • The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.
  • The OAuth2 filters decrypt the tokens in the cookies before forwarding them to the backend services.

So there is no behavior change from the users and applications's perspective, do we still need a runtime flag in this case?

zhaohuabing avatar Jun 24 '25 04:06 zhaohuabing

  • The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.

From proxy/client's perspective, some set-cookie values format are changed (no backward compatibility), I would think it's a minor behavior change. And a flag with default value true should be harmless.

wbpcode avatar Jun 24 '25 06:06 wbpcode

pr ci just broke, apologies, working on a fix

phlax avatar Jun 24 '25 07:06 phlax

  • The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.

From proxy/client's perspective, some set-cookie values format are changed (no backward compatibility), I would think it's a minor behavior change. And a flag with default value true should be harmless.

Sounds good. I'm adding a runtime guard.

zhaohuabing avatar Jun 24 '25 07:06 zhaohuabing

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/39059 was synchronize by zhaohuabing.

see: more, trace.

How could I fix this? @wbpcode @phlax

==================== Test output for //tools/code:check_test:
CodeChecker ERROR [runtime_guards] Missing from changelogs: envoy_reloadable_features_oauth2_encrypt_tokens
CodeChecker ERROR [runtime_guards] Check failed
CodeChecker ERROR ERRORS Summary [runtime_guards]:
--------------------------------------------------------------------------------
Missing from changelogs: envoy_reloadable_features_oauth2_encrypt_tokens

I did mention this runtime guard in the change log:

- area: oauth2
  change: |
    The access token, id token and refresh token in the cookies are now encrypted using the HMAC secret. This behavior can
    be reverted by setting the runtime guard ``"envoy.reloadable_features.oauth2_encrypt_tokens"`` to ``false``.

zhaohuabing avatar Jun 24 '25 10:06 zhaohuabing

~its in the wrong section - possibly you want 2 changelog entries~

see below

phlax avatar Jun 24 '25 10:06 phlax

/retest

zhaohuabing avatar Jun 26 '25 00:06 zhaohuabing

/retest

wbpcode avatar Jul 04 '25 09:07 wbpcode