From 84fbeb57214cb949822fefd53a93914a3eb94f11 Mon Sep 17 00:00:00 2001 From: Jens L Date: Fri, 23 Dec 2022 14:12:58 +0100 Subject: [PATCH] security: fix CVE 2022 46172 (#4275) * fallback to current user in user_write, add flag to disable user creation Signed-off-by: Jens Langhammer * update api and web ui Signed-off-by: Jens Langhammer * update default flows Signed-off-by: Jens Langhammer * add cve post to website Signed-off-by: Jens Langhammer * add tests Signed-off-by: Jens Langhammer Signed-off-by: Jens Langhammer --- authentik/stages/user_write/api.py | 1 + .../0006_userwritestage_can_create_users.py | 21 +++++++ authentik/stages/user_write/models.py | 10 ++++ authentik/stages/user_write/stage.py | 22 +++++--- authentik/stages/user_write/tests.py | 55 ++++++++++++++++--- .../default/0-flow-password-change.yaml | 2 + .../20-flow-default-source-enrollment.yaml | 2 + .../30-flow-default-user-settings-flow.yaml | 2 + blueprints/default/91-flow-oobe.yaml | 2 + .../example/flows-enrollment-2-stage.yaml | 3 +- .../flows-enrollment-email-verification.yaml | 1 + .../flows-recovery-email-verification.yaml | 2 + schema.yml | 16 ++++++ .../stages/user_write/UserWriteStageForm.ts | 15 +++++ .../details/UserSettingsFlowExecutor.ts | 7 --- website/docs/security/CVE-2022-46172.md | 25 +++++++++ website/sidebars.js | 6 +- 17 files changed, 167 insertions(+), 25 deletions(-) create mode 100644 authentik/stages/user_write/migrations/0006_userwritestage_can_create_users.py create mode 100644 website/docs/security/CVE-2022-46172.md diff --git a/authentik/stages/user_write/api.py b/authentik/stages/user_write/api.py index c944e2dae..32e324349 100644 --- a/authentik/stages/user_write/api.py +++ b/authentik/stages/user_write/api.py @@ -15,6 +15,7 @@ class UserWriteStageSerializer(StageSerializer): fields = StageSerializer.Meta.fields + [ "create_users_as_inactive", "create_users_group", + "can_create_users", "user_path_template", ] diff --git a/authentik/stages/user_write/migrations/0006_userwritestage_can_create_users.py b/authentik/stages/user_write/migrations/0006_userwritestage_can_create_users.py new file mode 100644 index 000000000..dda74f3fa --- /dev/null +++ b/authentik/stages/user_write/migrations/0006_userwritestage_can_create_users.py @@ -0,0 +1,21 @@ +# Generated by Django 4.1.4 on 2022-12-22 14:30 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_stages_user_write", "0005_userwritestage_user_path_template"), + ] + + operations = [ + migrations.AddField( + model_name="userwritestage", + name="can_create_users", + field=models.BooleanField( + default=True, + help_text="When set, this stage can create users. If not enabled and no user is available, stage will fail.", + ), + ), + ] diff --git a/authentik/stages/user_write/models.py b/authentik/stages/user_write/models.py index c77efed49..5fcb7e842 100644 --- a/authentik/stages/user_write/models.py +++ b/authentik/stages/user_write/models.py @@ -13,6 +13,16 @@ class UserWriteStage(Stage): """Writes currently pending data into the pending user, or if no user exists, creates a new user with the data.""" + can_create_users = models.BooleanField( + default=True, + help_text=_( + ( + "When set, this stage can create users. " + "If not enabled and no user is available, stage will fail." + ) + ), + ) + create_users_as_inactive = models.BooleanField( default=False, help_text=_("When set, newly created users are inactive and cannot login."), diff --git a/authentik/stages/user_write/stage.py b/authentik/stages/user_write/stage.py index 05f71ebe4..def921042 100644 --- a/authentik/stages/user_write/stage.py +++ b/authentik/stages/user_write/stage.py @@ -1,10 +1,9 @@ """Write stage logic""" -from typing import Any +from typing import Any, Optional -from django.contrib import messages from django.contrib.auth import update_session_auth_hash from django.db import transaction -from django.db.utils import IntegrityError +from django.db.utils import IntegrityError, InternalError from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ @@ -47,7 +46,7 @@ class UserWriteStageView(StageView): """Wrapper for post requests""" return self.get(request) - def ensure_user(self) -> tuple[User, bool]: + def ensure_user(self) -> tuple[Optional[User], bool]: """Ensure a user exists""" user_created = False path = self.executor.plan.context.get( @@ -55,7 +54,11 @@ class UserWriteStageView(StageView): ) if path == "": path = User.default_path() + if not self.request.user.is_anonymous: + self.executor.plan.context.setdefault(PLAN_CONTEXT_PENDING_USER, self.request.user) if PLAN_CONTEXT_PENDING_USER not in self.executor.plan.context: + if not self.executor.current_stage.can_create_users: + return None, False self.executor.plan.context[PLAN_CONTEXT_PENDING_USER] = User( is_active=not self.executor.current_stage.create_users_as_inactive, path=path, @@ -112,11 +115,14 @@ class UserWriteStageView(StageView): a new user is created.""" if PLAN_CONTEXT_PROMPT not in self.executor.plan.context: message = _("No Pending data.") - messages.error(request, message) self.logger.debug(message) - return self.executor.stage_invalid() + return self.executor.stage_invalid(message) data = self.executor.plan.context[PLAN_CONTEXT_PROMPT] user, user_created = self.ensure_user() + if not user: + message = _("No user found and can't create new user.") + self.logger.info(message) + return self.executor.stage_invalid(message) # Before we change anything, check if the user is the same as in the request # and we're updating a password. In that case we need to update the session hash # Also check that we're not currently impersonating, so we don't update the session @@ -139,9 +145,9 @@ class UserWriteStageView(StageView): user.ak_groups.add(self.executor.current_stage.create_users_group) if PLAN_CONTEXT_GROUPS in self.executor.plan.context: user.ak_groups.add(*self.executor.plan.context[PLAN_CONTEXT_GROUPS]) - except (IntegrityError, ValueError, TypeError) as exc: + except (IntegrityError, ValueError, TypeError, InternalError) as exc: self.logger.warning("Failed to save user", exc=exc) - return self.executor.stage_invalid() + return self.executor.stage_invalid(_("Failed to save user")) user_write.send(sender=self, request=request, user=user, data=data, created=user_created) # Check if the password has been updated, and update the session auth hash if should_update_session: diff --git a/authentik/stages/user_write/tests.py b/authentik/stages/user_write/tests.py index 8c3be63e6..0bd101fd0 100644 --- a/authentik/stages/user_write/tests.py +++ b/authentik/stages/user_write/tests.py @@ -1,6 +1,4 @@ """write tests""" -import string -from random import SystemRandom from unittest.mock import patch from django.urls import reverse @@ -14,6 +12,7 @@ from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan from authentik.flows.tests import FlowTestCase from authentik.flows.tests.test_executor import TO_STAGE_RESPONSE_MOCK from authentik.flows.views.executor import SESSION_KEY_PLAN +from authentik.lib.generators import generate_key from authentik.stages.prompt.stage import PLAN_CONTEXT_PROMPT from authentik.stages.user_write.models import UserWriteStage from authentik.stages.user_write.stage import PLAN_CONTEXT_GROUPS, UserWriteStageView @@ -32,12 +31,11 @@ class TestUserWriteStage(FlowTestCase): ) self.binding = FlowStageBinding.objects.create(target=self.flow, stage=self.stage, order=2) self.source = Source.objects.create(name="fake_source") + self.user = create_test_admin_user() def test_user_create(self): """Test creation of user""" - password = "".join( - SystemRandom().choice(string.ascii_uppercase + string.digits) for _ in range(8) - ) + password = generate_key() plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) plan.context[PLAN_CONTEXT_PROMPT] = { @@ -66,9 +64,7 @@ class TestUserWriteStage(FlowTestCase): def test_user_update(self): """Test update of existing user""" - new_password = "".join( - SystemRandom().choice(string.ascii_uppercase + string.digits) for _ in range(8) - ) + new_password = generate_key() plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) plan.context[PLAN_CONTEXT_PENDING_USER] = User.objects.create( username="unittest", email="test@goauthentik.io" @@ -142,6 +138,49 @@ class TestUserWriteStage(FlowTestCase): component="ak-stage-access-denied", ) + def test_authenticated_no_user(self): + """Test user in session and none in plan""" + plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) + self.client.force_login(self.user) + session = self.client.session + plan.context[PLAN_CONTEXT_PROMPT] = { + "username": "foo", + "attribute_some-custom-attribute": "test", + "some_ignored_attribute": "bar", + } + session[SESSION_KEY_PLAN] = plan + session.save() + + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + ) + self.assertStageRedirects(response, reverse("authentik_core:root-redirect")) + self.user.refresh_from_db() + self.assertEqual(self.user.username, "foo") + + def test_no_create(self): + """Test can_create_users set to false""" + self.stage.can_create_users = False + self.stage.save() + plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) + session = self.client.session + plan.context[PLAN_CONTEXT_PROMPT] = { + "username": "foo", + "attribute_some-custom-attribute": "test", + "some_ignored_attribute": "bar", + } + session[SESSION_KEY_PLAN] = plan + session.save() + + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + ) + self.assertStageResponse( + response, + self.flow, + component="ak-stage-access-denied", + ) + @patch( "authentik.flows.views.executor.to_stage_response", TO_STAGE_RESPONSE_MOCK, diff --git a/blueprints/default/0-flow-password-change.yaml b/blueprints/default/0-flow-password-change.yaml index 60a4d72e7..6c6866bfc 100644 --- a/blueprints/default/0-flow-password-change.yaml +++ b/blueprints/default/0-flow-password-change.yaml @@ -45,6 +45,8 @@ entries: name: default-password-change-write id: default-password-change-write model: authentik_stages_user_write.userwritestage + attrs: + can_create_users: false - identifiers: order: 0 stage: !KeyOf default-password-change-prompt diff --git a/blueprints/default/20-flow-default-source-enrollment.yaml b/blueprints/default/20-flow-default-source-enrollment.yaml index e354104db..8631190c4 100644 --- a/blueprints/default/20-flow-default-source-enrollment.yaml +++ b/blueprints/default/20-flow-default-source-enrollment.yaml @@ -57,6 +57,8 @@ entries: name: default-source-enrollment-write id: default-source-enrollment-write model: authentik_stages_user_write.userwritestage + attrs: + can_create_users: true - attrs: re_evaluate_policies: true identifiers: diff --git a/blueprints/default/30-flow-default-user-settings-flow.yaml b/blueprints/default/30-flow-default-user-settings-flow.yaml index 34a593be7..c50779025 100644 --- a/blueprints/default/30-flow-default-user-settings-flow.yaml +++ b/blueprints/default/30-flow-default-user-settings-flow.yaml @@ -109,6 +109,8 @@ entries: model: authentik_policies_expression.expressionpolicy - identifiers: name: default-user-settings-write + attrs: + can_create_users: false id: default-user-settings-write model: authentik_stages_user_write.userwritestage - attrs: diff --git a/blueprints/default/91-flow-oobe.yaml b/blueprints/default/91-flow-oobe.yaml index 8e659d532..66e957aee 100644 --- a/blueprints/default/91-flow-oobe.yaml +++ b/blueprints/default/91-flow-oobe.yaml @@ -102,6 +102,8 @@ entries: identifiers: name: default-password-change-write model: authentik_stages_user_write.userwritestage + attrs: + can_create_users: false - attrs: evaluate_on_plan: true invalid_response_action: retry diff --git a/blueprints/example/flows-enrollment-2-stage.yaml b/blueprints/example/flows-enrollment-2-stage.yaml index 670befc99..3e4f3dcf3 100644 --- a/blueprints/example/flows-enrollment-2-stage.yaml +++ b/blueprints/example/flows-enrollment-2-stage.yaml @@ -95,7 +95,8 @@ entries: name: default-enrollment-user-write id: default-enrollment-user-write model: authentik_stages_user_write.userwritestage - attrs: {} + attrs: + can_create_users: true - identifiers: target: !KeyOf flow stage: !KeyOf default-enrollment-prompt-first diff --git a/blueprints/example/flows-enrollment-email-verification.yaml b/blueprints/example/flows-enrollment-email-verification.yaml index 3528906a1..ffb6d63f6 100644 --- a/blueprints/example/flows-enrollment-email-verification.yaml +++ b/blueprints/example/flows-enrollment-email-verification.yaml @@ -114,6 +114,7 @@ entries: model: authentik_stages_user_write.userwritestage attrs: create_users_as_inactive: true + can_create_users: true - identifiers: target: !KeyOf flow stage: !KeyOf default-enrollment-prompt-first diff --git a/blueprints/example/flows-recovery-email-verification.yaml b/blueprints/example/flows-recovery-email-verification.yaml index 8b1994cc8..79a7cfc10 100644 --- a/blueprints/example/flows-recovery-email-verification.yaml +++ b/blueprints/example/flows-recovery-email-verification.yaml @@ -63,6 +63,8 @@ entries: name: default-recovery-user-write id: default-recovery-user-write model: authentik_stages_user_write.userwritestage + attrs: + can_create_users: false - identifiers: name: default-recovery-identification id: default-recovery-identification diff --git a/schema.yml b/schema.yml index 73ca76df9..0e8ade713 100644 --- a/schema.yml +++ b/schema.yml @@ -24634,6 +24634,10 @@ paths: operationId: stages_user_write_list description: UserWriteStage Viewset parameters: + - in: query + name: can_create_users + schema: + type: boolean - in: query name: create_users_as_inactive schema: @@ -35122,6 +35126,10 @@ components: format: uuid nullable: true description: Optionally add newly created users to this group. + can_create_users: + type: boolean + description: When set, this stage can create users. If not enabled and no + user is available, stage will fail. user_path_template: type: string PatchedWebAuthnDeviceRequest: @@ -38341,6 +38349,10 @@ components: format: uuid nullable: true description: Optionally add newly created users to this group. + can_create_users: + type: boolean + description: When set, this stage can create users. If not enabled and no + user is available, stage will fail. user_path_template: type: string required: @@ -38369,6 +38381,10 @@ components: format: uuid nullable: true description: Optionally add newly created users to this group. + can_create_users: + type: boolean + description: When set, this stage can create users. If not enabled and no + user is available, stage will fail. user_path_template: type: string required: diff --git a/web/src/admin/stages/user_write/UserWriteStageForm.ts b/web/src/admin/stages/user_write/UserWriteStageForm.ts index 1b6db9776..189ee8649 100644 --- a/web/src/admin/stages/user_write/UserWriteStageForm.ts +++ b/web/src/admin/stages/user_write/UserWriteStageForm.ts @@ -59,6 +59,21 @@ export class UserWriteStageForm extends ModelForm { ${t`Stage-specific settings`}
+ +
+ + +
+

+ ${t`When enabled, this stage has the ability to create new users. If no user is available in the flow with this disabled, the stage will fail.`} +

+
{ - this.nextChallenge(); - }); }); } diff --git a/website/docs/security/CVE-2022-46172.md b/website/docs/security/CVE-2022-46172.md new file mode 100644 index 000000000..e44c298af --- /dev/null +++ b/website/docs/security/CVE-2022-46172.md @@ -0,0 +1,25 @@ +# CVE-2022-46172 + +## Existing Authenticated Users can Create Arbitrary Accounts + +### Summary + +Any authenticated user can create an arbitrary number of accounts through the default flows. This would circumvent any policy in a situation where it is undesirable for users to create new accounts by themselves. This may also have carry over consequences to other applications being how these new basic accounts would exist throughout the SSO infrastructure. By default the newly created accounts cannot be logged into as no password reset exists by default. However password resets are likely to be enabled by most installations. + +### Patches + +authentik 2022.11.4, 2022.10.4 and 2022.12.0 fix this issue. + +### Impact + +This vulnerability could make it much easier for name and email collisions to occur, making it harder for user to log in. This also makes it more difficult for admins to properly administer users since more and more confusing users will exist. This paired with password reset flows if enabled would mean a circumvention of on-boarding policies. Say for instance a company wanted to invite a limited number of beta testers, those beta testers would be able to create an arbitrary number of accounts themselves. + +### Details + +This vulnerability has already been submitted over email, this security advisory serves as formalization towards broader information dissemination. This vulnerability pertains to the user context used in the default-user-settings-flow. /api/v3/flows/instances/default-user-settings-flow/execute/ + +### 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 9ab48af07..60bfbb4ef 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -292,7 +292,11 @@ module.exports = { title: "Security", slug: "security", }, - items: ["security/policy", "security/CVE-2022-46145"], + items: [ + "security/policy", + "security/CVE-2022-46145", + "security/CVE-2022-46172", + ], }, ], };