Skip to content

Conversation

@zhaohuabing
Copy link
Member

@zhaohuabing zhaohuabing commented Dec 13, 2024

Commit Message: The current CSRF token in the OAuth2 filter state is a random string. This PR improved it by signing the random string with the HMAC secret, adding protection agains CSRF token forgery.

The new CSRF token is in the format of <nonce>.<hmac(nonce)>, which is the Signed Double-Submit Cookie pattern recommended by OWASP.

Additional Description:
Risk Level: low
Testing: Unit test and integration test
Docs Changes: No
Release Notes: Yes
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #37560]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

cc @denniskniep @missBerg @arkodg

@zhaohuabing zhaohuabing marked this pull request as draft December 13, 2024 08:07
@zhaohuabing zhaohuabing changed the title Oauth2 csrf token single double submit cookie OAuth2 filter: improve the csrf token with "Signed Double-Submit Cookie" Dec 13, 2024
@zhaohuabing zhaohuabing force-pushed the oauth2-csrf-token-single-double-submit-cookie branch from 8336232 to 3c03b55 Compare December 13, 2024 12:44
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Dec 13, 2024
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @mattklein123

🐱

Caused by: #37646 was synchronize by zhaohuabing.

see: more, trace.

@zhaohuabing zhaohuabing marked this pull request as ready for review December 16, 2024 04:44
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing force-pushed the oauth2-csrf-token-single-double-submit-cookie branch from c5eec55 to 485c659 Compare December 16, 2024 07:11
@@ -0,0 +1,29 @@
diff --git a/single-page-app/verify.sh b/single-page-app/verify.sh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is added as a workaround for envoyproxy/examples#384 . It can be removed after envoyproxy/examples#384 is merged.

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Dec 17, 2024
@mattklein123 mattklein123 merged commit dcd8a22 into envoyproxy:main Dec 17, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve OAuth csrf token with "Signed Double-Submit Cookie"

2 participants