From b88e39411c12e3f9e04125a7887f12354f760a14 Mon Sep 17 00:00:00 2001 From: Jens L Date: Tue, 21 Nov 2023 18:10:07 +0100 Subject: [PATCH] security: fix CVE-2023-48228 (#7666) Signed-off-by: Jens Langhammer --- .../providers/oauth2/tests/test_token_pkce.py | 187 ++++++++++++++++++ authentik/providers/oauth2/views/token.py | 5 +- website/docs/security/CVE-2023-48228.md | 61 ++++++ website/sidebars.js | 1 + 4 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 authentik/providers/oauth2/tests/test_token_pkce.py create mode 100644 website/docs/security/CVE-2023-48228.md diff --git a/authentik/providers/oauth2/tests/test_token_pkce.py b/authentik/providers/oauth2/tests/test_token_pkce.py new file mode 100644 index 000000000..23bcf359f --- /dev/null +++ b/authentik/providers/oauth2/tests/test_token_pkce.py @@ -0,0 +1,187 @@ +"""Test token view""" +from base64 import b64encode, urlsafe_b64encode +from hashlib import sha256 + +from django.test import RequestFactory +from django.urls import reverse + +from authentik.core.models import Application +from authentik.core.tests.utils import create_test_admin_user, create_test_flow +from authentik.flows.challenge import ChallengeTypes +from authentik.lib.generators import generate_id +from authentik.providers.oauth2.constants import GRANT_TYPE_AUTHORIZATION_CODE +from authentik.providers.oauth2.models import AuthorizationCode, OAuth2Provider +from authentik.providers.oauth2.tests.utils import OAuthTestCase + + +class TestTokenPKCE(OAuthTestCase): + """Test token view""" + + def setUp(self) -> None: + super().setUp() + self.factory = RequestFactory() + self.app = Application.objects.create(name=generate_id(), slug="test") + + def test_pkce_missing_in_token(self): + """Test full with pkce""" + flow = create_test_flow() + provider = OAuth2Provider.objects.create( + name=generate_id(), + client_id="test", + authorization_flow=flow, + redirect_uris="foo://localhost", + access_code_validity="seconds=100", + ) + Application.objects.create(name="app", slug="app", provider=provider) + state = generate_id() + user = create_test_admin_user() + self.client.force_login(user) + challenge = generate_id() + header = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() + # Step 1, initiate params and get redirect to flow + self.client.get( + reverse("authentik_providers_oauth2:authorize"), + data={ + "response_type": "code", + "client_id": "test", + "state": state, + "redirect_uri": "foo://localhost", + "code_challenge": challenge, + "code_challenge_method": "S256", + }, + ) + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + ) + code: AuthorizationCode = AuthorizationCode.objects.filter(user=user).first() + self.assertJSONEqual( + response.content.decode(), + { + "component": "xak-flow-redirect", + "type": ChallengeTypes.REDIRECT.value, + "to": f"foo://localhost?code={code.code}&state={state}", + }, + ) + response = self.client.post( + reverse("authentik_providers_oauth2:token"), + data={ + "grant_type": GRANT_TYPE_AUTHORIZATION_CODE, + "code": code.code, + # Missing the code_verifier here + "redirect_uri": "foo://localhost", + }, + HTTP_AUTHORIZATION=f"Basic {header}", + ) + self.assertJSONEqual( + response.content, + {"error": "invalid_request", "error_description": "The request is otherwise malformed"}, + ) + self.assertEqual(response.status_code, 400) + + def test_pkce_correct_s256(self): + """Test full with pkce""" + flow = create_test_flow() + provider = OAuth2Provider.objects.create( + name=generate_id(), + client_id="test", + authorization_flow=flow, + redirect_uris="foo://localhost", + access_code_validity="seconds=100", + ) + Application.objects.create(name="app", slug="app", provider=provider) + state = generate_id() + user = create_test_admin_user() + self.client.force_login(user) + verifier = generate_id() + challenge = ( + urlsafe_b64encode(sha256(verifier.encode("ascii")).digest()) + .decode("utf-8") + .replace("=", "") + ) + header = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() + # Step 1, initiate params and get redirect to flow + self.client.get( + reverse("authentik_providers_oauth2:authorize"), + data={ + "response_type": "code", + "client_id": "test", + "state": state, + "redirect_uri": "foo://localhost", + "code_challenge": challenge, + "code_challenge_method": "S256", + }, + ) + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + ) + code: AuthorizationCode = AuthorizationCode.objects.filter(user=user).first() + self.assertJSONEqual( + response.content.decode(), + { + "component": "xak-flow-redirect", + "type": ChallengeTypes.REDIRECT.value, + "to": f"foo://localhost?code={code.code}&state={state}", + }, + ) + response = self.client.post( + reverse("authentik_providers_oauth2:token"), + data={ + "grant_type": GRANT_TYPE_AUTHORIZATION_CODE, + "code": code.code, + "code_verifier": verifier, + "redirect_uri": "foo://localhost", + }, + HTTP_AUTHORIZATION=f"Basic {header}", + ) + self.assertEqual(response.status_code, 200) + + def test_pkce_correct_plain(self): + """Test full with pkce""" + flow = create_test_flow() + provider = OAuth2Provider.objects.create( + name=generate_id(), + client_id="test", + authorization_flow=flow, + redirect_uris="foo://localhost", + access_code_validity="seconds=100", + ) + Application.objects.create(name="app", slug="app", provider=provider) + state = generate_id() + user = create_test_admin_user() + self.client.force_login(user) + verifier = generate_id() + header = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() + # Step 1, initiate params and get redirect to flow + self.client.get( + reverse("authentik_providers_oauth2:authorize"), + data={ + "response_type": "code", + "client_id": "test", + "state": state, + "redirect_uri": "foo://localhost", + "code_challenge": verifier, + }, + ) + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + ) + code: AuthorizationCode = AuthorizationCode.objects.filter(user=user).first() + self.assertJSONEqual( + response.content.decode(), + { + "component": "xak-flow-redirect", + "type": ChallengeTypes.REDIRECT.value, + "to": f"foo://localhost?code={code.code}&state={state}", + }, + ) + response = self.client.post( + reverse("authentik_providers_oauth2:token"), + data={ + "grant_type": GRANT_TYPE_AUTHORIZATION_CODE, + "code": code.code, + "code_verifier": verifier, + "redirect_uri": "foo://localhost", + }, + HTTP_AUTHORIZATION=f"Basic {header}", + ) + self.assertEqual(response.status_code, 200) diff --git a/authentik/providers/oauth2/views/token.py b/authentik/providers/oauth2/views/token.py index 146978fe1..0c0d7f4a8 100644 --- a/authentik/providers/oauth2/views/token.py +++ b/authentik/providers/oauth2/views/token.py @@ -222,7 +222,10 @@ class TokenParams: raise TokenError("invalid_grant") # Validate PKCE parameters. - if self.code_verifier: + if self.authorization_code.code_challenge: + # Authorization code had PKCE but we didn't get one + if not self.code_verifier: + raise TokenError("invalid_request") if self.authorization_code.code_challenge_method == PKCE_METHOD_S256: new_code_challenge = ( urlsafe_b64encode(sha256(self.code_verifier.encode("ascii")).digest()) diff --git a/website/docs/security/CVE-2023-48228.md b/website/docs/security/CVE-2023-48228.md new file mode 100644 index 000000000..dc0c73116 --- /dev/null +++ b/website/docs/security/CVE-2023-48228.md @@ -0,0 +1,61 @@ +# CVE-2023-48228 + +_Reported by [@Sapd](https://github.com/Sapd)_ + +## OAuth2: Insufficient PKCE check + +### Summary + +When initialising a OAuth2 flow with a `code_challenge` and `code_method` (thus requesting PKCE), the SSO provider (authentik) **must** check if there is a matching **and** existing `code_verifier` during the token step. + +authentik checks if the contents of code*verifier is matching \*\*\_ONLY*\*\* when it is provided. When it is left out completely, authentik simply accepts the token request with out it; even when the flow was started with a `code_challenge`. + +### Patches + +authentik 2023.8.5 and 2023.10.4 fix this issue. + +### Details + +The `code_verifier` is only checked when the user provides it. Note that in line 209 there is a check if the code_parameter is left out. But there is no check if the PKCE parameter simply was omitted WHEN the request was started with a `code_challenge_method`. + +This oversight likely did not stem from a coding error but from a misinterpretation of the RFC, where the backward compatibility section may be somewhat confusing. +https://datatracker.ietf.org/doc/html/rfc7636#section-4.5 +RFC7636 explicitly says in Section 4.5: + +> The "code_challenge_method" is bound to the Authorization Code when +> the Authorization Code is issued. That is the method that the token +> endpoint MUST use to verify the "code_verifier". + +Section 5, Compatibility + +> Server implementations of this specification MAY accept OAuth2.0 +> clients that do not implement this extension. If the "code_verifier" +> is not received from the client in the Authorization Request, servers +> supporting backwards compatibility revert to the OAuth 2.0 [[RFC6749](https://datatracker.ietf.org/doc/html/rfc6749)] +> protocol without this extension. + +Section 5, Compatibility, allows server implementations of this specification to accept OAuth 2.0 clients that do not implement this extension. However, if a `code_verifier` is not received from the client in the Authorization Request, servers that support backward compatibility should revert to the standard OAuth 2.0 protocol sans this extension (including all steps). + +It should be noted that this does not mean that the `code_verifier` check can be disregarded at any point if the initial request included `code_challenge` or `code_challenge_method`. Since Authentik supports PKCE, it **MUST** verify the code_verifier as described in Section 4.5 **AND** fail if it was not provided. + +Ofc verification can be skipped if the original authorization request did not invoke PKCE (no `code_challenge_method` and no `code_challenge`). + +Failure to check the `code_verifier` renders the PKCE flow ineffective. This vulnerability particularly endangers public or hybrid clients, as their `code` is deemed non-confidential. + +While not explicitly stated in the standard, it is generally recommended that OAuth2 flows accepting public clients should enforce PKCE - at least when redirecting to a non HTTPS URL (like http or an app link). + +### Impact + +The vulnerability poses a high risk to both public and hybrid clients. +When for example a mobile app implements oauth2, a malicious app can simply also register the same in-app-link (e.g. `mycoolapp://oauth2`) for the redirect callback URL, possibly receiving `code` during callback. With PKCE working, a malicious app would still receive a `code` but the `code` would not work without the correct unhashed code-challenge. +This is especially problematic, because authentik claims to support PKCE, and a developer can expect that the proper checks are in place. Note that app-links cannot be protected by HTTPS or similar mechanisms. + +Note also that this vulnerability poses a threat to confidential clients. Many confidential clients act as a proxy for OAuth2 API requests, typically from mobile apps or single-page applications. These proxies relay `code_challenge`, `code_challenge_method` (in auth request, which most libraries force and provide on default settings) and `code_verifier` in the token request unchanged and supplement the CLIENT_SECRET which only the relay knows. The relay can but does not have to check for an existing `code_verifier` as the standard does not define that PKCE can be ignored on confidential clients during the token request when the client requested PKCE during the authorization request. + +An attacker could potentially gain full access to the application. If the code grants access to an admin account, the confidentiality, integrity, and availability of that application are compromised. + +### For more information + +If you have any questions or comments about this advisory: + +- Email us at [security@goauthentik.io](mailto:security@goauthentik.io) diff --git a/website/sidebars.js b/website/sidebars.js index 310eeba17..fe1b6725e 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -407,6 +407,7 @@ const docsSidebar = { }, items: [ "security/policy", + "security/CVE-2023-48228", "security/GHSA-rjvp-29xq-f62w", "security/CVE-2023-39522", "security/CVE-2023-36456",