OAuth2: Add samesite attribute support for all OAuth2 supported cookie types #37917
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
ready for review. all unit tests added and passed |
Yueren-Wang
changed the title
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Yueren-Wang
force-pushed
the
yueren-wang-samesite-cookie-support
branch
from
cf8679c to
5f0d302
Compare
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
|
CC @envoyproxy/coverage-shephards: FYI only for changes made to |
Signed-off-by: Yueren Wang <[email protected]>
…en-Wang/envoy into yueren-wang-samesite-cookie-support
…y#37832) An LRU cache was introduced to cache `Envoy::Network::Address` instances because they are expensive to create. These addresses are cached for reading source and destination addresses from `recvmsg` and `recvmmsg` calls on QUIC UDP sockets. The current size of the cache is 4 entries for each IoHandle (i.e. each socket). A locally run CPU profile of Envoy Mobile showed about 1.75% of CPU cycles going towards querying and inserting into the `quic::QuicLRUCache`. Given the small number of elements in the cache, this commit uses a `std::vector` data structure instead of `QuicLRUCache`. `QuicLRUCache`, `std::vector`, and `std::deque` were compared using newly added benchmark tests, and the following were the results: QuicLRUCache: ``` ------------------------------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations ------------------------------------------------------------------------------------------------------------------------- BM_GetOrCreateEnvoyAddressInstanceNoCache/iterations:1000 31595 ns 31494 ns 1000 BM_GetOrCreateEnvoyAddressInstanceConnectedSocket/iterations:1000 5538 ns 5538 ns 1000 BM_GetOrCreateEnvoyAddressInstanceUnconnectedSocket/iterations:1000 38918 ns 38814 ns 1000 BM_GetOrCreateEnvoyAddressInstanceUnconnectedSocketLargerCache/iterations:1000 52969 ns 52846 ns 1000 ``` std::deque: ``` ------------------------------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations ------------------------------------------------------------------------------------------------------------------------- BM_GetOrCreateEnvoyAddressInstanceNoCache/iterations:1000 31805 ns 31716 ns 1000 BM_GetOrCreateEnvoyAddressInstanceConnectedSocket/iterations:1000 1553 ns 1550 ns 1000 BM_GetOrCreateEnvoyAddressInstanceUnconnectedSocket/iterations:1000 27243 ns 27189 ns 1000 BM_GetOrCreateEnvoyAddressInstanceUnconnectedSocketLargerCache/iterations:1000 39335 ns 39235 ns 1000 ``` std::vector: ``` ------------------------------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations ------------------------------------------------------------------------------------------------------------------------- BM_GetOrCreateEnvoyAddressInstanceNoCache/iterations:1000 31960 ns 31892 ns 1000 BM_GetOrCreateEnvoyAddressInstanceConnectedSocket/iterations:1000 1514 ns 1514 ns 1000 BM_GetOrCreateEnvoyAddressInstanceUnconnectedSocket/iterations:1000 26361 ns 26261 ns 1000 BM_GetOrCreateEnvoyAddressInstanceUnconnectedSocketLargerCache/iterations:1000 43987 ns 43738 ns 1000 ``` `std::vector` uses 3.5x less CPU cycles than `quic::QuicLRUCache` and performs very slightly better than `std::deque` at small cache sizes. If considering creating a bigger cache size (e.g. >= 50 entries), `std::deque` may perform better and it's worth profiling, though in such a situation, no cache at all seems to perform better than a cache. Risk Level: low Testing: unit and benchmark tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a --------- Signed-off-by: Ali Beyad <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
) ## Description This PR rewords the [example](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/aggregate_cluster#load-balancing-example) we have in the aggregate cluster to explain how the cluster level load balancer selects the cluster. Signed-off-by: Rohit Agrawal <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
https://github.com/google/quiche/compare/71111c723..e599ad25d ``` $ git log 71111c723..e599ad25d --date=short --no-merges --format="%ad %al %s" 2025-01-06 martinduke Refresh MoQT integration test for FETCH and fix detected errors. ``` Risk Level: low Testing: n/a Docs Changes: n.a Release Notes: n/ah Signed-off-by: Alyssa Wilk <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Ali Beyad <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Fix envoyproxy#36432 Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com> Signed-off-by: Yueren Wang <[email protected]>
…xy#37556) Signed-off-by: Yueren Wang <[email protected]>
…nvoyproxy#37776) Reapply the RBAC change by reverting envoyproxy#35899. The original commit is reverted for flaky RBAC integration test. The fix is pretty simple by just removing the simulated time in the test ([diff](envoyproxy@d3297b4)), I'm not sure of the root cause but the test does not really need the simulated test. Before the fix, the flaky test can be reproduced easily with `--runs_per_test=1000`, After the test, I can no longer reproduce the failure. To be on the safer side, I ran the test with `--runs_per_test=10000` and still couldn't reproduce the test failure. Test output: ``` % bazel test //test/extensions/filters/network/rbac:integration_test --runs_per_test=10000 --test_timeout=60 --test_env=ENVOY_IP_TEST_VERSIONS=v4only INFO: Invocation ID: c1ebbc18-4887-4626-9d19-e257d42fe4cd INFO: Analyzed target //test/extensions/filters/network/rbac:integration_test (0 packages loaded, 19 targets configured). INFO: Found 1 test target... Target //test/extensions/filters/network/rbac:integration_test up-to-date: bazel-bin/test/extensions/filters/network/rbac/integration_test INFO: Elapsed time: 2486.472s, Critical Path: 24.73s INFO: 10001 processes: 1 internal, 10000 processwrapper-sandbox. INFO: Build completed successfully, 10001 total actions Executed 1 out of 1 test: 1 test passes. ``` --------- Signed-off-by: Yangmin Zhu <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
…`wamr` -> 2.2.0 (envoyproxy#37868) Commit Message: deps: Update `proxy_wasm_cpp_host` -> c4d7bb0, `wasmtime` -> 24.0.2, `wamr` -> 2.2.0 Additional Description: proxy-wasm/proxy-wasm-cpp-host@f199214...c4d7bb0 Risk Level: low Testing: `bazel test test/...` passes, with `--define=wasm=v8`, `--define=wasm=wamr`, and `--define=wasm=wasmtime`. Docs Changes: None. Release Notes: Mentioned new support for [Go SDK](github.com/proxy-wasm/proxy-wasm-go-sdk) plugins. Supercedes envoyproxy#36880 and envoyproxy#36857 --------- Signed-off-by: Matt Leon <[email protected]> Signed-off-by: Keith Mattix II <[email protected]> Co-authored-by: Ryan Northey <[email protected]> Co-authored-by: Keith Mattix II <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Fix envoyproxy#37553 Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com> Signed-off-by: Yueren Wang <[email protected]>
…proxy#37897) Currently this file is getting the includes transitively though Abseil, and I am trying to remove the includes from Abseil without breaking Envoy. Risk Level: low Testing: N/A Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A Signed-off-by: Derek Mauro <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Commit Message: [mobile] Report network subtype when on cellular Additional Description: Cellular networks will be further categorized as 2G/3G/4G/5G Risk Level: low Testing: unit tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: android only Signed-off-by: Renjie Tang <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
…oyproxy#37894) Signed-off-by: Alyssa Wilk <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
The ability to truncate a file is a prerequisite for the next round of file-based cache. --------- Signed-off-by: Raven Black <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
…envoyproxy#37899) I tested dynamic modules from rust and was getting a segfault. I noticed that drop was getting called twice. It turns out that `onDestroy` gets called twice. The right solution here is probably to prevent `onDestroy()` from being called twice... but barring that, this change works. Commit Message: dynamic_modules: don't call on_http_filter_destroy_ if filter is null Additional Description: Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Ben Plotnick <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Commit Message: support backoff between retries in udp_proxy Additional Description: This feature also fixes a bug in the retry mechanism in udp proxy in the following situation: 1. We are tunneling UDP over HTTP. 2. A new stream is created on existing upstream connection (multiplexing) and waiting for response headers. 3. The upstream connection is closed. 4. During the close process, all streams created on this connection will be reset. 5. The udp_proxy receives a callback on the stream reset. 6. They retry to connect. 7. The closed connection is picked as it is still in the connection pool (we are still in the process of the close). 8. The new stream that is created on the second attempt will be reset immediately. 9. The same process will happen (step 5 - step 8) until we reach the max_connect_attepts. 10. This means that we are doing only one real attempt in this situation. Risk Level: medium Testing: unit tests, integration tests Docs Changes: added Release Notes: added --------- Signed-off-by: Issa Abu Kalbein <[email protected]> Co-authored-by: Issa Abu Kalbein <[email protected]>
…ision (envoyproxy#37794) Commit Message: ext_proc: change default sampling decision to false Fixes: envoyproxy#37783 Additional Description: Default to using the parent span's sampled status instead of true (default in [StreamOptions](https://github.com/envoyproxy/envoy/blob/df5260957fc9446b57114a74ffbdc5d9b5831bfd/envoy/http/async_client.h#L315-L316)), so that we don't emit orphan spans for all ext_proc streams where the parent span is not sampled. That's the same issue seen in Lua httpCall (envoyproxy#33200) and ext_authz (envoyproxy#19343). Risk Level: Low Testing: changed the tracing integration test behavior and validated in production. --------- Signed-off-by: Fernando Cainelli <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
…mobile-android config (envoyproxy#37936) This moves `--noincompatible_enable_android_toolchain_resolution` to `--config=mobile-android` so that it works on both local and remote builds. Risk Level: low Testing: CI Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Fredy Wijaya <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: dependency-envoy[bot] <148525496+dependency-envoy[bot]@users.noreply.github.com> Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Commit Message: Add `envoy_dynamic_module_callback_http_send_response` to the dynamic modules ABI and rust SDK Additional Description: This adds the ability to send a local response back. I've also fixed up `abi_impl_test.cc`. Before, it was not asserting on the correct headers. Now i've moved to parameterized tests so we can assert on the correct headers and also split out the tests into smaller units. Risk Level: low Testing: unit tests Docs Changes: Release Notes: Platform Specific Features: --------- Signed-off-by: Ben Plotnick <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
…T_STREAM (envoyproxy#37784) We have found this patch useful in our testing. The delta between the first and second commit demonstrates test coverage for the new code. The new behavior can be disabled with the runtime guard `envoy.reloadable_features.http2_propagate_reset_events`. Commit Message: propagates reset events to CodecEventCallbacks when sending RST_STREAM Additional Description: Risk Level: low Testing: ran unit and integration tests locally Docs Changes: Release Notes: Platform Specific Features: Runtime guard: `envoy.reloadable_features.http2_propagate_reset_events` --------- Signed-off-by: Biren Roy <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Ryan Northey <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
…envoyproxy#37627) After the recreatStream() call, both the decode and encode filter chains will abort. Here, by checking if the filter chain has aborted, it prevents the subsequent filters in the filter chain from continuing. Risk Level: Low Testing: Add tests to verify that when recreateStream is executed during the decodeHeader/encodeHeader phase and returns StopIteration, executing continueDecoding/continueEncoding does not proceed with the processing of subsequent filters Runtime guard: envoy.reloadable_features.filter_chain_aborted_can_not_continue Fixes envoyproxy#37625 --------- Signed-off-by: zty98751 <[email protected]> Signed-off-by: 澄潭 <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Commit Message: upstream lb: optimize the upstream load balancer Additional Description: 1. Splitted the legacy LB configuration processing out to independent method. The loadConfig() will always take the lb's extension proto now. It's more clear and more match to common sense. 2. reduce exceptions when loading subset lb configuration. Risk Level: low. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wangbaiping/wbpcode <[email protected]> Signed-off-by: wangbaiping(wbpcode) <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Fixes some notes in the AWS Credentials docs and reword some text to make it clear. Risk Level: N/A Testing: N/A Docs Changes: yes Release Notes: N/A Signed-off-by: Rohit Agrawal <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Risk Level: low Testing: updated tests Docs Changes: n/a Release Notes: n/a envoyproxy/envoy-mobile#176 Signed-off-by: Alyssa Wilk <[email protected]> Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
Signed-off-by: Yueren Wang <[email protected]>
…en-Wang/envoy into yueren-wang-samesite-cookie-support
|
closed due to messed up rebased commits. please refer to #37952 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
The samesite attribute value allowed are:
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 pending, adding in next iteration