Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsolicited Response: KeyError exception when allow_unsolicited = true #330

Open
senorkrabs opened this issue May 28, 2020 · 5 comments
Open
Labels

Comments

@senorkrabs
Copy link

Code Version

latest (6.1.0)

Expected Behavior

I'm working with an SP that does not support sp-initiated requests. Ideally, I would like to use do something like:
SAMLFrontend Idp-initated request (specifying Target SP) > SAMLBackend > Target IdP (shibboleth) > SAMLBackend > SAMLFrontend > SP ACS with RelayState=https://some.target.url

Based on the docs, it doesn't appear that Satosa/pysaml2 have a way of supporting this, so I'm trying to enable unsolicited responses to the SAMLBackend and do this instead:
Target IdP (shibboleth) Idp-initiated request with RelayState=https://some.target.url > SAMLBackend > SAMLFrontEnd > SP ACS with RelayState=https://some.target.url

Note: I'm not sure if using unsolicited responses from the SP will work, since I'll need a way to tell the SAMLFrontend what SP to response to. I'm open to other approaches, but the SP supporting sp-initiated auth is not an option at this time.

Current Behavior

After receiving the assertion from the Target IdP, Satosa throws the following error:

[2020-05-28 14:14:56,291] [ERROR] [satosa.base.run] [urn:uuid:8a8edc2a-7daa-43e6-ae00-83bd82a1b33f] Uncaught exception
Traceback (most recent call last):
  File "/src/satosa/src/satosa/base.py", line 240, in run
    resp = self._run_bound_endpoint(context, spec)
  File "/src/satosa/src/satosa/base.py", line 180, in _run_bound_endpoint
    return spec(context)
  File "/src/satosa/src/satosa/backends/saml2.py", line 340, in authn_response
    if context.state[self.name]["relay_state"] != context.request["RelayState"]:
  File "/usr/lib/python3.7/collections/__init__.py", line 1025, in __getitem__
    raise KeyError(key)
KeyError: 'Saml2'
[2020-05-28 14:14:56,293] [ERROR] [satosa.proxy_server.__call__] Unknown error
Traceback (most recent call last):
  File "/src/satosa/src/satosa/base.py", line 240, in run
    resp = self._run_bound_endpoint(context, spec)
  File "/src/satosa/src/satosa/base.py", line 180, in _run_bound_endpoint
    return spec(context)
  File "/src/satosa/src/satosa/backends/saml2.py", line 340, in authn_response
    if context.state[self.name]["relay_state"] != context.request["RelayState"]:
  File "/usr/lib/python3.7/collections/__init__.py", line 1025, in __getitem__
    raise KeyError(key)
KeyError: 'Saml2'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/src/satosa/src/satosa/proxy_server.py", line 118, in __call__
    resp = self.run(context)
  File "/src/satosa/src/satosa/base.py", line 258, in run
    raise SATOSAUnknownError("Unknown error") from err
satosa.exception.SATOSAUnknownError: Unknown error

Possible Solution

I believe the SAMLBackend is attempting to compare a stored RelayState value that doesn't exist because the response was unsolicited.

Steps to Reproduce

  1. Send an unsolicited response to SAMLBackend
@senorkrabs
Copy link
Author

I took the code from #231 and made a few modifications to the RelayState behavior + logging. It works great for my use case. I also see there were plans to improve the code: #232. Is there any additional background on this?

@peppelinux
Copy link
Member

Hi @senorkrabs, can you share your changes in the code?
The PRs you mentioned was already merged, is it time to close this issue or do we have any possibility to share some more hints on this topic?

@senorkrabs
Copy link
Author

senorkrabs commented Dec 15, 2020

Looking at #232 I thought this was reverted and that actual PR was never merged. I couldn't find it in the source.

I made some minor modifications to the original PR code and put it here: https://github.com/senorkrabs/aws-saml-proxy/blob/master/satosa/frontends/saml2_custom.py. It worked with 6.x but haven't tested in 7.x yet.

Edit: This was needed for the frontend microservice so that I could create idp-initiated requests. If there's another or better way to do this, it would be great to know. I also added allowed_relay_state_urls as a config parameter and logic to validate the relay state: https://github.com/senorkrabs/aws-saml-proxy/blob/master/satosa/config/plugins/frontends/saml2_frontend.yaml.

@peppelinux
Copy link
Member

peppelinux commented Dec 15, 2020

Probably a diff might help

In my environment that problem Is often linked to clients that start sessions directly from the Discovery service. Once the auth happens, when they come back to satosa, this cannot link the response to any previous states... Key error

@domq
Copy link

domq commented Mar 11, 2022

FWIW, we ran into the same issue today and the cause turned out to be that we attempted to use SATOSA over unencrypted HTTP. Since the Set-Cookie header has ;secure; in them, this results in the cookies getting ignored. As a result, every session is new, and SATOSA remembers nothing inbetween...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants