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

SAML: IdP initiated requests do not work #129

Open
pavelnikolov opened this issue Mar 29, 2017 · 3 comments
Open

SAML: IdP initiated requests do not work #129

pavelnikolov opened this issue Mar 29, 2017 · 3 comments

Comments

@pavelnikolov
Copy link

pavelnikolov commented Mar 29, 2017

We use OneLogin for SAML authentication. When an user is authenticated against OneLogin and clicks the Sign in link in Confidant - he/she is automatically authenticated. However, if the user is in the OneLogin dashboard and clicks on the Confidant icon (the authentication is initiated from outside of Confidant) - an error is displayed:

{
  "errors": [
    "invalid_response"
  ], 
  "message": "SAML request failed", 
  "reason": "No AuthNRequest ID from SP found to match with InResponseTo of response"
}

I think that authentication with IdP initiated requests is not currently supported by Confidant. I am not a Python expert but I believe that in order to fix this issue you need to change this piece of code:

        try:
            request_id = session['saml_authn_request_id']
        except KeyError:
            logging.warning('No saml_authn_request_id in session')
            resp = jsonify(errors=['invalid_response'],
                           message='SAML request failed',
                           reason=('No AuthNRequest ID from SP found '
                                   'to match with InResponseTo of response'))
            resp.status_code = 401
            return resp

Instead of always throwing an exception when the request_id is not found, you should first check if the response contains InResponseTo="..." field and only if it does, then throw an exception.

<samlp:Response xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                ID="R6814b187947641b39d27763029820d5527xxxxx"
                Version="2.0"
                IssueInstant="2017-03-29T22:00:09Z"
                Destination="https://<our_host_here>/v1/saml/consume"
                InResponseTo="ONELOGIN_7c4612b6a6361c50f10e2aa2a04f0b5f7dxxxxxx"
                >

The InResponseTo field is not present when the authentication request is initiated from the IdP (e.g. Onelogin).

@ryan-lane
Copy link
Contributor

Hey @pavelnikolov. There's a minor vulnerability opened by doing so, and the underlying library doesn't have support for choosing whether to open or close it, but @vivianho tracked this down and opened an issue with python-saml: SAML-Toolkits/python-saml#188

The vulnerability requires interception, though, so overall we don't feel like this changes the threat model. Based on that assessment, we're going to loosen up the restriction here and we'll update the library in the requirements when the setting becomes available in a release, for those who want to close this.

@ryan-lane
Copy link
Contributor

Sorry, we're digging into this a bit deeper. OWASP recommendation is to never accept an assertion without InResponseTo set to avoid man-in-the-middle, potentially from malicious SPs.

@ryan-lane
Copy link
Contributor

Following up on this, IdP initiated login introduces a login CSRF vulnerability that could be used to log a user in via CSRF, and could also be used to escalate privileges of that user. Escalation of privileges isn't an issue in confidant right now, as we don't have access controls, but we do have plans on adding support for segregated user classes in the future.

Based on that, I'm going to update the attached PR a bit to wrap this around a setting, that will default to false.

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

No branches or pull requests

2 participants