From 9f846d94be69ac7014b40e7293c3a3430408c082 Mon Sep 17 00:00:00 2001 From: Jens L Date: Fri, 23 Dec 2022 14:13:49 +0100 Subject: [PATCH] security: fix CVE 2022 23555 (#4274) * add flow to invitation Signed-off-by: Jens Langhammer * show warning on invitation page Signed-off-by: Jens Langhammer * add security advisory Signed-off-by: Jens Langhammer * add tests Signed-off-by: Jens Langhammer Signed-off-by: Jens Langhammer --- authentik/stages/invitation/api.py | 8 +- .../migrations/0007_invitation_flow.py | 26 ++++ authentik/stages/invitation/models.py | 7 ++ authentik/stages/invitation/stage.py | 26 ++-- authentik/stages/invitation/tests.py | 29 ++++- schema.yml | 24 ++++ .../admin/stages/invitation/InvitationForm.ts | 37 +++++- .../stages/invitation/InvitationListLink.ts | 113 ++++++++++-------- .../stages/invitation/InvitationListPage.ts | 26 +++- website/docs/security/CVE-2022-23555.md | 29 +++++ website/sidebars.js | 1 + 11 files changed, 257 insertions(+), 69 deletions(-) create mode 100644 authentik/stages/invitation/migrations/0007_invitation_flow.py create mode 100644 website/docs/security/CVE-2022-23555.md diff --git a/authentik/stages/invitation/api.py b/authentik/stages/invitation/api.py index 56b60c02b..0c77cec4a 100644 --- a/authentik/stages/invitation/api.py +++ b/authentik/stages/invitation/api.py @@ -8,6 +8,7 @@ from rest_framework.viewsets import ModelViewSet from authentik.core.api.groups import GroupMemberSerializer from authentik.core.api.used_by import UsedByMixin from authentik.core.api.utils import is_dict +from authentik.flows.api.flows import FlowSerializer from authentik.flows.api.stages import StageSerializer from authentik.stages.invitation.models import Invitation, InvitationStage @@ -49,6 +50,7 @@ class InvitationSerializer(ModelSerializer): created_by = GroupMemberSerializer(read_only=True) fixed_data = JSONField(validators=[is_dict], required=False) + flow_obj = FlowSerializer(read_only=True, required=False, source="flow") class Meta: @@ -60,6 +62,8 @@ class InvitationSerializer(ModelSerializer): "fixed_data", "created_by", "single_use", + "flow", + "flow_obj", ] @@ -69,8 +73,8 @@ class InvitationViewSet(UsedByMixin, ModelViewSet): queryset = Invitation.objects.all() serializer_class = InvitationSerializer ordering = ["-expires"] - search_fields = ["name", "created_by__username", "expires"] - filterset_fields = ["name", "created_by__username", "expires"] + search_fields = ["name", "created_by__username", "expires", "flow__slug"] + filterset_fields = ["name", "created_by__username", "expires", "flow__slug"] def perform_create(self, serializer: InvitationSerializer): serializer.save(created_by=self.request.user) diff --git a/authentik/stages/invitation/migrations/0007_invitation_flow.py b/authentik/stages/invitation/migrations/0007_invitation_flow.py new file mode 100644 index 000000000..2842a3039 --- /dev/null +++ b/authentik/stages/invitation/migrations/0007_invitation_flow.py @@ -0,0 +1,26 @@ +# Generated by Django 4.1.4 on 2022-12-20 13:43 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_flows", "0024_flow_authentication"), + ("authentik_stages_invitation", "0001_squashed_0006_invitation_name"), + ] + + operations = [ + migrations.AddField( + model_name="invitation", + name="flow", + field=models.ForeignKey( + default=None, + help_text="When set, only the configured flow can use this invitation.", + null=True, + on_delete=django.db.models.deletion.SET_DEFAULT, + to="authentik_flows.flow", + ), + ), + ] diff --git a/authentik/stages/invitation/models.py b/authentik/stages/invitation/models.py index 6c69cbd36..6ea564ae0 100644 --- a/authentik/stages/invitation/models.py +++ b/authentik/stages/invitation/models.py @@ -55,6 +55,13 @@ class Invitation(SerializerModel, ExpiringModel): name = models.SlugField() + flow = models.ForeignKey( + "authentik_flows.Flow", + default=None, + null=True, + on_delete=models.SET_DEFAULT, + help_text=_("When set, only the configured flow can use this invitation."), + ) single_use = models.BooleanField( default=False, help_text=_("When enabled, the invitation will be deleted after usage."), diff --git a/authentik/stages/invitation/stage.py b/authentik/stages/invitation/stage.py index 44688cddd..d916ac68a 100644 --- a/authentik/stages/invitation/stage.py +++ b/authentik/stages/invitation/stage.py @@ -35,22 +35,30 @@ class InvitationStageView(StageView): return self.executor.plan.context[PLAN_CONTEXT_PROMPT][INVITATION_TOKEN_KEY_CONTEXT] return None + def get_invite(self) -> Optional[Invitation]: + """Check the token, find the invite and check it's flow""" + token = self.get_token() + if not token: + return None + invite: Invitation = Invitation.objects.filter(pk=token).first() + if not invite: + self.logger.debug("invalid invitation", token=token) + return None + if invite.flow and invite.flow.pk != self.executor.plan.flow_pk: + self.logger.debug("invite for incorrect flow", expected=invite.flow.slug) + return None + return invite + def get(self, request: HttpRequest) -> HttpResponse: """Apply data to the current flow based on a URL""" stage: InvitationStage = self.executor.current_stage - token = self.get_token() - if not token: - # No Invitation was given, raise error or continue + + invite = self.get_invite() + if not invite: if stage.continue_flow_without_invitation: return self.executor.stage_ok() return self.executor.stage_invalid() - invite: Invitation = Invitation.objects.filter(pk=token).first() - if not invite: - self.logger.debug("invalid invitation", token=token) - if stage.continue_flow_without_invitation: - return self.executor.stage_ok() - return self.executor.stage_invalid() self.executor.plan.context[INVITATION_IN_EFFECT] = True self.executor.plan.context[INVITATION] = invite diff --git a/authentik/stages/invitation/tests.py b/authentik/stages/invitation/tests.py index 0441b2921..75989a147 100644 --- a/authentik/stages/invitation/tests.py +++ b/authentik/stages/invitation/tests.py @@ -23,7 +23,7 @@ from authentik.stages.password import BACKEND_INBUILT from authentik.stages.password.stage import PLAN_CONTEXT_AUTHENTICATION_BACKEND -class TestUserLoginStage(FlowTestCase): +class TestInvitationStage(FlowTestCase): """Login tests""" def setUp(self): @@ -98,6 +98,33 @@ class TestUserLoginStage(FlowTestCase): self.assertEqual(response.status_code, 200) self.assertStageRedirects(response, reverse("authentik_core:root-redirect")) + def test_invalid_flow(self): + """Test with invitation, invalid flow limit""" + invalid_flow = create_test_flow(FlowDesignation.ENROLLMENT) + plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + data = {"foo": "bar"} + invite = Invitation.objects.create( + created_by=get_anonymous_user(), fixed_data=data, flow=invalid_flow + ) + + with patch("authentik.flows.views.executor.FlowExecutorView.cancel", MagicMock()): + base_url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + args = urlencode({INVITATION_TOKEN_KEY: invite.pk.hex}) + response = self.client.get(base_url + f"?query={args}") + + session = self.client.session + plan: FlowPlan = session[SESSION_KEY_PLAN] + + self.assertStageResponse( + response, + flow=self.flow, + component="ak-stage-access-denied", + ) + def test_with_invitation_prompt_data(self): """Test with invitation, check data in session""" data = {"foo": "bar"} diff --git a/schema.yml b/schema.yml index 0e8ade713..b2949dd65 100644 --- a/schema.yml +++ b/schema.yml @@ -22391,6 +22391,10 @@ paths: schema: type: string format: date-time + - in: query + name: flow__slug + schema: + type: string - in: query name: name schema: @@ -28531,8 +28535,18 @@ components: single_use: type: boolean description: When enabled, the invitation will be deleted after usage. + flow: + type: string + format: uuid + nullable: true + description: When set, only the configured flow can use this invitation. + flow_obj: + allOf: + - $ref: '#/components/schemas/Flow' + readOnly: true required: - created_by + - flow_obj - name - pk InvitationRequest: @@ -28553,6 +28567,11 @@ components: single_use: type: boolean description: When enabled, the invitation will be deleted after usage. + flow: + type: string + format: uuid + nullable: true + description: When set, only the configured flow can use this invitation. required: - name InvitationStage: @@ -33904,6 +33923,11 @@ components: single_use: type: boolean description: When enabled, the invitation will be deleted after usage. + flow: + type: string + format: uuid + nullable: true + description: When set, only the configured flow can use this invitation. PatchedInvitationStageRequest: type: object description: InvitationStage Serializer diff --git a/web/src/admin/stages/invitation/InvitationForm.ts b/web/src/admin/stages/invitation/InvitationForm.ts index 8a0066199..b8174faa2 100644 --- a/web/src/admin/stages/invitation/InvitationForm.ts +++ b/web/src/admin/stages/invitation/InvitationForm.ts @@ -9,8 +9,15 @@ import { t } from "@lingui/macro"; import { TemplateResult, html } from "lit"; import { customElement } from "lit/decorators.js"; +import { ifDefined } from "lit/directives/if-defined.js"; +import { until } from "lit/directives/until.js"; -import { Invitation, StagesApi } from "@goauthentik/api"; +import { + FlowsApi, + FlowsInstancesListDesignationEnum, + Invitation, + StagesApi, +} from "@goauthentik/api"; @customElement("ak-invitation-form") export class InvitationForm extends ModelForm { @@ -66,6 +73,34 @@ export class InvitationForm extends ModelForm { value="${dateTimeLocal(first(this.instance?.expires, new Date()))}" /> + + +

+ ${t`When selected, the invite will only be usable with the flow. By default the invite is accepted on all flows with invitation stages.`} +

+ +
+ ${t`Select an enrollment flow`} +
+
+
+ +
+
+ `; } render(): TemplateResult { return html`
-
-
- ${t`Select an enrollment flow`} -
-
-
- -
-
-
+ ${this.invitation?.flow === undefined ? this.renderFlowSelector() : html``}
{ @@ -49,12 +50,24 @@ export class InvitationListPage extends TablePage { @state() invitationStageExists = false; + @state() + multipleEnrollmentFlows = false; + async apiEndpoint(page: number): Promise> { + // Check if any invitation stages exist const stages = await new StagesApi(DEFAULT_CONFIG).stagesInvitationStagesList({ noFlows: false, }); this.invitationStageExists = stages.pagination.count > 0; this.expandable = this.invitationStageExists; + stages.results.forEach((stage) => { + const enrollmentFlows = (stage.flowSet || []).filter( + (flow) => flow.designation === FlowDesignationEnum.Enrollment, + ); + if (enrollmentFlows.length > 1) { + this.multipleEnrollmentFlows = true; + } + }); return new StagesApi(DEFAULT_CONFIG).stagesInvitationInvitationsList({ ordering: this.order, page: page, @@ -96,7 +109,14 @@ export class InvitationListPage extends TablePage { row(item: Invitation): TemplateResult[] { return [ - html`${item.name}`, + html`
${item.name}
+ ${!item.flowObj && this.multipleEnrollmentFlows + ? html` + + ${t`Invitation not limited to any flow, and can be used with any enrollment flow.`} + + ` + : html``}`, html`${item.createdBy?.username}`, html`${item.expires?.toLocaleString() || t`-`}`, html` @@ -114,7 +134,7 @@ export class InvitationListPage extends TablePage { return html`
diff --git a/website/docs/security/CVE-2022-23555.md b/website/docs/security/CVE-2022-23555.md new file mode 100644 index 000000000..0c922ac05 --- /dev/null +++ b/website/docs/security/CVE-2022-23555.md @@ -0,0 +1,29 @@ +# CVE-2022-23555 + +## Token reuse in invitation URLs leads to access control bypass via the use of a different enrollment flow + +### Summary + +Token reuse in invitation URLs leads to access control bypass via the use of a different enrollment flow than in the one provided. + +### Patches + +authentik 2022.11.4, 2022.10.4 and 2022.12.0 fix this issue, for other versions the workaround can be used. + +### Impact + +Only configurations using both invitations and have multiple enrollment flows with invitation stages that grant different permissions are affected. The default configuration is not vulnerable, and neither are configurations with a single enrollment flow. + +### Details + +The vulnerability allows an attacker that knows different invitation flows names (e.g. `enrollment-invitation-test` and `enrollment-invitation-admin`) via either different invite links or via brute forcing to signup via a single invitation url for any valid invite link received (it can even be a url for a third flow as long as it's a valid invite) as the token used in the `Invitations` section of the Admin interface does NOT change when a different `enrollment flow` is selected via the interface and it is NOT bound to the selected flow, so it will be valid for any flow when used. + +### Workarounds + +As a workaround, fixed data can be added to invitations which can be checked in the flow to deny requests. Alternatively, an identifier with high entropy (like a UUID) can be used as flow slug, mitigating the attack vector by exponentially decreasing the possibility of discovering other flows. + +### 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 60bfbb4ef..f51ecedb3 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -296,6 +296,7 @@ module.exports = { "security/policy", "security/CVE-2022-46145", "security/CVE-2022-46172", + "security/CVE-2022-23555", ], }, ],