CZ-NIC / django-fido

Django application for FIDO protocol U2F
GNU General Public License v3.0
28 stars 11 forks source link

TypeError: In order to allow non-dict objects to be serialized set the safe parameter to False. #186

Closed mscansian closed 1 year ago

mscansian commented 1 year ago

Hello,

I'm getting an error when accessing the endpoint authentication/request/. I'm not sure if it's something in my code, but from the error and the code it seems that it's an issue with django-fido.

What is the error and why it's related to django-fido?

So the error I'm hitting when accessing the endpoint is this:

TypeError: In order to allow non-dict objects to be serialized set the safe parameter to False.

This error is caused by the django-fido calling JsonResponseand passing a non-dict to be encoded without the safe=True flag.

    def get(self, request: HttpRequest) -> JsonResponse:
        """Return JSON with FIDO 2 request."""
        try:
            request_data, state = self.create_fido2_request()
        except ValueError as error:
            return JsonResponse({
                'error_code': getattr(error, 'error_code', Fido2ServerError.DEFAULT),
                'message': force_str(error),
                'error': force_str(error),  # error key is deprecated and will be removed in the future
            }, status=BAD_REQUEST)

        # Store the state into session
        self.request.session[self.session_key] = state

        return JsonResponse(request_data, encoder=Fido2Encoder)

https://github.com/CZ-NIC/django-fido/blob/master/django_fido/views.py#L164

If we look at Django code, it seems that if we pass anything other than a dict without the flag we will get this exception.

class JsonResponse(HttpResponse):
    def __init__( ... ):
        if safe and not isinstance(data, dict):
            raise TypeError(
                "In order to allow non-dict objects to be serialized set the "
                "safe parameter to False."
            )
        if json_dumps_params is None:
            json_dumps_params = {}
        kwargs.setdefault("content_type", "application/json")
        data = json.dumps(data, cls=encoder, **json_dumps_params)
        super().__init__(content=data, **kwargs)

https://github.com/django/django/blob/004f985b918d5ea36fbed9b050459dd22edaf396/django/http/response.py#L678

When it started happening?

This error started in version 1.0.0 and does not occur in version 0.42. It's easy to see why, since these lines were modified during the updated and we started using a Django JSON encoder. https://github.com/CZ-NIC/django-fido/compare/0.42...1.0.0#diff-918f1e7fc0ca9225c78224e07e38006adf5fedfa0cf3781ef9b5376dd7570239L166

Whats the proposed solution?

I think the django-fido lib is misusing the Django JSON encoder. I would say we set safe=True but I'm not 100% confident we have not a single byte of user input in that variable that could cause a security issue. If would guys help me understand this, I can gladly open a PR.

tpazderka commented 1 year ago

It looks like this is caused by a change in fido2 where the return value from Fido2Server.register_begin changed with version 1.1.0 from a Dict to a Dataclass.

You can probably verify by making sure that the underlying fido2 version is 1.0.0.

And this probably should be fixed by converting the dataclass to dictionary before encoding it.

tpazderka commented 1 year ago

This should be fixed once #182 is done.