Skip to content

Commit

Permalink
security: fix CVE-2024-38371 (#10229)
Browse files Browse the repository at this point in the history
  • Loading branch information
BeryJu authored and rissson committed Jun 26, 2024
1 parent 6bb180f commit c9c6a73
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 53 deletions.
23 changes: 22 additions & 1 deletion authentik/providers/oauth2/tests/test_device_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

from django.urls import reverse

from authentik.core.models import Application
from authentik.core.models import Application, Group
from authentik.core.tests.utils import create_test_admin_user, create_test_brand, create_test_flow
from authentik.lib.generators import generate_id
from authentik.policies.models import PolicyBinding
from authentik.providers.oauth2.models import DeviceToken, OAuth2Provider
from authentik.providers.oauth2.tests.utils import OAuthTestCase
from authentik.providers.oauth2.views.device_init import QS_KEY_CODE
Expand Down Expand Up @@ -77,3 +78,23 @@ def test_device_init_qs(self):
+ "?"
+ urlencode({QS_KEY_CODE: token.user_code}),
)

def test_device_init_denied(self):
"""Test device init"""
group = Group.objects.create(name="foo")
PolicyBinding.objects.create(
group=group,
target=self.application,
order=0,
)
token = DeviceToken.objects.create(
user_code="foo",
provider=self.provider,
)
res = self.client.get(
reverse("authentik_providers_oauth2_root:device-login")
+ "?"
+ urlencode({QS_KEY_CODE: token.user_code})
)
self.assertEqual(res.status_code, 200)
self.assertIn(b"Permission denied", res.content)
7 changes: 5 additions & 2 deletions authentik/providers/oauth2/views/device_backchannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
from rest_framework.throttling import AnonRateThrottle
from structlog.stdlib import get_logger

from authentik.core.models import Application
from authentik.lib.config import CONFIG
from authentik.lib.utils.time import timedelta_from_string
from authentik.providers.oauth2.models import DeviceToken, OAuth2Provider
from authentik.providers.oauth2.views.device_init import QS_KEY_CODE, get_application
from authentik.providers.oauth2.views.device_init import QS_KEY_CODE

LOGGER = get_logger()

Expand All @@ -38,7 +39,9 @@ def parse_request(self) -> Optional[HttpResponse]:
).first()
if not provider:
return HttpResponseBadRequest()
if not get_application(provider):
try:
_ = provider.application
except Application.DoesNotExist:
return HttpResponseBadRequest()
self.provider = provider
self.client_id = client_id
Expand Down
107 changes: 57 additions & 50 deletions authentik/providers/oauth2/views/device_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

from django.http import HttpRequest, HttpResponse
from django.utils.translation import gettext as _
from django.views import View
from rest_framework.exceptions import ErrorDetail
from typing import Any

from rest_framework.exceptions import ValidationError
from rest_framework.fields import CharField, IntegerField
from structlog.stdlib import get_logger

