security: fix CVE 2022 23555 (#4274)

* add flow to invitation Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org> * show warning on invitation page Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org> * add security advisory Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org> * add tests Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org> Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
2022-12-23 14:13:49 +01:00 · 2022-12-23 14:13:49 +01:00 · cd88b91686
parent 8eb73d3a16
commit cd88b91686
11 changed files with 257 additions and 69 deletions
--- 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)
--- a/authentik/stages/invitation/migrations/0007_invitation_flow.py
+++ 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",
+            ),
+        ),
+    ]
--- 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."),
--- a/authentik/stages/invitation/stage.py
+++ b/authentik/stages/invitation/stage.py
@ -37,22 +37,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

--- 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"}
--- a/schema.yml
+++ b/schema.yml
@ -22277,6 +22277,10 @@ paths:
        schema:
          type: string
          format: date-time
+      - in: query
+        name: flow__slug
+        schema:
+          type: string
      - in: query
        name: name
        schema:
@ -28391,8 +28395,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:
@ -28413,6 +28427,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:
@ -33708,6 +33727,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
--- 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<Invitation, string> {
@ -66,6 +73,34 @@ export class InvitationForm extends ModelForm<Invitation, string> {
                    value="${dateTimeLocal(first(this.instance?.expires, new Date()))}"
                />
            </ak-form-element-horizontal>
+            <ak-form-element-horizontal label=${t`Flow`} ?required=${true} name="flow">
+                <select class="pf-c-form-control">
+                    <option value="" ?selected=${this.instance?.flow === undefined}>
+                        ---------
+                    </option>
+                    ${until(
+                        new FlowsApi(DEFAULT_CONFIG)
+                            .flowsInstancesList({
+                                ordering: "slug",
+                                designation: FlowsInstancesListDesignationEnum.Enrollment,
+                            })
+                            .then((flows) => {
+                                return flows.results.map((flow) => {
+                                    return html`<option
+                                        value=${ifDefined(flow.pk)}
+                                        ?selected=${this.instance?.flow === flow.pk}
+                                    >
+                                        ${flow.name} (${flow.slug})
+                                    </option>`;
+                                });
+                            }),
+                        html`<option>${t`Loading...`}</option>`,
+                    )}
+                </select>
+                <p class="pf-c-form__helper-text">
+                    ${t`When selected, the invite will only be usable with the flow. By default the invite is accepted on all flows with invitation stages.`}
+                </p>
+            </ak-form-element-horizontal>
            <ak-form-element-horizontal label=${t`Attributes`} name="fixedData">
                <ak-codemirror
                    mode="yaml"
--- a/web/src/admin/stages/invitation/InvitationListLink.ts
+++ b/web/src/admin/stages/invitation/InvitationListLink.ts
@ -14,12 +14,12 @@ import PFFormControl from "@patternfly/patternfly/components/FormControl/form-co
 import PFFlex from "@patternfly/patternfly/layouts/Flex/flex.css";
 import PFBase from "@patternfly/patternfly/patternfly-base.css";

-import { StagesApi } from "@goauthentik/api";
+import { Invitation, StagesApi } from "@goauthentik/api";

@customElement("ak-stage-invitation-list-link")
 export class InvitationListLink extends AKElement {
-    @property()
-    invitation?: string;
+    @property({ attribute: false })
+    invitation?: Invitation;

    @property()
    selectedFlow?: string;
@ -29,12 +29,14 @@ export class InvitationListLink extends AKElement {
    }

    renderLink(): string {
-        return `${window.location.protocol}//${window.location.host}/if/flow/${this.selectedFlow}/?itoken=${this.invitation}`;
+        if (this.invitation?.flowObj) {
+            this.selectedFlow = this.invitation.flowObj?.slug;
+        }
+        return `${window.location.protocol}//${window.location.host}/if/flow/${this.selectedFlow}/?itoken=${this.invitation?.pk}`;
    }

-    render(): TemplateResult {
-        return html`<dl class="pf-c-description-list pf-m-horizontal">
-            <div class="pf-c-description-list__group">
+    renderFlowSelector(): TemplateResult {
+        return html`<div class="pf-c-description-list__group">
            <dt class="pf-c-description-list__term">
                <span class="pf-c-description-list__text">${t`Select an enrollment flow`}</span>
            </dt>
@ -82,7 +84,12 @@ export class InvitationListLink extends AKElement {
                    </select>
                </div>
            </dd>
-            </div>
+        </div>`;
+    }
+
+    render(): TemplateResult {
+        return html`<dl class="pf-c-description-list pf-m-horizontal">
+            ${this.invitation?.flow === undefined ? this.renderFlowSelector() : html``}
            <div class="pf-c-description-list__group">
                <dt class="pf-c-description-list__term">
                    <span class="pf-c-description-list__text"
--- a/web/src/admin/stages/invitation/InvitationListPage.ts
+++ b/web/src/admin/stages/invitation/InvitationListPage.ts
@ -2,6 +2,7 @@ import "@goauthentik/admin/stages/invitation/InvitationForm";
 import "@goauthentik/admin/stages/invitation/InvitationListLink";
 import { DEFAULT_CONFIG } from "@goauthentik/common/api/config";
 import { uiConfig } from "@goauthentik/common/ui/config";
+import { PFColor } from "@goauthentik/elements/Label";
 import "@goauthentik/elements/buttons/ModalButton";
 import "@goauthentik/elements/buttons/SpinnerButton";
 import "@goauthentik/elements/forms/DeleteBulkForm";
@ -18,7 +19,7 @@ import { ifDefined } from "lit/directives/if-defined.js";

 import PFBanner from "@patternfly/patternfly/components/Banner/banner.css";

-import { Invitation, StagesApi } from "@goauthentik/api";
+import { FlowDesignationEnum, Invitation, StagesApi } from "@goauthentik/api";

@customElement("ak-stage-invitation-list")
 export class InvitationListPage extends TablePage<Invitation> {
@ -49,12 +50,24 @@ export class InvitationListPage extends TablePage<Invitation> {
    @state()
    invitationStageExists = false;

+    @state()
+    multipleEnrollmentFlows = false;
+
    async apiEndpoint(page: number): Promise<PaginatedResponse<Invitation>> {
+        // 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<Invitation> {

    row(item: Invitation): TemplateResult[] {
        return [
-            html`${item.name}`,
+            html`<div>${item.name}</div>
+                ${!item.flowObj && this.multipleEnrollmentFlows
+                    ? html`
+                          <ak-label color=${PFColor.Orange}>
+                              ${t`Invitation not limited to any flow, and can be used with any enrollment flow.`}
+                          </ak-label>
+                      `
+                    : html``}`,
            html`${item.createdBy?.username}`,
            html`${item.expires?.toLocaleString() || t`-`}`,
            html` <ak-forms-modal>
@ -114,7 +134,7 @@ export class InvitationListPage extends TablePage<Invitation> {
        return html` <td role="cell" colspan="3">
                <div class="pf-c-table__expandable-row-content">
                    <ak-stage-invitation-list-link
-                        invitation=${item.pk}
+                        .invitation=${item}
                    ></ak-stage-invitation-list-link>
                </div>
            </td>
--- a/website/docs/security/CVE-2022-23555.md
+++ 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)
--- a/website/sidebars.js
+++ b/website/sidebars.js
@ -293,6 +293,7 @@ module.exports = {
                "security/policy",
                "security/CVE-2022-46145",
                "security/CVE-2022-46172",
+                "security/CVE-2022-23555",
            ],
        },
    ],