From aa874dd92a770d5f8cd8f265b7cdd31cd73a4599 Mon Sep 17 00:00:00 2001 From: Jens L Date: Tue, 29 Aug 2023 19:07:49 +0200 Subject: [PATCH] security: fix CVE-2023-39522 (#6665) * stages/email: don't disclose whether a user exists or not when recovering Signed-off-by: Jens Langhammer * update website Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/stages/email/stage.py | 7 +++- authentik/stages/email/tests/test_sending.py | 39 +++++++++++++++++++- authentik/stages/identification/stage.py | 4 ++ authentik/stages/identification/tests.py | 34 ++++++++++++++++- website/docs/releases/2023/v2023.5.md | 4 ++ website/docs/releases/2023/v2023.6.md | 4 ++ website/docs/security/CVE-2023-39522.md | 27 ++++++++++++++ website/sidebars.js | 1 + 8 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 website/docs/security/CVE-2023-39522.md diff --git a/authentik/stages/email/stage.py b/authentik/stages/email/stage.py index f116f7de0..a28fb3f39 100644 --- a/authentik/stages/email/stage.py +++ b/authentik/stages/email/stage.py @@ -12,7 +12,7 @@ from rest_framework.fields import CharField from rest_framework.serializers import ValidationError from authentik.flows.challenge import Challenge, ChallengeResponse, ChallengeTypes -from authentik.flows.models import FlowToken +from authentik.flows.models import FlowDesignation, FlowToken from authentik.flows.planner import PLAN_CONTEXT_IS_RESTORED, PLAN_CONTEXT_PENDING_USER from authentik.flows.stage import ChallengeStageView from authentik.flows.views.executor import QS_KEY_TOKEN @@ -82,6 +82,11 @@ class EmailStageView(ChallengeStageView): """Helper function that sends the actual email. Implies that you've already checked that there is a pending user.""" pending_user = self.get_pending_user() + if not pending_user.pk and self.executor.flow.designation == FlowDesignation.RECOVERY: + # Pending user does not have a primary key, and we're in a recovery flow, + # which means the user entered an invalid identifier, so we pretend to send the + # email, to not disclose if the user exists + return email = self.executor.plan.context.get(PLAN_CONTEXT_EMAIL_OVERRIDE, None) if not email: email = pending_user.email diff --git a/authentik/stages/email/tests/test_sending.py b/authentik/stages/email/tests/test_sending.py index b1c2aea15..424d474ce 100644 --- a/authentik/stages/email/tests/test_sending.py +++ b/authentik/stages/email/tests/test_sending.py @@ -5,18 +5,20 @@ from unittest.mock import MagicMock, PropertyMock, patch from django.core import mail from django.core.mail.backends.locmem import EmailBackend from django.urls import reverse -from rest_framework.test import APITestCase +from authentik.core.models import User from authentik.core.tests.utils import create_test_admin_user, create_test_flow from authentik.events.models import Event, EventAction from authentik.flows.markers import StageMarker from authentik.flows.models import FlowDesignation, FlowStageBinding from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan +from authentik.flows.tests import FlowTestCase from authentik.flows.views.executor import SESSION_KEY_PLAN +from authentik.lib.generators import generate_id from authentik.stages.email.models import EmailStage -class TestEmailStageSending(APITestCase): +class TestEmailStageSending(FlowTestCase): """Email tests""" def setUp(self): @@ -44,6 +46,13 @@ class TestEmailStageSending(APITestCase): ): response = self.client.post(url) self.assertEqual(response.status_code, 200) + self.assertStageResponse( + response, + self.flow, + response_errors={ + "non_field_errors": [{"string": "email-sent", "code": "email-sent"}] + }, + ) self.assertEqual(len(mail.outbox), 1) self.assertEqual(mail.outbox[0].subject, "authentik") events = Event.objects.filter(action=EventAction.EMAIL_SENT) @@ -54,6 +63,32 @@ class TestEmailStageSending(APITestCase): self.assertEqual(event.context["to_email"], [self.user.email]) self.assertEqual(event.context["from_email"], "system@authentik.local") + def test_pending_fake_user(self): + """Test with pending (fake) user""" + self.flow.designation = FlowDesignation.RECOVERY + self.flow.save() + plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) + plan.context[PLAN_CONTEXT_PENDING_USER] = User(username=generate_id()) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + with patch( + "authentik.stages.email.models.EmailStage.backend_class", + PropertyMock(return_value=EmailBackend), + ): + response = self.client.post(url) + self.assertEqual(response.status_code, 200) + self.assertStageResponse( + response, + self.flow, + response_errors={ + "non_field_errors": [{"string": "email-sent", "code": "email-sent"}] + }, + ) + self.assertEqual(len(mail.outbox), 0) + def test_send_error(self): """Test error during sending (sending will be retried)""" plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) diff --git a/authentik/stages/identification/stage.py b/authentik/stages/identification/stage.py index a54de07d9..3a6a5bd25 100644 --- a/authentik/stages/identification/stage.py +++ b/authentik/stages/identification/stage.py @@ -118,8 +118,12 @@ class IdentificationChallengeResponse(ChallengeResponse): username=uid_field, email=uid_field, ) + self.pre_user = self.stage.executor.plan.context[PLAN_CONTEXT_PENDING_USER] if not current_stage.show_matched_user: self.stage.executor.plan.context[PLAN_CONTEXT_PENDING_USER_IDENTIFIER] = uid_field + if self.stage.executor.flow.designation == FlowDesignation.RECOVERY: + # When used in a recovery flow, always continue to not disclose if a user exists + return attrs raise ValidationError("Failed to authenticate.") self.pre_user = pre_user if not current_stage.password_stage: diff --git a/authentik/stages/identification/tests.py b/authentik/stages/identification/tests.py index cdd7bf0e9..dabdea050 100644 --- a/authentik/stages/identification/tests.py +++ b/authentik/stages/identification/tests.py @@ -188,7 +188,7 @@ class TestIdentificationStage(FlowTestCase): ], ) - def test_recovery_flow(self): + def test_link_recovery_flow(self): """Test that recovery flow is linked correctly""" flow = create_test_flow() self.stage.recovery_flow = flow @@ -226,6 +226,38 @@ class TestIdentificationStage(FlowTestCase): ], ) + def test_recovery_flow_invalid_user(self): + """Test that an invalid user can proceed in a recovery flow""" + self.flow.designation = FlowDesignation.RECOVERY + self.flow.save() + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}), + ) + self.assertStageResponse( + response, + self.flow, + component="ak-stage-identification", + user_fields=["email"], + password_fields=False, + show_source_labels=False, + primary_action="Continue", + sources=[ + { + "challenge": { + "component": "xak-flow-redirect", + "to": "/source/oauth/login/test/", + "type": ChallengeTypes.REDIRECT.value, + }, + "icon_url": "/static/authentik/sources/default.svg", + "name": "test", + } + ], + ) + form_data = {"uid_field": generate_id()} + url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + response = self.client.post(url, form_data) + self.assertEqual(response.status_code, 200) + def test_api_validate(self): """Test API validation""" self.assertTrue( diff --git a/website/docs/releases/2023/v2023.5.md b/website/docs/releases/2023/v2023.5.md index f879b205e..5ec1d2f68 100644 --- a/website/docs/releases/2023/v2023.5.md +++ b/website/docs/releases/2023/v2023.5.md @@ -152,6 +152,10 @@ image: - \*: fix [CVE-2023-36456](../security/CVE-2023-36456), Reported by [@thijsa](https://github.com/thijsa) +## Fixed in 2023.5.6 + +- \*: fix [CVE-2023-39522](../security/CVE-2023-39522), Reported by [@markrassamni](https://github.com/markrassamni) + ## API Changes #### What's Changed diff --git a/website/docs/releases/2023/v2023.6.md b/website/docs/releases/2023/v2023.6.md index a81ade848..7032044d0 100644 --- a/website/docs/releases/2023/v2023.6.md +++ b/website/docs/releases/2023/v2023.6.md @@ -88,6 +88,10 @@ helm upgrade authentik authentik/authentik -f values.yaml --version ^2023.6 - sources/ldap: fix more errors (#6191) - sources/ldap: fix page size (#6187) +## Fixed in 2023.6.2 + +- \*: fix [CVE-2023-39522](../security/CVE-2023-39522), Reported by [@markrassamni](https://github.com/markrassamni) + ## API Changes #### What's New diff --git a/website/docs/security/CVE-2023-39522.md b/website/docs/security/CVE-2023-39522.md new file mode 100644 index 000000000..18f09b134 --- /dev/null +++ b/website/docs/security/CVE-2023-39522.md @@ -0,0 +1,27 @@ +# CVE-2023-39522 + +_Reported by [@markrassamni](https://github.com/markrassamni)_ + +## Username enumeration attack + +### Summary + +Using a recovery flow with an identification stage an attacker is able to determine if a username exists. + +### Patches + +authentik 2023.5.6 and 2023.6.2 fix this issue. + +### Impact + +Only setups configured with a recovery flow are impacted by this. + +### Details + +An attacker can easily enumerate and check users' existence using the recovery flow, as a clear message is shown when a user doesn't exist. Depending on configuration this can either be done by username, email, or both. + +### 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 79a8886fe..01f84ba55 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -361,6 +361,7 @@ const docsSidebar = { }, items: [ "security/policy", + "security/CVE-2023-39522", "security/CVE-2023-36456", "security/2023-06-cure53", "security/CVE-2023-26481",