Expand All @@ -18,7 +19,8 @@
from authentik.flows.stage import ChallengeStageView
from authentik.flows.views.executor import SESSION_KEY_PLAN
from authentik.lib.utils.urls import redirect_with_qs
from authentik.providers.oauth2.models import DeviceToken, OAuth2Provider
from authentik.policies.views import PolicyAccessView
from authentik.providers.oauth2.models import DeviceToken
from authentik.providers.oauth2.views.device_finish import (
PLAN_CONTEXT_DEVICE,
OAuthDeviceCodeFinishStage,
Expand All @@ -44,48 +46,52 @@ def get_application(provider: OAuth2Provider) -> Optional[Application]:
return None


def validate_code(code: int, request: HttpRequest) -> Optional[HttpResponse]:
"""Validate user token"""
token = DeviceToken.objects.filter(
user_code=code,
).first()
if not token:
return None
class CodeValidatorView(PolicyAccessView):
"""Helper to validate frontside token"""

app = get_application(token.provider)
if not app:
return None
def __init__(self, code: str, **kwargs: Any) -> None:
super().__init__(**kwargs)
self.code = code

scope_descriptions = UserInfoView().get_scope_descriptions(token.scope, token.provider)
planner = FlowPlanner(token.provider.authorization_flow)
planner.allow_empty_flows = True
try:
plan = planner.plan(
request,
{
PLAN_CONTEXT_SSO: True,
PLAN_CONTEXT_APPLICATION: app,
# OAuth2 related params
PLAN_CONTEXT_DEVICE: token,
# Consent related params
PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.")
% {"application": app.name},
PLAN_CONTEXT_CONSENT_PERMISSIONS: scope_descriptions,
},
def resolve_provider_application(self):
self.token = DeviceToken.objects.filter(user_code=self.code).first()
if not self.token:
raise Application.DoesNotExist
self.provider = self.token.provider
self.application = self.token.provider.application

def get(self, request: HttpRequest, *args, **kwargs):
scope_descriptions = UserInfoView().get_scope_descriptions(self.token.scope, self.provider)
planner = FlowPlanner(self.provider.authorization_flow)
planner.allow_empty_flows = True
planner.use_cache = False
try:
plan = planner.plan(
request,
{
PLAN_CONTEXT_SSO: True,
PLAN_CONTEXT_APPLICATION: self.application,
# OAuth2 related params
PLAN_CONTEXT_DEVICE: self.token,
# Consent related params
PLAN_CONTEXT_CONSENT_HEADER: _("You're about to sign into %(application)s.")
% {"application": self.application.name},
PLAN_CONTEXT_CONSENT_PERMISSIONS: scope_descriptions,
},
)
except FlowNonApplicableException:
LOGGER.warning("Flow not applicable to user")
return None
plan.insert_stage(in_memory_stage(OAuthDeviceCodeFinishStage))
request.session[SESSION_KEY_PLAN] = plan
return redirect_with_qs(
"authentik_core:if-flow",
request.GET,
flow_slug=self.token.provider.authorization_flow.slug,
)
except FlowNonApplicableException:
LOGGER.warning("Flow not applicable to user")
return None
plan.insert_stage(in_memory_stage(OAuthDeviceCodeFinishStage))
request.session[SESSION_KEY_PLAN] = plan
return redirect_with_qs(
"authentik_core:if-flow",
request.GET,
flow_slug=token.provider.authorization_flow.slug,
)


class DeviceEntryView(View):
class DeviceEntryView(PolicyAccessView):
"""View used to initiate the device-code flow, url entered by endusers"""

def dispatch(self, request: HttpRequest) -> HttpResponse:
Expand All @@ -95,7 +101,9 @@ def dispatch(self, request: HttpRequest) -> HttpResponse:
LOGGER.info("Brand has no device code flow configured", brand=brand)
return HttpResponse(status=404)
if QS_KEY_CODE in request.GET:
validation = validate_code(request.GET[QS_KEY_CODE], request)
validation = CodeValidatorView(request.GET[QS_KEY_CODE], request=request).dispatch(
request
)
if validation:
return validation
LOGGER.info("Got code from query parameter but no matching token found")
Expand Down Expand Up @@ -130,6 +138,13 @@ class OAuthDeviceCodeChallengeResponse(ChallengeResponse):
code = IntegerField()
component = CharField(default="ak-provider-oauth2-device-code")

def validate_code(self, code: int) -> HttpResponse | None:
"""Validate code and save the returned http response"""
response = CodeValidatorView(code, request=self.stage.request).dispatch(self.stage.request)
if not response:
raise ValidationError(_("Invalid code"), "invalid")
return response


class OAuthDeviceCodeStage(ChallengeStageView):
"""Flow challenge for users to enter device codes"""
Expand All @@ -145,12 +160,4 @@ def get_challenge(self, *args, **kwargs) -> Challenge:
)

def challenge_valid(self, response: ChallengeResponse) -> HttpResponse:
code = response.validated_data["code"]
validation = validate_code(code, self.request)
if not validation:
response._errors.setdefault("code", [])
response._errors["code"].append(ErrorDetail(_("Invalid code"), "invalid"))
return self.challenge_invalid(response)
# Run cancel to cleanup the current flow
self.executor.cancel()
return validation
return response.validated_data["code"]
23 changes: 23 additions & 0 deletions website/docs/security/CVE-2024-38371.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# CVE-2024-38371

_Reported by Stefan Zwanenburg_

## Insufficient access control for OAuth2 Device Code flow

### Impact

Due to a bug, access restrictions assigned to an application were not checked when using the OAuth2 Device code flow. This could potentially allow users without the correct authorization to get OAuth tokens for an application, and access the application.

### Patches

authentik 2024.6.0, 2024.4.3 and 2024.2.4 fix this issue, for other versions the workaround can be used.

### Workarounds

As authentik flows are still used as part of the OAuth2 Device code flow, it is possible to add access control to the configured flows.

### For more information

If you have any questions or comments about this advisory:

- Email us at [security@goauthentik.io](mailto:security@goauthentik.io)
1 change: 1 addition & 0 deletions website/sidebars.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ const docsSidebar = {
},
items: [
"security/policy",
"security/CVE-2024-38371",
"security/CVE-2024-23647",
"security/CVE-2024-21637",
"security/CVE-2023-48228",
Expand Down

0 comments on commit c9c6a73

Please sign in to comment.